Make CCoinsViewCache::Cursor() return latest data #9306

pull ryanofsky wants to merge 1 commits into bitcoin:master from ryanofsky:pr/coins-cursor changing 3 files +160 −3

ryanofsky commented at 3:10 PM on December 9, 2016: contributor

Change CCoinsViewCache::Cursor() method to return a cursor yielding the latest CCoins entries, instead of just previous entries prior to the last cache flush.

The CCoinsViewCache::Cursor method is not currently used. This change just enables new features that rely on scanning the UXTO set to work correctly (for example #9152, which adds a sweepprivkeys RPC, and #9137, which improves handling of imported keys for nodes with pruning enabled.)
sipa commented at 12:38 AM on December 10, 2016: member

Concept ACK
fanquake added the label UTXO Db and Indexes on Dec 14, 2016
laanwj added this to the milestone 0.15.0 on Feb 6, 2017
ryanofsky force-pushed on Jun 2, 2017
laanwj commented at 2:10 PM on June 6, 2017: member

The CCoinsViewCache::Cursor method is not currently used

GetUTXOStats uses it. Currently it makes use of the fact that it iterates over the current database instead of the in-memory data. Leveldb can do an iteration in the background, as it makes a snapshot. This change means that it can no longer run in the background, and needs the appropriate lock on pCoinsTip to prevent concurrent changes.
sipa commented at 5:26 PM on June 6, 2017: member

My hope is that at some point we can switch to a rolling UTXO hash (see my MuHash PR, and mailinglist thread), so gettxoutsetinfo does not need to interate over the set anymore but compute everything using continuously updated stats.
sipa commented at 7:15 PM on June 6, 2017: member

It's probably better to not merge this until we can rid of the necessity to iterate over the entire set in gettxoutsetinfo. And when we do, maybe this PR isn't needed anymore?
ryanofsky commented at 5:30 PM on June 7, 2017: contributor

Probably this PR isn't needed unless #9152 or #9137 or something similar will be added. But I do think the fact that CCCoinsViewCache::Cursor() returns a cursor that doesn't actually iterate over the latest data in the view is confusing and could lead to bugs. Opened #10550 for a more direct solution to that problem.
laanwj cross-referenced this on Jun 8, 2017 from issue Don't return stale data from CCoinsViewCache::Cursor() by ryanofsky

Make CCoinsViewCache::Cursor() return latest data

Change CCoinsViewCache::Cursor() method to return a cursor yielding the latest
CCoins entries, instead of just previous entries prior to the last cache flush.

The CCoinsViewCache::Cursor method is not currently used. This change just
enables new features that rely on scanning the UXTO set to work correctly (for
example https://github.com/bitcoin/bitcoin/pull/9152, which adds a
sweepprivkeys RPC, and https://github.com/bitcoin/bitcoin/pull/9137, which
improves handling of imported keys for nodes with pruning enabled.)

095c2e7e78

ryanofsky force-pushed on Jun 12, 2017
sipa commented at 6:40 PM on June 12, 2017: member

I think #10550 is sufficient for now, and we won't need this until there is an actual use for iterating the UTXO set in a non-well-defined order?
ryanofsky commented at 7:00 PM on June 12, 2017: contributor

All right, I'm not really working on #9137 anyway, will close.
ryanofsky closed this on Jun 12, 2017
ryanofsky commented at 11:20 AM on December 15, 2017: contributor

@jonasschnelli, do you think this PR might be useful for your "scantxoutset" RPC (https://botbot.me/freenode/bitcoin-core-dev/msg/94684446/). It makes CCoinsViewCache::Cursor() return the latest data instead of stale data.
ryanofsky commented at 8:26 AM on December 21, 2017: contributor

@jonasschnelli, do you think this PR might be useful for your "scantxoutset" RPC

Followup: Code is currently calling FlushStateToDisk so this should not be needed (https://botbot.me/freenode/bitcoin-core-dev/msg/94920590/)
ryanofsky commented at 6:18 PM on January 16, 2018: contributor

Followup: Flush was removed according to https://botbot.me/freenode/bitcoin-core-dev/msg/95804751/ so this PR might be useful again. Implementation in #12196 does include a flush though, currently.
jonasschnelli commented at 6:28 AM on January 17, 2018: contributor

This is definitively useful (could lead to removing the state flush in #12196). @ryanofsky: Would be great if you'd like to reopen this (need probably rebase).
MarcoFalke commented at 1:15 PM on April 28, 2020: member

Should this be marked "Up for grabs"? cc @ryanofsky
MarcoFalke removed this from the milestone 0.15.0 on Apr 28, 2020
ryanofsky commented at 2:39 PM on April 28, 2020: contributor

Should this be marked "Up for grabs"? cc @ryanofsky

I don't know if the PR is up for grabs, more like uses of the PR are up for grabs. My motivation for this PR was let to wallet scan utxo set instead of blocks, and be able to get wallet balance if not full transaction history with a pruned node. Sipa, laanwj, and jonas above and luke in #9152 had other potential uses. The PR itself is simple and probably very easy to rebase. Just doesn't make sense without a use-case
bitcoin locked this on Feb 15, 2022

Contributors

Labels

Linked (view graph)

#9137 WIP: Allow wallet key import RPCs to track TxOut amounts on -prune nodes (on top of #9306)#9152 Wallet/RPC: sweepprivkeys method to scan UTXO set and send to local wallet #10550 Don't return stale data from CCoinsViewCache::Cursor()#12196 Add scantxoutset RPC method