WOETH yield buckets #2447

sparrowDom · 2025-03-17T22:16:52Z

This is the third attempt at the implementation of limiting the yield increase and, consequen the exchange rate between WOETH and OETH. The other 2:

This approach:

does the internal accounting of credits the same way WOETH: Donation attack prevention #2106 does it
the WOETH exchange rate is in sync with OETH yield as long as it doesn't surpass the 91% yearly yield rate. In such case the yield is delayed and will be distributed over the next yield period. Which lasts 23 hours
similarly to WOETH - Fixed yield rate each day #2421 this PR requires interaction with the contract in order to increase the exchange rate / start new yield period by calling increaseYieldLimit()

To be completed before internal review begins:

The contract code is complete
Executable deployment file
Fork tests that test after the deployment file runs
Unit tests *if needed
The owner has done a full checklist review of the code + tests

Internal review:

Two approvals by internal reviewers

…oken can affect the exchange ratio of WOETH to OETH

github-actions · 2025-03-17T22:17:22Z

	Warnings
⚠️	👀 This PR needs at least 2 reviewers

Generated by 🚫 dangerJS against b68269d

codecov · 2025-03-17T22:23:00Z

Codecov Report

Attention: Patch coverage is 84.21053% with 3 lines in your changes missing coverage. Please review.

Please upload report for BASE (sparrowDom/woeth_hack_proof@79af39f). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
contracts/contracts/token/WOETH.sol	84.21%	3 Missing ⚠️

Additional details and impacted files

@@                      Coverage Diff                       @@
##             sparrowDom/woeth_hack_proof    #2447   +/-   ##
==============================================================
  Coverage                               ?   47.22%           
==============================================================
  Files                                  ?      100           
  Lines                                  ?     4898           
  Branches                               ?     1294           
==============================================================
  Hits                                   ?     2313           
  Misses                                 ?     2581           
  Partials                               ?        4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

contracts/contracts/token/WOETH.sol

DanielVF · 2025-03-18T13:27:45Z

contracts/contracts/token/WOETH.sol

+        uint256 amount
+    ) internal override {
+        super._transfer(sender, recipient, amount);
+        increaseYieldLimit();


I'm not sure if this is a problem, but I have vague dread about updating this in the middle of a transfer.

I don't think that an external integration would expect the value of WOETH to change before and after a transfer. Might cause exploitable bugs in other people's code.

In the asset drip implementation, updating things was okay, since any changes only took effect on the next block, and this everyone using the system had a consistent exchange rate for the entire block. I don't know that we have to have one exchange rate per block, but changing during a transfer feels sketch.

Hmm that's a good point. One might expect the following:

woeth.balanceOf(micah) // 5.0 units woeth.connect(micah).transfer(somewhere, 3 units) woeth.balanceOf(micah) // can be more than 2.0 units because of the `increaseYieldLimit`

remove here: 4c4a2cb

contracts/contracts/token/WOETH.sol

DanielVF · 2025-03-18T13:36:06Z

contracts/contracts/token/WOETH.sol

+     * are in sync with WOETH. Resulting in no economical gain from wrapping OETH to WOETH
+     * and back to try to game the yield distribution.
+     */
+    uint256 public constant MAX_YIELD_INCREASE = 25e14; // 0.25%


We are the blessed summer children who have never lived hyper-inflation.

But if the underlying currency is inflating faster than this limit allows, then our users will be in deep trouble. I think we just want to head off donations that damage lending platforms, rather than bet our user funds on stable economies for the next fifty years.

So I'd run this limit something more like 3-5%.

One really cool thing about this code's approach is that in the good case, with high limits, and non-attack yield amounts, even if the limit is not reset, it continues perfectly tracking OETH's value for quite a while after the end time is over.

An attacker can basically get double the yield that this number says, by donating when a period has ended. That's probably okay, but means that with this approach, we should set the yield to half of what we want.

That is a very good point.

DanielVF · 2025-03-18T13:42:55Z

contracts/contracts/token/WOETH.sol

+     */
+    function increaseYieldLimit() public {
+        // not yet time to increase the limit
+        if (block.timestamp < cptLimitEndTime) {


I am wondering if there should be a way where we can cleanly allow pushing forward the end time with every action, rather than requiring one limit to end before the next begins. This would be much nicer user facing behavior. We do have to do some math to see how much of the partially completed phase to carry over. Or maybe this is unnecessary complication.

The current approach around limits is:

very simple

correctly handles the normal case use of the protocol

correctly blocks donation attacks

But it is also feels like it has a lot weirdness when the limits are hit.

2x the limit is possible at the end of the period

someone can get doubled yielded at the end of a period

ratios can change mid block, separate from OETH ratios changing

"someone can get doubled yielded at the end of a period"

Are you thinking that a person would be holding OETH, and knowing WOETH needs time to catch up, they would wrap their OETH just after a fat rebase?

"ratios can change mid block, separate from OETH ratios changing"

Do you think this can have negative consequences?

So this is talking about behavior when the limits will be hit, and the time is past the end:

User knows/causes a BIG rebase to be about to occur.

They have/get a large OETH position.

The call rebase(), collecting the OETH yield

They then deposit into WEOTH, at a privileged exchange rate.

The end of the deposit calls increaseYield. Limits are raised again

User withdraws, collecting OETH at the new higher rate. No one else was able to do this.

I'm actually concerned that is a full on attack, not just a yield steal. Minting funds at the wrong rate could be bad. I'm not sure it is, so I'm going to write a test.

I think it's just a yield steal, since the someone can't mint at a lower rate than the existing users got in at.

"ratios can change mid block, separate from OETH ratios changing"

Do you think this can have negative consequences?

Yes, it allows someone to selectively take actions between these two. Presumably yield stealing.

Ouch!

Confirmed with tests that this is a flashloanable enabled yield steal in the current implementation.

🟥 But it's actually much worse. Stuff does get really out of wack when things are at over the rate limit, and interacting users can actually end up with less funds than they started with depending on how they interact. It gets deeply squirrelly.

DanielVF · 2025-03-18T20:05:49Z

contracts/contracts/token/WOETH.sol

@@ -65,7 +85,7 @@ contract WOETH is ERC4626, Governable, Initializable {
         * WOETH has already seen transactions. But it is rather annoying in unit test
         * environment.
         */
-        oethCreditsHighres = _getOETHCredits();


Still need the oethCreditsHighres = _getOETHCredits();

uff thanks nice catch: b68269d

…saction

sparrowDom · 2025-04-10T08:07:19Z

closing in favour of this one: #2453

limit the yield increase by limiting the speed at which credits per t…

49ff10e

…oken can affect the exchange ratio of WOETH to OETH

sparrowDom requested review from DanielVF, naddison36, shahthepro and clement-ux as code owners March 17, 2025 22:16

sparrowDom changed the base branch from master to sparrowDom/woeth_hack_proof March 17, 2025 22:17

DanielVF reviewed Mar 18, 2025

View reviewed changes

contracts/contracts/token/WOETH.sol Outdated Show resolved Hide resolved

DanielVF reviewed Mar 18, 2025

View reviewed changes

contracts/contracts/token/WOETH.sol Outdated Show resolved Hide resolved

DanielVF reviewed Mar 18, 2025

View reviewed changes

contracts/contracts/token/WOETH.sol Show resolved Hide resolved

DanielVF reviewed Mar 18, 2025

View reviewed changes

sparrowDom added 3 commits March 18, 2025 15:14

change error message

9da2721

simplify code

821a756

increase the precision when dividing

6c255b7

DanielVF reviewed Mar 18, 2025

View reviewed changes

sparrowDom added 2 commits March 18, 2025 23:26

remove the possibility to increase yield as a result of transfer tran…

4c4a2cb

…saction

set the oethDCreditsHighres on initialize

b68269d

sparrowDom closed this Apr 10, 2025

WOETH yield buckets #2447

WOETH yield buckets #2447

Uh oh!

Conversation

sparrowDom commented Mar 17, 2025

Uh oh!

github-actions bot commented Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Mar 17, 2025

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DanielVF Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DanielVF Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DanielVF Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DanielVF Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sparrowDom commented Apr 10, 2025

Uh oh!

Uh oh!

github-actions bot commented Mar 17, 2025 •

edited

Loading

DanielVF Mar 18, 2025 •

edited

Loading

DanielVF Mar 18, 2025 •

edited

Loading

DanielVF Mar 18, 2025 •

edited

Loading

DanielVF Mar 18, 2025 •

edited

Loading