Skip to content

Conversation

@algorandskiy
Copy link
Contributor

@algorandskiy algorandskiy commented Nov 4, 2022

Summary

Transaction pools might store failed transactions for quite a long time on its StatusCache.
When someone requests a pending transaction and it gets retrieved from the StatusCache,
there is no indication it actually failed. A user might have an impression the txn is fine but stuck.
In order to give a correct feedback, report any errors from the StatusCache via REST API.

@codecov
Copy link

codecov bot commented Nov 4, 2022

Codecov Report

Merging #4755 (7f94674) into master (7f94674) will not change coverage.
The diff coverage is n/a.

❗ Current head 7f94674 differs from pull request most recent head 6606939. Consider uploading reports for the commit 6606939 to get more accurate results

@@           Coverage Diff           @@
##           master    #4755   +/-   ##
=======================================
  Coverage   54.59%   54.59%           
=======================================
  Files         414      414           
  Lines       53665    53665           
=======================================
  Hits        29297    29297           
  Misses      21945    21945           
  Partials     2423     2423           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

jasonpaulos
jasonpaulos previously approved these changes Nov 4, 2022
Copy link
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me

@algorandskiy
Copy link
Contributor Author

ok, removed the statusCache pruning

Copy link
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

I think this is good to take as is. I didn't open a PR for the original issue because I wanted to find a way of testing the change, but I don't think that's worth waiting for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants