-
Notifications
You must be signed in to change notification settings - Fork 358
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
Allow instantiating relayer with different wallets in integration test #2040
Labels
A: blocked
Admin: blocked by another (internal/external) issue or PR
O: tests
Objective: Test more aspect of the relayer
Milestone
Comments
6 tasks
6 tasks
adizere
added
the
A: blocked
Admin: blocked by another (internal/external) issue or PR
label
Apr 5, 2022
romac
pushed a commit
that referenced
this issue
Apr 13, 2022
This PR attempts to rework the core logic of `send_tx` methods in `CosmosSdkChain` into plain functions. This is essential in helping us to solve the root problems in issues such as #2012, #2040, and supporting sending of transactions with fees in ICS29. At this stage, I have managed to refactor `send_tx_with_account_sequence_retry` and its inner calls into plain functions, and simplify the data access a little. Since this is a refactoring PR, no logic should be changed. The integration tests should help to ensure that nothing breaks with the refactoring. --- * Refactoring send_tx * WIP refactoring * More code reorganization * Split out functions from main cosmos.rs * Use refactored code to send_tx * Walk through send_tx_with_account_sequence * Refactor code * Reorder function arguments * Refactor send_tx_with_account_sequence_retry into plain function * Refactor account query functions * Refactor query_tx * Refactor wait_for_block_commits * Turn wait_for_block_commits into simple loop * Refactor send_messages_and_wait_commit * Refactor send_messages_and_wait_check_tx * Refactor sign_message * Refactor gas config * Move out query account module * Reorganize types * Remove pub const * Simplify arguments * Remove redundant account query function * Refactor query status * Introduce TransferTimeout abstraction * Use prost::Message::encoded_len() to compute encoded message length * Address review feedback * Re-add missing comments * Fix clippy error * Remove check for both timeout height offset and duration being zero * Do not set timeout height or time when input is zero
hu55a1n1
pushed a commit
to hu55a1n1/hermes
that referenced
this issue
Sep 13, 2022
This PR attempts to rework the core logic of `send_tx` methods in `CosmosSdkChain` into plain functions. This is essential in helping us to solve the root problems in issues such as informalsystems#2012, informalsystems#2040, and supporting sending of transactions with fees in ICS29. At this stage, I have managed to refactor `send_tx_with_account_sequence_retry` and its inner calls into plain functions, and simplify the data access a little. Since this is a refactoring PR, no logic should be changed. The integration tests should help to ensure that nothing breaks with the refactoring. --- * Refactoring send_tx * WIP refactoring * More code reorganization * Split out functions from main cosmos.rs * Use refactored code to send_tx * Walk through send_tx_with_account_sequence * Refactor code * Reorder function arguments * Refactor send_tx_with_account_sequence_retry into plain function * Refactor account query functions * Refactor query_tx * Refactor wait_for_block_commits * Turn wait_for_block_commits into simple loop * Refactor send_messages_and_wait_commit * Refactor send_messages_and_wait_check_tx * Refactor sign_message * Refactor gas config * Move out query account module * Reorganize types * Remove pub const * Simplify arguments * Remove redundant account query function * Refactor query status * Introduce TransferTimeout abstraction * Use prost::Message::encoded_len() to compute encoded message length * Address review feedback * Re-add missing comments * Fix clippy error * Remove check for both timeout height offset and duration being zero * Do not set timeout height or time when input is zero
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A: blocked
Admin: blocked by another (internal/external) issue or PR
O: tests
Objective: Test more aspect of the relayer
Summary
Make it simple to spawn relayer supervisor and
ChainHandle
s with different wallet provided in the config.Problem Definition
A key challenge with the current design of
ChainHandle
is that the wallet is configured pseudo-globally and cannot be changed dynamically or overridden when a transaction is submitted. This present challenges when writing integration tests, because if the same wallet is used concurrently in different operations, the account sequence mismatch errors will happen and cause the tests to potentially fail.This presents problem if we want to spawn multiple relayers to run in parallel, as in the case of #2024. To do so, we need to have ways to instantiate multiple relayer configurations with different wallets, and specify which relayer or
ChainHandle
we want to choose from.We also have similar problem of sending transactions with custom wallets which is described in #1975.
One complication for the design is that we cannot easily swap the wallets within a relayer's
Config
value. This is because the wallets are different for each chain, so we have to effectively change a whole set of wallets depending on how many chains are present in the config.Proposal
#2039 is a work in progress to figure out how we can solve this problem without too much boilerplate. It uses const generics with a hardcoded array of relayer wallets, so that tests can for example get 3 sets of relayers to be used.
For Admin Use
The text was updated successfully, but these errors were encountered: