Merge pull request #3561 from Roasbeef/chan-validation-cleanup

chanvalidate: create new channel validation package

lnwallet/channel.go

@@ -4947,7 +4947,7 @@ func (lc *LightningChannel) getSignedCommitTx() (*wire.MsgTx, error) {
 	// Fetch the current commitment transaction, along with their signature
 	// for the transaction.
 	localCommit := lc.channelState.LocalCommitment
-	commitTx := localCommit.CommitTx
+	commitTx := localCommit.CommitTx.Copy()
 	theirSig := append(localCommit.CommitSig, byte(txscript.SigHashAll))
 
 	// With this, we then generate the full witness so the caller can

lnwallet/chanvalidate/validate.go

@@ -0,0 +1,205 @@
+package chanvalidate
+
+import (
+	"bytes"
+	"fmt"
+
+	"github.com/btcsuite/btcd/txscript"
+	"github.com/btcsuite/btcd/wire"
+	"github.com/btcsuite/btcutil"
+	"github.com/lightningnetwork/lnd/lnwire"
+)
+
+var (
+	// ErrInvalidOutPoint is returned when the ChanLocator is unable to
+	// find the target outpoint.
+	ErrInvalidOutPoint = fmt.Errorf("output meant to create channel cannot " +
+		"be found")
+
+	// ErrWrongPkScript is returned when the alleged funding transaction is
+	// found to have an incorrect pkSript.
+	ErrWrongPkScript = fmt.Errorf("wrong pk script")
+
+	// ErrInvalidSize is returned when the alleged funding transaction
+	// output has the wrong size (channel capacity).
+	ErrInvalidSize = fmt.Errorf("channel has wrong size")
+)
+
+// ErrScriptValidateError is returned when Script VM validation fails for an
+// alleged channel output.
+type ErrScriptValidateError struct {
+	err error
+}
+
+// Error returns a human readable string describing the error.
+func (e *ErrScriptValidateError) Error() string {
+	return fmt.Sprintf("script validation failed: %v", e.err)
+}
+
+// Unwrap returns the underlying wrapped VM execution failure error.
+func (e *ErrScriptValidateError) Unwrap() error {
+	return e.err
+}
+
+// ChanLocator abstracts away obtaining the output that created the channel, as
+// well as validating its existence given the funding transaction.  We need
+// this as there are several ways (outpoint, short chan ID) to identify the
+// output of a channel given the funding transaction.
+type ChanLocator interface {
+	// Locate attempts to locate the funding output within the funding
+	// transaction. It also returns the final out point of the channel
+	// which uniquely identifies the output which creates the channel. If
+	// the target output cannot be found, or cannot exist on the funding
+	// transaction, then an error is to be returned.
+	Locate(*wire.MsgTx) (*wire.TxOut, *wire.OutPoint, error)
+}
+
+// OutPointChanLocator is an implementation of the ChanLocator that can be used
+// when one already knows the expected chan point.
+type OutPointChanLocator struct {
+	// ChanPoint is the expected chan point.
+	ChanPoint wire.OutPoint
+}
+
+// Locate attempts to locate the funding output within the passed funding
+// transaction.
+//
+// NOTE: Part of the ChanLocator interface.
+func (o *OutPointChanLocator) Locate(fundingTx *wire.MsgTx) (
+	*wire.TxOut, *wire.OutPoint, error) {
+
+	// If the expected index is greater than the amount of output in the
+	// transaction, then we'll reject this channel as it's invalid.
+	if int(o.ChanPoint.Index) >= len(fundingTx.TxOut) {
+		return nil, nil, ErrInvalidOutPoint
+	}
+
+	// As an extra sanity check, we'll also ensure the txid hash matches.
+	fundingHash := fundingTx.TxHash()
+	if !bytes.Equal(fundingHash[:], o.ChanPoint.Hash[:]) {
+		return nil, nil, ErrInvalidOutPoint
+	}
+
+	return fundingTx.TxOut[o.ChanPoint.Index], &o.ChanPoint, nil
+}
+
+// ShortChanIDChanLocator is an implementation of the ChanLocator that can be
+// used when one only knows the short channel ID of a channel. This should be
+// used in contexts when one is verifying a 3rd party channel.
+type ShortChanIDChanLocator struct {
+	// ID is the short channel ID of the target channel.
+	ID lnwire.ShortChannelID
+}
+
+// Locate attempts to locate the funding output within the passed funding
+// transaction.
+//
+// NOTE: Part of the ChanLocator interface.
+func (s *ShortChanIDChanLocator) Locate(fundingTx *wire.MsgTx) (
+	*wire.TxOut, *wire.OutPoint, error) {
+
+	// If the expected index is greater than the amount of output in the
+	// transaction, then we'll reject this channel as it's invalid.
+	outputIndex := s.ID.TxPosition
+	if int(outputIndex) >= len(fundingTx.TxOut) {
+		return nil, nil, ErrInvalidOutPoint
+	}
+
+	chanPoint := wire.OutPoint{
+		Hash:  fundingTx.TxHash(),
+		Index: uint32(outputIndex),
+	}
+
+	return fundingTx.TxOut[outputIndex], &chanPoint, nil
+}
+
+// CommitmentContext is optional validation context that can be passed into the
+// main Validate for self-owned channel. The information in this context allows
+// us to fully verify out initial commitment spend based on the on-chain state
+// of the funding output.
+type CommitmentContext struct {
+	// Value is the known size of the channel.
+	Value btcutil.Amount
+
+	// FullySignedCommitTx is the fully signed commitment transaction. This
+	// should include a valid witness.
+	FullySignedCommitTx *wire.MsgTx
+}
+
+// Context is the main validation contxet. For a given channel, all fields but
+// the optional CommitCtx should be populated based on existing
+// known-to-be-valid parameters.
+type Context struct {
+	// Locator is a concrete implementation of the ChanLocator interface.
+	Locator ChanLocator
+
+	// MultiSigPkScript is the fully serialized witness script of the
+	// multi-sig output. This is the final witness program that should be
+	// found in the funding output.
+	MultiSigPkScript []byte
+
+	// FundingTx is channel funding transaction as found confirmed in the
+	// chain.
+	FundingTx *wire.MsgTx
+
+	// CommitCtx is an optional additional set of validation context
+	// required to validate a self-owned channel. If present, then a full
+	// Script VM validation will be performed.
+	CommitCtx *CommitmentContext
+}
+
+// Validate given the specified context, this function validates that the
+// alleged channel is well formed, and spendable (if the optional CommitCtx is
+// specified).  If this method returns an error, then the alleged channel is
+// invalid and should be abandoned immediately.
+func Validate(ctx *Context) (*wire.OutPoint, error) {
+	// First, we'll attempt to locate the target outpoint in the funding
+	// transaction. If this returns an error, then we know that the
+	// outpoint doesn't actually exist, so we'll exit early.
+	fundingOutput, chanPoint, err := ctx.Locator.Locate(
+		ctx.FundingTx,
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	// The scripts should match up exactly, otherwise the channel is
+	// invalid.
+	fundingScript := fundingOutput.PkScript
+	if !bytes.Equal(ctx.MultiSigPkScript, fundingScript) {
+		return nil, ErrWrongPkScript
+	}
+
+	// If there's no commitment context, then we're done here as this is a
+	// 3rd party channel.
+	if ctx.CommitCtx == nil {
+		return chanPoint, nil
+	}
+
+	// Now that we know this is our channel, we'll verify the amount of the
+	// created output against our expected size of the channel.
+	fundingValue := fundingOutput.Value
+	if btcutil.Amount(fundingValue) != ctx.CommitCtx.Value {
+		return nil, ErrInvalidSize
+	}
+
+	// If we reach this point, then all other checks have succeeded, so
+	// we'll now attempt a full Script VM execution to ensure that we're
+	// able to close the channel using this initial state.
+	vm, err := txscript.NewEngine(
+		ctx.MultiSigPkScript, ctx.CommitCtx.FullySignedCommitTx,
+		0, txscript.StandardVerifyFlags, nil, nil, fundingValue,
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	// Finally, we'll attempt to verify our full spend, if this fails then
+	// the channel is definitely invalid.
+	err = vm.Execute()
+	if err != nil {
+		return nil, &ErrScriptValidateError{err: err}
+	}
+
+	return chanPoint, nil
+}

lnwallet/chanvalidate/validate_test.go

@@ -0,0 +1,307 @@
+package chanvalidate
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/btcsuite/btcd/btcec"
+	"github.com/btcsuite/btcd/chaincfg/chainhash"
+	"github.com/btcsuite/btcd/txscript"
+	"github.com/btcsuite/btcd/wire"
+	"github.com/btcsuite/btcutil"
+	"github.com/lightningnetwork/lnd/input"
+	"github.com/lightningnetwork/lnd/lnwire"
+)
+
+var (
+	aliceKey = chainhash.Hash{
+		0xb7, 0x94, 0x38, 0x5f, 0x2d, 0x1e, 0xf7, 0xab,
+		0x4d, 0x92, 0x73, 0xd1, 0x90, 0x63, 0x81, 0xb4,
+		0x4f, 0x2f, 0x6f, 0x25, 0x18, 0xa3, 0xef, 0xb9,
+		0x64, 0x49, 0x18, 0x83, 0x31, 0x98, 0x47, 0x53,
+	}
+	bobKey = chainhash.Hash{
+		0xb7, 0x94, 0x38, 0x5f, 0x2d, 0x1e, 0xf7, 0xab,
+		0x4d, 0x92, 0x73, 0xd1, 0x90, 0x63, 0x81, 0xb4,
+		0x4f, 0x2f, 0x6f, 0x25, 0x98, 0xa3, 0xef, 0xb9,
+		0x69, 0x49, 0x18, 0x83, 0x31, 0x98, 0x47, 0x53,
+	}
+
+	alicePriv, alicePub = btcec.PrivKeyFromBytes(btcec.S256(), aliceKey[:])
+	bobPriv, bobPub     = btcec.PrivKeyFromBytes(btcec.S256(), bobKey[:])
+)
+
+// channelTestCtx holds shared context that will be used in all tests cases
+// below.
+type channelTestCtx struct {
+	fundingTx *wire.MsgTx
+
+	invalidCommitTx, validCommitTx *wire.MsgTx
+
+	chanPoint wire.OutPoint
+	cid       lnwire.ShortChannelID
+
+	fundingScript []byte
+}
+
+// newChannelTestCtx creates a new channelCtx for use in the validation tests
+// below. This creates a fake funding transaction, as well as an invalid and
+// valid commitment transaction.
+func newChannelTestCtx(chanSize int64) (*channelTestCtx, error) {
+	multiSigScript, err := input.GenMultiSigScript(
+		alicePub.SerializeCompressed(), bobPub.SerializeCompressed(),
+	)
+	if err != nil {
+		return nil, err
+	}
+	pkScript, err := input.WitnessScriptHash(multiSigScript)
+	if err != nil {
+		return nil, err
+	}
+
+	fundingOutput := wire.TxOut{
+		Value:    chanSize,
+		PkScript: pkScript,
+	}
+
+	fundingTx := &wire.MsgTx{
+		TxIn: []*wire.TxIn{
+			{},
+		},
+		TxOut: []*wire.TxOut{
+			&fundingOutput,
+			{
+				Value:    9999,
+				PkScript: bytes.Repeat([]byte{'a'}, 32),
+			},
+			{
+				Value:    99999,
+				PkScript: bytes.Repeat([]byte{'b'}, 32),
+			},
+		},
+	}
+
+	fundingTxHash := fundingTx.TxHash()
+
+	commitTx := &wire.MsgTx{
+		TxIn: []*wire.TxIn{
+			{
+				PreviousOutPoint: wire.OutPoint{
+					Hash:  fundingTxHash,
+					Index: 0,
+				},
+			},
+		},
+		TxOut: []*wire.TxOut{
+			&fundingOutput,
+		},
+	}
+
+	sigHashes := txscript.NewTxSigHashes(commitTx)
+	aliceSig, err := txscript.RawTxInWitnessSignature(
+		commitTx, sigHashes, 0, chanSize,
+		multiSigScript, txscript.SigHashAll, alicePriv,
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	bobSig, err := txscript.RawTxInWitnessSignature(
+		commitTx, sigHashes, 0, chanSize,
+		multiSigScript, txscript.SigHashAll, bobPriv,
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	commitTx.TxIn[0].Witness = input.SpendMultiSig(
+		multiSigScript, alicePub.SerializeCompressed(), aliceSig,
+		bobPub.SerializeCompressed(), bobSig,
+	)
+
+	invalidCommitTx := commitTx.Copy()
+	invalidCommitTx.TxIn[0].PreviousOutPoint.Index = 2
+
+	return &channelTestCtx{
+		fundingTx:       fundingTx,
+		validCommitTx:   commitTx,
+		invalidCommitTx: invalidCommitTx,
+		chanPoint: wire.OutPoint{
+			Hash:  fundingTxHash,
+			Index: 0,
+		},
+		cid: lnwire.ShortChannelID{
+			TxPosition: 0,
+		},
+		fundingScript: pkScript,
+	}, nil
+}
+
+// TestValidate ensures that the Validate method is able to detect all cases of
+// invalid channels, and properly accept invalid channels.
+func TestValidate(t *testing.T) {
+	t.Parallel()
+
+	chanSize := int64(1000000)
+	channelCtx, err := newChannelTestCtx(chanSize)
+	if err != nil {
+		t.Fatalf("unable to make channel context: %v", err)
+	}
+
+	testCases := []struct {
+		// expectedErr is the error we expect, this should be nil if
+		// the channel is valid.
+		expectedErr error
+
+		// locator is how the Validate method should find the target
+		// outpoint.
+		locator ChanLocator
+
+		// chanPoint is the expected final out point.
+		chanPoint wire.OutPoint
+
+		// chanScript is the funding pkScript.
+		chanScript []byte
+
+		// fundingTx is the funding transaction to use in the test.
+		fundingTx *wire.MsgTx
+
+		// commitTx is the commitment transaction to use in the test,
+		// this is optional.
+		commitTx *wire.MsgTx
+
+		// expectedValue is the value of the funding transaction we
+		// should expect. This is only required if commitTx is non-nil.
+		expectedValue int64
+	}{
+		// Short chan ID channel locator, unable to find target
+		// outpoint.
+		{
+			expectedErr: ErrInvalidOutPoint,
+			locator: &ShortChanIDChanLocator{
+				ID: lnwire.NewShortChanIDFromInt(9),
+			},
+			fundingTx: &wire.MsgTx{},
+		},
+
+		// Chan point based channel locator, unable to find target
+		// outpoint.
+		{
+			expectedErr: ErrInvalidOutPoint,
+			locator: &OutPointChanLocator{
+				ChanPoint: wire.OutPoint{
+					Index: 99,
+				},
+			},
+			fundingTx: &wire.MsgTx{},
+		},
+
+		// Invalid pkScript match on mined funding transaction, chan
+		// point based locator.
+		{
+			expectedErr: ErrWrongPkScript,
+			locator: &OutPointChanLocator{
+				ChanPoint: channelCtx.chanPoint,
+			},
+			chanScript: bytes.Repeat([]byte("a"), 32),
+			fundingTx:  channelCtx.fundingTx,
+		},
+
+		// Invalid pkScript match on mined funding transaction, short
+		// chan ID based locator.
+		{
+			expectedErr: ErrWrongPkScript,
+			locator: &ShortChanIDChanLocator{
+				ID: channelCtx.cid,
+			},
+			chanScript: bytes.Repeat([]byte("a"), 32),
+			fundingTx:  channelCtx.fundingTx,
+		},
+
+		// Invalid amount on funding transaction.
+		{
+			expectedErr: ErrInvalidSize,
+			locator: &OutPointChanLocator{
+				ChanPoint: channelCtx.chanPoint,
+			},
+			chanScript:    channelCtx.fundingScript,
+			fundingTx:     channelCtx.fundingTx,
+			expectedValue: 555,
+			commitTx:      channelCtx.validCommitTx,
+		},
+
+		// Validation failure on final commitment transaction
+		{
+			expectedErr: &ErrScriptValidateError{},
+			locator: &OutPointChanLocator{
+				ChanPoint: channelCtx.chanPoint,
+			},
+			chanScript:    channelCtx.fundingScript,
+			fundingTx:     channelCtx.fundingTx,
+			expectedValue: chanSize,
+			commitTx:      channelCtx.invalidCommitTx,
+		},
+
+		// Fully valid 3rd party verification.
+		{
+			expectedErr: nil,
+			locator: &OutPointChanLocator{
+				ChanPoint: channelCtx.chanPoint,
+			},
+			chanScript: channelCtx.fundingScript,
+			fundingTx:  channelCtx.fundingTx,
+			chanPoint:  channelCtx.chanPoint,
+		},
+
+		// Fully valid self-channel verification.
+		{
+			expectedErr: nil,
+			locator: &OutPointChanLocator{
+				ChanPoint: channelCtx.chanPoint,
+			},
+			chanScript:    channelCtx.fundingScript,
+			fundingTx:     channelCtx.fundingTx,
+			expectedValue: chanSize,
+			commitTx:      channelCtx.validCommitTx,
+			chanPoint:     channelCtx.chanPoint,
+		},
+	}
+
+	for i, testCase := range testCases {
+		ctx := &Context{
+			Locator:          testCase.locator,
+			MultiSigPkScript: testCase.chanScript,
+			FundingTx:        testCase.fundingTx,
+		}
+
+		if testCase.commitTx != nil {
+			ctx.CommitCtx = &CommitmentContext{
+				Value: btcutil.Amount(
+					testCase.expectedValue,
+				),
+				FullySignedCommitTx: testCase.commitTx,
+			}
+		}
+
+		chanPoint, err := Validate(ctx)
+		if err != testCase.expectedErr {
+			_, ok := testCase.expectedErr.(*ErrScriptValidateError)
+			_, scriptErr := err.(*ErrScriptValidateError)
+			if ok && scriptErr {
+				continue
+			}
+
+			t.Fatalf("test #%v: validation failed: expected %v, "+
+				"got %v", i, testCase.expectedErr, err)
+		}
+
+		if err != nil {
+			continue
+		}
+
+		if *chanPoint != testCase.chanPoint {
+			t.Fatalf("test #%v: wrong outpoint: want %v, got %v",
+				i, testCase.chanPoint, chanPoint)
+		}
+	}
+}

lnwallet/wallet.go

@@ -21,6 +21,7 @@ import (
 	"github.com/lightningnetwork/lnd/channeldb"
 	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/keychain"
+	"github.com/lightningnetwork/lnd/lnwallet/chanvalidate"
 	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/lightningnetwork/lnd/shachain"
 )
@@ -1636,3 +1637,58 @@ func coinSelectSubtractFees(feeRate SatPerKWeight, amt,
 
 	return selectedUtxos, outputAmt, changeAmt, nil
 }
+
+// ValidateChannel will attempt to fully validate a newly mined channel, given
+// its funding transaction and existing channel state. If this method returns
+// an error, then the mined channel is invalid, and shouldn't be used.
+func (l *LightningWallet) ValidateChannel(channelState *channeldb.OpenChannel,
+	fundingTx *wire.MsgTx) error {
+
+	// First, we'll obtain a fully signed commitment transaction so we can
+	// pass into it on the chanvalidate package for verification.
+	channel, err := NewLightningChannel(l.Cfg.Signer, channelState, nil)
+	if err != nil {
+		return err
+	}
+	signedCommitTx, err := channel.getSignedCommitTx()
+	if err != nil {
+		return err
+	}
+
+	// We'll also need the multi-sig witness script itself so the
+	// chanvalidate package can check it for correctness against the
+	// funding transaction, and also commitment validity.
+	localKey := channelState.LocalChanCfg.MultiSigKey.PubKey
+	remoteKey := channelState.RemoteChanCfg.MultiSigKey.PubKey
+	witnessScript, err := input.GenMultiSigScript(
+		localKey.SerializeCompressed(),
+		remoteKey.SerializeCompressed(),
+	)
+	if err != nil {
+		return err
+	}
+	pkScript, err := input.WitnessScriptHash(witnessScript)
+	if err != nil {
+		return err
+	}
+
+	// Finally, we'll pass in all the necessary context needed to fully
+	// validate that this channel is indeed what we expect, and can be
+	// used.
+	_, err = chanvalidate.Validate(&chanvalidate.Context{
+		Locator: &chanvalidate.OutPointChanLocator{
+			ChanPoint: channelState.FundingOutpoint,
+		},
+		MultiSigPkScript: pkScript,
+		FundingTx:        fundingTx,
+		CommitCtx: &chanvalidate.CommitmentContext{
+			Value:               channel.Capacity,
+			FullySignedCommitTx: signedCommitTx,
+		},
+	})
+	if err != nil {
+		return err
+	}
+
+	return nil
+}