Bugfix/fix collateral twap

[gitcoindev]

Resolves #830

[gitcoindev]

This pull request is still a draft I will notify when it is ready for the review.

[gitcoindev]

I am finally back to this issue, in the background I spent quite some time on reading related articles and Frax sources and their concept of twap from either an Oracle or double oracles (https://github.com/FraxFinance/frax-oracles/tree/master/src).

[molecula451]

I am finally back to this issue, in the background I spent quite some time on reading related articles and Frax sources and their concept of twap from either an Oracle or double oracles (https://github.com/FraxFinance/frax-oracles/tree/master/src).

it's what a good programmer does

[gitcoindev]

I am finally back to this issue, in the background I spent quite some time on reading related articles and Frax sources and their concept of twap from either an Oracle or double oracles (https://github.com/FraxFinance/frax-oracles/tree/master/src).

it's what a good programmer does

Thank you, it takes me longer than expected but moving forward.

[gitcoindev]

Still in progress but I will speed up now.

[gitcoindev]

Note to myself: need to resolve conflicts after last merge.

[ubiquibot]

# Comment event received without a recognized user command.

[gitcoindev]

Hi @rndquu I added a new twap test skeleton today, and got interrupted in trying to solve the broken Yarn Audit check. It seems that this is broken due to yarnpkg/berry#4117 and more specifically yarnpkg/berry#4117 (comment) . After I upgraded locally to yarn 4, yarn npm audit --all --recursive seems to work correctly, but the 'fix' part still fails with the uncaught exception

$ yarn yarn-audit-fix
Resolve bins
Runtime digest

  isMonorepo true
  bins 
    yarn yarn
    npm npm
  
  versions 
    node v20.10.0
    npm 10.2.3
    yarn 4.0.2
    yaf 10.0.7
    yafLatest 10.0.7

..

{"value":"xml2js","children":{"ID":1092301,"Issue":"xml2js is vulnerable to prototype pollution","URL":"https://github.com/advisories/GHSA-776f-qx25-q3cc","Severity":"moderate","Vulnerable Versions":"<0.5.0","Tree Versions":["0.4.23"],"Dependents":["parse-bmfont-xml@npm:1.1.4"]}}
{"value":"yaml","children":{"ID":1094785,"Issue":"Uncaught Exception in yaml","URL":"https://github.com/advisories/GHSA-f9xv-q969-pqx4","Severity":"high","Vulnerable Versions":">=2.0.0-5 <2.2.2","Tree Versions":["2.2.1"],"Dependents":["lint-staged@npm:13.2.0"]}}

Failure!
{
  status: 1,
  signal: null,
  output: [
    null,
    <Buffer 7b 22 76 61 6c 75 65 22 3a 22 40 62 61 62 65 6c 2f 74 72 61 76 65 72 73 65 22 2c 22 63 68 69 6c 64 72 65 6e 22 3a 7b 22 49 44 22 3a 31 30 39 35 32 31 ... 18105 more bytes>,
    <Buffer >
  ],
  pid: 1434896,
  stdout: <Buffer 7b 22 76 61 6c 75 65 22 3a 22 40 62 61 62 65 6c 2f 74 72 61 76 65 72 73 65 22 2c 22 63 68 69 6c 64 72 65 6e 22 3a 7b 22 49 44 22 3a 31 30 39 35 32 31 ... 18105 more bytes>,
  stderr: <Buffer >
}

node:internal/process/esm_loader:40
      internalBinding('errors').triggerUncaughtException(

...

I guess we'll need to open a separate issue for that. In the meantime let's continue to solve this issue first.

[rndquu]

@gitcoindev hey

I added a new twap test skeleton today

Ok, thank you

I will fix the TWAP and the yarn audit script + will check how chainlink price oracle works in the current PR. As far as I understand you have unresolved tasks on the ubiquibot's side so you may concentrate on them.

[gitcoindev]

@rndquu thank you! Indeed, let me tackle unresolved bot tasks today and move them out of my plate.

[rndquu]

@pavlovcik

We're planning to use the LUSD token as collateral in the Ubiquity pool contract to mint/redeem Dollar tokens. Somehow we need to get the LUSD/USD price. Chainlink seems to be the most reliable(i.e. flashloan resistant) source of such data but chainlink price feeds support LUSD/USD quotes only from 2 networks: arbitrum and optimism. Since we're going to deploy on mainnet it seems to be wrong to fetch prices from some other network. From what I understand even if we take the LUSD/USD quote from some other (not mainnet) network and LUSD/USD quote deviates significantly then arbitrage bots should make the LUSD/USD quote equal across all networks so this is not an issue.

So the question is should we get the LUSD/USD quote from chainlink's arbitrum/optimism network OR from the LUSD curve metapool?

[0x4007]

Curve metapool seems to make the most sense to me. Can also consider uniswap. Possibly average both

[gitcoindev]

@pavlovcik @rndquu

While working on this issue I did some research and read multiple articles.

This one has a good recipe of the algorithm which is relevant for us:

https://news.curve.fi/chainlink-oracles-and-curve-pools/

About LUSD itself I will also paste a few interesting links I read on their blog a few days ago:

https://www.liquity.org/blog/price-oracles-in-liquity

https://www.liquity.org/blog/the-oracle-conundrum

The Oracle conundrum is a good article on price oracles in general, btw. I had to check myself what conundrum is -)

The defillama link below provides overall allocation of LUSD across different chains:

https://defillama.com/stablecoin/liquity-usd

[molecula451]

@rndquu are you sure? i'm seeing a deployed chainlink LUSD/USD oracle, check the tag name https://etherscan.io/address/0x3D7aE7E594f2f2091Ad8798313450130d0Aba3a0

[rndquu]

@rndquu are you sure? i'm seeing a deployed chainlink LUSD/USD oracle, check the tag name https://etherscan.io/address/0x3D7aE7E594f2f2091Ad8798313450130d0Aba3a0

Hmm, I can't find this price feed at https://data.chain.link/

[molecula451]

@rndquu are you sure? i'm seeing a deployed chainlink LUSD/USD oracle, check the tag name https://etherscan.io/address/0x3D7aE7E594f2f2091Ad8798313450130d0Aba3a0

Hmm, I can't find this price feed at https://data.chain.link/

I couldn't either, probably a bug on their front end, check the search, https://github.com/search?q=0x3D7aE7E594f2f2091Ad8798313450130d0Aba3a0&type=code this one is used by some different protocols

[molecula451]

https://github.com/aave/gho-aip/blob/90ddf49b65007da9ba6dbf9f80cc966275cde4f0/diffs/preTestEngineListing_postTestEngineListing.md?plain=1#L545

[rndquu]

https://github.com/aave/gho-aip/blob/90ddf49b65007da9ba6dbf9f80cc966275cde4f0/diffs/preTestEngineListing_postTestEngineListing.md?plain=1#L545

Yeah, I see that's a live LUSD/USD oracle but I can't understand why it's missing in https://data.chain.link/

[molecula451]

Yeah, I see that's a live LUSD/USD oracle but I can't understand why it's missing in https://data.chain.link/

it's a clear front end bug on their end and it doesnt make sense to be on rollups only, it's first mainnet then others

[rndquu]

I missed it but this issue was solved when we were refactoring the Ubiquity pool contract.

Anyway the current PR adds oracle (via chainlink) support for collateral token pricing which is the feature we'd earlier decided to implement.

Simply put the oracles in the whole protocol work this way:

1. For Dollar token pricing we ask Curve's Dollar-3CRVLP metapool
2. For collateral token pricing in the UbiquityPool contract we ask chainlink's data fees

I'll update the deployment scripts (to deploy TWAP and set chainlink price feed) in another PR.

@gitcoindev @molecula451 Pls review.

P.S. Diamond storage CI run is failing but this is expected and ok since the contracts are not deployed yet.
P.P.S. I'm still checking whether https://etherscan.io/address/0x3D7aE7E594f2f2091Ad8798313450130d0Aba3a0 is a valid chainlink price feed but it really looks legit. I'll let you know once I find the answer.

Update: @molecula451 just found out (thank you a lot) that aave uses https://etherscan.io/address/0x3D7aE7E594f2f2091Ad8798313450130d0Aba3a0 in their LUSD oracle contract so https://etherscan.io/address/0x3D7aE7E594f2f2091Ad8798313450130d0Aba3a0 seems to be legit.

[gitcoindev]

Thank you a lot @rndquu for the summary. I am going through your changes now, should finish the review in the next 30-60 minutes.

[gitcoindev]

P.P.S. I'm still checking whether https://etherscan.io/address/0x3D7aE7E594f2f2091Ad8798313450130d0Aba3a0 is a valid chainlink price feed but it really looks legit. I'll let you know once I find the answer.

@rndquu I also was struggling to find it but confirmed it in the official Liquity / LUSD documentation:

see https://docs.liquity.org/documentation/resources , on the page find 0x3D7aE7E594f2f2091Ad8798313450130d0Aba3a0

therefore, for me also legit. Funnily enough, they made a typo on the website and instead of Chainlink there is Chanlink so it was hard to find.

[rndquu]

P.P.S. I'm still checking whether https://etherscan.io/address/0x3D7aE7E594f2f2091Ad8798313450130d0Aba3a0 is a valid chainlink price feed but it really looks legit. I'll let you know once I find the answer.

@rndquu I also was struggling to find it but confirmed it in the official Liquity / LUSD documentation:

see https://docs.liquity.org/documentation/resources , on the page find 0x3D7aE7E594f2f2091Ad8798313450130d0Aba3a0

therefore, for me also legit. Funnily enough, they made a typo on the website and instead of Chainlink there is Chanlink so it was hard to find.

Cool, thank you, @molecula451 also found that aave uses https://etherscan.io/address/0x3D7aE7E594f2f2091Ad8798313450130d0Aba3a0 in their LUSD oracle contract

[gitcoindev]

I have reviewed the latest changes from @rndquu , lgtm! I leave the final review to @molecula451

[molecula451]

the implementation looks good and the revamped tests are much better now, once again everyone good work here