Deployment changes - Githubissues

fvictorio commented 6 years ago

I just deployed the PoPA contracts to Rinkeby (main contract address: 0x3e3d9DdE1bb65B82eCb904832fD9D11091d607D7). I had to do some changes to be able to do that, and I think we should take some decisions around that.

I added a network in the Truffle configuration for Rinkeby. I used truffle-hdwallet-provider and the mnemonic from start-testrpc. It also uses https://rinkeby.infura.io/ as the provider URL. (Changes)
Right now the migrations deploy all the contracts but the main one from the first account, and the main one is deployed from the second account. This doesn't work when using the hdwallet-provider, since that only unlocks one account. Since, as far as I can tell, this is mostly something left from the dev environment, I just modified the migration scripts to deploy all contracts from the first account. (Changes)
I added a key rpc: 'https://rinkeby.infura.io/' to server-config-private.js. Please note that the server might be listening to some chain while the user is connected to another, and as far as I know there is no mechanism to detect this discrepancy.
The ERC780 is deployed in some random address (see #85). Another option is to not deploy it and use the address of an already deployed ERC780.
There is a check that fails if the transaction registering the address has a to address different than the address of the contract that the server knows. If you put the address in the .env with the checksum (as I did), the check fails because the comparison is case sensitive. (Changes)

Please let me know what you think about this approach.

cc @Janther (I can't add you as a reviewer for some reason)

fvictorio commented 6 years ago

Note that the change to the migration breaks the truffle tests. If we decide to make that change, I will fix them.

fvictorio commented 6 years ago

@phahulin Please take a look at the last commit I pushed.

I modified the migrations scripts to only deploy the ERC780 in the dev environment. I also merged the two migrations file; it was cleaner and it also saved us one unnecessary transaction during the deploy.
Since there is one transaction less, the default address of the PoPA contract when deploying against ganache is different, so I updated the .env.example.

phahulin commented 6 years ago

@fvictorio I probably missed something, but got different address when deploying against ganache: I ran ./node_modules/.bin/truffle migrate from blockchain/ subfolder and got

Writing artifacts to ./build/contracts

Using network 'development'.

Running migration: 1_initial_migration.js
  Deploying Migrations...
  ... 0x2c7d51de43a0d4ae62e2d0abb2ef43d0257442b036988d15a7d5da3f646874ff
  Migrations: 0x7eae39c6ba1f4d0a6a09b28b6c2d2f979606357c
Saving successful migration to network...
  ... 0xc910c6ccadc624cbf551bbff107844d3b4d21c1a9ff5885adfaefd1d35e3d980
Saving artifacts...
Running migration: 1522104575_popa.js
  Running step...
  Deploying PhysicalAddressClaim...
  ... 0x47ac5d781c5b7ffbf8fd5c8e01640c6e2c827dd02e92ca25683a5cc7acbaf5d9
  PhysicalAddressClaim: 0x2b72f1d288d4b6cb5d86218ea6a8b3d25e72d12c
  Linking PhysicalAddressClaim to ProofOfPhysicalAddress
  Deploying EthereumClaimsRegistry...
  ... 0x014913a92bfc01a4460361753f2766992f19bc29b4cd59177e0615229081c66e
  EthereumClaimsRegistry: 0x0211579468a2025472fb8e4fe2658db3dd37bccb
  Deploying ProofOfPhysicalAddress...
  ... 0xfbec5ccf0cfe736a500048b365706a92ccb57fcef512fc50acd304f0d56983d7
  ProofOfPhysicalAddress: 0xc395234b5d14bed13eb69595da09d7795892087a
Saving successful migration to network...
  ... 0x8429c6d33a7732b92c254b50653e2d3077194137743c08cd3ac7d088368ac63e
Saving artifacts...

also you mention that

I added a key rpc: 'https://rinkeby.infura.io/' to server-config-private.js

you probably meant truffle.js?

fvictorio commented 6 years ago

but got different address when deploying against ganache

That's weird. Are you sure you started ganache doing npm run start-testrpc?

you probably meant truffle.js?

No, I meant the configuration, since the server needs to know in which chain to test the transaction.

phahulin commented 6 years ago

That's weird. Are you sure you started ganache doing npm run start-testrpc?

Yes, could you try on your side from scratch? Maybe I've missed something

No, I meant the configuration, since the server needs to know in which chain to test the transaction.

Ok, I misunderstood you, I thought you meant that there must have been updates of server-config-private.js in this PR

fvictorio commented 6 years ago

You are right, my bad. I don't know where I got that address from.

phahulin commented 6 years ago

OK, can this be merged or you plan to add something more?

phahulin commented 6 years ago

Are tests failing because of address change?

fvictorio commented 6 years ago

Yes, since the PoPA contract is no longer deployed from the second address, the private key that was being used was wrong. They should pass now.

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 365

1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
45 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.2%) to 74.252%

Files with Coverage Reduction	New Missed Lines	%
web-dapp/src/components/RegisterAddressPage.js	45	69.87%
<!--	Total:	45		-->

Totals
Change from base Build 352:	-0.2%
Covered Lines:	876
Relevant Lines:	1129

poanetwork / poa-popa

Deployment changes #111

Pull Request Test Coverage Report for Build 365

💛 - Coveralls