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

Improve sending of bls transaction by checkpointing module. #155

Closed
KonradStaniec opened this issue Sep 22, 2022 · 2 comments
Closed

Improve sending of bls transaction by checkpointing module. #155

KonradStaniec opened this issue Sep 22, 2022 · 2 comments

Comments

@KonradStaniec
Copy link
Contributor

Currently at appropriate time checkpointing module just start concurrent goroutine which send bls transactions:
https://github.com/babylonchain/babylon/blob/main/x/checkpointing/abci.go#L45

This fine for now as it get job done.

The main downside is that if at epoch boundary, validator account would send another transaction concurrently (for example transfer) , then bls transaction could fail due to invalid sequence number error.

This is a bit corner case scenario as epoch won't change that ofter but taking into account that this is consensus critical tx it is worth improving this situation.

Probably simplest one is to add some kinda of retry mechanism.
Another one, is to track life time of bls transaction, and buffer transactions send from validator account through this node during this lifetime.

@gitferry
Copy link
Contributor

Thanks for reporting this issue. Indeed, it is critical to consensus and should be improved carefully. @gusin13 has been working on a retry mechanism (#50 and #72) that has been applied in the vigilante reporter which also has the incorrect sequence number issue when sending BTC block headers to Babylon. As suggested by @vitsalis, maybe we can also use the same retry mechanism in the checkpointing module.

One bit is that I can't catch the invalid sequence number error as it is reported back in the raw_log of the tx response and it seems that the tx is successfully executed regardless of the error. @KonradStaniec do you have any ideas of how we can catch the error and do the retry?

@gitferry
Copy link
Contributor

Resolved in #166

maurolacy added a commit that referenced this issue Jan 22, 2024
Add download contract from release script
maurolacy added a commit that referenced this issue Jan 22, 2024
* Update babylon-contract

* Update README

* Fix localnet nodes 3 & 4

* Fix babylon contract instantiation

* Fix / adapt contract execution test

* Contract download script (#155)

Add download contract from release script

* Fix typos

* Reduce network / IB relayer start wait time

* Move IBC config to chain config

* Add BTC Timestamping Phase 2 test

* Improve syntax

Co-authored-by: Runchao Han <me@runchao.rocks>

* Revert "Fix localnet nodes 3 & 4"

This reverts commit aaf3707b28985d5f03261107d0fad518fc478527.

* Now fix localnet nodes 3 & 4

* Simplify phase-2 vs phase-1 test setup

* Simplify phase 2 test

* Make chain-B the CZ chain

* Use regtest const for network

* Use default checkpoint tag const for babylon tag

Update contract to custom

* Fix comment

Co-authored-by: Runchao Han <me@runchao.rocks>

* Update contract to custom

* Fix original test as well

* Increase delay to wait for chain ready

* Add (failing) next IBC packet sequence check

* Fix: contains wasm check

* Storage contract e2e (#171)

* Revert "Update babylon-contract"

This reverts commit 7c656b183818098f2662ac9996cde80b8f4009ba.

* Update storage-contract to custom

* Revert "Update README"

This reverts commit 3669c88d16b2ea1c8185ed53a74be0a0ae3b11ea.

* Revert "Fix babylon contract instantiation"

This reverts commit 1fb44d7fcfac3d79a4c8b9323a56eb8b0a837d83.

* Revert "Fix / adapt contract execution test"

This reverts commit 124099dd64dd519aba0774d3e5a7e43d7a44162a.

* Fix imports

* fix phase-2 e2e test (#163)

* Upgrade keyring / go-keychain dependency (#170)

* Add cosmos relayer docker image build

* Reduce cosmos-relayer image

* Set / use relayer tag

* Improve hermes bootstrap script

* Set ABCI packet persistence

* Add Cosmos relayer setup

* Enable debug logs

* Reduce relayer setup wait time

* Simplify seup / remove commented lines

* fix

* Fix: packet acknowledgements check

* Rename for consistency

* Same acknowledgements logic for Hermes relayer

* More test renaming for consistency

* Skip BTC timestamping phase 2 test (Hermes)

* Fix: BTC timestamping phase 1 tests

* Improve flaky BTC timestamping tests

* Return network to RegTest

* Update contracts code to latest

* Remove docker Platform directive / commented code

* Increase wait for blocks delay

* Add cosmos relayer docker build target to e2e target deps

* Fix rebase errors

* Remove unused helper

* Add TODOs

* Remove commented code

* Comment BTC timestamping hermes test out

* Restore original BTC timestamping test

* Report target arch / Go lang version

* Hardcode target arch for CI

* Hardcode next expected sequence

---------

Co-authored-by: Runchao Han <me@runchao.rocks>
Co-authored-by: Mauro Lacy <mauro@babylonchain.io>
maurolacy added a commit that referenced this issue Jan 22, 2024
* Update babylon-contract

* Update README

* Fix localnet nodes 3 & 4

* Fix babylon contract instantiation

* Fix / adapt contract execution test

* Contract download script (#155)

Add download contract from release script

* Fix typos

* Reduce network / IB relayer start wait time

* Move IBC config to chain config

* Add BTC Timestamping Phase 2 test

* Improve syntax

Co-authored-by: Runchao Han <me@runchao.rocks>

* Revert "Fix localnet nodes 3 & 4"

This reverts commit aaf3707b28985d5f03261107d0fad518fc478527.

* Now fix localnet nodes 3 & 4

* Simplify phase-2 vs phase-1 test setup

* Simplify phase 2 test

* Make chain-B the CZ chain

* Use regtest const for network

* Use default checkpoint tag const for babylon tag

Update contract to custom

* Fix comment

Co-authored-by: Runchao Han <me@runchao.rocks>

* Update contract to custom

* Fix original test as well

* Increase delay to wait for chain ready

* Add (failing) next IBC packet sequence check

* Fix: contains wasm check

* Storage contract e2e (#171)

* Revert "Update babylon-contract"

This reverts commit 7c656b183818098f2662ac9996cde80b8f4009ba.

* Update storage-contract to custom

* Revert "Update README"

This reverts commit 3669c88d16b2ea1c8185ed53a74be0a0ae3b11ea.

* Revert "Fix babylon contract instantiation"

This reverts commit 1fb44d7fcfac3d79a4c8b9323a56eb8b0a837d83.

* Revert "Fix / adapt contract execution test"

This reverts commit 124099dd64dd519aba0774d3e5a7e43d7a44162a.

* Fix imports

* fix phase-2 e2e test (#163)

* Upgrade keyring / go-keychain dependency (#170)

* Add cosmos relayer docker image build

* Reduce cosmos-relayer image

* Set / use relayer tag

* Improve hermes bootstrap script

* Set ABCI packet persistence

* Add Cosmos relayer setup

* Enable debug logs

* Reduce relayer setup wait time

* Simplify seup / remove commented lines

* fix

* Fix: packet acknowledgements check

* Rename for consistency

* Same acknowledgements logic for Hermes relayer

* More test renaming for consistency

* Skip BTC timestamping phase 2 test (Hermes)

* Fix: BTC timestamping phase 1 tests

* Improve flaky BTC timestamping tests

* Return network to RegTest

* Update contracts code to latest

* Remove docker Platform directive / commented code

* Increase wait for blocks delay

* Add cosmos relayer docker build target to e2e target deps

* Fix rebase errors

* Remove unused helper

* Add TODOs

* Remove commented code

* Comment BTC timestamping hermes test out

* Restore original BTC timestamping test

* Report target arch / Go lang version

* Hardcode target arch for CI

* Hardcode next expected sequence

---------

Co-authored-by: Runchao Han <me@runchao.rocks>
Co-authored-by: Mauro Lacy <mauro@babylonchain.io>
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

No branches or pull requests

2 participants