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

Change hermes to use broadcast_tx_sync for transaction submission #1091

Merged
merged 67 commits into from
Jun 22, 2021

Conversation

ancazamfir
Copy link
Collaborator

@ancazamfir ancazamfir commented Jun 15, 2021

Closes: #930 #986 #919 #977 #978

Description

TBD

TODO

  • Adjust retry strategy in wait_for_block_commits (code, 92df6b5)
  • Clean up config file (rpc_addrs, code, 529c20d)
  • Clear up the two definitions of send_msgs (impl, trait, b00063c)
  • Update changelog

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.

@romac romac removed the in progress label Jun 18, 2021
@adizere adizere self-requested a review June 21, 2021 08:57
let pk_any = Any {
type_url: "/cosmos.crypto.secp256k1.PubKey".to_string(),
value: pk_buf,
self.acct_seq = acct_response.sequence;
Copy link
Member

Choose a reason for hiding this comment

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

It looks like we're retrieving here the account information (using self.account()?; on line 169) and we use the account sequence number only if this number is not already cached (it gets cached in field self.acct_seq). If the sequence is already cached, we ignore the retrieved value. On line 256 we manually increase the cached sequence number (self.acct_seq += 1;). Wouldn't it make more sense to always use the retrieved sequence number?

@adizere adizere merged commit 3e53090 into master Jun 22, 2021
@adizere adizere deleted the anca/evets_multi_tx branch June 22, 2021 07:19
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
…formalsystems#1091)

* temp change for ft-transfer to send one msg/Tx

* event monitor: Bulk events from all transactions included in a block

* Update changelog

* temp change for ft-transfer to send one msg/Tx

* Optimize spawning of workers - draft

* Add back check to start workers only if channel is open

* Cleanup

* Check connection state

* temp change for ft-transfer to send one msg/Tx

* Improve config loading message (informalsystems#933)

* Improve config load message

* Raised log level to error. Log link to config example in the guide.

* Changelog

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

* Migration to tx sync

* Add Tx simulate

* Add adjustment to the tx simulate result, some cleanup

* Nitpick in CosmosSdkChain::key_and_bytes

* Small cleanup

* Remove duplicate send_msgs method

* Cleanup config file

* Fix typo after refactoring

* Fix `query packet tx` description

* Rework `wait_for_block_commits` to use the `retry` crate

* Compute tx fee based on gas from tx simulate and gas price

* Re-add missing error type

* Combine `fee_denom` and `gas_price` into `gas_price` config option

* Add tests for `mul_ceil`

* Fix config serialization

* Remove `fee_amount` config property

* Update changelog

* Avoid op data regeneration if retries exhausted.

* Increase the number of retries while checking Tx is included in a block.

* Move `query packet tx` to `query tx events`

* better error msgs

* Add Display instance for GasPrice

* Fix default gas price denomination

* Improve some debug messages

* Rename `gas_price.amount` to `gase_price.price`

* Add configurable fee adjustment to make it more likely gas estimate ends up being enough

* Add Tx hash to ChainErr

* Fix config files

* Masked tonic::Code::NotFound result for query_client_connections.

* Modified cfg option from gas to max_gas

* Consistent trust_threshold in default config.toml

* Revert guide updates

* Nit: Imports for query.rs

* Print info message when Hermes starts

* Fix gas adjustement to be percentage on top of computed gas

* Variables renamed to be more explicit in calculate_fee and send_tx.

* Cached acct number in CosmosSDK

* Cache account information

* Remove noisy tracing in counterparty module

* Fix unused import

* Improve logs a bit by using the Display impl for Height

* Only show first 100 consensus states in logs

* Attempt to fix gas_limit

cherry-picked
0886401
from romac/channel-filter

* Adjust up to max_gas

Co-authored-by: Romain Ruetschi <[email protected]>
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
3 participants