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

[Sapling] Try to recover corruption of notes cache during send operation #2027

Merged

Conversation

random-zebra
Copy link

This is a contingency plan, in case of missing cached nullifier of a spent note in the wallet.
Since, during the spend, the wallet is unlocked, and we have the spending key, we can directly recover the nullifier from the note (provided that we have a valid cached witness) and fix the issue in the background, without requiring any user intervention.

If we don't have the witness, we can still provide a better log and error message in the gui, than a generic, and probably obscure

bad-txns-shielded-requirements-not-met

@random-zebra random-zebra added this to the 5.0.0 milestone Dec 4, 2020
@random-zebra random-zebra self-assigned this Dec 4, 2020
src/qt/pivx/send.cpp Outdated Show resolved Hide resolved
src/qt/walletmodel.cpp Outdated Show resolved Hide resolved
src/sapling/sapling_operation.cpp Outdated Show resolved Hide resolved
src/sapling/sapling_operation.cpp Outdated Show resolved Hide resolved
@random-zebra
Copy link
Author

Locks updated.

furszy
furszy previously approved these changes Dec 4, 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.

really nice and straightforward contingency plan while we search for the cause 👍.
ACK 0dc0896

@furszy furszy self-requested a review December 4, 2020 21:39
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.

Haven't checked it with the corrupted/invalid wallet, pure code review, ACK 9c6bf2d.

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.

Have tested it with the corrupted wallet and all went well but have to say haven't seen any message of the added logging which is weird.
But well, it's an edge case scenario that we should discover the issue soon anyway (in testnet).
All good for me to go ahead with this one.

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 9c6bf2d

@random-zebra random-zebra merged commit 1ee90a4 into PIVX-Project:master Dec 5, 2020
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.

None yet

3 participants