[Wallet] Shielded transactions balance cache system. #1952
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… db (only used in unit tests).
… pointer. The block index is not needed and it will make unit tests simpler to code.
… dummy transparent inputs.
…AddToWalletIfInvolvingMe.
…pling note if needed.
* Validates: * 1) CWalletTx getCredit for shielded credit. * Incoming spendable shielded balance must be cached in the cacheableAmounts. * * 2) CWalletTx getDebit & getCredit for shielded debit to transparent address. * Same wallet as point (1), spending half of the credit received in (1) to a transparent remote address. * The other half of the balance - minus fee - must appear as credit (shielded change).
…lded transactions.
furszy
force-pushed
the
2020_v5_privacy_cached_tx_balances
branch
from
23bf366 to
e14bbbf
Compare
Author
|
rebased on master now that #1798 got merged. |
…ote every time that the wallet needs to know the note value.
Now GetCredit will not need to decrypt the note on every single call. Only will do it once if the note value is not cached and set it. Plus, added a GetCredit method to retrieve the value of an specific shielded output.
furszy
force-pushed
the
2020_v5_privacy_cached_tx_balances
branch
from
e14bbbf to
9047f74
Compare
Author
|
Pushed the fix for the lock held assertion issue on the new test cases. Travis happy now, ready for review. |
random-zebra
approved these changes
Nov 8, 2020
random-zebra
left a comment
random-zebra
left a comment
There was a problem hiding this comment.
utACK 9047f74 (with few nits that can be ignored).
| { | ||
| LOCK(cs_wallet); | ||
| if (!AddToWalletIfInvolvingMe(tx, pindex, posInBlock, true)) | ||
| if (!AddToWalletIfInvolvingMe(tx, (pindex) ? pindex->GetBlockHash() : uint256(), posInBlock, true)) |
| ISMINE_SPENDABLE_SHIELDED = 1 << 5, | ||
| ISMINE_SPENDABLE_ALL = ISMINE_SPENDABLE_DELEGATED | ISMINE_SPENDABLE | ISMINE_SPENDABLE_SHIELDED, | ||
| ISMINE_WATCH_ONLY_ALL = ISMINE_WATCH_ONLY | ISMINE_WATCH_ONLY_SHIELDED, | ||
| ISMINE_ALL = ISMINE_WATCH_ONLY | ISMINE_SPENDABLE | ISMINE_COLD | ISMINE_SPENDABLE_DELEGATED | ISMINE_SPENDABLE_SHIELDED | ISMINE_WATCH_ONLY_SHIELDED, |
There was a problem hiding this comment.
nit:
Suggested change
| ISMINE_ALL = ISMINE_WATCH_ONLY | ISMINE_SPENDABLE | ISMINE_COLD | ISMINE_SPENDABLE_DELEGATED | ISMINE_SPENDABLE_SHIELDED | ISMINE_WATCH_ONLY_SHIELDED, | |
| ISMINE_ALL = ISMINE_SPENDABLE_ALL | ISMINE_WATCH_ONLY_ALL | ISMINE_COLD |
Comment on lines
+505
to
+506
| CAmount SaplingScriptPubKeyMan::GetCredit(const CWalletTx& tx, const isminefilter& filter, const bool fUnspent) | ||
| { |
There was a problem hiding this comment.
Might be good to assert(tx.SapData) at the beginning of this function.
random-zebra
added a commit
that referenced
this pull request
Nov 10, 2020
…isSaplingVersion fc9ffe5 SSPKM: fix GetShieldedChange, tx.isSapling migrating to tx.isSaplingVersion (furszy) Pull request description: Small and quick fix, master is currently not compiling due #1955 and #1952 getting merged without being based one on the other. the first PR renamed the tx `isSapling` method to `isSaplingVersion` and in the second PR, a new function was implemented `GetShieldedChange` that was using using `isSapling`. ACKs for top commit: Fuzzbawls: ACK fc9ffe5 random-zebra: utACK fc9ffe5 and merging... Tree-SHA512: 463df6059b756bf1301faad5b3780b74720e5520232686b225bb3af02288e441af70a44a19803e530c5b922d92fd7feb949ecfc59685f797d7943471b6137bc3
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Up until now, one of the several capabilities that #1798 introduced was the wallet's shielded transactions balances calculation, a purely on-demand process (which is not so performant as txs balances are requested more than once during the entire software lifecycle).
This PR is going further, introducing a new wallet balance cache system for shielded transactions. Covered by a good amount of units tests that can be run pre and post the new cache system.
This PR is obviously built on top of 1798. Part of the shielded GUI work that will be coming soon.