add support for sending mortal txs#43
Conversation
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
| let block_ref = sink | ||
| .api() | ||
| .backend() | ||
| .latest_finalized_block_ref() | ||
| .await | ||
| .expect("to get the last finalized block ref. qed"); | ||
| let block = sink | ||
| .api() | ||
| .blocks() | ||
| .at(block_ref) | ||
| .await | ||
| .expect("to get the corresponding block header. qed"); | ||
| let block_number = block.number().into(); |
There was a problem hiding this comment.
This deserves some comment, as it is tricky and the logic is aligned with subxt:
https://github.com/paritytech/subxt/blob/77b6abccbacf194f3889610024e2f4024e8c2822/subxt/src/tx/tx_client.rs#L600
Would be nice to see a reference to inject_account_nonce_and_block function.
I think that it also allows for some little race condition - if some block is finalized in meantime. But don't see a better way to implement it now. Maybe we could ask subxt if they could expose actual blocks in Params/DefaultExtrinsicParams somehow?
There was a problem hiding this comment.
I think that it also allows for some little race condition - if some block is finalized in meantime. But don't see a better way to implement it now.
Yes, but thought that even if a new finalized block happens in the meantime, that means will wait just a little longer with the block monitor for the transaction to be finalized (which is not that bad I think).
Would be nice to see a reference to inject_account_nonce_and_block function.
Done here: 2df3030
There was a problem hiding this comment.
Maybe we could ask subxt if they could expose actual blocks in Params/DefaultExtrinsicParams somehow?
Yes, to be 100% accurate with this. I'll file a question.
There was a problem hiding this comment.
I am reconsidering this a bit. I think we'll need either way some online capability to fetch last finalized block. Can leave this unresolved for now until we'll tackle #44 . We should do some preliminary research then about options before asking for things.
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
michalkucharczyk
left a comment
michalkucharczyk
left a comment
There was a problem hiding this comment.
Do we have issue to follow-up with work done in
paritytech/subxt#2025
?
The code could be simplified, right? We would not need the online client probably?
Left two more comments, once addressed we could merge it.
Co-authored-by: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>
Yes, here: #44 |
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
# Description Based on michalkucharczyk/tx-test-tool#43. ## Integration N/A ## Review Notes - added tests with future mortal txs that are dropped due to not being included in blocks within their lifetime, but also future mortal txs that have a sufficient lifetime to be included in blocks - added test which uses priorities to delay mortal txs inclusion and make them invalid --------- Signed-off-by: Iulian Barbu <iulian.barbu@parity.io> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Sebastian Kunert <mail@skunert.dev>
# Description Based on michalkucharczyk/tx-test-tool#43. ## Integration N/A ## Review Notes - added tests with future mortal txs that are dropped due to not being included in blocks within their lifetime, but also future mortal txs that have a sufficient lifetime to be included in blocks - added test which uses priorities to delay mortal txs inclusion and make them invalid --------- Signed-off-by: Iulian Barbu <iulian.barbu@parity.io> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Sebastian Kunert <mail@skunert.dev>
# Description Based on michalkucharczyk/tx-test-tool#43. ## Integration N/A ## Review Notes - added tests with future mortal txs that are dropped due to not being included in blocks within their lifetime, but also future mortal txs that have a sufficient lifetime to be included in blocks - added test which uses priorities to delay mortal txs inclusion and make them invalid --------- Signed-off-by: Iulian Barbu <iulian.barbu@parity.io> Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Sebastian Kunert <mail@skunert.dev>
2 participants
This PR takes the mortal flag value passed in CLI and passes it over up to subxt transaction building, where we can configure its tx params to contain the mortality.
Closes #14
Review notes
TODO:
test this work by writing a scenario in polkadot-sdk and see how mortal txs work(works after using subxt correctly)look for unit tests that might be implemented in the repo(not that straight forward, useful thing to test is mortal/immortal txs creation, but we have integration tests inpolkadot-sdkthat covers it)maybe enable support for mixing mortal with immortal txs?(not really needed for now)