Skip to content

Conversation

@aalavandhan
Copy link
Member

Added a new SPOT pricing strategy for the charm manager.

The bill broker uses the spot appraiser which has stricter rules around DR and tranche CDRs. However, the AMM pool can use a much simpler pricing strategy.

@openzeppelin-code
Copy link

openzeppelin-code bot commented Aug 9, 2024

Spot pricer for automanager

Generated at commit: e6120e31411cfd5bbc20bd630bba54ecc7f33407

🚨 Report Summary

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

For more details view the full report in OpenZeppelin Code Inspector

@aalavandhan aalavandhan force-pushed the spot-pricer branch 3 times, most recently from 3cb469e to 2b7d9ec Compare August 9, 2024 18:15
// If the market price of the USD coin fallen too much below 1$,
// it's an indication of some systemic issue with the USD token
// and thus its price should be considered unreliable.
return (ONE, (v && p > USD_LOWER_BOUND));
Copy link
Member

Choose a reason for hiding this comment

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

WDYT about checking deviation USDC from $1.00, which also covers the case where the dollar token is >> $1? That's also a sign of market trouble, that's missed by just checking the downside.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that's specifically a risk when it comes to USDC. If usdc is trading above one, folks will just mint more? Custody/Bank failure risk is only on the downside.

Copy link
Member

@brandoniles brandoniles Aug 14, 2024

Choose a reason for hiding this comment

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

I'm more coming from the perspective of-- IF the price of USDC is high, then something is going wrong.

Perhaps minting is halted by Circle corp (there are humans in the loop there), a liquidity pool address gets blacklisted through their AML system, there's an chainlink oracle attack/failure. Maybe other things I can't think of as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. Worth noting, that the current spot appraiser which is deployed and used by Billy doesn't have this check.

Copy link
Member

@brandoniles brandoniles left a comment

Choose a reason for hiding this comment

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

LGTM, with the slight name change

uint256 private constant ONE = (10 ** DECIMALS);
uint256 public constant CL_ORACLE_DECIMALS = 8;
uint256 public constant CL_ORACLE_STALENESS_THRESHOLD_SEC = 3600 * 48; // 2 days
uint256 public constant USD_LOWER_UPPER = (101 * ONE) / 100; // 1.01$
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
uint256 public constant USD_LOWER_UPPER = (101 * ONE) / 100; // 1.01$
uint256 public constant USD_UPPER_BOUND = (101 * ONE) / 100; // 1.01$

// it's an indication of some systemic issue with the USD token
// and thus its price should be considered unreliable.
return (ONE, (v && p > USD_LOWER_BOUND));
return (ONE, (v && p < USD_LOWER_UPPER && p > USD_LOWER_BOUND));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return (ONE, (v && p < USD_LOWER_UPPER && p > USD_LOWER_BOUND));
return (ONE, (v && p < USD_UPPER_BOUND && p > USD_LOWER_BOUND));

@aalavandhan aalavandhan merged commit 6aee5a6 into main Aug 17, 2024
@aalavandhan aalavandhan deleted the spot-pricer branch August 17, 2024 18:48
@aalavandhan aalavandhan restored the spot-pricer branch September 18, 2024 00:21
@aalavandhan aalavandhan deleted the spot-pricer branch September 18, 2024 00:21
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.

4 participants