Skip to content

Conversation

@random-zebra
Copy link

@random-zebra random-zebra commented Jun 18, 2020

This is an alternative to #1664.
It takes the opposite approach.
When ATMP fails in CommitTransaction, instead of just returning the txid (hoping that the failure is not permanent) try to directly abandon the transaction.
Save the result of these operations in a new struct CWallet::CommitResult, and use it to give a detailed feedback to the user (via log lines, JSON-RPC errors and GUI dialogs).

Based on top of:

This PR also cherry-picks 5f12263 from bitcoin#6898, in order to use FormatStateMessage outside of main.

@random-zebra random-zebra added this to the 4.2.0 milestone Jun 18, 2020
@random-zebra random-zebra self-assigned this Jun 18, 2020
@random-zebra random-zebra force-pushed the 202006_ATMP-in-CommitTx branch from 62cf90c to 7086794 Compare June 20, 2020 17:19
@random-zebra
Copy link
Author

Rebased after merge of #1670. Ready for review.

Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

Concept ACK 👍 . Left a comment.

A minor code style topic would be if it's better to return CommitResult instead of pass it by reference (it would make easier back ports like #1663 that adds a new arg).

@random-zebra random-zebra force-pushed the 202006_ATMP-in-CommitTx branch from 7086794 to cecc6cc Compare June 22, 2020 10:22
@random-zebra
Copy link
Author

Rebased and moved CommitResult to the return value of CommitTransaction as per @furszy's suggestion.

furszy
furszy previously approved these changes Jun 22, 2020
Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

Side from the comment above, code ACK cecc6cc.

Also introduce CommitResult struct to send proper feedback to the
user in case of failures.
Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

ACK a0de4da

@random-zebra random-zebra requested a review from Fuzzbawls June 22, 2020 21:29
Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

ACK a0de4da

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.

4 participants