refactor(contracts): internal M0 and Sherlock audit changes#55
refactor(contracts): internal M0 and Sherlock audit changes#55johnletey merged 19 commits intonoble-assets:contractsfrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
dcfc6b8 to
f1170e4
Compare
…l minting after yield has been received, max uint256 minting scenario
f1170e4 to
6e94ce3
Compare
| // NOTE: We don't want to perform any principal updates in the case of yield accrual. | ||
| if (to == address(this)) { | ||
|
|
||
| if ($.totalPrincipal == 0) return; |
There was a problem hiding this comment.
I have doubts this is needed, getIndexRoundedDown should revert with DivisionByZero
There was a problem hiding this comment.
There was a problem hiding this comment.
If we were to revert, we'd be doing this on a cross chain message. In that case, what would happen to the yield the message intended to send? By returning instead, there is no ambiguity here, the yield is received by the contract, and the next time there is principal present to avoid a division by zero, the index is still updated to the correct value.
Maybe this would not be a problem given how Hyperlane works, but I chose this as I am not 100% familiar with that - would be good to know that, though.
There was a problem hiding this comment.
fair take, worth adding comment for auditors, just in case
…ine with dependencies
Sherlock 5 - DOS of claim
changed NobleDollar.sol pragma to be 0.8.22 to allow compilation in line with dependencies
included word on rounding behavior in readme
johnletey
left a comment
There was a problem hiding this comment.
Thanks for your work on this @mesozoic-technology! Left two comments!
No description provided.