Conversation
This reverts commit 63b6514.
There was a problem hiding this comment.
Overall it looks great and I don't see any bugs. However, a couple of requests before we go live:
-
@Joeysantoro you're more familiar with which pcv deposits are protocol-owned (which would make the second deploy parameter True). You should definitely check over all of the ones here to make sure this parameter is set correctly for each one.
-
In the "Deploy Actions" comment, there are 28 actions. Can you number each deployment action in the script corresponding to this comment? This will ensure we aren't missing anything.
-
You should do one more check over to make sure there's no PCV deposits we forgot to add.
I had them organized into two groups so it should be false in the first and true in the second
Done
Can't think of any more after we add ERC20 one. I also incremented the manual PC FEI for Idle and BarnBridge. I made the guardian the owner until we ratify on-chain so we can add/remove |
ERC20 PCV deposit wrapper
xklob
left a comment
There was a problem hiding this comment.
Overall, this looks great. We should definitely add a bunch of unit & integration tests before using it "in prod" however.
|
linting & prettier for you, and also pushed the small fix for the typescript-casting for the contracts. |
…rotocol-core into feat/CR-Oracle-Deploy
…rotocol-core into feat/CR-Oracle-Deploy
TODO: