-
Notifications
You must be signed in to change notification settings - Fork 36
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
[#1452] TX Resubmission-the wallet has to periodically resubmit unmined transactions #1454
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK. I just left two comments inline. Other than that, I see some lint warnings, which we could resolve in a separate PR as they seem unrelated to these changes.
Sources/ZcashLightClientKit/Block/Actions/TxResubmissionAction.swift
Outdated
Show resolved
Hide resolved
Example/ZcashLightClientSample/ZcashLightClientSample/DemoAppConfig.swift
Show resolved
Hide resolved
Example/ZcashLightClientSample/ZcashLightClientSample/Send/SendViewController.swift
Outdated
Show resolved
Hide resolved
…ically resubmit unmined transactions (Electric-Coin-Company#1454) - addressed comments from the PR, logs enhanced to feature height and demo app logs issues first before any other code is executed
…ically resubmit unmined transactions (Electric-Coin-Company#1454) - addressed comments from the PR, logs enhanced to feature height and demo app logs issues first before any other code is executed [Electric-Coin-Company#1452] TX Resubmission-the wallet has to periodically resubmit unmined transactions (Electric-Coin-Company#1454) - txId log
0f155b2
to
9bf8fce
Compare
…ically resubmit unmined transactions (Electric-Coin-Company#1454) - addressed comments from the PR, logs enhanced to feature height and demo app logs issues first before any other code is executed [Electric-Coin-Company#1452] TX Resubmission-the wallet has to periodically resubmit unmined transactions (Electric-Coin-Company#1454) - txId log [Electric-Coin-Company#1452] TX Resubmission-the wallet has to periodically resubmit unmined transactions (Electric-Coin-Company#1454) - logs tweaked
9bf8fce
to
de9b8b5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK. The only concern I have is that the current Zashi wallet UX prompts users with a trx submission error and still keeps them on the Send.Confirmation screen, which could lead users to retry the trx confirmation. This new feature might potentially successfully resubmit the first failed one later, resulting in two transactions. We could rethink this Zashi UX when adopting the updated SDK version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed de9b8b5
…ically resubmit unmined transactions (Electric-Coin-Company#1454) - State machine puml and png files updated - Order of transaction removed because it has no effect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 85c7834
…ically resubmit unmined transactions - functional version is done [Electric-Coin-Company#1452] TX Resubmission-the wallet has to periodically resubmit unmined transactions - code cleanup [Electric-Coin-Company#1452] TX Resubmission-the wallet has to periodically resubmit unmined transactions - changelog updated [Electric-Coin-Company#1452] TX Resubmission-the wallet has to periodically resubmit unmined transactions - unit tests fixed - mocks generated
…ically resubmit unmined transactions (Electric-Coin-Company#1454) - addressed comments from the PR, logs enhanced to feature height and demo app logs issues first before any other code is executed [Electric-Coin-Company#1452] TX Resubmission-the wallet has to periodically resubmit unmined transactions (Electric-Coin-Company#1454) - txId log [Electric-Coin-Company#1452] TX Resubmission-the wallet has to periodically resubmit unmined transactions (Electric-Coin-Company#1454) - logs tweaked
…ically resubmit unmined transactions (Electric-Coin-Company#1454) - State machine puml and png files updated - Order of transaction removed because it has no effect
85c7834
to
25582fe
Compare
This code review checklist is intended to serve as a starting point for the author and reviewer, although it may not be appropriate for all types of changes (e.g. fixing a spelling typo in documentation). For more in-depth discussion of how we think about code review, please see Code Review Guidelines.
Author
Reviewer