Script to verify on Etherscan

[e18r]

This Python script takes the output of hevm flatten and transforms it such that all unnecessary library functions and comments are removed. It also adds a warning comment to the library code explaining that it is not the actual implementation. If addNewCollateral is not used, then it also removes the pragma experimental. This conditional can be generalized in the future.

Then it takes all the metadata from dapp.sol.json and uses the Etherscan API to verify the contract. It also takes the libraries from the Makefile, but in the future this will be changed in favor of extracting it from dapp.sol.json.

Tested with the current DssSpell. It needs testing with other contracts.

[talbaneth]

Was reading the script and trying to run it without actually deploying anything (not a real review at this point).

Something rather small that might be worth mentioning is that I think the reverse algo for removing unused functions assumes that a function is defined before it is called, which is not a must in solidity.

For example, assuming in both cases below g() anf f() are unused elsewhere, so we expect them to be wipedout:

case A: function g(){} function f(){g()} => here f() will be deleted as it is unused, then g() for the same reason

case B: function f(){g()} function g(){} => here g will be not be deleted as used in f(). only f() will be deleted.

I guess this is not an issue if we don't "have to" delete all unused code. But just worth mentioning. A possible solution for that is to run that algo twice.

[e18r]

Hey @talbaneth thank you for noticing this rather subtle issue. I was aware of it, but hadn't come up with a solution. Your suggestion is a simple way to solve it. Thank you!

I guess there could be cases where it should be run more than twice, but I don't think they're very common.

[e18r]

Ready for another round of review. I removed the contents of the functions as suggested by @gbalabasquer and I ran the function filtering code more than once, as suggested by @talbaneth. It's actually running on a loop until no changes are made.

[e18r]

Fixed two bugs found in the Goerli repo.

[e18r]

@talbaneth regarding your general comments:

1. Would this work with contract libraries which are not deployed already? e.g safemath. Not sure we are allowed to replace their content with stabs.

It won't work, but AFAIK we never do that. Also, I'm not sure how to detect that scenario. Does the library code just becomes part of the contract's bytecode as with inheritance?

2. Do we support more than one deployed library? (don't think so, at least for the verify stage where we pass only one). If not maybe we should add a validation that there is not more than one.

Added that validation.

3. Not a must, but we can also remove all the internal function declarations and "constant internal" lines in the library processed code.

That seems like more work than it's worth. Removing something like WAD is hard because that keyword appears everywhere and there's no easy way to tell whether it's a reference to this library's WAD or something else.

[e18r]

@talbaneth changes included in spells-goerli and spells-kovan's PR for you to approve: https://github.com/makerdao/spells-kovan/pull/72 Thanks

[e18r]

I think it makes sense to support our current needs at first, and then expand if necessary.

[e18r]

@gbalabasquer please consider approving this.