This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
rpc: Use the blocks pinning API for chainHead methods #13233
Merged
Merged
Changes from 36 commits
Commits
Show all changes
48 commits
Select commit
Hold shift + click to select a range
a72b8a8
rpc/chain_head: Add backend to subscription management
lexnv d672047
rpc/chain_head: Pin blocks internally and adjust testing
lexnv c9a0d7e
client/in_mem: Reference for the number of pinned blocks
lexnv 3536c94
rpc/tests: Check in-memory references to pinned blocks
lexnv bdfdb7d
rpc/chain_head: Fix clippy
lexnv 43d6825
rpc/chain_head: Remove unused comment
lexnv 6bc2e87
rpc/chain_head: Place subscription handle under `Arc` and unpin block…
lexnv 9015061
rpc/tests: Check all pinned blocks are unpinned on drop
lexnv 09773d3
Apply suggestions from code review
lexnv 4a38133
Update client/rpc-spec-v2/src/chain_head/subscription.rs
lexnv e509399
rpc/tests: Retry fetching the pinned references for CI correctness
lexnv 3c2cc6c
Merge remote-tracking branch 'origin/master' into lexnv/chainhead_pin…
lexnv c6bfb6f
Merge remote-tracking branch 'origin/master' into lexnv/chainhead_pin…
09fe732
Merge remote-tracking branch 'origin/master' into lexnv/chainhead_pin…
b7dcacd
client/service: Use 512 as maximum number of pinned blocks
lexnv d67c6ed
Merge remote-tracking branch 'origin/master' into lexnv/chainhead_pin…
lexnv fa5323f
chain_head: Fix merging conflicts
lexnv 852c302
rpc/chain_head: Adjust subscriptions to use pinning API
lexnv cdd1428
rpc/chain_head/tests: Test subscription management
lexnv ad9e921
rpc/chain_head: Adjust chain_head follow to the new API
lexnv 866bb87
rpc/chain_head: Adjust chain_head.rs to the new API
lexnv b2e08ce
rpc/chain_head/tests: Adjust test.rs to the new API
lexnv b63f102
client/builder: Use new chainHead API
lexnv c46d7d4
rpc/chain_head: Fix documentation
lexnv 7da0406
rpc/chain_head: Fix clippy
lexnv 5561e56
client/in_mem: ChainHead no longer uses `in_mem::children`
lexnv d36f748
Merge remote-tracking branch 'origin/master' into lexnv/chainhead_pin…
lexnv ff3c9d5
Merge remote-tracking branch 'origin/master' into lexnv/chainhead_pin…
lexnv 9781584
Update client/rpc-spec-v2/src/chain_head/subscription.rs
lexnv cc31043
Update client/rpc-spec-v2/src/chain_head/subscription.rs
lexnv efb14c4
Update client/rpc-spec-v2/src/chain_head/subscription.rs
lexnv 10b61d5
Update client/rpc-spec-v2/src/chain_head/subscription.rs
lexnv c5266cc
chain_head: Add block state machine
lexnv f5a911f
Merge remote-tracking branch 'origin/master' into lexnv/chainhead_pin…
lexnv 7bf9da0
Address feedback
lexnv 6c054ba
Merge remote-tracking branch 'origin/master' into lexnv/chainhead_pin…
lexnv 6a3f9e5
Use new_native_or_wasm_executor
lexnv d287533
chain_head: Remove 'static on Backend
lexnv 6073ae2
chain_head: Add documentation
lexnv 871dcb8
chain_head: Lock blocks before async blocks
lexnv 083e3c3
chain_head_follower: Remove static on backend
lexnv 1bb4b00
Update client/service/src/builder.rs
lexnv 04a167c
Update client/service/src/builder.rs
lexnv cc24ad1
chain_head: Add BlockHeaderAbsent to the PartialEq impl
lexnv b3ca736
client: Add better documentation around pinning constants
lexnv 3b1764f
chain_head: Move subscription to dedicated module
lexnv 9cd53f1
Merge remote-tracking branch 'origin/master' into lexnv/chainhead_pin…
lexnv 8b0eaff
subscription: Rename global pin / unpin functions
lexnv File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a nit; nowadays I'm leaning towards not putting any trait bounds on generic params in structs unless they are needed for the definition of the struct (though I've been back and forth on this myself), so I guess I'd lean towards removing the bounds if possible (but curious on what others think!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I do like that approach. One nice thing about not having bounds on the structure is that we do not contaminate every structure containing that type.
For soundness reasons I presume, the compiler requires the bounds for the
Drop
impl to also be present on the struct.The
SubscriptionHandle
implementsDrop
that needs access to theunpin_block
method defined byBackend<Block>
trait.And unfortunately, this then leaks into
SubscriptionManagement
and intoChainHead
itself.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this ; TIL
Drop
impls with trait bounds need matching ones on the struct :)