client/asset: custom address stringer

The exchange wallets had an address decoder function for converting
a string to a btcutil.Address, which is required for assets like BCH
with special address formats. However, it is incorrect to then use
the btcutil.Address String method go get a human readable address.

This adds a complementary helper function for converting from a
btcutil.Address to a correct string. Normally this is simply using
the Address as a Stringer, but for BCH it is the new EncodeCashAddress
function, which powers the existing RecodeCashAddress.

This updates the btc baseWallet methods to use the new stringAddr
to achieve the reverse of decodeAddr. This ensures that the string
address stored in the fundingCoins is correct for the asset. This
also ensures the recipient address in AuditContract is correct.
I believe the main issue resolved by this is providing the correct
address to the node.privKeyForAddress method when signing.
This also provides the correct string to the getaddressinfo RPC.
It is not clear if bitcoincashd was previously ok with the incorrect
address strings when handling RPCs.

This also removes the unused (*auditInfo).Recipient method, and
fixes the documentation for the other methods since there is no longer
an asset.AuditInfo *interface*, just a struct.

client/asset/bch/bch.go

@@ -159,9 +159,11 @@ func NewWallet(cfg *asset.WalletConfig, logger dex.Logger, network dex.Network)
 		DefaultFallbackFee: defaultFee,
 		Segwit:             false,
 		LegacyBalance:      true,
-		// Bitcoin Cash uses the Cash Address encoding, which is Bech32, but
-		// not indicative of segwit. We provide a custom encoder.
-		AddressDecoder: dexbch.DecodeCashAddress,
+		// Bitcoin Cash uses the Cash Address encoding, which is Bech32, but not
+		// indicative of segwit. We provide a custom encoder and decode to go
+		// to/from a btcutil.Address and a string.
+		AddressDecoder:  dexbch.DecodeCashAddress,
+		AddressStringer: dexbch.EncodeCashAddress,
 		// Bitcoin Cash has a custom signature hash algorithm. Since they don't
 		// have segwit, Bitcoin Cash implemented a variation of the withdrawn
 		// BIP0062 that utilizes Shnorr signatures.
@@ -192,31 +194,6 @@ type BCHWallet struct {
 	*btc.ExchangeWalletFullNode
 }
 
-// Address converts the Bitcoin base58-encoded address returned by the embedded
-// ExchangeWallet into a Cash Address.
-func (bch *BCHWallet) Address() (string, error) {
-	btcAddrStr, err := bch.ExchangeWalletFullNode.Address()
-	if err != nil {
-		return "", err
-	}
-	return dexbch.RecodeCashAddress(btcAddrStr, bch.Net())
-}
-
-// AuditContract modifies the *asset.Contract returned by the ExchangeWallet
-// AuditContract method by converting the Recipient to the Cash Address
-// encoding.
-func (bch *BCHWallet) AuditContract(coinID, contract, txData dex.Bytes, rebroadcast bool) (*asset.AuditInfo, error) { // AuditInfo has address
-	ai, err := bch.ExchangeWalletFullNode.AuditContract(coinID, contract, txData, rebroadcast)
-	if err != nil {
-		return nil, err
-	}
-	ai.Recipient, err = dexbch.RecodeCashAddress(ai.Recipient, bch.Net())
-	if err != nil {
-		return nil, err
-	}
-	return ai, nil
-}
-
 // rawTxSigner signs the transaction using Bitcoin Cash's custom signature
 // hash and signing algorithm.
 func rawTxInSigner(btcTx *wire.MsgTx, idx int, subScript []byte, hashType txscript.SigHashType, btcKey *btcec.PrivateKey, val uint64) ([]byte, error) {

client/asset/btc/btc.go

@@ -228,7 +228,11 @@ type BTCCloneCFG struct {
 	// AddressDecoder is an optional argument that can decode an address string
 	// into btcutil.Address. If AddressDecoder is not supplied,
 	// btcutil.DecodeAddress will be used.
-	AddressDecoder dexbtc.AddressDecoder
+	AddressDecoder dexbtc.AddressDecoder // string => btcutil.Address
+	// AddressStringer is an optional argument that can encode a btcutil.Address
+	// into an address string. If AddressStringer is not supplied, the
+	// (btcutil.Address).String method will be used.
+	AddressStringer dexbtc.AddressStringer // btcutil.Address => string, may be an override or just the String method
 	// BlockDeserializer can be used in place of (*wire.MsgBlock).Deserialize.
 	BlockDeserializer func([]byte) (*wire.MsgBlock, error)
 	// ArglessChangeAddrRPC can be true if the getrawchangeaddress takes no
@@ -332,27 +336,19 @@ func (op *output) wireOutPoint() *wire.OutPoint {
 // auditInfo is information about a swap contract on that blockchain.
 type auditInfo struct {
 	output     *output
-	recipient  btcutil.Address
+	recipient  btcutil.Address // caution: use stringAddr, not the Stringer
 	contract   []byte
 	secretHash []byte
 	expiration time.Time
 }
 
-// Recipient returns a base58 string for the contract's receiving address. Part
-// of the asset.AuditInfo interface.
-func (ci *auditInfo) Recipient() string {
-	return ci.recipient.String()
-}
-
 // Expiration returns the expiration time of the contract, which is the earliest
-// time that a refund can be issued for an un-redeemed contract. Part of the
-// asset.AuditInfo interface.
+// time that a refund can be issued for an un-redeemed contract.
 func (ci *auditInfo) Expiration() time.Time {
 	return ci.expiration
 }
 
-// Coin returns the output as an asset.Coin. Part of the asset.AuditInfo
-// interface.
+// Coin returns the output as an asset.Coin.
 func (ci *auditInfo) Coin() asset.Coin {
 	return ci.output
 }
@@ -587,8 +583,9 @@ type baseWallet struct {
 	useLegacyBalance  bool
 	segwit            bool
 	signNonSegwit     TxInSigner
-	estimateFee       func(RawRequester, uint64) (uint64, error)
+	estimateFee       func(RawRequester, uint64) (uint64, error) // TODO: resolve the awkwardness of an RPC-oriented func in a generic framework
 	decodeAddr        dexbtc.AddressDecoder
+	stringAddr        dexbtc.AddressStringer
 
 	tipMtx     sync.RWMutex
 	currentTip *block
@@ -773,6 +770,7 @@ func newRPCWallet(requester RawRequesterWithContext, cfg *BTCCloneCFG, walletCon
 		requester:                requester,
 		segwit:                   cfg.Segwit,
 		decodeAddr:               btc.decodeAddr,
+		stringAddr:               btc.stringAddr,
 		deserializeBlock:         blockDeserializer,
 		arglessChangeAddrRPC:     cfg.ArglessChangeAddrRPC,
 		legacyRawSends:           cfg.LegacyRawFeeLimit,
@@ -824,6 +822,13 @@ func newUnconnectedWallet(cfg *BTCCloneCFG, walletCfg *WalletConfig) (*baseWalle
 		addrDecoder = cfg.AddressDecoder
 	}
 
+	addrStringer := cfg.AddressStringer
+	if cfg.AddressStringer == nil {
+		addrStringer = func(addr btcutil.Address, _ *chaincfg.Params) (string, error) {
+			return addr.String(), nil
+		}
+	}
+
 	nonSegwitSigner := rawTxInSig
 	if cfg.NonSegwitSigner != nil {
 		nonSegwitSigner = cfg.NonSegwitSigner
@@ -848,6 +853,7 @@ func newUnconnectedWallet(cfg *BTCCloneCFG, walletCfg *WalletConfig) (*baseWalle
 		signNonSegwit:       nonSegwitSigner,
 		estimateFee:         cfg.FeeEstimator,
 		decodeAddr:          addrDecoder,
+		stringAddr:          addrStringer,
 		walletInfo:          cfg.WalletInfo,
 	}
 
@@ -887,12 +893,6 @@ func (btc *baseWallet) Info() *asset.WalletInfo {
 	return btc.walletInfo
 }
 
-// Net returns the ExchangeWallet's *chaincfg.Params. This is not part of the
-// asset.Wallet interface, but is provided as a convenience for embedding types.
-func (btc *baseWallet) Net() *chaincfg.Params {
-	return btc.chainParams
-}
-
 // Connect connects the wallet to the RPC server. Satisfies the dex.Connector
 // interface.
 func (btc *baseWallet) Connect(ctx context.Context) (*sync.WaitGroup, error) {
@@ -1705,6 +1705,10 @@ func (btc *baseWallet) split(value uint64, lots uint64, outputs []*output,
 	if err != nil {
 		return nil, false, fmt.Errorf("error creating split transaction address: %w", err)
 	}
+	addrStr, err := btc.stringAddr(addr, btc.chainParams)
+	if err != nil {
+		return nil, false, fmt.Errorf("failed to stringify the change address: %w", err)
+	}
 
 	reqFunds := calc.RequiredOrderFundsAlt(value, swapInputSize, lots, nfo.SwapSizeBase, nfo.SwapSize, bumpedMaxRate)
 
@@ -1734,7 +1738,7 @@ func (btc *baseWallet) split(value uint64, lots uint64, outputs []*output,
 	fundingCoins = map[outPoint]*utxo{op.pt: {
 		txHash:  op.txHash(),
 		vout:    op.vout(),
-		address: addr.String(),
+		address: addrStr,
 		amount:  reqFunds,
 	}}
 
@@ -2054,12 +2058,18 @@ func (btc *baseWallet) Swap(swaps *asset.Swaps) ([]asset.Receipt, asset.Coin, ui
 			btc.log.Errorf("failed to lock change output: %v", err)
 		}
 
+		addrStr, err := btc.stringAddr(changeAddr, btc.chainParams)
+		if err != nil {
+			btc.log.Errorf("Failed to stringify address %v (default encoding): %v", changeAddr, err)
+			addrStr = changeAddr.String() // may or may not be able to retrieve the private keys for the next swap!
+		}
+
 		// Log it as a fundingCoin, since it is expected that this will be
 		// chained into further matches.
 		btc.fundingCoins[change.pt] = &utxo{
 			txHash:  change.txHash(),
 			vout:    change.vout(),
-			address: changeAddr.String(),
+			address: addrStr,
 			amount:  change.value,
 		}
 	}
@@ -2217,9 +2227,9 @@ func (btc *baseWallet) convertAuditInfo(ai *asset.AuditInfo) (*auditInfo, error)
 	}
 
 	return &auditInfo{
-		output:     newOutput(txHash, vout, ai.Coin.Value()), //     *output
-		recipient:  recip,                                    //  btcutil.Address
-		contract:   ai.Contract,                              //   []byte
+		output:     newOutput(txHash, vout, ai.Coin.Value()), // *output
+		recipient:  recip,                                    // btcutil.Address
+		contract:   ai.Contract,                              // []byte
 		secretHash: ai.SecretHash,                            // []byte
 		expiration: ai.Expiration,                            // time.Time
 	}, nil
@@ -2348,9 +2358,15 @@ func (btc *baseWallet) AuditContract(coinID, contract, txData dex.Bytes, rebroad
 		}()
 	}
 
+	addrStr, err := btc.stringAddr(receiver, btc.chainParams)
+	if err != nil {
+		btc.log.Errorf("Failed to stringify receiver address %v (default): %v", receiver, err)
+		addrStr = receiver.String() // potentially misleading AuditInfo.Recipient
+	}
+
 	return &asset.AuditInfo{
 		Coin:       newOutput(txHash, vout, uint64(txOut.Value)),
-		Recipient:  receiver.String(),
+		Recipient:  addrStr,
 		Contract:   contract,
 		SecretHash: secretHash,
 		Expiration: time.Unix(int64(stamp), 0).UTC(),
@@ -2741,7 +2757,7 @@ func (btc *baseWallet) Address() (string, error) {
 	if err != nil {
 		return "", err
 	}
-	return addr.String(), nil
+	return btc.stringAddr(addr, btc.chainParams)
 }
 
 // NewAddress returns a new address from the wallet. This satisfies the
@@ -3245,8 +3261,9 @@ func (btc *baseWallet) signTxAndAddChange(baseTx *wire.MsgTx, addr btcutil.Addre
 		// Add the change output.
 		vSize0 := dexbtc.MsgTxVBytes(baseTx)
 		baseTx.AddTxOut(changeOutput)
-		changeSize := dexbtc.MsgTxVBytes(baseTx) - vSize0 // may be dexbtc.P2WPKHOutputSize
-		btc.log.Debugf("Change output size = %d, addr = %s", changeSize, addr.String())
+		changeSize := dexbtc.MsgTxVBytes(baseTx) - vSize0   // may be dexbtc.P2WPKHOutputSize
+		addrStr, _ := btc.stringAddr(addr, btc.chainParams) // just for logging
+		btc.log.Debugf("Change output size = %d, addr = %s", changeSize, addrStr)
 
 		vSize += changeSize
 		fee := feeRate * vSize
@@ -3328,7 +3345,11 @@ func (btc *baseWallet) broadcastTx(signedTx *wire.MsgTx) error {
 // createSig creates and returns the serialized raw signature and compressed
 // pubkey for a transaction input signature.
 func (btc *baseWallet) createSig(tx *wire.MsgTx, idx int, pkScript []byte, addr btcutil.Address, val uint64) (sig, pubkey []byte, err error) {
-	privKey, err := btc.node.privKeyForAddress(addr.String())
+	addrStr, err := btc.stringAddr(addr, btc.chainParams)
+	if err != nil {
+		return nil, nil, err
+	}
+	privKey, err := btc.node.privKeyForAddress(addrStr)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -3343,8 +3364,11 @@ func (btc *baseWallet) createSig(tx *wire.MsgTx, idx int, pkScript []byte, addr
 // input and the pubkey associated with the address.
 func (btc *baseWallet) createWitnessSig(tx *wire.MsgTx, idx int, pkScript []byte,
 	addr btcutil.Address, val uint64, sigHashes *txscript.TxSigHashes) (sig, pubkey []byte, err error) {
-
-	privKey, err := btc.node.privKeyForAddress(addr.String())
+	addrStr, err := btc.stringAddr(addr, btc.chainParams)
+	if err != nil {
+		return nil, nil, err
+	}
+	privKey, err := btc.node.privKeyForAddress(addrStr)
 	if err != nil {
 		return nil, nil, err
 	}

client/asset/btc/rpcclient.go

@@ -74,6 +74,7 @@ type rpcCore struct {
 	requester                RawRequesterWithContext
 	segwit                   bool
 	decodeAddr               dexbtc.AddressDecoder
+	stringAddr               dexbtc.AddressStringer
 	deserializeBlock         func([]byte) (*wire.MsgBlock, error)
 	arglessChangeAddrRPC     bool
 	legacyRawSends           bool
@@ -395,7 +396,7 @@ func (wc *rpcClient) address(aType string) (btcutil.Address, error) {
 	if err != nil {
 		return nil, err
 	}
-	return wc.decodeAddr(addrStr, wc.chainParams)
+	return wc.decodeAddr(addrStr, wc.chainParams) // we should consider returning a string
 }
 
 // signTx attempts to have the wallet sign the transaction inputs.
@@ -497,7 +498,11 @@ func (wc *rpcClient) GetWalletInfo() (*GetWalletInfoResult, error) {
 // getaddressinfo or validateaddress RPC command.
 func (wc *rpcClient) getAddressInfo(addr btcutil.Address, method string) (*GetAddressInfoResult, error) {
 	ai := new(GetAddressInfoResult)
-	return ai, wc.call(method, anylist{addr.String()}, ai)
+	addrStr, err := wc.stringAddr(addr, wc.chainParams)
+	if err != nil {
+		return nil, err
+	}
+	return ai, wc.call(method, anylist{addrStr}, ai)
 }
 
 // ownsAddress indicates if an address belongs to the wallet.

client/asset/dcr/dcr.go

@@ -235,24 +235,16 @@ func (op *output) wireOutPoint() *wire.OutPoint {
 
 // auditInfo is information about a swap contract on the blockchain, not
 // necessarily created by this wallet, as would be returned from AuditContract.
-// auditInfo satisfies the asset.AuditInfo interface.
 type auditInfo struct {
 	output     *output
 	secretHash []byte
 	contract   []byte
-	recipient  stdaddr.Address
+	recipient  stdaddr.Address // unused?
 	expiration time.Time
 }
 
-// Recipient is a base58 string for the contract's receiving address. Part of
-// the asset.AuditInfo interface.
-func (ci *auditInfo) Recipient() string {
-	return ci.recipient.String()
-}
-
 // Expiration is the expiration time of the contract, which is the earliest time
-// that a refund can be issued for an un-redeemed contract. Part of the
-// asset.AuditInfo interface.
+// that a refund can be issued for an un-redeemed contract.
 func (ci *auditInfo) Expiration() time.Time {
 	return ci.expiration
 }
@@ -262,8 +254,7 @@ func (ci *auditInfo) Contract() dex.Bytes {
 	return ci.contract
 }
 
-// Coin returns the output as an asset.Coin. Part of the asset.AuditInfo
-// interface.
+// Coin returns the output as an asset.Coin.
 func (ci *auditInfo) Coin() asset.Coin {
 	return ci.output
 }
@@ -291,9 +282,9 @@ func convertAuditInfo(ai *asset.AuditInfo, chainParams *chaincfg.Params) (*audit
 	}
 
 	return &auditInfo{
-		output:     op,            //     *output
-		recipient:  recip,         //  btcutil.Address
-		contract:   ai.Contract,   //   []byte
+		output:     op,            // *output
+		recipient:  recip,         // btcutil.Address
+		contract:   ai.Contract,   // []byte
 		secretHash: ai.SecretHash, // []byte
 		expiration: ai.Expiration, // time.Time
 	}, nil

dex/networks/bch/cashaddr.go

@@ -19,8 +19,17 @@ func RecodeCashAddress(addr string, net *chaincfg.Params) (string, error) {
 	if err != nil {
 		return "", err
 	}
+	return EncodeCashAddress(btcAddr, net)
+}
 
+// EncodeCashAddress converts a btcutil.Address that the BTC backend into a Cash
+// Address string. For example, if a pkScript was decoded to a btcutil.Address,
+// the correct address string would not be from the Address' String method, but
+// from EncodeCashAddress. This is more direct than doing
+// RecodeCashAddress(addr.String()), which needlessly decodes the string.
+func EncodeCashAddress(btcAddr btcutil.Address, net *chaincfg.Params) (string, error) {
 	var bchAddr bchutil.Address
+	var err error
 	switch at := btcAddr.(type) {
 	case *btcutil.AddressPubKeyHash:
 		bchAddr, err = bchutil.NewAddressPubKeyHash(btcAddr.ScriptAddress(), convertParams(net))

dex/networks/btc/clone.go

@@ -12,6 +12,11 @@ import (
 // AddressDecoder decodes a string address to a btcutil.Address.
 type AddressDecoder func(addr string, net *chaincfg.Params) (btcutil.Address, error)
 
+// AddressStringer converts the btcutil.Address into a string address. This may
+// be different from using (btcutil.Address).String when there is a special
+// encoding such as a BCH "Cash Address".
+type AddressStringer func(btcutil.Address, *chaincfg.Params) (string, error)
+
 // ReadCloneParams translates a CloneParams into a btcsuite chaincfg.Params.
 func ReadCloneParams(cloneParams *CloneParams) *chaincfg.Params {
 	return &chaincfg.Params{

dex/networks/btc/script.go

@@ -647,7 +647,7 @@ func InputInfo(pkScript, redeemScript []byte, chainParams *chaincfg.Params) (*Sp
 	}
 	if nonStandard {
 		return &SpendInfo{
-			// SigScriptSize and WitnessSizecannot be determined, leave zero.
+			// SigScriptSize and WitnessSize cannot be determined, leave zero.
 			ScriptAddrs:       scriptAddrs,
 			ScriptType:        scriptType,
 			NonStandardScript: true,