poanetwork / poa-popa

DApp for proof of physical address (PoPA) attestation for validators of POA Network
https://popa.poa.network
GNU General Public License v3.0
24 stars 18 forks source link

Get contract address and ABI from environment #98

Closed fvictorio closed 6 years ago

fvictorio commented 6 years ago

After the changes that @Janther made, the PoPA contract is not a standalone contract anymore, so the best way to compile it and deploy it is via truffle. But this means that we can't use contract-output.json anymore to get the address and the ABI of the PoPA contract.

This PR gets the address and the ABI from the environment. So now, for starting the app, you have to follow these steps:

  1. Create .env in the root of the project. Just copying the example (cp .env.example .env) should be enough.
  2. In a terminal, start ganache with npm run start-testrpc
  3. Go to the blockchain directory and run ./node_modules/.bin/truffle migrate --network test
  4. Go to the root of the project and npm start

For this to work, I had to do several hacky things:

Keep in mind that this means that if we change the PoPA contract we'll need to udate the env variable.

Please test this and let me know if you have any issues.

pablofullana commented 6 years ago

@fvictorio there's a / missing in step 3, right? Should it be: ./node_modules/.bin/truffle migrate --network test ?

phahulin commented 6 years ago

@matiasgaratortiz @fvictorio what do you think about this: we add the following to the bottom of blockchain/migrations/1522104575_popa.js:

...
    if (process.env.SAVE_OUTPUT) {
        let fs = require('fs');
        let path = require('path');
        let contractOutputPath = path.join(__dirname, '../../web-dapp/src/contract-output.json');
        let contractData = await POPA.deployed();
        let contractOutputJson = {};
        contractOutputJson.ProofOfPhysicalAddress = {
            address: contractData.address,
            abi: contractData.abi
        };
        console.log('Saving contract output to: ' + contractOutputPath);
        fs.writeFileSync(contractOutputPath, JSON.stringify(contractOutputJson, null, 4), 'utf8');
    }

then we can run migration as

SAVE_OUTPUT=1 ./node_modules/.bin/truffle migrate --network test

it will create/rewrite contract-output.json.

fvictorio commented 6 years ago

@phahulin My concern with an approach like that is, how does that affect the app deploy? I guess the server won't be responsible for deploying the contracts, and it seems like contract-output is not added to git.

Besides, and at least in my experience, env variables are more flexible (for example, if for some reason the contract needs to be re-deployed, it can be done just modifying an env var in whatever service is used for serving the app, e.g. Heroku).

phahulin commented 6 years ago

Yes, deployment to the "real" network can be done manually. Initially I didn't add contract-output.json to git because keeping a file for a particular network in the repo would make the whole repo too specific.

You're probably right that we should support env vars, but we also have other variables that would need to be updated this way, those set in web-dapp/server-config-private.js. It's like env but not in plain text :)

fvictorio commented 6 years ago

Yes, I agree. Specially things like LOB's API key. Maybe we should create an issue for doing that?

phahulin commented 6 years ago

Opened https://github.com/poanetwork/poa-popa/issues/99

fvictorio commented 6 years ago

Great, thanks!

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 279


Totals Coverage Status
Change from base Build 261: -0.07%
Covered Lines: 799
Relevant Lines: 1046

💛 - Coveralls