Fix the defects and test failures CI found once it could run - #3
Open
kwsantiago wants to merge 35 commits into
Open
kwsantiago wants to merge 35 commits into
kwsantiago wants to merge 35 commits into
Conversation
kwsantiago
force-pushed
the
wf-cleanup
branch
from
September 16, 2026 14:18
35371d3 to
694ff87
Compare
kwsantiago
force-pushed
the
wf-cleanup
branch
from
September 16, 2026 17:19
694ff87 to
3f9aea6
Compare
kwsantiago
force-pushed
the
wf-cleanup
branch
11 times, most recently
from
September 17, 2026 14:14
0838a8a to
f05293a
Compare
kwsantiago
force-pushed
the
wf-cleanup
branch
from
September 17, 2026 16:48
c1aad13 to
65a56f9
Compare
kwsantiago
force-pushed
the
wf-cleanup
branch
from
September 19, 2026 18:15
7504349 to
b93f343
Compare
kwsantiago
force-pushed
the
wf-cleanup
branch
from
September 19, 2026 21:17
bcc0ef7 to
77dda72
Compare
…o it needs sqlite
kwsantiago
force-pushed
the
wf-cleanup
branch
5 times, most recently
from
September 20, 2026 22:28
45ff38d to
7182e87
Compare
kwsantiago
force-pushed
the
wf-cleanup
branch
from
September 20, 2026 22:29
7182e87 to
11e9407
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Continuous Integration had never run to completion here: every pull request failed at
Check changelog, step 4 ofPre-build checks, so flake8, the builds, the unit tests and the integration suite were all skipped. With that cleared the suite ran, and this is what it found.Defects
Reachable in production, not test-only.
hsmdrebuilt the whole transaction for every input.sign_unified_wallet_input()cloned the entire PSBT per input. Held ontmpctx, which is released when the request completes rather than per input, that was 700 copies at once for a 700-input withdrawal: 13.2 GB resident inlightning_hsmd, 200 MB after, and consolidating a few hundred UTXOs was enough to exhaust the machine. Freeing each clone fixed the memory but left the work quadratic, so the clone is now built once per signing pass and dropped only where the anchored-close branch changes a prevout script or amount, which is all of it the digest commits to.test_bitcoin_backend_gianttxgoes 358s to 59s, and from the 1800s Valgrind timeout to 561s.--dev-warn-on-overgrinddid nothing for wallet inputs. The unified signing branch returns before the check the legacy path reaches, so a short signature was never reported.hsmdfailed open after the seed was freed.tal_bytelen(NULL)is 0, souse_bip86_derivation(0)was false and the signer fell back to legacy derivation instead of refusing. It now clearsinitializedfirst.gossipdcould abort a syncing node.probe_random_scidspasses the available block count topseudorand, which asserts it is non-zero; that count is zero when the tip sits exactlynum_blockspast activation. Verified at heights 962,640 and 971,640.signpsbton an input carrying onlynon_witness_utxoreached the signer without the prevout metadata the unified path authenticates against, andhsmdanswers that withmaster_badmsg().walletrpcfilledwitness_utxoin only when both records were absent, so the shape BIP174 recommends, and several tools emit, was fatal. Reproduced and fixed; regression test added.wally_txidrecomputation to make signing linear also skipped the bounds check and thewitness_utxovsutxo->outputs[index]comparison. Only the hashing is expensive, so the comparisons are back for every input.sign_tx_input()read the PSBT before bounds-checking the index, wherecheck_tx_sig()already checked it. Twohsmdhandlers pass an index straight off the wire.Tests
test_coinmoves: five balances were 7 sat high. One extra witness byte, since0x21forces a 65 byte p2tr signature whereSIGHASH_DEFAULTuses 64. The same adjustment already exists intest_wallet.py.test_gossip: bit 68 is even, so it appears innode_announcementand widens every stored record.test_gossip_lease_rates,test_bookkeeping_missed_chans_leases,test_channel_lease_post_expiry: that same witness byte raises the defaultfunding_weightfrom 584 to 586, and so the lease fee by 4 sat at 2000 perkw.test_unified_emergencyrecover,test_unified_penalty: one unlinkslightningd.sqlite3and the other copies it, so both carry the same sqlite guard the release already uses for its own.test_blake2b_required_peer_bit: assertedconnectraises, which only holds when the peer's hangup beats the RPC reply.test_sendpay_grouping,test_unified_onchain_htlc_success: both raced asynchronous state; the second pinned an exact txid while onchaind was fee-bumping it.test_currencyrate,test_pay: an API that now needs a key, and an invoice that expired in May.simple_reorg()askedprioritisetransactionto hold a transaction back and this backend mines it anyway; canned blocks are 487 days old, so the node sat in initial block download.did_short_sig()polls rather than sleeping a fixed second. This diverges from v26.06.7 deliberately: signing here is slower, so the warning can arrive after the sleep upstream relies on.test_openchannel2_inflight_limit: the hand-builtopen_channel2pinned a commitment feerate of 253 and a channel type of[12, 22]. This backend's floor is higher, and the type needsunified_sigs; both are now derived rather than hardcoded.test_onchain_missing_no_p2tr_migrate,test_rescan_missing_utxo: these had gained askipIf(TEST_NETWORK == 'regtest')alongside an existing!= 'regtest', so they ran nowhere while reporting as skipped. They now set the activation height past their canned SHA256 headers, the waytest_db.pyalready does, and run again.test_splice_stored_feerate_repaired_on_upgrade: usesdb_manip, which is sqlite-only inpyln. Every other caller in the tree is guarded; this one was not. Upstream has since added the same guard, so this is a backport rather than a divergence.pyln: thebitcoindfixture reaped a process that a test had never started. Proposed upstream separately.CI
.github/workflows/ci.yamlnow matches v26.06.7 exactly, apart fromBLAKE2B_CI, the Blake2b backend job replacing the minimum-supported-Bitcoin job, and the dropped elements job below.--maxfail=1is dropped, which the release does not set and which aborted a whole group at its first casualty.paperclip-build.ymltargeted a branch this repository does not have.Elements
The elements integration job is dropped. Elements is a different chain, so Bitcoin's proof of work becoming BLAKE2b does not reach it and every unified-signing path refuses it: the job tested nothing this branch changes; it was also the one remaining red job, failing
test_bitcoin_bad_estimatefeeon a feerate floor, in a test and plugin identical to v26.06.7.Removing it surfaced something worth recording rather than losing:
default_features()runs before--networkis parsed, so a node built from this branch advertisesoption_blake2bandoption_unified_sigsas compulsory on elements while being unable to honor either. Nothing this branch targets runs there, but the job should come back once that is gated.Changelog-None