MyHush / hush3

Hush: Speak And Transact Freely
https://myhush.org
Other
15 stars 13 forks source link

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

Open leto opened 4 years ago

leto commented 4 years ago

This needs testing. It implements the good idea from https://github.com/zcash/zcash/issues/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 commented 3 months ago

@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 commented 3 months ago

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

See: https://github.com/MarkLTZ/bitcoinz/commit/6b8ad2ff83e906b39d392b4424a7f626895a8b4a 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