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

Bridges - changes for Bridges V2 - relay client part #4494

Merged
merged 16 commits into from
Jun 14, 2024

Conversation

bkontur
Copy link
Contributor

@bkontur bkontur commented May 17, 2024

Contains mainly changes/nits/refactors related to the relayer code (client-substrate and lib-substrate-relay) migrated from the Bridges V2 branch.

Relates to: paritytech/parity-bridges-common#2976
Companion: paritytech/parity-bridges-common#2988

TODO

  • fix comments

Questions

  • Do we need more testing for client V2 stuff? If so, how/what is the ultimate test? @svyatonik

@bkontur bkontur added R0-silent Changes should not be mentioned in any release notes T15-bridges This PR/Issue is related to bridges. labels May 17, 2024
@svyatonik svyatonik force-pushed the bko-bridges-backports-and-nits branch from 1cdb84f to 7a394d8 Compare May 17, 2024 09:49
@svyatonik svyatonik self-requested a review May 17, 2024 09:51
@svyatonik
Copy link
Contributor

@svyatonik In traitified v2 client we are using shared subscriptions, so #4481 shouldn't be a problem when we talk about long-lived subscriptions (justification). But we need to test it for transaction subscriptions - right now I've rebased completely without #4481

@bkontur bkontur force-pushed the bko-bridges-backports-and-nits branch 3 times, most recently from c4c55e2 to 12c59b5 Compare May 22, 2024 23:12
@bkontur bkontur changed the title Bridges - backport changes/nits as a part of preparation for Bridges V2 Bridges - changes for Bridges V2 - relay client part May 23, 2024
@bkontur bkontur self-assigned this May 23, 2024
svyatonik and others added 6 commits May 23, 2024 10:34
* Revert "Revert new client (#2153)"

This reverts commit 17cf7b596170419a1e9b9d39d7e329acdb1a87de.

* add comment re cache to CachingClient (#2162)

* do not wait for reader when sending items (#2163)

* Use unsync cache + async-aware sync primitives (#2164)

* use unsync cache + async-aware sync primitives

* clippy

* trigger CI

* Revert "trigger CI"

This reverts commit b5c1405492409b85abf552ba0b089c000824f722.
…ble (#2206)

* Avoid using extra background task for TransactionTracker

* Add docs
* final check before transaction submit

* pass pre-encoded transaction to validate
* read best + best finalized headers from subscription

* spelling

* clippy
@bkontur bkontur force-pushed the bko-bridges-backports-and-nits branch from 12c59b5 to 6a021c8 Compare May 23, 2024 08:37
@bkontur bkontur marked this pull request as ready for review May 23, 2024 08:38
@paritytech-review-bot paritytech-review-bot bot requested a review from a team May 23, 2024 08:39
github-merge-queue bot pushed a commit that referenced this pull request May 23, 2024
Cherry-picked fix from upcoming
#4494

---------

Co-authored-by: Svyatoslav Nikolsky <[email protected]>
Co-authored-by: command-bot <>
Copy link
Contributor

@svyatonik svyatonik left a comment

Choose a reason for hiding this comment

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

FWIW: all code changes were already reviewed before in a https://github.com/paritytech/parity-bridges-common/ repository. I did a shallow check and porting looks good and should work with all existing bridges. However I also want to perform that check: #4494 (comment). Will do that tomorrow and approve if all is ok or push some commits to fix it if changes are required

Copy link
Contributor

@svyatonik svyatonik left a comment

Choose a reason for hiding this comment

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

Checked the possible subscription leak issue - all is good

Copy link
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

Did shallow review only since this has already been reviewed on development branch

hitchhooker pushed a commit to ibp-network/polkadot-sdk that referenced this pull request Jun 5, 2024
Cherry-picked fix from upcoming
paritytech#4494

---------

Co-authored-by: Svyatoslav Nikolsky <[email protected]>
Co-authored-by: command-bot <>
Copy link
Contributor

@serban300 serban300 left a comment

Choose a reason for hiding this comment

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

Also took just a high-level look since this has already been reviewed before. Looks good !

@bkontur bkontur added this pull request to the merge queue Jun 14, 2024
Merged via the queue into master with commit 977254c Jun 14, 2024
156 of 157 checks passed
@bkontur bkontur deleted the bko-bridges-backports-and-nits branch June 14, 2024 11:55
bkontur added a commit to paritytech/parity-bridges-common that referenced this pull request Jun 14, 2024
bkontur added a commit to paritytech/parity-bridges-common that referenced this pull request Jun 14, 2024
* Dienerized to polkadot-sdk@bko-bridges-backports-and-nits

* cargo update -p sp-io

* Change substrate-relay version

* Update polkadot-sdk master ref

* Setup latest master with merged: paritytech/polkadot-sdk#4494

* Change relayer version for release

---------

Co-authored-by: Svyatoslav Nikolsky <[email protected]>
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
Cherry-picked fix from upcoming
paritytech#4494

---------

Co-authored-by: Svyatoslav Nikolsky <[email protected]>
Co-authored-by: command-bot <>
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
Contains mainly changes/nits/refactors related to the relayer code
(`client-substrate` and `lib-substrate-relay`) migrated from the Bridges
V2 [branch](paritytech#4427).

Relates to:
paritytech/parity-bridges-common#2976
Companion: paritytech/parity-bridges-common#2988


## TODO
- [x] fix comments

## Questions
- [x] Do we need more testing for client V2 stuff? If so, how/what is
the ultimate test? @svyatonik
- [x] check
[comment](paritytech#4494 (comment))
for more testing

---------

Co-authored-by: Svyatoslav Nikolsky <[email protected]>
Co-authored-by: Serban Iorga <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T15-bridges This PR/Issue is related to bridges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants