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

Rahul/ifl 2581 bug watch command watches expired transactions #4968

Merged

Conversation

patnir
Copy link
Contributor

@patnir patnir commented May 16, 2024

Summary

We treat the expired transaction state as an intermediate state like pending/ unconfirmed. It should be an end state of a transaction.

Before:

send command:
image

watch:command:
image

Now:

watch command:
image

send command:
image

Testing Plan

  1. Send a transaction with an expiration that is pretty close to the current block
  2. We no longer watch this transaction and exit early

Documentation

Does this change require any updates to the Iron Fish Docs (ex. the RPC API
Reference
)? If yes, link a
related documentation pull request for the website.

[ ] Yes

Breaking Change

Is this a breaking change? If yes, add notes below on why this is breaking and label it with breaking-change-rpc or breaking-change-sdk.

[ ] Yes

@patnir patnir changed the base branch from master to staging May 16, 2024 19:20
@patnir patnir marked this pull request as ready for review May 16, 2024 19:22
@patnir patnir requested a review from a team as a code owner May 16, 2024 19:22
@@ -488,8 +491,8 @@ export async function watchTransaction(options: {
let currentStatus = prevStatus

// If the transaction is already in the desired state, return
if (currentStatus === waitUntil) {
logger.log(`Transaction ${options.hash} is ${waitUntil}`)
if (currentStatus !== 'not found' && waitUntil.includes(currentStatus)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is currentStatus !== 'not found' necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was a weird type issue because of the assignment above. It was introduced here: https://github.com/iron-fish/ironfish/pull/3726/files . Probably not the best initial implementation tbh. I can look into fixing it but might be out of scope of this PR?

@patnir patnir requested a review from andiflabs May 21, 2024 23:52
@patnir patnir merged commit 7920d49 into staging May 28, 2024
20 of 22 checks passed
@patnir patnir deleted the rahul/ifl-2581-bug-watch-command-watches-expired-transactions branch May 28, 2024 17:01
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.

2 participants