Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add bulk upgrade CLI #1077

Merged
merged 16 commits into from
Jul 19, 2021
Merged

Add bulk upgrade CLI #1077

merged 16 commits into from
Jul 19, 2021

Conversation

hu55a1n1
Copy link
Member

Closes: #763

Description

Add a bulk upgrade CLI command that takes a source chain's id as input and upgrades clients for all chains in config target the specified source chain client.


For contributor use:

  • Updated the Unreleased section of CHANGELOG.md with the issue.
  • If applicable: Unit tests written, added test to CI.
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Updated relevant documentation (docs/) and code comments.
  • Re-reviewed Files changed in the Github PR explorer.

relayer-cli/src/conclude.rs Outdated Show resolved Hide resolved
relayer-cli/src/conclude.rs Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
relayer-cli/src/commands/tx/client.rs Outdated Show resolved Hide resolved
relayer-cli/src/commands/tx/client.rs Outdated Show resolved Hide resolved
relayer-cli/src/commands/tx/client.rs Outdated Show resolved Hide resolved
relayer-cli/src/commands/tx/client.rs Outdated Show resolved Hide resolved
relayer-cli/src/commands/tx/client.rs Outdated Show resolved Hide resolved
relayer-cli/src/conclude.rs Outdated Show resolved Hide resolved
@hu55a1n1
Copy link
Member Author

hu55a1n1 commented Jul 6, 2021

I tried testing this using hermes guide - testing client upgrade. I modified the procedure slightly to test with multiple host chains (i.e. ibc-1 & ibc-2) targeting a single chain (i.e. ibc-0) using the following commands ->

$ cat ~/.hermes/config.toml
[global]
strategy = 'packets'
log_level = 'info'

[[chains]]
id = 'ibc-0'
rpc_addr = 'http://127.0.0.1:26657'
grpc_addr = 'http://127.0.0.1:9090'
websocket_addr = 'ws://localhost:26657/websocket'
rpc_timeout = '10s'
account_prefix = 'cosmos'
key_name = 'testkey'
store_prefix = 'ibc'
gas = 200000
fee_denom = 'stake'
fee_amount = 10
clock_drift = '5s'
trusting_period = '14days'

[chains.trust_threshold]
numerator = '1'
denominator = '3'

[[chains]]
id = 'ibc-1'
rpc_addr = 'http://127.0.0.1:26557'
grpc_addr = 'http://127.0.0.1:9091'
websocket_addr = 'ws://localhost:26557/websocket'
rpc_timeout = '10s'
account_prefix = 'cosmos'
key_name = 'testkey'
store_prefix = 'ibc'
gas = 200000
fee_denom = 'stake'
fee_amount = 10
clock_drift = '5s'
trusting_period = '14days'

[chains.trust_threshold]
numerator = '1'
denominator = '3'

[[chains]]
id = 'ibc-2'
rpc_addr = 'http://127.0.0.1:26457'
grpc_addr = 'http://127.0.0.1:9092'
websocket_addr = 'ws://localhost:26457/websocket'
rpc_timeout = '10s'
account_prefix = 'cosmos'
key_name = 'testkey'
store_prefix = 'ibc'
gas = 200000
fee_denom = 'stake'
fee_amount = 10
clock_drift = '5s'
trusting_period = '14days'

[chains.trust_threshold]
numerator = '1'
denominator = '3'

$ ./scripts/dev-env ~/.hermes/config.toml ibc-0 ibc-1 ibc-2
WARNING: ./scripts/setup-chains will DELETE the '/home/hussaini/Documents/ibc-rs/data' folder.
> Do you wish to continue? (y/n): y
GAIA VERSION INFO: v4.2.1
Generating gaia configurations...
Creating gaiad instance: home=./data | chain-id=ibc-0 | p2p=:26656 | rpc=:26657 | profiling=:6060 | grpc=:9090 | samoleans=:100000000000
Change settings in config.toml file...
Start gaia on grpc port: 9090...
Balances for validator 'cosmos1hp2dcwqycyfcpn9xtg4tn9hv65klfks33szfgv' @ 'tcp://localhost:26657'
balances:
- amount: "0"
  denom: stake
pagination:
  next_key: null
  total: "0"
Balances for user 'cosmos1x9ueqshn8grl7qtj8agp6j359f4snm8yfgznd9' @ 'tcp://localhost:26657'
balances:
- amount: "100000000000"
  denom: samoleans
- amount: "100000000000"
  denom: stake
pagination:
  next_key: null
  total: "0"
Balances for user 'cosmos1qhv4lcqm772h4d9k2dfrsx2gvyfeh0gr8jf4zv' @ 'tcp://localhost:26657'
balances:
- amount: "100000000000"
  denom: samoleans
- amount: "100000000000"
  denom: stake
pagination:
  next_key: null
  total: "0"
Creating gaiad instance: home=./data | chain-id=ibc-1 | p2p=:26556 | rpc=:26557 | profiling=:6061 | grpc=:9091 | samoleans=:100000000000
Change settings in config.toml file...
Start gaia on grpc port: 9091...
Balances for validator 'cosmos196rtz4g28e5mc3nuwtd0j4tw5uxkkye6mv5hs7' @ 'tcp://localhost:26557'
balances: []
pagination:
  next_key: null
  total: "0"
Balances for user 'cosmos1ph9cnw62mlgd997whfjdrgcuaq3a7usypt090f' @ 'tcp://localhost:26557'
balances: []
pagination:
  next_key: null
  total: "0"
Balances for user 'cosmos1t3mmfnrprxl46gn4cru654gktzu5t7uf64flst' @ 'tcp://localhost:26557'
balances: []
pagination:
  next_key: null
  total: "0"
Creating gaiad instance: home=./data | chain-id=ibc-2 | p2p=:26456 | rpc=:26457 | profiling=:6062 | grpc=:9092 | samoleans=:100000000000
Change settings in config.toml file...
Start gaia on grpc port: 9092...
Balances for validator 'cosmos1ug9yrmkta2suar9cu4c3cfgh4d4gc4p5pekdne' @ 'tcp://localhost:26457'
balances: []
pagination:
  next_key: null
  total: "0"
Balances for user 'cosmos1d7llpvl2jt3ndwk3h3gjstxv7mlj9cjlhandyx' @ 'tcp://localhost:26457'
balances: []
pagination:
  next_key: null
  total: "0"
Balances for user 'cosmos1z7cvy9pz5emvshrqtplquyy2rw42fnn66m6fzt' @ 'tcp://localhost:26457'
balances: []
pagination:
  next_key: null
  total: "0"
ibc-0 initialized. Watch file /home/hussaini/Documents/ibc-rs/data/ibc-0.log to see its execution.
ibc-1 initialized. Watch file /home/hussaini/Documents/ibc-rs/data/ibc-1.log to see its execution.
ibc-2 initialized. Watch file /home/hussaini/Documents/ibc-rs/data/ibc-2.log to see its execution.
Building the Rust relayer...

Importing keys...
Success: Added key 'testkey' (cosmos1x9ueqshn8grl7qtj8agp6j359f4snm8yfgznd9) on chain ibc-0
Success: Added key 'user2' (cosmos1qhv4lcqm772h4d9k2dfrsx2gvyfeh0gr8jf4zv) on chain ibc-0
Success: Added key 'testkey' (cosmos1ph9cnw62mlgd997whfjdrgcuaq3a7usypt090f) on chain ibc-1
Success: Added key 'user2' (cosmos1t3mmfnrprxl46gn4cru654gktzu5t7uf64flst) on chain ibc-1
Success: Added key 'testkey' (cosmos1d7llpvl2jt3ndwk3h3gjstxv7mlj9cjlhandyx) on chain ibc-2
Success: Added key 'user2' (cosmos1z7cvy9pz5emvshrqtplquyy2rw42fnn66m6fzt) on chain ibc-2

$ hermes create client ibc-1 ibc-0
Success: CreateClient(
    CreateClient(
        Attributes {
            height: Height {
                revision: 1,
                height: 1858,
            },
            client_id: ClientId(
                "07-tendermint-0",
            ),
            client_type: Tendermint,
            consensus_height: Height {
                revision: 0,
                height: 1864,
            },
        },
    ),
)

$ hermes create client ibc-2 ibc-0
Jul 06 14:56:05.675  INFO ibc_relayer_cli::commands: using default configuration from '/home/hussaini/.hermes/config.toml'
Success: CreateClient(
    CreateClient(
        Attributes {
            height: Height {
                revision: 2,
                height: 1923,
            },
            client_id: ClientId(
                "07-tendermint-0",
            ),
            client_type: Tendermint,
            consensus_height: Height {
                revision: 0,
                height: 1942,
            },
        },
    ),
)

$ hermes tx raw upgrade-chain ibc-0 ibc-1 07-tendermint-0 10000000 300
Jul 06 15:00:01.034  INFO ibc_relayer_cli::commands: using default configuration from '/home/hussaini/.hermes/config.toml'
Jul 06 15:00:01.035  INFO ibc_relayer_cli::commands::tx::upgrade: Message UpdatePlanOptions { src_chain_config: ChainConfig { id: ChainId { id: "ibc-1", version: 1 }, rpc_addr: Url { inner: Url { scheme: "http", cannot_be_a_base: false, username: "", password: None, host: Some(Ipv4(127.0.0.1)), port: Some(26557), path: "/", query: None, fragment: None }, scheme: Http, host: "127.0.0.1", port: 26557 }, websocket_addr: Url { inner: Url { scheme: "ws", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("localhost")), port: Some(26557), path: "/websocket", query: None, fragment: None }, scheme: WebSocket, host: "localhost", port: 26557 }, grpc_addr: Url { inner: Url { scheme: "http", cannot_be_a_base: false, username: "", password: None, host: Some(Ipv4(127.0.0.1)), port: Some(9091), path: "/", query: None, fragment: None }, scheme: Http, host: "127.0.0.1", port: 9091 }, rpc_timeout: 10s, account_prefix: "cosmos", key_name: "testkey", store_prefix: "ibc", gas: Some(200000), fee_denom: "stake", fee_amount: Some(10), max_msg_num: None, max_tx_size: None, clock_drift: 5s, trusting_period: 1209600s, trust_threshold: TrustThresholdFraction { numerator: 1, denominator: 3 } }, dst_chain_config: ChainConfig { id: ChainId { id: "ibc-0", version: 0 }, rpc_addr: Url { inner: Url { scheme: "http", cannot_be_a_base: false, username: "", password: None, host: Some(Ipv4(127.0.0.1)), port: Some(26657), path: "/", query: None, fragment: None }, scheme: Http, host: "127.0.0.1", port: 26657 }, websocket_addr: Url { inner: Url { scheme: "ws", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("localhost")), port: Some(26657), path: "/websocket", query: None, fragment: None }, scheme: WebSocket, host: "localhost", port: 26657 }, grpc_addr: Url { inner: Url { scheme: "http", cannot_be_a_base: false, username: "", password: None, host: Some(Ipv4(127.0.0.1)), port: Some(9090), path: "/", query: None, fragment: None }, scheme: Http, host: "127.0.0.1", port: 9090 }, rpc_timeout: 10s, account_prefix: "cosmos", key_name: "testkey", store_prefix: "ibc", gas: Some(200000), fee_denom: "stake", fee_amount: Some(10), max_msg_num: None, max_tx_size: None, clock_drift: 5s, trusting_period: 1209600s, trust_threshold: TrustThresholdFraction { numerator: 1, denominator: 3 } }, src_client_id: ClientId("07-tendermint-0"), amount: 10000000, height_offset: 300 }
Success: []

$ gaiad tx gov vote 1 yes --home data/ibc-0/data/ --keyring-backend test --keyring-dir data/ibc-0/ --chain-id ibc-0 --from validator
{"body":{"messages":[{"@type":"/cosmos.gov.v1beta1.MsgVote","proposal_id":"1","voter":"cosmos1hp2dcwqycyfcpn9xtg4tn9hv65klfks33szfgv","option":"VOTE_OPTION_YES"}],"memo":"","timeout_height":"0","extension_options":[],"non_critical_extension_options":[]},"auth_info":{"signer_infos":[],"fee":{"amount":[],"gas_limit":"200000","payer":"","granter":""}},"signatures":[]}

confirm transaction before signing and broadcasting [y/N]: y
{"height":"2294","txhash":"A2AD88BA7C21B06773C9F2936EF506DF2670DE0309A7E3FBF00C5CD8AA8243F7","codespace":"","code":0,"data":"0A060A04766F7465","raw_log":"[{\"events\":[{\"type\":\"message\",\"attributes\":[{\"key\":\"action\",\"value\":\"vote\"},{\"key\":\"module\",\"value\":\"governance\"},{\"key\":\"sender\",\"value\":\"cosmos1hp2dcwqycyfcpn9xtg4tn9hv65klfks33szfgv\"}]},{\"type\":\"proposal_vote\",\"attributes\":[{\"key\":\"option\",\"value\":\"VOTE_OPTION_YES\"},{\"key\":\"proposal_id\",\"value\":\"3\"}]}]}]","logs":[{"msg_index":0,"log":"","events":[{"type":"message","attributes":[{"key":"action","value":"vote"},{"key":"module","value":"governance"},{"key":"sender","value":"cosmos1hp2dcwqycyfcpn9xtg4tn9hv65klfks33szfgv"}]},{"type":"proposal_vote","attributes":[{"key":"option","value":"VOTE_OPTION_YES"},{"key":"proposal_id","value":"1"}]}]}],"info":"","gas_wanted":"200000","gas_used":"43716","tx":null,"timestamp":""}

$ gaiad query gov proposal 1 --home data/ibc-0/
content:
  '@type': /cosmos.upgrade.v1beta1.SoftwareUpgradeProposal
  description: upgrade the chain software and unbonding period
  plan:
    height: "2564"
    info: upgrade the chain software and unbonding period
    name: test
    time: "0001-01-01T00:00:00Z"
    upgraded_client_state:
      '@type': /ibc.lightclients.tendermint.v1.ClientState
      allow_update_after_expiry: false
      allow_update_after_misbehaviour: false
      chain_id: ibc-0
      frozen_height:
        revision_height: "0"
        revision_number: "0"
      latest_height:
        revision_height: "2565"
        revision_number: "0"
      max_clock_drift: 0s
      proof_specs:
      - inner_spec:
          child_order:
          - 0
          - 1
          child_size: 33
          empty_child: null
          hash: SHA256
          max_prefix_length: 12
          min_prefix_length: 4
        leaf_spec:
          hash: SHA256
          length: VAR_PROTO
          prefix: AA==
          prehash_key: NO_HASH
          prehash_value: SHA256
        max_depth: 0
        min_depth: 0
      - inner_spec:
          child_order:
          - 0
          - 1
          child_size: 32
          empty_child: null
          hash: SHA256
          max_prefix_length: 1
          min_prefix_length: 1
        leaf_spec:
          hash: SHA256
          length: VAR_PROTO
          prefix: AA==
          prehash_key: NO_HASH
          prehash_value: SHA256
        max_depth: 0
        min_depth: 0
      trust_level:
        denominator: "0"
        numerator: "0"
      trusting_period: 0s
      unbonding_period: 1440000s
      upgrade_path:
      - upgrade
      - upgradedIBCState
  title: upgrade_ibc_clients
deposit_end_time: "2021-07-06T09:38:52.973644456Z"
final_tally_result:
  abstain: "0"
  "no": "0"
  no_with_veto: "0"
  "yes": "100000000000"
proposal_id: "1"
status: PROPOSAL_STATUS_PASSED
submit_time: "2021-07-06T09:35:32.973644456Z"
total_deposit:
- amount: "10000000"
  denom: stake
voting_end_time: "2021-07-06T09:38:52.973644456Z"
voting_start_time: "2021-07-06T09:35:32.973644456Z"

But when I try the upgrade clients command I get the below error ->

$ hermes upgrade clients ibc-0
Jul 06 15:11:36.189  INFO ibc_relayer_cli::commands: using default configuration from '/home/hussaini/.hermes/config.toml'
Jul 06 15:11:36.270  INFO ibc_relayer::foreign_client: [ibc-1 -> ibc-0:07-tendermint-0] upgrade Height: 1-2465
Jul 06 15:11:36.323  INFO ibc_relayer::foreign_client: [ibc-2 -> ibc-0:07-tendermint-0] upgrade Height: 2-2454
Error: .
├─.
├─.
│ └─ query error: error raised while updating client: failed querying client state on dst chain 07-tendermint-0 with error: Query error occurred (failed to query for client state): error converting message type into domain type: the client state was not found
└─.
  └─ query error: error raised while updating client: failed querying client state on dst chain 07-tendermint-0 with error: Query error occurred (failed to query for client state): error converting message type into domain type: the client state was not found

Interestingly, when I change the ForeignClient::restore() with ForeignClient::find() here based on what the upgrade client command is doing, the upgrade clients command works without a problem. Still trying to figure out why this is the case.

@hu55a1n1 hu55a1n1 force-pushed the hu55a1n1/cli-bulk-upgrade branch from 5930552 to 6d454df Compare July 8, 2021 18:18
@adizere adizere requested review from adizere and romac July 13, 2021 12:42
Copy link
Member

@adizere adizere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small comment I have is that there is an error message that could be improved, namely:

Error: .
├─.
│ └─ query error: failed while trying to upgrade client id 07-tendermint-1 with error: failed while fetching from chain ibc-1 the upgraded client state: Empty response value
└─.
└─ query error: failed while trying to upgrade client id 07-tendermint-3 with error: failed while fetching from chain ibc-1 the upgraded client state: Empty response value

To reproduce this error, simply invoke hermes upgrade clients ibc-1 so that the chain ibc-1 did not undergo an upgrade. The error simply states that chain ibc-1 defines no upgrade client state, but the user has no idea where is client id 07-tendermint-1 hosted -- on which chain? In ForeignClient terminology, this would be the destination chain.

To be more specific, all that's needed is to change the error type ClientUpgrade similartly to this:

diff --git a/relayer/src/foreign_client.rs b/relayer/src/foreign_client.rs
index 3b63e003..5e77d647 100644
--- a/relayer/src/foreign_client.rs
+++ b/relayer/src/foreign_client.rs
@@ -61,8 +61,8 @@ pub enum ForeignClientError {
     #[error("cannot run misbehaviour: {0}")]
     MisbehaviourExit(String),

-    #[error("failed while trying to upgrade client id {0} with error: {1}")]
-    ClientUpgrade(ClientId, String),
+    #[error("failed while trying to upgrade client id {0} hosted on chain id {1} with error: {2}")]
+    ClientUpgrade(ClientId, ChainId, String),

Note that after modifying ClientUpgrade definition you may need to adapt a few parts of the code in ForeignClient.

This is just a nice to have, so feel free to open a follow-up issue to capture this problem and merge this PR as it is! Or alternatively fix it in this PR, as you wish.

Thanks Shoaib, cool work!

relayer-cli/src/commands/tx.rs Outdated Show resolved Hide resolved
@hu55a1n1
Copy link
Member Author

hu55a1n1 commented Jul 16, 2021

This is just a nice to have, so feel free to open a follow-up issue to capture this problem and merge this PR as it is! Or alternatively fix it in this PR, as you wish.

Thanks for the suggestions and the concise comment @adizere! I agree that having the host chain id in the error will be very helpful. 👌 I have applied your suggestions to this PR itself.

relayer/src/foreign_client.rs Show resolved Hide resolved
@adizere adizere merged commit 9e928c0 into master Jul 19, 2021
@adizere adizere deleted the hu55a1n1/cli-bulk-upgrade branch July 19, 2021 15:09
@romac romac mentioned this pull request Jul 22, 2021
2 tasks
hu55a1n1 added a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
* Provide means to combine cli outputs

* Add bulk upgrade clients command

* Add missing raw upgrade-clients command

* Document upgrade commands in ADR 006

* Update CHANGELOG.md Unreleased section

* Polish Output::combined()

* Apply suggestions from code review

Co-authored-by: Adi Seredinschi <[email protected]>

* Reformat code

* Apply review suggestions

* Accumulate results and serialize manually into Output

* Remove Output::combined()

* Remove convert from impl for Output to string

* Minor refactoring

* Fix bug

* Apply suggestions

Co-authored-by: Adi Seredinschi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add bulk upgrade CLI
3 participants