commit bed23d8069d89e15e2497fd52905f24a11029c50
parent ead7cd3dff620c87d31f68bd7eed91929cda213a
Author: Jared Tobin <jared@jtobin.io>
Date: Sun, 19 Apr 2026 11:45:51 +0800
lib: address review findings
- PenaltyContext.pc_outputs now uses NonEmpty, eliminating the
error partial in spend_revoked_batch
- spend_revoked_htlc returns Maybe SpendingTx instead of silently
producing a malformed tx for non-HTLC output types
- identify_close: remove 5 unused parameters and update haddock
to reflect actual behavior (byte comparison only)
- Remove unused SpendSecondStage constructor from OutputResolution
- Clean up redundant import in Spend.hs
Diffstat:
3 files changed, 56 insertions(+), 81 deletions(-)
diff --git a/lib/Lightning/Protocol/BOLT5/Detect.hs b/lib/Lightning/Protocol/BOLT5/Detect.hs
@@ -38,57 +38,32 @@ import Lightning.Protocol.BOLT5.Types
-- | Identify the type of channel close from a transaction that
-- spends the funding output.
--
--- Checks the transaction against known commitment and closing
--- formats to determine whether it's a mutual close, local
--- commitment, remote commitment, or revoked commitment.
+-- Compares the on-chain transaction bytes against the known
+-- local and remote commitment transaction serializations
+-- (stripped/unsigned) to determine whether it's a local or
+-- remote commitment close.
--
--- Returns 'Nothing' if the transaction doesn't match any known
--- format.
---
--- The 'SecretStore' is checked to determine if the remote
--- commitment is revoked (i.e. we have the per-commitment secret
--- for that commitment number).
+-- Returns 'Nothing' if the transaction doesn't match either
+-- commitment. Mutual close and revoked commitment detection
+-- require additional checks by the caller (e.g. comparing
+-- closing tx format, checking a secret store for older
+-- commitment numbers).
identify_close
- :: OutPoint
- -- ^ Funding outpoint.
- -> CommitmentKeys
- -- ^ Keys for our local commitment.
- -> CommitmentKeys
- -- ^ Keys for the remote commitment.
- -> SecretStore
- -- ^ Store of received per-commitment secrets.
- -> ClosingContext
- -- ^ Closing tx context (for mutual close matching).
- -> CommitmentTx
+ :: CommitmentTx
-- ^ Our local commitment tx.
-> CommitmentTx
-- ^ The remote commitment tx (current).
-> BS.ByteString
-- ^ Raw serialized transaction found on chain.
-> Maybe CloseType
-identify_close
- !_fundingOutpoint
- !_localKeys
- !_remoteKeys
- !_secretStore
- !_closingCtx
- !localCommitTx
- !remoteCommitTx
- !onChainBytes =
- -- Compare the on-chain tx bytes against known tx
- -- serializations. We compare stripped (unsigned, legacy)
- -- serializations.
+identify_close !localCommitTx !remoteCommitTx
+ !onChainBytes =
let !localBytes = encode_tx_for_signing localCommitTx
!remoteBytes = encode_tx_for_signing remoteCommitTx
in if localBytes == Just onChainBytes
then Just LocalCommitClose
else if remoteBytes == Just onChainBytes
then Just RemoteCommitClose
- -- For mutual close and revoked detection, the caller
- -- should perform additional checks (e.g. comparing
- -- closing tx format, checking secret store for older
- -- commitment numbers). This function provides the
- -- basic identification.
else Nothing
-- output classification ----------------------------------------------
diff --git a/lib/Lightning/Protocol/BOLT5/Spend.hs b/lib/Lightning/Protocol/BOLT5/Spend.hs
@@ -37,9 +37,10 @@ module Lightning.Protocol.BOLT5.Spend (
, spend_anchor_anyone
) where
-import Bitcoin.Prim.Tx (Tx(..), TxIn(..), TxOut(..))
+import Bitcoin.Prim.Tx (TxOut(..))
import Bitcoin.Prim.Tx.Sighash (SighashType(..))
import Data.List.NonEmpty (NonEmpty(..))
+import qualified Data.List.NonEmpty as NE
import Data.Word (Word32)
import qualified Data.ByteString as BS
import Lightning.Protocol.BOLT3 hiding
@@ -290,40 +291,44 @@ spend_revoked_htlc
-> Script
-- ^ Destination scriptPubKey.
-> FeeratePerKw
- -> SpendingTx
+ -> Maybe SpendingTx
spend_revoked_htlc !op !value !otype !revpk !keys
!features !ph !destScript !feerate =
- let !(witnessScript, inputWeight) = case otype of
- OutputOfferedHTLC _ ->
- ( offered_htlc_script
- revpk
- (ck_remote_htlc keys)
- (ck_local_htlc keys)
- ph
- features
- , offered_htlc_penalty_input_weight
- )
- OutputReceivedHTLC expiry ->
- ( received_htlc_script
- revpk
- (ck_remote_htlc keys)
- (ck_local_htlc keys)
- ph
- expiry
- features
- , accepted_htlc_penalty_input_weight
- )
- _ ->
- ( Script BS.empty
- , 0
- )
- !weight = inputWeight + penalty_tx_base_weight
- !fee = spending_fee feerate weight
- !outputValue =
- Satoshi (unSatoshi value - unSatoshi fee)
- !tx = mk_spending_tx op 0xFFFFFFFF destScript
- outputValue 0
- in SpendingTx tx witnessScript value SIGHASH_ALL
+ case otype of
+ OutputOfferedHTLC _ ->
+ let !witnessScript = offered_htlc_script
+ revpk
+ (ck_remote_htlc keys)
+ (ck_local_htlc keys)
+ ph
+ features
+ !weight = offered_htlc_penalty_input_weight
+ + penalty_tx_base_weight
+ !fee = spending_fee feerate weight
+ !outputValue =
+ Satoshi (unSatoshi value - unSatoshi fee)
+ !tx = mk_spending_tx op 0xFFFFFFFF destScript
+ outputValue 0
+ in Just (SpendingTx tx witnessScript value
+ SIGHASH_ALL)
+ OutputReceivedHTLC expiry ->
+ let !witnessScript = received_htlc_script
+ revpk
+ (ck_remote_htlc keys)
+ (ck_local_htlc keys)
+ ph
+ expiry
+ features
+ !weight = accepted_htlc_penalty_input_weight
+ + penalty_tx_base_weight
+ !fee = spending_fee feerate weight
+ !outputValue =
+ Satoshi (unSatoshi value - unSatoshi fee)
+ !tx = mk_spending_tx op 0xFFFFFFFF destScript
+ outputValue 0
+ in Just (SpendingTx tx witnessScript value
+ SIGHASH_ALL)
+ _ -> Nothing
-- | Spend a revoked second-stage HTLC output (HTLC-timeout or
-- HTLC-success output) using the revocation key.
@@ -371,18 +376,15 @@ spend_revoked_batch !ctx =
-- Calculate total input value and weight
!(totalValue, totalWeight) =
- go (Satoshi 0) penalty_tx_base_weight outs
+ go (Satoshi 0) penalty_tx_base_weight
+ (NE.toList outs)
!fee = spending_fee feerate totalWeight
!outputValue =
Satoshi (unSatoshi totalValue - unSatoshi fee)
-- Build inputs
- !inputs = map mkPenaltyInput outs
- !txInputs = case inputs of
- [] -> error
- "spend_revoked_batch: no outputs"
- (i:is) -> i :| is
+ !txInputs = fmap mkPenaltyInput outs
-- Single output
!txOutput = TxOut
diff --git a/lib/Lightning/Protocol/BOLT5/Types.hs b/lib/Lightning/Protocol/BOLT5/Types.hs
@@ -41,6 +41,7 @@ module Lightning.Protocol.BOLT5.Types (
import Bitcoin.Prim.Tx (Tx(..))
import Bitcoin.Prim.Tx.Sighash (SighashType(..))
+import Data.List.NonEmpty (NonEmpty)
import Data.Word (Word64)
import GHC.Generics (Generic)
import Lightning.Protocol.BOLT3.Types
@@ -93,9 +94,6 @@ data OutputResolution
| SpendHTLCPreimageDirect !HTLC
-- ^ Spend HTLC directly with preimage (remote commit,
-- remote offer).
- | SpendSecondStage
- !ToSelfDelay !RevocationPubkey !LocalDelayedPubkey
- -- ^ Spend second-stage HTLC output after CSV delay.
| Revoke !RevocationPubkey
-- ^ Spend revoked to_local with revocation key.
| RevokeHTLC !RevocationPubkey !OutputType
@@ -125,8 +123,8 @@ data SpendingTx = SpendingTx
-- | Context for constructing batched penalty transactions.
data PenaltyContext = PenaltyContext
- { pc_outputs :: ![UnresolvedOutput]
- -- ^ Revoked outputs to sweep.
+ { pc_outputs :: !(NonEmpty UnresolvedOutput)
+ -- ^ Revoked outputs to sweep (must be non-empty).
, pc_revocation_key :: !RevocationPubkey
-- ^ Revocation pubkey for all outputs.
, pc_destination :: !Script
