Hermes may break the max_tx_size configuration param

[ancazamfir]

Summary of Bug

When we batch messages in a Tx we may exceed the max_tx_size if max_msg_num is not hit. The problem code is here:
https://github.com/informalsystems/ibc-rs/blob/1d188187721051a34d20841c0146b8c16b5b413b/relayer/src/chain/cosmos/batch.rs#L152-L157
We keep in the batch the msg that caused the crossing of max_tx_size.

It becomes an issue with chains like osmosis where Tx-es with size bigger than some threshold may be priced differently resulting in Tx simulation failure and no recovery (see #2422, the threshold is exceeded with 3 msgs, one update client and two packet messages)

In addition, the actual Tx will be slightly bigger due to extra auth encoding and we should do something about it.

Version

all recent

Steps to Reproduce

The code is obviously wrong but:

• add assert!(current_size < max_tx_size); after L154
• configure hermes with:

max_msg_num = 50
max_tx_size = 15000

• send 100 packets

Acceptance Criteria

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate milestone (priority) applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[soareschen]

Since batch_messages is now a pure function, we should write some unit tests that test the behavior of batch_messages directly without having to run the full integration test.

[mzabaluev]

Since batch_messages is now a pure function, we should write some unit tests that test the behavior of batch_messages directly without having to run the full integration test.

Thanks for reminding, it's good unit testable function indeed.

[seanchen1991]

@mzabaluev I've started working on adding some unit tests