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

C4: #35 #32, fixes rETH / cbETH / ankrETH ref unit and adds soft default checks #899

Merged
merged 12 commits into from Aug 22, 2023

Conversation

jankjr
Copy link
Contributor

@jankjr jankjr commented Aug 16, 2023

CBEth: Uses oracle rather contract method to convert from tok to ref as outlined in raised report.

RETH: Same modification made as RETH is more or less implemented the same way.

Copy link
Contributor

@tbrent tbrent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CBEth needs class-level documentation like rETH, as well

contracts/plugins/assets/cbeth/CBETHCollateral.sol Outdated Show resolved Hide resolved
contracts/plugins/assets/rocket-eth/RethCollateral.sol Outdated Show resolved Hide resolved
@tbrent
Copy link
Contributor

tbrent commented Aug 21, 2023

So the way out of this confusion was to get the units right. The 2nd feeds should be targetPerTok feeds instead of refPerTok, because the ref unit is ETH2. This suddenly clears everything up and makes it super obvious, and leads to, for example, catching that the pegPrice should be the reciprocal of what it was.

@tbrent
Copy link
Contributor

tbrent commented Aug 21, 2023

@akshatmittal tagging you in on this one. believe it is correct now

@tbrent tbrent requested review from pmckelvy1 and removed request for akshatmittal August 21, 2023 22:54
@tbrent
Copy link
Contributor

tbrent commented Aug 21, 2023

err -- @akshatmittal I'll save you some time since you're reviewing other PRs too, tagging in @pmckelvy1

@tbrent
Copy link
Contributor

tbrent commented Aug 21, 2023

contracts/plugins/assets/cbeth/README.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@tbrent tbrent changed the title C4: #35 #32, fixes RETH and CBETH ref unit exhange C4: #35 #32, fixes rETH / cbETH / ankrETH ref unit and adds soft default checks Aug 22, 2023
@tbrent
Copy link
Contributor

tbrent commented Aug 22, 2023

@tbrent tbrent merged commit 19d84ef into 3.0.0-rc6 Aug 22, 2023
8 checks passed
@tbrent tbrent deleted the audit/cb-eth-units branch August 22, 2023 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants