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

[Zerocoin][Validation] Cache checksum heights #938

Merged
merged 1 commit into from
May 3, 2021

Conversation

Zannick
Copy link
Collaborator

@Zannick Zannick commented Apr 30, 2021

Problem

While mining SHA256D, the wallet periodically gets stuck, failing to stay caught up with the chain, even with #915. (At the same time, the mining significantly slows down.)

Root Cause

Lock contention; the cs_main lock is held throughout critical parts of block template creation, done independently per mining thread. The intermittent stuckness appears correlated with large numbers of txins during zerocoin spend validation. This requires determining the height of an accumulator hash, which is done by searching through all the blocks in the chain starting from genesis, for every txin. My debug logs (with -debug=bench) show very consistent examples like - Verify 646 txins: 17392.00ms (26.923ms/txin).

Solution

Since the height of a particular hash-denomination pair is not going to change, we can cache it. One mining thread will still have to find the height to begin with, but all the other threads will not need to. (Also, hashes may persist to be reused in later blocks; there are likely multiple coins of the same denom with the same accumulator hash.)

Adds a SimpleLRUCache template based on 7c9e97a and uses it to store checksum heights. I chose a somewhat arbitrary size for it that should be large enough for significant spends.

This results in a significant speedup of zerocoin spend validation (alternately got - Verify 101 txins: 1.00ms (0.010ms/txin) , - Verify 101 txins: 0.00ms (0.000ms/txin) on one block). Vastly improves #862 alongside #915--I can confirm it still gets stuck without #915.

Bounty PR

#862

Bounty Payment Address

sv1qqpswvjy7s9yrpcmrt3fu0kd8rutrdlq675ntyjxjzn09f965z9dutqpqgg85esvg8mhmyka5kq5vae0qnuw4428vs9d2gu4nz643jv5a72wkqqq73mnxr

Testing

testnet: built with -with-sanitizers=thread -O0 -g and run with 3/4 threads mining sha256d, and spending tons of 10s at once to stress-test this section.
mainnet: running 12 threads sha256d, built with #915 and -O3 and

diff --git a/src/validation.cpp b/src/validation.cpp
index f92427109..a2a2c9ef5 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp              
@@ -2863,0 +2895,2 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl
         return state.DoS(100, error("%s: CheckQueue failed", __func__), REJECT_INVALID, "block-validation-failed");

     int64_t nTime4 = GetTimeMicros(); nTimeVerify += nTime4 - nTime2;
+    if (nInputs > 20)
+        LogPrintf("    - Verify %u txins: %.2fms (%.3fms/txin) [%.2fs (%.2fms/blk)]\n", nInputs - 1, MILLI * (nTime4 - nTime2), nInputs <= 1 ? 0 : MILLI * (nTime4 - nTime2) / (nInputs-1), nTimeVerify * MICRO, nTimeVerify * MILLI / nBlocksTotal);
     LogPrint(BCLog::BENCH, "    - Verify %u txins: %.2fms (%.3fms/txin) [%.2fs (%.2fms/blk)]\n", nInputs - 1, MILLI * (nTime4 - nTime2), nInputs <= 1 ? 0 : MILLI * (nTime4 - nTime2) / (nInputs-1), nTimeVerify * MICRO, nTimeVerify * MILLI / nBlocksTotal); 

for simple performance monitoring.

@codeofalltrades codeofalltrades added Coin Type: Zerocoin Specifically related to zerocoin Component: Consensus Part of the core cryptocurrency consensus protocol Component: Miner Both PoW and PoS block creation Tag: Waiting For Code Review Waiting for code review from a core developer labels May 1, 2021
Validating zerocoin spends requires determining the height of an
accumulator hash, which is done by searching through all the blocks
in the chain starting from genesis, for every txin.

Since the height of a particular hash-denomination pair is not going
to change, we can cache it. This is most useful for in-wallet block
template generation, since each mining thread performs these
validations separately (while holding cs_main).

Adds a SimpleLRUCache template based on 7c9e97a and uses it to
store checksum heights. I chose a particular size for it that should
be large enough for significant spends.

This results in a roughly 30+x speedup of zerocoin spend validation
(26-30ms/txin -> 0.4-0.9ms/txin). Vastly improves Veil-Project#862 alongside Veil-Project#915.
Copy link
Collaborator

@CaveSpectre11 CaveSpectre11 left a comment

Choose a reason for hiding this comment

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

awesome job!

ACK 1235e1c

Copy link
Collaborator

@codeofalltrades codeofalltrades left a comment

Choose a reason for hiding this comment

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

ACK 1235e1c
Mining hard for almost 24 hours, never got stuck.
Testing on Win 10 and Ubuntu 18.04.

@codeofalltrades codeofalltrades added Code Review: Passed and removed Tag: Waiting For Code Review Waiting for code review from a core developer labels May 3, 2021
@codeofalltrades codeofalltrades merged commit 97b1508 into Veil-Project:master May 3, 2021
@Zannick Zannick deleted the lru branch May 18, 2021 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Review: Passed Coin Type: Zerocoin Specifically related to zerocoin Component: Consensus Part of the core cryptocurrency consensus protocol Component: Miner Both PoW and PoS block creation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants