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 Status Handling #183

Closed
Voxelot opened this issue Feb 22, 2022 · 5 comments
Closed

Transaction Status Handling #183

Voxelot opened this issue Feb 22, 2022 · 5 comments

Comments

@Voxelot
Copy link
Member

Voxelot commented Feb 22, 2022

We need to come up with a more comprehensive strategy for tracking transaction status. Ideally we avoid TTL based complexity, but there's also a need to balance error feedback UX so sdk users know when or why their transaction failed without storing too much junk data in the node.

We currently have 3 simple states:

  • Submitted
  • Success
  • Failed

However, if the transaction was very invalid (i.e. not includable in a block) this seems like it might need to be separate type of error from a tx that was partially processed within a block (i.e. utxos were spent and outputs created, but the script panicked). The former failure would likely be a transitory concern, while the latter would be a permanent part of the node history.

@rakita
Copy link
Contributor

rakita commented Feb 23, 2022

all Included transctions are found in database, while most of pending transaction are found in txpool. We can probably use same mechanism of subscription writen here: #168 to notify on: insertion into txpool/removal from txpool/inclusion in block (and status of that inclusion) so that user can track what happens with its tx.

most troublesome is handling of local pending transactions, eth has priority on them so that user who sends tx to client will be sure that that tx will not be rejected because of low gas and it will have ability to track its state.

@Voxelot
Copy link
Member Author

Voxelot commented Feb 24, 2022

yeah we might just want to do the following when we query for transactions status:

  1. check if transaction status is saved to the db for success or failure state
  2. If not saved to the db, check the txpool to see if it's still pending and return status as submitted.

If a tx is dropped from the txpool, then the status would no longer exist at all. I think this should be ok, as long as the submit API immediately returns any errors to the user if there are obvious problems with the tx.

@Voxelot Voxelot added this to the PoA Testnet milestone Feb 25, 2022
@Voxelot Voxelot removed this from the PoA Testnet milestone Apr 1, 2022
@ControlCplusControlV
Copy link
Contributor

Continuing discussion from my last PR was planning taking on part of this issue, as ideally find_tx checks status to see where a tx is rather its current implementation which just checks if it exists in tx_pool.

AFAIK implementing this PR would expand status to


    - Submitted
    - Success
    - Failed
    - Invalid (With this one being an error more so than a status)
    - Reverted

What more does this issue entail, as I think atleast part of it could be addressed when I fix find_tx, unless I have overlooked something (which is definitely possible)?

@Voxelot
Copy link
Member Author

Voxelot commented Apr 5, 2022

Continuing discussion from my last PR was planning taking on part of this issue, as ideally find_tx checks status to see where a tx is rather its current implementation which just checks if it exists in tx_pool.

I don't think it would make sense to check status first for find_tx, since status is an optional field on the transaction object schema which is only loaded after the transaction is already resolved. https://github.com/FuelLabs/fuel-core/blob/v0.5.0/fuel-core/src/schema/tx/types.rs#L215

We wouldn't need any additional transaction statuses, based on the determination above:

  1. check if transaction status is saved to the db for success or failure state
  2. If not saved to the db, check the txpool to see if it's still pending and return status as submitted.

This is because the status would only exist if the tx was either:

  • still pending in the txpool (Submitted)
  • was included in block
    • and processed successfully (Success)
    • or failed due to panic or revert (Failed)

Since we don't store transaction status unless the tx is committed in a block, we wouldn't have a place to set and store the statuses for Invalid. Reverted isn't adding any extra detail that isn't already included in Failed so I don't think we'd need that one either.

If the transaction was invalid, that should be returned to the user immediately in response to the submit_tx API, rather than being stored as an independent status to be queried later. This has a downside of potentially making it harder to debug scenarios like your transaction being booted from the txpool due to one of it's inputs being used somewhere else, but storing these details would require a more advanced TTL strategy than needed right now.

@xgreenx
Copy link
Collaborator

xgreenx commented May 14, 2024

We don't need to store new statuses in the database because we provide a subscription to the transaction, and the user receives a notification when the transaction is squeezed out. Closing this issue as solved by subscription.

@xgreenx xgreenx closed this as completed May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants