Skip to content

validation: keep script cache enabled when RDTS deployment inactive#323

Open
kwsantiago wants to merge 1 commit into
bitcoinknots:29.x-knotsfrom
privkeyio:fix-rdts-script-cache-inactive
Open

validation: keep script cache enabled when RDTS deployment inactive#323
kwsantiago wants to merge 1 commit into
bitcoinknots:29.x-knotsfrom
privkeyio:fix-rdts-script-cache-inactive

Conversation

@kwsantiago

Copy link
Copy Markdown

In Chainstate::ConnectBlock, when the REDUCED_DATA deployment is inactive at the block being connected, GetBlockScriptFlags does not set REDUCED_DATA_MANDATORY_VERIFY_FLAGS and reduced_data_start_height is set to std::numeric_limits<int>::max().

The per-input loop then evaluates:

if (prevheights[j] < reduced_data_start_height) {   // < INT_MAX, true for every input
    flags_per_input.resize(tx.vin.size(), flags);
    flags_per_input[j] = flags & ~REDUCED_DATA_MANDATORY_VERIFY_FLAGS;   // == flags, bits already absent
}

So flags_per_input gets populated for every input even though each element equals flags. The script-verification results are identical (there is no consensus difference), but CheckInputScripts only writes to the script-execution cache when flags_per_input is empty:

if (cacheFullScriptStore && (!pvChecks) && flags_per_input.empty()) {
    validation_cache.m_script_execution_cache.insert(hashCacheEntry);
}

The net effect is that the script-execution cache is silently bypassed for every block connected while REDUCED_DATA is inactive, i.e. the entire pre-activation history on every node syncing from genesis. This is a pure performance regression with no effect on validation results.

This gates the per-input branch on reduced_data_active so flags_per_input stays empty when the deployment is inactive, restoring the cache. The DeploymentActiveAt result is also computed once instead of twice.

Behavior when REDUCED_DATA is active is unchanged: inputs spending pre-activation coins still receive per-input flags (and correctly forgo caching for those transactions).

Fix suggested by @chrisguida in review of #238.

Verification: builds cleanly; feature_rdts.py passes.

@pdath

pdath commented Jul 11, 2026

Copy link
Copy Markdown

ctACK ff900da

The complexity of this change exceeds my knowledge, so I can not do a human code review.
I reviewed the code with Code Rabbit and found no issues.

I tested using the following commands on Ubuntu 24.04 running on ARMv8-A (64-bit):

git clone https://github.com/bitcoinknots/bitcoin.git pr323
cd pr323
git fetch origin pull/323/head:pr323
git switch pr323

cmake -B build
-DRDTS_CONSENT=IMPLICIT
-DBUILD_GUI=OFF
-DCMAKE_BUILD_TYPE=Release
nice cmake --build build -j4

cd build
ctest -R "validation_tests|mempool_tests" --output-on-failure
python3 test/functional/test_runner.py mempool_reorg.py feature_rdts.py feature_reduced_data_utxo_height.py

@kwsantiago

kwsantiago commented Jul 17, 2026

Copy link
Copy Markdown
Author

@chrisguida mind taking a look? Four-line fix, keeps the script cache enabled while the RDTS deployment is inactive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants