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.

Sign up for GitHub

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

Jump to bottom

[RPC-Spec-V2]: transactionWatch_unstable_submitAndWatch shouldn't use pipe_from_stream #3076

Closed
Tracked by #1516
niklasad1 opened this issue Jan 26, 2024 · 0 comments · Fixed by #4901
Closed
Tracked by #1516

[RPC-Spec-V2]: transactionWatch_unstable_submitAndWatch shouldn't use pipe_from_stream #3076

niklasad1 opened this issue Jan 26, 2024 · 0 comments · Fixed by #4901
Labels
D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. T3-RPC_API This PR/Issue is related to RPC APIs.

Comments

@niklasad1
Copy link
Member

niklasad1 commented Jan 26, 2024 •
edited
Loading

JSON-RPC servers should maintain a buffer of three notifications (one for each property), and overwrite any unsent notification with a more recent status update.

The implementation of transactionWatch_unstable_submitAndWatch of is using pipe_from_stream which in turn keeps a buffer which will drop the subscription if the client can't keep with the server.

Thus, it doesn't comply with the spec and ideally the implementation should keep a buffer of max 3 notifications and replace/merge older ones if it's can't keep up with it.

I propose a custom implementation with a manual buffer to ensure that.

//cc @paritytech/subxt-team
@niklasad1 niklasad1 added T3-RPC_API This PR/Issue is related to RPC APIs. D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. labels Jan 26, 2024
@lexnv lexnv mentioned this issue Jan 26, 2024
Open
@niklasad1 niklasad1 mentioned this issue Jan 26, 2024
Closed
@jsdw jsdw changed the title rpc v2: transactionWatch_unstable_submitAndWatch shouldn't use pipe_from_stream [RPC-Spec-V2]: transactionWatch_unstable_submitAndWatch shouldn't use pipe_from_stream Feb 9, 2024
niklasad1 added a commit that referenced this issue Jun 27, 2024
@niklasad1
rpc v2: submitAndWatch replace msg's lagging
29ed95d 
Close #3076
@niklasad1 niklasad1 mentioned this issue Jun 27, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Jul 29, 2024
@niklasad1
rpc v2: submitAndWatch replace old messages if it's lagging (#4901)
fc10887 
Close #3076

The fix is really just that older messages are replaced if the client
can't keep up with the server instead.
Because I wanted the same functionality as `pipe_from_stream` for both
pending/subscription I added two wrapper types on-top of the types from
jsonrpsee to make it nicer.

I added a trait `Buffer` so I could still use pipe_from_stream but that
abstraction is a little leaky but only to avoid adding an identical
method/function with another strategy...
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this issue Aug 2, 2024
@niklasad1 @TarekkMA
rpc v2: submitAndWatch replace old messages if it's lagging (parity…
bbbd816 
…tech#4901)

Close paritytech#3076

The fix is really just that older messages are replaced if the client
can't keep up with the server instead.
Because I wanted the same functionality as `pipe_from_stream` for both
pending/subscription I added two wrapper types on-top of the types from
jsonrpsee to make it nicer.

I added a trait `Buffer` so I could still use pipe_from_stream but that
abstraction is a little leaky but only to avoid adding an identical
method/function with another strategy...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. T3-RPC_API This PR/Issue is related to RPC APIs.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

rpc v2: submitAndWatch replace old messages if it's lagging paritytech/polkadot-sdk
1 participant
@niklasad1