-
Notifications
You must be signed in to change notification settings - Fork 98
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
Eth to DAI LBP #880
Eth to DAI LBP #880
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, would clear up some of the comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
{ | ||
target: 'pcvGuardianNew', | ||
values: '0', | ||
method: 'setSafeAddresses(address[])', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metacomment - there is a singular version of this function that is easier to call
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Want to deploy and verify the contracts, then add to this PR before merging?
expect(ethGained).to.be.bignumber.at.most(ethers.constants.WeiPerEther.mul(6)); | ||
|
||
// Put in 10k DAI, got out ~5 WETH | ||
// Implies price of $2000 per WETH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice
Contracts deployed and verified Calldata:
Execute Calldata:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, minor comments
console.log('WETH gained: ', ethGained); | ||
|
||
// Accelerate time and check ended | ||
await time.increase(LBP_FREQUENCY); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also fast forwarded time, performed a second swap and validated that the amountOut increased - to check that the weights were decreasing as expected. No strong feelings, it's a bit paranoid
}, | ||
ethToDaiLBPPool: { | ||
artifactName: 'IWeightedPool', | ||
address: '0x34809aEDF93066b49F638562c42A9751eDb36DF5', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would recommend verifying this LBPPool contract and inspecting the variables. Neat trick to verify these LBP pools is to pull the source code of an already verified contract and then use the standard-json input verification method:
curl -s [https://api.etherscan.io/api\?module\=contract\&action\=getsourcecode\&address\=0x32F0DC9E2D890BAC95106a65eB82db70Bc58BaDB\&apikey\=AETHERSCANPIKEY](https://api.etherscan.io/api/?module\=contract\&action\=getsourcecode\&address\=0x32F0DC9E2D890BAC95106a65eB82db70Bc58BaDB\&apikey\=AETHERSCANPIKEY) | jq -r '.result[0].SourceCode' | sed 's/{{/{/' | sed 's/}}/}/' > sol.json
PR Type: Feature
PR Description: This proposal accomplishes two ETH PCV related actions.
PR Checklist - Feature (Proposal)