Skip to content

[Sapling][Refactoring] Save wallet reference inside SaplingOperation#2294

Merged
random-zebra merged 4 commits into
PIVX-Project:masterfrom
random-zebra:202104_sapling_wallet
Apr 14, 2021
Merged

[Sapling][Refactoring] Save wallet reference inside SaplingOperation#2294
random-zebra merged 4 commits into
PIVX-Project:masterfrom
random-zebra:202104_sapling_wallet

Conversation

@random-zebra
Copy link
Copy Markdown

This complements #2293, trying to achieve the same goal on the sapling code (remove all rogue accesses to pwalletMain).
Save a pointer to a wallet inside SaplingOperation, initialized in the constructor, and passed directly to the TransactionBuilder (which needs a keystore to produce signatures for the transparent inputs).

The transaction builder should be initialized and set only when the
sapling_operation is constructed.
Save a wallet reference when initializing sapling_operation, and pass it
to the txbuilder (so we are sure that the keystore used to sign the keys
belongs to the wallet used to retrieve the inputs).
Copy link
Copy Markdown

@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.

code review ACK 80054f7

Comment thread src/sapling/sapling_operation.h Outdated
Copy link
Copy Markdown

@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 b1e1f4e

@random-zebra random-zebra requested a review from Fuzzbawls April 14, 2021 07:40
Copy link
Copy Markdown
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 b1e1f4e

@random-zebra
Copy link
Copy Markdown
Author

Merging...

@random-zebra random-zebra merged commit b776825 into PIVX-Project:master Apr 14, 2021
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