Merge pull request #10048 from ziggie1984/fix-utxonursery-encoding

contractcourt: fix encoding

contractcourt/utxonursery.go

@@ -1514,21 +1514,70 @@ func (k *kidOutput) Encode(w io.Writer) error {
 // Decode takes a byte array representation of a kidOutput and converts it to an
 // struct. Note that the witnessFunc method isn't added during deserialization
 // and must be added later based on the value of the witnessType field.
+//
+// NOTE: We need to support both formats because we did not migrate the database
+// to the new format so the support for the legacy format is still needed.
 func (k *kidOutput) Decode(r io.Reader) error {
+	// Read all available data into a buffer first so we can try both
+	// formats.
+	//
+	// NOTE: We can consume the whole reader here because every kidOutput is
+	// saved separately via a key-value pair and we are only decoding them
+	// individually so there is no risk of reading multiple kidOutputs.
+	var buf bytes.Buffer
+	_, err := io.Copy(&buf, r)
+	if err != nil {
+		return err
+	}
+
+	data := buf.Bytes()
+	bufReader := bytes.NewReader(data)
+
+	// Try the new format first. A successful decode must consume all bytes.
+	newErr := k.decodeNewFormat(bufReader)
+	if newErr == nil && bufReader.Len() == 0 {
+		return nil
+	}
+
+	// If that fails, reset the reader and try the legacy format.
+	_, err = bufReader.Seek(0, io.SeekStart)
+	if err != nil {
+		return err
+	}
+
+	legacyErr := k.decodeLegacyFormat(bufReader)
+	if legacyErr != nil {
+		return fmt.Errorf("failed to decode with both new and "+
+			"legacy formats: new=%v, legacy=%v", newErr, legacyErr)
+	}
+
+	// The legacy format must also consume all bytes.
+	if bufReader.Len() > 0 {
+		return fmt.Errorf("legacy decode has %d trailing bytes",
+			bufReader.Len())
+	}
+
+	return nil
+}
+
+// decodeNewFormat decodes using the new format with variable-length outpoint
+// encoding.
+func (k *kidOutput) decodeNewFormat(r *bytes.Reader) error {
 	var scratch [8]byte
 
-	if _, err := r.Read(scratch[:]); err != nil {
+	if _, err := io.ReadFull(r, scratch[:]); err != nil {
 		return err
 	}
 	k.amt = btcutil.Amount(byteOrder.Uint64(scratch[:]))
 
-	err := graphdb.ReadOutpoint(io.LimitReader(r, 40), &k.outpoint)
-	if err != nil {
+	// The outpoint does use the new format without a preceding varint.
+	if err := graphdb.ReadOutpoint(r, &k.outpoint); err != nil {
 		return err
 	}
 
-	err = graphdb.ReadOutpoint(io.LimitReader(r, 40), &k.originChanPoint)
-	if err != nil {
+	// The origin chan point does use the new format without a preceding
+	// varint..
+	if err := graphdb.ReadOutpoint(r, &k.originChanPoint); err != nil {
 		return err
 	}
 
@@ -1536,22 +1585,22 @@ func (k *kidOutput) Decode(r io.Reader) error {
 		return err
 	}
 
-	if _, err := r.Read(scratch[:4]); err != nil {
+	if _, err := io.ReadFull(r, scratch[:4]); err != nil {
 		return err
 	}
 	k.blocksToMaturity = byteOrder.Uint32(scratch[:4])
 
-	if _, err := r.Read(scratch[:4]); err != nil {
+	if _, err := io.ReadFull(r, scratch[:4]); err != nil {
 		return err
 	}
 	k.absoluteMaturity = byteOrder.Uint32(scratch[:4])
 
-	if _, err := r.Read(scratch[:4]); err != nil {
+	if _, err := io.ReadFull(r, scratch[:4]); err != nil {
 		return err
 	}
 	k.confHeight = byteOrder.Uint32(scratch[:4])
 
-	if _, err := r.Read(scratch[:2]); err != nil {
+	if _, err := io.ReadFull(r, scratch[:2]); err != nil {
 		return err
 	}
 	k.witnessType = input.StandardWitnessType(byteOrder.Uint16(scratch[:2]))
@@ -1579,6 +1628,91 @@ func (k *kidOutput) Decode(r io.Reader) error {
 	return nil
 }
 
+// decodeLegacyFormat decodes using the legacy format with fixed-length outpoint
+// encoding.
+func (k *kidOutput) decodeLegacyFormat(r *bytes.Reader) error {
+	var scratch [8]byte
+
+	if _, err := io.ReadFull(r, scratch[:]); err != nil {
+		return err
+	}
+	k.amt = btcutil.Amount(byteOrder.Uint64(scratch[:]))
+
+	// Outpoint uses the legacy format with a preceding varint.
+	if err := readOutpointVarBytes(r, &k.outpoint); err != nil {
+		return err
+	}
+
+	// Origin chan point uses the legacy format with a preceding varint.
+	if err := readOutpointVarBytes(r, &k.originChanPoint); err != nil {
+		return err
+	}
+
+	if err := binary.Read(r, byteOrder, &k.isHtlc); err != nil {
+		return err
+	}
+
+	if _, err := io.ReadFull(r, scratch[:4]); err != nil {
+		return err
+	}
+	k.blocksToMaturity = byteOrder.Uint32(scratch[:4])
+
+	if _, err := io.ReadFull(r, scratch[:4]); err != nil {
+		return err
+	}
+	k.absoluteMaturity = byteOrder.Uint32(scratch[:4])
+
+	if _, err := io.ReadFull(r, scratch[:4]); err != nil {
+		return err
+	}
+	k.confHeight = byteOrder.Uint32(scratch[:4])
+
+	if _, err := io.ReadFull(r, scratch[:2]); err != nil {
+		return err
+	}
+	k.witnessType = input.StandardWitnessType(byteOrder.Uint16(scratch[:2]))
+
+	if err := input.ReadSignDescriptor(r, &k.signDesc); err != nil {
+		return err
+	}
+
+	// If there's anything left in the reader, then this is a taproot
+	// output that also wrote a control block.
+	ctrlBlock, err := wire.ReadVarBytes(r, 0, 1000, "control block")
+	switch {
+	// If there're no bytes remaining, then we'll return early.
+	case errors.Is(err, io.EOF):
+		fallthrough
+	case errors.Is(err, io.ErrUnexpectedEOF):
+		return nil
+
+	case err != nil:
+		return err
+	}
+
+	k.signDesc.ControlBlock = ctrlBlock
+
+	return nil
+}
+
+// readOutpointVarBytes reads an outpoint using the variable-length encoding.
+func readOutpointVarBytes(r io.Reader, o *wire.OutPoint) error {
+	scratch := make([]byte, 4)
+
+	txid, err := wire.ReadVarBytes(r, 0, 32, "prevout")
+	if err != nil {
+		return err
+	}
+	copy(o.Hash[:], txid)
+
+	if _, err := r.Read(scratch); err != nil {
+		return err
+	}
+	o.Index = byteOrder.Uint32(scratch)
+
+	return nil
+}
+
 // Compile-time constraint to ensure kidOutput implements the
 // Input interface.
 

contractcourt/utxonursery_test.go

@@ -2,7 +2,9 @@ package contractcourt
 
 import (
 	"bytes"
+	"encoding/binary"
 	"fmt"
+	"io"
 	"math"
 	"os"
 	"reflect"
@@ -1113,3 +1115,150 @@ func (s *mockSweeperFull) sweepAll() {
 		}
 	}
 }
+
+// writeOutpointVarBytes writes an outpoint using the variable-length encoding.
+func writeOutpointVarBytes(w io.Writer, o *wire.OutPoint) error {
+	if err := wire.WriteVarBytes(w, 0, o.Hash[:]); err != nil {
+		return err
+	}
+
+	var scratch [4]byte
+	byteOrder.PutUint32(scratch[:], o.Index)
+	_, err := w.Write(scratch[:])
+
+	return err
+}
+
+// encodeKidOutputLegacy encodes a kidOutput using the legacy format.
+func encodeKidOutputLegacy(w io.Writer, k *kidOutput) error {
+	var scratch [8]byte
+	byteOrder.PutUint64(scratch[:], uint64(k.Amount()))
+	if _, err := w.Write(scratch[:]); err != nil {
+		return err
+	}
+
+	op := k.OutPoint()
+	if err := writeOutpointVarBytes(w, &op); err != nil {
+		return err
+	}
+	if err := writeOutpointVarBytes(w, k.OriginChanPoint()); err != nil {
+		return err
+	}
+
+	if err := binary.Write(w, byteOrder, k.isHtlc); err != nil {
+		return err
+	}
+
+	byteOrder.PutUint32(scratch[:4], k.BlocksToMaturity())
+	if _, err := w.Write(scratch[:4]); err != nil {
+		return err
+	}
+
+	byteOrder.PutUint32(scratch[:4], k.absoluteMaturity)
+	if _, err := w.Write(scratch[:4]); err != nil {
+		return err
+	}
+
+	byteOrder.PutUint32(scratch[:4], k.ConfHeight())
+	if _, err := w.Write(scratch[:4]); err != nil {
+		return err
+	}
+
+	byteOrder.PutUint16(scratch[:2], uint16(k.witnessType))
+	if _, err := w.Write(scratch[:2]); err != nil {
+		return err
+	}
+
+	if err := input.WriteSignDescriptor(w, k.SignDesc()); err != nil {
+		return err
+	}
+
+	if k.SignDesc().ControlBlock == nil {
+		return nil
+	}
+
+	return wire.WriteVarBytes(w, 1000, k.SignDesc().ControlBlock)
+}
+
+// TestKidOutputDecode tests that we can decode a kidOutput from both the
+// new and legacy formats. It also checks that the decoded output matches the
+// original output, except for the deadlineHeight field, which is not encoded
+// in the legacy format.
+func TestKidOutputDecode(t *testing.T) {
+	t.Parallel()
+
+	op := wire.OutPoint{
+		Hash:  chainhash.Hash{1},
+		Index: 1,
+	}
+	originOp := wire.OutPoint{
+		Hash:  chainhash.Hash{2},
+		Index: 2,
+	}
+	pkScript := []byte{
+		0x00, 0x14, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
+		0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12,
+		0x13, 0x14,
+	}
+	signDesc := &input.SignDescriptor{
+		Output: &wire.TxOut{
+			Value:    12345,
+			PkScript: pkScript,
+		},
+		HashType:      txscript.SigHashAll,
+		WitnessScript: []byte{},
+	}
+
+	// Since makeKidOutput is not exported, we construct the kid output
+	// manually.
+	kid := kidOutput{
+		breachedOutput: breachedOutput{
+			amt:         btcutil.Amount(signDesc.Output.Value),
+			outpoint:    op,
+			witnessType: input.CommitmentRevoke,
+			signDesc:    *signDesc,
+			confHeight:  100,
+		},
+		originChanPoint:  originOp,
+		blocksToMaturity: 144,
+		isHtlc:           false,
+		absoluteMaturity: 0,
+	}
+
+	// Encode the kid output in both formats.
+	var newBuf bytes.Buffer
+	err := kid.Encode(&newBuf)
+	require.NoError(t, err)
+
+	var legacyBuf bytes.Buffer
+	err = encodeKidOutputLegacy(&legacyBuf, &kid)
+	require.NoError(t, err)
+
+	testCases := []struct {
+		name string
+		data []byte
+	}{
+		{
+			name: "new format",
+			data: newBuf.Bytes(),
+		},
+		{
+			name: "legacy format",
+			data: legacyBuf.Bytes(),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			var decodedKid kidOutput
+			err := decodedKid.Decode(bytes.NewReader(tc.data))
+			require.NoError(t, err)
+
+			// The deadlineHeight field is not encoded, so we need
+			// to set it manually for the comparison.
+			kid.deadlineHeight = decodedKid.deadlineHeight
+
+			require.Equal(t, kid, decodedKid)
+		})
+	}
+}

docs/release-notes/release-notes-0.19.2.md

@@ -42,6 +42,10 @@
 - Fixed a [case](https://github.com/lightningnetwork/lnd/pull/10045) that a
   panic may happen which prevents the node from starting up.
 
+- Fixed a [case](https://github.com/lightningnetwork/lnd/pull/10048) where we
+  would not be able to decode persisted data in the utxo nursery and therefore
+  would fail to start up.
+
 # New Features
 
 ## Functional Enhancements