Skip to content
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

Clarify Checks-Effects-Interactions pattern #208

Closed
BrendanChou opened this issue Apr 3, 2019 · 2 comments

Comments

@BrendanChou
Copy link
Contributor

commented Apr 3, 2019

Not following the Checks-Effects-Interactions Pattern
Solidity recommends the usage of the Check-Effects-Interaction Pattern to avoid potential problems, like reentrancy.

In a couple of places the code of Solo the checks are not done first:

The invalid oracle price check in _setPriceOracle.
The primary account check in _verifyFinalState.
While in these cases there is no risk of reentrancy, consider moving the checks to the start of the corresponding block to make sure that no issues are introduced by later changes.

@BrendanChou BrendanChou added the P1 label Apr 3, 2019

@BrendanChou BrendanChou self-assigned this Apr 3, 2019

@BrendanChou

This comment has been minimized.

Copy link
Contributor Author

commented Apr 3, 2019

Add admin events #193 fixed this issue in AdminImpl._setPriceOracle() and AdminImpl._setInterestRate()

@BrendanChou

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

Not planning on fixing this issue for OperationImpl._verifyFinalState()

@BrendanChou BrendanChou closed this Apr 4, 2019

@BrendanChou BrendanChou changed the title Clarify Checks-Effects-Interactions pattern [Zepp] Clarify Checks-Effects-Interactions pattern Apr 5, 2019

@BrendanChou BrendanChou changed the title [Zepp] Clarify Checks-Effects-Interactions pattern Clarify Checks-Effects-Interactions pattern Apr 5, 2019

@BrendanChou BrendanChou added the zeppelin label Apr 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.