urbit / azimuth

General-purpose PKI on Ethereum
MIT License
134 stars 36 forks source link

Tests & deployment broken #28

Closed jtobin closed 4 years ago

jtobin commented 4 years ago

This is what we get for not running CI.. :smile: (because #13)

I'm not sure when this happened exactly, but the test suite doesn't pass. Like, more than is described in #13. There are some problems with migrations/2_deploy_contracts.js that will prevent it from running at all -- web3 is used, but never imported nor a web3 object created, and the web3 version pinned in package.json has a different API than the code used in there anyway (e.g. it seems web3.fromAscii should be used in place of web3.utils.fromAscii).

But after fixing that up or checking out c950c6ae681b7fdaa5cb5e7e12e8d387d3751799, the last commit before those changes were made, the test suite doesn't pass. Even checking out v1.2.1, the tests don't pass. I haven't checked back any further yet.

/cc @liam-fitzgerald @Fang-

jtobin commented 4 years ago

(I can confirm that they do run as expected at v1.2.0.)

Fang- commented 4 years ago

Well, I can repro this at least, getting:

/Users/fang/repos/urbit/constitution/migrations/2_deploy_contracts.js:15
const condit2 = web3.utils.fromAscii("1234");
                           ^

TypeError: Cannot read property 'fromAscii' of undefined
    at /Users/fang/repos/urbit/constitution/migrations/2_deploy_contracts.js:15:28

Tests run if you replace web3.utils. with web3. in that file, so maybe we just need to either upgrade the pinned web3 version, or change that file?

Some linear star release tests still fail, because web3 handles overloaded functions rather poorly. Maybe that'd be fixed in a new version...

jtobin commented 4 years ago

Tests run if you replace web3.utils. with web3. in that file, so maybe we just need to either upgrade the pinned web3 version, or change that file?

Aha. So I was getting much worse failures yesterday. Apparently we don't have a web3 dependency pinned in package.json at all, so I must have implicitly been pulling in a different web3 version when trying to test / fix the issue.

The lockfile on master includes web3 at at least v1.2.1, v1.2.2, and v1.2.6 as it pulls it in for various other dependencies. If I just add var web3 = require('web3'); at the top of migrations/2_deploy_contracts.js today, I don't even have to patch web3.utils.fromAscii to web3.fromAscii. Just importing the library is good enough to make the tests run (although many don't pass).

Adding the latest version of web3 (v1.2.7) to devDependencies and requiring it in the deployment script seems to make most of the tests pass, although I do still see a substantial number of failures (not in the linear star release contract, either).

It's safe to say that this is all a bit of a mess, in any case, and heavily depends on the state of one's node_modules directory.

jtobin commented 4 years ago

As an aside, azimuth-js is specifying a much older web3 peer dependency of v1.0.0-beta.54, so it's possible those should be unified as well.

johnmendonca commented 4 years ago

I am getting the same fromAscii error during npx truffle deploy. It looks like just importing web3 would be the fix as mentioned above, but I wasn't sure if truffle was supposed to provide an instance implicitly. Here are the versions I ended up with:

$ npm list web3
azimuth-solidity@1.2.1 /home/john/code/azimuth
└─┬ solidity-coverage@0.7.7
  ├─┬ @truffle/provider@0.1.19
  │ ├─┬ @truffle/interface-adapter@0.3.3
  │ │ └── web3@1.2.2
  │ └── web3@1.2.1
  └── web3@1.2.6

I am using the gui version of Ganache, after changing the port number in truffle.js and importing web3 in the deploy script it has some success:

$ npx truffle deploy
Using network 'development'.

Running migration: 1_initial_migration.js
Saving artifacts...
Running migration: 2_deploy_contracts.js
  Running step...
Saving artifacts...
  Deploying Azimuth...
  ... 0xd9811675e716ab99c9da6d41a878a2602aad43d800ee1d4b8e180c5d4bc35e9b
  Azimuth: 0xf15034ccdf5338eabe9d1f3a71eb6709329e7f11
  Deploying Polls...
  ... 0x82744164c80a388c62e39d3665fb6b833af01ab701ad4a4cc70aa44b0e3c9f26
  Polls: 0x83b7772a894987f60bcb1b34bd070ef5bfcca9da
  Deploying Claims...
  ... 0x0db5c0a1854ca24a4bdc316d41280c421fe7a856d66933dde0bb2f9a2c33754d
  Claims: 0x8e6c6a6618a87b8218407fa135df194c9005fcec
  Deploying Censures...
  ... 0x7846b722d2ab44007189acfaba86f56ff83186aebe73be5973ad0bd9847c3a20
  Censures: 0x8db6eb253afb4953b00cb38f3d1ffb9e11f24688
  Deploying Ecliptic...
  ... 0x9ca2f421a40606111dd95a8148311d1ba3167cfac75c82f93b835cc71b7d091d
  Ecliptic: 0xab2cb7fa1784e915afa43131d7739ce30e9ee6c5
  ... 0xab791622b8c842a7dc576e59c59bd1e659e3e44f3b1ae22c1ec0e8a703c904b7
  ... 0x2a2b2c87bfa205f905e5d31529442c695cda6615b91d88934d052394dfbc9b6d
  Deploying DelegatedSending...
  ... 0xb28746f865131d6f50284da9fcb5fe7b198c46f3ea3ec2c49741c69962390421
  DelegatedSending: 0x737f059c91ec1717d536febf78e3515eb39e00d9
(node:29484) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'getBlock' of undefined
(node:29484) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
(node:29484) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

So it looks like importing web3 fixed the first issue as web3.utils is no longer undefined, but the next error is because web3.eth is undefined. I believe this means web3 isn't setup with a provider, which makes sense given the explicit import. I guess truffle is supposed to provide the web3 instance with provider somehow magically in these scripts.

johnmendonca commented 4 years ago

I've got the deploy script working by adding the following to the top of 2_deploy_contracts.js:

var Web3 = require('web3');
var web3 = new Web3(new Web3.providers.HttpProvider('http://localhost:7545'));

I am running a Ganache instance on port 7545, and updated the port number in truffle.js

cxk280 commented 4 years ago

Regarding the post immediately above: you probably want localhost:8545, not localhost:7545. The former is what is currently configured in truffle.js and is the default configuration.