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

Upgrade test contracts to 0.5.0 and fix all resulting problems #1465

Merged
merged 4 commits into from
Nov 28, 2018

Conversation

haltman-at
Copy link
Contributor

[Note: This branch is kind of a mess; it might not end up getting merged in as such.]

This branch upgrades all the test contracts to 0.5.0, and attempts to fix all the resulting problems. I've already fixed one such problem, namely the lack of support for STATICCALL, in PR #1446. But there's more.

One issue that's come up is that a number of my tests are quite shaky. This isn't an 0.5.0 problem, but I in my investigations I think I've figured out what's causing it anyway -- namely, I was relying on sourceId to be consistent between runs, when it's not. That's an easy fix.

But the real problems are the 0.5.0-specific problems, which are very consistent. The failing tests (now that I've fixed the storage decoding tests, where the problem was STATICCALL) are:

  1. The memory decoding tests
  2. The variable IDs tests
  3. The two mapping tests where the domain is numeric

Note that these errors exist also, in near-identical form, on the truffle-decoder branch; I made a separate branch for testing that, namely decoder-0.5.0-tests, which see.

The memory decoding tests, as best I can tell, do seem to be due to a decoding problem. However, I'm not certain of this.

The variable ID tests seem to be due to some difference in how variable declarations are handled, except for one of them which also seems to be due to a decoding problem like above. Again, all this is uncertain.

The two mapping tests... I'd say also a decoding problem, probably, but a different one. However I've made no real exploration of this at all.

Basically, I don't know what's going on, but that's my current progress. Will update.

@haltman-at
Copy link
Contributor Author

Note: The intermittent failure should now be addressed due to PR #1466.

@haltman-at haltman-at added the wip label Nov 27, 2018
{
unsafeCleanup: true,
setGracefulCleanup: true,
name: "default#web3-one"
Copy link
Contributor

Choose a reason for hiding this comment

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

This change is necessary for Migrations.sol

@@ -1,3 +1,4 @@
/* eslint-disable no-unused-vars */
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably get rid of this


const __VARIABLES = `
pragma solidity ^0.4.18;
pragma solidity ~0.5;
Copy link
Contributor

Choose a reason for hiding this comment

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

These of course are necessary

@@ -11,7 +11,7 @@ import Debugger from "lib/debugger";
import sessionSelector from "lib/session/selectors";

const __OUTER = `
pragma solidity ^0.4.18;
pragma solidity ~0.5;
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -33,7 +33,7 @@ contract OuterContract {
`;

const __INNER = `
pragma solidity ^0.4.18;
pragma solidity ~0.5;
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -1,10 +1,9 @@
import debugModule from "debug";
Copy link
Contributor

Choose a reason for hiding this comment

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

yep these too

version: "0.4.25"
}
};

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotta put this back in and turn the version up, apparently!

@@ -1,10 +1,9 @@
import debugModule from "debug";
Copy link
Contributor

Choose a reason for hiding this comment

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

All this

@@ -12,14 +12,14 @@ import data from "lib/data/selectors";
import solidity from "lib/solidity/selectors";
Copy link
Contributor

Choose a reason for hiding this comment

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

yep

@@ -12,7 +12,7 @@ import solidity from "lib/solidity/selectors";
import trace from "lib/trace/selectors";
Copy link
Contributor

Choose a reason for hiding this comment

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

yep

@haltman-at haltman-at removed the wip label Nov 28, 2018
@haltman-at haltman-at changed the title [WIP] Upgrade test contracts to 0.5.0 and fix all resulting problems Upgrade test contracts to 0.5.0 and fix all resulting problems Nov 28, 2018
@haltman-at
Copy link
Contributor Author

OK, the optimizer has now been turned off for the tests (thanks @gnidan), solving most of the problems.

In addition, I've passed the files argument to prepareDebugger in tests that were previously missing it, which should (independently of the 0.5.0 update) fix their intermittent failures.

A word of warning, the compiler version is currently set to 0.5.0 rather than ^0.5.0; this is because the latter doesn't work on my machine. We'll have to figure out some way of getting the latter to work, because we don't want to have to constantly update the version every time there's a new version of solc.

@coveralls
Copy link

coveralls commented Nov 28, 2018

Coverage Status

Coverage decreased (-0.03%) to 67.51% when pulling 2b5a836 on solc-0.5.0-tests into b81fa2f on next.

@gnidan gnidan merged commit a4be893 into next Nov 28, 2018
@gnidan gnidan deleted the solc-0.5.0-tests branch November 28, 2018 22:44
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.

3 participants