Platform: Code4rena
Start Date: 27/05/2021
Pot Size: $100,000 USDC
Total HM: 12
Participants: 7
Period: 7 days
Judge: LSDan
Total Solo HM: 10
Id: 12
League: ETH
Rank: 4/7
Findings: 4
Award: $9,353.29
🌟 Selected for report: 2
🚀 Solo Findings: 1
🌟 Selected for report: cmichel
4295.5942 USDC - $4,295.59
cmichel
The Uniswap oracle uses a mock contract with hard-coded prices to retrieve the price which is not feasible in production.
Not sure if this is part of the contest, this will probably still be changed? But note that even when using the "real deal" @uniswap/v3-periphery/contracts/libraries/OracleLibrary.sol it does not return the prices.
The price could change from the set price and always updating new prices with set will be too slow and gas expensive.
Use cumulativeTicks = pool.observe([secondsAgo, 0]) // [a_t1, a_t2] and apply equation 5.5 from the Uniswap V3 whitepaper to compute the token0 TWAP.
Note that even the official .consult call seems to only return the averaged cumulative ticks, you'd still need to compute the 1.0001^timeWeightedAverageTick in the function.
#0 - alcueca
2021-06-02T09:35:11Z
We probably should have not included this contract, it's too confusing since at the time the Uniswap v3 OracleLibrary was still a mock, and this hasn't gone real testing.
The price source in a production version would be a Uniswap v3 pool, not one of our mock oracle sources. We never expected to call set in production, but to retrieve the prices from a Uniswap v3 pool using the mentioned library (which was not even merged into main at the start of the contest).
We will check with the Uniswap team what is the recommended way of using their oracles. The equation 5.5 in the whitepaper is problematic, because an exponentiation of two fractional numbers in solidity is neither trivial nor cheap. Our understanding is that one of the goals of the OracleLibrary was to provide a consistent implementation to this formula.
From a conversation with @moodysalem, I understand that the code in getQuoteAtTick might achieve the same result as the 5.5 equation, so maybe we need to retrieve the average tick with consult, and then the actual price with getQuoteAtTick.
#1 - alcueca
2021-06-02T09:36:01Z
I'm using the acknowledged label for findings that require further investigation to assess.
1933.0174 USDC - $1,933.02
cmichel
The witch can Witch.grab vaults and the vaultOwners[vaultId] field is set to the original owner.
However, when the auction time is over and the debt has not been fully paid back, the original owner is not restored, and the witch can grab the same vault again, overwriting the original owner vaultOwners[vaultId] field permanently with the witch.
function grab(bytes12 vaultId) public { DataTypes.Vault memory vault = cauldron.vaults(vaultId); vaultOwners[vaultId] = vault.owner; cauldron.grab(vaultId, address(this)); }
Even a full repayment will not restore the original vault owner anymore.
No funds will be stuck as the vault can still be correctly liquidated (calling settle).
However, the vault owner will not be restored which is bad if it is a valuable vaultId (low number) that has a special meaning or would be used as an NFT/for retroactive airdrops for initial liquidity providers down the road.
When grabbing check if vaultOwners[vaultId] is already the witch and in that case just do an early return of the function - not overwriting the vaultOwners[vaultId] field.
#0 - alcueca
2021-06-02T09:51:05Z
Duplicate with #8
🌟 Selected for report: cmichel
0 USDC - $0.00
cmichel
In Witch.buy there's the possibility to do one multiplication instead of two divisions: Instead of computing the ink price as 1 / artPrice and then dividing by it to get the ink amount as ink = art / price, just keep the art price and multiply it by the art amount.
These lines need to be changed:
price = term1.wmul(term2); // this is the art price in terms of ink now, instead of ink price ink = uint256(art).wmulup(price); // can just multiply by art price
One saves gas by doing one multiplication instead of two divisions which seems to be an important goal of Yield v2.
#0 - alcueca
2021-06-03T01:55:06Z
I need to confirm the math is right, before I confirm.