Skip to content

Commit

Permalink
kernel: De-globalize script execution cache
Browse files Browse the repository at this point in the history
Move its ownership to the ChainstateManager class.

Next to simplifying usage of the kernel library by no longer requiring
manual setup of the cache prior to using validation code, it also slims
down the amount of memory allocated by BasicTestingSetup.
  • Loading branch information
TheCharlatan committed Jul 4, 2024
1 parent ab14d1d commit 13a3661
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 41 deletions.
1 change: 0 additions & 1 deletion src/bitcoin-chainstate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ int main(int argc, char* argv[])
// performing the check with the signature cache.
kernel::ValidationCacheSizes validation_cache_sizes{};
Assert(InitSignatureCache(validation_cache_sizes.signature_cache_bytes));
Assert(InitScriptExecutionCache(validation_cache_sizes.script_execution_cache_bytes));

ValidationSignals validation_signals{std::make_unique<util::ImmediateTaskRunner>()};

Expand Down
1 change: 0 additions & 1 deletion src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1157,7 +1157,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
ValidationCacheSizes validation_cache_sizes{};
ApplyArgsManOptions(args, validation_cache_sizes);
(void)InitSignatureCache(validation_cache_sizes.signature_cache_bytes);
(void)InitScriptExecutionCache(validation_cache_sizes.script_execution_cache_bytes);

assert(!node.scheduler);
node.scheduler = std::make_unique<CScheduler>();
Expand Down
2 changes: 2 additions & 0 deletions src/kernel/chainstatemanager_opts.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include <arith_uint256.h>
#include <dbwrapper.h>
#include <script/sigcache.h>
#include <txdb.h>
#include <uint256.h>
#include <util/time.h>
Expand Down Expand Up @@ -48,6 +49,7 @@ struct ChainstateManagerOpts {
ValidationSignals* signals{nullptr};
//! Number of script check worker threads. Zero means no parallel verification.
int worker_threads_num{0};
size_t script_execution_cache_bytes{DEFAULT_SCRIPT_EXECUTION_CACHE_BYTES};
};

} // namespace kernel
Expand Down
1 change: 0 additions & 1 deletion src/kernel/validation_cache_sizes.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
namespace kernel {
struct ValidationCacheSizes {
size_t signature_cache_bytes{DEFAULT_MAX_SIG_CACHE_BYTES / 2};
size_t script_execution_cache_bytes{DEFAULT_MAX_SIG_CACHE_BYTES / 2};
};
}

Expand Down
9 changes: 9 additions & 0 deletions src/node/chainstatemanager_args.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& args, ChainstateManage
opts.worker_threads_num = std::clamp(script_threads - 1, 0, MAX_SCRIPTCHECK_THREADS);
LogPrintf("Script verification uses %d additional threads\n", opts.worker_threads_num);

if (auto max_size = args.GetIntArg("-maxsigcachesize")) {
// 1. When supplied with a max_size of 0, both the signature cache and
// script execution cache create the minimum possible cache (2
// elements). Therefore, we can use 0 as a floor here.
// 2. Multiply first, divide after to avoid integer truncation.
size_t clamped_size_each = std::max<int64_t>(*max_size, 0) * (1 << 20) / 2;
opts.script_execution_cache_bytes = clamped_size_each;
}

return {};
}
} // namespace node
1 change: 0 additions & 1 deletion src/node/validation_cache_args.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ void ApplyArgsManOptions(const ArgsManager& argsman, ValidationCacheSizes& cache
size_t clamped_size_each = std::max<int64_t>(*max_size, 0) * (1 << 20) / 2;
cache_sizes = {
.signature_cache_bytes = clamped_size_each,
.script_execution_cache_bytes = clamped_size_each,
};
}
}
Expand Down
1 change: 1 addition & 0 deletions src/script/sigcache.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
// systems). Due to how we count cache size, actual memory usage is slightly
// more (~32.25 MiB)
static constexpr size_t DEFAULT_MAX_SIG_CACHE_BYTES{32 << 20};
static constexpr size_t DEFAULT_SCRIPT_EXECUTION_CACHE_BYTES{DEFAULT_MAX_SIG_CACHE_BYTES / 2};

class CPubKey;

Expand Down
36 changes: 19 additions & 17 deletions src/test/txvalidationcache_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <consensus/validation.h>
#include <key.h>
#include <random.h>
#include <script/sigcache.h>
#include <script/sign.h>
#include <script/signingprovider.h>
#include <test/util/setup_common.h>
Expand All @@ -22,6 +23,7 @@ struct Dersig100Setup : public TestChain100Setup {
bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
const CCoinsViewCache& inputs, unsigned int flags, bool cacheSigStore,
bool cacheFullScriptStore, PrecomputedTransactionData& txdata,
ValidationCache& validation_cache,
std::vector<CScriptCheck>* pvChecks) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

BOOST_AUTO_TEST_SUITE(txvalidationcache_tests)
Expand Down Expand Up @@ -118,7 +120,7 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, Dersig100Setup)
// should fail.
// Capture this interaction with the upgraded_nop argument: set it when evaluating
// any script flag that is implemented as an upgraded NOP code.
static void ValidateCheckInputsForAllFlags(const CTransaction &tx, uint32_t failing_flags, bool add_to_cache, CCoinsViewCache& active_coins_tip) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
static void ValidateCheckInputsForAllFlags(const CTransaction &tx, uint32_t failing_flags, bool add_to_cache, CCoinsViewCache& active_coins_tip, ValidationCache& validation_cache) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
{
PrecomputedTransactionData txdata;

Expand All @@ -140,7 +142,7 @@ static void ValidateCheckInputsForAllFlags(const CTransaction &tx, uint32_t fail
// WITNESS requires P2SH
test_flags |= SCRIPT_VERIFY_P2SH;
}
bool ret = CheckInputScripts(tx, state, &active_coins_tip, test_flags, true, add_to_cache, txdata, nullptr);
bool ret = CheckInputScripts(tx, state, &active_coins_tip, test_flags, true, add_to_cache, txdata, validation_cache, nullptr);
// CheckInputScripts should succeed iff test_flags doesn't intersect with
// failing_flags
bool expected_return_value = !(test_flags & failing_flags);
Expand All @@ -150,13 +152,13 @@ static void ValidateCheckInputsForAllFlags(const CTransaction &tx, uint32_t fail
if (ret && add_to_cache) {
// Check that we get a cache hit if the tx was valid
std::vector<CScriptCheck> scriptchecks;
BOOST_CHECK(CheckInputScripts(tx, state, &active_coins_tip, test_flags, true, add_to_cache, txdata, &scriptchecks));
BOOST_CHECK(CheckInputScripts(tx, state, &active_coins_tip, test_flags, true, add_to_cache, txdata, validation_cache, &scriptchecks));
BOOST_CHECK(scriptchecks.empty());
} else {
// Check that we get script executions to check, if the transaction
// was invalid, or we didn't add to cache.
std::vector<CScriptCheck> scriptchecks;
BOOST_CHECK(CheckInputScripts(tx, state, &active_coins_tip, test_flags, true, add_to_cache, txdata, &scriptchecks));
BOOST_CHECK(CheckInputScripts(tx, state, &active_coins_tip, test_flags, true, add_to_cache, txdata, validation_cache, &scriptchecks));
BOOST_CHECK_EQUAL(scriptchecks.size(), tx.vin.size());
}
}
Expand Down Expand Up @@ -214,20 +216,20 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
TxValidationState state;
PrecomputedTransactionData ptd_spend_tx;

BOOST_CHECK(!CheckInputScripts(CTransaction(spend_tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, nullptr));
BOOST_CHECK(!CheckInputScripts(CTransaction(spend_tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, m_node.chainman->m_validation_cache, nullptr));

// If we call again asking for scriptchecks (as happens in
// ConnectBlock), we should add a script check object for this -- we're
// not caching invalidity (if that changes, delete this test case).
std::vector<CScriptCheck> scriptchecks;
BOOST_CHECK(CheckInputScripts(CTransaction(spend_tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, &scriptchecks));
BOOST_CHECK(CheckInputScripts(CTransaction(spend_tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG, true, true, ptd_spend_tx, m_node.chainman->m_validation_cache, &scriptchecks));
BOOST_CHECK_EQUAL(scriptchecks.size(), 1U);

// Test that CheckInputScripts returns true iff DERSIG-enforcing flags are
// not present. Don't add these checks to the cache, so that we can
// test later that block validation works fine in the absence of cached
// successes.
ValidateCheckInputsForAllFlags(CTransaction(spend_tx), SCRIPT_VERIFY_DERSIG | SCRIPT_VERIFY_LOW_S | SCRIPT_VERIFY_STRICTENC, false, m_node.chainman->ActiveChainstate().CoinsTip());
ValidateCheckInputsForAllFlags(CTransaction(spend_tx), SCRIPT_VERIFY_DERSIG | SCRIPT_VERIFY_LOW_S | SCRIPT_VERIFY_STRICTENC, false, m_node.chainman->ActiveChainstate().CoinsTip(), m_node.chainman->m_validation_cache);
}

// And if we produce a block with this tx, it should be valid (DERSIG not
Expand All @@ -253,7 +255,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
std::vector<unsigned char> vchSig2(p2pk_scriptPubKey.begin(), p2pk_scriptPubKey.end());
invalid_under_p2sh_tx.vin[0].scriptSig << vchSig2;

ValidateCheckInputsForAllFlags(CTransaction(invalid_under_p2sh_tx), SCRIPT_VERIFY_P2SH, true, m_node.chainman->ActiveChainstate().CoinsTip());
ValidateCheckInputsForAllFlags(CTransaction(invalid_under_p2sh_tx), SCRIPT_VERIFY_P2SH, true, m_node.chainman->ActiveChainstate().CoinsTip(), m_node.chainman->m_validation_cache);
}

// Test CHECKLOCKTIMEVERIFY
Expand All @@ -276,13 +278,13 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
vchSig.push_back((unsigned char)SIGHASH_ALL);
invalid_with_cltv_tx.vin[0].scriptSig = CScript() << vchSig << 101;

ValidateCheckInputsForAllFlags(CTransaction(invalid_with_cltv_tx), SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY, true, m_node.chainman->ActiveChainstate().CoinsTip());
ValidateCheckInputsForAllFlags(CTransaction(invalid_with_cltv_tx), SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY, true, m_node.chainman->ActiveChainstate().CoinsTip(), m_node.chainman->m_validation_cache);

// Make it valid, and check again
invalid_with_cltv_tx.vin[0].scriptSig = CScript() << vchSig << 100;
TxValidationState state;
PrecomputedTransactionData txdata;
BOOST_CHECK(CheckInputScripts(CTransaction(invalid_with_cltv_tx), state, m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY, true, true, txdata, nullptr));
BOOST_CHECK(CheckInputScripts(CTransaction(invalid_with_cltv_tx), state, m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY, true, true, txdata, m_node.chainman->m_validation_cache, nullptr));
}

// TEST CHECKSEQUENCEVERIFY
Expand All @@ -304,13 +306,13 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
vchSig.push_back((unsigned char)SIGHASH_ALL);
invalid_with_csv_tx.vin[0].scriptSig = CScript() << vchSig << 101;

ValidateCheckInputsForAllFlags(CTransaction(invalid_with_csv_tx), SCRIPT_VERIFY_CHECKSEQUENCEVERIFY, true, m_node.chainman->ActiveChainstate().CoinsTip());
ValidateCheckInputsForAllFlags(CTransaction(invalid_with_csv_tx), SCRIPT_VERIFY_CHECKSEQUENCEVERIFY, true, m_node.chainman->ActiveChainstate().CoinsTip(), m_node.chainman->m_validation_cache);

// Make it valid, and check again
invalid_with_csv_tx.vin[0].scriptSig = CScript() << vchSig << 100;
TxValidationState state;
PrecomputedTransactionData txdata;
BOOST_CHECK(CheckInputScripts(CTransaction(invalid_with_csv_tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_CHECKSEQUENCEVERIFY, true, true, txdata, nullptr));
BOOST_CHECK(CheckInputScripts(CTransaction(invalid_with_csv_tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_CHECKSEQUENCEVERIFY, true, true, txdata, m_node.chainman->m_validation_cache, nullptr));
}

// TODO: add tests for remaining script flags
Expand All @@ -333,11 +335,11 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
UpdateInput(valid_with_witness_tx.vin[0], sigdata);

// This should be valid under all script flags.
ValidateCheckInputsForAllFlags(CTransaction(valid_with_witness_tx), 0, true, m_node.chainman->ActiveChainstate().CoinsTip());
ValidateCheckInputsForAllFlags(CTransaction(valid_with_witness_tx), 0, true, m_node.chainman->ActiveChainstate().CoinsTip(), m_node.chainman->m_validation_cache);

// Remove the witness, and check that it is now invalid.
valid_with_witness_tx.vin[0].scriptWitness.SetNull();
ValidateCheckInputsForAllFlags(CTransaction(valid_with_witness_tx), SCRIPT_VERIFY_WITNESS, true, m_node.chainman->ActiveChainstate().CoinsTip());
ValidateCheckInputsForAllFlags(CTransaction(valid_with_witness_tx), SCRIPT_VERIFY_WITNESS, true, m_node.chainman->ActiveChainstate().CoinsTip(), m_node.chainman->m_validation_cache);
}

{
Expand All @@ -362,7 +364,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
}

// This should be valid under all script flags
ValidateCheckInputsForAllFlags(CTransaction(tx), 0, true, m_node.chainman->ActiveChainstate().CoinsTip());
ValidateCheckInputsForAllFlags(CTransaction(tx), 0, true, m_node.chainman->ActiveChainstate().CoinsTip(), m_node.chainman->m_validation_cache);

// Check that if the second input is invalid, but the first input is
// valid, the transaction is not cached.
Expand All @@ -372,12 +374,12 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
TxValidationState state;
PrecomputedTransactionData txdata;
// This transaction is now invalid under segwit, because of the second input.
BOOST_CHECK(!CheckInputScripts(CTransaction(tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true, true, txdata, nullptr));
BOOST_CHECK(!CheckInputScripts(CTransaction(tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true, true, txdata, m_node.chainman->m_validation_cache, nullptr));

std::vector<CScriptCheck> scriptchecks;
// Make sure this transaction was not cached (ie because the first
// input was valid)
BOOST_CHECK(CheckInputScripts(CTransaction(tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true, true, txdata, &scriptchecks));
BOOST_CHECK(CheckInputScripts(CTransaction(tx), state, &m_node.chainman->ActiveChainstate().CoinsTip(), SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, true, true, txdata, m_node.chainman->m_validation_cache, &scriptchecks));
// Should get 2 script checks back -- caching is on a whole-transaction basis.
BOOST_CHECK_EQUAL(scriptchecks.size(), 2U);
}
Expand Down
1 change: 0 additions & 1 deletion src/test/util/setup_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vecto
ValidationCacheSizes validation_cache_sizes{};
ApplyArgsManOptions(*m_node.args, validation_cache_sizes);
Assert(InitSignatureCache(validation_cache_sizes.signature_cache_bytes));
Assert(InitScriptExecutionCache(validation_cache_sizes.script_execution_cache_bytes));

m_node.chain = interfaces::MakeChain(m_node);
static bool noui_connected = false;
Expand Down
Loading

0 comments on commit 13a3661

Please sign in to comment.