-
Notifications
You must be signed in to change notification settings - Fork 17
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
SD Collateral Deposit Using SD Reward #208
SD Collateral Deposit Using SD Reward #208
Conversation
if (!IERC20(staderConfig.getStaderToken()).transferFrom(msg.sender, address(this), _sdAmount)) { | ||
revert SDTransferFailed(); | ||
} | ||
operatorSDBalance[_operator] += _sdAmount; |
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.
Although not required here. But it is recommended to follow check-effects-interaction
security pattern.
i.e. move line 64 to line 61
https://fravoll.github.io/solidity-patterns/checks_effects_interactions.html
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 think it follows this pattern, you do not want to increase operatorSDBalance before you get SD in the contract, the main aim of check-effects-interaction is if your interaction calls get hacked then your storage should not be exploited, if we move line 64 to 61 then in case tranferFrom does not work properly your operatorSDBalance will keep on increasing.
@dulguun-staderlabs would love to hear your thoughts here
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.
transactions are atomic, hence whole txn will revert if transferFrom
fails.
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
Below are the changes across SDCollateral.sol and SocializingPool.sol