Skip to content

feat: disallow deposits with output token set to 0x0 #1031

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mrice32
Copy link
Contributor

@mrice32 mrice32 commented Jun 5, 2025

No description provided.

@mrice32 mrice32 marked this pull request as ready for review June 5, 2025 06:35
bmzig
bmzig previously approved these changes Jun 5, 2025
After the removal of the _deposit function from the SpokePool contract, the InvalidRelayerFeePct and MaxTransferSizeExceeded errors are no longer in use.

Consider removing the unused errors.
* [L-13] Function Selectors On Deprecated Functions Are Not Locked

Pull request 1031 removes already announced deprecated functionalities, such as the depositDeprecated_5947912356 and depositFor functions, alongside their _deposit internal function.

However, as these entry points are removed, future versions of the codebase might introduce new functions that could have the same function selector as the removed ones. In such case, a protocol that might have used the deprecated functionalities could now call to the new ones with unexpected outcomes.

Similarly, if a fallback function is used in the future, depending on its implementation, it might take the calldata of protocols using the old deprecated functionalities with similar results.

In order to prevent the reuse of the deprecated function selectors, consider keeping the public functions' declaration, without their original definition, and reverting the calls to them.

* Add note about selector collision with original deposit function
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.

2 participants