Improve package release of deliverables - security

[loredanacirstea]

Our current development & release process is:

• change smart contracts source code -> we need to run make compile_contracts manually and also merge the new compiled contracts data, otherwise our CI tests fail (we have a test checking that the compiled data contains the correct source code checksums)
• when making a release:
  • we need to decide if we need to redeploy the contracts or not, by running the deployment script python -m raiden_contracts.deploy, which also updates our deployment_[CHAIN_NAME].json
  • there is no current check for the correctness of the deployment information
  • the correctness of the compiled data is checked by our current CI tests

We can see that there are ways to fail in the above process - e.g. forgetting that we changed the source code and releasing a package without updating the deployment addresses. Also, always having to change the compiled data for each source code changing PRs is not that clean in my opinion.

I am proposing some changes:

• updating the contracts source code should not trigger CI tests for the compiled contracts data (the contract tests run on the source code anyway, not on the compiled data)
• package releases (now done automatically by travis when pushing the tag to master):
  • must check that the compiled data contains the source code checksums
  • must check that the deployment files contain the correct contracts versions (same as the compiled data contracts_version)

[pirapira]

I'm not seeing the big picture.

How does the Python codebase specify the expected contract version(s)? By the hash of the deployed code? By a commit id in GitHub? By an address and a chain_id?

When are the contracts checked against the expectation? During the packaging of Raiden clients? At runtime? Both?

[loredanacirstea]

We have a:

• package version in https://github.com/raiden-network/raiden-contracts/blob/aea36ce403605670edc23fe0d14cf422e2b8e69b/setup.py#L15, controlled by .bumpversion.cfg

• global contracts version (same for all contracts in a package) in https://github.com/raiden-network/raiden-contracts/blob/aea36ce403605670edc23fe0d14cf422e2b8e69b/raiden_contracts/constants.py#L4 controlled by .bumpversion_contracts.cfg (past discussion about the above: https://github.com/raiden-network/raiden-contracts/issues/296)

• there are tests that check the contracts version for each contract - e.g. https://github.com/raiden-network/raiden-contracts/blob/aea36ce403605670edc23fe0d14cf422e2b8e69b/raiden_contracts/tests/test_secret_registry.py#L9-L11

• there is a test currently that checks if the current compiled data from contracts.json has the current contracts version: https://github.com/raiden-network/raiden-contracts/blob/aea36ce403605670edc23fe0d14cf422e2b8e69b/raiden_contracts/tests/test_contracts_compilation.py#L80-L82

• there are some tests on the deployment script (https://github.com/raiden-network/raiden-contracts/blob/aea36ce403605670edc23fe0d14cf422e2b8e69b/raiden_contracts/tests/test_deploy_script.py), that use components from the deploy script (default contracts version is the one from constants.py - this only checks wether the deployment information created after deploying on the tester chain is correct (also checking that the contracts version is correct - e.g. the one that would be stored in a deployment info file: https://github.com/raiden-network/raiden-contracts/blob/aea36ce403605670edc23fe0d14cf422e2b8e69b/raiden_contracts/data/deployment_kovan.json

When are the contracts checked against the expectation? During the packaging of Raiden clients? At runtime? Both?

So, currently:

• CI tests check wether the constants.py -> CONTRACTS_VERSION matches the compiled data contracts.json -> contracts_version
• when bumping the contracts version tests fail if you don't also re-compile contracts.json
• CI tests check that each contract has the same version as constants.py -> CONTRACTS_VERSION
• CI tests check that the deployment script produces data with the correct contracts_version

[pirapira]

Consider using the metadata info from Solidity. The deployed bytecode contains the keccack hash of this metadata at the end. The onchain metadata hash changes whenever the Solidity source or the compiler binary changes. Web3 exposes an interface for checking the code of an address.

The metadata hash can be found on chain like this:

The current version of the Solidity compiler thus adds the following to the end of the deployed bytecode: 0xa1 0x65 'b' 'z' 'z' 'r' '0' 0x58 0x20 <32 bytes swarm hash> 0x00 0x29

[pirapira]

Should a package release also contain deployment_mainnet.json and its siblings? Should the release happen before the deployment or after the deployment?

Maybe the first step is to document the current release process. I can try to follow the draft, and you'll notice I'm doing something wrong, and you can fix the draft, and so on.

[loredanacirstea]

Should a package release also contain deployment_mainnet.json and its siblings?

The package release should contain deployment_mainnet.json and this should only contain the info for the contracts that are mainnet ready - in our case, only the core contracts (not services).

Should the release happen before the deployment or after the deployment?

After deployment, because it needs to contain the deployment addresses.

Maybe the first step is to document the current release process.

Agree. We should have this documented - maybe a Release.md file. In short, this is:

1. create and verify that contracts.json file matches the current source code
2. deploy contracts with our deployment scripts and verify them on etherscan (we have a script for this that needs improvements)
3. Step 2 also creates the deployment_*.json files automatically & also verifies the correctness of the info against the chain.
4. Commit, make a PR
5. Push the release tag on master directly -> this triggers the package release in travis; at this point everything should be checked and verified already by the CI tests (compiled data from contracts.json) and the checks that we have in place (deployment info, checks are in the deployment scripts)