liquity / ChickenBond

GNU General Public License v3.0
32 stars 5 forks source link

Use Curve v2 factory in mainnet forking tests #162

Closed danielattilasimon closed 2 years ago

danielattilasimon commented 2 years ago

Previously, the tests were using the v1 factory (while referring to it as v2) to deploy a "plain" pool, which is appropriate for pegged assets, but probably not so appropriate for LUSD:bLUSD. Besides, I think the intent was always to use v2.

This PR deploys a pool + gauge for LUSD:bLUSD using the v2 factory, in order to give us greater confidence that we'll be able to do the same at launch.

bingen commented 2 years ago

One more comment, do we know which version of Gauge this factory deploys? We are supposed to use v4 from here: https://github.com/curvefi/curve-dao-contracts/blob/master/contracts/gauges/LiquidityGaugeV4.vy

That repo has also a deployment-logs folder, but it seems outdated and only for testnets: https://github.com/curvefi/curve-dao-contracts/tree/master/deployment-logs

It looks different from this one at first glance: https://github.com/curvefi/curve-factory-crypto/blob/master/contracts/LiquidityGauge.vy

danielattilasimon commented 2 years ago

One more comment, do we know which version of Gauge this factory deploys? We are supposed to use v4 from here: https://github.com/curvefi/curve-dao-contracts/blob/master/contracts/gauges/LiquidityGaugeV4.vy

Where did that information originate from? Can it be double-checked?

Here's for example a v2 factory pool (TOKE-ETH): https://curve.fi/factory-crypto/55 https://etherscan.io/address/0xe0e970a99bc4f53804d8145bebbc7ebc9422ba7f

This is its gauge that was deployed using the same v2 factory: https://etherscan.io/address/0xa0C08C0Aede65a0306F7dD042D2560dA174c91fC

It is not the v4 gauge. You can get from the pool contract to the gauge by first querying factory() on the pool, then get_gauge(pool) on the factory.

bingen commented 2 years ago

The main reason to use v4 is that it allows for continuous rewards. I’ll check if that one does it too.

danielattilasimon commented 2 years ago

The main reason to use v4 is that it allows for continuous rewards. I’ll check if that one does it too.

Thanks, that makes sense. Hopefully this one allows continuous rewards too, because the v2 factory doesn't allow you to choose between gauge implementations, it only supports this one.

bingen commented 2 years ago

It does. I checked and the logic for both add_reward and deposit_reward_token is identical. I’ll add some more mainnet tests to make sure though (I can do it in a separate PR, so feel free to merge this one)

danielattilasimon commented 2 years ago

That's excellent! I was concerned about the lack of gauge-tests; I think that'll be very useful, because we have to get it right (seeing as there's no way to replace the gauge address after the fact).