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

transaction-pool: Improve transaction status documentation and add helpers #3215

Merged
merged 7 commits into from
Feb 12, 2024

Conversation

lexnv
Copy link
Contributor

@lexnv lexnv commented Feb 5, 2024

This PR improves the transaction status documentation.

  • Added doc references for describing the main states
  • Extra comment wrt pool ready / future queues
  • FinalityTimeout no longer describes a lagging finality gadget, it signals that the maximum number of finality gadgets has been reached

A few helper methods are added to indicate when:

  • a final event is generated by the transaction pool for a given event
  • a final event is provided, although the transaction might become valid at a later time and could be re-submitted

The helper methods are used and taken from #3079 to help us better keep it in sync.

cc @paritytech/subxt-team

The documentation is improved by having doc links to the states that
the transaction pool enumarates. This is to ensure that a rename
would cause the `docs` check to fail.

Also, the `FinalityTimeout` only happens when the maximum number
of watchers has been reached. Contrary to the documentation that
states a lagging finality gadget.

Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv lexnv self-assigned this Feb 5, 2024
@lexnv lexnv added A1-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). R0-silent Changes should not be mentioned in any release notes I5-enhancement An additional feature request. labels Feb 5, 2024
@lexnv lexnv requested review from bkchr and skunert February 5, 2024 17:54
/// finalized. The `FinalityTimeout` event will be emitted when the block did not reach finality
/// within 512 blocks. This either indicates that finality is not available for your chain,
/// or that finality gadget is lagging behind. If you choose to wait for finality longer, you can
/// re-subscribe for a particular transaction hash manually again.
Copy link
Contributor

@michalkucharczyk michalkucharczyk Feb 5, 2024

Choose a reason for hiding this comment

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

I think description of FinalityTimeout is worth keeping. It reflects current implementation.

I am not sure if transaction is retriable when finality timeout event was reported. It could be the case, but it also could mean that transaction was actually finalized. Maybe it is worth putting more info on this.

@lexnv lexnv enabled auto-merge February 8, 2024 16:39
@lexnv lexnv added this pull request to the merge queue Feb 12, 2024
Merged via the queue into master with commit 4f13d5b Feb 12, 2024
95 of 125 checks passed
@lexnv lexnv deleted the lexnv/improve-tx-docs branch February 12, 2024 10:15
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
…lpers (paritytech#3215)

This PR improves the transaction status documentation.
- Added doc references for describing the main states
- Extra comment wrt pool ready / future queues
- `FinalityTimeout` no longer describes a lagging finality gadget, it
signals that the maximum number of finality gadgets has been reached

A few helper methods are added to indicate when:
- a final event is generated by the transaction pool for a given event
- a final event is provided, although the transaction might become valid
at a later time and could be re-submitted

The helper methods are used and taken from
paritytech#3079 to help us better
keep it in sync.


cc @paritytech/subxt-team

---------

Signed-off-by: Alexandru Vasile <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A1-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). I5-enhancement An additional feature request. R0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants