Build and test modernization — the section that lowers the cost of every other section #43
Labels
No labels
do-not-open-till-X-Day
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
SubGeniusFinance/dobbscoin-source#43
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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
autogen.sh+configure.ac+Makefile.am; there is noCMakeLists.txt. Combined with the staticdb-4.8.30requirement fromdoc/build-unix.md:155, a first build is a ritual rather than a command.src/test/and 23functional tests under
qa/rpc-tests/. This is a better starting position thanthe roadmap prose implied — the gap is not that tests do not exist, it is that
nothing runs them automatically.
.github/workflows/release.yml, and it only buildsreleases. Nothing runs the unit or functional suites on a push.
What Core did
platform-specific build folklore.
explorers and tooling can reuse validation without shipping a whole node.
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 anad-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.
The 62 failing tests are diagnosed
test_dobbscoinfails 62 cases onmain— 48 inscript_tests, 14 intransaction_tests. Verified identical before and after the CLTV merge, so they predate it.They are all the same test
Every failing
script_buildcase has "without DERSIG" in its name:And not one failing
transaction_testscase includesDERSIGin its flags — they run underP2SHandP2SH,NULLDUMMYonly.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_SECP256K1is not defined, soCPubKey::Verifyuses 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:
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.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_testsfails by default, so "the tests pass" is not a statement anyone can currently make.Two ways to fix, not evaluated here:
ecdsa_signature_parse_der_lax()specifically to reproduce OpenSSL 1.0's leniency deterministically — but the vendoredsrc/secp256k1/here predates that helper and exposes the oldersecp256k1_ecdsa_verify(DER…)API, so whether enabling it restores these cases is untested. Worth checking before treating #35 as the fix for this.The test suite is green —
test_dobbscoinis 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.cppbuilds roughly 70 cases at run time and asserts each one appears insrc/test/data/script_valid.json/script_invalid.json. 42 did not — 20 valid, 22 invalid — each reported asMissing 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, 6script_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, runscript_tests/script_build, and copyscript_*.json.genover the fixtures. Here that would delete about a thousand hand-written vectors:.genscript_valid.jsonscript_invalid.jsonThe
.genfiles 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_tests9/9test_dobbscoin163 cases, no errorsckey=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.