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

add(state): Track spending transaction ids by spent outpoints and revealed nullifiers #8895

Open
wants to merge 30 commits into
base: main
Choose a base branch
from

Conversation

arya2
Copy link
Contributor

@arya2 arya2 commented Sep 27, 2024

Motivation

We want to lookup transaction ids by their transparent inputs and revealed nullifiers.

Closes #8837,
Closes #8838.
Closes #8922.

Solution

  • Adds a new tx_loc_by_spent_out_loc column family
  • Updates revealed nullifier column families to store spending transaction locations as values instead of ()
  • Stores the TransactionLocation of spending transactions by spent OutputLocations and nullifiers when writing blocks to the finalized state
  • Adds the hashes of spending transactions as the values in the spent_utxos field on non-finalized Chains
  • Adds ReadRequest and ReadResponse variants for querying spending tx ids by outpoint with the ReadStateService
  • Adds a spending_transaction_hash() read function used to handle the new ReadRequest
  • Updates snapshots

It may be possible to update the tx_loc_by_transparent_addr_loc column instead, but adding a new one seemed easier.

Related Changes:

  • Updates cancel_receiver to a crossbeam-channel::mpmc and parallelizes the db format upgrade by block

Tests

  • Adds a test that checks the last 500 blocks in the finalized and non-finalized state for spending transaction ids (uses a cached state)
  • Manually tested the db format upgrade
  • Full Mainnet sync test running here

PR Author's Checklist

  • The PR name will make sense to users.
  • The PR provides a CHANGELOG summary.
  • The solution is tested.
  • The documentation is up to date.
  • The PR has a priority label.

PR Reviewer's Checklist

  • The PR Author's checklist is complete.
  • The PR resolves the issue.

@arya2 arya2 added C-enhancement Category: This is an improvement A-state Area: State / database changes P-Medium ⚡ labels Sep 27, 2024
@arya2 arya2 self-assigned this Sep 27, 2024
@arya2 arya2 changed the title add(state): Track spending transaction ids by spent outpoint add(state): Track spending transaction ids by spent outpoints and revealed nullifiers Oct 1, 2024
@arya2 arya2 marked this pull request as ready for review October 7, 2024 18:41
@arya2 arya2 requested a review from a team as a code owner October 7, 2024 18:41
@arya2 arya2 requested review from upbqdn and removed request for a team October 7, 2024 18:41
@arya2 arya2 added the do-not-merge Tells Mergify not to merge this PR label Oct 10, 2024
@arya2
Copy link
Contributor Author

arya2 commented Oct 10, 2024

Added a do-not-merge label so that this won't be published until after #8922 has been implemented to avoid substantially increasing storage requirements for users that won't be using these indexes.

@arya2 arya2 force-pushed the index-tx-loc-by-spent-out-loc branch from fd2bcca to f6faf42 Compare October 21, 2024 20:05
@arya2
Copy link
Contributor Author

arya2 commented Oct 25, 2024

@mpguerra It looks like it's actually not using much storage space, I was looking at the db metrics printed at startup which were about double the expected storage requirements prior to the change, but the total size of the state cache directory is about the same as it was before, so I think the db metrics are overestimating the total db size.

I checked the number of keys by column family, and by height 2M on Mainnet, it's ~10M transparent outputs and ~150M nullifiers in total, not all of which are spent. It's 10B per spent transparent output and 5 bytes per nullifier, so it should be, at most, ~1GB of additional storage requirements by block 2M. I'll update this comment with the number of nullifiers and transparent outputs at the network chain tip once my node finishes syncing, but it's looking like hiding this behind a feature may have been unnecessary.

Having the indexes behind a feature still seems nice to have, but there's also unnecessary complexity to be reviewed and maintained around adding/deleting the indexes. Should we keep them behind a feature in this PR or remove the feature?

Update:

At the current network chain tip, it's about 6.2GB of extra data (5.5gb + 140M * 5B), also it's 14B per spent transparent output, not 10B (I had forgot about the output index).

6.2GB doesn't seem excessive, but we could use the feature later if/when caching blocks in their compact format.

Relevant Column Family Sizes
sprout_nullifiers (Disk: 146.7 MB, Memory: 9.4 MB, num_keys: Some(1663236)) 
sapling_nullifiers (Disk: 230 MB, Memory: 4.2 MB, num_keys: Some(3068534)) 
orchard_nullifiers (Disk: 6.3 GB, Memory: 55.1 MB, num_keys: Some(134798732)) 
tx_loc_by_spent_out_loc (Disk: 5.5 GB, Memory: 6.3 MB, num_keys: Some(316786532)) 

Cargo.lock Show resolved Hide resolved
@arya2
Copy link
Contributor Author

arya2 commented Oct 25, 2024

The scan_starts_where_left test is failing here: https://github.com/ZcashFoundation/zebra/actions/runs/11513520050/job/32050881919?pr=8895#step:15:615

I thought it was because a column family could be dropped earlier, but it happened again after switching to removing a comprehensive range instead of dropping the column family, so now I'm thinking it's because it's trying to open a secondary db before opening the primary db, where opening the primary db will create any missing column families but opening the secondary panics when one is missing because it lacks write access.

I'll confirm that manually, if that is the case, I think TrustedChainSync should just log a warning saying "Run Zebra first or downgrade your Zebra version". This PR should also either bump the db format version or hide the new column family behind the indexer feature, in the latter case, that would mean Zebra can't clear the column family once it's been populated, so I'm leaning towards bumping the db format version.

Test failure is unrelated, should be fixed now.

@mpguerra mpguerra removed the do-not-merge Tells Mergify not to merge this PR label Nov 18, 2024
@arya2 arya2 force-pushed the index-tx-loc-by-spent-out-loc branch 2 times, most recently from 1ed1c45 to bf37b97 Compare November 19, 2024 15:19
arya2 added 18 commits November 22, 2024 14:51
…aDb instead of DiskDb, checks cancel_receiver before every db operation
- adds the build metadata to the db version file before adding indexes.
- deletes indexes when running without the `indexer` feature
…c when trying to open the db with that column family.
@arya2 arya2 force-pushed the index-tx-loc-by-spent-out-loc branch from bf37b97 to 1ebb1a5 Compare November 22, 2024 19:51
@oxarbitrage oxarbitrage self-requested a review November 27, 2024 16:06
oxarbitrage
oxarbitrage previously approved these changes Nov 29, 2024
Copy link
Contributor

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks really good and should be safe to merge as almost everything is behind a feature flag.

The only thing i found is that the acceptance test has been running for more than 30 mins now. I will like to know your experience with it.

Part of the logs:

2024-11-29T13:19:41.580755Z  WARN {zebrad="1ebb1a5" net="Main"}: zebrad::components::sync::progress: chain updates have stalled, state height has not increased for 29 minutes. Hint: check your network connection, and your computer clock and time zone sync_percent=99.742% current_height=Height(2726357) network_upgrade=Nu5 time_since_last_state_block=29m target_block_spacing=1m 15s max_block_spacing=None is_syncer_stopped=false
2024-11-29T13:20:01.150022Z  INFO {zebrad="1ebb1a5" net="Main"}:peer_cache_updater: zebra_network::config: updated cached peer IP addresses cached_ip_count=46 peer_cache_file="/home/alfredo/.cache/zebra/network/mainnet.peers"
2024-11-29T13:20:19.131087Z  INFO {zebrad="1ebb1a5" net="Main"}:crawl_and_dial{new_peer_interval=30s}:crawl{should_always_dial=false}: zebra_network::peer_set::candidate_set: timeout waiting for peer service readiness or peer responses
2024-11-29T13:20:41.593014Z  WARN {zebrad="1ebb1a5" net="Main"}: zebrad::components::sync::progress: chain updates have stalled, state height has not increased for 30 minutes. Hint: check your network connection, and your computer clock and time zone sync_percent=99.742% current_height=Height(2726357) network_upgrade=Nu5 time_since_last_state_block=30m target_block_spacing=1m 15s max_block_spacing=None is_syncer_stopped=false
2024-11-29T13:20:41.947396Z  INFO {zebrad="1ebb1a5" net="Main"}:sync:try_to_sync: zebrad::components::sync: starting sync, obtaining new tips state_tip=Some(Height(2726357))
2024-11-29T13:21:19.130654Z  INFO {zebrad="1ebb1a5" net="Main"}:crawl_and_dial{new_peer_interval=30s}:crawl{should_always_dial=false}: zebra_network::peer_set::candidate_set: timeout waiting for peer service readiness or peer responses

@oxarbitrage
Copy link
Contributor

Test i ommented out here ended up failing locally for me:

2024-11-29T13:34:16.260577Z  INFO load_tip_height_from_state_directory{network=Mainnet state_path="/media/alfredo/stuff/chain/zebra"}: checking database format produced by new blocks in this instance is valid running_version=26.0.0+indexer
2024-11-29T13:34:27.993693Z  INFO load_tip_height_from_state_directory{network=Mainnet state_path="/media/alfredo/stuff/chain/zebra"}: database format is valid running_version=26.0.0+indexer inital_disk_version=26.0.0+indexer
2024-11-29T13:34:28.287676Z  INFO got finalized tip height from state directory finalized_tip_height=2733291 non_finalized_tip_height=2733390 estimated_finalized_tip_height=2733291

/home/alfredo/zebra/pr8895/zebra/target/release/zebrad Child Stderr:
Thank you for running a mainnet zebrad 2.0.1+48.g1ebb1a5 node!
You're helping to strengthen the network and contributing to a social good :)

2024-11-29T13:34:28.295409Z  WARN start_state_service_with_cache_dir{network=Mainnet}: could not canonicalize "/media/alfredo/stuff/chain/zebra/state/v25/mainnet": No such file or directory (os error 2)
2024-11-29T13:34:28.295420Z  INFO start_state_service_with_cache_dir{network=Mainnet}: trying to open current database format running_version=26.0.0+indexer
2024-11-29T13:34:28.295478Z  INFO start_state_service_with_cache_dir{network=Mainnet}: the open file limit is high enough for Zebra current_limit=1024 min_limit=512 ideal_limit=1024
2024-11-29T13:34:28.800416Z  INFO start_state_service_with_cache_dir{network=Mainnet}: Opened Zebra state cache at /media/alfredo/stuff/chain/zebra/state/v26/mainnet
2024-11-29T13:34:28.800537Z  INFO start_state_service_with_cache_dir{network=Mainnet}: loaded Zebra state cache tip=Some((Height(2733291), block::Hash("0000000000f6ab8efa168e3b3b83fdd22a441379f469cb62204d143d22fe6302")))
2024-11-29T13:34:28.800630Z  INFO start_state_service_with_cache_dir{network=Mainnet}: checking database format produced by a previous zebra instance is current and valid running_version=26.0.0+indexer
2024-11-29T13:34:28.800835Z  INFO start_state_service_with_cache_dir{network=Mainnet}: started checking/adding indexes for spending tx ids
2024-11-29T13:34:28.802132Z  INFO start_state_service_with_cache_dir{network=Mainnet}: starting legacy chain check
2024-11-29T13:34:28.803231Z  INFO start_state_service_with_cache_dir{network=Mainnet}: cached state consensus branch is valid: no legacy chain found
2024-11-29T13:34:28.803289Z  INFO committing blocks to non-finalized state

The application panicked (crashed).
Message:  can call blocking only when running on the multi-threaded runtime
Location: zebra-state/src/service.rs:912

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ SPANTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

   0: zebra_state::service::state
      at zebra-state/src/service.rs:886

Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.
2024-11-29T13:34:28.807754Z  INFO dropping the state: logging database metrics
2024-11-29T13:34:28.807794Z  INFO start_state_service_with_cache_dir{network=Mainnet}: StateService closed the block reset channel. Is Zebra shutting down?
2024-11-29T13:34:28.807823Z  INFO the open file limit is high enough for Zebra current_limit=1024 min_limit=512 ideal_limit=1024
2024-11-29T13:34:28.818175Z  INFO Total Database Disk Size: 270.2 GB
2024-11-29T13:34:28.818187Z  INFO Total Live Data Disk Size: 268.8 GB
2024-11-29T13:34:28.818189Z  INFO Total Database Memory Size: 51.2 KB
2024-11-29T13:34:28.818247Z  INFO checking new blocks were written in current database format running_version=26.0.0+indexer
2024-11-29T13:34:28.818252Z  INFO checking database format produced by new blocks in this instance is valid running_version=26.0.0+indexer
2024-11-29T13:34:40.438333Z  INFO database format is valid running_version=26.0.0+indexer inital_disk_version=26.0.0+indexer
2024-11-29T13:34:40.512623Z  INFO waiting for the block write task to finish
2024-11-29T13:34:40.512747Z  INFO checking new blocks were written in current database format running_version=26.0.0+indexer
2024-11-29T13:34:40.512764Z  INFO checking database format produced by new blocks in this instance is valid running_version=26.0.0+indexer
2024-11-29T13:34:52.479942Z  INFO database format is valid running_version=26.0.0+indexer inital_disk_version=26.0.0+indexer
test has_spending_transaction_ids ... FAILED

failures:

failures:
    has_spending_transaction_ids

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 48 filtered out; finished in 2728.65s

error: test failed, to rerun pass `-p zebrad --test acceptance`

…aded tokio runtime in has_spending_transaction_ids test
@arya2 arya2 added do-not-merge Tells Mergify not to merge this PR and removed do-not-merge Tells Mergify not to merge this PR labels Dec 5, 2024
@arya2
Copy link
Contributor Author

arya2 commented Dec 13, 2024

Test .. ended up failing locally for me

The test checks that a prepared finalized state has the indexes. Documented and updated to use a multi-threaded async runtime in f71c897 (I'm not sure how it was working for me before, the spawn_blocking call was always there).

the acceptance test has been running for more than 30 mins now. I will like to know your experience with it.

It syncs to the network tip, but it takes ~10 minutes with an up-to-date cached state for me, mostly waiting for the "finished initial sync" log. It should add the indexes within 30 minutes (depending on system resources, but the format upgrade is ~10 minutes for me).

It keeps failing with "should have spending transaction hash", I'm not sure why yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-state Area: State / database changes C-enhancement Category: This is an improvement P-Medium ⚡
Projects
None yet
3 participants