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

Bug fix: Repair compiler output from Solidity <0.4.20 #4363

Merged
merged 2 commits into from
Oct 12, 2021
Merged

Conversation

haltman-at
Copy link
Contributor

@haltman-at haltman-at commented Oct 11, 2021

It seems that versions of Solidity prior to 0.4.20 get confused by file paths containing colons... which all of our file paths now do, due to #4137 and its followups. This resulted in us not getting any artifacts for them. This PR adds a step to the processing of the compiler output and repair it, solving the problem.

So what was the problem? The problem is that, let's say you have a file path project:/path and a contract Contract. The contracts output of the compiler is supposed to look like this:

{
  "project:/path": {
    Contract: {
      ...
    }
  }
}

But instead, it splits on that first colon, yielding:

{
  project: {
    "/path:Contract": {
      ...
    }
  }
}

(In case it's not clear: It seems that the whole thing was originally one string, "project:/path:Contract", and it split on the first colon instead of the last, is the problem, and 0.4.20 apparently fixed this problem and started correctly splitting on the last colon instead of the first.)

So, we check to see if any contract names contain a colon; and, if so, we transform the second format into the first format. (I decided to have it skip this if no contract names contain a colon, even though notionally we could just always run the transform regardless.)

Note: 0.4.19 specifically still doesn't work, despite this bug fix. However it looks to me like 0.4.19 is just broken and there's not much we can do about it, so, yeah. Like, we're just straight up not getting bytecode from it for whatever reason, and I don't think there's much we can do about that...

Note: We've also been having other issues recently with versions 0.4.9 and 0.4.10. (As for prior to 0.4.9, we don't support that; adding support for that is issue #1679.) This PR doesn't do anything about those; I'll probably go open an issue for those. But this at least gets 0.4.11-18 working again.

Copy link
Member

@cds-amal cds-amal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it! Can you add a unit test for repairOldContracts? It would be nice to have the test infrastructure to lean on in future work here.

@haltman-at
Copy link
Contributor Author

OK, I've added a test.

Copy link
Member

@cds-amal cds-amal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @haltman-at ! LGTM!

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

Successfully merging this pull request may close these issues.

2 participants