From cf012a965635677c72afac7adff422b18d6138c6 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Mon, 8 Aug 2022 14:58:17 +0300 Subject: [PATCH 01/19] Remove a useless clone --- relayer/src/chain/cosmos/retry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/relayer/src/chain/cosmos/retry.rs b/relayer/src/chain/cosmos/retry.rs index d9b572a50c..35e23cf426 100644 --- a/relayer/src/chain/cosmos/retry.rs +++ b/relayer/src/chain/cosmos/retry.rs @@ -69,7 +69,7 @@ async fn refresh_account_and_retry_send_tx_with_account_sequence( refresh_account(&config.grpc_address, &key_entry.account, account).await?; // Retry after delay. thread::sleep(Duration::from_millis(ACCOUNT_SEQUENCE_RETRY_DELAY)); - estimate_fee_and_send_tx(config, key_entry, account, tx_memo, messages.clone()).await + estimate_fee_and_send_tx(config, key_entry, account, tx_memo, messages).await } async fn do_send_tx_with_account_sequence_retry( From 081ef0075a4d737323979b7db137b1d509172c4f Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Wed, 10 Aug 2022 16:52:59 +0300 Subject: [PATCH 02/19] De-clone estimate_tx_fees --- relayer/src/chain/cosmos/encode.rs | 6 +++--- relayer/src/chain/cosmos/estimate.rs | 2 +- relayer/src/chain/cosmos/tx.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/relayer/src/chain/cosmos/encode.rs b/relayer/src/chain/cosmos/encode.rs index 416bc049d0..6e4f1a04fa 100644 --- a/relayer/src/chain/cosmos/encode.rs +++ b/relayer/src/chain/cosmos/encode.rs @@ -22,7 +22,7 @@ pub fn sign_and_encode_tx( messages: Vec, fee: &Fee, ) -> Result, Error> { - let signed_tx = sign_tx(config, key_entry, account, tx_memo, messages, fee)?; + let signed_tx = sign_tx(config, key_entry, account, tx_memo, &messages, fee)?; let tx_raw = TxRaw { body_bytes: signed_tx.body_bytes, @@ -38,7 +38,7 @@ pub fn sign_tx( key_entry: &KeyEntry, account: &Account, tx_memo: &Memo, - messages: Vec, + messages: &[Any], fee: &Fee, ) -> Result { let key_bytes = encode_key_bytes(key_entry)?; @@ -159,7 +159,7 @@ fn auth_info_and_bytes(signer_info: SignerInfo, fee: Fee) -> Result<(AuthInfo, V Ok((auth_info, auth_buf)) } -fn tx_body_and_bytes(proto_msgs: Vec, memo: &Memo) -> Result<(TxBody, Vec), Error> { +fn tx_body_and_bytes(proto_msgs: &[Any], memo: &Memo) -> Result<(TxBody, Vec), Error> { // Create TxBody let body = TxBody { messages: proto_msgs.to_vec(), diff --git a/relayer/src/chain/cosmos/estimate.rs b/relayer/src/chain/cosmos/estimate.rs index 9764598a2c..c05b937c0f 100644 --- a/relayer/src/chain/cosmos/estimate.rs +++ b/relayer/src/chain/cosmos/estimate.rs @@ -19,7 +19,7 @@ pub async fn estimate_tx_fees( key_entry: &KeyEntry, account: &Account, tx_memo: &Memo, - messages: Vec, + messages: &[Any], ) -> Result { let gas_config = &config.gas_config; diff --git a/relayer/src/chain/cosmos/tx.rs b/relayer/src/chain/cosmos/tx.rs index e6a0519e1f..45c3a403fe 100644 --- a/relayer/src/chain/cosmos/tx.rs +++ b/relayer/src/chain/cosmos/tx.rs @@ -18,7 +18,7 @@ pub async fn estimate_fee_and_send_tx( tx_memo: &Memo, messages: Vec, ) -> Result { - let fee = estimate_tx_fees(config, key_entry, account, tx_memo, messages.clone()).await?; + let fee = estimate_tx_fees(config, key_entry, account, tx_memo, &messages).await?; send_tx_with_fee(config, key_entry, account, tx_memo, messages, &fee).await } From 6c67f9fab0a58a40ae2165c0e9f593a2717dad35 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Wed, 10 Aug 2022 16:56:21 +0300 Subject: [PATCH 03/19] cosmos: More de-owning around sign_and_encode_tx --- relayer/src/chain/cosmos/encode.rs | 4 ++-- relayer/src/chain/cosmos/retry.rs | 4 ++-- relayer/src/chain/cosmos/tx.rs | 6 +++--- tools/test-framework/src/relayer/tx.rs | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/relayer/src/chain/cosmos/encode.rs b/relayer/src/chain/cosmos/encode.rs index 6e4f1a04fa..9d8f7553c8 100644 --- a/relayer/src/chain/cosmos/encode.rs +++ b/relayer/src/chain/cosmos/encode.rs @@ -19,10 +19,10 @@ pub fn sign_and_encode_tx( key_entry: &KeyEntry, account: &Account, tx_memo: &Memo, - messages: Vec, + messages: &[Any], fee: &Fee, ) -> Result, Error> { - let signed_tx = sign_tx(config, key_entry, account, tx_memo, &messages, fee)?; + let signed_tx = sign_tx(config, key_entry, account, tx_memo, messages, fee)?; let tx_raw = TxRaw { body_bytes: signed_tx.body_bytes, diff --git a/relayer/src/chain/cosmos/retry.rs b/relayer/src/chain/cosmos/retry.rs index 35e23cf426..a890b89fb9 100644 --- a/relayer/src/chain/cosmos/retry.rs +++ b/relayer/src/chain/cosmos/retry.rs @@ -69,7 +69,7 @@ async fn refresh_account_and_retry_send_tx_with_account_sequence( refresh_account(&config.grpc_address, &key_entry.account, account).await?; // Retry after delay. thread::sleep(Duration::from_millis(ACCOUNT_SEQUENCE_RETRY_DELAY)); - estimate_fee_and_send_tx(config, key_entry, account, tx_memo, messages).await + estimate_fee_and_send_tx(config, key_entry, account, tx_memo, &messages).await } async fn do_send_tx_with_account_sequence_retry( @@ -79,7 +79,7 @@ async fn do_send_tx_with_account_sequence_retry( tx_memo: &Memo, messages: Vec, ) -> Result { - match estimate_fee_and_send_tx(config, key_entry, account, tx_memo, messages.clone()).await { + match estimate_fee_and_send_tx(config, key_entry, account, tx_memo, &messages).await { // Gas estimation failed with acct. s.n. mismatch at estimate gas step. // It indicates that the account sequence cached by hermes is stale (got < expected). // This can happen when the same account is used by another agent. diff --git a/relayer/src/chain/cosmos/tx.rs b/relayer/src/chain/cosmos/tx.rs index 45c3a403fe..4b78789e04 100644 --- a/relayer/src/chain/cosmos/tx.rs +++ b/relayer/src/chain/cosmos/tx.rs @@ -16,9 +16,9 @@ pub async fn estimate_fee_and_send_tx( key_entry: &KeyEntry, account: &Account, tx_memo: &Memo, - messages: Vec, + messages: &[Any], ) -> Result { - let fee = estimate_tx_fees(config, key_entry, account, tx_memo, &messages).await?; + let fee = estimate_tx_fees(config, key_entry, account, tx_memo, messages).await?; send_tx_with_fee(config, key_entry, account, tx_memo, messages, &fee).await } @@ -28,7 +28,7 @@ async fn send_tx_with_fee( key_entry: &KeyEntry, account: &Account, tx_memo: &Memo, - messages: Vec, + messages: &[Any], fee: &Fee, ) -> Result { let tx_bytes = sign_and_encode_tx(config, key_entry, account, tx_memo, messages, fee)?; diff --git a/tools/test-framework/src/relayer/tx.rs b/tools/test-framework/src/relayer/tx.rs index 1cce4ca4f3..81300e7653 100644 --- a/tools/test-framework/src/relayer/tx.rs +++ b/tools/test-framework/src/relayer/tx.rs @@ -96,7 +96,7 @@ pub async fn simple_send_tx( let message_count = messages.len(); let response = - estimate_fee_and_send_tx(config, key_entry, &account, &Default::default(), messages) + estimate_fee_and_send_tx(config, key_entry, &account, &Default::default(), &messages) .await?; let tx_sync_result = TxSyncResult { From 87b95dee3dbba8468bc57950d00aa1554a9b580e Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Thu, 11 Aug 2022 10:29:24 +0300 Subject: [PATCH 04/19] Simplify prepending of ClientUpdate --- relayer/src/link/operational_data.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/relayer/src/link/operational_data.rs b/relayer/src/link/operational_data.rs index b3767fbaf4..621f7db199 100644 --- a/relayer/src/link/operational_data.rs +++ b/relayer/src/link/operational_data.rs @@ -1,5 +1,4 @@ use core::fmt; -use core::iter; use std::time::{Duration, Instant}; use ibc_proto::google::protobuf::Any; @@ -211,12 +210,10 @@ impl OperationalData { } }; - let msgs: Vec = match client_update_msg { - Some(client_update) => iter::once(client_update) - .chain(self.batch.iter().map(|gm| gm.msg.clone())) - .collect(), - None => self.batch.iter().map(|gm| gm.msg.clone()).collect(), - }; + let msgs = client_update_msg + .into_iter() + .chain(self.batch.iter().map(|gm| gm.msg.clone())) + .collect(); let tm = TrackedMsgs::new(msgs, self.tracking_id); From 886e983daa82759405339471851db75e0e004073 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Thu, 11 Aug 2022 11:28:25 +0300 Subject: [PATCH 05/19] cosmos: encoded_tx_len function The function evaluates the length of an encoded transaction, to perform more accurate batching. --- relayer/src/chain/cosmos/encode.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/relayer/src/chain/cosmos/encode.rs b/relayer/src/chain/cosmos/encode.rs index 9d8f7553c8..f93cdf1404 100644 --- a/relayer/src/chain/cosmos/encode.rs +++ b/relayer/src/chain/cosmos/encode.rs @@ -4,6 +4,7 @@ use ibc::core::ics24_host::identifier::ChainId; use ibc_proto::cosmos::tx::v1beta1::mode_info::{Single, Sum}; use ibc_proto::cosmos::tx::v1beta1::{AuthInfo, Fee, ModeInfo, SignDoc, SignerInfo, TxBody, TxRaw}; use ibc_proto::google::protobuf::Any; +use prost::Message; use tendermint::account::Id as AccountId; use crate::chain::cosmos::types::account::{Account, AccountNumber, AccountSequence}; @@ -33,6 +34,27 @@ pub fn sign_and_encode_tx( encode_tx_raw(tx_raw) } +pub fn encoded_tx_len( + config: &TxConfig, + key_entry: &KeyEntry, + account: &Account, + tx_memo: &Memo, + messages: &[Any], + fee: &Fee, +) -> Result { + let signed_tx = sign_tx(config, key_entry, account, tx_memo, messages, fee)?; + + let tx_raw = TxRaw { + body_bytes: signed_tx.body_bytes, + auth_info_bytes: signed_tx.auth_info_bytes, + signatures: signed_tx.signatures, + }; + + let len = tx_raw.encoded_len(); + + Ok(len) +} + pub fn sign_tx( config: &TxConfig, key_entry: &KeyEntry, From 13ce6a54245a32762d591da7ea00332f8b0b8d0c Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Thu, 18 Aug 2022 07:25:17 +0300 Subject: [PATCH 06/19] Account for field encoding in tx batch size --- relayer/src/chain/cosmos/batch.rs | 12 ++++++++---- relayer/src/error.rs | 4 ++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/relayer/src/chain/cosmos/batch.rs b/relayer/src/chain/cosmos/batch.rs index 9118d27152..2a8507c5d4 100644 --- a/relayer/src/chain/cosmos/batch.rs +++ b/relayer/src/chain/cosmos/batch.rs @@ -155,11 +155,15 @@ fn batch_messages( for message in messages { let message_len = message.encoded_len(); - if message_len > max_tx_size { - return Err(Error::message_exceeds_max_tx_size(message_len)); + // The total length the message adds to the encoding includes the + // field tag (small varint) and the length delimiter. + let tagged_len = 1 + prost::length_delimiter_len(message_len) + message_len; + + if tagged_len > max_tx_size { + return Err(Error::message_too_big_for_tx(message_len)); } - if current_count >= max_message_count || current_size + message_len > max_tx_size { + if current_count >= max_message_count || current_size + tagged_len > max_tx_size { let insert_batch = mem::take(&mut current_batch); assert!( @@ -173,7 +177,7 @@ fn batch_messages( } current_count += 1; - current_size += message_len; + current_size += tagged_len; current_batch.push(message); } diff --git a/relayer/src/error.rs b/relayer/src/error.rs index f1d7e26fb4..f2dec4b1b8 100644 --- a/relayer/src/error.rs +++ b/relayer/src/error.rs @@ -512,10 +512,10 @@ define_error! { "Query/DenomTrace RPC returned an empty denom trace for trace hash: {}", e.hash) }, - MessageExceedsMaxTxSize + MessageTooBigForTx { len: usize } |e| { - format_args!("message length {} exceeds maximum transaction size", e.len) + format_args!("message with length {} is too large for a transaction", e.len) } } } From f0e9c589f552cc67df2fd8e334508ffa68b6fbd5 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Thu, 18 Aug 2022 08:14:43 +0300 Subject: [PATCH 07/19] Account for tx envelope in message batching --- relayer/src/chain/cosmos/batch.rs | 115 +++++++++++++++++++++++++++--- relayer/src/config/types.rs | 3 +- 2 files changed, 106 insertions(+), 12 deletions(-) diff --git a/relayer/src/chain/cosmos/batch.rs b/relayer/src/chain/cosmos/batch.rs index 2a8507c5d4..567dba9dc6 100644 --- a/relayer/src/chain/cosmos/batch.rs +++ b/relayer/src/chain/cosmos/batch.rs @@ -6,6 +6,8 @@ use ibc_proto::google::protobuf::Any; use prost::Message; use tendermint_rpc::endpoint::broadcast::tx_sync::Response; +use crate::chain::cosmos::encode::encoded_tx_len; +use crate::chain::cosmos::gas::gas_amount_to_fee; use crate::chain::cosmos::retry::send_tx_with_account_sequence_retry; use crate::chain::cosmos::types::account::Account; use crate::chain::cosmos::types::config::TxConfig; @@ -70,7 +72,15 @@ pub async fn send_batched_messages_and_wait_check_tx( return Ok(Vec::new()); } - let batches = batch_messages(max_msg_num, max_tx_size, messages)?; + let batches = batch_messages( + config, + max_msg_num, + max_tx_size, + key_entry, + account, + tx_memo, + messages, + )?; let mut responses = Vec::new(); @@ -97,7 +107,15 @@ async fn send_messages_as_batches( return Ok(Vec::new()); } - let batches = batch_messages(max_msg_num, max_tx_size, messages)?; + let batches = batch_messages( + config, + max_msg_num, + max_tx_size, + key_entry, + account, + tx_memo, + messages, + )?; let mut tx_sync_results = Vec::new(); @@ -139,8 +157,12 @@ async fn send_messages_as_batches( } fn batch_messages( + config: &TxConfig, max_msg_num: MaxMsgNum, max_tx_size: MaxTxSize, + key_entry: &KeyEntry, + account: &Account, + tx_memo: &Memo, messages: Vec, ) -> Result>, Error> { let max_message_count = max_msg_num.to_usize(); @@ -148,8 +170,14 @@ fn batch_messages( let mut batches = vec![]; + // Estimate the overhead of the transaction envelope's encoding, + // by taking the encoded length of an empty tx with the same auth info and signatures. + // Use the maximum possible fee to get an upper bound for varint encoding. + let max_fee = gas_amount_to_fee(&config.gas_config, config.gas_config.max_gas); + let tx_envelope_len = encoded_tx_len(config, key_entry, account, tx_memo, &[], &max_fee)?; + let mut current_count = 0; - let mut current_size = 0; + let mut current_size = tx_envelope_len; let mut current_batch = vec![]; for message in messages { @@ -191,11 +219,19 @@ fn batch_messages( #[cfg(test)] mod tests { use super::batch_messages; - use crate::config::types::{MaxMsgNum, MaxTxSize}; + use crate::chain::cosmos::types::account::Account; + use crate::chain::cosmos::types::config::TxConfig; + use crate::config::types::{MaxMsgNum, MaxTxSize, Memo}; + use crate::keyring::KeyEntry; use ibc_proto::google::protobuf::Any; + fn test_fixture() -> (TxConfig, KeyEntry, Account) { + todo!() + } + #[test] fn batch_does_not_exceed_max_tx_size() { + let (config, key_entry, account) = test_fixture(); let messages = vec![ Any { type_url: "/example.Foo".into(), @@ -210,8 +246,16 @@ mod tests { value: vec![0; 2], }, ]; - let batches = - batch_messages(MaxMsgNum::default(), MaxTxSize::new(42).unwrap(), messages).unwrap(); + let batches = batch_messages( + &config, + MaxMsgNum::default(), + MaxTxSize::new(42).unwrap(), + &key_entry, + &account, + &Memo::new("").unwrap(), + messages, + ) + .unwrap(); assert_eq!(batches.len(), 2); assert_eq!(batches[0].len(), 2); @@ -227,14 +271,19 @@ mod tests { #[test] fn batch_error_on_oversized_message() { + let (config, key_entry, account) = test_fixture(); let messages = vec![Any { type_url: "/example.Foo".into(), value: vec![0; 6], }]; let batches = batch_messages( + &config, MaxMsgNum::default(), MaxTxSize::new(22).unwrap(), + &key_entry, + &account, + &Memo::new("").unwrap(), messages.clone(), ) .unwrap(); @@ -242,13 +291,22 @@ mod tests { assert_eq!(batches.len(), 1); assert_eq!(batches[0].len(), 1); - let res = batch_messages(MaxMsgNum::default(), MaxTxSize::new(21).unwrap(), messages); + let res = batch_messages( + &config, + MaxMsgNum::default(), + MaxTxSize::new(21).unwrap(), + &key_entry, + &account, + &Memo::new("").unwrap(), + messages, + ); assert!(res.is_err()); } #[test] fn test_batches_are_structured_appropriately_per_max_msg_num() { + let (config, key_entry, account) = test_fixture(); // Ensure that when MaxMsgNum is 1, the resulting batch // consists of 5 smaller batches, each with a single message let messages = vec![ @@ -275,8 +333,12 @@ mod tests { ]; let batches = batch_messages( + &config, MaxMsgNum::new(1).unwrap(), MaxTxSize::default(), + &key_entry, + &account, + &Memo::new("").unwrap(), messages.clone(), ) .unwrap(); @@ -289,8 +351,16 @@ mod tests { // Ensure that when MaxMsgNum > the number of messages, the resulting // batch consists of a single smaller batch with all of the messages - let batches = - batch_messages(MaxMsgNum::new(100).unwrap(), MaxTxSize::default(), messages).unwrap(); + let batches = batch_messages( + &config, + MaxMsgNum::new(100).unwrap(), + MaxTxSize::default(), + &key_entry, + &account, + &Memo::new("").unwrap(), + messages, + ) + .unwrap(); assert_eq!(batches.len(), 1); assert_eq!(batches[0].len(), 5); @@ -298,6 +368,7 @@ mod tests { #[test] fn test_batches_are_structured_appropriately_per_max_tx_size() { + let (config, key_entry, account) = test_fixture(); // Ensure that when MaxTxSize == the size of each message, the resulting batch // consists of 5 smaller batches, each with a single message let messages = vec![ @@ -324,8 +395,12 @@ mod tests { ]; let batches = batch_messages( + &config, MaxMsgNum::default(), MaxTxSize::new(26).unwrap(), + &key_entry, + &account, + &Memo::new("").unwrap(), messages.clone(), ) .unwrap(); @@ -339,7 +414,16 @@ mod tests { // Ensure that when MaxTxSize > the size of all the messages, the // resulting batch consists of a single smaller batch with all of // messages inside - let batches = batch_messages(MaxMsgNum::default(), MaxTxSize::max(), messages).unwrap(); + let batches = batch_messages( + &config, + MaxMsgNum::default(), + MaxTxSize::max(), + &key_entry, + &account, + &Memo::new("").unwrap(), + messages, + ) + .unwrap(); assert_eq!(batches.len(), 1); assert_eq!(batches[0].len(), 5); @@ -348,6 +432,15 @@ mod tests { #[test] #[should_panic(expected = "`max_msg_num` must be greater than or equal to 1, found 0")] fn test_max_msg_num_of_zero_panics() { - let _batches = batch_messages(MaxMsgNum::new(0).unwrap(), MaxTxSize::default(), vec![]); + let (config, key_entry, account) = test_fixture(); + let _batches = batch_messages( + &config, + MaxMsgNum::new(0).unwrap(), + MaxTxSize::default(), + &key_entry, + &account, + &Memo::new("").unwrap(), + vec![], + ); } } diff --git a/relayer/src/config/types.rs b/relayer/src/config/types.rs index 9d46f70948..00346505a5 100644 --- a/relayer/src/config/types.rs +++ b/relayer/src/config/types.rs @@ -199,7 +199,8 @@ pub mod memo { impl Memo { const MAX_LEN: usize = 50; - pub fn new(memo: String) -> Result { + pub fn new(memo: impl Into) -> Result { + let memo = memo.into(); if memo.len() > Self::MAX_LEN { return Err(Error::too_long(memo.len())); } From 4f90ce391051f3a911972949ae628fdc99268572 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Mon, 22 Aug 2022 17:03:36 +0300 Subject: [PATCH 08/19] More credible test fixture for batch_messages --- relayer/src/chain/cosmos/batch.rs | 48 +++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/relayer/src/chain/cosmos/batch.rs b/relayer/src/chain/cosmos/batch.rs index 567dba9dc6..a20e1efd62 100644 --- a/relayer/src/chain/cosmos/batch.rs +++ b/relayer/src/chain/cosmos/batch.rs @@ -219,14 +219,58 @@ fn batch_messages( #[cfg(test)] mod tests { use super::batch_messages; - use crate::chain::cosmos::types::account::Account; + use crate::chain::cosmos::types::account::{Account, AccountNumber, AccountSequence}; use crate::chain::cosmos::types::config::TxConfig; + use crate::config; use crate::config::types::{MaxMsgNum, MaxTxSize, Memo}; use crate::keyring::KeyEntry; + use bitcoin::network::constants::Network; + use bitcoin::util::bip32::{ExtendedPrivKey, ExtendedPubKey}; + use bitcoin::util::key::Secp256k1; + use ibc::core::ics24_host::identifier::ChainId; use ibc_proto::google::protobuf::Any; + fn decode_bech32(input: &str) -> Vec { + use bech32::FromBase32; + + bech32::decode(input) + .and_then(|(_, data, _)| Vec::from_base32(&data)) + .unwrap() + } + fn test_fixture() -> (TxConfig, KeyEntry, Account) { - todo!() + let path = concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/config/fixtures/relayer_conf_example.toml" + ); + let config = config::load(path).expect("could not parse config"); + let chain_config = config.find_chain(&ChainId::from_string("chain_A")).unwrap(); + + let tx_config = TxConfig::try_from(chain_config).expect("could not obtain tx config"); + + let secp256k1 = Secp256k1::new(); + let private_key = ExtendedPrivKey::new_master( + Network::Testnet, + b"the quick brown fox jumps over the lazy dog", + ) + .unwrap(); + // Derive a private subkey with a path? + let public_key = ExtendedPubKey::from_priv(&secp256k1, &private_key); + let account = String::from("cosmos1m2rdz42g8xwqa63r4s8tnsfwyvktcajpn93cvn"); + let address = decode_bech32(&account); + let key_entry = KeyEntry { + public_key, + private_key, + account, + address, + }; + + let account = Account { + number: AccountNumber::new(0), + sequence: AccountSequence::new(0), + }; + + (tx_config, key_entry, account) } #[test] From dde83dfee375be39968deed19639a90a72653aba Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Mon, 22 Aug 2022 20:43:40 +0300 Subject: [PATCH 09/19] Fix calculations in batch_messages, adjust tests --- relayer/src/chain/cosmos/batch.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/relayer/src/chain/cosmos/batch.rs b/relayer/src/chain/cosmos/batch.rs index a20e1efd62..3429b64ea5 100644 --- a/relayer/src/chain/cosmos/batch.rs +++ b/relayer/src/chain/cosmos/batch.rs @@ -187,7 +187,7 @@ fn batch_messages( // field tag (small varint) and the length delimiter. let tagged_len = 1 + prost::length_delimiter_len(message_len) + message_len; - if tagged_len > max_tx_size { + if tx_envelope_len + tagged_len > max_tx_size { return Err(Error::message_too_big_for_tx(message_len)); } @@ -201,7 +201,7 @@ fn batch_messages( batches.push(insert_batch); current_count = 0; - current_size = 0; + current_size = tx_envelope_len; } current_count += 1; @@ -293,7 +293,7 @@ mod tests { let batches = batch_messages( &config, MaxMsgNum::default(), - MaxTxSize::new(42).unwrap(), + MaxTxSize::new(214).unwrap(), &key_entry, &account, &Memo::new("").unwrap(), @@ -324,7 +324,7 @@ mod tests { let batches = batch_messages( &config, MaxMsgNum::default(), - MaxTxSize::new(22).unwrap(), + MaxTxSize::new(192).unwrap(), &key_entry, &account, &Memo::new("").unwrap(), @@ -338,7 +338,7 @@ mod tests { let res = batch_messages( &config, MaxMsgNum::default(), - MaxTxSize::new(21).unwrap(), + MaxTxSize::new(191).unwrap(), &key_entry, &account, &Memo::new("").unwrap(), @@ -413,8 +413,8 @@ mod tests { #[test] fn test_batches_are_structured_appropriately_per_max_tx_size() { let (config, key_entry, account) = test_fixture(); - // Ensure that when MaxTxSize == the size of each message, the resulting batch - // consists of 5 smaller batches, each with a single message + // Ensure that when MaxTxSize is only enough to fit each one of the messages, + // the resulting batch consists of 5 smaller batches, each with a single message. let messages = vec![ Any { type_url: "/example.Foo".into(), @@ -441,7 +441,7 @@ mod tests { let batches = batch_messages( &config, MaxMsgNum::default(), - MaxTxSize::new(26).unwrap(), + MaxTxSize::new(196).unwrap(), &key_entry, &account, &Memo::new("").unwrap(), From 3f0e8812a36f4fc13cd2241275657376e51ae963 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Wed, 24 Aug 2022 15:36:38 +0300 Subject: [PATCH 10/19] Initialize KeyEntry data from a seed file --- relayer/src/chain/cosmos/batch.rs | 43 +++++++------------ .../tests/config/fixtures/relayer-seed.json | 7 +++ 2 files changed, 22 insertions(+), 28 deletions(-) create mode 100644 relayer/tests/config/fixtures/relayer-seed.json diff --git a/relayer/src/chain/cosmos/batch.rs b/relayer/src/chain/cosmos/batch.rs index 65a6cbae59..d43a49000b 100644 --- a/relayer/src/chain/cosmos/batch.rs +++ b/relayer/src/chain/cosmos/batch.rs @@ -341,20 +341,12 @@ mod tests { use crate::chain::cosmos::types::config::TxConfig; use crate::config; use crate::config::types::{MaxMsgNum, MaxTxSize, Memo}; - use crate::keyring::KeyEntry; - use bitcoin::network::constants::Network; - use bitcoin::util::bip32::{ExtendedPrivKey, ExtendedPubKey}; - use bitcoin::util::key::Secp256k1; + use crate::keyring::{self, KeyEntry, KeyRing}; use ibc::core::ics24_host::identifier::ChainId; use ibc_proto::google::protobuf::Any; + use std::fs; - fn decode_bech32(input: &str) -> Vec { - use bech32::FromBase32; - - bech32::decode(input) - .and_then(|(_, data, _)| Vec::from_base32(&data)) - .unwrap() - } + const COSMOS_HD_PATH: &str = "m/44'/118'/0'/0/0"; fn test_fixture() -> (TxConfig, KeyEntry, Account) { let path = concat!( @@ -362,26 +354,21 @@ mod tests { "/tests/config/fixtures/relayer_conf_example.toml" ); let config = config::load(path).expect("could not parse config"); - let chain_config = config.find_chain(&ChainId::from_string("chain_A")).unwrap(); + let chain_id = ChainId::from_string("chain_A"); + let chain_config = config.find_chain(&chain_id).unwrap(); let tx_config = TxConfig::try_from(chain_config).expect("could not obtain tx config"); - let secp256k1 = Secp256k1::new(); - let private_key = ExtendedPrivKey::new_master( - Network::Testnet, - b"the quick brown fox jumps over the lazy dog", - ) - .unwrap(); - // Derive a private subkey with a path? - let public_key = ExtendedPubKey::from_priv(&secp256k1, &private_key); - let account = String::from("cosmos1m2rdz42g8xwqa63r4s8tnsfwyvktcajpn93cvn"); - let address = decode_bech32(&account); - let key_entry = KeyEntry { - public_key, - private_key, - account, - address, - }; + let path = concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/config/fixtures/relayer-seed.json" + ); + let seed_file_content = fs::read_to_string(path).unwrap(); + let keyring = KeyRing::new(keyring::Store::Memory, "cosmos", &chain_id).unwrap(); + let hd_path = COSMOS_HD_PATH.parse().unwrap(); + let key_entry = keyring + .key_from_seed_file(&seed_file_content, &hd_path) + .unwrap(); let account = Account { number: AccountNumber::new(0), diff --git a/relayer/tests/config/fixtures/relayer-seed.json b/relayer/tests/config/fixtures/relayer-seed.json new file mode 100644 index 0000000000..4dad31a216 --- /dev/null +++ b/relayer/tests/config/fixtures/relayer-seed.json @@ -0,0 +1,7 @@ +{ + "name": "relayer-584b9db9", + "type": "local", + "address": "cosmos1dvu2zm4vzma30q403d0ezww3wpl7wcpp0nhcph", + "pubkey": "{\"@type\":\"/cosmos.crypto.secp256k1.PubKey\",\"key\":\"A5W0C7iEAuonX56sR81PiwaKTE0GvZlCYuGwHTMpWJo+\"}", + "mnemonic": "such walnut usual noble image raise cabin suspect combine key absurd detail present bless yard grief amazing slam brown donate fabric opera desk minor" +} \ No newline at end of file From 6a8e31f7d150f0596ddad73a87ef05a6ffd38af3 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Wed, 24 Aug 2022 23:02:02 +0300 Subject: [PATCH 11/19] relayer: module chain::cosmos::test_utils Move utilities from chain::cosmos::batch to share them with other modules' unit tests. --- relayer/src/chain/cosmos.rs | 3 + relayer/src/chain/cosmos/batch.rs | 87 +++++++++++--------------- relayer/src/chain/cosmos/test_utils.rs | 54 ++++++++++++++++ 3 files changed, 93 insertions(+), 51 deletions(-) create mode 100644 relayer/src/chain/cosmos/test_utils.rs diff --git a/relayer/src/chain/cosmos.rs b/relayer/src/chain/cosmos.rs index f0a548c167..0d17a6d8b7 100644 --- a/relayer/src/chain/cosmos.rs +++ b/relayer/src/chain/cosmos.rs @@ -106,6 +106,9 @@ pub mod types; pub mod version; pub mod wait; +#[cfg(test)] +mod test_utils; + /// fraction of the maximum block size defined in the Tendermint core consensus parameters. pub const GENESIS_MAX_BYTES_MAX_FRACTION: f64 = 0.9; // https://github.com/cosmos/cosmos-sdk/blob/v0.44.0/types/errors/errors.go#L115-L117 diff --git a/relayer/src/chain/cosmos/batch.rs b/relayer/src/chain/cosmos/batch.rs index d43a49000b..625d31e206 100644 --- a/relayer/src/chain/cosmos/batch.rs +++ b/relayer/src/chain/cosmos/batch.rs @@ -286,6 +286,8 @@ fn batch_messages( let max_message_count = max_msg_num.to_usize(); let max_tx_size = max_tx_size.into(); + dbg!(config, key_entry, account, tx_memo); + let mut batches = vec![]; // Estimate the overhead of the transaction envelope's encoding, @@ -337,50 +339,17 @@ fn batch_messages( #[cfg(test)] mod tests { use super::batch_messages; - use crate::chain::cosmos::types::account::{Account, AccountNumber, AccountSequence}; - use crate::chain::cosmos::types::config::TxConfig; - use crate::config; + use crate::chain::cosmos::test_utils::TestFixture; use crate::config::types::{MaxMsgNum, MaxTxSize, Memo}; - use crate::keyring::{self, KeyEntry, KeyRing}; - use ibc::core::ics24_host::identifier::ChainId; use ibc_proto::google::protobuf::Any; - use std::fs; - - const COSMOS_HD_PATH: &str = "m/44'/118'/0'/0/0"; - - fn test_fixture() -> (TxConfig, KeyEntry, Account) { - let path = concat!( - env!("CARGO_MANIFEST_DIR"), - "/tests/config/fixtures/relayer_conf_example.toml" - ); - let config = config::load(path).expect("could not parse config"); - let chain_id = ChainId::from_string("chain_A"); - let chain_config = config.find_chain(&chain_id).unwrap(); - - let tx_config = TxConfig::try_from(chain_config).expect("could not obtain tx config"); - - let path = concat!( - env!("CARGO_MANIFEST_DIR"), - "/tests/config/fixtures/relayer-seed.json" - ); - let seed_file_content = fs::read_to_string(path).unwrap(); - let keyring = KeyRing::new(keyring::Store::Memory, "cosmos", &chain_id).unwrap(); - let hd_path = COSMOS_HD_PATH.parse().unwrap(); - let key_entry = keyring - .key_from_seed_file(&seed_file_content, &hd_path) - .unwrap(); - - let account = Account { - number: AccountNumber::new(0), - sequence: AccountSequence::new(0), - }; - - (tx_config, key_entry, account) - } #[test] fn batch_does_not_exceed_max_tx_size() { - let (config, key_entry, account) = test_fixture(); + let TestFixture { + tx_config, + key_entry, + account, + } = TestFixture::new(); let messages = vec![ Any { type_url: "/example.Foo".into(), @@ -396,7 +365,7 @@ mod tests { }, ]; let batches = batch_messages( - &config, + &tx_config, MaxMsgNum::default(), MaxTxSize::new(214).unwrap(), &key_entry, @@ -420,14 +389,18 @@ mod tests { #[test] fn batch_error_on_oversized_message() { - let (config, key_entry, account) = test_fixture(); + let TestFixture { + tx_config, + key_entry, + account, + } = TestFixture::new(); let messages = vec![Any { type_url: "/example.Foo".into(), value: vec![0; 6], }]; let batches = batch_messages( - &config, + &tx_config, MaxMsgNum::default(), MaxTxSize::new(192).unwrap(), &key_entry, @@ -441,7 +414,7 @@ mod tests { assert_eq!(batches[0].len(), 1); let res = batch_messages( - &config, + &tx_config, MaxMsgNum::default(), MaxTxSize::new(191).unwrap(), &key_entry, @@ -455,7 +428,11 @@ mod tests { #[test] fn test_batches_are_structured_appropriately_per_max_msg_num() { - let (config, key_entry, account) = test_fixture(); + let TestFixture { + tx_config, + key_entry, + account, + } = TestFixture::new(); // Ensure that when MaxMsgNum is 1, the resulting batch // consists of 5 smaller batches, each with a single message let messages = vec![ @@ -482,7 +459,7 @@ mod tests { ]; let batches = batch_messages( - &config, + &tx_config, MaxMsgNum::new(1).unwrap(), MaxTxSize::default(), &key_entry, @@ -501,7 +478,7 @@ mod tests { // Ensure that when MaxMsgNum > the number of messages, the resulting // batch consists of a single smaller batch with all of the messages let batches = batch_messages( - &config, + &tx_config, MaxMsgNum::new(100).unwrap(), MaxTxSize::default(), &key_entry, @@ -517,7 +494,11 @@ mod tests { #[test] fn test_batches_are_structured_appropriately_per_max_tx_size() { - let (config, key_entry, account) = test_fixture(); + let TestFixture { + tx_config, + key_entry, + account, + } = TestFixture::new(); // Ensure that when MaxTxSize is only enough to fit each one of the messages, // the resulting batch consists of 5 smaller batches, each with a single message. let messages = vec![ @@ -544,7 +525,7 @@ mod tests { ]; let batches = batch_messages( - &config, + &tx_config, MaxMsgNum::default(), MaxTxSize::new(196).unwrap(), &key_entry, @@ -564,7 +545,7 @@ mod tests { // resulting batch consists of a single smaller batch with all of // messages inside let batches = batch_messages( - &config, + &tx_config, MaxMsgNum::default(), MaxTxSize::max(), &key_entry, @@ -581,9 +562,13 @@ mod tests { #[test] #[should_panic(expected = "`max_msg_num` must be greater than or equal to 1, found 0")] fn test_max_msg_num_of_zero_panics() { - let (config, key_entry, account) = test_fixture(); + let TestFixture { + tx_config, + key_entry, + account, + } = TestFixture::new(); let _batches = batch_messages( - &config, + &tx_config, MaxMsgNum::new(0).unwrap(), MaxTxSize::default(), &key_entry, diff --git a/relayer/src/chain/cosmos/test_utils.rs b/relayer/src/chain/cosmos/test_utils.rs new file mode 100644 index 0000000000..45768d196a --- /dev/null +++ b/relayer/src/chain/cosmos/test_utils.rs @@ -0,0 +1,54 @@ +//! Shared utilities for unit tests. + +use std::fs; + +use ibc::core::ics24_host::identifier::ChainId; + +use crate::chain::cosmos::types::account::{Account, AccountNumber, AccountSequence}; +use crate::chain::cosmos::types::config::TxConfig; +use crate::config; +use crate::keyring::{self, KeyEntry, KeyRing}; + +const COSMOS_HD_PATH: &str = "m/44'/118'/0'/0/0"; + +pub struct TestFixture { + pub tx_config: TxConfig, + pub key_entry: KeyEntry, + pub account: Account, +} + +impl TestFixture { + pub fn new() -> Self { + let path = concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/config/fixtures/relayer_conf_example.toml" + ); + let config = config::load(path).expect("could not parse config"); + let chain_id = ChainId::from_string("chain_A"); + let chain_config = config.find_chain(&chain_id).unwrap(); + + let tx_config = TxConfig::try_from(chain_config).expect("could not obtain tx config"); + + let path = concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/config/fixtures/relayer-seed.json" + ); + let seed_file_content = fs::read_to_string(path).unwrap(); + let keyring = KeyRing::new(keyring::Store::Memory, "cosmos", &chain_id).unwrap(); + let hd_path = COSMOS_HD_PATH.parse().unwrap(); + let key_entry = keyring + .key_from_seed_file(&seed_file_content, &hd_path) + .unwrap(); + + let account = Account { + number: AccountNumber::new(0), + sequence: AccountSequence::new(0), + }; + + TestFixture { + tx_config, + key_entry, + account, + } + } +} From c8cebc06796171ec9ad7d9e64ff96e3f95a8b6ab Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Thu, 25 Aug 2022 00:11:51 +0300 Subject: [PATCH 12/19] Take tx_body varint lenth delimiter into account In batch_messages, the varint encoding of the length of the tx_body field of TxRaw may increase the length of the envelope as the body size grows. --- relayer/src/chain/cosmos/batch.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/relayer/src/chain/cosmos/batch.rs b/relayer/src/chain/cosmos/batch.rs index 625d31e206..95953e0d4a 100644 --- a/relayer/src/chain/cosmos/batch.rs +++ b/relayer/src/chain/cosmos/batch.rs @@ -286,8 +286,6 @@ fn batch_messages( let max_message_count = max_msg_num.to_usize(); let max_tx_size = max_tx_size.into(); - dbg!(config, key_entry, account, tx_memo); - let mut batches = vec![]; // Estimate the overhead of the transaction envelope's encoding, @@ -296,8 +294,15 @@ fn batch_messages( let max_fee = gas_amount_to_fee(&config.gas_config, config.gas_config.max_gas); let tx_envelope_len = encoded_tx_len(config, key_entry, account, tx_memo, &[], &max_fee)?; + // Full length of the transaction can then be derived from the envelope length + // and the length of the body field, taking into account the varint encoding + // of the body field's length delimiter. + fn tx_len(envelope_len: usize, body_len: usize) -> usize { + envelope_len + prost::length_delimiter_len(body_len) - 1 + body_len + } + let mut current_count = 0; - let mut current_size = tx_envelope_len; + let mut current_len = 0; let mut current_batch = vec![]; for message in messages { @@ -307,11 +312,15 @@ fn batch_messages( // field tag (small varint) and the length delimiter. let tagged_len = 1 + prost::length_delimiter_len(message_len) + message_len; - if tx_envelope_len + tagged_len > max_tx_size { + // Check if the message is too big to fit into the transaction size limit + // even if the message is taken alone. + if tx_len(tx_envelope_len, tagged_len) > max_tx_size { return Err(Error::message_too_big_for_tx(message_len)); } - if current_count >= max_message_count || current_size + tagged_len > max_tx_size { + if current_count >= max_message_count + || tx_len(tx_envelope_len, current_len + tagged_len) > max_tx_size + { let insert_batch = mem::take(&mut current_batch); assert!( @@ -321,11 +330,11 @@ fn batch_messages( batches.push(insert_batch); current_count = 0; - current_size = tx_envelope_len; + current_len = 0; } current_count += 1; - current_size += tagged_len; + current_len += tagged_len; current_batch.push(message); } From 4a83ed56ed470b16ded2c0268fa931318b4b5bfb Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Thu, 25 Aug 2022 20:00:56 +0300 Subject: [PATCH 13/19] Account for body encoding when batching messages The tx_body member of TxRaw encodes more than the messages, so its total length needs to be taken into account when estimating the total transaction size from the previously computed envelope and the vector of messages. --- relayer/src/chain/cosmos/batch.rs | 30 +++++++++++++----------------- relayer/src/chain/cosmos/encode.rs | 17 +++++++++++++---- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/relayer/src/chain/cosmos/batch.rs b/relayer/src/chain/cosmos/batch.rs index 95953e0d4a..a523cc40a9 100644 --- a/relayer/src/chain/cosmos/batch.rs +++ b/relayer/src/chain/cosmos/batch.rs @@ -8,7 +8,7 @@ use prost::Message; use tendermint_rpc::endpoint::broadcast::tx_sync::Response; use tracing::debug; -use crate::chain::cosmos::encode::encoded_tx_len; +use crate::chain::cosmos::encode::encoded_tx_metrics; use crate::chain::cosmos::gas::gas_amount_to_fee; use crate::chain::cosmos::retry::send_tx_with_account_sequence_retry; use crate::chain::cosmos::types::account::Account; @@ -292,17 +292,19 @@ fn batch_messages( // by taking the encoded length of an empty tx with the same auth info and signatures. // Use the maximum possible fee to get an upper bound for varint encoding. let max_fee = gas_amount_to_fee(&config.gas_config, config.gas_config.max_gas); - let tx_envelope_len = encoded_tx_len(config, key_entry, account, tx_memo, &[], &max_fee)?; + let tx_metrics = encoded_tx_metrics(config, key_entry, account, tx_memo, &[], &max_fee)?; + let empty_body_len = tx_metrics.body_bytes_len; + let tx_envelope_len = tx_metrics.total_len - prost::length_delimiter_len(empty_body_len); - // Full length of the transaction can then be derived from the envelope length - // and the length of the body field, taking into account the varint encoding + // Full length of the transaction can then be derived from the length of the invariable + // envelope and the length of the body field, taking into account the varint encoding // of the body field's length delimiter. fn tx_len(envelope_len: usize, body_len: usize) -> usize { - envelope_len + prost::length_delimiter_len(body_len) - 1 + body_len + envelope_len + prost::length_delimiter_len(body_len) + body_len } let mut current_count = 0; - let mut current_len = 0; + let mut current_len = empty_body_len; let mut current_batch = vec![]; for message in messages { @@ -312,25 +314,19 @@ fn batch_messages( // field tag (small varint) and the length delimiter. let tagged_len = 1 + prost::length_delimiter_len(message_len) + message_len; - // Check if the message is too big to fit into the transaction size limit - // even if the message is taken alone. - if tx_len(tx_envelope_len, tagged_len) > max_tx_size { - return Err(Error::message_too_big_for_tx(message_len)); - } - if current_count >= max_message_count || tx_len(tx_envelope_len, current_len + tagged_len) > max_tx_size { let insert_batch = mem::take(&mut current_batch); - assert!( - !insert_batch.is_empty(), - "max message count should not be 0" - ); + if insert_batch.is_empty() { + assert!(max_message_count != 0); + return Err(Error::message_too_big_for_tx(message_len)); + } batches.push(insert_batch); current_count = 0; - current_len = 0; + current_len = empty_body_len; } current_count += 1; diff --git a/relayer/src/chain/cosmos/encode.rs b/relayer/src/chain/cosmos/encode.rs index e6a18376f8..43dcfa5f49 100644 --- a/relayer/src/chain/cosmos/encode.rs +++ b/relayer/src/chain/cosmos/encode.rs @@ -34,14 +34,19 @@ pub fn sign_and_encode_tx( encode_tx_raw(tx_raw) } -pub fn encoded_tx_len( +pub struct EncodedTxMetrics { + pub total_len: usize, + pub body_bytes_len: usize, +} + +pub fn encoded_tx_metrics( config: &TxConfig, key_entry: &KeyEntry, account: &Account, tx_memo: &Memo, messages: &[Any], fee: &Fee, -) -> Result { +) -> Result { let signed_tx = sign_tx(config, key_entry, account, tx_memo, messages, fee)?; let tx_raw = TxRaw { @@ -50,9 +55,13 @@ pub fn encoded_tx_len( signatures: signed_tx.signatures, }; - let len = tx_raw.encoded_len(); + let total_len = tx_raw.encoded_len(); + let body_bytes_len = tx_raw.body_bytes.len(); - Ok(len) + Ok(EncodedTxMetrics { + total_len, + body_bytes_len, + }) } pub fn sign_tx( From be3e0ed74879def59e69bd5ffd347b433a5412cd Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Thu, 25 Aug 2022 20:05:33 +0300 Subject: [PATCH 14/19] Document EncodedTxMetrics --- relayer/src/chain/cosmos/encode.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/relayer/src/chain/cosmos/encode.rs b/relayer/src/chain/cosmos/encode.rs index 43dcfa5f49..68a61fd2ca 100644 --- a/relayer/src/chain/cosmos/encode.rs +++ b/relayer/src/chain/cosmos/encode.rs @@ -34,8 +34,11 @@ pub fn sign_and_encode_tx( encode_tx_raw(tx_raw) } +/// Length information for an encoded transaction. pub struct EncodedTxMetrics { + /// Total length of the transaction encoding. pub total_len: usize, + /// Length of the byte array in the `body_bytes` field of the `TxRaw` message. pub body_bytes_len: usize, } From 8d624785b036d92606402a86a81db27130268e63 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Fri, 26 Aug 2022 10:43:13 +0300 Subject: [PATCH 15/19] Revert "relayer: module chain::cosmos::test_utils" This reverts commit 6a8e31f7d150f0596ddad73a87ef05a6ffd38af3. --- relayer/src/chain/cosmos.rs | 3 - relayer/src/chain/cosmos/batch.rs | 85 +++++++++++++++----------- relayer/src/chain/cosmos/test_utils.rs | 54 ---------------- 3 files changed, 51 insertions(+), 91 deletions(-) delete mode 100644 relayer/src/chain/cosmos/test_utils.rs diff --git a/relayer/src/chain/cosmos.rs b/relayer/src/chain/cosmos.rs index 2023c8a180..2db9175591 100644 --- a/relayer/src/chain/cosmos.rs +++ b/relayer/src/chain/cosmos.rs @@ -106,9 +106,6 @@ pub mod types; pub mod version; pub mod wait; -#[cfg(test)] -mod test_utils; - /// fraction of the maximum block size defined in the Tendermint core consensus parameters. pub const GENESIS_MAX_BYTES_MAX_FRACTION: f64 = 0.9; // https://github.com/cosmos/cosmos-sdk/blob/v0.44.0/types/errors/errors.go#L115-L117 diff --git a/relayer/src/chain/cosmos/batch.rs b/relayer/src/chain/cosmos/batch.rs index a523cc40a9..4fcdc3ef93 100644 --- a/relayer/src/chain/cosmos/batch.rs +++ b/relayer/src/chain/cosmos/batch.rs @@ -344,17 +344,50 @@ fn batch_messages( #[cfg(test)] mod tests { use super::batch_messages; - use crate::chain::cosmos::test_utils::TestFixture; + use crate::chain::cosmos::types::account::{Account, AccountNumber, AccountSequence}; + use crate::chain::cosmos::types::config::TxConfig; + use crate::config; use crate::config::types::{MaxMsgNum, MaxTxSize, Memo}; + use crate::keyring::{self, KeyEntry, KeyRing}; + use ibc::core::ics24_host::identifier::ChainId; use ibc_proto::google::protobuf::Any; + use std::fs; + + const COSMOS_HD_PATH: &str = "m/44'/118'/0'/0/0"; + + fn test_fixture() -> (TxConfig, KeyEntry, Account) { + let path = concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/config/fixtures/relayer_conf_example.toml" + ); + let config = config::load(path).expect("could not parse config"); + let chain_id = ChainId::from_string("chain_A"); + let chain_config = config.find_chain(&chain_id).unwrap(); + + let tx_config = TxConfig::try_from(chain_config).expect("could not obtain tx config"); + + let path = concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/config/fixtures/relayer-seed.json" + ); + let seed_file_content = fs::read_to_string(path).unwrap(); + let keyring = KeyRing::new(keyring::Store::Memory, "cosmos", &chain_id).unwrap(); + let hd_path = COSMOS_HD_PATH.parse().unwrap(); + let key_entry = keyring + .key_from_seed_file(&seed_file_content, &hd_path) + .unwrap(); + + let account = Account { + number: AccountNumber::new(0), + sequence: AccountSequence::new(0), + }; + + (tx_config, key_entry, account) + } #[test] fn batch_does_not_exceed_max_tx_size() { - let TestFixture { - tx_config, - key_entry, - account, - } = TestFixture::new(); + let (config, key_entry, account) = test_fixture(); let messages = vec![ Any { type_url: "/example.Foo".into(), @@ -370,7 +403,7 @@ mod tests { }, ]; let batches = batch_messages( - &tx_config, + &config, MaxMsgNum::default(), MaxTxSize::new(214).unwrap(), &key_entry, @@ -394,18 +427,14 @@ mod tests { #[test] fn batch_error_on_oversized_message() { - let TestFixture { - tx_config, - key_entry, - account, - } = TestFixture::new(); + let (config, key_entry, account) = test_fixture(); let messages = vec![Any { type_url: "/example.Foo".into(), value: vec![0; 6], }]; let batches = batch_messages( - &tx_config, + &config, MaxMsgNum::default(), MaxTxSize::new(192).unwrap(), &key_entry, @@ -419,7 +448,7 @@ mod tests { assert_eq!(batches[0].len(), 1); let res = batch_messages( - &tx_config, + &config, MaxMsgNum::default(), MaxTxSize::new(191).unwrap(), &key_entry, @@ -433,11 +462,7 @@ mod tests { #[test] fn test_batches_are_structured_appropriately_per_max_msg_num() { - let TestFixture { - tx_config, - key_entry, - account, - } = TestFixture::new(); + let (config, key_entry, account) = test_fixture(); // Ensure that when MaxMsgNum is 1, the resulting batch // consists of 5 smaller batches, each with a single message let messages = vec![ @@ -464,7 +489,7 @@ mod tests { ]; let batches = batch_messages( - &tx_config, + &config, MaxMsgNum::new(1).unwrap(), MaxTxSize::default(), &key_entry, @@ -483,7 +508,7 @@ mod tests { // Ensure that when MaxMsgNum > the number of messages, the resulting // batch consists of a single smaller batch with all of the messages let batches = batch_messages( - &tx_config, + &config, MaxMsgNum::new(100).unwrap(), MaxTxSize::default(), &key_entry, @@ -499,11 +524,7 @@ mod tests { #[test] fn test_batches_are_structured_appropriately_per_max_tx_size() { - let TestFixture { - tx_config, - key_entry, - account, - } = TestFixture::new(); + let (config, key_entry, account) = test_fixture(); // Ensure that when MaxTxSize is only enough to fit each one of the messages, // the resulting batch consists of 5 smaller batches, each with a single message. let messages = vec![ @@ -530,7 +551,7 @@ mod tests { ]; let batches = batch_messages( - &tx_config, + &config, MaxMsgNum::default(), MaxTxSize::new(196).unwrap(), &key_entry, @@ -550,7 +571,7 @@ mod tests { // resulting batch consists of a single smaller batch with all of // messages inside let batches = batch_messages( - &tx_config, + &config, MaxMsgNum::default(), MaxTxSize::max(), &key_entry, @@ -567,13 +588,9 @@ mod tests { #[test] #[should_panic(expected = "`max_msg_num` must be greater than or equal to 1, found 0")] fn test_max_msg_num_of_zero_panics() { - let TestFixture { - tx_config, - key_entry, - account, - } = TestFixture::new(); + let (config, key_entry, account) = test_fixture(); let _batches = batch_messages( - &tx_config, + &config, MaxMsgNum::new(0).unwrap(), MaxTxSize::default(), &key_entry, diff --git a/relayer/src/chain/cosmos/test_utils.rs b/relayer/src/chain/cosmos/test_utils.rs deleted file mode 100644 index 45768d196a..0000000000 --- a/relayer/src/chain/cosmos/test_utils.rs +++ /dev/null @@ -1,54 +0,0 @@ -//! Shared utilities for unit tests. - -use std::fs; - -use ibc::core::ics24_host::identifier::ChainId; - -use crate::chain::cosmos::types::account::{Account, AccountNumber, AccountSequence}; -use crate::chain::cosmos::types::config::TxConfig; -use crate::config; -use crate::keyring::{self, KeyEntry, KeyRing}; - -const COSMOS_HD_PATH: &str = "m/44'/118'/0'/0/0"; - -pub struct TestFixture { - pub tx_config: TxConfig, - pub key_entry: KeyEntry, - pub account: Account, -} - -impl TestFixture { - pub fn new() -> Self { - let path = concat!( - env!("CARGO_MANIFEST_DIR"), - "/tests/config/fixtures/relayer_conf_example.toml" - ); - let config = config::load(path).expect("could not parse config"); - let chain_id = ChainId::from_string("chain_A"); - let chain_config = config.find_chain(&chain_id).unwrap(); - - let tx_config = TxConfig::try_from(chain_config).expect("could not obtain tx config"); - - let path = concat!( - env!("CARGO_MANIFEST_DIR"), - "/tests/config/fixtures/relayer-seed.json" - ); - let seed_file_content = fs::read_to_string(path).unwrap(); - let keyring = KeyRing::new(keyring::Store::Memory, "cosmos", &chain_id).unwrap(); - let hd_path = COSMOS_HD_PATH.parse().unwrap(); - let key_entry = keyring - .key_from_seed_file(&seed_file_content, &hd_path) - .unwrap(); - - let account = Account { - number: AccountNumber::new(0), - sequence: AccountSequence::new(0), - }; - - TestFixture { - tx_config, - key_entry, - account, - } - } -} From 0555dd10cea19bbcf3957cb7facd01aea39e8c7a Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Sat, 27 Aug 2022 00:27:53 +0300 Subject: [PATCH 16/19] batch: fix size estimation for empty body When the TxBody struct is completely empty (all fields have default values), it is encoded as an empty array and the body_bytes field is then also omitted from the encoding of TxRaw. Add transaction size verification to the unit tests of batch_messages, by creating encoded tx from the batched messages (using the maximum fee to keep unit tests simple) and checking the size corresponds to the `MaxTxSize` parameter. --- relayer/src/chain/cosmos/batch.rs | 59 +++++++++++++++++++++++++----- relayer/src/chain/cosmos/encode.rs | 11 ++++-- 2 files changed, 57 insertions(+), 13 deletions(-) diff --git a/relayer/src/chain/cosmos/batch.rs b/relayer/src/chain/cosmos/batch.rs index 4fcdc3ef93..2176405d98 100644 --- a/relayer/src/chain/cosmos/batch.rs +++ b/relayer/src/chain/cosmos/batch.rs @@ -293,14 +293,19 @@ fn batch_messages( // Use the maximum possible fee to get an upper bound for varint encoding. let max_fee = gas_amount_to_fee(&config.gas_config, config.gas_config.max_gas); let tx_metrics = encoded_tx_metrics(config, key_entry, account, tx_memo, &[], &max_fee)?; + let tx_envelope_len = tx_metrics.envelope_len; let empty_body_len = tx_metrics.body_bytes_len; - let tx_envelope_len = tx_metrics.total_len - prost::length_delimiter_len(empty_body_len); // Full length of the transaction can then be derived from the length of the invariable // envelope and the length of the body field, taking into account the varint encoding // of the body field's length delimiter. fn tx_len(envelope_len: usize, body_len: usize) -> usize { - envelope_len + prost::length_delimiter_len(body_len) + body_len + if body_len == 0 { + // An empty body is not encoded as a field + envelope_len + } else { + envelope_len + 1 + prost::length_delimiter_len(body_len) + body_len + } } let mut current_count = 0; @@ -344,6 +349,8 @@ fn batch_messages( #[cfg(test)] mod tests { use super::batch_messages; + use crate::chain::cosmos::encode::sign_and_encode_tx; + use crate::chain::cosmos::gas::gas_amount_to_fee; use crate::chain::cosmos::types::account::{Account, AccountNumber, AccountSequence}; use crate::chain::cosmos::types::config::TxConfig; use crate::config; @@ -387,6 +394,8 @@ mod tests { #[test] fn batch_does_not_exceed_max_tx_size() { + const MAX_TX_SIZE: usize = 216; + let (config, key_entry, account) = test_fixture(); let messages = vec![ Any { @@ -402,17 +411,20 @@ mod tests { value: vec![0; 2], }, ]; + let memo = Memo::new("").unwrap(); let batches = batch_messages( &config, MaxMsgNum::default(), - MaxTxSize::new(214).unwrap(), + MaxTxSize::new(MAX_TX_SIZE).unwrap(), &key_entry, &account, - &Memo::new("").unwrap(), + &memo, messages, ) .unwrap(); + let max_fee = gas_amount_to_fee(&config.gas_config, config.gas_config.max_gas); + assert_eq!(batches.len(), 2); assert_eq!(batches[0].len(), 2); assert_eq!(batches[0][0].type_url, "/example.Foo"); @@ -420,26 +432,39 @@ mod tests { assert_eq!(batches[0][1].type_url, "/example.Bar"); assert_eq!(batches[0][1].value.len(), 4); + let tx_bytes = + sign_and_encode_tx(&config, &key_entry, &account, &memo, &batches[0], &max_fee) + .unwrap(); + assert_eq!(tx_bytes.len(), MAX_TX_SIZE); + assert_eq!(batches[1].len(), 1); assert_eq!(batches[1][0].type_url, "/example.Baz"); assert_eq!(batches[1][0].value.len(), 2); + + let tx_bytes = + sign_and_encode_tx(&config, &key_entry, &account, &memo, &batches[1], &max_fee) + .unwrap(); + assert!(tx_bytes.len() <= MAX_TX_SIZE); } #[test] fn batch_error_on_oversized_message() { + const MAX_TX_SIZE: usize = 203; + let (config, key_entry, account) = test_fixture(); let messages = vec![Any { type_url: "/example.Foo".into(), value: vec![0; 6], }]; + let memo = Memo::new("example").unwrap(); let batches = batch_messages( &config, MaxMsgNum::default(), - MaxTxSize::new(192).unwrap(), + MaxTxSize::new(MAX_TX_SIZE).unwrap(), &key_entry, &account, - &Memo::new("").unwrap(), + &memo, messages.clone(), ) .unwrap(); @@ -447,13 +472,19 @@ mod tests { assert_eq!(batches.len(), 1); assert_eq!(batches[0].len(), 1); + let max_fee = gas_amount_to_fee(&config.gas_config, config.gas_config.max_gas); + let tx_bytes = + sign_and_encode_tx(&config, &key_entry, &account, &memo, &batches[0], &max_fee) + .unwrap(); + assert_eq!(tx_bytes.len(), MAX_TX_SIZE); + let res = batch_messages( &config, MaxMsgNum::default(), - MaxTxSize::new(191).unwrap(), + MaxTxSize::new(MAX_TX_SIZE - 1).unwrap(), &key_entry, &account, - &Memo::new("").unwrap(), + &memo, messages, ); @@ -524,6 +555,8 @@ mod tests { #[test] fn test_batches_are_structured_appropriately_per_max_tx_size() { + const MAX_TX_SIZE: usize = 198; + let (config, key_entry, account) = test_fixture(); // Ensure that when MaxTxSize is only enough to fit each one of the messages, // the resulting batch consists of 5 smaller batches, each with a single message. @@ -549,22 +582,28 @@ mod tests { value: vec![0; 10], }, ]; + let memo = Memo::new("").unwrap(); let batches = batch_messages( &config, MaxMsgNum::default(), - MaxTxSize::new(196).unwrap(), + MaxTxSize::new(MAX_TX_SIZE).unwrap(), &key_entry, &account, - &Memo::new("").unwrap(), + &memo, messages.clone(), ) .unwrap(); assert_eq!(batches.len(), 5); + let max_fee = gas_amount_to_fee(&config.gas_config, config.gas_config.max_gas); + for batch in batches { assert_eq!(batch.len(), 1); + let tx_bytes = + sign_and_encode_tx(&config, &key_entry, &account, &memo, &batch, &max_fee).unwrap(); + assert_eq!(tx_bytes.len(), MAX_TX_SIZE); } // Ensure that when MaxTxSize > the size of all the messages, the diff --git a/relayer/src/chain/cosmos/encode.rs b/relayer/src/chain/cosmos/encode.rs index 68a61fd2ca..181892336b 100644 --- a/relayer/src/chain/cosmos/encode.rs +++ b/relayer/src/chain/cosmos/encode.rs @@ -36,8 +36,8 @@ pub fn sign_and_encode_tx( /// Length information for an encoded transaction. pub struct EncodedTxMetrics { - /// Total length of the transaction encoding. - pub total_len: usize, + /// Length of the encoded message, excluding the `body_bytes` field. + pub envelope_len: usize, /// Length of the byte array in the `body_bytes` field of the `TxRaw` message. pub body_bytes_len: usize, } @@ -60,9 +60,14 @@ pub fn encoded_tx_metrics( let total_len = tx_raw.encoded_len(); let body_bytes_len = tx_raw.body_bytes.len(); + let envelope_len = if body_bytes_len == 0 { + total_len + } else { + total_len - 1 - prost::length_delimiter_len(body_bytes_len) - body_bytes_len + }; Ok(EncodedTxMetrics { - total_len, + envelope_len, body_bytes_len, }) } From d68dededb641d9f0cd2a9afc48066b5bbcc80f32 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Mon, 29 Aug 2022 00:10:52 +0300 Subject: [PATCH 17/19] Improve batch_does_not_exceed_max_tx_size test Test larger numbers of messages, up to 100 (maximum allowed per MaxMsgNum limits), with length increasing in the arithmetic progression. For each round, make sure that the size limit is just equal to the size of an encoded tx with all messages except the last one. Verify that the last message is spilled into the next batch and the resulting tx size for the first batch equals the limit. --- relayer/src/chain/cosmos/batch.rs | 91 +++++++++++++++---------------- 1 file changed, 45 insertions(+), 46 deletions(-) diff --git a/relayer/src/chain/cosmos/batch.rs b/relayer/src/chain/cosmos/batch.rs index 2176405d98..d330462a73 100644 --- a/relayer/src/chain/cosmos/batch.rs +++ b/relayer/src/chain/cosmos/batch.rs @@ -394,57 +394,56 @@ mod tests { #[test] fn batch_does_not_exceed_max_tx_size() { - const MAX_TX_SIZE: usize = 216; - let (config, key_entry, account) = test_fixture(); - let messages = vec![ - Any { - type_url: "/example.Foo".into(), - value: vec![0; 6], - }, - Any { - type_url: "/example.Bar".into(), - value: vec![0; 4], - }, - Any { - type_url: "/example.Baz".into(), - value: vec![0; 2], - }, - ]; - let memo = Memo::new("").unwrap(); - let batches = batch_messages( - &config, - MaxMsgNum::default(), - MaxTxSize::new(MAX_TX_SIZE).unwrap(), - &key_entry, - &account, - &memo, - messages, - ) - .unwrap(); - let max_fee = gas_amount_to_fee(&config.gas_config, config.gas_config.max_gas); + let mut messages = vec![Any { + type_url: "/example.Baz".into(), + value: vec![0; 2], + }]; + let memo = Memo::new("").unwrap(); + for n in 0..100 { + messages.insert( + messages.len() - 1, + Any { + type_url: "/example.Foo".into(), + value: vec![0; n], + }, + ); + let expected_batch_len = messages.len() - 1; + let tx_bytes = sign_and_encode_tx( + &config, + &key_entry, + &account, + &memo, + &messages[..expected_batch_len], + &max_fee, + ) + .unwrap(); + let max_tx_size = MaxTxSize::new(tx_bytes.len()).unwrap(); + + let batches = batch_messages( + &config, + MaxMsgNum::new(100).unwrap(), + max_tx_size, + &key_entry, + &account, + &memo, + messages.clone(), + ) + .unwrap(); - assert_eq!(batches.len(), 2); - assert_eq!(batches[0].len(), 2); - assert_eq!(batches[0][0].type_url, "/example.Foo"); - assert_eq!(batches[0][0].value.len(), 6); - assert_eq!(batches[0][1].type_url, "/example.Bar"); - assert_eq!(batches[0][1].value.len(), 4); - - let tx_bytes = - sign_and_encode_tx(&config, &key_entry, &account, &memo, &batches[0], &max_fee) - .unwrap(); - assert_eq!(tx_bytes.len(), MAX_TX_SIZE); + assert_eq!(batches.len(), 2); + assert_eq!(batches[0].len(), expected_batch_len); - assert_eq!(batches[1].len(), 1); - assert_eq!(batches[1][0].type_url, "/example.Baz"); - assert_eq!(batches[1][0].value.len(), 2); + let tx_bytes = + sign_and_encode_tx(&config, &key_entry, &account, &memo, &batches[0], &max_fee) + .unwrap(); + assert_eq!(tx_bytes.len(), max_tx_size.to_usize()); - let tx_bytes = - sign_and_encode_tx(&config, &key_entry, &account, &memo, &batches[1], &max_fee) - .unwrap(); - assert!(tx_bytes.len() <= MAX_TX_SIZE); + assert_eq!(batches[1].len(), 1); + assert_eq!(batches[1][0].type_url, "/example.Baz"); + assert_eq!(batches[1][0].value.len(), 2); + } } #[test] From 17425692a9058a30e82f5147c231e30582827bd8 Mon Sep 17 00:00:00 2001 From: Mikhail Zabaluev Date: Mon, 29 Aug 2022 13:21:29 +0300 Subject: [PATCH 18/19] Remove an unnecessary branch --- relayer/src/chain/cosmos/batch.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/relayer/src/chain/cosmos/batch.rs b/relayer/src/chain/cosmos/batch.rs index d330462a73..c51a3ec73f 100644 --- a/relayer/src/chain/cosmos/batch.rs +++ b/relayer/src/chain/cosmos/batch.rs @@ -300,12 +300,9 @@ fn batch_messages( // envelope and the length of the body field, taking into account the varint encoding // of the body field's length delimiter. fn tx_len(envelope_len: usize, body_len: usize) -> usize { - if body_len == 0 { - // An empty body is not encoded as a field - envelope_len - } else { - envelope_len + 1 + prost::length_delimiter_len(body_len) + body_len - } + // The caller has at least one message field length added to the body's + debug_assert!(body_len != 0); + envelope_len + 1 + prost::length_delimiter_len(body_len) + body_len } let mut current_count = 0; From 3acf895e115be73cd19314630485a5d7733b0f15 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 30 Aug 2022 09:32:11 +0200 Subject: [PATCH 19/19] Add changelog entry --- .../improvements/ibc-relayer/2575-tx-size-encoding.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/ibc-relayer/2575-tx-size-encoding.md diff --git a/.changelog/unreleased/improvements/ibc-relayer/2575-tx-size-encoding.md b/.changelog/unreleased/improvements/ibc-relayer/2575-tx-size-encoding.md new file mode 100644 index 0000000000..d82d64e5d5 --- /dev/null +++ b/.changelog/unreleased/improvements/ibc-relayer/2575-tx-size-encoding.md @@ -0,0 +1,2 @@ +- Account for full transaction encoding when batching messages + ([#2575](https://github.com/informalsystems/ibc-rs/issues/2575)) \ No newline at end of file