Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Proxy): remove unneeded fallback function #5

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

sambacha
Copy link
Contributor

https://forum.openzeppelin.com/t/proxy-sol-fallback/36951/8

The fallback alone would indeed be enough.

Fallback is not limited to msg.value == 0 (if its marked payable). Both functions can support value.

The difference between receive and fallback is in the msg.data. If the calldata is empty and if there is a receive function, it fill be used. Otherwise, fallback is used. This means that regardless of the value, fallback will be called if there is some data. fallback is also the one that is called if there is no data, but receive is not defined.

So why do we have a receive function that is not really needed? To silent solidity warnings that sometimes happen when you have a fallback function but no receive function.

see OpenZeppelin/openzeppelin-contracts#4434 (comment)

https://forum.openzeppelin.com/t/proxy-sol-fallback/36951/8


The fallback alone would indeed be enough.

 Fallback is not limited to` msg.value == 0` (if its marked `payable`). Both functions can support value.

The difference between receive and fallback is in the msg.data. If the calldata is empty and if there is a receive function, it fill be used. Otherwise, fallback is used. This means that regardless of the value, fallback will be called if there is some data. `fallback` is also the one that is called if there is no data, but receive is not defined.

So why do we have a receive function that is not really needed? To silent solidity warnings that sometimes happen when you have a fallback function but no receive function.
- @Amxx 

see <OpenZeppelin/openzeppelin-contracts#4434 (comment)>
@sambacha sambacha merged commit a16f20b into oz-20230716 Sep 27, 2023
@sambacha sambacha deleted the remove-fallback branch September 27, 2023 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant