Repository navigation
Add option_independent_secrets channel type - #48
davelund110 wants to merge 41 commits into
Conversation
kwsantiago
left a comment
kwsantiago
left a comment
There was a problem hiding this comment.
A sold lease skips the cap's backstop, so storage is unbounded until expiry; the cap's mutual close also needs a timeout.
Thanks, both fixed in b2e57b6: the backstop now applies during a lease (we still don’t start the mutual close before it expires), and the cap’s mutual close times out after 48h, like close’s default. A lowered cap also counts its 10% from where the channel was now. There are three new tests: lease, timeout, lowered cap. |
kwsantiago
left a comment
kwsantiago
left a comment
There was a problem hiding this comment.
Both fixed, thanks. A restart resets the slack and timer, and re-arms closes we didn't start.
|
Good catch, fixed in 4df42d5: the count at which our close started is now saved in the db (a new column), so the backstop doesn't reset on a restart, and at startup the timeout is armed only for a close we started, with the time it has left. There are two more tests: restart mid-close, and a user's close left alone. |
|
Two small things from reading this alongside the LND side:
|
6aa6ca8 to
8aebea0
Compare
|
Thanks Paul, both done:
@kwsantiago: also af1d1fa from the spec review on bolts#17: a peer that sets 266 in init is now refused even when we offer the option, matching Lightning Fork. And the branch is rebased onto blake2b-unified at e48ea5b to clear the conflicts; ours now come after the base's new migration, hsmd message and rewind test. On that base: test_independent_secrets.py passes both ways (26 v1, 17 dual-funded), all 61 of test_splicing.py, the unit tests and check-hsm-versions, and all nine interop scenarios against Lightning Fork 9b0d0b3. |
|
Thanks Dave, both look right, and the breach tests on the splice's funding are good to have. I built 8aebea0 and ran test_independent_secrets.py (v1: 26 passed; dual-funded: 17 passed) and the two db-rewind tests in test_splicing.py; all pass. The rebase only changed the three commits with conflicts, and the migration order is safe since the base migration shipped in .8 and these are in no release. A few small things:
|
|
Thanks Paul, all done in five new commits on top:
With them: |
|
Thanks Dave, the five commits look right, and test_independent_secrets.py passes here in both modes (v1: 27 passed; dual-funded: 18 passed). Disabling the two accept-side refusals of 2055f81 makes the new sender test fail in both, so it covers them. One thing to update: v26.06.9-blake2b.9 was tagged today (bf4e4ea) without this PR, so the "added in v26.06.9-blake2b.9" labels in the docs and schemas want to become .10, and the branch a rebase onto .9. Smaller, optional:
|
Proposed in a draft bLIP: a channel type add-on under which a node doesn't require its peer's per-commitment secrets to come from a shachain, so the peer can generate them k-of-n. This adds the bits (even in the channel type, odd in init and node_announcement), their name, the dependency on option_channel_type, and accepts the add-on as a variant of either basic channel type when we offer it. Nothing offers it yet.
…n one place How many per-commitment secrets the peer has revealed, and the last of them, come from the shachain in four places: the revocation check, channeld's init and channel_reestablish for a closing or long-closed channel. Put them behind channel_their_revocations() and channel_their_last_secret() (and closed_channel_their_last_secret() for closed ones), which on an option_independent_secrets channel whose secrets outgrew the shachain will read a count and the latest secret instead. No change for shachain channels.
…hain On a channel with option_independent_secrets the peer's per-commitment secrets needn't be a shachain. They still go in it for as long as they fit, as they always do from a peer which uses one: then nothing changes, down to the static channel backup, which carries the shachain and so can still punish. From the first secret which doesn't fit, peer_got_revoke() stores that one and every later one in a new table, channel_revocation_secrets, in the same db transaction as the rest of the revocation. channeld has already checked that each secret generates the point of the commitment it revokes. Loading a channel, open or closed, reads whether its secrets outgrew the shachain, and if so how many there are and the latest, for channel_reestablish. Once the channel is closed and resolved, wallet_channel_close() keeps only the latest: nothing can spend the funding output any more. The migration's revert refuses while a channel which hasn't closed uses the add-on, since an older version would expect the next secret to extend a shachain and fail the channel. test_splice_stored_feerate_repaired_on_upgrade re-runs the last migrations, so it now reaches back one more, and drops the new table first.
onchaind derives a revoked commitment's secret from the shachain it is given, which on an option_independent_secrets channel may hold only the secrets before the first which didn't fit. So lightningd unmasks the number of the commitment that spent the funding output, exactly as onchaind does, and if the peer revoked it and its secret is in the db, passes that secret in onchaind_init, along with the number of secrets held, which onchaind now takes from lightningd rather than counting the shachain. A mutual close or one of our own commitments can unmask to a revoked number too: onchaind recognizes those first and ignores the secret.
…nel types listpeerchannels, listclosedchannels and the opening commands report the channel type's names, which are a closed enum in the schemas, cln-rpc's ChannelTypeName and cln-grpc's primitives.proto: add the new one in each, so a channel with the add-on validates and deserializes. primitives_pb2.py is regenerated with protoc 31.1, which reproduces the previous file exactly.
With --independent-secrets we signal the optional bit, and every new channel with a peer which signals it too uses the add-on, including a channel type the caller names (it changes nothing but what the receiver accepts). Channels with other peers open as before. It is off by default: the receiver of such secrets stores each one, so an operator chooses to. It can't be used with an external signer, an hsmd replaced with --subdaemon such as VLS, which checks the peer's secrets against a shachain: lightningd refuses to start with both, or with an external signer while any channel with the add-on is open or being resolved, since a channel keeps it after the option is removed. Like option_blake2b and option_unified_sigs, it is not available on Liquid. Changelog-Added: Protocol: `option_independent_secrets` (266/267): a peer's per-commitment secrets need not come from a shachain, so it can generate them k-of-n; `--independent-secrets` offers it (off by default, and not with an external signer).
To exercise a peer's support for option_independent_secrets, this developer option makes hsmd derive our own per-commitment secrets as HMAC-SHA256(shaseed, "independent-per-commitment-secret" || n || counter), the same function as Lightning Fork's dev option, instead of from a shachain. Like the shachain it needs nothing but the seed, so every secret can be produced again. It applies to every channel, so lightningd refuses to start with it changed while any channel is open or being resolved, and it needs --independent-secrets, since only a peer with the add-on accepts them.
Negotiation, which only --independent-secrets offers, and the options it refuses (an external signer, also while a channel with the add-on is open, and the dev sender without it); that a channel keeps the add-on, storing and punishing, after a restart without the option; that a peer which uses a shachain costs nothing, including to a static backup (test_emergencyrecoverpenaltytxn now also runs with the add-on); storage across restarts and reconnections; that a shachain channel refuses the dev sender's secrets (which shows the other tests really give the receiver unrelated ones); the startup guard on the dev option; penalties from the db and from the shachain part, with and without an HTLC, and across restarts of onchaind; the peer's own unilateral close; pruning once the channel is forgotten, and channel_reestablish for it afterwards. Most run with both v1 and v2 opens.
What the add-on is for, what this node does with it, how to turn it on, its limits (static channel backups, external signers, downgrades) and the dev option.
On a channel with option_independent_secrets, listpeerchannels now shows independent_secrets_stored: how many of the peer's per-commitment secrets we store one by one, from the first which didn't fit the shachain. It stays 0 with a peer which uses one, and it is how an operator sees what the add-on costs on each channel. Changelog-Added: JSON-RPC: `listpeerchannels` shows `independent_secrets_stored` on channels with `option_independent_secrets`.
On a channel with option_independent_secrets, every update adds a secret we store one by one once they outgrow the shachain, and nothing else bounds them while the channel is open. --max-independent-secrets (default 1,000,000: 32MB of secrets, about 60MB in sqlite3; 0 for no limit) bounds them: at 90% we log a warning, and at the limit we start a mutual close, as `close` would, sending P2WPKH if the peer can't take our default P2TR address. After `shutdown` the peer only needs as many more as it takes to settle the HTLCs in flight, but it can keep a closing channel updating with update_fee, so once we store 10% more than the limit (at least one more) we close the channel unilaterally. Like `close`, we keep a lease we sold, and close at the first update after it expires. Changelog-Added: Config: `max-independent-secrets` closes a channel with `option_independent_secrets` once it stores that many of the peer's secrets (default 1000000).
… close Three fixes to max-independent-secrets, from review: - A channel the peer leased from us skipped the backstop as well as the mutual close, so until the lease expired a peer could make us store its secrets without limit. We still don't start a mutual close before the lease expires, but the 10% backstop now applies: our funds stay locked until then however the channel closes. - The mutual close started at the cap had no timeout, so a peer could leave it unfinished for ever. After 48 hours, as `close` waits by default, we now close unilaterally. The timer isn't saved, so at startup it is armed again for a channel that is closing and over the cap. --dev-independent-secrets-close-timeout shortens it for tests. - The 10% now counts from where we first saw the channel at or over the cap, so a cap lowered below what a channel already stores starts a mutual close at its next update instead of failing the channel at the one after. Also point the docs and comments at lightning-blake2b/bolts#17, where the spec now lives, instead of the draft bLIP.
From review: a restart reset the 10% backstop and the 48-hour timeout of the mutual close started at max-independent-secrets, and at startup the timeout was armed for any closing channel over the cap, including a close we didn't start. - How many secrets the channel stored when we started the close is now saved, in a new channels column, and the backstop counts from there. Without such a close it counts from the cap itself, so nothing resets on a restart. - At startup the timeout is armed only for a close we started, with whatever is left of its time since the channel moved to CHANNELD_SHUTTING_DOWN, as its state changes record.
…t in init The spec (lightning-blake2b/bolts#17) now says a receiver of bit 266 in init MUST close the connection: the even bit is only valid in channel_type. A node without --independent-secrets already did, since the bit was unknown to it, but one that offers the option knows the bit and kept the peer. Lightning Fork refuses such a peer either way, so the two now match.
channel_start_mutual_close() accepted CHANNELD_AWAITING_SPLICE, so reaching max-independent-secrets while a splice was pending sent shutdown before the splice locked, which BOLT 2 doesn't allow. close does the same today, but here nobody asked for a close. It now refuses a channel with a pending splice, and the cap logs that it will close once the splice locks; handle_peer_splice_locked() starts the close then. The 10% backstop still bounds what we store meanwhile.
The peer's secrets are stored by commitment number, and one secret revokes that number on every funding output, so a revoked commitment on a splice's funding output is punished like any other. Test it with a cheater that sends independent secrets: its revoked commitment on the new funding, broadcast while the splice is pending (spending the splice transaction), and after the splice locked.
If max-independent-secrets is reached while a splice is pending, the mutual close waits for the splice to lock. If the splice is aborted instead, nothing checked the cap again until the next revocation. Check it once channeld restarts after the abort.
The 10% backstop can close a channel unilaterally without a mutual close having been tried (during a lease, while a splice is pending, or after the 48 hours); the close waits for a pending splice to lock; a peer that sets 266 in init is refused; a limit below 1,000,000 is logged. v26.06.9-blake2b.8 was tagged without this, so the options and the new field are added in the next release, .9.
At the backstop, check_independent_secrets_cap() fails the channel. On a channel whose funding we withheld, drop_to_chain() then forgets it, which frees it, yet peer_got_revoke() went on to save it and to fill the commitment_revocation hook's payload from it: a use-after-free, which valgrind reports and which can crash lightningd or write the forgotten channel back. Check the cap after everything else instead. The new test opens a withheld zeroconf channel with a peer using the dev sender and a cap of 10, and settles a payment past the backstop: the channel is forgotten, stays forgotten across a restart, and valgrind reports nothing. Under valgrind it fails without this change.
…ered cap's slack Two problems from a security review of the cap: - After a restart, the timeout of the mutual close we started was re-armed with 1 second if lightningd had been down for longer than the whole timeout, or if the clock had gone back (time_between() of a later timestamp). Timers already run during startup, before connectd, so the channel was closed unilaterally before the peer could reconnect. Now a clock that has gone back counts as no time, and the peer gets a tenth of the timeout, up to 10 minutes (so 10 minutes by default). Whether the close is ours now comes from its state change, so independent_secrets_close_at can be used for the backstop alone. - When the limit was lowered below what a channel stores and we couldn't start a mutual close (a lease we sold, a pending splice, a close someone else started, or one that couldn't start), the backstop counted from the new limit and failed the channel at its next update. It now counts from where the channel first reached the limit, saved likewise, or from the limit if that was raised since. The docs also say that update_fee can only keep a closing channel updating while HTLCs remain. New tests: a restart after the timeout ran out, one with the clock gone back, and a limit lowered while a splice is pending. All three fail without this change. The splice tests' payment helper now times out rather than hanging when its channel closes.
With --dev-independent-secrets-sender, fundchannel_start refused a channel type without option_independent_secrets only after its command_check_only(), so `check fundchannel_start` said it would succeed. Derive the type and refuse it first, as openchannel_init already does. The test checks both commands with `check`.
handle_splice_abort() checks the cap again, but nothing tested it. A splice aborted with the channel past a limit lowered while we were down now has to start the close without another revocation: the test fails without that check. And an aborted RBF attempt drops only the attempt, so the close waiting at the cap keeps waiting for the splice under it, and starts once that locks.
v26.06.9-blake2b.9 was tagged without option_independent_secrets, so the options and the listpeerchannels field come in the next release. And an aborted RBF attempt leaves the splice it would have replaced pending, so the cap's close keeps waiting for that one: say so, rather than that an aborted splice always starts the close.
383eace to
d4d1391
Compare
|
Thanks Paul. The branch is rebased onto v26.06.9-blake2b.9 (bf4e4ea), without conflicts, and your points are in three new commits:
Before those are two commits from a security review of the whole series, read as an attacker would. It found nothing that could lose funds, but three things in the cap code:
The new tests for the fixes fail without them. With everything, on the new base: |
|
Thanks Dave. I went through the five new commits and found nothing more to fix. The rebase onto .9 changed none of the earlier commits, and with each fix reverted its test fails: the cap check moved back above the save gives valgrind's invalid read in Here: test_independent_secrets.py 33 passed (v1) and 21 (dual-funded); the cap tests under valgrind 13 (v1) and 6 (dual-funded) with no errors. Our lab scenarios against this head, with our next release candidate, pass as before apart from one that now fails in setup (a reconnect during Two small notes:
|
peer_got_revoke() started the cap's close after replying to channeld. A peer that sent stfu and splice_init right after the revoke_and_ack that reached max-independent-secrets could have channeld accept the splice before it learned of the shutdown. channeld then held the shutdown while quiescent and sent it once the splice was signed but unlocked, which BOLT 2 forbids; lightningd went from CHANNELD_SHUTTING_DOWN back to CHANNELD_AWAITING_SPLICE, the mutual close spent the old funding output and conflicted with the splice, and the channel stayed in CLOSINGD_COMPLETE until a restart. Now peer_got_revoke() starts the close before it replies, so channeld, which waits for that reply, queues the shutdown request and handles it before anything else from the peer. The backstop, which can free the channel, still runs last. And channeld stops splicing once either side has sent shutdown, or lightningd has asked it to send one: an incoming splice_init or tx_init_rbf gets a warning and the connection is closed, as BOLT 2 asks of a node that has received shutdown (we reestablish with ours); a local splice_init is refused; and a local one still waiting for quiescence is cancelled the same way. test_independent_secrets_cap_refuses_splice has the peer start a splice right after the revocation that reaches the cap: without this change its splice is accepted; with it, the splice is refused and the close completes on the old funding output.
…limit From Paul's review of commits 22 to 26: - The comment on the independent_secrets_close_at migration still said it records where the channel stood when we started our close. Until then (a lease, a pending splice) it's where the channel first reached max-independent-secrets. - After a restart, the timeout of a close we started at the limit is re-armed even if max-independent-secrets is now 0, as close's own timeout is. The docs say so.
|
Thanks Paul, and for reverting each fix to check its test. Both notes are in cbc4c55: the migration's comment now says the 10% counts from where the channel stood when our close started or, until then, where it first reached the cap; and the docs say a close's timeout stays if the limit is then set to 0, as Before it is one more commit, d3e988f, from a second security review of the whole series. Its storage, reestablish, penalty and database half found nothing to fix (it also tried a reorg that swaps one revoked commitment for another: the second is punished). The cap half found one thing:
The review also caught three claims in the description that were broader than the code: our latest commitment, not any of ours, is recognised before a secret is used; the 10% restarts when our close starts after a lease or splice wait, so storage stays below about 1.2x the cap rather than 1.1x; and the migration's revert only runs in tests, since @kwsantiago, three older splice issues turned up while reproducing it, none from this PR (they're also at the end of the description): With everything: |
…htning Fork and the CLN port scenario-independent-secrets.sh runs each case on a BLAKE2b regtest of its own (isec-knots) on the lab network: negotiation with each side on, off or the previous release; normal operation with both ends sending non-shachain secrets; a revoked state broadcast by either side and punished by the other, before, during and after a splice; an lnd tower punishing for an offline lnd; a peer setting 266 in init. It needs the CLN port with privkeyio/lightning#48 (CLN_IMAGE) and lnd with lightning-fork#7 (LND_IMAGE, and LND_INT_IMAGE, the same tree built with the integration tag and SIGHASH_UNIFIED on); LND_REL_IMAGE is the previous release. lnd runs with --nobootstrap: once the peer's node announcement is in its graph the bootstrapper dials the peer again, and the peer drops the connection it had along with a channel being opened on it.
|
Thanks Dave. d3e988f fixes the revocation path, but the same race is still open when the cap's close starts at a splice lock. If the cap is reached while a splice is pending, the close waits for the lock, and I reproduced it with a probe that widens the window (a plugin delaying lightningd's database write for the lock by 4 s; normally the window is that write against one round trip to the peer): l1 sent On the new test: with only the ordering change reverted it still refuses the splice and completes the close, and fails only because the "deferred WIRE_CHANNELD_SEND_SHUTDOWN" log line is missing; with channeld's refusal removed and the ordering kept, the splice is accepted. So the refusal does the work and the ordering is extra safety, which the test could assert on behaviour rather than the log line. Everything else checks out: the early cap check never frees the channel (4585d53's ordering holds), and HTLC failures after the shutdown request are fine under BOLT 2. Here: test_independent_secrets.py 34 (v1) and 22 (dual-funded), test_splicing.py and test_splicing_disconnect.py 65, the cap and splice tests under valgrind clean, our lab scenarios all pass, and your nine interop scenarios pass against Lightning Fork's |
…t a splice lock d3e988f started the cap's close before replying to the revocation that reaches max-independent-secrets, but when the cap is reached while a splice is pending, the close starts at the lock instead, in handle_peer_splice_locked(), and channeld didn't wait for that: it told lightningd of the lock and carried on. A peer that sent stfu and splice_init right after the lock could have the new splice accepted before channeld learned of the shutdown. channeld then sent shutdown with that splice unlocked, which BOLT 2 forbids; lightningd went from CHANNELD_SHUTTING_DOWN back to CHANNELD_AWAITING_SPLICE; the mutual close conflicted with the splice; and the channel stayed in CLOSINGD_COMPLETE until a restart. A slow database write at the lock (a db_write plugin, say) makes the window wide. Now channeld waits for a reply to channeld_got_splice_locked, and lightningd starts the close before it replies, so channeld handles the shutdown request before anything else from the peer, and refuses the splice. The backstop, which can free the channel, runs after the reply.
… what happens test_independent_secrets_cap_refuses_splice_at_lock has the peer start a new splice as soon as the splice the cap's close waited for locks, while a db_write plugin (tests/plugins/dbwrite_stall.py) holds lightningd at the lock for 8 seconds. Without the previous commit l1 sends splice_ack; with it, the splice is refused and the close spends the funding that locked. test_independent_secrets_cap_refuses_splice now checks what happens (the splice refused, no splice_ack, the close spending the funding we had) rather than the order of channeld's log lines, as Paul suggested: it's channeld's refusal that stops the splice there.
…eestablish Messages a peer sends before its channel_reestablish are stashed, and then handled in one loop, without first handling requests lightningd queued meanwhile, as the main loop does. So a peer could send the revoke_and_ack that reaches max-independent-secrets, then stfu and splice_init, all before its channel_reestablish: lightningd asks for the shutdown while the revocation is handled, but channeld would accept the splice before it got to that request. Now it handles lightningd's queued requests before each stashed message.
Every penalty test had the node that stores the secrets as the channel's accepter. lightningd unmasks a revoked commitment's number with the opener's payment basepoint first, which only a channel we opened tells apart: with that order broken, the existing tests only noticed through a log line on the cheating node. test_independent_secrets_penalty_we_opened swaps the roles, and fails directly.
check-source-bolt takes a comment that starts with "BOLT" for a quote from the BOLTs, and wants "BOLT #n:" with the quoted text. Three comments in this series start "BOLT 2 doesn't allow ..." or "BOLT 3 says ...", which made check-source fail. Reword them.
run-wallet includes peer_htlcs.c, whose cap code calls channel_start_mutual_close(). gcc's builds drop the unused reference, but clang's (the macOS CI job, with --disable-compat) doesn't, and the link fails. Add the stub update-mocks would generate.
|
Thanks Paul, and for reproducing it. A review here found the same race, and the fix is in four new commits, plus two for the first CI run on this PR. f72e90c: channeld now waits for lightningd at a splice lock (a new reply, channeld_got_splice_locked_reply), and lightningd starts the cap's close before it replies, as the revocation does since d3e988f. The backstop still runs after the reply. I went with this rather than a tx_abort when a shutdown request arrives: it's the same shape as d3e988f, and the new splice is refused before any negotiation starts. A tx_abort would also help a user's close during a splice, the older issue, but it touches splice negotiation beyond this PR, so I'd leave it for a separate one. 231c6a6: the same review found a related path by reading. Messages a peer sends before its channel_reestablish are handled in one loop, without first handling requests lightningd queued meanwhile, so the revoke_and_ack that reaches the cap, then stfu and splice_init, all sent before the reestablish, would get past the refusal. channeld now handles queued requests before each of those messages, as its main loop does. I couldn't test this one: it needs a peer that sends them out of order. On the tests:
With everything: test_independent_secrets.py passes both ways (36 with v1 opens, 24 dual-funded), all of test_splicing.py and test_splicing_disconnect.py (65), check-units, the changed cap and splice tests under valgrind with no errors, and all nine interop scenarios against Lightning Fork's blake2b at b13cb18. The first CI run on this PR also turned up two older problems, both fixed: 44c4d93 rewords three comments that started with "BOLT 2" or "BOLT 3", which check-source-bolt takes for quotes from the spec; and 03c4d78 adds the run-wallet stub for channel_start_mutual_close, which the macOS (clang) build needs to link. Both pass here, check-source-bolt against the BOLTs and the unit tests built with clang and --disable-compat. |
A plugin's getmanifest featurebits were merged into our features with only a check that a bit wasn't already set. So a static plugin could set bit 267 and turn option_independent_secrets on without --independent-secrets (listconfigs still showing it off, and the Liquid refusal skipped), or set the even bit 266, which BOLT 3 forbids in init and node_announcement. Refuse a plugin that sets either bit, in any place, as lnd refuses the equivalent custom features.
…needs both peers fundchannel_start and openchannel_init add bit 266 to a channel_type the caller names when both peers offer the feature, but sent a named type with 266 to a peer that doesn't offer 267, which BOLT 2 says the opener SHOULD NOT do (a type with a feature that wasn't negotiated). As fundee, openingd and dualopend accepted 266 from such a peer. Refuse both, as lnd does. --dev-any-channel-type still allows any type.
…sing BOLT 2 asks the splice_init receiver to warn and close the connection if it has received shutdown, and to refuse with tx_abort otherwise. When only we are closing we warn and reconnect too: after a tx_abort channeld restarts on the same connection, and the new channeld can't tell whether our shutdown went out, so it could send it twice (lnd disconnects on a second shutdown) or never, while reestablish retransmits it. Say so in the comment and the doc rather than change it.
The docs said that, like close's own timeout, the timeout of a close started at max-independent-secrets stays if the limit is set to 0. But close's timeout belongs to the command and a restart forgets it, while the cap's is armed again at startup (and changing the limit takes a restart).
|
Four small commits for the remaining low items from the review. None of them could lose funds:
test_independent_secrets passes (v1 and dual-funded), as do the splicing tests and the plugin feature tests. |
listpeerchannels gained independent_secrets_stored, so the sql plugin's peerchannels table has the column too (doc/schemas/sql.json already lists it), but test_sql's expected schema didn't.
The cap's close timer is armed before plugins are configured, and lightningd runs its event loop while they are, so on a slow start (under valgrind) a timeout with a few seconds left fires before "Server started", where the log search after restart() begins. test_independent_secrets_cap_close_restart and _restart_late[overdue] then never saw the line. Look for it from the restart instead.
test_independent_secrets_cap_refuses_splice_at_lock stalls lightningd with a db_write hook, and only sqlite3 reports writes to that hook, so with postgres the stall never comes. Skip it there, as test_db_hook is.
|
The first full CI run turned up three problems in the new tests, not in the code. All are fixed:
All checks pass now. @kwsantiago @paulscode this is ready for another look when you have time. |
What
This PR is Core Lightning's side of
option_independent_secrets. The add-on is specified in the fork's BOLTs (lightning-blake2b/bolts#17), and Lightning Fork has it too (paulscode/lightning-fork#7, merged); the two were tested against each other.channel_revocation_secrets, in the same database transaction as the rest of the revocation.--independent-secretsturns it on, since the receiver of such secrets pays for storing them.--subdaemon, such as VLS), which checks the peer's secrets against a shachain: lightningd refuses to start with both, or with an external signer while any channel with the add-on is open or being resolved.listpeerchannelsshowsindependent_secrets_storedon such a channel: how many of the peer's secrets are stored one by one.--max-independent-secrets(default 1,000,000; 0 for no limit; a value below 1,000,000, the lowest the spec recommends, is logged as a warning). At 90% the node logs a warning, and at the cap it starts a mutual close of the channel, asclosewould. A peer that keeps the closing channel updating has it closed unilaterally once it has made us store 10% more, and so does a close that hasn't finished in 48 hours.channel_type. A peer that doesn't signal it gets a normal channel.listpeerchannels,listclosedchannelsand the opening commands show it asindependent_secrets/even.Channels without the bit behave exactly as before. They keep their shachain, run the same checks, and store the same rows.
Size: about 1,280 added lines of C outside the tests; with tests, docs and regenerated bindings, 61 files, +3,070/−115. GitHub shows about 1,000 more lines each way, because it counts the regenerated
node_pb2.pyline by line; most of that is generated offsets that move whenever node.proto changes.Why
With a shachain, every signer of a k-of-n node must hold the seed, because any of them may need to revoke a commitment.
Independent secrets let the signers generate and reveal secrets jointly.
This PR doesn't add a threshold signer. It adds what every peer must support first: accepting secrets that don't come from a shachain. For testing, it also adds a developer-only way for this node to send such secrets.
Commits
Each commit builds and passes the unit tests, so they can be reviewed one at a time.
common: the feature bits, their name, the dependency onoption_channel_type, and acceptance as a channel type variant.lightningd: the count of the peer's revocations and its last secret come from one place. A refactor: shachain channels see no change.wallet, lightningd: keep the peer's secrets in the shachain while they fit, and store them from the first that doesn't. Reload the count and the latest; keep only the latest once the channel is closed. Includes the migration and its guarded revert.onchaind: take a revoked commitment's secret from lightningd.doc/schemas, cln-rpc, cln-grpc: the new channel type name.lightningd:--independent-secrets(off by default, refused with an external signer), docs,listconfigsand the regenerated bindings.hsmd, lightningd:--dev-independent-secrets-sender, which needs--independent-secrets.pytest: the new tests.doc:doc/independent-secrets.mdand the README.lightningd:independent_secrets_storedinlistpeerchannels.lightningd:--max-independent-secrets, the warning at 90% and the mutual close at the cap.lightningd: review fixes to the cap: the 10% backstop also applies while a lease we sold runs, the cap's mutual close times out after 48 hours, and a lowered cap counts its 10% from where the channel was.lightningd: keep the backstop and the close's timeout across a restart, and re-arm the timeout only for a close we started.connectd: refuse a peer that sets the even bit, 266, ininit, as the spec now requires.lightningd: hold the cap's mutual close until a pending splice locks.pytest: punish a breach on a splice's funding output with stored secrets.lightningd: start the cap's close when a pending splice is aborted.lightningd: warn about amax-independent-secretsbelow 1,000,000.lightningd: keep the dev sender to channels with the add-on (--dev-independent-secrets-any-channellifts that for the test that needs it).pytest: check the spliced penalty's funding output and amount.doc: the backstop and splices, 266 ininit, and the next release.lightningd: check the cap last, since its backstop can free the channel.lightningd: give the cap's close time after a restart, and keep a lowered cap's slack.lightningd: refuse the dev sender's plain channel beforecheckreturns.pytest: the cap's close after an aborted splice or RBF attempt.doc: added in v26.06.9-blake2b.10, and an aborted RBF attempt.lightningd, channeld: refuse a splice once the cap's close is coming.wallet, doc: where the backstop counts from, and the timeout with no limit.How
Negotiation (
common,lightningd/channel.c,opening_control.c,dual_open_control.c)init,node_announcementandchannel_type, and depend onoption_channel_type.initis refused (commit 14). The spec only allows the even bit inchannel_type, and now says a receiver of it ininitMUST close the connection. A node without the option already refused it as an unknown even bit; one with the option knew the bit and kept the peer. Lightning Fork refuses it either way, so the two match.channel_type_accept()treats 266 as a variant of both basic types, and only when we offer it. So a node without--independent-secretsrefuses a channel type that includes it.desired_channel_type()adds the bit when both sides have it.fundchannel_startandopenchannel_initwhen the caller names a type, because it changes nothing but what the receiver accepts.option_unified_sigs.Receiving secrets (
peer_htlcs.c)peer_got_revoke()checks that the revocation number equals the count, then offers the secret to the shachain, as now.shachain_add_hash()checks a secret against the ones before it before it changes anything, so a refused secret leaves the shachain as it was.Reestablish (
channel.c,channel_control.c,peer_control.c)channel_their_revocations()andchannel_their_last_secret()give the count and the latest secret: from the stored secrets once there are any (kept in memory, and reloaded at startup), and from the shachain otherwise.CLOSINGD_COMPLETE), and the direct reply to a reestablish on any other closed channel, including a long-closed one, all go through them.Database (
wallet/)channel_revocation_secrets(channel_id, commitnum, secret).(channel_id, commitnum).ON DELETE CASCADE, as the other per-channel tables do.wallet_channel_close()deletes all but the latest stored secret, whichchannel_reestablishstill needs. It runs only once the channel is irrevocably resolved, never funded, or forgotten withdev-forget-channel.lightning-downgraderefuses this fork's wallets anyway, so for now the revert only runs inrun-wallet.)channels.independent_secrets_close_at(commit 13, with its own migration and revert) records how many secrets the channel stored when it first reached the cap, so the backstop survives a restart.Onchain (
onchain_control.c,onchaind/)onchaind_init, along with the count. Otherwise onchaind derives it from the shachain, as now.Count and cap (
channel.c,peer_control.c,peer_htlcs.c,closing_control.c)channel_their_stored_secrets()is the count of stored secrets: the total revocations less those the shachain holds, since the stored ones run without gaps from the first that didn't fit.listpeerchannelsreports it on channels with the add-on.peer_got_revoke()compares the count withmax-independent-secrets. It logs a warning (UNUSUAL) once the count reaches 90% of the cap (once per run, so also after a restart or a lowered cap), and at the cap callschannel_start_mutual_close().channel_start_mutual_close()does whatclosedoes with no options, without a command: it moves a channel inCHANNELD_NORMALorCHANNELD_AWAITING_LOCKINtoCHANNELD_SHUTTING_DOWNwith reasonlocal, and tells channeld to sendshutdownto the channel's close address. Likeclose, it doesn't send a script the peer won't accept: our default P2TR address becomes P2WPKH for a peer withoutoption_shutdown_anysegwit, and a script promised upfront that the peer can't take is logged and left alone. If channeld isn't running, it sendsshutdownwhen it next starts, as for any channel in that state.shutdownthen. The cap logs that it will close once the splice locks, andhandle_peer_splice_locked()starts the close at the lock. If the splice is aborted instead,handle_splice_abort()checks the cap again once channeld restarts (commit 17, tested in commit 25). An aborted RBF attempt only drops that attempt, so the close then keeps waiting for the splice it would have replaced. The backstop below still applies meanwhile, and is all that bounds a splice that never confirms.peer_got_revoke()starts the close before it replies to channeld, so channeld, which waits for that reply, queues the shutdown request and handles it before anything else the peer sends. channeld then stops splicing once either side has sentshutdown, or it has been asked to send one: asplice_initortx_init_rbffrom the peer gets a warning and the connection is closed, as BOLT 2 asks of a node that has receivedshutdown(we reestablish with ours); a localsplice_initis refused; and a local one still waiting for quiescence is cancelled the same way. The backstop, which can free the channel, still runs last inpeer_got_revoke().shutdown, the peer only needs as many more commitments as it takes to settle the HTLCs in flight. But it can keep a closing channel updating withupdate_feewhile HTLCs remain, so once it has made us store 10% more than the cap (at least one more),channel_fail_permanent()closes the channel unilaterally. Storage stays bounded either way.channel_fail_permanent()closes the channel unilaterally, asclosedoes when its default timeout runs out. At startup the timeout is armed again only for a close we started (its last state change toCHANNELD_SHUTTING_DOWNhas our reason), with whatever is left of its time since then, but at least a tenth of the timeout, up to 10 minutes (so 10 minutes by default), so the peer can reconnect and finish even if we were down longer than the timeout; a clock that has gone back counts as no time (commit 23). A close the user or the peer started is left alone.independent_secrets_close_at); before then (a lease, a pending splice, a close someone else started, or one that couldn't start), from where we first found it at the cap. So a cap lowered below what a channel already stores never fails the channel at its next update, a restart doesn't give the peer another 10%, and a close that starts once a lease expires or a splice locks gets its own 10% for the HTLCs in flight: at most about 1.2 times the cap in all. A cap raised since counts from itself.check_independent_secrets_cap()runs last inpeer_got_revoke(), after the channel is saved and thecommitment_revocationhook is called: at the backstop, failing a channel whose funding we withheld forgets it, which frees it (commit 22).close, it keeps a lease we sold: a channel the peer leased from us isn't closed mutually before the lease expires, but the 10% backstop still applies, since our funds stay locked until the lease expires however the channel closes. (Commit 12; before it, a peer could make us store without limit until expiry.)Dev sender (
hsmd,lightningd)--dev-independent-secrets-sender(developer mode only, sent inhsmd_dev_preinit), hsmd derives this node's secrets asHMAC-SHA256(shaseed, "independent-per-commitment-secret" || n || counter), the same function as Lightning Fork's dev sender. It applies to every channel, so it is only for testing against peers that support the option.fundchannel_startandopenchannel_initrefuse a type without it, beforecheckwould return (commit 24), and so do both opening hooks' offers.--dev-independent-secrets-any-channellifts that, for the test that shows such a channel failing: the peer refuses our second secret (the first always fits an empty shachain).--independent-secrets, since only a peer with the add-on accepts its secrets.Bindings
cln-rpcandcln-grpc, so it is added to all three. Without that, Rust clients and plugins would fail to deserialize a channel with the add-on.listconfigsgainsindependent-secretsandmax-independent-secrets, andlistpeerchannelsgainsindependent_secrets_stored..msggen.json,node.proto,model.rs,convert.rsandgrpc2py.py.*_pb2.pyfiles were generated with protoc and put in the layout of the pinned grpcio-tools 1.75.1 (protobuf 6.31.1). Applied to the previous.protofiles, the same method reproduces the committed*_pb2.pyfiles byte for byte.Prior work
This implements the "remove shachain" option that ZmnSCPxj first proposed on Delving Bitcoin as
no_more_shachains(Option 2A in his September summary, https://delvingbitcoin.org/t/towards-a-k-of-n-lightning-network-node/2395), negotiated per channel throughchannel_typerather than with node-level bits.His version has an odd bit meaning "I still send shachain secrets", so the other side can keep them compactly. Here the receiver finds that out for itself, from whether each secret still fits its shachain, so no bit is needed. The lnd PR covers the other approaches.
Review
A first review found no defect that could lose funds, and nothing that breaks the protocol or the database. Its findings, all addressed:
hsmd_wire.csvchanged without a line incommon/hsm_version.h, whichcheck-hsm-versionswould catch. Added.openchannelmarkers, and pass both ways.Left as is: a caller who names a channel type gets the add-on whenever both peers support it, with no per-channel opt-out, as with
option_unified_sigs.One design change came after the review. The first version stored every secret in the table and left the shachain empty on these channels.
test_emergencyrecoverpenaltytxnshowed what that cost: a static channel backup carries the shachain, so a node restored from one could no longer punish any old state on such a channel. With the add-on on by default, as it was then, that would have been every new channel, including all those with peers that use a shachain. Secrets now stay in the shachain while they fit, which limits the loss to peers that really do send independent secrets. The spec PR was amended to allow this: the receiver MAY keep secrets in the compact form for as long as each one fits, and MUST store them individually from the first that doesn't.A second review then went through that design: the switch-over, the boundaries between the shachain's numbers and the table's, crashes mid-revocation, onchaind restarts and replays, pruning, and the backup path. It found no defect that could lose funds. Its low-severity notes are either documented limits (backups, storage growth) or behave as before this PR (a revocation still queued when the funding spend is seen). One more: a duplicate row would stop lightningd with a database error rather than fail the channel, but nothing a peer sends can cause one.
Maintainer review
Kyle's review found that a sold lease skipped the cap's backstop, leaving storage unbounded until the lease expired, and that the cap's mutual close needed a timeout. Commit 12 fixes both, and also counts a lowered cap's 10% from where the channel was. It comes with three new tests (lowered cap, close timeout, lease). Our own review had flagged the same two points.
His second review confirmed both and found that a restart reset the 10% and the timeout, and re-armed the timeout for closes we didn't start. Commit 13 saves the count at which our close started and arms the timeout only for our close, with the time it has left. It comes with two more tests (restart, a close we didn't start).
Paul, reviewing alongside the Lightning Fork side, found that nothing tested a breach on a splice's funding output with stored secrets, which the Lightning Fork side had needed a fix for, and that the cap's close could send
shutdownwhile a splice wasn't locked. Commit 16 adds the test (a revoked commitment on the new funding, broadcast while the splice is pending and after it locked; both are punished, since secrets are stored by commitment number), and commit 15 holds the close until the splice locks, with a test that fails without it.His second pass found five smaller points, all addressed: a splice aborted at the cap now starts the close at once (commit 17); a limit below the spec's recommended minimum is logged (commit 18); the dev sender keeps to channels with the add-on, as the spec requires (commit 19); the spliced-breach test checks the funding output and the amount (commit 20); and the docs cover when the backstop closes without a mutual close, the splice wait, 266 in
init, and the next release's name (commit 21).His third pass confirmed those, and found that v26.06.9-blake2b.9 was tagged without this PR: "added in" now says v26.06.9-blake2b.10 (commit 26), and the branch is rebased onto .9. Of his optional points, the docs now say that an aborted RBF attempt keeps the close waiting for the splice under it (commit 26), the re-check after an aborted splice has a test of its own (commit 25), and
check fundchannel_startreports the dev sender's refusal (commit 24).His fourth pass found nothing more to fix in commits 22 to 26, and checked each fix by reverting it: its test fails. It had two small notes, both in commit 28: the comment on the
independent_secrets_close_atmigration now says where the 10% counts from, and the docs say that the timeout of a close already started stays if the limit is then set to 0, asclose's does.Commit 14 comes from the spec review on lightning-blake2b/bolts#17, which found that the two implementations treated 266 in
initdifferently. The spec's storage limit rules are unchanged for this PR: the default cap (1,000,000) is the minimum the spec now recommends, and the 10% backstop is the margin it asks for revocations our own updates cause.Security review
A further security review then read the whole series, as an attacker would. It found nothing that could lose funds, and confirmed the penalty path, secret receipt, reestablish, the database and the dev sender. It found:
channel_fail_permanent()on a channel whose funding we withheld (a zeroconf channel opened withwithhold=true) forgets it, which frees it, andpeer_got_revoke()went on to save it. Under valgrind the new test reports invalid reads and lightningd crashes; with the fix it's clean, and the channel stays forgotten across a restart.max-independent-secretsplus 10% rows in one statement; the cap is per channel, so a peer with many channels can make us store it on each (documented); and the external-signer check only sees an hsmd given with--subdaemon.A second security review then went over the whole series again, in two parts (secret storage, reestablish, punishment and the database; and the cap, negotiation, options and the dev sender), with tests of its own. The first part found nothing to fix: no way to lose funds, crash lightningd, corrupt the database, desync the count or get a bad secret accepted, including through a reorg that swaps one revoked commitment for another (tried in a test), and three claims in this description that were broader than the code (now corrected). The second found:
stfuandsplice_initright after therevoke_and_ackreaching the cap got its splice accepted: channeld learned of the shutdown only afterwards, held it while quiescent, and sent it once the splice was signed but unlocked, which BOLT 2 forbids. lightningd went fromCHANNELD_SHUTTING_DOWNback toCHANNELD_AWAITING_SPLICE, the mutual close spent the old funding output and conflicted with the splice, and the channel stayed inCLOSINGD_COMPLETEuntil a restart. No funds were at risk: a node in that state still sees a commitment spending the splice's output and handles it. Parts of this are older than this PR: see "Older splice issues" at the end.Maintainer decisions
After the first round, the fork's maintainers decided three design points, all in this version:
--independent-secretsto turn it on.--subdaemon.independent_secrets_storedinlistpeerchannels, and--max-independent-secrets, which closes a channel at the cap.Tests
After the bits moved from 516/517 to 264/265,
check-unitsand the pytest files that check feature bits and channel types were rerun (test_independent_secrets,test_misc,test_opening,test_plugin,test_connection,test_gossip,test_splicing,test_closing: 578 passed), along with all nine interop scenarios. Every failure was one of the environment-only ones listed below, apart fromtest_htlc_rexmit_while_closing, which failed once under load and passed on rerun, and the timing testrun-secret_eq_consttime, which passes when not run alongside the rest. The full-suite numbers below are from the run with 516/517: only constants, test expectations and docs changed since. The bits then moved once more, to 266/267, because an open bLIP PR already claims 264/265. That again changed only constants, test expectations and docs (8 files).The branch was then rebased, without conflicts, onto the fork's
blake2b-unifiedat1dd56e94d: the v26.06.9-blake2b.7 release (482c8c521) plus the fork's new test vectors and a doc change. On that base, with the bits at 266/267, it builds, and these were rerun: therun-channel_type,run-walletandrun-grind_feerateunit tests;test_independent_secrets.pyand the feature-bit and channel-type tests intest_misc,test_opening,test_plugin,test_connectionandtest_splicing(24 passed); and all nine interop scenarios.After the maintainers' decisions (off by default, refused with an external signer, the count and the cap), and a further review of that code whose findings are all addressed (the option on Liquid, an external signer with existing add-on channels, the backstop for a peer that keeps a closing channel updating, the close address and states, the warning), the branch was rebased, without conflicts, onto
blake2b-unifiedat13391ab17. On that base every commit builds and passes therun-channel_type,run-wallet,run-invoice-select-inchanandrun-grind_feerateunit tests, and these pass with the final code:test_independent_secrets.py, all oftest_splicing.py, and the reestablish, backup-recovery (with and without the add-on), feature-bit and channel-type tests intest_closing,test_connection,test_security_backport,test_misc,test_openingandtest_plugin(55 passed);test_independent_secrets.pyagain with dual-funded opens (all passed, the v1-only tests skipped); the penalty, onchain and closing tests intest_closing.py(61 passed, 1 skipped);test_independent_secrets.pyas it stands at each commit that changes it (the tests, the count and the cap commits); and all nine interop scenarios, with CLN started with--independent-secrets.The branch was then rebased again, without conflicts, onto
blake2b-unifiedat7974591c7(the fork's splice shared-input signature check, coinbase-input refusal, reserve and xpay fixes). On that base it builds and these pass again: the four unit tests above;test_independent_secrets.py, all oftest_splicing.pyandtest_splicing_disconnect.py, and the backup-recovery, feature-bit, channel-type and reestablish tests (58 passed);test_independent_secrets.pywith dual-funded opens (13 passed, the v1-only tests skipped); and all nine interop scenarios against Lightning Fork 0.21.4-beta-blake2b.20.Commits 12 and 13 (the review fixes) build and pass the four unit tests above and
run-db, andtest_independent_secrets.pypasses in full both ways: 21 passed with v1 opens (the lease test runs with dual-funded opens only), and 14 passed with dual-funded opens (the v1-only tests skipped), including the five new cap tests.Commits 14 to 16 come from the spec review and Paul's review. With them, the branch was rebased onto
blake2b-unifiedate48ea5bad. Three files conflicted, all because the base added things where ours go: a migration (ours now come after it), an hsmd wire message (thehsm_version.hline for the dev sender has the new hash, andcheck-hsm-versionspasses), and a test that rewinds the database by one migration (test_splice_sent_sigs_marked_on_upgrade, which now drops our table and column and rewinds by three, astest_splice_stored_feerate_repaired_on_upgradenow does by six). On that base:run-wallet,run-db,run-channel_typeandrun-invoice-select-inchanpass, and so doescheck-hsm-versions.test_independent_secrets.pypasses both ways: 26 passed with v1 opens (the lease test runs with dual-funded opens only), and 17 with dual-funded opens (the v1-only tests skipped). The new tests for commits 14 and 15 fail without them.test_splicing.pypasses (61 passed), including both tests that rewind the database.9b0d0b3.Commits 17 to 21 come from Paul's second pass. On the same base, every commit builds, and with all of them:
check-unitspasses in full.test_independent_secrets.pypasses both ways: 27 passed with v1 opens, and 18 with dual-funded opens. The new tests for commits 18 and 19 fail without them.test_splicing.pypasses (61 passed), including its splice-abort tests.blake2b(the same code as9b0d0b3).Commits 22 and 23 come from the security review. With them:
check-unitsandcheck-hsm-versionspass;test_independent_secrets.pypasses both ways (31 passed with v1 opens, 19 with dual-funded opens); all oftest_splicing.pypasses (61 passed); the seven cap tests pass under valgrind with no errors; and all nine interop scenarios pass. The new tests for commits 22 and 23 fail without them (commit 22's under valgrind).Commits 24 to 26 come from Paul's third pass. With them, the branch was rebased onto
blake2b-unifiedatbf4e4ea6b(the v26.06.9-blake2b.9 release) without conflicts; the base's only change sincee48ea5badis a splice restart fix inpeer_control.candtest_splicing.py. On that base every commit from 22 on builds, and with all of them:check-unitsandcheck-hsm-versionspass.test_independent_secrets.pypasses both ways: 33 passed with v1 opens, and 21 with dual-funded opens.test_splicing.pypasses (62 passed).blake2batb13cb18(0.21.4-beta-blake2b.21, with feature, routing: treat 266 in a node announcement as unknown paulscode/lightning-fork#13).openchannel_initalready refused beforecheck).Commit 27 comes from the second security review. On the same base, with it:
check-unitspasses.test_independent_secrets.pypasses both ways: 34 passed with v1 opens, and 22 with dual-funded opens.test_splicing.pyandtest_splicing_disconnect.pypass (65 passed), since commit 27 touches channeld's splice code.blake2batb13cb18.New (
tests/test_independent_secrets.py; most run with v1 and v2 opens):initis refused, with and without the option; one setting 267 connects.--dev-independent-secrets-sendera node won't open a channel with a peer that doesn't offer 267 (andcheckof the opening command says so), nor accept one without the add-on, and opens one with a peer that has it.to_local, which matches its balance less the commitment's fee and anchors. Reaching the cap while a splice is pending sends noshutdown; the close starts once the splice locks.--independent-secrets, not without; added to a type the caller names; refused by a node that doesn't offer it; bit 267 ininitandnode_announcementonly with the option, and inlistconfigs.--independent-secretsand--subdaemon=hsmd:..., or with the dev sender but without--independent-secrets. Replacing hsmd without the option still works, but not while a channel with the add-on is open.--independent-secrets, keeps storing the peer's secrets and punishes a revoked commitment with one; the node no longer signals bit 267.test_emergencyrecoverpenaltytxnalso runs with the add-on: a peer that uses a shachain is still punished from a restored backup.independent_secrets_storedmatches the rows stored, across restarts; it is 0 with a shachain peer and absent without the add-on.max-independent-secrets=20, the startup warning says the limit is below 1,000,000 (a node at the default doesn't log it); the 90% warning comes once, at 18 stored secrets, with the channel still normal, and the 20th starts a mutual close while a payment is in flight. The payment completes (one more secret, within the slack), the close confirms, andstate_changesrecords the reason.max-independent-secrets=10the slack is one secret, so settling the payment in flight when the close starts is one too many: the channel is closed unilaterally, with its reason instate_changes.withhold=trueand a peer using the dev sender reaches the backstop; the channel is forgotten, stays forgotten across a restart, and valgrind reports nothing.shutdownand doesn't come back; after the timeout (10 seconds in the test, through a developer option) we close unilaterally, with its reason instate_changes.your_last_per_commitment_secretis the one it revealed.--dev-independent-secrets-any-channel, against a node without the add-on, has its second secret refused (Bad per_commitment_secret ... for 1). This shows the other tests really give the receiver secrets no shachain would accept.Also new:
run-wallet: the shachain taking secret 0 and refusing secret 1; storing, loading, looking up, pruning on close, the closed-channel loader, and the guarded revert. Eight mutations of that code (wrong number stored, wrong secret looked up, count off by one, either loader skipped, pruning everything or nothing, the revert unguarded) are each caught.run-channel_type: names and acceptance.Interop with Lightning Fork, on a Bitcoin Knots BLAKE2b regtest: nine scenarios, each on a fresh chain, all passing on v26.06.9-blake2b.7 and again on
blake2b-unifiedat 13391ab, 7974591, e48ea5b and bf4e4ea with the bits at 266/267 (scriptinterop.pyand its pluginhold.py, in https://github.com/davelund110/independent-secrets-interop).lnd sends KDF secrets, CLN keeps them. This needs lnd's integration build, the only one with its dev sender. That build never signs with the unified sighash, so CLN ran with
--dev-force-features=-515: it requiresoption_unified_sigson new channels while it offers it. Channels opened as[12, 22, 266](static remote key, anchors, independent secrets).CLN sends KDF secrets, lnd keeps them. These use a release build of lnd and an unmodified CLN, so the channels also had
option_unified_sigs:[12, 22, 266, 514].your_last_per_commitment_secret, and keep paying. CLN's table stays empty, since lnd's own secrets come from its shachain. CLN broadcasts revoked state 13, and lnd's breach arbiter takes everything: it ends with 990,347 of the 1,000,000 sat.A peer that uses a shachain. shachain-peer: a release lnd sends ordinary secrets. CLN stores none, and punishes revoked state 12 from its shachain.
A pre-existing bug the interop found.
onchaind_tx_unsigned()leaves the witness's stack element (a revocation pubkey or a preimage) out of a spend's weight. When feerate estimates are at the floor, as on a regtest without any, an HTLC penalty or fulfill then pays less than the minimum relay fee, and bitcoind refuses it for as long as the estimates stay there. Upstream Core Lightning has the same line. It isn't part of this PR: A separate PR (#49) fixes it, with a test that fails without it.Existing tests: in the earlier version, with the add-on on by default, every CLN-to-CLN channel in the suite used it. Their secrets stayed in the shachain, so the existing penalty, onchain, reestablish and backup tests checked that nothing changed for shachain peers. The whole pytest suite (all 36 files) was run with that code: 1,070 passed. Now that it is off by default, those tests run as they do without this PR; the ones the earlier version had to adjust for the extra bit are back to upstream's.
test_closing.pyandtest_connection.py: 196 passed.test_opening.py,test_misc.py,test_plugin.py,test_splicing.pyand the new tests: 321 passed, includingtest_emergencyrecoverpenaltytxn, which punishes from a restored backup.test_currencyrate.py, thetest_bookkeeper.pyreports that use currency rates,test_reckless.py,test_announce_and_connect_via_dns), modules or Rust plugins this sandbox doesn't have (test_cln_lsps.py,test_cln_rs.py,test_clnrest.py), man pages (test_cli_multiline_help), a user other than root (test_setconfig_access), or IPv6 (test_ipv4_and_ipv6, andtest_websocket's teardown).test_channel_lease_lessor_cheat/test_channel_lease_lessee_cheat(dual-funded penalties), withEXPERIMENTAL_DUAL_FUND=1.check-unitsat every commit of the series;check-source-boltfor the changed files, and the quotes from the draft bLIP against its text;check-includes,check-config-docs,check-hsm-versions, flake8 andrustfmt(edition 2024).Not run here:
cln-rpcandcln-grpcwere only checked withrustfmt, andtest_cln_rs.py, which exercises them, couldn't import.check-doc-examples. The fork hasn't regenerated the doc examples forunified_sigs/eveneither. Thelistconfigsexample gained the two new options by hand.Limits
recoverchanneloremergencyrecovercan only punish states revoked before the first secret that didn't fit. With the dev sender, that is state 0 alone.hsmd_validate_revocationis a stub; channeld does the check). A signer that keeps them in a shachain of its own, such as VLS, would refuse the first that doesn't fit, so lightningd refuses an hsmd replaced by--subdaemonwith--independent-secrets, or while a channel with the add-on is open. A signer that supports the add-on later can lift that.max-independent-secretsis closed cooperatively, or unilaterally if the peer keeps it updating past 10% more or the close hasn't finished in 48 hours. A channel the peer leased from us is only closed mutually once the lease expires, and one with a pending splice once the splice locks or is aborted (an aborted RBF attempt leaves the splice under it pending); the 10% backstop applies before then, so it can close a channel unilaterally without a mutual close having been tried. There is no limit across channels: a peer with several such channels can make us store the cap on each.lightning-downgradealready refuses wallets of this fork.Rollout
--independent-secrets. Lightning Fork, which has merged its side, ships it off by default too in its first release with it, and turns it on by default in the release after. New channels use it when both sides offer it. Channels with older nodes open as before.Release note
Not in this PR
listconfigsandlistpeerchannelsschemas and the config docs say "added in v26.06.9-blake2b.10", since .9 was tagged without this. Change that if the release is named differently.Older splice issues
Found while reproducing the splice race above. They don't depend on this PR, and it doesn't change them:
closeduring a pending splice.json_closeacceptsCHANNELD_AWAITING_SPLICE, and channeld then sendsshutdownwhile the splice is unlocked (the DTODO inmaybe_send_shutdown()), which BOLT 2 forbids.closed_inflight_splice_found()is meant to close the channel unilaterally then, butdrop_to_chain()takes the splice transaction for a unilateral close it has already seen ("Not dropping our unilateral close onchain since we already saw … confirm") and broadcasts nothing, so the channel stays inCLOSINGD_COMPLETEuntil lightningd restarts. A commitment the peer broadcasts on the splice's output is still seen and handled.splice_initdoesn't return when the peer closes the connection after oursplice.