Internal calls from constructor fail to compile #433

mrice32 · 2019-11-20T16:10:27Z

We've been running into a compilation issue related to calling internal functions from a constructor (I think this used to be an issue in solc). The following contract fails to compile after being instrumented:

pragma solidity ^0.5.0;

pragma experimental ABIEncoderV2;

library TestLib {
    struct State {
        uint integer;
    }

    function internalFunction(State storage state) internal {
        state.integer = 5;
    }
}


contract Test {
    using TestLib for TestLib.State;
    TestLib.State state;

    constructor() public {
        state.internalFunction();
    }

    function getInteger() public returns (uint) {
        return state.integer;
    }
}

The error that the compiler gives is:

InternalCompilerError: Assembly exception for bytecode

We've noticed that moving the internal call to a public method seems to avoid the issue. The following contract compiles fine after being instrumented:

pragma solidity ^0.5.0;

pragma experimental ABIEncoderV2;

library TestLib {
    struct State {
        uint integer;
    }

    function internalFunction(State storage state) internal {
        state.integer = 5;
    }
}


contract Test {
    using TestLib for TestLib.State;
    TestLib.State state;

    constructor() public {
        callInternalFunction();
    }

    function getInteger() public returns (uint) {
        return state.integer;
    }

    function callInternalFunction() public {
        state.internalFunction();
    }
}

Two other things to note:

We have noticed that this only happens when ABIEncoderV2 is enabled.
These contracts all compile (without instrumentation) with the most recent version of solc.

cc @ptare

The text was updated successfully, but these errors were encountered:

mrice32 · 2019-11-20T16:19:13Z

This seems very similar to this issue on older versions of the Solidity compiler. Not sure what would make it show up again - the above was compiled with 0.5.13.

cgewecke · 2019-11-20T16:35:26Z

@mrice32

(Assuming you are using the 0.7.0-beta)

This sounds a lot like the reports in #417. There, it's been attributed to solidity-coverage forcing solc to compile without optimization. Do you get the same error if you disable optimization in a regular test run?

mrice32 · 2019-11-20T16:42:47Z

Thanks for the quick response. We're currently using the latest stable release, 0.6.7.

I'll try 1) compiling with the optimizer disabled and 2) a coverage run with the 0.7.0-beta and let you know the results. Thanks!

cgewecke · 2019-11-20T16:47:06Z

@mrice32 Ah cool ok, just lmk if you run into problems...there are links to the 0.7.0 docs and an upgrade guide at the top of the current README.

mrice32 · 2019-11-20T16:57:15Z

I tried compiling with the optimizer off, and I had no problems, which leads me to believe that this may be a distinct issue from #417

As for the beta, do you suspect that installing it may solve this issue assuming it's not a duplicate of #417? I only ask because the beta may take me a bit longer to set up since we have a bit of custom plumbing built around the 0.6.x. We'll get around to this eventually, but I just don't want to gate solving this issue on installing the beta if it's unrelated. Thanks for your help!

cgewecke · 2019-11-20T17:01:52Z

@mrice32 Yes I do suspect that (unfortunately). The beta uses a much less problematic instrumentation strategy and if you can compile without optimization normally you shouldn't see that error.

Could I ask what your additional plumbing is like?

mrice32 · 2019-11-20T17:25:10Z

It's not super complicated, it's just these files:

https://github.com/UMAprotocol/protocol/blob/master/ci/coverage.sh
https://github.com/UMAprotocol/protocol/blob/master/common/globalSolcoverConfig.js

I'll need to work through the beta instructions, which will almost certainly get rid of or simplify most of this code.

I'll move to the beta and let you know what I find! Thanks!

mrice32 · 2019-11-20T20:08:43Z

The contracts seem to compile correctly with the beta, which is great news! Thanks for your help.

I've attempted to upgrade our repo to the beta, but I've run into an issue that I can't seem to resolve. I'm not sure if it's a coverage issue or a truffle issue.

This error throws before any tests are run.

> Istanbul reports written to ./coverage/ and ./coverage.json                                                                                                   
> solidity-coverage cleaning up, shutting down ganache server                                          
Error: Error: Error                                                                                                                                                           
    at Object.run (/Users/matt/git/protocol/core/node_modules/solidity-coverage/dist/plugin-assets/truffle.library.js:194599:13)                                     
    at processTicksAndRejections (internal/process/task_queues.js:86:5)
Truffle v5.0.21 (core: 5.0.21)                                                                                                                                       
Node v11.15.0

Not sure why the error has no description, but I narrowed it to this line in our migrations: https://github.com/UMAprotocol/protocol/blob/285311051c2a0847ac2b0ede399d6e066e6d64c3/core/migrations/10_deploy_tokenized_derivative_creator.js#L31. It's the only place where we link a library, so my guess is it has something to do with linking. Are you aware of any issues related to linking libraries?

Side note: this error does not show up when running tests normally.

mrice32 · 2019-11-20T20:21:50Z

Also, please let me know if you'd prefer that I move this discussion to a new issue since it's unrelated to the title.

cgewecke · 2019-11-20T21:27:04Z

@mrice32

In your package.json Truffle looks pinned at 5.0.21. Is it possible to upgrade to a newer Truffle version and see if the regular tests work?

Then, if they do - try solidity-coverage again with the newer version. The beta relies on plugin resources that were added after 5.0.31. It tries to handle older Truffle versions but maybe that's not working too well.

This thread is fine! Super into finding and fixing any bugs on the beta.

mrice32 · 2019-11-20T21:38:47Z

Thanks for the tip - I'l make sure I run with >= 5.0.31. I locally upgraded to the newest truffle release 5.1.0, and got the same error:

> Istanbul reports written to ./coverage/ and ./coverage.json
> solidity-coverage cleaning up, shutting down ganache server
Error: Error: Error
    at Object.run (/Users/matt/git/protocol/node_modules/truffle/build/webpack:/packages/migrate/index.js?abc3:92:1)
    at processTicksAndRejections (internal/process/task_queues.js:86:5)
Truffle v5.1.0 (core: 5.1.0)
Node v11.15.0

cgewecke · 2019-11-20T21:38:48Z

Also @mrice32 if you push your change-set to UMA I'd be happy to take a look and debug locally.

mrice32 · 2019-11-20T21:42:48Z

Sure, here's the branch I'm using: https://github.com/mrice32/protocol/tree/coverage. Sorry if it's a little messy!

mrice32 · 2019-11-20T21:44:29Z

To run coverage you'll need to run the following commands from the top level directory:

npm install
./ci/coverage.sh core

cgewecke · 2019-11-20T21:45:01Z

Awesome, checking it out rn.

cgewecke · 2019-11-20T22:22:06Z

Ok! In coverage.sh

- $(npm bin)/truffle run coverage --network coverage
+ $(npm bin)/truffle run coverage --network ci

...gets the tests running. Still debugging though because I see a couple failing.

cgewecke · 2019-11-20T22:40:50Z

I got this:

But I see this set of errors - not sure what's expected here. Can you clarify?

 1) Contract: Registry
       Initialize roles:
     TypeError: registry.initializeRolesOnce is not a function
      at Context.<anonymous> (test/Registry.js:30:44)
      at process._tickCallback (internal/process/next_tick.js:68:7)

  2) Contract: Store
       Compute fees basic check:
     Error: Invalid number of parameters for "computeRegularFee". Got 4 expected 3!
     at PromiEvent (/Users/cgewecke/code/sc-forks/protocol/node_modules/truffle/build/webpack:/packages/contract/lib/promievent.js?f772:9:1)
      at TruffleContract.computeRegularFee (/Users/cgewecke/code/sc-forks/protocol/node_modules/truffle/build/webpack:/packages/contract/lib/execute.js?9956:121:1)
      at Context.<anonymous> (test/Store.js:35:28)
      at process._tickCallback (internal/process/next_tick.js:68:7)

  3) Contract: Store
       Compute fees at 20%:
     Error: Invalid number of parameters for "computeRegularFee". Got 4 expected 3!
     at PromiEvent (/Users/cgewecke/code/sc-forks/protocol/node_modules/truffle/build/webpack:/packages/contract/lib/promievent.js?f772:9:1)
      at TruffleContract.computeRegularFee (/Users/cgewecke/code/sc-forks/protocol/node_modules/truffle/build/webpack:/packages/contract/lib/execute.js?9956:121:1)
      at Context.<anonymous> (test/Store.js:52:28)
      at process._tickCallback (internal/process/next_tick.js:68:7)

  4) Contract: Voting
       Constructor:
     TypeError: voting.initializeOnce is not a function
      at Context.<anonymous> (test/Voting.js:67:16)
      at process._tickCallback (internal/process/next_tick.js:68:7)

  5) Contract: scripts/Voting.js
       basic case:
     Error: Returned error: VM Exception while processing transaction: revert The current voting round has not ended
     at PromiEvent (/Users/cgewecke/code/sc-forks/protocol/node_modules/truffle/build/webpack:/packages/contract/lib/promievent.js?f772:9:1)
      at TruffleContract.getPrice (/Users/cgewecke/code/sc-forks/protocol/node_modules/truffle/build/webpack:/packages/contract/lib/execute.js?9956:121:1)
      at Context.<anonymous> (test/scripts/Voting.js:90:31)
      at process._tickCallback (internal/process/next_tick.js:68:7)

  6) Contract: scripts/Voting.js
       simultaneous and overlapping votes:
     Error: Returned error: VM Exception while processing transaction: revert The current voting round has not ended
     at PromiEvent (/Users/cgewecke/code/sc-forks/protocol/node_modules/truffle/build/webpack:/packages/contract/lib/promievent.js?f772:9:1)
      at TruffleContract.getPrice (/Users/cgewecke/code/sc-forks/protocol/node_modules/truffle/build/webpack:/packages/contract/lib/execute.js?9956:121:1)
      at Context.<anonymous> (test/scripts/Voting.js:163:31)
      at process._tickCallback (internal/process/next_tick.js:68:7)

  7) Contract: scripts/Voting.js
       Intrinio price:
     Error: Returned error: VM Exception while processing transaction: revert The current voting round has not ended
     at PromiEvent (/Users/cgewecke/code/sc-forks/protocol/node_modules/truffle/build/webpack:/packages/contract/lib/promievent.js?f772:9:1)
      at TruffleContract.getPrice (/Users/cgewecke/code/sc-forks/protocol/node_modules/truffle/build/webpack:/packages/contract/lib/execute.js?9956:121:1)
      at Context.<anonymous> (test/scripts/Voting.js:199:32)
      at process._tickCallback (internal/process/next_tick.js:68:7)

  8) Contract: scripts/Voting.js
       Constant price:
     Error: Returned error: VM Exception while processing transaction: revert The current voting round has not ended
     at PromiEvent (/Users/cgewecke/code/sc-forks/protocol/node_modules/truffle/build/webpack:/packages/contract/lib/promievent.js?f772:9:1)
      at TruffleContract.getPrice (/Users/cgewecke/code/sc-forks/protocol/node_modules/truffle/build/webpack:/packages/contract/lib/execute.js?9956:121:1)
      at Context.<anonymous> (test/scripts/Voting.js:252:32)
      at process._tickCallback (internal/process/next_tick.js:68:7)

  9) Contract: scripts/Voting.js
       Numerator/Denominator:
     Error: Returned error: VM Exception while processing transaction: revert The current voting round has not ended
     at PromiEvent (/Users/cgewecke/code/sc-forks/protocol/node_modules/truffle/build/webpack:/packages/contract/lib/promievent.js?f772:9:1)
      at TruffleContract.getPrice (/Users/cgewecke/code/sc-forks/protocol/node_modules/truffle/build/webpack:/packages/contract/lib/execute.js?9956:121:1)
      at Context.<anonymous> (test/scripts/Voting.js:280:32)
      at process._tickCallback (internal/process/next_tick.js:68:7)

One thing that might be important is to set the network_id param for the launched client - I see that specified in the "ci" network in the truffle config.

In .solcover.js I think you'd put:

module.exports = {
  providerOptions: {
    network_id: 1234
  },
  skipFiles: ["Migrations.sol", "echidna_tests", "test"]
};

mrice32 · 2019-11-20T23:28:11Z

Gotcha. That makes a lot of sense. Just out of curiosity, why can't you just use * for the network id?

I'm guessing those test errors have to do with some of the other stuff that I changed, but I'll dig in and double check. Regardless, I think this is really close to working. I really appreciate you taking the time to help!

cgewecke · 2019-11-21T00:18:26Z

@mrice32 "*" is very likely fine, was just mentioning it in case there's some network id specific logic in your code. Often people give that a concrete value when they're using it to toggle something on/off.

I really appreciate you taking the time to help!

No problem! Thanks for trying the beta and just ping this ticket if there's more to fix.

Closing for now though because the 0.6.x branch isn't going to get further development.

mrice32 changed the title ~~Internal calls from constructor sporadically fail to compile~~ Internal calls from constructor fail to compile Nov 20, 2019

cgewecke added the 0.6.0 label Nov 20, 2019

cgewecke closed this as completed Nov 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Internal calls from constructor fail to compile #433

Internal calls from constructor fail to compile #433

mrice32 commented Nov 20, 2019

mrice32 commented Nov 20, 2019 •

edited

Loading

cgewecke commented Nov 20, 2019

mrice32 commented Nov 20, 2019

cgewecke commented Nov 20, 2019

mrice32 commented Nov 20, 2019 •

edited

Loading

cgewecke commented Nov 20, 2019

mrice32 commented Nov 20, 2019 •

edited

Loading

mrice32 commented Nov 20, 2019 •

edited

Loading

mrice32 commented Nov 20, 2019 •

edited

Loading

cgewecke commented Nov 20, 2019 •

edited

Loading

mrice32 commented Nov 20, 2019

cgewecke commented Nov 20, 2019

mrice32 commented Nov 20, 2019

mrice32 commented Nov 20, 2019

cgewecke commented Nov 20, 2019

cgewecke commented Nov 20, 2019 •

edited

Loading

cgewecke commented Nov 20, 2019

mrice32 commented Nov 20, 2019

cgewecke commented Nov 21, 2019

Internal calls from constructor fail to compile #433

Internal calls from constructor fail to compile #433

Comments

mrice32 commented Nov 20, 2019

mrice32 commented Nov 20, 2019 • edited Loading

cgewecke commented Nov 20, 2019

mrice32 commented Nov 20, 2019

cgewecke commented Nov 20, 2019

mrice32 commented Nov 20, 2019 • edited Loading

cgewecke commented Nov 20, 2019

mrice32 commented Nov 20, 2019 • edited Loading

mrice32 commented Nov 20, 2019 • edited Loading

mrice32 commented Nov 20, 2019 • edited Loading

cgewecke commented Nov 20, 2019 • edited Loading

mrice32 commented Nov 20, 2019

cgewecke commented Nov 20, 2019

mrice32 commented Nov 20, 2019

mrice32 commented Nov 20, 2019

cgewecke commented Nov 20, 2019

cgewecke commented Nov 20, 2019 • edited Loading

cgewecke commented Nov 20, 2019

mrice32 commented Nov 20, 2019

cgewecke commented Nov 21, 2019

mrice32 commented Nov 20, 2019 •

edited

Loading

mrice32 commented Nov 20, 2019 •

edited

Loading

mrice32 commented Nov 20, 2019 •

edited

Loading

mrice32 commented Nov 20, 2019 •

edited

Loading

mrice32 commented Nov 20, 2019 •

edited

Loading

cgewecke commented Nov 20, 2019 •

edited

Loading

cgewecke commented Nov 20, 2019 •

edited

Loading