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.

Sign up for GitHub

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

Jump to bottom

Give a better error message for transaction expiry #3593

Merged
merged 1 commit into from Aug 14, 2020
Merged

Give a better error message for transaction expiry #3593

merged 1 commit into from Aug 14, 2020

Conversation

arielgabizon
Copy link
Contributor

As mentioned in #3393, user gets an error when sending a tx while wallet not caught up. Here we give the user a better explanation of the possible error.

@ebfull
Copy link
Contributor

ebfull commented Oct 14, 2018

The string you've modified is a technical error name, not a description. We'd want to modify ContextualCheckTransaction(): transaction is expired instead, but since this ends up being produced as part of a log I'm not sure that would be appropriate.

@arielgabizon
Copy link
Contributor Author

There should be some place where it's appropriate to explain to the user a probable cause of their error :)

@mms710 mms710 added this to PRs That Need Review + Their Associated Issues in Arborist Team Dec 13, 2018
@zkbot
Copy link
Contributor

zkbot commented Feb 7, 2020

☔ The latest upstream changes (presumably #4331) made this pull request unmergeable. Please resolve the merge conflicts.

daira
daira approved these changes Aug 12, 2020
View reviewed changes
Copy link
Contributor

@daira daira left a comment

Choose a reason for hiding this comment

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

This is technically consensus code but it doesn't change anything other than the error message, so I don't think it needs 3 reviews.

utACK (with @therealyingtong )

@daira
Copy link
Contributor

daira commented Aug 12, 2020

@zkbot r+

@zkbot
Copy link
Contributor

zkbot commented Aug 12, 2020

📌 Commit bb0d3ac has been approved by daira

@zkbot
Copy link
Contributor

zkbot commented Aug 12, 2020

⌛ Testing commit bb0d3ac with merge 0d2a8e18c0cdb83a6b81f0e7952f5cafec4ccac2...

@zkbot
Copy link
Contributor

zkbot commented Aug 12, 2020

💔 Test failed - pr-merge

@daira daira changed the title explain expiry error Give a better error message for transaction expiry Aug 14, 2020
@daira
Copy link
Contributor

daira commented Aug 14, 2020

Transient failure. @zkbot retry

@zkbot
Copy link
Contributor

zkbot commented Aug 14, 2020

⌛ Testing commit bb0d3ac with merge 3e6ae4e1382bbcae061a38e43171770d5a64ad1e...

@zkbot
Copy link
Contributor

zkbot commented Aug 14, 2020

💔 Test failed - pr-merge

@str4d
Copy link
Contributor

str4d commented Aug 14, 2020

Another transient failure.

@zkbot retry

@zkbot
Copy link
Contributor

zkbot commented Aug 14, 2020

⌛ Testing commit bb0d3ac with merge 99bfa46...

@zkbot
Copy link
Contributor

zkbot commented Aug 14, 2020

☀️ Test successful - pr-merge
Approved by: daira
Pushing 99bfa46 to master...

@zkbot zkbot merged commit 99bfa46 into zcash:master Aug 14, 2020
Arborist Team automation moved this from PRs That Need Review + Their Associated Issues to Released (Merged in Master) Aug 14, 2020
@zkbot zkbot mentioned this pull request Aug 14, 2020
Merged
@str4d str4d added C-cleanup Category: PRs that clean code up or issues documenting cleanup. and removed spring_cleaning PR_cleanup labels Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daira daira daira approved these changes

Assignees
No one assigned
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup.
Projects
Arborist Team
  
Released (Merged in Master)
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@arielgabizon @ebfull @zkbot @daira @str4d @mms710