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

QA Report #556

Open
howlbot-integration bot opened this issue May 10, 2024 · 8 comments
Open

QA Report #556

howlbot-integration bot opened this issue May 10, 2024 · 8 comments
Labels
bug Something isn't working edited-by-warden grade-b Q-10 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sufficient quality report This report is of sufficient quality

Comments

@howlbot-integration
Copy link

See the markdown file with the details of this report here.

@howlbot-integration howlbot-integration bot added bug Something isn't working edited-by-warden QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sufficient quality report This report is of sufficient quality labels May 10, 2024
howlbot-integration bot added a commit that referenced this issue May 10, 2024
@CloudEllie
Copy link
Contributor

@c4-judge
Copy link
Contributor

alcueca marked the issue as grade-b

@Slavchew
Copy link

Hey @alcueca,

Sorry for writing after PJQA, but that's because there are issues that were flagged as valid after it, which are also in our QA report.

Please review them again and upgrade, if think so.

[L-05] is a dup of #282
[L-06] is a possible dup of #13

Sorry one more time.

@Slavchew
Copy link

Slavchew commented May 28, 2024

Hey @alcueca,

Sorry for writing again, but why isn't [L-06] a dup of #13 and #424, it is 1:1 the same issue.

Also [L-03] it is the same as #534.

Please review it again and upgrade, if think so.

@alcueca
Copy link

alcueca commented May 28, 2024

#13 refers to the fact that there are different sources of price data for stETH, which are market data (from DEXs) and exchange data (from Lido contracts). L-06 is about the long heartbeat of STETH/ETH, which has been declared invalid elsewhere. #424 is about arbitrage due to implying a 1:1 price to several different collaterals, which in fact diverge. Nothing to do with hearbeat.

L-03 upgraded to be a duplicate of #534

@Slavchew
Copy link

What about #427, #428, they are very similar to our, it may not be fully duplicate, but it is at least partial-75.

@Slavchew
Copy link

@alcueca Here is my final explanation of the situation about L-06.

The issue describes very well that arbitrage opportunities can happen based on the stETH/ETH price feed heartbeat and divination.

The exact same is stated in these reports #429, #428, #427, #424 and #426, which even talk about tokens that will not be used in Renzo.

They are all considered duplicates of #13 and they all talk about a 0.5% price deviation.

Here is what we stated in L-06

Renzo protocol relies on stETH/ETH price feed to convert stETH when used for deposit, withdrawal, etc. But this price feed has too long heartbeat - 24 hours, which can open arbitrage opportunities if the price is not updated accurately. The 24-hour heartbeat and 0.5% deviation threshold means that price can move up to 0.5% or stay flat for 24 hours before triggering a price update, which is unlikely to be reached, but historically it is not impossible, you can check this graph for example. This allows you to deposit at these times to mint more ezETH or withdraw at a better rate.

Here is what is stated in #428

The value of stETH deposited into the protocol is priced using chainlink oracle. The current deviation and heartbeat of stETH/ETH are 0.5% and 86400s. This means stETH price will not be updated it the price doesn't deviate 0.5% in 24 hours. This difference between market price and oracle price can be used to perform arbitrage. The 0.5% deviation might seem less here. But attacker can front run oracle updates to exploit the above scenario which have more deviation thus more profit.

and in #426

The protocol relies solely on Chainlink oracles to get the prices of the collateral deposited into the protocol.
The issue is that different LST have different heartbeats and deviation threshold. For example, a deviation threshold of 2% means the feed price can be off by up to 2% as compared to the real tracked price.

Chainlink's STETH/ETH has a heartbeat of 24 hours and a deviation threshold of 0.5%.

RETH/ETH has a heartbeat of 24 hours and a deviation threshold of 2%.

cbETH/ETH has a heartbeat of 24 hours and a deviation threshold of 1%.

These collateral are also updated at different times of the day.

In Renzo, users can deposit one collateral token and immediately call withdraw and exchange their ezETH for another collateral token.

Copied reasoning from similar issue:

If that doesn't show that the L-06 is a valid duplicate of them, I don't know what will.

@Bauchibred
Copy link

To add to the above comment @alcueca correct me if I'm wrong, but I believe you're now finalizing all the findings suggesting how the feeds being used would lead to arbitrage under the umbrella of #13.

I'd like to indicate that the sponsor also acknowledged #8 and it's duplicates just like they did for all the other satisfactory arbitrage findings currently under #13, albeit in the case of #8 it's on the ground of how using this feed with a long duration would allow for an arbitrage on deposits, quoting them from here:

... On the other hand, yes there can be minor arbitrage opportunities in stETH deposit, which is an expected behaviour in the protocol. therefore, acknowledging for arbitrage behaviour.

Not to make this comment too long, but to summarize: I believe if you're indeed grouping all the findings that showcase a better implementation for the protocol to curb "feed arbitrage" under #13, then L-06 from here, L-17 from #539, #8, and its duplicates should also be considered under this umbrella. Or maybe they should be treated as partial duplications if you still consider them not direct duplicates. However, if my assessment is wrong about the umbrella under which you're grouping them, then please ignore this comment. Thank you for your time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working edited-by-warden grade-b Q-10 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

6 participants