SubGeniusFinance/Offerings-to-Cthulhu

Watch

Fork 0

miner: CreateNewBlock errors on mempool tx that fails stricter post-fork script flags #39

New issue

Closed

opened 2026-06-16 02:18:55 +00:00 by dobbscoin · 1 comment

dobbscoin commented

2026-06-16 02:18:55 +00:00

Goal

Fix a miner-side flaw uncovered while writing the qa/rpc-tests/cltv_boundary.py adversarial test for #34. Surfaces only post-fork (h ≥ 1,055,555) when block-validation flags become stricter than mempool flags.

Problem

Sequence:

Mempool runs script evaluation with P2SH | STRICTENC only.
Post-fork (after h=1,055,555), block validation runs with P2SH | STRICTENC | DERSIG | CHECKLOCKTIMEVERIFY (issues #33 + #34).
A tx can be perfectly valid against mempool's flags but invalid against block-validation's flags. The two valid cases that exist today:
- A P2SH-CLTV spend with tx.nLockTime < required_locktime — mempool's script eval treats OP_NOP2 as a no-op and accepts; block-validation evaluates CLTV and rejects.
- A P2SH-CLTV spend with nSequence == 0xFFFFFFFF — same shape (mempool: no-op pass; block-validation: CLTV reject).
Once such a tx is in the mempool, calling setgenerate true 1 (or anything that runs the miner) causes the daemon to error or exit instead of gracefully excluding the offending tx from the next block.

Reproduction: see qa/rpc-tests/cltv_boundary.py (Phase 3, currently deferred with a comment). Briefly:

# regtest, HARDFORK_CLTV_REGTEST_OFF=110
# Mine past h=110; fund a P2SH-CLTV redeem; build a spending tx with
# nLockTime < required_locktime, sequence=0; sendrawtransaction (accepted);
# setgenerate true 1 (daemon goes away).

Observed in regtest with the freshly built feat/v2.0.x-rc-bipsoft binary.

Impact

Pre-fork mainnet: no impact. Block-validation flags match mempool's, so the gap doesn't exist.
Post-fork mainnet (h ≥ 1,055,555): any miner running unpatched binary can be crashed by a single malformed tx accepted to its mempool. DoS surface against mining nodes.
- The OFFSIG window has already closed at h=1,050,666, so mining is permissionless again. Every mining operator is exposed.
- The Conclave's own pool wallet is exposed too.

Likely cause

CreateNewBlock (or its CCheckQueue-driven script-check path) almost certainly treats a script-check failure during block assembly as an unexpected state — Bitcoin Core's invariant historically was "if it's in the mempool, it passes block-validation script checks." Once mempool and block-validation flags diverge, that invariant breaks. The miner needs to gracefully skip the tx instead of erroring/asserting.

Investigation should look at:

src/miner.cpp — CreateNewBlock, the loop that pulls txs from mempool.mapTx and runs CheckInputs
src/main.cpp — CheckInputs and its CScriptCheck callers; whether failure paths trip an assertion or DoS-100 the caller
src/checkqueue.h — CCheckQueueControl destruction semantics if a check returns false mid-flight

Proposed fix shape

Detect failed CheckInputs for a mempool tx during CreateNewBlock
Log a warning (AdvanceTo: tx %s failed block-flag script check, excluding)
Drop the tx from mempool (it's no longer valid) and skip
Continue assembling the block with remaining txs

~30-50 LoC, plus mempool cleanup logic.

Test for the fix

The deferred Phase 3 in qa/rpc-tests/cltv_boundary.py is the regression for this. Once fixed:

Send a CLTV-violating P2SH spend; mempool accepts.
setgenerate true 1 succeeds.
The new block does NOT contain the bad tx.
Daemon stays up.

Same test pattern applies to BIP66 DERSIG (#33) once the block-construction primitives land — a malformed-sig tx that bypasses STRICTENC won't exist via sendrawtransaction (mempool catches it), but if anyone constructs and submits a block via submitblock with such a tx, block-validation must reject without crashing.

Should it ship in v2.0.x-rc-bipsoft?

Yes — recommend bundling. The bug is introduced by the bundle's stricter post-fork flags. Shipping the bundle without the fix means the bundle's first day of mainnet activation comes with a known DoS surface. Better to fix-and-ship as one release than ship-then-hotfix.

References

qa/rpc-tests/cltv_boundary.py — reproduction in Phase 3 (deferred)
Issue #34 — BIP65 OP_CHECKLOCKTIMEVERIFY (introduces the divergent flag)
Issue #33 — BIP66 strict-DER (same divergence pattern; mempool already enforces STRICTENC so #33 doesn't trip this via sendrawtransaction, but a hand-crafted block could)
Activation height for the divergence: h=1,055,555 (mainnet)

## Goal Fix a miner-side flaw uncovered while writing the `qa/rpc-tests/cltv_boundary.py` adversarial test for #34. **Surfaces only post-fork (h ≥ 1,055,555) when block-validation flags become stricter than mempool flags.** ## Problem Sequence: 1. Mempool runs script evaluation with `P2SH | STRICTENC` only. 2. Post-fork (after h=1,055,555), block validation runs with `P2SH | STRICTENC | DERSIG | CHECKLOCKTIMEVERIFY` (issues #33 + #34). 3. A tx can be perfectly valid against mempool's flags but invalid against block-validation's flags. The two valid cases that exist today: - A P2SH-CLTV spend with `tx.nLockTime < required_locktime` — mempool's script eval treats `OP_NOP2` as a no-op and accepts; block-validation evaluates CLTV and rejects. - A P2SH-CLTV spend with `nSequence == 0xFFFFFFFF` — same shape (mempool: no-op pass; block-validation: CLTV reject). 4. Once such a tx is in the mempool, calling `setgenerate true 1` (or anything that runs the miner) causes the daemon to error or exit instead of gracefully excluding the offending tx from the next block. Reproduction: see `qa/rpc-tests/cltv_boundary.py` (Phase 3, currently deferred with a comment). Briefly: ``` # regtest, HARDFORK_CLTV_REGTEST_OFF=110 # Mine past h=110; fund a P2SH-CLTV redeem; build a spending tx with # nLockTime < required_locktime, sequence=0; sendrawtransaction (accepted); # setgenerate true 1 (daemon goes away). ``` Observed in regtest with the freshly built `feat/v2.0.x-rc-bipsoft` binary. ## Impact - **Pre-fork mainnet:** no impact. Block-validation flags match mempool's, so the gap doesn't exist. - **Post-fork mainnet (h ≥ 1,055,555):** any miner running unpatched binary can be crashed by a single malformed tx accepted to its mempool. **DoS surface against mining nodes.** - The OFFSIG window has already closed at h=1,050,666, so mining is permissionless again. Every mining operator is exposed. - The Conclave's own pool wallet is exposed too. ## Likely cause `CreateNewBlock` (or its `CCheckQueue`-driven script-check path) almost certainly treats a script-check failure during block assembly as an unexpected state — Bitcoin Core's invariant historically was "if it's in the mempool, it passes block-validation script checks." Once mempool and block-validation flags diverge, that invariant breaks. The miner needs to gracefully skip the tx instead of erroring/asserting. Investigation should look at: - `src/miner.cpp` — `CreateNewBlock`, the loop that pulls txs from `mempool.mapTx` and runs `CheckInputs` - `src/main.cpp` — `CheckInputs` and its `CScriptCheck` callers; whether failure paths trip an assertion or DoS-100 the caller - `src/checkqueue.h` — `CCheckQueueControl` destruction semantics if a check returns false mid-flight ## Proposed fix shape - Detect failed `CheckInputs` for a mempool tx during `CreateNewBlock` - Log a warning (`AdvanceTo: tx %s failed block-flag script check, excluding`) - Drop the tx from `mempool` (it's no longer valid) and skip - Continue assembling the block with remaining txs ~30-50 LoC, plus mempool cleanup logic. ## Test for the fix The deferred Phase 3 in `qa/rpc-tests/cltv_boundary.py` is the regression for this. Once fixed: 1. Send a CLTV-violating P2SH spend; mempool accepts. 2. `setgenerate true 1` succeeds. 3. The new block does NOT contain the bad tx. 4. Daemon stays up. Same test pattern applies to BIP66 DERSIG (#33) once the block-construction primitives land — a malformed-sig tx that bypasses STRICTENC won't exist via sendrawtransaction (mempool catches it), but if anyone constructs and submits a block via `submitblock` with such a tx, block-validation must reject without crashing. ## Should it ship in v2.0.x-rc-bipsoft? **Yes — recommend bundling.** The bug is *introduced by* the bundle's stricter post-fork flags. Shipping the bundle without the fix means the bundle's first day of mainnet activation comes with a known DoS surface. Better to fix-and-ship as one release than ship-then-hotfix. ## References - `qa/rpc-tests/cltv_boundary.py` — reproduction in Phase 3 (deferred) - Issue #34 — BIP65 OP_CHECKLOCKTIMEVERIFY (introduces the divergent flag) - Issue #33 — BIP66 strict-DER (same divergence pattern; mempool already enforces STRICTENC so #33 doesn't trip this *via sendrawtransaction*, but a hand-crafted block could) - Activation height for the divergence: h=1,055,555 (mainnet)

dobbscoin commented

2026-06-16 03:36:24 +00:00

Fixed on feat/v2.0.x-rc-bipsoft as commit 1f63d5f.

Approach: align the miner's pre-screen flags in CreateNewBlock with the flags ConnectBlock will use at the same height. Pre-fork no behavior change; post-fork the miner now gracefully continues past a mempool tx that fails block-validation script flags, instead of adding it to the candidate block and then crashing the final ConnectBlock test-pass.

Regression test: qa/rpc-tests/cltv_boundary.py Phase 3 (the original repro from the issue body) now PASSes — bad-CLTV tx accepted to mempool → setgenerate true 1 → miner skips the tx → block lands cleanly → daemon stays up → bad tx confirmed NOT in any block.

Not yet on main — bundled with the rest of feat/v2.0.x-rc-bipsoft for merge at freeze-end (post h=1,050,667). Closing as "fix shipped on the working branch."

Fixed on `feat/v2.0.x-rc-bipsoft` as commit [`1f63d5f`](https://github.com/SubGeniusFinance/Offerings-to-Cthulhu/commit/1f63d5f). Approach: align the miner's pre-screen flags in `CreateNewBlock` with the flags `ConnectBlock` will use at the same height. Pre-fork no behavior change; post-fork the miner now gracefully `continue`s past a mempool tx that fails block-validation script flags, instead of adding it to the candidate block and then crashing the final `ConnectBlock` test-pass. Regression test: `qa/rpc-tests/cltv_boundary.py` Phase 3 (the original repro from the issue body) now PASSes — bad-CLTV tx accepted to mempool → `setgenerate true 1` → miner skips the tx → block lands cleanly → daemon stays up → bad tx confirmed NOT in any block. Not yet on `main` — bundled with the rest of `feat/v2.0.x-rc-bipsoft` for merge at freeze-end (post h=1,050,667). Closing as "fix shipped on the working branch."

dobbscoin closed this issue

2026-06-16 03:36:25 +00:00

No labels

enhancement

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

SubGeniusFinance/Offerings-to-Cthulhu#39

No description provided.

Rows
Columns