Merge pull request #1011 from lightninglabs/tap-channel-testing-bugfixes

Custom channel testing bugfixes

Author: guggero

config.go

@@ -197,8 +197,6 @@ type Config struct {
 
 	UniverseStats universe.Telemetry
 
-	AuxLeafCreator *tapchannel.AuxLeafCreator
-
 	AuxLeafSigner *tapchannel.AuxLeafSigner
 
 	AuxFundingController *tapchannel.FundingController

fn/errors.go

@@ -2,6 +2,7 @@ package fn
 
 import (
 	"context"
+	"errors"
 	"strings"
 
 	"google.golang.org/grpc/codes"
@@ -44,3 +45,42 @@ func IsRpcErr(err error, candidate error) bool {
 
 	return strings.Contains(err.Error(), candidate.Error())
 }
+
+// CriticalError is an error type that should be used for errors that are
+// critical and should cause the application to exit.
+type CriticalError struct {
+	Err error
+}
+
+// NewCriticalError creates a new CriticalError instance.
+func NewCriticalError(err error) *CriticalError {
+	return &CriticalError{Err: err}
+}
+
+// Error implements the error interface.
+func (e *CriticalError) Error() string {
+	return e.Err.Error()
+}
+
+// Unwrap implements the errors.Wrapper interface.
+func (e *CriticalError) Unwrap() error {
+	return e.Err
+}
+
+// ErrorAs behaves the same as `errors.As` except there's no need to declare
+// the target error as a variable first.
+// Instead of writing:
+//
+//	var targetErr *TargetErr
+//	errors.As(err, &targetErr)
+//
+// We can write:
+//
+//	lnutils.ErrorAs[*TargetErr](err)
+//
+// To save us from declaring the target error variable.
+func ErrorAs[Target error](err error) bool {
+	var targetErr Target
+
+	return errors.As(err, &targetErr)
+}

itest/tapd_harness.go

@@ -222,8 +222,9 @@ func newTapdHarness(t *testing.T, ht *harnessTest, cfg tapdConfig,
 	// Set the experimental config for the RFQ service.
 	tapCfg.Experimental = &tapcfg.ExperimentalConfig{
 		Rfq: rfq.CliConfig{
-			PriceOracleAddress:   rfq.MockPriceOracleServiceAddress,
-			MockOracleCentPerSat: 5_820_600,
+			//nolint:lll
+			PriceOracleAddress:     rfq.MockPriceOracleServiceAddress,
+			MockOracleAssetsPerBTC: 5_820_600,
 		},
 	}
 

rfq/cli.go

@@ -7,6 +7,12 @@ import (
 const (
 	MockPriceOracleServiceAddress = "use_mock_price_oracle_service_" +
 		"promise_to_not_use_on_mainnet"
+
+	// MinAssetsPerBTC is the minimum number of asset units that one BTC
+	// should cost. If the value is lower, then one asset unit would cost
+	// too much to be able to represent small amounts of satoshis. With this
+	// value, one asset unit would still cost 1k sats.
+	MinAssetsPerBTC = 100_000
 )
 
 // CliConfig is a struct that holds tapd cli configuration options for the RFQ
@@ -18,18 +24,21 @@ type CliConfig struct {
 
 	SkipAcceptQuotePriceCheck bool `long:"skipacceptquotepricecheck" description:"Accept any price quote returned by RFQ peer, skipping price validation"`
 
-	MockOracleCentPerSat uint64 `long:"mockoraclecentpersat" description:"Mock price oracle static USD cent per sat rate"`
+	MockOracleAssetsPerBTC uint64 `long:"mockoracleassetsperbtc" description:"Mock price oracle static asset units per BTC rate (for example number of USD cents per BTC if one asset unit represents a USD cent); whole numbers only, use either this or mockoraclesatsperasset depending on required precision"`
+
+	MockOracleSatsPerAsset uint64 `long:"mockoraclesatsperasset" description:"Mock price oracle static satoshis per asset unit rate (for example number of satoshis to pay for one USD cent if one asset unit represents a USD cent); whole numbers only, use either this or mockoracleassetsperbtc depending on required precision"`
 }
 
 // Validate returns an error if the configuration is invalid.
 func (c *CliConfig) Validate() error {
 	// If the user has specified a mock oracle USD per BTC rate but the
 	// price oracle address is not the mock price oracle service address,
 	// then we'll return an error.
-	if c.MockOracleCentPerSat > 0 &&
+	if (c.MockOracleAssetsPerBTC > 0 || c.MockOracleSatsPerAsset > 0) &&
 		c.PriceOracleAddress != MockPriceOracleServiceAddress {
 
-		return fmt.Errorf("mockoraclecentpersat can only be used "+
+		return fmt.Errorf("mockoracleassetsperbtc or "+
+			"mockoraclesatsperasset can only be used "+
 			"with the mock price oracle service, set "+
 			"priceoracleaddress to %s",
 			MockPriceOracleServiceAddress)
@@ -39,12 +48,35 @@ func (c *CliConfig) Validate() error {
 	// has not set the mock oracle USD per BTC rate, then we'll return an
 	// error.
 	if c.PriceOracleAddress == MockPriceOracleServiceAddress &&
-		c.MockOracleCentPerSat == 0 {
+		c.MockOracleAssetsPerBTC == 0 && c.MockOracleSatsPerAsset == 0 {
 
-		return fmt.Errorf("mockoraclecentpersat must be set when " +
+		return fmt.Errorf("mockoracleassetsperbtc or " +
+			"mockoraclesatsperasset must be set when " +
 			"using the mock price oracle service")
 	}
 
+	// Only one of the mock oracle rates can be set.
+	if c.MockOracleAssetsPerBTC > 0 && c.MockOracleSatsPerAsset > 0 {
+		return fmt.Errorf("only one of mockoracleassetsperbtc or " +
+			"mockoraclesatsperasset can be set")
+	}
+
+	// The MockOracleAssetsPerBTC is more precise for tracking the actual
+	// BTC price but less optimal for just specifying a dummy or test rate
+	// for an asset that isn't BTC. The smaller the value, the more one
+	// asset costs. If we allowed a value of 1, then one asset unit would
+	// cost 1 BTC, which cannot really easily be transported over the
+	// network. So we require a value of at least 100k, which would still
+	// mean that one asset unit costs 1k sats.
+	if c.MockOracleAssetsPerBTC > 0 &&
+		c.MockOracleAssetsPerBTC < MinAssetsPerBTC {
+
+		return fmt.Errorf("mockoracleassetsperbtc must be at least "+
+			"%d asset units per BTC, otherwise one asset "+
+			"unit would cost more than 1k sats per unit",
+			MinAssetsPerBTC)
+	}
+
 	// Ensure that if the price oracle address not the mock price oracle
 	// service address then it must be a valid gRPC address.
 	if c.PriceOracleAddress != "" &&

rfq/manager.go

@@ -239,6 +239,18 @@ func (m *Manager) startSubsystems(ctx context.Context) error {
 	return err
 }
 
+// handleError logs an error and sends it to the main server error channel if
+// it is a critical error.
+func (m *Manager) handleError(err error) {
+	log.Errorf("Error in RFQ manager: %v", err)
+
+	// If the error is a critical error, send it to the main server error
+	// channel, which will cause the daemon to shut down.
+	if fn.ErrorAs[*fn.CriticalError](err) {
+		m.cfg.ErrChan <- err
+	}
+}
+
 // Start attempts to start a new RFQ manager.
 func (m *Manager) Start() error {
 	var startErr error
@@ -363,8 +375,10 @@ func (m *Manager) handleIncomingMessage(incomingMsg rfqmsg.IncomingMsg) error {
 				*msg.Request.AssetID, msg.Peer,
 			)
 			if err != nil {
-				m.cfg.ErrChan <- fmt.Errorf("error adding "+
-					"local alias: %w", err)
+				m.handleError(
+					fmt.Errorf("error adding local alias: "+
+						"%w", err),
+				)
 				return
 			}
 
@@ -488,7 +502,12 @@ func (m *Manager) addScidAlias(scidAlias uint64, assetID asset.ID,
 	ctxb := context.Background()
 	localChans, err := m.cfg.ChannelLister.ListChannels(ctxb)
 	if err != nil {
-		return fmt.Errorf("error listing local channels: %w", err)
+		// Not being able to call lnd to add the alias is a critical
+		// error, which warrants shutting down, as something is wrong.
+		return fn.NewCriticalError(
+			fmt.Errorf("add alias: error listing local channels: "+
+				"%w", err),
+		)
 	}
 
 	// Filter for channels with the given peer.
@@ -534,15 +553,26 @@ func (m *Manager) addScidAlias(scidAlias uint64, assetID asset.ID,
 	// At this point, if the base SCID is still not found, we return an
 	// error. We can't map the SCID alias to a base SCID.
 	if baseSCID == 0 {
-		return fmt.Errorf("base SCID not found for asset: %v", assetID)
+		return fmt.Errorf("add alias: base SCID not found for asset: "+
+			"%v", assetID)
 	}
 
 	log.Debugf("Adding SCID alias %d for base SCID %d", scidAlias, baseSCID)
 
-	return m.cfg.AliasManager.AddLocalAlias(
+	err = m.cfg.AliasManager.AddLocalAlias(
 		ctxb, lnwire.NewShortChanIDFromInt(scidAlias),
 		lnwire.NewShortChanIDFromInt(baseSCID),
 	)
+	if err != nil {
+		// Not being able to call lnd to add the alias is a critical
+		// error, which warrants shutting down, as something is wrong.
+		return fn.NewCriticalError(
+			fmt.Errorf("add alias: error adding SCID alias to "+
+				"lnd alias manager: %w", err),
+		)
+	}
+
+	return nil
 }
 
 // mainEventLoop is the main event loop of the RFQ manager.
@@ -556,8 +586,10 @@ func (m *Manager) mainEventLoop() {
 
 			err := m.handleIncomingMessage(incomingMsg)
 			if err != nil {
-				m.cfg.ErrChan <- fmt.Errorf("failed to "+
-					"handle incoming message: %w", err)
+				m.handleError(
+					fmt.Errorf("failed to handle "+
+						"incoming message: %w", err),
+				)
 			}
 
 		// Handle outgoing message.
@@ -567,8 +599,10 @@ func (m *Manager) mainEventLoop() {
 
 			err := m.handleOutgoingMessage(outgoingMsg)
 			if err != nil {
-				m.cfg.ErrChan <- fmt.Errorf("failed to "+
-					"handle outgoing message: %w", err)
+				m.handleError(
+					fmt.Errorf("failed to handle outgoing "+
+						"message: %w", err),
+				)
 			}
 
 		case acceptHtlcEvent := <-m.acceptHtlcEvents:
@@ -577,12 +611,12 @@ func (m *Manager) mainEventLoop() {
 
 		// Handle subsystem errors.
 		case err := <-m.subsystemErrChan:
-			log.Errorf("Manager main event loop received "+
-				"subsystem error: %v", err)
-
-			// Report the subsystem error to the main server.
-			m.cfg.ErrChan <- fmt.Errorf("encountered RFQ "+
-				"subsystem error: %w", err)
+			// Report the subsystem error to the main server, in
+			// case the root cause is a critical error.
+			m.handleError(
+				fmt.Errorf("encountered RFQ subsystem error "+
+					"in main event loop: %w", err),
+			)
 
 		case <-m.Quit:
 			log.Debug("Manager main event loop has received the " +

rfq/oracle.go

@@ -368,15 +368,29 @@ var _ PriceOracle = (*RpcPriceOracle)(nil)
 // MockPriceOracle is a mock implementation of the PriceOracle interface.
 // It returns the suggested rate as the exchange rate.
 type MockPriceOracle struct {
-	expiryDelay uint64
-	usdPerBTC   uint64
+	expiryDelay  uint64
+	mSatPerAsset lnwire.MilliSatoshi
 }
 
 // NewMockPriceOracle creates a new mock price oracle.
-func NewMockPriceOracle(expiryDelay, usdPerBTC uint64) *MockPriceOracle {
+func NewMockPriceOracle(expiryDelay, assetsPerBTC uint64) *MockPriceOracle {
+	mSatPerAsset := lnwire.NewMSatFromSatoshis(btcutil.SatoshiPerBitcoin) /
+		lnwire.MilliSatoshi(assetsPerBTC)
+
+	return &MockPriceOracle{
+		expiryDelay:  expiryDelay,
+		mSatPerAsset: mSatPerAsset,
+	}
+}
+
+// NewMockPriceOracleSatPerAsset creates a new mock price oracle with a
+// specified satoshis per asset rate.
+func NewMockPriceOracleSatPerAsset(expiryDelay uint64,
+	satPerAsset btcutil.Amount) *MockPriceOracle {
+
 	return &MockPriceOracle{
-		expiryDelay: expiryDelay,
-		usdPerBTC:   usdPerBTC,
+		expiryDelay:  expiryDelay,
+		mSatPerAsset: lnwire.NewMSatFromSatoshis(satPerAsset),
 	}
 }
 
@@ -388,11 +402,8 @@ func (m *MockPriceOracle) QueryAskPrice(_ context.Context,
 	// Calculate the rate expiryDelay lifetime.
 	expiry := uint64(time.Now().Unix()) + m.expiryDelay
 
-	mSatPerUsd := lnwire.NewMSatFromSatoshis(btcutil.SatoshiPerBitcoin) /
-		lnwire.MilliSatoshi(m.usdPerBTC)
-
 	return &OracleAskResponse{
-		AskPrice: &mSatPerUsd,
+		AskPrice: &m.mSatPerAsset,
 		Expiry:   expiry,
 	}, nil
 }
@@ -404,11 +415,8 @@ func (m *MockPriceOracle) QueryBidPrice(_ context.Context, _ *asset.ID,
 	// Calculate the rate expiryDelay lifetime.
 	expiry := uint64(time.Now().Unix()) + m.expiryDelay
 
-	mSatPerUsd := lnwire.NewMSatFromSatoshis(btcutil.SatoshiPerBitcoin) /
-		lnwire.MilliSatoshi(m.usdPerBTC)
-
 	return &OracleBidResponse{
-		BidPrice: &mSatPerUsd,
+		BidPrice: &m.mSatPerAsset,
 		Expiry:   expiry,
 	}, nil
 }

sample-tapd.conf

@@ -320,5 +320,13 @@
 ; Accept any price quote returned by RFQ peer, skipping price validation
 ; experimental.rfq.skipacceptquotepricecheck=false
 
-; Mock price oracle static USD cent per sat rate
-; experimental.rfq.mockoraclecentpersat=
+; Mock price oracle static asset units per BTC rate (for example number of USD
+; cents per BTC if one asset unit represents a USD cent); whole numbers only,
+; use either this or mockoraclesatsperasset depending on required precision
+; experimental.rfq.mockoracleassetsperbtc=
+
+; Mock price oracle static satoshis per asset unit rate (for example number of
+; satoshis to pay for one USD cent if one asset unit represents a USD cent);
+; whole numbers only, use either this or mockoracleassetsperbtc depending on
+; required precision
+; experimental.rfq.mockoraclesatsperasset=