lnwallet: fix HTLC sig-to-transaction mapping in test vector generator
What the developer wrote
Authored by Olaoluwa Osuntokun
lnwallet: fix HTLC sig-to-transaction mapping in test vector generator
In this commit, we fix two interrelated bugs in the way HTLC signatures
are associated with their corresponding second-level transactions in the
taproot test vector generator.
The first issue was that HtlcSigs are sorted by BIP 69 output index
(matching the commitment transaction's output ordering), but the old
code was assigning signatures using the iteration order of incoming
HTLCs followed by outgoing HTLCs. This meant timeout transaction
signatures were getting paired with success transactions and vice versa
whenever the output ordering didn't happen to match the incoming-first
iteration order. This is the root cause of the invalid HTLC-timeout
signatures that eclair reported when cross-validating.
We now collect all HTLC entries (both incoming and outgoing) into a
single slice, sort them by their commitment output index, then zip them
against the HtlcSigs array so each signature lines up with the correct
second-level transaction.
The second issue was in the HTLC-success preimage extraction path. The
old code read the witness script from index [4] (the control block) and
used a hardcoded byte offset of 69 to locate the payment hash, then
wrote the preimage into index [3] (overwriting the script). The correct
taproot witness layout is [remoteSig, localSig, preimage, script,
controlBlock], so the script lives at [3] and the preimage slot is [2].
We now use `txscript.ScriptTokenizer` to walk the script opcodes and
find OP_HASH160 followed by the 20-byte push data, which is far more
robust than relying on fragile byte offsets that break if the script
template ever changes.
What changed, and why it matters
This commit fixes bugs in a test-data generator used by LND, not in the live Lightning node software itself. The generator produces sample HTLC (multi-hop payment) resolution transactions and signatures that other implementations use to cross-check compatibility. The old code paired signatures with the wrong transactions when HTLC outputs were not ordered 'incoming first,' and it also read preimages from the wrong witness slot using a hardcoded byte offset. These bugs caused invalid test vectors, which another implementation (Eclair) noticed during cross-validation. The fix only changes test code, so it does not create or close a real attack path against running LND nodes.
No urgent security action is required for production nodes. Reviewers should verify that the regenerated test vectors now pass cross-implementation validation, and consider regenerating and publishing updated vectors so downstream implementations can re-sync their test suites.
Security signals we found
Fixes invalid HTLC-timeout signatures reported by Eclair during cross-validation
Corrects signature-to-transaction mapping in test vector generator
Replaces fragile hardcoded witness/script byte offsets with script tokenizer parsing
Changes only test code; no live consensus or P2P code modified
Evidence from the diff
The change is confined to lnwallet/taproot_test_vectors_test.go. It corrects two bugs in the taproot test vector generator: (1) HtlcSigs are ordered by BIP 69 commitment output index, but the generator previously assigned signatures by iterating incoming HTLCs then outgoing HTLCs, causing timeout/success signature mismatches whenever output ordering differed. The fix collects all HTLCs, sorts by output index, and zips against HtlcSigs. (2) The HTLC-success preimage extraction used witness index [4] (control block) with a hardcoded offset of 69 bytes to find the payment hash, and wrote the preimage into index [3] (overwriting the script). The correct taproot witness layout is [remoteSig, localSig, preimage, script, controlBlock], so the script is at [3] and preimage slot is [2]. The fix reads the script from index [3] and uses txscript.ScriptTokenizer to locate the OP_HASH160 20-byte push, then writes the preimage into index [2].
Changed components
lnwallet/taproot_test_vectors_test.gotaproot test vector generatorHTLC second-level transaction test casesInspect captured patch +79 / −39
diff --git a/lnwallet/taproot_test_vectors_test.go b/lnwallet/taproot_test_vectors_test.go
index ecbb126..84a91ce 100644
--- a/lnwallet/taproot_test_vectors_test.go
+++ b/lnwallet/taproot_test_vectors_test.go
@@ -1001,58 +1001,68 @@ func (tc *taprootTestContext) generateTransactionVectors() []TransactionTestCase
resolutions := forceCloseSum.ContractResolutions.UnwrapOrFail(t)
htlcResolutions := resolutions.HtlcResolutions
- secondLevelTxes := map[uint32]*wire.MsgTx{}
- secondLevelSigs := map[uint32]string{}
- storeTx := func(
- index uint32, tx *wire.MsgTx, sig string,
- ) {
- secondLevelTxes[index] = tx
- secondLevelSigs[index] = sig
+ // Build a map from commitment tx output index to
+ // the second-level transaction and remote sig.
+ // HtlcSigs are sorted by output index (BIP 69),
+ // so we collect all HTLC output indices, sort
+ // them, and map each to the correct sig.
+ type htlcEntry struct {
+ outputIdx uint32
+ tx *wire.MsgTx
}
+ var allHtlcs []htlcEntry
- for i, r := range htlcResolutions.IncomingHTLCs {
+ for _, r := range htlcResolutions.IncomingHTLCs {
successTx := r.SignedSuccessTx
- // Complete the witness with the preimage.
- witnessScript := successTx.TxIn[0].Witness[4]
- var hash160 [20]byte
- copy(hash160[:], witnessScript[69:69+20])
- preimage := hash160map[hash160]
- successTx.TxIn[0].Witness[3] = preimage[:]
- sigHex := hex.EncodeToString(
- remoteNewCommit.HtlcSigs[i].ToSignatureBytes(),
- )
- storeTx(
- r.HtlcPoint().Index, successTx, sigHex,
- )
+ // Complete the witness with the preimage.
+ // Witness layout for HTLC-success:
+ // [0]=remoteSig [1]=localSig
+ // [2]=preimage [3]=script
+ // [4]=controlBlock
+ //
+ // Parse the success script to extract the
+ // RIPEMD160 hash using the script tokenizer
+ // rather than hardcoded offsets.
+ script := successTx.TxIn[0].Witness[3]
+ payHash := extractHash160FromScript(t, script)
+ preimage := hash160map[payHash]
+ successTx.TxIn[0].Witness[2] = preimage[:]
+
+ allHtlcs = append(allHtlcs, htlcEntry{
+ outputIdx: r.HtlcPoint().Index,
+ tx: successTx,
+ })
}
- for i, r := range htlcResolutions.OutgoingHTLCs {
- sigIdx := len(htlcResolutions.IncomingHTLCs) + i
- sigHex := hex.EncodeToString(
- remoteNewCommit.HtlcSigs[sigIdx].ToSignatureBytes(),
- )
- storeTx(
- r.HtlcPoint().Index,
- r.SignedTimeoutTx, sigHex,
- )
+ for _, r := range htlcResolutions.OutgoingHTLCs {
+ allHtlcs = append(allHtlcs, htlcEntry{
+ outputIdx: r.HtlcPoint().Index,
+ tx: r.SignedTimeoutTx,
+ })
}
- var keys []uint32
- for k := range secondLevelTxes {
- keys = append(keys, k)
- }
- sort.Slice(keys, func(a, b int) bool {
- return keys[a] < keys[b]
+ // Sort by output index to match HtlcSigs ordering.
+ sort.Slice(allHtlcs, func(a, b int) bool {
+ return allHtlcs[a].outputIdx < allHtlcs[b].outputIdx
})
- for _, idx := range keys {
- tx := secondLevelTxes[idx]
+ require.Equal(t,
+ len(allHtlcs),
+ len(remoteNewCommit.HtlcSigs),
+ "htlc sig count mismatch",
+ )
+
+ for i, entry := range allHtlcs {
+ sigHex := hex.EncodeToString(
+ remoteNewCommit.HtlcSigs[i].ToSignatureBytes(),
+ )
+
var b bytes.Buffer
- err := tx.Serialize(&b)
+ err := entry.tx.Serialize(&b)
require.NoError(t, err)
htlcDescs = append(htlcDescs, HtlcDesc{
- RemotePartialSigHex: secondLevelSigs[idx],
+ RemotePartialSigHex: sigHex,
ResolutionTxHex: hex.EncodeToString(b.Bytes()),
})
}
@@ -1274,3 +1284,33 @@ func verifyTaprootVectors(t *testing.T) {
})
}
+// extractHash160FromScript uses the script tokenizer to find and extract the
+// 20-byte RIPEMD160 payment hash from an HTLC success script. The script
+// contains: OP_SIZE 32 OP_EQUALVERIFY OP_HASH160 <20-byte-hash> OP_EQUALVERIFY
+// followed by checksig operations.
+func extractHash160FromScript(t *testing.T, script []byte) [20]byte {
+ t.Helper()
+
+ tokenizer := txscript.MakeScriptTokenizer(0, script)
+ for tokenizer.Next() {
+ if tokenizer.Opcode() == txscript.OP_HASH160 {
+ // The next token should be the 20-byte push data.
+ require.True(t, tokenizer.Next(),
+ "expected data push after OP_HASH160")
+
+ data := tokenizer.Data()
+ require.Len(t, data, 20,
+ "expected 20-byte hash after OP_HASH160")
+
+ var hash160 [20]byte
+ copy(hash160[:], data)
+ return hash160
+ }
+ }
+
+ require.NoError(t, tokenizer.Err(), "script tokenizer error")
+ t.Fatal("OP_HASH160 not found in script")
+
+ var zero [20]byte
+ return zero
+}
Why this scored 24/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.