Investigate & fix bug surfaced in 5.0.44

[CruzMolina]

Greenkeeper caught a bug released in 5.0.44 over in Colony's repo: JoinColony/colonyNetwork#734.

Appears to be related to InterfaceAdapter changes, but it could be something else. Seems like this is relatively high priority.

EDIT: So far it seems like this is actually a Web3.js issue that appeared in the past. See #1504, #1461, & web3/web3.js#2105.

[CruzMolina]

When investigating locally, I hit this type of error:

 Contract: Colony Network
    when initialised
      ✓ should accept ether
      ✓ should have the correct current Colony version set
      1) "before each" hook for "should have the Resolver for current Colony version set"

  Contract: Colony Payment
    2) "before all" hook: prepare suite

  Contract: ColonyPermissions
    3) "before all" hook: prepare suite

  Contract: Colony Recovery
    4) "before all" hook: prepare suite

  Contract: Colony Reward Payouts
    5) "before all" hook: prepare suite

  Contract: Colony Task Work Rating
    6) "before all" hook: prepare suite

  Contract: ColonyTask
    7) "before all" hook: prepare suite

  Contract: Colony Token Integration
    8) "before all" hook: prepare suite

  Contract: Colony
    9) "before all" hook: prepare suite

  Contract: Meta Colony
    10) "before all" hook: prepare suite

  Contract: Reputation mining - basic functionality
    11) "before all" hook: prepare suite

  Contract: Reputation Updates
    12) "before all" hook: prepare suite

  Contract: EtherRouter / Resolver
    13) "before all" hook: prepare suite

  Contract: Token Locking
    14) "before all" hook: prepare suite

  Contract: One transaction payments
    15) "before all" hook: prepare suite


  155 passing (26m)
  15 failing

  1) Contract: Colony Network
       "before each" hook for "should have the Resolver for current Colony version set":
     
Could not connect to your Ethereum client with the following parameters:
    - host       > localhost
    - port       > 8545
    - network_id > 1575397088637
Please check that your Ethereum client:
    - is running
    - is accepting RPC connections (i.e., "--rpc" option is used in geth)
    - is accessible over the network
    - is properly configured in your Truffle configuration file (truffle-config.js)

  Error:     at PromiEvent (node_modules/truffle/build/webpack:/packages/contract/lib/promievent.js:6:1)
      at TruffleContract.lookup (node_modules/truffle/build/webpack:/packages/contract/lib/execute.js:109:1)
      at lookup (helpers/upgradable-contracts.js:67:42)
      at tryCatch (node_modules/regenerator-runtime/runtime.js:45:40)
      at Generator.invoke [as _invoke] (node_modules/regenerator-runtime/runtime.js:271:22)
      at Generator.prototype.(anonymous function) [as next] (node_modules/regenerator-runtime/runtime.js:97:21)
      at asyncGeneratorStep (helpers/upgradable-contracts.js:24:103)
      at _next (helpers/upgradable-contracts.js:26:194)
      at process._tickCallback (internal/process/next_tick.js:68:7)

[CruzMolina]

In 5.0.44 we bumped Web3.js to 1.2.2 (#2558), so there could be something going on with the "backported" typings, or the fact that the ganache-core dep truffle & colony uses (ganache-core@2.8.0) is still on 1.2.1 and appears to have unpinned types being pulled in.

Going to downgrade priority since this appears to be more of a connectivity issue.

[jjgonecrypto]

We received this error as well in CI for https://github.com/Synthetixio/synthetix when upgrading truffle from 5.0.43 to 50.0.44, and we're on web3 1.2.2 and ganache 6.7.0. FWIW I also tried upgrading web3 1.2.4 and ganache-cli to 6.8.1-beta.0 (which uses ganache-core 2.9.1-beta.0) but it still appears.

[cgewecke]

@CruzMolina I think I have a repro (or a guess about the problem).

• unbox metacoin
• in test/metacoin.js, add

const MetaCoin = artifacts.require("MetaCoin");
const util = require('util');

contract('MetaCoin', (accounts) => {

  it('should put 10000 MetaCoin in the first account', async () => {
    const metaCoinInstance = await MetaCoin.deployed();
    const balance = await metaCoinInstance.getBalance.call(accounts[0]);

    console.log(
      "web3.currentProvider --> " +
      util.inspect(web3.currentProvider)
    );

    console.log(
      "Contract.web3 (web3Shim) --> " +
      util.inspect(MetaCoin.web3.currentProvider)
    );

    console.log(
      "instance.contract.currentProvider (also web3Shim?)--> " +
      util.inspect(metaCoinInstance.contract.currentProvider)
    );

    assert.equal(balance.valueOf(), 10000, "10000 wasn't in the first account");
  });
})

If you compare outputs for Truffle v5.0.43 vs v5.1.2, you'll see that the web.currentProvider injected into the global mocha context has a connected property which is true in the earlier version, but false on latest. The web3s attached directly to the contracts are true for both versions.

Truffle 5.1.2: web3.currentProvider

{
  host: 'http://127.0.0.1:7545/',
  httpAgent:
   Agent {...},
  withCredentials: false,
  timeout: 0,
  headers: undefined,
  connected: false,  // <-- Should be true?
  send: [Function],
  _alreadyWrapped: true 
}

Truffle 5.0.43: web3.currentProvider

{
  host: 'http://127.0.0.1:7545/',
  httpAgent: 
   Agent {...},
  timeout: 0,
  headers: undefined,
  connected: true, // True!
  send: [Function],
  _alreadyWrapped: true 
}

Both Synthetix and JoinColony use this injected web3.currentProvider instance in their test helpers

• Synthetix helper example
• JoinColony helper example

[cgewecke]

@CruzMolina Just circling back here, for a quick look. In case the note above isn't clear - believe the bug is at Truffle, possibly where Web3 is instantiated in core/test.

#2653 looks related and might provide a simple test case as well.

But if you discover the problem is the dependency, please just lmk and will try to get it fixed.

[CruzMolina]

So far #2653 looks somewhat related but a different issue altogether.

[CruzMolina]

Hmm, I'm not exactly sure what's going on here. Is this a timeout issue or an issue with using web3's low-level rpc request method web3.currentProvider.send?

It doesn't seem like we should be using the connected boolean as the determining factor. 5.0.44's connected is coming up true locally, yet Colony's tests fail on that version.

Furthermore, the web3js default instantiation in core/test was implemented before 5.0.44. 🤷‍♂

[CruzMolina]

It's looking like downgrading to web3 1.2.1 is the solution here.

[cgewecke]

@CruzMolina I will open a PR on latest with a reproduction / test case. If it passes then downgrading Web3 makes sense. If it doesn't, then downgrading Web3 won't fix the problem.

[cgewecke]

Furthermore, the web3js default instantiation in core/test was implemented before 5.0.44.

Ok yes, I'm wrong about currentProvider (unhappily because this problem seems worryingly non-deterministic). Will look back over on the Web3 side...

Colony's tests are:

• Truffle 5.0.44 155 passing / 15 before blocks failing
• Truffle 5.1.3 428 passing / 6 before blocks failing

[nivida]

@cgewecke Thanks for giving support here to the Truffle team!
I've checked the changeset between 1.2.1 and 1.2.2 related to the used HttpProvider. We were changing some smaller stuff which shouldn't have an impact on existing code.
The withCredentials options property does now default to false instead to true and we added a method called supportsSubscription which does return the boolean value false. The default options change was required to fix a CORS issue for Firefox/Safari and the added method was a small feature request from a web3.js user.
We also improved the transaction confirmations over a HTTP connection. It was before just a setInterval which triggered a confirmation each second but didn't check if actually a new block was mined.

The mentioned improvement of the transaction confirmations could probably be the thing that does give some problems in the Colony test suite but I'm not sure.

[PaulRBerg]

Not sure if this helps, but for https://github.com/sablierhq/sablier, downgrading and pinning web3@1.2.1 alone was not sufficient. I also had to also downgrade and pin truffle to 5.0.37.

[CruzMolina]

Hey @PaulRBerg, I see a failing job using 5.1.3. Are there logs available for any version between 5.0.37 and 5.0.44?

[PaulRBerg]

I think the latest CircleCI workflow finished right after you replied: https://circleci.com/workflow-run/00101565-3959-48d0-9e9e-b587c768c9e8

Are there logs available for any version between 5.0.37 and 5.0.44?

Unfortunately, no. I left the contracts untouched for a while.

[CruzMolina]

Based on the diff, it looks like the unpinned truffle deps were pulling in 5.1.3. Would love to know if this same issue was present between 5.0.37 & 5.0.44.

[PaulRBerg]

There you go, I created a branch for you where truffle is pinned to 4.0.40.

https://circleci.com/workflow-run/6bad4d9a-c04c-4820-990e-3f69cca867f3

[CruzMolina]

@PaulRBerg , not quite sure what's going on with your workflow. Seems like it is either an unrelated issue or a different one.