search

raiden-network / raiden

Raiden Network
https://developer.raiden.network
Other
1.84k stars 378 forks source link

Validate the ABI for non-official deployments of the Raiden contracts #3744

Open hackaugusto opened 5 years ago

hackaugusto commented 5 years ago

Abstract

Currently the proxies only check if the given address contains code [1], but it does not verify that the code in the address matches the expected ABI, this is because we assume the configuration is correct by default.

For our smart contracts we can check the address bytecode against the ABI's runtime bytecode, as long the deployment use our tools and the corresponding JSON file is provided. For third party smart contracts, e.g. tokens, another approach may be necessary, it can be done with bytecode checks, but this will have to be handled separately.

Motivation

RPC calls to eth_call and eth_sendRawTransaction will naturally fail, because the address is the wrong smart contract, as a consequence the proxies pre and/or pos conditions checks will report a wrong error, which is misleading and can be time consuming to debug. The goal is to save ourselves the trouble of detecting a bad configuration when a custom deployed smart contract is used (a deployment which is not in the raiden-contracts package, since these addresses are assumed to be correct).

Specification

For this to work, Raiden needs the smart contract address, the ABI, and its corresponding runtime bytecode to be available together. A file format should be formalized and the CLI configuration should be changed to use these files.

Related

Implement a whitelist for smartcontract code #344 (For the smart contracts that use libraries the check has to be done recursively and the linking has to be done at runtime.) This is not sufficient for custom deployements. With a custom deployment the compiler flags may differ, as a consequence the bytecode can also be different and runtime linking is not sufficient. For this reason the spec above requires both the ABI and the runtime code to be provided together.

related: #2117 raiden-network/raiden-contracts#359

Backwards Compatibility

This is just an additional correctness check, does not affect the protocol nor the database formats, so it's completely backwards compatible.

References

[1] Example of check for code in the given address:

https://github.com/raiden-network/raiden/blob/3eb936e4ad8a744c7d54643e01860e2bf0652207/raiden/network/proxies/token_network.py#L105-L109

hackaugusto commented 5 years ago

So, I opened this issue with our fixture setup in mind, the goal was to detect bad deployments in the tests early on, and save the time debugging a bad configuration. However, for our tests this issue is not sufficient, if we have bad interfaces (function wise) that allows us to deploy a smart contract and mix it with the wrong ABI, errors can still popup. However, this may make life easier for custom users, or for ourselves if we remove the smart contract deployment code from Raiden and rely exclusively on the tools from the raiden-contracts repo.

hackaugusto commented 5 years ago

Another simpler alternative, is to just call a view function that we know does not return empty, e.g. like how it is done in the token network contract:

https://github.com/raiden-network/raiden-contracts/blob/401544cef31326d155f89d1de1c1519079a912df/raiden_contracts/data/source/raiden/TokenNetwork.sol#L251-L252

Token: totalSupply EndpointRegistry: None SecretRegistry: None TokenNetworkRegistry: secret_registry_address TokenNetwork: token

contract_version cannot be used because all our smart contracts have it.