search

informalsystems / hermes

IBC Relayer in Rust
https://hermes.informal.systems
Apache License 2.0
449 stars 329 forks source link

Channel handshake ignores `--src-chain` passed to `tx chan-open-init` #3989

Open joaotav opened 6 months ago

joaotav commented 6 months ago

Summary of Bug

When attempting to open a channel using hermes tx chan-open-init, the chain-id passed to --src-chain has no influence on the subsequent steps of the channel handshake. This allows a channel handshake to be started between a pair of chains chain1 and chain2, but end up creating a channel between a pair of chains chain2 and chain3.

Consider the following topology:

                                            connection-0
  +--------+                  +--------+ <---------------- +--------+   
  | chain1 |                  | chain2 |                   | chain3 | 
  +--------+                  +--------+ ----------------> +--------+ 
                                            connection-0

By executing the following commands: hermes tx chan-open-init --dst-chain chain2 --src-chain chain1 --dst-connection connection-0 --dst-port transfer --src-port transfer

hermes tx chan-open-try --dst-chain chain3 --dst-connection connection-0 --dst-port transfer --src-chain chain2 --src-channel channel-0 --src-port transfer

hermes tx chan-open-ack --dst-chain chain2 --src-chain chain3 --dst-connection connection-0 --dst-port transfer --src-port transfer --dst-channel channel-0 --src-channel channel-0

hermes tx chan-open-confirm --dst-chain chain3 --src-chain chain2 --dst-connection connection-0 --dst-port transfer --src-port transfer --dst-channel channel-0 --src-channel channel-0

chain2 has now a channel with chain3 in the OPEN state on both ends.

Notice that chain1 is passed to --src-chain on tx chan-open-init, but the channel is eventually opened between chain2 and chain3.

Version

hermes : hermes 1.8.2+06dfbafb simd : 8.0.0-beta.1-517-gedeefa8f0

Steps to Reproduce

Here's a minimum working example. Hermes config.toml file:

[global]
log_level = 'debug'

[mode]

[mode.clients]
enabled = true
refresh = true
misbehaviour = true

[mode.connections]
enabled = true

# Specify the channels mode.
[mode.channels]
enabled = true

[mode.packets]
enabled = true
clear_interval = 100
clear_on_start = true
tx_confirmation = true
auto_register_counterparty_payee = false

[rest]
enabled = false
host = '127.0.0.1'
port = 3000

[telemetry]
enabled = false
host = '127.0.0.1'
port = 3001

[telemetry.buckets]

[[chains]]
id = 'chain1'
ccv_consumer_chain = false
rpc_addr = 'http://127.0.0.1:16657'
grpc_addr = 'http://127.0.0.1:8090'
event_source = { mode = 'push', url = 'ws://127.0.0.1:16657/websocket', batch_delay = '500ms' }
rpc_timeout = '10s'
trusted_node = true
account_prefix = 'cosmos'
key_name = 'rly_chain1'
address_type = { derivation = 'cosmos' }
store_prefix = 'ibc'
default_gas = 10000000
max_gas = 10000000
gas_price = { price = 0.001, denom = 'stake' }
gas_multiplier = 1.2
max_msg_num = 30
max_tx_size = 2097152
clock_drift = '5s'
max_block_time = '30s'
trusting_period = '14days'
trust_threshold = { numerator = '2', denominator = '3' }
memo_prefix = ''

[[chains]]
id = 'chain2'
rpc_addr = 'http://127.0.0.1:26657'
grpc_addr = 'http://127.0.0.1:8091'
event_source = { mode = 'push', url = 'ws://127.0.0.1:26657/websocket', batch_delay = '500ms' }
rpc_timeout = '10s'
trusted_node = true
account_prefix = 'cosmos'
key_name = 'rly_chain2'
store_prefix = 'ibc'
default_gas = 10000000
max_gas = 10000000
gas_price = { price = 0.001, denom = 'stake' }
gas_multiplier = 1.2
max_msg_num = 30
max_tx_size = 2097152
clock_drift = '5s'
max_block_time = '30s'
trusting_period = '14days'
trust_threshold = { numerator = '1', denominator = '3' }
address_type = { derivation = 'cosmos' }

[[chains]]
id = 'chain3'
rpc_addr = 'http://127.0.0.1:36657'
grpc_addr = 'http://127.0.0.1:8092'
event_source = { mode = 'push', url = 'ws://127.0.0.1:36657/websocket', batch_delay = '500ms' }
rpc_timeout = '10s'
trusted_node = true
account_prefix = 'cosmos'
key_name = 'rly_chain3'
store_prefix = 'ibc'
default_gas = 10000000
max_gas = 10000000
gas_price = { price = 0.001, denom = 'stake' }
gas_multiplier = 1.2
max_msg_num = 30
max_tx_size = 2097152
clock_drift = '5s'
max_block_time = '30s'
trusting_period = '14days'
trust_threshold = { numerator = '1', denominator = '3' }
address_type = { derivation = 'cosmos' }

Replace HERMES_BINARY, HERMES_CONFIG_DIR and CHAIN_BIN with your own configs.

HERMES_BINARY= ...
HERMES_CONFIG_DIR= ...
CHAIN_BIN= ...

VAL_MNEMONIC_1="clock post desk civil pottery foster expand merit dash seminar song memory figure uniform spice circle try happy obvious trash crime hybrid hood cushion"
VAL_MNEMONIC_2="angry twist harsh drastic left brass behave host shove marriage fall update business leg direct reward object ugly security warm tuna model broccoli choice"
VAL_MNEMONIC_3="crawl off pony bamboo truck title lock brick toss enforce suit elephant save few wonder vacant walk tennis warrior sound harbor renew width stool"

WALLET_MNEMONIC_1="banner spread envelope side kite person disagree path silver will brother under couch edit food venture squirrel civil budget number acquire point work mass"
WALLET_MNEMONIC_2="veteran try aware erosion drink dance decade comic dawn museum release episode original list ability owner size tuition surface ceiling depth seminar capable only"
WALLET_MNEMONIC_3="vacuum burst ordinary enact leaf rabbit gather lend left chase park action dish danger green jeans lucky dish mesh language collect acquire waste load"

RLY_MNEMONIC_1="alley afraid soup fall idea toss can goose become valve initial strong forward bright dish figure check leopard decide warfare hub unusual join cart"
RLY_MNEMONIC_2="record gift you once hip style during joke field prize dust unique length more pencil transfer quit train device arrive energy sort steak upset"
RLY_MNEMONIC_3="street shrug return host gadget risk shadow mansion upset fish sure foster talk stable swim typical fish degree fall winner slam page raise own"

P2PPORT_1=16656
P2PPORT_2=26656
P2PPORT_3=36656

RPCPORT_1=16657
RPCPORT_2=26657
RPCPORT_3=36657

GRPCPORT_1=8090
GRPCPORT_2=8091
GRPCPORT_3=8092

RESTPORT_1=1316
RESTPORT_2=1317
RESTPORT_3=1318

$CHAIN_BIN init test --home chain_data/chain1 --chain-id chain1
$CHAIN_BIN init test --home chain_data/chain2 --chain-id chain2
$CHAIN_BIN init test --home chain_data/chain3 --chain-id chain3

echo $VAL_MNEMONIC_1 | $CHAIN_BIN keys add val1 --home chain_data/chain1 --recover --keyring-backend=test
echo $VAL_MNEMONIC_2 | $CHAIN_BIN keys add val2 --home chain_data/chain2 --recover --keyring-backend=test
echo $VAL_MNEMONIC_3 | $CHAIN_BIN keys add val3 --home chain_data/chain3 --recover --keyring-backend=test

echo $WALLET_MNEMONIC_1 | $CHAIN_BIN keys add wallet1 --home chain_data/chain1 --recover --keyring-backend=test
echo $WALLET_MNEMONIC_2 | $CHAIN_BIN keys add wallet1 --home chain_data/chain2 --recover --keyring-backend=test
echo $WALLET_MNEMONIC_3 | $CHAIN_BIN keys add wallet1 --home chain_data/chain3 --recover --keyring-backend=test

echo $RLY_MNEMONIC_1 | $CHAIN_BIN keys add rly_chain1 --home chain_data/chain1 --recover --keyring-backend=test 
echo $RLY_MNEMONIC_2 | $CHAIN_BIN keys add rly_chain2 --home chain_data/chain2 --recover --keyring-backend=test 
echo $RLY_MNEMONIC_3 | $CHAIN_BIN keys add rly_chain3 --home chain_data/chain3 --recover --keyring-backend=test 

$CHAIN_BIN genesis add-genesis-account $($CHAIN_BIN --home chain_data/chain1 keys show val1 --keyring-backend test -a) 100000000000stake  --home chain_data/chain1
$CHAIN_BIN genesis add-genesis-account $($CHAIN_BIN --home chain_data/chain2 keys show val2 --keyring-backend test -a) 100000000000stake  --home chain_data/chain2
$CHAIN_BIN genesis add-genesis-account $($CHAIN_BIN --home chain_data/chain3 keys show val3 --keyring-backend test -a) 100000000000stake  --home chain_data/chain3

$CHAIN_BIN genesis add-genesis-account $($CHAIN_BIN --home chain_data/chain1 keys show wallet1 --keyring-backend test -a) 100000000000stake  --home chain_data/chain1
$CHAIN_BIN genesis add-genesis-account $($CHAIN_BIN --home chain_data/chain2 keys show wallet1 --keyring-backend test -a) 100000000000stake  --home chain_data/chain2
$CHAIN_BIN genesis add-genesis-account $($CHAIN_BIN --home chain_data/chain3 keys show wallet1 --keyring-backend test -a) 100000000000stake  --home chain_data/chain3

$CHAIN_BIN genesis add-genesis-account $($CHAIN_BIN --home chain_data/chain1 keys show rly_chain1 --keyring-backend test -a) 100000000000stake  --home chain_data/chain1
$CHAIN_BIN genesis add-genesis-account $($CHAIN_BIN --home chain_data/chain2 keys show rly_chain2 --keyring-backend test -a) 100000000000stake  --home chain_data/chain2
$CHAIN_BIN genesis add-genesis-account $($CHAIN_BIN --home chain_data/chain3 keys show rly_chain3 --keyring-backend test -a) 100000000000stake  --home chain_data/chain3

$CHAIN_BIN genesis gentx val1 10000000000stake --home chain_data/chain1 --chain-id chain1 --keyring-backend test
$CHAIN_BIN genesis gentx val2 10000000000stake --home chain_data/chain2 --chain-id chain2 --keyring-backend test
$CHAIN_BIN genesis gentx val3 10000000000stake --home chain_data/chain3 --chain-id chain3 --keyring-backend test

$CHAIN_BIN genesis collect-gentxs --home chain_data/chain1
$CHAIN_BIN genesis collect-gentxs --home chain_data/chain2
$CHAIN_BIN genesis collect-gentxs --home chain_data/chain3

sed -i -e 's#"tcp://0.0.0.0:26656"#"tcp://0.0.0.0:'"$P2PPORT_1"'"#g' chain_data/"chain1"/config/config.toml
sed -i -e 's#"tcp://127.0.0.1:26657"#"tcp://0.0.0.0:'"$RPCPORT_1"'"#g' chain_data/"chain1"/config/config.toml
sed -i -e 's/timeout_commit = "5s"/timeout_commit = "1s"/g' chain_data/"chain1"/config/config.toml
sed -i -e 's/timeout_propose = "3s"/timeout_propose = "1s"/g' chain_data/"chain1"/config/config.toml
sed -i -e 's#"tcp://0.0.0.0:1317"#"tcp://0.0.0.0:'"$RESTPORT_1"'"#g' chain_data/"chain1"/config/app.toml

sed -i -e 's#"tcp://0.0.0.0:26656"#"tcp://0.0.0.0:'"$P2PPORT_2"'"#g' chain_data/"chain2"/config/config.toml
sed -i -e 's#"tcp://127.0.0.1:26657"#"tcp://0.0.0.0:'"$RPCPORT_2"'"#g' chain_data/"chain2"/config/config.toml
sed -i -e 's/timeout_commit = "5s"/timeout_commit = "1s"/g' chain_data/"chain2"/config/config.toml
sed -i -e 's/timeout_propose = "3s"/timeout_propose = "1s"/g' chain_data/"chain2"/config/config.toml
sed -i -e 's#"tcp://0.0.0.0:1317"#"tcp://0.0.0.0:'"$RESTPORT_2"'"#g' chain_data/"chain2"/config/app.toml

sed -i -e 's#"tcp://0.0.0.0:26656"#"tcp://0.0.0.0:'"$P2PPORT_3"'"#g' chain_data/"chain3"/config/config.toml
sed -i -e 's#"tcp://127.0.0.1:26657"#"tcp://0.0.0.0:'"$RPCPORT_3"'"#g' chain_data/"chain3"/config/config.toml
sed -i -e 's/timeout_commit = "5s"/timeout_commit = "1s"/g' chain_data/"chain3"/config/config.toml
sed -i -e 's/timeout_propose = "3s"/timeout_propose = "1s"/g' chain_data/"chain3"/config/config.toml
sed -i -e 's#"tcp://0.0.0.0:1317"#"tcp://0.0.0.0:'"$RESTPORT_3"'"#g' chain_data/"chain3"/config/app.toml

$CHAIN_BIN start --log_level info --log_format json --home chain_data/chain1  --grpc.address="0.0.0.0:$GRPCPORT_1" > chain_data/chain1.log 2>&1 &
$CHAIN_BIN start --log_level info --log_format json --home chain_data/chain2  --grpc.address="0.0.0.0:$GRPCPORT_2" > chain_data/chain2.log 2>&1 &
$CHAIN_BIN start --log_level info --log_format json --home chain_data/chain3  --grpc.address="0.0.0.0:$GRPCPORT_3" > chain_data/chain3.log 2>&1 &

sleep 10 # Allow chains to initialize

echo $RLY_MNEMONIC_1 | $HERMES_BINARY --config $HERMES_CONFIG_DIR keys add --chain chain1 --mnemonic-file /dev/stdin --key-name rly_chain1 --overwrite
echo $RLY_MNEMONIC_2 | $HERMES_BINARY --config $HERMES_CONFIG_DIR keys add --chain chain2 --mnemonic-file /dev/stdin --key-name rly_chain2 --overwrite
echo $RLY_MNEMONIC_3 | $HERMES_BINARY --config $HERMES_CONFIG_DIR keys add --chain chain3 --mnemonic-file /dev/stdin --key-name rly_chain3 --overwrite

$HERMES_BINARY --config $HERMES_CONFIG_DIR create client --host-chain chain2 --reference-chain chain3
$HERMES_BINARY --config $HERMES_CONFIG_DIR create client --host-chain chain3 --reference-chain chain2
$HERMES_BINARY --config $HERMES_CONFIG_DIR create connection --a-chain chain2 --a-client 07-tendermint-0 --b-client 07-tendermint-0

$HERMES_BINARY tx chan-open-init --dst-chain chain2 --src-chain chain1 --dst-connection connection-0 --dst-port transfer --src-port transfer
$HERMES_BINARY tx chan-open-try --dst-chain chain3 --dst-connection connection-0 --dst-port transfer --src-chain chain2 --src-channel channel-0 --src-port transfer
$HERMES_BINARY tx chan-open-ack --dst-chain chain2 --src-chain chain3 --dst-connection connection-0 --dst-port transfer --src-port transfer --dst-channel channel-0 --src-channel channel-0
$HERMES_BINARY tx chan-open-confirm --dst-chain chain3 --src-chain chain2 --dst-connection connection-0 --dst-port transfer --src-port transfer --dst-channel channel-0 --src-channel channel-0

We can check that the channel has been successfully opened with: simd --node tcp://localhost:26657 query ibc channel channels --output json | jq # For chain2 simd --node tcp://localhost:36657 query ibc channel channels --output json | jq # For chain3

Additional comments

The problems seems to be that Hermes checks --dst-chain for the existence of --dst-connection in tx chan-open-init, but does not check whether this connection is with the chain passed to --src-chain or not. So whichever chain is the counterparty of --dst-connection in --dst-chain can continue the handshake.

Acceptance Criteria

TBD

For Admin Use

ancazamfir commented 6 months ago

Great find and writeup @joaotav.

So build_chan_open_init() (called from build_chan_open_init_and_send) doesn't use the source chain and we indeed don't do any validation in the tx chan-open-init CLI when we build the Channel struct before calling build_chan_open_init_and_send

These tx .. CLIs were used early to help with the development process and were not really meant to be used in production. So not much attention was given to these, although we did some cleanup as we found issues. There are some redundant parameters in the tx CLIs and in this case probably src-chain param should be dropped and rather derived from the dst-connection.

The "production" CLI for creating channels does not allow both, e.g. you will see something like:

$ hermes create channel --a-chain chain2 --a-connection connection-0 --b-chain chain1 --a-port transfer --b-port transfer

error: The argument '--a-connection <A_CONNECTION_ID>' cannot be used with '--b-chain <B_CHAIN_ID>'

You can see here how we get the "src_chain" (chain_b in this function) from the "dst_connection" (connection_a). If we keep the "src_chain" parameter we can check it against the chain_b determined in a similar way.

There are other CLIs with redundant params and missing checks, for example if we continue with the try step we get a chain error because we don't do the check and send a bad IBC payload to the chain:

$ hermes tx chan-open-try --dst-chain chain1 --dst-connection connection-0 --dst-port transfer --src-chain chain2 --src-channel channel-0 --src-port transfer
...
ERROR channel error: failed during a transaction submission step to chain 'chain1': gRPC call `send_tx_simulate` failed with status: status: Unknown, message: "failed to execute message; message index: 1: channel handshake open try failed: failed channel state verification for client (07-tendermint-0): chained membership proof failed to verify membership of value: 080110011A0A0A087472616E73666572220C636F6E6E656374696F6E2D312A0769637332302D31 in subroot 8C1078ABB4CD0C802BBEC0A24A70DC91B47AE893ACA0C5471DDD1A685A7F48A4 at index 0. Please ensure the path and value are both correct.: invalid proof [cosmos/ibc-go/v8/modules/core/23-commitment/types/merkle.go:218] With gas wanted: '18446744073709551615' and gas used: '125054' ", details: [], metadata: MetadataMap { headers: {"content-type": "application/grpc", "x-cosmos-block-height": "1457"} }

As you see the error is really cryptic and not easy to figure out that this is a misconfiguration.

Note: while using the setup I found some bad UX:

2024-05-21T23:13:00.753873Z INFO ThreadId(01) using default configuration from '/Users/ancaz/.hermes/config.toml' 2024-05-21T23:13:00.762028Z INFO ThreadId(01) running Hermes v1.8.2+25f547595 ERROR channel end for transfer/channel-0 on chain chain2 @ Height { revision: 0, height: 1030 } does not have counterparty channel id: ChannelEnd { state: Init, ordering: Unordered, remote: Counterparty { port_id: PortId("transfer"), channel_id: None }, connection_hops: [ConnectionId("connection-0")], version: Version("ics20-1"), upgrade_sequence: 0 }

joaotav commented 6 months ago

Thanks for your investigation and insights @ancazamfir. I'll keep looking out for such issues as we go through each step of the channel handshake, and will open a tracking issue to clean up those CLIs in case we identify other problems.