paritytech/parity-bridge

Deprecated Bridges

This repo is deprecated. Originally it contained the ETH <> ETH-PoA bridge (see tumski tag). Later it was repurposed for ETH-PoA <> Substrate bridge development, however we've decided to work on the bridge from scratch, and you can find the latest version in:

https://github.com/paritytech/parity-bridges-common

DISCLAIMER: we recommend not using the bridge in "production" (to bridge significant amounts) just yet. it's missing a code audit and should still be considered alpha. we can't rule out that there are bugs that might result in loss of the bridged amounts. we'll update this disclaimer once that changes

(contracts only)

Bridge is able to pass arbitrary messages between an Ethereum Proof of Authority chain and a Substrate Chain - either Standalone or Polkadot Parachain.

TODO: Docs

These docs are very incomplete yet. Describe high-level goals here in the (near) future.

parity-bridge's Issues

rewrite KovanBridge::CollectSignatuers solidity method

Currently it produces one event after n-th authority confirms the transaction. It's incorrect on many levels:

1. it's not possible to retrieve signatures from produced log
2. it's not possible to check (n-1)th authority, so if n-th authority is disconnected, other authorities don't know who should relay the transaction

consider splitting ForeignBridge.transfer into two functions

currently ForeignBridge.transfer takes a boolean as last argument that switches between two very different behaviours. false simply changes balances whereas true kicks off an entire relay.

parity-bridge/contracts/bridge.sol

Line 210 in 0149ef8

function transfer (address recipient, uint value, bool externalTransfer) {

imho this is confusing and easy to get wrong with potentially bad consequences.

i think it would be good to split this into two functions.
transferOnChain(recipient, value) and transferToHomeChain(recipient, value) for example.

usage of eth_sign needs fix

eth_sign is actually sign(keccak256("\x19Ethereum Signed Message:\n" + len(message) + message)))

solidity contract also needs to prepend this dumb string before hashing ;/

write tests for HomeBridge.withdraw

• should allow correct withdraw
• should not allow withdraw if sender is not authority
  • everyone can call withdraw. it does multisig authority validation which is enough of a check
• should allow second withdraw with different transactionHash but same recipient and value
• should not allow second withdraw with same transactionHash but different recipient and value
• should allow correct withdraw with multiple signatures
• should not allow withdraw from misbehaving authority
• should not allow withdraw with duplicate signature
• should not allow withdraw without funds
• should not allow withdraw with message.length != 84
• should not allow reentry (DAO-bug) in withdraw

Transaction from bridge is not mined in Kovan (Foreign side)

Parity/v1.8.0-beta
Bridge is from master branch.
Home is private PoA network with spec and toml. Foreign is Kovan. Bridge config is here.
ForeignBridge contract: https://kovan.etherscan.io/address/0xdcf351544fdfe470a7b618928642399b93ffa958#code

Investor: 0xEaeBA7869E23A328a0A92620BbA1A7a6aaED26cB
0.001 eth is sent

Logs:

Private PoA side (home):

2017-10-18 18:36:42   INFO own_tx  Transaction mined (hash 9f4328917ecae09c81bc94295327c2254bd8d88f152464e51f444b80e013ee4d)

Kovan side (foreign):

2017-10-18 18:36:42   TRACE rpc  Request: {"jsonrpc":"2.0","method":"eth_sendTransaction","params":[{"data":"0x26b3293f000000000000000000000000eaeba7869e23a328a0a92620bba1a7a6aaed26cb00000000000000000000000000000000000000000000000000038d7ea4c680009f4328917ecae09c81bc94295327c2254bd8d88f152464e51f444b80e013ee4d","from":"0x00db9af45c6f241432f2cbe412c6969cb7778d98","gas":"0x186a0","gasPrice":"0x0","to":"0xdcf351544fdfe470a7b618928642399b93ffa958"}],"id":4097}.
2017-10-18 18:36:42   DEBUG rpc  [Some(Num(4097))] Took 68ms
2017-10-18 18:36:42   DEBUG rpc  Response: {"jsonrpc":"2.0","result":"0x749e745a593323cae6b41bbda1e3c80405e23e37ac23f4f069a0565a1bd1b491","id":4097}.

But transaction with hash 0x749e745a593323cae6b41bbda1e3c80405e23e37ac23f4f069a0565a1bd1b491 can't be find as mined in Kovan. I can't find it in Etherscan.

Another one try with full trace at Kovan side logging="poa=trace,signer=trace,engine=trace,own_tx=trace,txqueue=trace,sync=trace"

Private PoA side (home):

2017-10-18 19:15:15   INFO own_tx  Transaction mined (hash 4911eaec4cadc2469ba9b43767957757ae0e011e3f2ad57bb28825f9ea6cc03e)

Kovan side (foreign):

2017-10-18 19:15:15   TRACE own_tx  Importing transaction: PendingTransaction { transaction: SignedTransaction { transaction: UnverifiedTransaction { unsigned: Transaction { nonce: 73, gas_price: 0, gas: 100000, action: Call(dcf351544fdfe470a7b618928642399b93ffa958), value: 0, data: [38, 179, 41, 63, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 234, 235, 167, 134, 158, 35, 163, 40, 160, 169, 38, 32, 187, 161, 167, 166, 170, 237, 38, 203, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 141, 126, 164, 198, 128, 0, 73, 17, 234, 236, 76, 173, 194, 70, 155, 169, 180, 55, 103, 149, 119, 87, 174, 14, 1, 30, 63, 42, 213, 123, 178, 136, 37, 249, 234, 108, 192, 62] }, v: 120, r: 40664849902588265502832923571841725693800088266545607532580286953484288571259, s: 46970124881255361458992124855158539901137055806175960944316180293039990823097, hash: 19c9029588ffeb599e495361acc41cd2dfc3e0b81f5d3dc2986f7aa65823e3e9 }, sender: 00db9af45c6f241432f2cbe412c6969cb7778d98, public: Some(fd915be0765b59747bf0cd6a46fdefb52a85302bbdcd3c5b5e3ad59107afa6f4d212ddbfe831dd58786f641147277e75650a3955473de05771a1d323330b9d4e) }, condition: None }
2017-10-18 19:15:15   TRACE txqueue  Inserting: TransactionOrder { nonce_height: 4, gas_price: 0, gas_factor: 0, gas: 100000, mem_usage: 112, strategy: GasPriceOnly, hash: 19c9029588ffeb599e495361acc41cd2dfc3e0b81f5d3dc2986f7aa65823e3e9, insertion_id: 549, origin: Local, penalties: 0 }
2017-10-18 19:15:15   DEBUG txqueue  Imported transaction to current: 19c9029588ffeb599e495361acc41cd2dfc3e0b81f5d3dc2986f7aa65823e3e9
2017-10-18 19:15:15   DEBUG txqueue  status: TransactionQueueStatus { pending: 6, future: 133 }
2017-10-18 19:15:15   DEBUG own_tx  Imported to Current (hash 19c9029588ffeb599e495361acc41cd2dfc3e0b81f5d3dc2986f7aa65823e3e9)
2017-10-18 19:15:15   TRACE own_tx  Status: TransactionQueueStatus { pending: 6, future: 133 }
2017-10-18 19:15:15   TRACE engine  Fetched proposer for step 377085828: 0020…81a1
2017-10-18 19:15:15   TRACE engine  generate_seal: 00db…8d98 not a proposer for step 377085828.
201

introduce/use labels from https://github.com/paritytech/parity/wiki/Labelling

is there an automated way to do so?

fuzz/stress-test/property-test and assert that invariants hold

maybe we can cook up something with https://github.com/tomusdrw/sol-rs and https://github.com/BurntSushi/quickcheck (or just a simple generator) that creates pseudorandom action sequences (withdraw, deposit) and validator faults, runs them against the whole bridge and asserts that relays never get stuck, the total amount of value in the system stays constant and other invariants

it's not as good as formal verification or model checking but would get us a quite far on that track without that much effort

invariants

• withdraw is the only function authorities ever call on HomeBridge
  • else attacks that deplete authorities funds could be possible
    • see #54
• ensure that the total amount of value in the system is correct

tests for withdraw confirm

similar to #6

CollectedSignatures authority should be an indexed topic

Invalid contracts' binary code at deploy stage

This is a left-side contract of bridge (mainnet_contract) deployed with the bridge in the Kovan network:
https://kovan.etherscan.io/tx/0x283e8365f60c4cc7f5d62377fa0e8fb48a29b93ba65e2622962c1074b72467ba

The transaction is marked as invalid because the invalid binary of contract is sent: if check the binary, it is the hex of the real binary of contract, that is stored in .\bridge\src\contracts\Ethereum.bridge.json. It seem, that double hex is somewhere in the code.

make App::ensured_deployed a Future

separate binary from a library

Purpose of truncate function

    function truncate (address[] storage self, uint len) internal {
        for (uint i = len; i < self.length; i++) {
            delete self[i];
        }
        self.length = len;
    }

it's not used.
Question is what is the purpose of this function?

extend test_app_stream macro with transaction settings

needed for #7 #8 #9 #18

Authorities should depend on a ValidatorSet contract

https://github.com/paritytech/parity/wiki/Validator-Set#contracts

fix solidity compilation warnings

Add license

tests for withdraw relay

similar to #6

contract's abi should not be configurable

replace json containing raw payloads in stream tests

currently the stream tests assert against raw json rpc bodies containing raw hex payloads

example:

parity-bridge/tests/tests/withdraw_relay.rs

Line 30 in 7e6d443

res => r#""0x1011""#;

replace with something easier to write and maintain

replicate truffle tests with sol-rs

https://github.com/paritytech/sol-rs

• to exercise the sol-rs api and help improve it
• in preparation for #62
• and later #63

integration tests to confirm that both contracts and the rust bridge code work together correctly

use https://github.com/paritytech/sol-rs so we can test the whole system in rust

has higher priority than #63

tests to write:

• deposit should not get relayed if it doesn't get mined on home_chain
• deposit should get relayed if it gets mined and authorities are honest
• deposit should get relayed if (authorities.length - requiredSignatures) authorities are malicious
• deposit should not get relayed if (authorities.length - requiredSignatures + 1) authorities are malicious
• deposit should get relayed if submitSignature repeatedly doesn't get mined on foreign_chain
• withdraw should not get relayed if it doesn't get included on foreign_chain
• withdraw should get relayed if it gets included and authorities are honest
• withdraw should get relayed if (authorities.length - requiredSignatures) authorities are malicious
• withdraw should get relayed if authority initially responsible for relay is malicous
• withdraw should not get relayed if (authorities.length - requiredSignatures + 1) authorities are malicious
• withdraw should get relayed if withdraw repeatedly doesn't get mined on home_chain

refactor ethabi Event::decode_log

have solc versions for test and deploy match up

single malicious authority can block withdraw

currently for each withdraw one authority (the one submitting the last required signature) is selected to do the relay (call HomeBridge.withdraw).

if this authority is malicious and doesn't call HomeBridge.withdraw then the ether is never paid out on home_chain.

we should remove this single point of failure

update:
withdraws are not "stuck forever". anyone can execute a transaction to make them complete. it's just that this doesn't happen automatically. also read: #64 (comment)

check, write tests and audit for solidity functions

make authorities configurable from config file level

The balance of the investor in testnet doesn't change

Steps to reproduce:

1. I've started Parity as described in docs with this parity-spec.json:

{
	"name": "BridgeExample",
	"engine": {
		"instantSeal": { "params": {} }
	},
	"params": {
		"gasLimitBoundDivisor": "0x0400",
		"accountStartNonce": "0x0",
		"maximumExtraDataSize": "0x20",
		"minGasLimit": "0x1388",
		"networkID" : "0x11"
	},
	"genesis": {
		"seal": {
			"generic": "0x0"
		},
		"difficulty": "0x20000",
		"author": "0x0000000000000000000000000000000000000000",
		"timestamp": "0x00",
		"parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
		"extraData": "0x",
		"gasLimit": "0x5B8D80"
	},
	"accounts": {
		"0000000000000000000000000000000000000001": { "balance": "1", "builtin": { "name": "ecrecover", "pricing": { "linear": { "base": 3000, "word": 0 } } } },
		"0000000000000000000000000000000000000002": { "balance": "1", "builtin": { "name": "sha256", "pricing": { "linear": { "base": 60, "word": 12 } } } },
		"0000000000000000000000000000000000000003": { "balance": "1", "builtin": { "name": "ripemd160", "pricing": { "linear": { "base": 600, "word": 120 } } } },
		"0000000000000000000000000000000000000004": { "balance": "1", "builtin": { "name": "identity", "pricing": { "linear": { "base": 15, "word": 3 } } } },
		"0xcace118480aa444aa8fe4318d97b05be927ca127": { "balance": "1606938044258990275541962092341162602522202993782792835301376" }
	}
}

2. and config:

[parity]
chain = "examples/parity_spec.json"

[account]
unlock = ["0xcace118480aa444aa8fe4318d97b05be927ca127"]
password = ["examples/parity_password"]

[mining]
engine_signer = "0xcace118480aa444aa8fe4318d97b05be927ca127"
reseal_on_txs = "all"
usd_per_tx = "0"
reseal_min_period = 0

3. The bridge config is:

[mainnet]
account = "0xcace118480aa444aa8fe4318d97b05be927ca127"
ipc = "/Users/viktor/Library/Application Support/io.parity.ethereum/jsonrpc.ipc"
required_confirmations = 0
request_timeout = 120

[mainnet.contract]
bin = "contracts/EthereumBridge.bin"

[testnet]
account = "0xcace118480aa444aa8fe4318d97b05be927ca127"
ipc = "/Users/viktor/Library/Application Support/io.parity.ethereum/jsonrpc.ipc"
required_confirmations = 0
request_timeout = 120

[testnet.contract]
bin = "contracts/KovanBridge.bin"

[authorities]
accounts = [
	"0xcace118480aa444aa8fe4318d97b05be927ca127"
]
required_signatures = 1

[transactions]
mainnet_deploy = { gas = 4538560 }
testnet_deploy = { gas = 4538560 }

So, the bridge listens the same instance for "testnet" and "mainnet".

4. I've started bridge: contracts are deployed successfully, the database file with the addresses of contracts is created.
5. I sent some eth to "mainnet" contract from address A = 0xcace118480aa444aa8fe4318d97b05be927ca127. I've checked, that balance of the contract is changed. I've checked, that Deposit Event was appeared. But balances method for the same address A at "testnet" contract still returns 0.

What am I doing wrong?

get to 100% test coverage for the solidity contracts

already at 100% line coverage

Rename to parity-bridge

Use more generic naming

The bridge can connect any ValidatorSet based Parity chain (not just testnet) with any other Parity chain (not just mainnet).

Primary/secondary, home/foreign ...?

republishing transactions

It may happen, that relayed transaction are not included in a block, or an authority responsible for relaying them is malicious and hasn't done that. In such case, we should republish the transaction

use solium for linting

see setup in https://github.com/paritytech/contracts

Describe required parity version in the docs

tests for deposit relay

similar to #6

in HomeBridge.withdraw: recipient gets loaded from message with wrong offset and contains wrong data

code in question:

parity-bridge/contracts/bridge.sol

Line 119 in 7406015

recipient := mload(add(message, 0x20))

look here for how message is constructed:

parity-bridge/bridge/src/bridge/withdraw_confirm.rs

Line 29 in 5940396

result[0..20].copy_from_slice(&withdraw_log.recipient);

message format:

• 20 bytes recipient
• 32 bytes value
• 32 bytes transactionHash

the code currently mistakenly thinks the format is:

• 32 bytes recipient
• 32 bytes value
• 32 bytes transactionHash

all three values read in HomeBridge.withdraw are corrupted because of this.

found while writing tests for HomeBridge.withdraw (#60).

i've got a fix on the issue-54-close-attack-vector branch:
fed0cc1

merge in tests from https://github.com/oraclesorg/parity-bridge

https://github.com/oraclesorg have written more tests on https://github.com/oraclesorg/parity-bridge. we should integrate them

config transactions should not be a property of node

append constructor args to contract creation transaction request

Potential attack vector

Current version of the Bridge doesn't charge users for making withdraws so that Authorities running the Bridge have to cover all costs. This potentially allows an attacker to deplete Authorities’ funds.

Potential solution (proposed by @debris ):

Withdraws should not be free for users. The exact amount of gas that is spent on a withdraw should be measured and charged to users. It's a simple feature which could be added to function transfer() of ForeignBridge smart contract. Truffle tests should be written and run. Truffle tests for foreign contract are located in truffle/test/foreign.js.

add configurable request timeout

compiled contracts in version control

currently some compiled contracts .bin and .abi are checked into git.

these have the tendency to get stale. it's cumbersome and easy to forget to keep them up to date.

why are they checked in?
should we remove them from version control?

afaik https://github.com/paritytech/contracts doesn't have them checked in either

compute code coverage for truffle tests

follow up by writing more tests that put coverage to 100%

Add documentation for config file

transaction options should have a default values

docs

ecrecover from bytes should be a library function

audit for contract code

replaces #4

CI should run truffle tests

for tests maybe use ES2015 with babel as in https://github.com/paritytech/contracts/

tests for api::LogStream

similar to rust-web tests https://github.com/tomusdrw/rust-web3/blob/9dabf9be5cfa8a8b5df67182e3f30bd8a419bd72/src/api/eth_filter.rs#L424-L444