Build and test modernization — the section that lowers the cost of every other section #43

Open
opened 2026-09-15 01:51:26 +00:00 by btcbob · 2 comments
Owner

Roadmap section 7 (forum topic 24, msg 70), filed so it is tracked rather than prose.

This is the one whose payoff is not a feature. The stated motivation for following
Core at all is to avoid a large amount of C++ work by a team that does not have
deep C++ fluency. That is exactly what this section buys, and it is why it belongs
before the feature ports rather than after them.

Where we are

  • Autotools only. autogen.sh + configure.ac + Makefile.am; there is no
    CMakeLists.txt. Combined with the static db-4.8.30 requirement from
    doc/build-unix.md:155, a first build is a ritual rather than a command.
  • Tests exist and are not small: 45 unit test files under src/test/ and 23
    functional tests under qa/rpc-tests/. This is a better starting position than
    the roadmap prose implied — the gap is not that tests do not exist, it is that
    nothing runs them automatically.
  • CI is one workflow, .github/workflows/release.yml, and it only builds
    releases. Nothing runs the unit or functional suites on a push.

What Core did

  • The CMake migration (~v29) retires Autotools, and with it most of the
    platform-specific build folklore.
  • libbitcoinkernel extracts the consensus engine as a reusable library, so
    explorers and tooling can reuse validation without shipping a whole node.
  • assumeUTXO (0.26) bootstraps a usable node from a signed UTXO snapshot in
    minutes instead of a full replay.

The highest-leverage item is the least glamorous

Running the suites we already have, on every push, is worth more than any single
port on this board. Good tests are how a small team modernizes safely without
being Core developers: they catch regressions that no amount of careful reading
will. The AuxPoW work in qa/auxpow-smoke/ is a worked example — packaging an
ad-hoc check as a repeatable suite immediately surfaced a case that had been
passing by luck.

The constraint to design around

Where the runner lives is a real question rather than a detail, and it should be
settled before anyone writes a workflow file. Self-hosted, third-party, and
"run it manually before a release" are all defensible; picking one is the work.

Proposal in the roadmap, restated

Make build + test + dependency modernization milestone zero, before any
feature port, because it makes everything after it cheaper and safer. Sequencing
is open; the dependency claim is not — nothing else on this board gets easier
until this lands.

Roadmap section 7 (forum topic 24, msg 70), filed so it is tracked rather than prose. This is the one whose payoff is not a feature. The stated motivation for following Core at all is to avoid a large amount of C++ work by a team that does not have deep C++ fluency. That is exactly what this section buys, and it is why it belongs before the feature ports rather than after them. ### Where we are - **Autotools only.** `autogen.sh` + `configure.ac` + `Makefile.am`; there is no `CMakeLists.txt`. Combined with the static `db-4.8.30` requirement from `doc/build-unix.md:155`, a first build is a ritual rather than a command. - **Tests exist and are not small**: 45 unit test files under `src/test/` and 23 functional tests under `qa/rpc-tests/`. This is a better starting position than the roadmap prose implied — the gap is not that tests do not exist, it is that nothing runs them automatically. - **CI is one workflow**, `.github/workflows/release.yml`, and it only builds releases. Nothing runs the unit or functional suites on a push. ### What Core did - **The CMake migration** (~v29) retires Autotools, and with it most of the platform-specific build folklore. - **libbitcoinkernel** extracts the consensus engine as a reusable library, so explorers and tooling can reuse validation without shipping a whole node. - **assumeUTXO** (0.26) bootstraps a usable node from a signed UTXO snapshot in minutes instead of a full replay. ### The highest-leverage item is the least glamorous Running the suites we already have, on every push, is worth more than any single port on this board. Good tests are how a small team modernizes safely **without** being Core developers: they catch regressions that no amount of careful reading will. The AuxPoW work in `qa/auxpow-smoke/` is a worked example — packaging an ad-hoc check as a repeatable suite immediately surfaced a case that had been passing by luck. ### The constraint to design around Where the runner lives is a real question rather than a detail, and it should be settled before anyone writes a workflow file. Self-hosted, third-party, and "run it manually before a release" are all defensible; picking one is the work. ### Proposal in the roadmap, restated Make build + test + dependency modernization **milestone zero**, before any feature port, because it makes everything after it cheaper and safer. Sequencing is open; the dependency claim is not — nothing else on this board gets easier until this lands.
Author
Owner

The 62 failing tests are diagnosed

test_dobbscoin fails 62 cases on main — 48 in script_tests, 14 in transaction_tests. Verified identical before and after the CLTV merge, so they predate it.

They are all the same test

Every failing script_build case has "without DERSIG" in its name:

BIP66 example 1, without DERSIG
BIP66 example 2, without DERSIG
P2PK with too much R padding but no DERSIG
P2PK with too little R padding but no DERSIG
P2PK with too much S padding but no DERSIG
...

And not one failing transaction_tests case includes DERSIG in its flags — they run under P2SH and P2SH,NULLDUMMY only.

These are tests that deliberately construct non-canonical DER signatures (extra or missing padding) and assert they are accepted when BIP66 is not being enforced. They are testing pre-BIP66 leniency.

Cause

USE_SECP256K1 is not defined, so CPubKey::Verify uses the OpenSSL path. A default build on a current distro links OpenSSL 3, which refuses to parse non-canonical DER. OpenSSL 1.0.x accepted it. The leniency these tests assert is therefore not expressible by the library the node is linked against.

This is not a consensus risk, and here is why

The obvious worry is that a node which cannot parse lenient signatures cannot sync a chain that contains them. It cannot happen here:

  1. Script verification is skipped below the last checkpoint. fScriptChecks = pindex->nHeight >= Checkpoints::GetTotalBlocksEstimate() (main.cpp:1766), and the highest mainnet checkpoint is 1,848,000. On a fresh sync the first 1.85M blocks are never script-verified.
  2. Above that height BIP66 is enforced anyway. Blocks have been version 3 since somewhere between heights 50,000 and 200,000, so IsSuperMajority(3, …) holds continuously and strict DER is already the consensus rule.

So the pre-BIP66 leniency window is unreachable: below the checkpoint nothing is checked, above it strict DER is mandatory. OpenSSL 3's strictness matches the rule the chain enforces. There is no divergence in the current configuration.

This also settles the scope of #35 — it is build hygiene, not a live correctness bug. That is the narrowing already recorded there, now with evidence rather than argument.

What it does cost

62 red tests mean nobody can tell a new regression from existing noise. That is this issue's point, sharpened: the suite is not merely unrun, a quarter of script_tests fails by default, so "the tests pass" is not a statement anyone can currently make.

Two ways to fix, not evaluated here:

  1. Mark the affected cases expected-fail when linked against OpenSSL ≥ 1.1, so a genuine regression stands out.
  2. Adopt libsecp256k1 (#35). Later Bitcoin ships ecdsa_signature_parse_der_lax() specifically to reproduce OpenSSL 1.0's leniency deterministically — but the vendored src/secp256k1/ here predates that helper and exposes the older secp256k1_ecdsa_verify(DER…) API, so whether enabling it restores these cases is untested. Worth checking before treating #35 as the fix for this.
## The 62 failing tests are diagnosed `test_dobbscoin` fails 62 cases on `main` — 48 in `script_tests`, 14 in `transaction_tests`. Verified identical before and after the CLTV merge, so they predate it. ### They are all the same test Every failing `script_build` case has **"without DERSIG"** in its name: ``` BIP66 example 1, without DERSIG BIP66 example 2, without DERSIG P2PK with too much R padding but no DERSIG P2PK with too little R padding but no DERSIG P2PK with too much S padding but no DERSIG ... ``` And **not one** failing `transaction_tests` case includes `DERSIG` in its flags — they run under `P2SH` and `P2SH,NULLDUMMY` only. These are tests that deliberately construct **non-canonical DER signatures** (extra or missing padding) and assert they are *accepted* when BIP66 is not being enforced. They are testing pre-BIP66 leniency. ### Cause `USE_SECP256K1` is not defined, so `CPubKey::Verify` uses the OpenSSL path. A default build on a current distro links OpenSSL 3, which **refuses to parse** non-canonical DER. OpenSSL 1.0.x accepted it. The leniency these tests assert is therefore not expressible by the library the node is linked against. ### This is not a consensus risk, and here is why The obvious worry is that a node which cannot parse lenient signatures cannot sync a chain that contains them. It cannot happen here: 1. **Script verification is skipped below the last checkpoint.** `fScriptChecks = pindex->nHeight >= Checkpoints::GetTotalBlocksEstimate()` (`main.cpp:1766`), and the highest mainnet checkpoint is **1,848,000**. On a fresh sync the first 1.85M blocks are never script-verified. 2. **Above that height BIP66 is enforced anyway.** Blocks have been version 3 since somewhere between heights 50,000 and 200,000, so `IsSuperMajority(3, …)` holds continuously and strict DER is already the consensus rule. So the pre-BIP66 leniency window is unreachable: below the checkpoint nothing is checked, above it strict DER is mandatory. OpenSSL 3's strictness *matches* the rule the chain enforces. There is no divergence in the current configuration. This also settles the scope of #35 — it is build hygiene, not a live correctness bug. That is the narrowing already recorded there, now with evidence rather than argument. ### What it does cost **62 red tests mean nobody can tell a new regression from existing noise.** That is this issue's point, sharpened: the suite is not merely unrun, a quarter of `script_tests` fails by default, so "the tests pass" is not a statement anyone can currently make. Two ways to fix, not evaluated here: 1. Mark the affected cases expected-fail when linked against OpenSSL ≥ 1.1, so a genuine regression stands out. 2. Adopt libsecp256k1 (#35). Later Bitcoin ships `ecdsa_signature_parse_der_lax()` specifically to reproduce OpenSSL 1.0's leniency deterministically — but **the vendored `src/secp256k1/` here predates that helper** and exposes the older `secp256k1_ecdsa_verify(DER…)` API, so whether enabling it restores these cases is untested. Worth checking before treating #35 as the fix for this.
Author
Owner

The test suite is green — test_dobbscoin is 163 cases, no errors (fb9397d1)

This issue's most expensive symptom is gone. The suite had reported failures for as long as anyone ran it: 62 before the libsecp256k1 port (#35), 42 after, and 0 now. A new regression is no longer indistinguishable from the standing noise.

What the 42 actually were

script_tests.cpp builds roughly 70 cases at run time and asserts each one appears in src/test/data/script_valid.json / script_invalid.json. 42 did not — 20 valid, 22 invalid — each reported as Missing auto script_valid test: <comment>.

Every one of the 42 has a same-named entry in the file that is identical in every field except the scriptSig: a different signature over the same sighash, because the fixtures were generated by a different signer than the tree now uses. Checked for all 42, not sampled.

So nothing was verifying incorrectly. The checked-in vectors all pass on their own, and the earlier 20 (14 transaction_tests, 6 script_tests) were a different problem entirely — real OpenSSL-3-vs-non-canonical-DER failures that #35 removed. This was set membership.

The part worth keeping: do not overwrite the fixtures

The implied upstream workflow is to build with -DUPDATE_JSON_TESTS, run script_tests/script_build, and copy script_*.json.gen over the fixtures. Here that would delete about a thousand hand-written vectors:

file entries checked in entries in .gen
script_valid.json 591 30
script_invalid.json 435 42

The .gen files hold only the builder cases. The fix diffs and merges the 42 missing vectors, keeping the originals, so each of those cases now runs with two valid signatures instead of one and no coverage is lost. Both files carry a comment row recording that and the refresh recipe.

Diff is +254 lines, no deletions, data files only.

Verified on this commit

  • script_tests 9/9
  • full test_dobbscoin 163 cases, no errors
  • release gates unchanged: 33 consensus cases green, and the encrypt-wallet round trip passes (ckey=202 key=0 mkey=1, unlock works, wrong passphrase refused)

Leaving this issue open — the rest of the build/test modernization it asks for still stands. What is closed is the part that made the suite unusable as a signal.

## The test suite is green — `test_dobbscoin` is 163 cases, no errors (`fb9397d1`) This issue's most expensive symptom is gone. The suite had reported failures for as long as anyone ran it: **62** before the libsecp256k1 port (#35), **42** after, and **0** now. A new regression is no longer indistinguishable from the standing noise. ### What the 42 actually were `script_tests.cpp` builds roughly 70 cases at run time and asserts each one appears in `src/test/data/script_valid.json` / `script_invalid.json`. 42 did not — 20 valid, 22 invalid — each reported as `Missing auto script_valid test: <comment>`. **Every one of the 42 has a same-named entry in the file that is identical in every field except the `scriptSig`**: a different signature over the same sighash, because the fixtures were generated by a different signer than the tree now uses. Checked for all 42, not sampled. So nothing was verifying incorrectly. The checked-in vectors all pass on their own, and the earlier 20 (14 `transaction_tests`, 6 `script_tests`) were a different problem entirely — real OpenSSL-3-vs-non-canonical-DER failures that #35 removed. This was set membership. ### The part worth keeping: do not overwrite the fixtures The implied upstream workflow is to build with `-DUPDATE_JSON_TESTS`, run `script_tests/script_build`, and copy `script_*.json.gen` over the fixtures. **Here that would delete about a thousand hand-written vectors:** | file | entries checked in | entries in `.gen` | |---|---|---| | `script_valid.json` | 591 | 30 | | `script_invalid.json` | 435 | 42 | The `.gen` files hold only the builder cases. The fix diffs and **merges** the 42 missing vectors, keeping the originals, so each of those cases now runs with two valid signatures instead of one and no coverage is lost. Both files carry a comment row recording that and the refresh recipe. Diff is +254 lines, no deletions, data files only. ### Verified on this commit - `script_tests` 9/9 - full `test_dobbscoin` **163 cases, no errors** - release gates unchanged: 33 consensus cases green, and the encrypt-wallet round trip passes (`ckey=202 key=0 mkey=1`, unlock works, wrong passphrase refused) Leaving this issue open — the rest of the build/test modernization it asks for still stands. What is closed is the part that made the suite unusable as a signal.
btcbob referenced this issue from a commit 2026-09-25 13:07:06 +00:00
btcbob referenced this issue from a commit 2026-09-25 23:11:44 +00:00
btcbob referenced this issue from a commit 2026-09-26 01:31:02 +00:00
btcbob referenced this issue from a commit 2026-09-26 02:42:16 +00:00
btcbob referenced this issue from a commit 2026-09-26 02:49:35 +00:00
btcbob referenced this issue from a commit 2026-09-26 04:49:41 +00:00
Sign in to join this conversation.
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/dobbscoin-source#43
No description provided.