implement variant of subscription that returns finalized storage changes#237
Conversation
7ca22c1 to
5ab0ed3
Compare
|
Hmm this feature might need to wait until after the jsonrpsee V2 upgrade since for some reason the finalized block subscription isn't dropped... |
|
I've merged #214 so you can update this branch now |
34ed3e4 to
7ebf91e
Compare
|
@ascjones please review |
ascjones
left a comment
There was a problem hiding this comment.
Overall looks good. As mentioned I do think we should still allow subscribing to non finalized events when submitting an extrinsic still if we want to trade immediacy for finality.
Was just looking at the polkadot.js api an they do it slightly differently with a status field: https://polkadot.js.org/docs/api/start/api.tx.subs/, but it's a bit more low level than what we have here.
7ebf91e to
d8ac8ff
Compare
|
@ascjones added a workaround to support configuration of the event subscription on |
ascjones
left a comment
There was a problem hiding this comment.
Yes that API looks fine, I'd like the comments expanding so it's clear exactly what is going on to avoid footguns.
| } | ||
|
|
||
| /// Subscribe to finalized events. | ||
| pub async fn subscribe_finalized_events( |
There was a problem hiding this comment.
Perhaps instead of the two different methods we could just check the weak_inclusion flag like we do in the submit transaction method. Though you would need different client instances if you wanted to do both in that case. What do you think?
There was a problem hiding this comment.
For now it's probably cleaner with two distinct methods as we also have subscribe_blocks and subscribe_finalized_blocks.
There was a problem hiding this comment.
Anyway I think we should remove accept_weak_inclusion in favour of some config on the generated traits (like how it is done in polkadot{.js}) pending refactoring of the proc-macro crate.
d8ac8ff to
3e22f0d
Compare
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
3e22f0d to
cc29156
Compare
|
BTW the force pushes make it hard to review updates because I can't see just the changes since I last reviewed. Merge commits are fine. |
Signed-off-by: Gregory Hill gregorydhill@outlook.com
Closes #238
https://github.com/paritytech/substrate-subxt/blob/9959f0d29950219f20b5cc79dfff46f8987239fc/src/rpc.rs#L465
https://github.com/paritytech/substrate-subxt/blob/9959f0d29950219f20b5cc79dfff46f8987239fc/src/rpc.rs#L481