Bill broker fee curve change #237

aalavandhan · 2025-01-07T22:46:30Z

Previously the bill broker swap fee curve was piecewise function with 2 lines. This change makes it a piecewise with 3 lines as shown in the image and now supports negative fees.

We stored fee percentages as a unsigned fixed-point number with 18 decimals (1e18 = 100% or 1.0) which doesn't support negative fee values. Now we simply covert the existing percentages into a factor by adding 1e18. Under the new scheme:
0% => 1.0 or 1e18
100% => 2.0 or 2e18
-20% => 0.8 or 8e17

openzeppelin-code · 2025-01-07T22:50:56Z

Bill broker fee curve change

Generated at commit: 035c546f412eff8dd7d9045831780e5278485fa7

🚨 Report Summary

	Severity Level	Results
Contracts	Critical High Medium Low Note Total	1 0 0 3 23 27
Dependencies	Critical High Medium Low Note Total	0 0 0 0 0 0

For more details view the full report in OpenZeppelin Code Inspector

brandoniles · 2025-01-13T20:57:31Z

spot-vaults/contracts/_utils/LineHelpers.sol

+
+        // Validate range inputs (custom errors omitted here).
+        if (xl > xu) revert InvalidRange();
+        if (xl <= bpl && xu > bpu) revert UnexpectedRangeDelta();


Should this be an error condition? The x-range could span all three line segments on a large enough trade.

This could be a case F

Could be. I just chose to keep the implementation simple in the first iteration. We can add a case F in the next pass.

spot-vaults/contracts/BillBroker.sol

brandoniles · 2025-01-21T23:02:57Z

spot-vaults/contracts/BillBroker.sol

+        // push the pool back into balance.
        uint256 percOfAmtInSwapped = ONE.mulDiv(
-            usdAmtIn - mintAmt.mulDiv(s.usdBalance, totalSupply_),
+            usdAmtIn - (s.usdBalance + usdAmtIn).mulDiv(mintAmt, totalSupply_),


Curious how you derived this. I get...
usdAmtIn * (1 - s.usdBalance / (s.usdBalance + s.perpBalance))

Basically applying the current asset ratio to the incoming USD amount. Maybe they're the same, but it feels more natural to use asset balances rather than LP-notes for some reason.

hmm. You'd have to value the incoming tokens based on both the usd and perp balance (and their current prices) of the reserve. We do that do calculate mintAmt which is valueIn / tvl * totalSupply.

Now with mintAmt, we know what share of the reserve assets the user has a claim on.

usdClaim = mintAmt / totalSupply * (s.usdBalance + usdAmtIn)
percUsdSwapped = (usdAmtIn - usdClaim) / usdAmtIn

I think you can factor out the prices, kind of like how any non-oraclized DEX pools works. Any normal double-sided deposit is an op that doesn't change the asset ratio of the system, so you can just look at token balances.

I don't think so? Prices might have diverged since the last interaction. (fmv can change for example)
And swap isn't a single sided operation though, the following deposit is.

Got it. These two lines from your comment above clicked for me:

usdClaim = mintAmt / totalSupply * (s.usdBalance + usdAmtIn) percUsdSwapped = (usdAmtIn - usdClaim) / usdAmtIn

From there, it's easy to see it in the code. If you have a way to communicate that in the code comments that's great 👍

brandoniles · 2025-01-21T23:09:54Z

spot-vaults/contracts/BillBroker.sol

+                        y1: feeP2U.lower,
                        x2: arHardBound.upper,
-                        y2: (f1.lower + f2.lower - f2.upper)
+                        y2: (feeP2U.lower - feeU2PDelta)


Design note:
If we want these fee curves to "converge" at the hard bounds, then we could change this one line to -feeU2P.upper I think?
We wouldn't have to change any storage slots. I need to think more before I actually advocate for this, though...

yea I'd also need to think about it more

This should nudge the pool more back into balance. There's just a question of is it giving too much away? Hard to say. If we intend to lower the "feeLower" (on both directions), from 2.5% to say 1% anyway, then the impact of this should be marginal.

No strong feelings, but thinking of leaving it as is and revisiting later if need be. Curious to hear what @gemini thinks ..

spot-vaults/contracts/BillBroker.sol

aalavandhan · 2025-01-27T18:49:44Z

Okay, went ahead with the convergent curve implementation.

brandoniles

Just the one comment on MIN & MAX configuration, but looks gtg otherwise 👍

brandoniles · 2025-01-27T19:57:23Z

spot-vaults/contracts/BillBroker.sol

            fees_.usdToPerpSwapFeeFactors.lower > fees_.usdToPerpSwapFeeFactors.upper ||
            fees_.usdToPerpSwapFeeFactors.lower < ONE ||
-            fees_.protocolSwapSharePerc > ONE
+            fees_.usdToPerpSwapFeeFactors.upper > (TWO - MIN_FEE_FACTOR)


Let's just define MAX_FEE_FACTOR as a function of MIN_FEE_FACTOR so it's explicit.

MIN is already opinionated (i.e. -20% rather than -100%), so I don't see any reason why MAX has to be +100%. This would get us to treat each in a consistent way

There are still uses of the existing MAX as a sort of error-condition, by using a fee of 100% as a way to say the trade won't go through. We could keep that, if we want, but call it something else that's more fitting.

* Bump coverallsapp/github-action from 2.3.0 to 2.3.3 (#228) Bumps [coverallsapp/github-action](https://github.com/coverallsapp/github-action) from 2.3.0 to 2.3.3. - [Release notes](https://github.com/coverallsapp/github-action/releases) - [Commits](coverallsapp/github-action@v2.3.0...v2.3.3) --- updated-dependencies: - dependency-name: coverallsapp/github-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * updated cdr pricer (#229) * Deployed v4.1.0 implementations * Bump coverallsapp/github-action from 2.3.3 to 2.3.4 (#230) Bumps [coverallsapp/github-action](https://github.com/coverallsapp/github-action) from 2.3.3 to 2.3.4. - [Release notes](https://github.com/coverallsapp/github-action/releases) - [Commits](coverallsapp/github-action@v2.3.3...v2.3.4) --- updated-dependencies: - dependency-name: coverallsapp/github-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * updated spot subgraph to keep track of vault swap volume * Vault Refactor & Meta Oracle (#224) * removed pricing strategies in favor of meta oracle * Bill broker refactor, using new meta oracle * Charm vault refactor, using new meta oracle * Updated Interfaces and Errors * updated unit tests * code review changes * Scalable usdc spot strategy (#231) Apply suggestions from code review Co-authored-by: Brandon Iles <brandon@fragments.org> review fix caching bool instead of prev deviation * billy subgraph (#234) * Deployed new oracle contract and managers (#233) * deployed new oracle and managers * updated readme * Staking subgraph (#235) * staking subgraph, refreshing store on rebase * charm wampl vault subgraph * adds timestamp to swap events subgraph (#236) * adds timestamp to swap events subgraph Signed-off-by: nms-7 <nixon.ubud@gmail.com> * linting Signed-off-by: nms-7 <nixon.ubud@gmail.com> * fixed null swap creation with 0 timestamp Signed-off-by: nms-7 <nixon.ubud@gmail.com> --------- Signed-off-by: nms-7 <nixon.ubud@gmail.com> * Bump coverallsapp/github-action from 2.3.4 to 2.3.6 (#238) Bumps [coverallsapp/github-action](https://github.com/coverallsapp/github-action) from 2.3.4 to 2.3.6. - [Release notes](https://github.com/coverallsapp/github-action/releases) - [Commits](coverallsapp/github-action@v2.3.4...v2.3.6) --- updated-dependencies: - dependency-name: coverallsapp/github-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bill broker fee curve change (#237) * using fee factor instead of percentages to support -ve fees * unit tests * code review * convergent fee curve * updated charm fee yield logic (#239) * Fee curve update (#232) * fee curve update * code review fixes * Rollover fee curve update (#240) * rollover fee handles undersubscribed vault * bumped up hardhat * code review comments * DR based flash swap fees * comment updates * updated unit tests to use ethers v6, using custom mocking library * updated tasks and deps * updated perp mint fees to be paid to the vault * removed perp share of fees from flash swaps, all fees go to the vault * added configurable dr soft bounds (curve cutoff point) to flash swap fees * updated unit tests * removed rollover fees from perp * Daily Rebalance * updated unit tests * constant rate enrichment/debasement * mint2, redeem2 * added linked library to limit contract size * unit tests * melding perps immediately after rebalance * Apply suggestions from code review Suggestions Co-authored-by: Brandon Iles <brandon@fragments.org> * Update spot-contracts/contracts/FeePolicy.sol Co-authored-by: Brandon Iles <brandon@fragments.org> * code review fixes * code review fixes v2 * code review fixes v3 * code review v4 * during rebalance, vault pays perps by transferring tranches into perp's reserve * rebalance eql * configurable rebalance freq * code review v5 * Apply suggestions from code review Co-authored-by: Brandon Iles <brandon@fragments.org> * code review fixes * Code review fixes Co-authored-by: Brandon Iles <brandon@fragments.org> * bill broker single sided deposit fix (#253) * Dev Audit Changes (#252) * fee policy restructure * updated perp fee and rebalance handling, audit cleanup * updated vault fee and rebalance handling, audit cleanup * interface and lib updates * updated unit tests * code review fixes #1 * code review fixes #2 * code review fix #3 * Removed subscription ratio for a global vault:perp tvl (or system ratio) (#254) * Using a system ratio target * using system state to calculate fees * using system state for fees and removed subscription liquidity check * interface updates * unit test updates * rename * updated deps --------- Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: nms-7 <nixon.ubud@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: nms-7 <57442379+nms-7@users.noreply.github.com> Co-authored-by: Brandon Iles <brandon@fragments.org>

aalavandhan added 2 commits January 7, 2025 17:33

using fee factor instead of percentages to support -ve fees

83f3d39

unit tests

87549f0

aalavandhan requested review from brandoniles and nms-7 January 7, 2025 22:46

brandoniles reviewed Jan 13, 2025

View reviewed changes

brandoniles reviewed Jan 17, 2025

View reviewed changes

spot-vaults/contracts/BillBroker.sol Show resolved Hide resolved

spot-vaults/contracts/BillBroker.sol Outdated Show resolved Hide resolved

spot-vaults/contracts/BillBroker.sol Outdated Show resolved Hide resolved

spot-vaults/contracts/BillBroker.sol Outdated Show resolved Hide resolved

aalavandhan force-pushed the bb-fee-change branch from 21cc6e1 to 48c09ce Compare January 21, 2025 18:27

brandoniles reviewed Jan 21, 2025

View reviewed changes

code review

c906f4b

aalavandhan force-pushed the bb-fee-change branch from 48c09ce to c906f4b Compare January 21, 2025 23:40

aalavandhan force-pushed the bb-fee-change branch from 79680ec to 969078b Compare January 27, 2025 19:25

brandoniles reviewed Jan 27, 2025

View reviewed changes

convergent fee curve

035c546

aalavandhan force-pushed the bb-fee-change branch from 969078b to 035c546 Compare January 27, 2025 23:39

brandoniles approved these changes Jan 28, 2025

View reviewed changes

aalavandhan merged commit 6c6311e into main Jan 28, 2025
1 check passed

aalavandhan deleted the bb-fee-change branch January 28, 2025 22:26

Bill broker fee curve change #237

Bill broker fee curve change #237

Uh oh!

Conversation

aalavandhan commented Jan 7, 2025

Uh oh!

openzeppelin-code bot commented Jan 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bill broker fee curve change

🚨 Report Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

aalavandhan Jan 22, 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!

aalavandhan Jan 24, 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!

Uh oh!

aalavandhan commented Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brandoniles left a comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

openzeppelin-code bot commented Jan 7, 2025 •

edited

Loading

aalavandhan Jan 22, 2025 •

edited

Loading

aalavandhan Jan 24, 2025 •

edited

Loading

aalavandhan commented Jan 27, 2025 •

edited

Loading