[codex] require loop:out for InstantOut

liquidity/autoloop_test.go

@@ -78,7 +78,7 @@ func TestAutoLoopDisabled(t *testing.T) {
 	c.stop()
 }
 
-// TestAutoLoopEnabled tests enabling the liquidity manger's autolooper. To keep
+// TestAutoLoopEnabled tests enabling the liquidity manager's autolooper. To keep
 // the test simple, we do not update actual lnd channel balances, but rather
 // run our mock with two channels that will always require a loop out according
 // to our rules. This allows us to test the other restrictions placed on the

liquidity/liquidity.go

@@ -340,7 +340,7 @@ func (m *Manager) Run(ctx context.Context) error {
 				}
 			}
 
-			// Try to automatically dispach an asset auto-loop.
+			// Try to automatically dispatch an asset auto-loop.
 			for assetID := range m.params.AssetAutoloopParams {
 				err = m.easyAssetAutoloop(ctx, assetID)
 				if err != nil {

loopdb/postgres.go

@@ -39,7 +39,7 @@ type PostgresConfig struct {
 	RequireSSL         bool   `long:"requiressl" description:"Whether to require using SSL (mode: require) when connecting to the server."`
 }
 
-// DSN returns the dns to connect to the database.
+// DSN returns the data source name used to connect to the database.
 func (s *PostgresConfig) DSN(hidePassword bool) string {
 	var sslMode = "disable"
 	if s.RequireSSL {

loopdb/protocol_version.go

@@ -24,7 +24,7 @@ const (
 	ProtocolVersionSegwitLoopIn ProtocolVersion = 2
 
 	// ProtocolVersionPreimagePush indicates that the client will push loop
-	// out preimages to the sever to speed up claim.
+	// out preimages to the server to speed up claim.
 	ProtocolVersionPreimagePush ProtocolVersion = 3
 
 	// ProtocolVersionUserExpiryLoopOut indicates that the client will

loopout_test.go

@@ -478,7 +478,7 @@ func testCustomSweepConfTarget(t *testing.T) {
 		require.Equal(t, swap.Preimage, preimage)
 	}
 
-	// Now that we have pushed our preimage to the sever, we send an update
+	// Now that we have pushed our preimage to the server, we send an update
 	// indicating that our off chain htlc is settled. We do this so that
 	// we don't have to keep consuming preimage pushes from our server mock
 	// for every sweep attempt.
@@ -779,22 +779,22 @@ func testPreimagePush(t *testing.T) {
 	require.NoError(t, <-errChan)
 }
 
-// TestFailedOffChainCancelation tests sending of a cancelation message to
+// TestFailedOffChainCancellation tests sending of a cancellation message to
 // the server when a swap fails due to off-chain routing.
-func TestFailedOffChainCancelation(t *testing.T) {
+func TestFailedOffChainCancellation(t *testing.T) {
 	t.Run("stable protocol", func(t *testing.T) {
-		testFailedOffChainCancelation(t)
+		testFailedOffChainCancellation(t)
 	})
 
 	t.Run("experimental protocol", func(t *testing.T) {
 		loopdb.EnableExperimentalProtocol()
 		defer loopdb.ResetCurrentProtocolVersion()
 
-		testFailedOffChainCancelation(t)
+		testFailedOffChainCancellation(t)
 	})
 }
 
-func testFailedOffChainCancelation(t *testing.T) {
+func testFailedOffChainCancellation(t *testing.T) {
 	defer test.Guard(t)()
 
 	lnd := test.NewMockLnd()
@@ -873,7 +873,7 @@ func testFailedOffChainCancelation(t *testing.T) {
 					FailureSourceIndex: 1,
 				},
 			},
-			// Add one htlc that failed in the network at wide.
+			// Add one htlc that failed in the network at large.
 			{
 				Status: lnrpc.HTLCAttempt_FAILED,
 				Route: &lnrpc.Route{
@@ -892,7 +892,7 @@ func testFailedOffChainCancelation(t *testing.T) {
 		State: lnrpc.Payment_SUCCEEDED,
 	}
 
-	// We want to fail our swap payment and succeed the prepush, so we send
+	// We want to fail our swap payment and succeed the prepayment, so we send
 	// a failure update to the payment that has the larger amount.
 	if pmt1.Amount > pmt2.Amount {
 		pmt1.TrackPaymentMessage.Updates <- failUpdate
@@ -908,7 +908,7 @@ func testFailedOffChainCancelation(t *testing.T) {
 	require.NoError(t, err)
 
 	payAddr := invoice.PaymentAddr.UnwrapOrFail(t)
-	swapCancelation := &outCancelDetails{
+	swapCancellation := &outCancelDetails{
 		hash:        swap.hash,
 		paymentAddr: payAddr,
 		metadata: routeCancelMetadata{
@@ -920,7 +920,7 @@ func testFailedOffChainCancelation(t *testing.T) {
 			},
 		},
 	}
-	server.assertSwapCanceled(t, swapCancelation)
+	server.assertSwapCanceled(t, swapCancellation)
 
 	// Finally, the swap should be recorded with failed off chain timeout.
 	cfg.store.(*loopdb.StoreMock).AssertLoopOutState(

looprpc/perms.go

@@ -181,6 +181,9 @@ var RequiredPermissions = map[string][]bakery.Op{
 	"/looprpc.SwapClient/InstantOut": {{
 		Entity: "swap",
 		Action: "execute",
+	}, {
+		Entity: "loop",
+		Action: "out",
 	}},
 	"/looprpc.SwapClient/InstantOutQuote": {{
 		Entity: "swap",

looprpc/perms_test.go

@@ -0,0 +1,35 @@
+package looprpc
+
+import (
+	"testing"
+
+	"gopkg.in/macaroon-bakery.v2/bakery"
+)
+
+func TestInstantOutRequiresLoopOutPermission(t *testing.T) {
+	requiredPerms, ok := RequiredPermissions["/looprpc.SwapClient/InstantOut"]
+	if !ok {
+		t.Fatalf("InstantOut permission entry missing")
+	}
+
+	assertPermission := func(want bakery.Op) {
+		t.Helper()
+
+		for _, perm := range requiredPerms {
+			if perm == want {
+				return
+			}
+		}
+
+		t.Fatalf("InstantOut permission entry missing %v", want)
+	}
+
+	assertPermission(bakery.Op{
+		Entity: "swap",
+		Action: "execute",
+	})
+	assertPermission(bakery.Op{
+		Entity: "loop",
+		Action: "out",
+	})
+}

server_mock_test.go

@@ -6,12 +6,14 @@ import (
 	"testing"
 	"time"
 
+	"github.com/btcsuite/btcd/btcec/v2/schnorr/musig2"
 	"github.com/btcsuite/btcd/btcutil"
 	"github.com/btcsuite/btcd/chaincfg"
 	"github.com/btcsuite/btcd/wire"
 	"github.com/lightninglabs/lndclient"
 	"github.com/lightninglabs/loop/loopdb"
 	"github.com/lightninglabs/loop/test"
+	"github.com/lightningnetwork/lnd/input"
 	invpkg "github.com/lightningnetwork/lnd/invoices"
 	"github.com/lightningnetwork/lnd/lntypes"
 	"github.com/lightningnetwork/lnd/lnwire"
@@ -32,6 +34,11 @@ var (
 	testMaxSwapAmount              = btcutil.Amount(1000000)
 )
 
+func mockMuSig2SigningData() ([]byte, []byte, error) {
+	return make([]byte, musig2.PubNonceSize),
+		make([]byte, input.MuSig2PartialSigSize), nil
+}
+
 // serverMock is used in client unit tests to simulate swap server behaviour.
 type serverMock struct {
 	expectedSwapAmt  btcutil.Amount
@@ -276,7 +283,7 @@ func (s *serverMock) MuSig2SignSweep(_ context.Context, _ loopdb.ProtocolVersion
 	_ lntypes.Hash, _ [32]byte, _ []byte, _ []byte) ([]byte,
 	[]byte, error) {
 
-	return nil, nil, nil
+	return mockMuSig2SigningData()
 }
 
 func (s *serverMock) MultiMuSig2SignSweep(ctx context.Context,
@@ -285,7 +292,7 @@ func (s *serverMock) MultiMuSig2SignSweep(ctx context.Context,
 	prevoutMap map[wire.OutPoint]*wire.TxOut) (
 	[]byte, []byte, error) {
 
-	return nil, nil, nil
+	return mockMuSig2SigningData()
 }
 
 func (s *serverMock) PushKey(_ context.Context, _ loopdb.ProtocolVersion,

staticaddr/loopin/manager.go

@@ -379,12 +379,14 @@ func (m *Manager) handleLoopInSweepReq(ctx context.Context,
 			return err
 		}
 
-		var (
-			serverNonce [musig2.PubNonceSize]byte
-			sigHash     [32]byte
-		)
+		var sigHash [32]byte
+
+		serverNonce, err := byteSliceTo66ByteSlice(nonce)
+		if err != nil {
+			return fmt.Errorf("invalid server nonce for "+
+				"deposit %v: %w", depositOutpoint, err)
+		}
 
-		copy(serverNonce[:], nonce)
 		musig2Session, err := staticutil.CreateMusig2Session(
 			ctx, m.cfg.Signer, loopIn.AddressParams, loopIn.Address,
 		)

staticaddr/script/parameters.go

@@ -18,8 +18,8 @@ type Parameters struct {
 	// used for the 2-of-2 funding output.
 	ServerPubkey *btcec.PublicKey
 
-	// Expiry is the CSV timout value at which the client can claim the
-	// static address's timout path.
+	// Expiry is the CSV timeout value at which the client can claim the
+	// static address's timeout path.
 	Expiry uint32
 
 	// PkScript is the unique static address's output script.

staticaddr/withdraw/manager.go

@@ -793,13 +793,32 @@ func (m *Manager) signMusig2Tx(ctx context.Context,
 
 	// We'll now add the nonce to our session and sign the tx.
 	for deposit, sigAndNonce := range sigInfo {
+		if sigAndNonce == nil {
+			return nil, fmt.Errorf("missing signing info for "+
+				"deposit %v", deposit)
+		}
+
 		session, ok := sessions[deposit]
 		if !ok {
 			return nil, errors.New("session not found")
 		}
 
-		nonce := [musig2.PubNonceSize]byte{}
+		if len(sigAndNonce.Nonce) != musig2.PubNonceSize {
+			return nil, fmt.Errorf("invalid nonce length for "+
+				"deposit %v: got %d, want %d", deposit,
+				len(sigAndNonce.Nonce), musig2.PubNonceSize)
+		}
+
+		if len(sigAndNonce.Sig) != input.MuSig2PartialSigSize {
+			return nil, fmt.Errorf("invalid partial signature "+
+				"length for deposit %v: got %d, want %d",
+				deposit, len(sigAndNonce.Sig),
+				input.MuSig2PartialSigSize)
+		}
+
+		var nonce [musig2.PubNonceSize]byte
 		copy(nonce[:], sigAndNonce.Nonce)
+
 		haveAllNonces, err := signer.MuSig2RegisterNonces(
 			ctx, session.SessionID,
 			[][musig2.PubNonceSize]byte{nonce},

staticaddr/withdraw/manager_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"testing"
 
+	"github.com/btcsuite/btcd/btcec/v2/schnorr/musig2"
 	"github.com/btcsuite/btcd/btcutil"
 	"github.com/btcsuite/btcd/chaincfg"
 	"github.com/btcsuite/btcd/chaincfg/chainhash"
@@ -377,6 +378,105 @@ func TestSignMusig2Tx_MissingOutpointInDepositMap(t *testing.T) {
 	require.ErrorContains(t, err, "tx outpoint not in deposit index map")
 }
 
+// TestSignMusig2Tx_InvalidServerSigningInfo tests that malformed server
+// signing data is rejected before it is passed to the signer.
+func TestSignMusig2Tx_InvalidServerSigningInfo(t *testing.T) {
+	t.Parallel()
+
+	tx := wire.NewMsgTx(2)
+	outpoint := wire.OutPoint{
+		Hash:  [32]byte{1},
+		Index: 0,
+	}
+	tx.AddTxIn(&wire.TxIn{
+		PreviousOutPoint: outpoint,
+	})
+
+	pkScript := []byte{
+		0x51, 0x20, // OP_1 OP_PUSHBYTES_32
+		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	}
+	tx.AddTxOut(&wire.TxOut{
+		Value:    10000,
+		PkScript: pkScript,
+	})
+
+	depositKey := outpoint.String()
+	sessions := map[string]*input.MuSig2SessionInfo{
+		depositKey: {
+			SessionID: [32]byte{1},
+		},
+	}
+	depositsToIdx := map[string]int{
+		depositKey: 0,
+	}
+	prevOutFetcher := txscript.NewMultiPrevOutFetcher(
+		map[wire.OutPoint]*wire.TxOut{
+			outpoint: {
+				Value:    5000,
+				PkScript: pkScript,
+			},
+		},
+	)
+
+	validNonce := make([]byte, musig2.PubNonceSize)
+	validSig := make([]byte, input.MuSig2PartialSigSize)
+
+	tests := []struct {
+		name        string
+		signingInfo *swapserverrpc.ServerPsbtWithdrawSigningInfo
+		errContains string
+	}{
+		{
+			name:        "nil signing info",
+			signingInfo: nil,
+			errContains: "missing signing info",
+		},
+		{
+			name: "invalid nonce length",
+			signingInfo: &swapserverrpc.ServerPsbtWithdrawSigningInfo{
+				Nonce: validNonce[:musig2.PubNonceSize-1],
+				Sig:   validSig,
+			},
+			errContains: "invalid nonce length",
+		},
+		{
+			name: "invalid partial signature length",
+			signingInfo: &swapserverrpc.ServerPsbtWithdrawSigningInfo{
+				Nonce: validNonce,
+				Sig:   validSig[:input.MuSig2PartialSigSize-1],
+			},
+			errContains: "invalid partial signature length",
+		},
+	}
+
+	lnd := test.NewMockLnd()
+	m := &Manager{
+		cfg: &ManagerConfig{
+			Signer: lnd.Signer,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			sigInfo := map[string]*swapserverrpc.ServerPsbtWithdrawSigningInfo{
+				depositKey: tc.signingInfo,
+			}
+
+			_, err := m.signMusig2Tx(
+				context.Background(), prevOutFetcher, lnd.Signer,
+				tx.Copy(), sessions, sigInfo, depositsToIdx,
+			)
+			require.ErrorContains(t, err, tc.errContains)
+		})
+	}
+}
+
 // TestCalculateWithdrawalTxValues tests various edge cases in withdrawal
 // transaction value calculations.
 func TestCalculateWithdrawalTxValues(t *testing.T) {