asset: Implement asset.FeeRater for SPV wallets and minor refactor

Signed-off-by: Philemon Ukane <[email protected]>

client/asset/bch/bch.go

@@ -212,15 +212,15 @@ func NewWallet(cfg *asset.WalletConfig, logger dex.Logger, network dex.Network)
 		// Bitcoin Cash don't take a change_type argument in their options
 		// unlike Bitcoin Core.
 		OmitAddressType: true,
-		// Bitcoin Cash uses estimatefee instead of estimatesmartfee, and even
-		// then, they modified it from the old Bitcoin Core estimatefee by
-		// removing the confirmation target argument.
-		FeeEstimator: estimateFee,
-		AssetID:      BipID,
+		AssetID:         BipID,
 	}
 
 	switch cfg.Type {
 	case walletTypeRPC, walletTypeLegacy:
+		// Bitcoin Cash uses estimatefee instead of estimatesmartfee, and even
+		// then, they modified it from the old Bitcoin Core estimatefee by
+		// removing the confirmation target argument.
+		cloneCFG.FeeEstimator = estimateFee
 		return btc.BTCCloneWallet(cloneCFG)
 	// case walletTypeElectrum:
 	// 	logger.Warnf("\n\nUNTESTED Bitcoin Cash ELECTRUM WALLET IMPLEMENTATION! DO NOT USE ON mainnet!\n\n")

client/asset/btc/btc.go

@@ -891,8 +891,8 @@ var _ asset.Wallet = (*intermediaryWallet)(nil)
 var _ asset.Accelerator = (*ExchangeWalletAccelerator)(nil)
 var _ asset.Accelerator = (*ExchangeWalletSPV)(nil)
 var _ asset.Withdrawer = (*baseWallet)(nil)
+var _ asset.FeeRater = (*baseWallet)(nil)
 var _ asset.Rescanner = (*ExchangeWalletSPV)(nil)
-var _ asset.FeeRater = (*ExchangeWalletFullNode)(nil)
 var _ asset.LogFiler = (*ExchangeWalletSPV)(nil)
 var _ asset.Recoverer = (*ExchangeWalletSPV)(nil)
 var _ asset.PeerManager = (*ExchangeWalletSPV)(nil)
@@ -964,9 +964,7 @@ func (btc *ExchangeWalletSPV) RemovePeer(addr string) error {
 }
 
 // FeeRate satisfies asset.FeeRater.
-func (btc *ExchangeWalletFullNode) FeeRate() uint64 {
-	// NOTE: With baseWallet having an optional external fee rate source, we may
-	// consider making baseWallet a FeeRater by allowing a nil local func.
+func (btc *baseWallet) FeeRate() uint64 {
 	rate, err := btc.feeRate(1)
 	if err != nil {
 		btc.log.Tracef("Failed to get fee rate: %v", err)
@@ -1236,7 +1234,7 @@ func newUnconnectedWallet(cfg *BTCCloneCFG, walletCfg *WalletConfig) (*baseWalle
 	w.cfgV.Store(baseCfg)
 
 	// Default to the BTC RPC estimator (see LTC). Consumers can use
-	// NoLocalFeeRate or a similar dummy function to power feeRate() requests
+	// noLocalFeeRate or a similar dummy function to power feeRate() requests
 	// with only an external fee rate source available. Otherwise, all method
 	// calls must provide a rate or accept the configured fallback.
 	if w.localFeeRate == nil {
@@ -1246,10 +1244,10 @@ func newUnconnectedWallet(cfg *BTCCloneCFG, walletCfg *WalletConfig) (*baseWalle
 	return w, nil
 }
 
-// NoLocalFeeRate is a dummy function for BTCCloneCFG.FeeEstimator for a wallet
+// noLocalFeeRate is a dummy function for BTCCloneCFG.FeeEstimator for a wallet
 // instance that cannot support a local fee rate estimate but has an external
 // fee rate source.
-func NoLocalFeeRate() (uint64, error) {
+func noLocalFeeRate(ctx context.Context, rr RawRequester, u uint64) (uint64, error) {
 	return 0, errors.New("no local fee rate estimate possible")
 }
 
@@ -1261,6 +1259,12 @@ func OpenSPVWallet(cfg *BTCCloneCFG, walletConstructor BTCWalletConstructor) (*E
 		return nil, err
 	}
 
+	// SPV wallets without a FeeEstimator will default to any enabled external
+	// fee estimator.
+	if cfg.FeeEstimator == nil {
+		cfg.FeeEstimator = noLocalFeeRate
+	}
+
 	btc, err := newUnconnectedWallet(cfg, walletCfg)
 	if err != nil {
 		return nil, err
@@ -1578,7 +1582,8 @@ func (btc *baseWallet) legacyBalance() (*asset.Balance, error) {
 // feeRate returns the current optimal fee rate in sat / byte using the
 // estimatesmartfee RPC or an external API if configured and enabled.
 func (btc *baseWallet) feeRate(confTarget uint64) (uint64, error) {
-	// Local estimate first. TODO: Allow only external (nil local).
+	// Local estimate first. localFeeRate might be a dummy function for spv
+	// wallets.
 	feeRate, err := btc.localFeeRate(btc.ctx, btc.node, confTarget) // e.g. rpcFeeRate
 	if err == nil {
 		return feeRate, nil

client/asset/btc/electrum.go

@@ -26,7 +26,6 @@ type ExchangeWalletElectrum struct {
 }
 
 var _ asset.Wallet = (*ExchangeWalletElectrum)(nil)
-var _ asset.FeeRater = (*ExchangeWalletElectrum)(nil)
 
 // ElectrumWallet creates a new ExchangeWalletElectrum for the provided
 // configuration, which must contain the necessary details for accessing the
@@ -64,12 +63,10 @@ func ElectrumWallet(cfg *BTCCloneCFG) (*ExchangeWalletElectrum, error) {
 		ew:         ew,
 	}
 	// In (*baseWallet).feeRate, use ExchangeWalletElectrum's walletFeeRate
-	// override for localFeeRate. No externalFeeRate is required. Note that we
-	// could set cfg.FeeEstimator to wrap ewc.FeeRate, but we'll use the
-	// ExchangeWalletElectrum method instead.
-	btc.localFeeRate = func(ctx context.Context, _ RawRequester, confTarget uint64) (uint64, error) {
-		return eew.walletFeeRate(ctx, confTarget)
-	}
+	// override for localFeeRate. No externalFeeRate is required but will be
+	// used if eew.walletFeeRate returned an error and an externalFeeRate is
+	// enabled.
+	btc.localFeeRate = eew.walletFeeRate
 
 	return eew, nil
 }
@@ -138,25 +135,15 @@ func (btc *ExchangeWalletElectrum) Connect(ctx context.Context) (*sync.WaitGroup
 	return wg, nil
 }
 
-func (btc *ExchangeWalletElectrum) walletFeeRate(ctx context.Context, confTarget uint64) (uint64, error) {
+// walletFeeRate satisfies BTCCloneCFG.FeeEstimator.
+func (btc *ExchangeWalletElectrum) walletFeeRate(ctx context.Context, _ RawRequester, confTarget uint64) (uint64, error) {
 	satPerKB, err := btc.ew.wallet.FeeRate(ctx, int64(confTarget))
 	if err != nil {
 		return 0, err
 	}
 	return uint64(dex.IntDivUp(satPerKB, 1000)), nil
 }
 
-// FeeRate gets a fee rate estimate. Satisfies asset.FeeRater. Electrum's fee
-// rate is already externally-sourced, so we simplify for FeeRate callers.
-func (btc *ExchangeWalletElectrum) FeeRate() uint64 {
-	feeRate, err := btc.walletFeeRate(btc.ew.ctx, 1)
-	if err != nil {
-		btc.log.Errorf("Failed to retrieve fee rate: %v", err)
-		return 0
-	}
-	return feeRate
-}
-
 // findRedemption will search for the spending transaction of specified
 // outpoint. If found, the secret key will be extracted from the input scripts.
 // If not found, but otherwise without an error, a nil Hash will be returned