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

TWAP Oracle needs to determine peg (track $1.00 USD accurately) #259

Closed
0x4007 opened this issue Sep 14, 2022 · 41 comments
Closed

TWAP Oracle needs to determine peg (track $1.00 USD accurately) #259

0x4007 opened this issue Sep 14, 2022 · 41 comments
Labels
Research Bounties that may include a report only, without code. Solidity Solidity development work is expected.

Comments

@0x4007
Copy link
Member

0x4007 commented Sep 14, 2022

This needs more research but currently our TWAP oracle tracks 3crv, which currently is trading ~$1.02. The net effect of this is that our dollar thinks that our peg is ~$1.02 (1:1 match)

The consult method in the oracle contract needs to be updated to actually track $1.00. Off hand and without research, there's several approaches we can take for this:

  1. We can based off of ether's price (chainlink oracles?)
  2. We can off of another stablecoin (centralized is bad, and DAI might do a freefloating peg)
  3. Some more complex math perhaps grabbing some sort of average of the underlying assets in 3crv
  4. Check all the curve/uniswap pools on chain and grab the median price of stablecoin pairs. I'm betting that most pairs on curve are dollar pegged so this could work. Best if we can do this without gas (reading from the chain should be free)

The method lives here:

https://github.com/ubiquity/ubiquity-dollar/blob/enhancement/styles/packages/contracts/dollar/contracts/TWAPOracle.sol#L70-L81

Also worth noting but 3pool is starting to deviate quite a bit from $1.00 due to the UST bank run (it accrues swap fees in the 3pool) we're going to need to account for this in our core contracts 😰 https://www.coingecko.com/en/coins/lp-3pool-curve

Originally posted by @pavlovcik in #161 (comment)

@0x4007 0x4007 changed the title TWAP Oracle needs to track USDC(?) TWAP Oracle needs to determine peg (track $1.00 USD accurately) Sep 14, 2022
@hashedMae
Copy link
Contributor

I would be able to take this on.

Approach 1
If wanting to use chainlink oracles I would use the ETH/DAI, ETH/USDC, and ETH/USDT feeds to average their prices against each other. Using the DAI/USD, USDC/USD, and USDT/USD feeds would be an alternative option.

Approach 2
Simple solution but then you're relying on another token to maintain peg.

Approach 3
I'm not sure how I would do this without replicating the current problem

Approach 4
Most decentralized approach and best solution if not wanting to use chainlink oracles IMO. The more sources you grab from the more anti-fragile it is but also increases gas cost for matching.

I would be open to using approaches 1 or 4 and am available to discuss them further. Would be able to complete it within a week of approval at a price of 1200 USDC.

@0x4007
Copy link
Member Author

0x4007 commented Sep 14, 2022

Approach 4 Most decentralized approach and best solution if not wanting to use chainlink oracles IMO. The more sources you grab from the more anti-fragile it is but also increases gas cost for matching.

I would be open to using approaches 1 or 4 and am available to discuss them further. Would be able to complete it within a week of approval at a price of 1200 USDC.

Can you elaborate on what you mean by matching? I assumed that if we only do calls vs writes then its free? I feel like this requires just a tad more research but your proposal seems very reasonable and I'm glad that you were able to respond so promptly!

@hashedMae
Copy link
Contributor

Sorry, autocorrect changed mathing to matching without me noticing.
I've written similar solutions for a simplified liquidity market and needing to calculate loan to value ratio with multiple tokens on each side. That was done using chainlink oracles.

If using stable pools then reading from each one won't cost anything, it's the math to find the median that will cost gas. If you're using 3 sources vs 6, there's less math to do when comparing 3 but is also more fragile if somehow one of the pools were to be exploited. It could be built to use a set number of sources or so that sources can be added and removed if needed.

If wanting to avoid using other stablecoins altogether it could be based on ETH/USD vs ETH/uAD. I don't fully understand the implications that this would have but instincts tell me it's a bad idea and would need massive ETH/uAD depth for it not to be manipulatable.

@0x4007
Copy link
Member Author

0x4007 commented Sep 14, 2022

it's the math to find the median that will cost gas.

Before committing to this architecture its prudent to sample the gas cost of calculating the median of 2 markets and then we can extrapolate to determine reasonable bounds (e.g. we should limit it to 10 markets because the tx cost will be like $50 at 10 gwei etc.)

This research is necessary but we don't really have a bounty structure for research related tasks. Since our internal devs are saturated, if this task is something of interest to you we can add an additional time/cost estimate to your quote and have a report be your first deliverable?

@hashedMae
Copy link
Contributor

The biggest gas cost of finding a median value would likely be sorting the values. I could build a relatively simple contract that does that with two values and fuzz testing. I could have the report done by Friday and possibly earlier depending on other workloads. Total would be 1400 USDC, 200 for report, 1200 for the rest if approved once method is decided on.

@0x4007
Copy link
Member Author

0x4007 commented Sep 14, 2022

The biggest gas cost of finding a median value would likely be sorting the values. I could build a relatively simple contract that does that with two values and fuzz testing. I could have the report done by Friday and possibly earlier depending on other workloads. Total would be 1400 USDC, 200 for report, 1200 for the rest if approved once method is decided on.

Since the simple contract is a prototype, security doesn't need to be a priority. My expectations are the following:

  1. Have a solid prototype done no later than Sunday which will allow us to reasonably assess the feasibility of this approach.
  2. The other experienced solidity devs will look at your implementation and make sure it checks out over the course of the upcoming week.
  3. In parallel to point 2. the research team will continue to refine the architecture and take your prototype's results into consideration.

As a heads up, we're considering a launch on Optimism which may grant a ton more flexibility for gas costs.

The report should include:

  • The gas fee per gwei, per market (e.g. 1 gwei with 1 market = $1 at $1600 ETH; 1 gwei with 10 markets = $10 at $1600 ETH)
  • Your critical feedback on how to improve the design, if any.
  • Your approval or denial of this approach based on your findings, and a suggestion on an alternative approach if denied.

Bonus: if you find a way to dynamically select the optimal (definition TBD but e.g. the highest liquidity) markets from Curve to choose from, then this will prompt @Draeieg and other math wizards on the research team to design an algorithm to filter for the subset thats relevant (e.g. $1 pegged stablecoins)

With all that said, do you need to update your time/price estimate?

@hashedMae
Copy link
Contributor

Nope, and fuzz testing is more to test the math/sorting implementation and try to find any outliers for gas costs. I can write it to take an array of values and do a test with uint[2] and another with uint[3]. Comparing the two will allow me to extrapolate gas costs.

@0x4007
Copy link
Member Author

0x4007 commented Sep 14, 2022

You're authorized to proceed on the research portion. After we review your report I presume that you'll also be assigned to the real implementation!

@hashedMae
Copy link
Contributor

Report is ready
https://github.com/hashedMae/findMedianGasReport

@0x4007
Copy link
Member Author

0x4007 commented Sep 16, 2022

Report is ready https://github.com/hashedMae/findMedianGasReport

@0xcodercrane @zgorizzo69 do you have any remarks on @hashedMae's implementation?

@hashedMae I'm not sure what the USD value equivalents are at current prices. Are you able to answer the following?

  • The gas fee per gwei, per market (e.g. 1 gwei with 1 market = $1 at $1600 ETH; 1 gwei with 10 markets = $10 at $1600 ETH)
  • Your critical feedback on how to improve the design, if any.
  • Your approval or denial of this approach based on your findings, and a suggestion on an alternative approach if denied.

@hashedMae
Copy link
Contributor

I updated the readme in my repo with a table of the cost for each with Ether at $1600 and 50 gwei per gas. At 10 gwei per gas, the examples in the right hand column were a couple cents each.

Ideally, would collect the information off chain, arrange it in order lowest to highest, then pass it to the contract.

If that's not possible without refactoring multiple other contracts then I would go for the mean instead of median. Creating an ordered list on chain is too expensive because it requires looping and several writes to storage.

@0x4007
Copy link
Member Author

0x4007 commented Sep 16, 2022

Ideally, would collect the information off chain, arrange it in order lowest to highest, then pass it to the contract.

Would we need to verify this on chain with something like merkle trees etc? Or is verification not necessary?

If that's not possible without refactoring multiple other contracts then I would go for the mean instead of median. Creating an ordered list on chain is too expensive because it requires looping and several writes to storage.

What contracts are you thinking need to be refactored?

Median is less risky and is highly preferred as the strategy. For example if one of the five markets we select implodes, it should not negatively affect our oracle price. Ideally if we can have dynamic selection (e.g. top 5 highest liquidity markets) then mean also probably wouldn't work (it could select RAI, or LUSD which both are generally not at $1.00)

@0xcodercrane
Copy link
Contributor

Report is ready https://github.com/hashedMae/findMedianGasReport

@0xcodercrane @zgorizzo69 do you have any remarks on @hashedMae's implementation?

@hashedMae I'm not sure what the USD value equivalents are at current prices. Are you able to answer the following?

  • The gas fee per gwei, per market (e.g. 1 gwei with 1 market = $1 at $1600 ETH; 1 gwei with 10 markets = $10 at $1600 ETH)
  • Your critical feedback on how to improve the design, if any.
  • Your approval or denial of this approach based on your findings, and a suggestion on an alternative approach if denied.

I didn't have a chance to take a look unfortnately. but will get a chance at least this weekend.

@hashedMae
Copy link
Contributor

I don't think it would require a merkle tree.

I'm not familiar enough with the current architecture to know which ones are calling the current TWAP contact.

Way I would do the ordering off chain is whenever a call is made that needs the TWAP, the website backend uses ABIs from Uniswap and Curve to pull the prices from the stable pools we've identified, it puts them in an uint array that's ordered from least to greatest, and then passes that through to the contract in the call. If wanting to use a weighted average it could poll the ABIs periodically and store the values, discarding them once they pass a certain threshold of time.

@0x4007
Copy link
Member Author

0x4007 commented Sep 16, 2022

I didn't have a chance to take a look unfortnately.

@Draeieg @hashedMae @0xcodercrane I propose that we hop on a call after you look. @0xcodercrane you think you could make a group chat when ready?

@hashedMae what is your ERC20 USDC address?

@0xcodercrane
Copy link
Contributor

For sure, I will make a group chat at least next Monday.

@sergfeldman sergfeldman added this to the V1.0.1 milestone Sep 16, 2022
@sergfeldman sergfeldman added Solidity Solidity development work is expected. Time: <2 Weeks labels Sep 16, 2022
@hashedMae
Copy link
Contributor

@pavlovcik
0x7fe65D99a0998Cdba8e1f749303a467dcf87e815 or hashedmae.eth
I'm PDT and can be flexible for a call Monday

@hashedMae
Copy link
Contributor

Thinking more about off chain data collection and ordering. I'm not a web2 security expert but some sort of on chain verification is likely needed to minimize risk.

I'd only seen static merkle trees used previously but it seems a dynamic merkle tree with on-chain verification is possible https://ethresear.ch/t/efficient-on-chain-dynamic-merkle-tree/11054

If not merkle tree then whitelisting addresses that can be polled for the info, then verify the information on chain by contract and the block it was polled. That way if someone were to achieve some sort of code injection on the website, and were using their own contracts with phony info, our own contracts could look at the addresses and reject them.

@0x4007
Copy link
Member Author

0x4007 commented Sep 17, 2022

@pavlovcik 0x7fe65D99a0998Cdba8e1f749303a467dcf87e815 or hashedmae.eth I'm PDT and can be flexible for a call Monday

Thanks!

https://etherscan.io/tx/0xfb57b27c9423a9e167823c866548c441e23cf5a5f085a3b5cfb28f2bb789caa8
https://etherscan.io/tx/0x9a5e598b7d532deddb6e75b45577f1c880401913e1e3861397864d9093f0ff48

@0x4007
Copy link
Member Author

0x4007 commented Sep 19, 2022

Note for call:

  1. I took the whole table of markets on curve from coingecko: https://www.coingecko.com/en/exchanges/curve
  2. Checked "display unconverted data" for more precision
  3. Used the below function to calculate the median
  4. Was able to determine that the median for the top 50 markets is 0.9990366045004995
  5. Realized that this is just for ratios of the two assets (can be ETH/stETH which is not $1 of course!)
  6. Realized that we probably need another oracle to determine which markets are dollar pegged
function median(numbers) {
    const sorted = Array.from(numbers).sort((a, b) => a - b);
    const middle = Math.floor(sorted.length / 2);

    if (sorted.length % 2 === 0) {
        return (sorted[middle - 1] + sorted[middle]) / 2;
    }

    return sorted[middle];
}

For whatever its worth, I used the CoinGecko converted values in a second test (the dollar representations, with only two decimal places) and the result was exactly $1. I suppose if we somehow use USDC as the source of truth from the top Curve markets and somehow relate it back to our dollar (I assume we will need more markets, but I hope this is not the only solution) then I think we have a solution.

@0x4007
Copy link
Member Author

0x4007 commented Sep 19, 2022

6. Realized that we probably need another oracle to determine which markets are dollar pegged

A parameter in the curve pools could offer a clue on the developers' predicted stability of the assets. Perhaps there is a correlation with stablecoins vs non stablecoins?

Amplification Coefficient

image
3pool (high correlation)

image
stETH (medium correlation)

image
CRV-ETH (low correlation)

Appendix

Curve Pool Registry

get pool asset type

https://curve.readthedocs.io/registry-registry.html#Registry.get_pool_asset_type -

Asset types are as follows:
0: USD
1: BTC
2: ETH
3: Other StableSwap
4: CryptoSwap

Is metapool?

boolean

https://curve.readthedocs.io/registry-registry.html#pool-metadata

@0x4007
Copy link
Member Author

0x4007 commented Sep 20, 2022

Minutes / homework

  1. Merkle tree proof the first one post from a trusted party (like one of us) and then the rest should be compared against that and should be legit for the sorting

  2. research risks with 3pool getting frozen

  3. research if merkle tree proofs are feasible in this scenario and what the risk profile is like (@Draeieg states that if can iterate infinitely then eventually can be exploited)

  4. research how robust "weighted average and then exclude outlier data" strategy is

  5. how often to update the list of markets (with sorting and the gas fee we discussed) is needed

@Draeieg
Copy link

Draeieg commented Sep 20, 2022

https://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.2.9432
https://en.wikipedia.org/wiki/Introselect
https://en.wikipedia.org/wiki/Median_of_medians
https://en.wikipedia.org/wiki/Quickselect

I'll also try to look into RAI to see if there is a way we can call the oracle based on a PID controller, sounds a lot more complex than an average but who knows

@0x4007
Copy link
Member Author

0x4007 commented Oct 5, 2022

In case I haven't shared this yet, our current oracle relies on a two-swap TWAP (built in to the metapool contract) meaning that if two blocks can be proposed consecutively, we can get rekt. I understand that Uniswap V2's twap oracle implementation encodes a few variables into every sample.

(I believe they include the price data with the timestamp and have some minimum time window like a few minutes? It's been a couple years since I last looked and I'm on my phone about to sleep now)

Perhaps we can borrow some tricks to make our TWAP less exploitable in this double MEV scenario

https://twitter.com/macromate8/status/1563118677549264896?s=46&t=_UGbHclguQLRq9mzlZadXg

@malik672
Copy link

hi, i want to be assigned to this, perhaps more can be explained

@ubiquibot
Copy link

ubiquibot bot commented Jun 17, 2023

Available commands

- /assign: Assign the origin sender to the issue automatically.
- /unassign: Unassign the origin sender from the issue automatically.
- /help: List all available commands.
- /payout: Disable automatic payment for the issue.
- /multiplier: Set bounty multiplier (for treasury)
- /allow: Set access control. (Admin Only)
- /wallet: <WALLET_ADDRESS | ENS_NAME>: Register the hunter's wallet address. 
  ex1: /wallet 0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266 
  ex2: /wallet vitalik.eth

@malik672

@malik672
Copy link

malik672 commented Jun 17, 2023

/start

@rndquu
Copy link
Member

rndquu commented Jun 17, 2023

@malik672 could you first describe what is the solution you are going to implement?

@0x4007
Copy link
Member Author

0x4007 commented Jun 17, 2023

@malik672 could you first describe what is the solution you are going to implement?

I feel like we should spend some weeks revising all of these old bounties on this repository so that we can ensure clear specifications and deliverables.

@malik672
Copy link

@rndquu i can either implement 1 or 4

@rndquu
Copy link
Member

rndquu commented Jun 17, 2023

@pavlovcik I removed time and priority labels because the issue desc is not clear

@malik672 pls select another bounty while we're making specs more clear for this one

@malik672
Copy link

@pavlovcik ok, thanks is there any solidity related bounty that needs urgent attention

@rndquu
Copy link
Member

rndquu commented Jun 17, 2023

@pavlovcik ok, thanks is there any solidity related bounty that needs urgent attention

there will be new solidity related tasks next week

@0x4007 0x4007 added ping and removed ping labels Aug 9, 2023
@ubiquibot
Copy link

ubiquibot bot commented Aug 29, 2023

Skipping /start since no time labels are set to calculate the timeline

@rndquu
Copy link
Member

rndquu commented Dec 26, 2023

Could somebody elaborate on why exactly the 3CRV/USD quote is $1.03 instead of $1.00?

@0x4007
Copy link
Member Author

0x4007 commented Dec 26, 2023

I distinctly remember after Luna imploded and tons of liquidity flowed through the curve markets when people were exiting, and that presumably the 3CRV token accrued trading fees. That was the reason why I originally wrote this.

I don't fully understand the 3CRV price mechanics but this is just an observation from CoinGecko price data.

@rndquu
Copy link
Member

rndquu commented Dec 29, 2023

I distinctly remember after Luna imploded and tons of liquidity flowed through the curve markets when people were exiting, and that presumably the 3CRV token accrued trading fees. That was the reason why I originally wrote this.

I don't fully understand the 3CRV price mechanics but this is just an observation from CoinGecko price data.

Curve's 3pool has a trading fee set to 0.1% where:

  • 50% of trading fees goes to liquidity providers
  • 50% of trading fees goes to veCRV holders (those users who staked LP tokens and then locked CRV tokens for some time)

So, as far as I understand, 3CRV LP token accumulates trading fees hence it's USD quote is greater than $1.00.

We have a TWAP oracle contract that relies on Dollar-3CRVLP price which seems to be not the best decision. I guess the future audit will mark this issue.

We could use chainlink for fetching DAI/USD, USDT/USD and USDC/USD quotes and taking average or median of those values. Anyway we need to think more about how to fetch the Dollar/USD quote especially in cases of black swan events.

@rndquu
Copy link
Member

rndquu commented Apr 9, 2024

Closing this as completed in #893

@rndquu rndquu closed this as completed Apr 9, 2024
Copy link

ubiquibot bot commented Apr 9, 2024

! No price label has been set. Skipping permit generation.

@0x4007
Copy link
Member Author

0x4007 commented Apr 9, 2024

Closing this as completed in #893

Do you have a brief explanation of the solution? Are you cross referencing the price of ETH?

@rndquu
Copy link
Member

rndquu commented Apr 9, 2024

Closing this as completed in #893

Do you have a brief explanation of the solution? Are you cross referencing the price of ETH?

We migrated to the latest Curve's metapool implementation which has a built-in oracle.

I haven't investigated in depth the Curve's code, but, as far as I understand, latest price oracle takes into account 3CRV LP "virtual price" which is basically a tripool growth overtime. If tripool growth is 3% then 3CRV LP virtual price is 1.03 which is equal to $1.03 USD value. So we're free to use 3CRV LP virtual price in calculations.

The implemented solution was proposed in one of the audit issues + cergyk (lead senior watson) signed off on the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Research Bounties that may include a report only, without code. Solidity Solidity development work is expected.
Projects
None yet
Development

No branches or pull requests

8 participants