Web3 1.0 instances seem to share state (provider overwritten)

[benjamincburns]

Observed this issue on 1.0-beta.26 - haven't tested other versions.

When I create an instance of web3, use it for a bit, then create a second instance with a different provider, the first instance's provider is overwritten.

I've modified test/setProvider.js to demonstrate the issue. These changes are given below as a diff taken against c4c92c3.

diff --git a/test/setProvider.js b/test/setProvider.js
index d0f4caf..52a3a15 100644
--- a/test/setProvider.js
+++ b/test/setProvider.js
@@ -54,7 +54,11 @@ describe('lib/web3/setProvider', function () {
         provider1.bzz = 'http://localhost:8500';
         provider2.bzz = 'http://swarm-gateways.net';
 
+        var provider3 = new FakeHttpProvider();
+        provider3.bzz = 'http://localhost2:8500';
+
         var lib = new Web3(provider1);
+        var lib2 = new Web3(provider3);
 
         assert.equal(lib.eth.currentProvider.constructor.name, provider1.constructor.name);
         assert.equal(lib.eth.net.currentProvider.constructor.name, provider1.constructor.name);
@@ -71,6 +75,21 @@ describe('lib/web3/setProvider', function () {
         assert.equal(lib.eth.accounts._requestManager.provider.constructor.name, provider1.constructor.name);
         assert.equal(lib.shh._requestManager.provider.constructor.name, provider1.constructor.name);
 
+        assert.equal(lib2.eth.currentProvider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.eth.net.currentProvider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.eth.personal.currentProvider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.eth.Contract.currentProvider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.eth.accounts.currentProvider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.shh.currentProvider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.bzz.currentProvider, provider3.bzz);
+
+        assert.equal(lib2.eth._requestManager.provider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.eth.net._requestManager.provider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.eth.personal._requestManager.provider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.eth.Contract._requestManager.provider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.eth.accounts._requestManager.provider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.shh._requestManager.provider.constructor.name, provider3.constructor.name);
+
 
         lib.setProvider(provider2);
 
@@ -89,6 +108,21 @@ describe('lib/web3/setProvider', function () {
         assert.equal(lib.eth.accounts._requestManager.provider.constructor.name, provider2.constructor.name);
         assert.equal(lib.shh._requestManager.provider.constructor.name, provider2.constructor.name);
 
+        assert.equal(lib2.eth.currentProvider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.eth.net.currentProvider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.eth.personal.currentProvider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.eth.Contract.currentProvider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.eth.accounts.currentProvider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.shh.currentProvider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.bzz.currentProvider, provider3.bzz);
+
+        assert.equal(lib2.eth._requestManager.provider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.eth.net._requestManager.provider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.eth.personal._requestManager.provider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.eth.Contract._requestManager.provider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.eth.accounts._requestManager.provider.constructor.name, provider3.constructor.name);
+        assert.equal(lib2.shh._requestManager.provider.constructor.name, provider3.constructor.name);
+
 
     });
 

[benjamincburns]

Note that this issue is blocking The addition of websockets and IPC to Ganache and ganache-cli (formerly TestRPC).

[benjamincburns]

1 failing

  1) lib/web3/setProvider
       Web3 submodules should set the provider using constructor:

      AssertionError: expected 'IpcProvider' to equal 'HttpProvider'
      + expected - actual

      -IpcProvider
      +HttpProvider

      at Context.<anonymous> (test/setProvider.js:114:16)

Looks like it's failing on assert.equal(lib2.eth.Contract.currentProvider.constructor.name, provider3.constructor.name); after the 2nd call to lib.setProvider - I suppose I'll have a look there.

[benjamincburns]

Contract.setProvider = function(provider, accounts) {
    // Contract.currentProvider = provider;
    core.packageInit(Contract, [provider]);

    Contract._ethAccounts = accounts;
};

The signature of core.packageInit is function (pkg, args) - in packageInit the current provider is tracked as pkg._provider. This apparently causes initialization of a second instance of Web3 to overwrite the first instance's .eth.Contract._provider.

[benjamincburns]

From what I can tell, I think the design is over-constrained. It seems that you want to be able to do something like:

var web3 = new Web3();
web3.setProvider(...);
// compilation steps and/or fetching of ABI go here
var myContract = new web3.eth.Contract(...);

And I'd assume that adding myContract.setProvider(web3.currentProvider); or passing web3.currentProvider to the Contract constructor would be considered unacceptable boilerplate.

I had some fun with util.inherit and a bit of monkey patching to essentially give each instance of web3.eth its own Contract type, but that introduces some other chicken/egg issues which I'm having difficulty sorting out. I'll keep the patch around if anybody wants to collaborate on it, otherwise I think I've unblocked myself by just reimporting web3 in my test suite so that each entire object/type tree is disconnected. It's more than a bit atom bomb vs anthill, however...

[benjamincburns]

I think I've unblocked myself by just reimporting web3 in my test suite so that each entire object/type tree is disconnected.

Yeah, that's not a reliable fix. For some reason it makes the tests pass on this computer, but imported modules are supposed to be cached. The fact that my tests pass makes me feel like I'm relying upon version-specific or undefined behaviour, and I'd rather not rely on it -- so this still blocks me.

[benjamincburns]

@frozeman I asked around and was told that you'd be the person to ping about this issue. I'm happy to submit a PR w/ the fix, but I don't want to guess too much at what strategy you'd prefer I take. Feel free to ping me on gitter or the Consensys slack if you'd like to discuss. Happy to set up a face-to-face via Zoom as well.

[frozeman]

Shared state was not planned, and I had quite some pains make the contract object inherit the provider. If you can. One up with a fix I’m happy to hear. You can contact me in Gitter and we can have a skype chat on Thursday

[benjamincburns]

@frozeman turns out I wasn't jumping through the right hoops last night and I managed to crack it. See #1191.

[raghwendra3]

1. Inbox
   has a default message:
   TypeError: inbox.methods.message is not a function
   at Context.it (test/Inbox.test.js:23:38)

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! inbox@1.0.0 test: mocha
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the inbox@1.0.0 test script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR! /home/raghwendra/.npm/_logs/2018-03-10T21_14_29_204Z-debug.log