-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
link transfer status #13177
link transfer status #13177
Conversation
Quality Gate passedIssues Measures |
@@ -78,7 +78,10 @@ contract AutomationRegistryLogicC2_3 is AutomationRegistryBase2_3 { | |||
uint96 balance = _updateTransmitterBalanceFromPool(from, s_hotVars.totalPremium, uint96(s_transmittersList.length)); | |||
s_transmitters[from].balance = 0; | |||
s_reserveAmounts[IERC20(address(i_link))] = s_reserveAmounts[IERC20(address(i_link))] - balance; | |||
i_link.transfer(to, balance); | |||
bool transferStatus = i_link.transfer(to, balance); |
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.
why it won't fail in CI bc of you did not generate wrappers?
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.
hmm, can you elaborate on this?
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.
bc the contract is changed, don't you need to regenerate Go wrappers?
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.
wow this is super interesting. My guess is that because the link token always returns true
, the optimizer considers this a "dead path" and essentially removes it, making the current bytecode match the old bytecode.
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.
I ran wrappers generation locally, and this change doesnt change any wrappers. Correct me if I need other steps..
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.
If LINk never returns false, do we still need to check the status? our automation codebase has the tradition of checking the status :)
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.
@RyanRHall @FelixFan1992 If no objection, I will abandon this PR since checking the bool return is not needed for LINK.
No description provided.