livepeer / protocol

Livepeer protocol
MIT License
154 stars 45 forks source link

proxy: Add receive() to ManagerProxy #538

Closed yondonfu closed 2 years ago

yondonfu commented 2 years ago

What does this pull request do? Explain your changes. (required)

Add a payable receive() function to ManagerProxy to allow proxies to accept ETH via vanilla transfers (i.e. no msg.data) only if the target implementation contract implements receive() as well (since the ManagerProxy receive() will trigger a delegatecall with empty msg.data on the target implementation). An example of a contract that needs to accept ETH via vanilla transfers is the L2Migrator.

Prior to this change, a ManagerProxy would be able to accept regular ETH transfers when there is no msg.data if the target implementation contract implements a fallback (either receive() or fallback() with the latter being invoked if receive() is not implemented). However, this left room for interface confusion - if a ManagerProxy successfully accepted a regular ETH transfer was it because of receive() or fallback() being called on the target implementation? This change makes the call path clearer - if a vanilla ETH transfer is accepted receive() was triggered on the target implementation and if a complex ETH transfer (i.e. msg.data is included) is accepted fallback() was triggered on the target implementation.

So, there aren't any functional changes in this PR, but the changes establish more clarity with call paths.

https://github.com/livepeer/arbitrum-lpt-bridge/blob/main/contracts/proxy/ManagerProxy.sol will need to be updated in arbitrum-lpt-bridge after this PR as well.

Specific updates (required)

See commit history.

How did you test each of these updates (required)

Added a test case for ManagerProxy receiving ETH.

Does this pull request close any open issues?

N/A

Checklist: