Skip to content
This repository has been archived by the owner on Mar 17, 2024. It is now read-only.

Not reporting on contracts using the delegateProxy pattern #106

Closed
elenadimitrova opened this issue Jan 9, 2019 · 5 comments
Closed

Not reporting on contracts using the delegateProxy pattern #106

elenadimitrova opened this issue Jan 9, 2019 · 5 comments

Comments

@elenadimitrova
Copy link

Contracts calls using a delegate proxy (for upgradability) aren't reported on. This used to work in v0.1.2 but no longer does. @area suspects this change might have introduced it.

For sample outputs see working examples in our Circle builds under ""
Broken (using v0.1.12): https://circleci.com/gh/JoinColony/colonyNetwork/5654#artifacts/containers/0
Working (using v0.1.2): https://circleci.com/gh/JoinColony/colonyNetwork/4841

@cgewecke
Copy link
Owner

cgewecke commented May 7, 2019

Hi @elenadimitrova :)

Have made headway on this with 0.2.0 although there are still issues. I've added colonyNetwork's gas tests to a job in CI here so you can see the differences between your current output and the new version:

The main problem is that some contracts have method signatures which shadow each other. If they're called through the router the reporter has to guess the target and just picks the first match. For example, in your current report you have results for NoLimitSubdomains that 0.2.0 attributes to Colony. Numbers look about the same. Neither contract is reported in the deployments table.

Really fixing this and making it accurate/reliable will require more thought.

Related issues: #94, #64

@elenadimitrova
Copy link
Author

Thanks @cgewecke this is already a lot better as all of the deployed contracts are reported.
Also on the function signature overlap it has improved as NoLimitSubdomains which was wrongly reported before e.g. here now correctly report the right Colony contract here. Maybe it's because it first sees these functions defined there as you say but still.
I'm going to merge this upgrade so we're back to tracking latest versions. Thanks for the support.

@cgewecke
Copy link
Owner

cgewecke commented May 9, 2019

Thanks @elenadimitrova!

@cgewecke
Copy link
Owner

cgewecke commented Jul 3, 2019

@elenadimitrova

0.2.2 includes support for proxy resolvers (#126) and ships with an EtherRouter handler. However, it turns out you don't benefit from this because in your case the 'wild guess' strategy generates the same data, more quickly, (LOL). That's what it looks like anyway - if not please lmk. In the future if you see missing data you can add the following to your reporterOptions and it should help:

proxyResolver: 'EtherRouter',

There is a new thing in 0.2.2 you might find useful since you're on Circle. You can now get something like "coveralls for gas" in CI thanks to a (free) service called codechecks that MakerDAO engineer Chris Kaczor has been building. It looks like this and includes a diff report for gas changes relative to the PRs target branch:

Screen Shot 2019-06-18 at 12 25 49 PM

If you have any interest in trying it out I can open a PR configuring it at colonyNetwork. If not - no problem :) There are also setup instructions and more info here.

@cgewecke
Copy link
Owner

Closing - colonyNetwork 669

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants