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

Ignore amount=0 zutxos in GetFilteredNotes() by default #104

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

leto
Copy link
Member

@leto leto commented Apr 18, 2020

This needs testing. It implements the good idea from zcash#4429 to ignore amount=0 zutxos unless they are explicitly asked for. Currently the only codepath which explicitly asks for them is Sapling Consolidation, since it can automatically clean up after zdust attacks. Since Hush and HushChat use amount=0 extensively, this optimization should be noticeable. Additionally, it gives users an automatic feature to clean up after shielded spam attacks while never reducing the anonymity set, since Sietch was added to Sapling Consolidation in our codebase.

This is not a consensus change, but it is a backward incompatible change, purposefully. Previously, z_listunspent would return all amount=0 zutxos, by historical accident. No consumer of this data ever uses amount=0 zutxos. So we can add a flag to that RPC and others if there is a use case for amount=0 zutxos. This should greatly reduce the amount of data exchanged by GUI wallet and full node and reduces the amount of JSON to parse.

We need to make sure there is no unintended consequences of this, for instance, listtransactions should still return amount=0 notes but may need to learn about the new options to GetFilteredNotes()

@MarkLTZ
Copy link

MarkLTZ commented Jul 30, 2024

@leto I tried you changes:

                if (notePt.value() == 0) {
                    continue;
                }

But performance of GetFilteredNotes are still horrible.

I think the root cause is the call to SaplingNotePlaintext::decrypt for decrypting each notes.

My opinion is to store with mapSaplingNoteData it's decryped value and when spent set this to -1 as bitcoin does on transparent coins.

In this way also IsSaplingSpent could be optimized.

Same way to solve this issue should be used for the sprout notes (mapSproutNoteData / IsSproutSpent).

I don't remember mapSaplingNoteData and mapSproutNoteData maps only owned notes or the entire list?

@MarkLTZ
Copy link

MarkLTZ commented Jul 31, 2024

A simple 10x booster could be done skipping notes which has been spent and locked notes before decrypting them

See: MarkLTZ/bitcoinz@6b8ad2f for details.

Before:

# time src/bitcoinz-cli z_getbalance "zs1xxxxxxxxxxxxxxxxxxx"
17437.49990000

real	0m9.669s
user	0m0.006s
sys	0m0.000s

Now:

# time src/bitcoinz-cli z_getbalance "zs1xxxxxxxxxxxxxxxxxxx"
23250.00030244

real	0m1.082s
user	0m0.004s
sys	0m0.004s

Also z_sendmany async operations are faster:

Before: ~ 10 sec
Now: ~ 1 sec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants