-
Notifications
You must be signed in to change notification settings - Fork 15
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.
I was confused about contracts/artifacts/SushiswapV2Router02.json
because the github diff renders this as if it c ontained the single line UniswapV2Router02.json
. Actually it's a symbolic link to that file.
Added info to the PR description |
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.
Really cool. Will look over the fact stuff again tomorrow morning.
One general comment, it looks like this kinda tooling could eventually fit more naturally in ethcontract. Given that it has become quite useful and sophisticated, it would be more easily accessible for other projects. |
|
} | ||
|
||
#[test] | ||
fn deployment_addresses() { |
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, I wonder how we can encourage people to add tests here whenever a new contract is added.
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.
Maybe we could somehow generate these lines from a constant list
include!(concat!(env!("OUT_DIR"), "/WETH9.rs"));
and include in this test and assertion each element from the list was verified here...
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.
This PR refactors how the artifact vendoring works in order to add new Balancer Vault contract. In particular this PR:
contractName
: the build script was overriding the contract name for all generated bindingsnetworks
: they were being set for all contracts that we neededContract::deployed()
for (with the exception of WETH, but that was a very small amount of data to add).bytecode
: For contracts that are only needed as an interface and are not deployed for E2ESushiswap*
toUniswapV2
symbolic linksvendor.rs
script.Furthermore, two contracts for which we had bindings but are unused have been removed (
UniswapV2Pair
andSushiswapV2Pair
, since we useIUniswapLikePair
for both those contracts).Test Plan
Added new tests to ensure that contracts have deployment information configured for all networks that we care about (to avoid another incident like we had in staging this morning).
Otherwise, no service changes, so CI will hopefully catch all issues.