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

Change tx status method doc #1499

Merged
merged 1 commit into from
Jan 8, 2024
Merged

Change tx status method doc #1499

merged 1 commit into from
Jan 8, 2024

Conversation

telezhnaya
Copy link
Contributor

https://docs.near.org/api/rpc/transactions#transaction-status
In reality, we wait until the block of the last corresponding receipt_id is final, and only then return the result. So the user does not need to check anything.

Return type is FinalExecutionOutcomeView, it's by the definition Final execution outcome of the transaction and all of subsequent the receipts.

I've found this while working on near/nearcore#6837
Maybe I'll change the signature of tx status method or add the new one with more fine-grained way to ask for the result without waiting until everything is final

Change tx status method doc
https://docs.near.org/api/rpc/transactions#transaction-status
In reality, we wait until the block of the last corresponding receipt_id is final, and only then return the result. So the user does not need to check anything.
@render
Copy link

render bot commented Sep 19, 2023

@bucanero
Copy link
Collaborator

let me know if we should merge it 👍

@wacban
Copy link

wacban commented Sep 20, 2023

I'm not sure this is the case, can you explain a bit more? This RPC call eventually ends up in get_final_transaction_result and there it doesn't actually check the status of each receipt, it just returns the status for all available receipts. In particular it could return FinalExecutionStatus::NotStarted for some.

https://github.com/near/nearcore/blob/315d186b5b792bf942455fcbab0a0a5ddb08a621/chain/chain/src/chain.rs#L3605

cc @jakmeier do you happen to know how this works?

@telezhnaya
Copy link
Contributor Author

Valid concern.
If it's true, then I want to clarify how EXPERIMENTAL_tx_status works
The doc is ambiguous
https://docs.near.org/api/rpc/transactions#transaction-status-with-receipts
I thought it gives fully final result
The main difference from tx is that it uses get_final_transaction_result_with_receipt underneath. It does not check if the corresponding blocks are finalised.
So, the receipts may not be included into finalised blocks yet, right?

@wacban @jakmeier I've created the proposal for improving tx status method, please leave any comments there
near/nearcore#9542

@jakmeier
Copy link
Contributor

I am not sure about this TBH. But the description in near/nearcore#6834 suggests we indeed wait for all execution outcomes to be available in the polling loop. So this part of the documentation seems outdated:

In the case of function call transactions, this query will not wait for all receipts generated by this transaction to finish before returning a result. Rather, it will only wait for its return value to finish before returning; which could be a promise.

On the other hand, this seems to be still true:

In addition, tx endpoint does not provide finality guarantees. To make sure that the entire execution is final, it suffices to ensure every block_hash in every outcome is final.

I wrote my exact (but probably flawed) understanding in near/nearcore#9542

@gagdiez gagdiez merged commit 20dee1f into master Jan 8, 2024
2 of 3 checks passed
@wacban
Copy link

wacban commented Jan 10, 2024

@gagdiez I see you merged this PR but I'm not sure if it was ready or abandonded. @telezhnaya, the author, did you mean to get it merged?

@gagdiez
Copy link
Collaborator

gagdiez commented Jan 10, 2024

I saw an approved PR that was not in draft state, so I went ahead and merged it.

If this was a mistake, please let me know so I roll it back.

@telezhnaya
Copy link
Contributor Author

@gagdiez please roll it back
It's not the time, we still need to wait for the release

bucanero added a commit that referenced this pull request Jan 11, 2024
@bucanero
Copy link
Collaborator

@telezhnaya change has been reverted in b9fc109

@bucanero
Copy link
Collaborator

@telezhnaya I have also created a new PR #1660 , and left it as draft state so you can let us know when this change is ready to be merged.

@bucanero bucanero mentioned this pull request Jan 11, 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

Successfully merging this pull request may close these issues.

5 participants