-
Notifications
You must be signed in to change notification settings - Fork 7
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
Sprint 1 poc #14
Sprint 1 poc #14
Conversation
Bridge tokens l1
…contracts into bridge-tokens-L2
Bridge tokens l2
…racts into deployment-scripts
…racts into SMR-1773-axelar-local
Smr 1773 axelar local
SMR-1834-WETH-Bridging
cleanup local axelar scripts for the Demo
if (!Strings.equal(sourceAddress, rootERC20BridgeAdaptor)) { | ||
revert InvalidSourceAddress(); | ||
} | ||
if (data.length == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to check if data.length > 32
here, so that we can throw a meaningful error in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ticket added: https://immutable.atlassian.net/browse/SMR-1914
_deposit(rootToken, receiver, amount); | ||
// invariant check to ensure that the root token balance has increased by the amount deposited | ||
// slither-disable-next-line incorrect-equality | ||
if (rootToken.balanceOf(address(this)) != expectedBalance) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a finding in the last ToB audit that this invariant check could fail for non-standard ERC20 tokens that take a percentage fee. Do we want to account for this? https://github.com/trailofbits/publications/blob/master/reviews/2023-08-immutable-securityreview.pdf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for bringing that up. Discussion started in slack
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
No description provided.