From dfaa91e0f8f72e460643b2fb1034c8628055399f Mon Sep 17 00:00:00 2001 From: buck54321 Date: Tue, 21 Jul 2020 20:25:22 -0500 Subject: [PATCH 1/5] enable split funding transactions --- client/asset/btc/btc.go | 154 +++++++++++++--- client/asset/btc/btc_test.go | 214 +++++++++++++++-------- client/asset/btc/livetest/livetest.go | 32 ++-- client/asset/btc/livetest/regnet_test.go | 8 +- client/asset/dcr/dcr.go | 170 ++++++++++++++---- client/asset/dcr/dcr_test.go | 151 +++++++++++----- client/asset/dcr/simnet_test.go | 26 ++- client/asset/ltc/regnet_test.go | 2 +- 8 files changed, 564 insertions(+), 193 deletions(-) diff --git a/client/asset/btc/btc.go b/client/asset/btc/btc.go index 2bb6117147..8ace555544 100644 --- a/client/asset/btc/btc.go +++ b/client/asset/btc/btc.go @@ -41,12 +41,16 @@ const ( // rpcclient.Client's GetBlockVerboseTx appears to be busted. methodGetBlockVerboseTx = "getblock" methodGetNetworkInfo = "getnetworkinfo" - BipID = 0 + // BipID is the BIP-0044 asset ID. + BipID = 0 // The default fee is passed to the user as part of the asset.WalletInfo // structure. defaultFee = 100 minNetworkVersion = 190000 minProtocolVersion = 70015 + // splitTxBaggage is the total number of additional bytes associated with + // using a split transaction to fund a swap. + splitTxBaggage = dexbtc.MimimumTxOverhead + 1 + dexbtc.RedeemP2WPKHInputSize + 2*(1+dexbtc.P2PKHOutputSize) ) var ( @@ -535,8 +539,8 @@ out: for { // If there are none left, we don't have enough. if len(utxos) == 0 { - return nil, fmt.Errorf("not enough to cover requested funds + fees = %d", - calc.RequiredOrderFunds(value, uint64(size), nfo)) + return nil, fmt.Errorf("not enough to cover requested funds + fees. need %d, have %d", + calc.RequiredOrderFunds(value, uint64(size), nfo), sum) } // On each loop, find the smallest UTXO that is enough for the value. If // no UTXO is large enough, add the largest and continue. @@ -553,6 +557,10 @@ out: utxos = utxos[:len(utxos)-1] } + if btc.useSplitTx { + return btc.split(value, spents, uint64(size), fundingCoins, nfo) + } + err = btc.wallet.LockUnspent(false, spents) if err != nil { return nil, err @@ -569,6 +577,103 @@ out: return coins, nil } +// split will send a split transaction and return the sized output. If the +// split transaction is determined to be un-economical, it will not be sent, +// there is no error, and the input coins will be returned unmodified, but an +// info message will be logged. +// +// A split transaction nets 1 extra transaction, 1 extra signed p2pkh-spending +// input, and two additional p2pkh outputs. If the fees associated with this +// extra baggage are more than the excess amount that would be locked if a split +// transaction were not used, then the split transaction is pointless. This +// might be common, for instance, if an order is canceled partially filled, and +// then the remainder resubmitted. We would already have an output of just the +// right size, and that would be recognized here. +func (btc *ExchangeWallet) split(value uint64, outputs []*output, inputsSize uint64, fundingCoins map[string]*compositeUTXO, nfo *dex.Asset) (asset.Coins, error) { + var err error + defer func() { + if err != nil { + return + } + btc.fundingMtx.Lock() + for _, fCoin := range fundingCoins { + btc.fundingCoins[outpointID(fCoin.txHash.String(), fCoin.vout)] = fCoin + } + btc.fundingMtx.Unlock() + err = btc.wallet.LockUnspent(false, outputs) + if err != nil { + btc.log.Errorf("error locking unspent outputs: %v", err) + } + }() + + // Calculate the extra fees associated with the additional inputs, outputs, + // and transaction overhead, and compare to the excess that would be locked. + baggageFees := nfo.MaxFeeRate * splitTxBaggage + + var coinSum uint64 + coins := make(asset.Coins, 0, len(outputs)) + for _, op := range outputs { + coins = append(coins, op) + coinSum += op.value + } + + excess := coinSum - calc.RequiredOrderFunds(value, inputsSize, nfo) + if baggageFees > excess { + btc.log.Infof("skipping split transaction because cost is greater than potential over-lock. %d > %d", baggageFees, excess) + return coins, nil + } + + // Use an internal address for the sized output. + addr, err := btc.wallet.ChangeAddress() + if err != nil { + return nil, fmt.Errorf("error creating split transaction address: %v", err) + } + + reqFunds := calc.RequiredOrderFunds(value, dexbtc.RedeemP2WPKHInputSize, nfo) + + baseTx, _, _, err := btc.fundedTx(coins) + splitScript, err := txscript.PayToAddrScript(addr) + if err != nil { + return nil, fmt.Errorf("error creating split tx script: %v", err) + } + baseTx.AddTxOut(wire.NewTxOut(int64(reqFunds), splitScript)) + + // Grab a change address. + changeAddr, err := btc.wallet.ChangeAddress() + if err != nil { + return nil, fmt.Errorf("error creating change address: %v", err) + } + + // Sign, add change, and send the transaction. + msgTx, _, _, err := btc.sendWithReturn(baseTx, changeAddr, coinSum, reqFunds, btc.feeRateWithFallback()) + if err != nil { + return nil, err + } + txHash := msgTx.TxHash() + + op := newOutput(btc.node, &txHash, 0, reqFunds, nil) + + // Need to save one funding coin (in the deferred function). + spendInfo, err := dexbtc.InputInfo(msgTx.TxOut[0].PkScript, nil, btc.chainParams) + if err != nil { + return nil, fmt.Errorf("error reading asset info: %v", err) + } + fundingCoins = map[string]*compositeUTXO{outpointID(txHash.String(), 0): { + txHash: &op.txHash, + vout: op.vout, + address: addr.String(), + redeemScript: nil, + amount: reqFunds, + input: spendInfo, + }} + + btc.log.Infof("sent split transaction %s to accommodate swap of size %d + fees = %d", op.txHash, value, reqFunds) + + // Assign to coins so the deferred function will lock the output. + outputs = []*output{op} + return asset.Coins{op}, nil +} + // ReturnCoins unlocks coins. This would be used in the case of a // canceled or partially filled order. Part of the asset.Wallet interface. func (btc *ExchangeWallet) ReturnCoins(unspents asset.Coins) error { @@ -670,31 +775,38 @@ func (btc *ExchangeWallet) Lock() error { return btc.wallet.Lock() } -// Swap sends the swaps in a single transaction. The Receipts returned can be -// used to refund a failed transaction. -func (btc *ExchangeWallet) Swap(swaps *asset.Swaps) ([]asset.Receipt, asset.Coin, error) { - var contracts [][]byte - var totalOut, totalIn uint64 - // Start with an empty MsgTx. +// fundedTx creates and returns a new MsgTx with the provided coins as inputs. +func (btc *ExchangeWallet) fundedTx(coins asset.Coins) (*wire.MsgTx, uint64, []string, error) { baseTx := wire.NewMsgTx(wire.TxVersion) - + var totalIn uint64 // Add the funding utxos. - spents := make([]*output, 0, len(swaps.Inputs)) - for _, coin := range swaps.Inputs { - output, err := btc.convertCoin(coin) + opIDs := make([]string, 0, len(coins)) + for _, coin := range coins { + op, err := btc.convertCoin(coin) if err != nil { - return nil, nil, fmt.Errorf("error converting coin: %v", err) + return nil, 0, nil, fmt.Errorf("error converting coin: %v", err) } - if output.value == 0 { - return nil, nil, fmt.Errorf("zero-valued output detected for %s:%d", output.txHash, output.vout) + if op.value == 0 { + return nil, 0, nil, fmt.Errorf("zero-valued output detected for %s:%d", op.txHash, op.vout) } - totalIn += output.value - prevOut := wire.NewOutPoint(&output.txHash, output.vout) + totalIn += op.value + prevOut := wire.NewOutPoint(&op.txHash, op.vout) txIn := wire.NewTxIn(prevOut, []byte{}, nil) baseTx.AddTxIn(txIn) - spents = append(spents, output) + opIDs = append(opIDs, outpointID(op.txHash.String(), op.vout)) } + return baseTx, totalIn, opIDs, nil +} +// Swap sends the swap contracts and prepares the receipts. +func (btc *ExchangeWallet) Swap(swaps *asset.Swaps) ([]asset.Receipt, asset.Coin, error) { + var contracts [][]byte + var totalOut uint64 + // Start with an empty MsgTx. + baseTx, totalIn, opIDs, err := btc.fundedTx(swaps.Inputs) + if err != nil { + return nil, nil, err + } // Add the contract outputs. // TODO: Make P2WSH contract and P2WPKH change outputs instead of // legacy/non-segwit swap contracts pkScripts. @@ -793,8 +905,8 @@ func (btc *ExchangeWallet) Swap(swaps *asset.Swaps) ([]asset.Receipt, asset.Coin } // Delete the UTXOs from the cache. - for _, spent := range spents { - delete(btc.fundingCoins, outpointID(spent.txHash.String(), spent.vout)) + for _, opID := range opIDs { + delete(btc.fundingCoins, opID) } return receipts, changeCoin, nil diff --git a/client/asset/btc/btc_test.go b/client/asset/btc/btc_test.go index 79cbb744a9..a5e49df474 100644 --- a/client/asset/btc/btc_test.go +++ b/client/asset/btc/btc_test.go @@ -17,6 +17,7 @@ import ( "decred.org/dcrdex/client/asset" "decred.org/dcrdex/dex" + "decred.org/dcrdex/dex/calc" dexbtc "decred.org/dcrdex/dex/networks/btc" "github.com/btcsuite/btcd/btcec" "github.com/btcsuite/btcd/btcjson" @@ -56,9 +57,41 @@ func randBytes(l int) []byte { return b } +func signFunc(t *testing.T, params []json.RawMessage, sizeTweak int, sigComplete bool) (json.RawMessage, error) { + signTxRes := SignTxResult{ + Complete: sigComplete, + } + var msgHex string + err := json.Unmarshal(params[0], &msgHex) + if err != nil { + t.Fatalf("error unmarshaling transaction hex: %v", err) + } + msgBytes, _ := hex.DecodeString(msgHex) + txReader := bytes.NewReader(msgBytes) + msgTx := wire.NewMsgTx(wire.TxVersion) + err = msgTx.Deserialize(txReader) + if err != nil { + t.Fatalf("error deserializing contract: %v", err) + } + // Set the sigScripts to random bytes of the correct length for spending a + // p2pkh output. + scriptSize := dexbtc.RedeemP2PKHSigScriptSize + sizeTweak + for i := range msgTx.TxIn { + msgTx.TxIn[i].SignatureScript = randBytes(scriptSize) + } + buf := new(bytes.Buffer) + err = msgTx.Serialize(buf) + if err != nil { + t.Fatalf("error serializing contract: %v", err) + } + signTxRes.Hex = buf.Bytes() + return mustMarshal(t, signTxRes), nil +} + type tRPCClient struct { sendHash *chainhash.Hash sendErr error + sentRawTx *wire.MsgTx txOutRes *btcjson.GetTxOutResult txOutErr error rawRes map[string]json.RawMessage @@ -100,6 +133,7 @@ func (c *tRPCClient) EstimateSmartFee(confTarget int64, mode *btcjson.EstimateSm } func (c *tRPCClient) SendRawTransaction(tx *wire.MsgTx, _ bool) (*chainhash.Hash, error) { + c.sentRawTx = tx if c.sendErr == nil && c.sendHash == nil { h := tx.TxHash() return &h, nil @@ -294,18 +328,19 @@ func TestAvailableFund(t *testing.T) { t.Fatalf("no wallet error for rpc error") } node.rawErr[methodGetBalances] = nil - littleBit := tLotSize * 12 - littleUnspent := &ListUnspentResult{ + littleOrder := tLotSize * 12 + littleFunds := calc.RequiredOrderFunds(littleOrder, dexbtc.RedeemP2PKHInputSize, tBTC) + littleUTXO := &ListUnspentResult{ TxID: tTxID, Address: "1Bggq7Vu5oaoLFV1NNp5KhAzcku83qQhgi", - Amount: float64(littleBit) / 1e8, + Amount: float64(littleFunds) / 1e8, Confirmations: 0, ScriptPubKey: tP2PKH, Safe: true, } - unspents = append(unspents, littleUnspent) + unspents = append(unspents, littleUTXO) node.rawRes[methodListUnspent] = mustMarshal(t, unspents) - bals.Mine.Trusted = float64(littleBit) / 1e8 + bals.Mine.Trusted = float64(littleFunds) / 1e8 node.rawRes[methodGetBalances] = mustMarshal(t, &bals) lockedVal := uint64(1e6) node.rawRes[methodListLockUnspent] = mustMarshal(t, []*RPCOutpoint{ @@ -323,8 +358,8 @@ func TestAvailableFund(t *testing.T) { if err != nil { t.Fatalf("error for 1 utxo: %v", err) } - if bal.Available != littleBit { - t.Fatalf("expected available = %d for confirmed utxos, got %d", littleBit, bal.Available) + if bal.Available != littleFunds { + t.Fatalf("expected available = %d for confirmed utxos, got %d", littleOrder, bal.Available) } if bal.Immature != 0 { t.Fatalf("expected immature = 0, got %d", bal.Immature) @@ -333,33 +368,34 @@ func TestAvailableFund(t *testing.T) { t.Fatalf("expected locked = %d, got %d", lockedVal, bal.Locked) } - lottaBit := tLotSize * 100 - unspents = append(unspents, &ListUnspentResult{ + lottaOrder := tLotSize * 100 + // Add funding for an extra input to accommodate the later combined tests. + lottaFunds := calc.RequiredOrderFunds(lottaOrder, 2*dexbtc.RedeemP2PKHInputSize, tBTC) + lottaUTXO := &ListUnspentResult{ TxID: tTxID, Address: "1Bggq7Vu5oaoLFV1NNp5KhAzcku83qQhgi", - Amount: float64(lottaBit) / 1e8, + Amount: float64(lottaFunds) / 1e8, Confirmations: 1, Vout: 1, ScriptPubKey: tP2PKH, Safe: true, - }) - littleUnspent.Confirmations = 1 + } + unspents = append(unspents, lottaUTXO) + littleUTXO.Confirmations = 1 node.rawRes[methodListUnspent] = mustMarshal(t, unspents) - bals.Mine.Trusted += float64(lottaBit) / 1e8 + bals.Mine.Trusted += float64(lottaFunds) / 1e8 node.rawRes[methodGetBalances] = mustMarshal(t, &bals) bal, err = wallet.Balance() if err != nil { t.Fatalf("error for 2 utxos: %v", err) } - if bal.Available != littleBit+lottaBit { - t.Fatalf("expected available = %d for 2 outputs, got %d", littleBit+lottaBit, bal.Available) + if bal.Available != littleFunds+lottaFunds { + t.Fatalf("expected available = %d for 2 outputs, got %d", littleFunds+lottaFunds, bal.Available) } if bal.Immature != 0 { t.Fatalf("expected immature = 0 for 2 outputs, got %d", bal.Immature) } - fundLittle := littleBit - tLotSize - // Zero value _, err = wallet.FundOrder(0, tBTC) if err == nil { @@ -368,7 +404,7 @@ func TestAvailableFund(t *testing.T) { // Nothing to spend node.rawRes[methodListUnspent] = mustMarshal(t, []struct{}{}) - _, err = wallet.FundOrder(fundLittle, tBTC) + _, err = wallet.FundOrder(littleOrder, tBTC) if err == nil { t.Fatalf("no error for zero utxos") } @@ -376,7 +412,7 @@ func TestAvailableFund(t *testing.T) { // RPC error node.rawErr[methodListUnspent] = tErr - _, err = wallet.FundOrder(fundLittle, tBTC) + _, err = wallet.FundOrder(littleOrder, tBTC) if err == nil { t.Fatalf("no funding error for rpc error") } @@ -384,17 +420,17 @@ func TestAvailableFund(t *testing.T) { // Negative response when locking outputs. node.rawRes[methodLockUnspent] = []byte(`false`) - _, err = wallet.FundOrder(fundLittle, tBTC) + _, err = wallet.FundOrder(littleOrder, tBTC) if err == nil { t.Fatalf("no error for lockunspent result = false: %v", err) } node.rawRes[methodLockUnspent] = []byte(`true`) - // Fund a little bit, with unsafe littleBit. - littleUnspent.Safe = false - littleUnspent.Confirmations = 0 + // Fund a little bit, with unsafe littleOrder. + littleUTXO.Safe = false + littleUTXO.Confirmations = 0 node.rawRes[methodListUnspent] = mustMarshal(t, unspents) - spendables, err := wallet.FundOrder(fundLittle, tBTC) + spendables, err := wallet.FundOrder(littleOrder, tBTC) if err != nil { t.Fatalf("error funding small amount: %v", err) } @@ -402,14 +438,14 @@ func TestAvailableFund(t *testing.T) { t.Fatalf("expected 1 spendable, got %d", len(spendables)) } v := spendables[0].Value() - if v != lottaBit { // has to pick the larger output - t.Fatalf("expected spendable of value %d, got %d", lottaBit, v) + if v != lottaFunds { // has to pick the larger output + t.Fatalf("expected spendable of value %d, got %d", lottaFunds, v) } - // Now with safe unconfirmed littleBit. - littleUnspent.Safe = true + // Now with safe unconfirmed littleOrder. + littleUTXO.Safe = true node.rawRes[methodListUnspent] = mustMarshal(t, unspents) - spendables, err = wallet.FundOrder(fundLittle, tBTC) + spendables, err = wallet.FundOrder(littleOrder, tBTC) if err != nil { t.Fatalf("error funding small amount: %v", err) } @@ -417,12 +453,12 @@ func TestAvailableFund(t *testing.T) { t.Fatalf("expected 1 spendable, got %d", len(spendables)) } v = spendables[0].Value() - if v != littleBit { // now picks the smaller output - t.Fatalf("expected spendable of value %d, got %d", littleBit, v) + if v != littleFunds { // now picks the smaller output + t.Fatalf("expected spendable of value %d, got %d", littleFunds, v) } - // Fund a lotta bit, covered by just the lottaBit UTXO. - spendables, err = wallet.FundOrder(lottaBit-littleBit/2 /* hefty fees for 95 lots */, tBTC) + // Fund a lotta bit, covered by just the lottaOrder UTXO. + spendables, err = wallet.FundOrder(lottaOrder /* hefty fees for 95 lots */, tBTC) if err != nil { t.Fatalf("error funding large amount: %v", err) } @@ -430,12 +466,13 @@ func TestAvailableFund(t *testing.T) { t.Fatalf("expected 1 spendable, got %d", len(spendables)) } v = spendables[0].Value() - if v != lottaBit { - t.Fatalf("expected spendable of value %d, got %d", lottaBit, v) + if v != lottaFunds { + t.Fatalf("expected spendable of value %d, got %d", lottaFunds, v) } // lotta bit++ to require both spendables - spendables, err = wallet.FundOrder(lottaBit+littleBit-littleBit/2, tBTC) + extraLottaOrder := littleOrder + lottaOrder + spendables, err = wallet.FundOrder(extraLottaOrder, tBTC) if err != nil { t.Fatalf("error funding large amount: %v", err) } @@ -443,15 +480,77 @@ func TestAvailableFund(t *testing.T) { t.Fatalf("expected 2 spendable, got %d", len(spendables)) } v = spendables[0].Value() - if v != lottaBit { - t.Fatalf("expected spendable of value %d, got %d", lottaBit, v) + if v != lottaFunds { + t.Fatalf("expected spendable of value %d, got %d", lottaFunds, v) } // Not enough to cover transaction fees. - _, err = wallet.FundOrder(lottaBit+littleBit-1, tBTC) + lottaUTXO.Amount -= 1e-6 + node.rawRes[methodListUnspent] = mustMarshal(t, unspents) + _, err = wallet.FundOrder(lottaOrder+littleOrder, tBTC) if err == nil { t.Fatalf("no error when not enough to cover tx fees") } + lottaUTXO.Amount += 1e-6 + node.rawRes[methodListUnspent] = mustMarshal(t, unspents) + + // Prepare for a split transaction. + baggageFees := tBTC.MaxFeeRate * splitTxBaggage + node.rawRes[methodChangeAddress] = mustMarshal(t, tP2WPKHAddr) + wallet.useSplitTx = true + // No error when no split performed cuz math. + coins, err := wallet.FundOrder(extraLottaOrder, tBTC) + if err != nil { + t.Fatalf("error for no-split split: %v", err) + } + + // Should be both coins. + if len(coins) != 2 { + t.Fatalf("no-split split didn't return both coins") + } + + // With a little more locked, the split should be performed. + node.signFunc = func(params []json.RawMessage) (json.RawMessage, error) { + return signFunc(t, params, 0, true) + } + lottaUTXO.Amount += float64(baggageFees) / 1e8 + node.rawRes[methodListUnspent] = mustMarshal(t, unspents) + coins, err = wallet.FundOrder(extraLottaOrder, tBTC) + if err != nil { + t.Fatalf("error for split tx: %v", err) + } + + // Should be just one coin. + if len(coins) != 1 { + t.Fatalf("split failed - coin count != 1") + } + if node.sentRawTx == nil { + t.Fatalf("split failed - no tx sent") + } + + // // Hit some error paths. + + // GetRawChangeAddress error + node.rawErr[methodChangeAddress] = tErr + _, err = wallet.FundOrder(extraLottaOrder, tBTC) + if err == nil { + t.Fatalf("no error for split tx change addr error") + } + node.rawErr[methodChangeAddress] = nil + + // SendRawTx error + node.sendErr = tErr + _, err = wallet.FundOrder(extraLottaOrder, tBTC) + if err == nil { + t.Fatalf("no error for split tx send error") + } + node.sendErr = nil + + // Success again. + _, err = wallet.FundOrder(extraLottaOrder, tBTC) + if err != nil { + t.Fatalf("error for split tx recovery run") + } } // Since ReturnCoins takes the asset.Coin interface, make sure any interface @@ -731,42 +830,17 @@ func TestSwap(t *testing.T) { LockChange: true, } - signTxRes := SignTxResult{ - Complete: true, - } + signatureComplete := true // Aim for 3 signature cycles. sigSizer := 0 node.signFunc = func(params []json.RawMessage) (json.RawMessage, error) { - var msgHex string - err := json.Unmarshal(params[0], &msgHex) - if err != nil { - t.Fatalf("error unmarshaling transaction hex: %v", err) - } - msgBytes, _ := hex.DecodeString(msgHex) - txReader := bytes.NewReader(msgBytes) - msgTx := wire.NewMsgTx(wire.TxVersion) - err = msgTx.Deserialize(txReader) - if err != nil { - t.Fatalf("error deserializing contract: %v", err) - } - // Set the sigScripts to random bytes of the correct length for spending a - // p2pkh output. - scriptSize := dexbtc.RedeemP2PKHSigScriptSize + var sizeTweak int if sigSizer%2 == 0 { - scriptSize -= 2 + sizeTweak = -2 } sigSizer++ - for i := range msgTx.TxIn { - msgTx.TxIn[i].SignatureScript = randBytes(scriptSize) - } - buf := new(bytes.Buffer) - err = msgTx.Serialize(buf) - if err != nil { - t.Fatalf("error serializing contract: %v", err) - } - signTxRes.Hex = buf.Bytes() - return mustMarshal(t, signTxRes), nil + return signFunc(t, params, sizeTweak, signatureComplete) } // This time should succeed. @@ -817,12 +891,12 @@ func TestSwap(t *testing.T) { node.rawErr[methodSignTx] = nil // incomplete signatures - signTxRes.Complete = false + signatureComplete = false _, _, err = wallet.Swap(swaps) if err == nil { t.Fatalf("no error for incomplete signature rpc error") } - signTxRes.Complete = true + signatureComplete = true // Make sure we can succeed again. _, _, err = wallet.Swap(swaps) diff --git a/client/asset/btc/livetest/livetest.go b/client/asset/btc/livetest/livetest.go index b168625ba4..08536761fe 100644 --- a/client/asset/btc/livetest/livetest.go +++ b/client/asset/btc/livetest/livetest.go @@ -30,6 +30,8 @@ import ( "decred.org/dcrdex/dex/config" ) +const tPW = "abc" + type WalletConstructor func(cfg *asset.WalletConfig, logger dex.Logger, network dex.Network) (asset.Wallet, error) // Convert the BTC value to satoshi. @@ -38,7 +40,8 @@ func toSatoshi(v float64) uint64 { } func tBackend(t *testing.T, ctx context.Context, newWallet WalletConstructor, symbol, conf, name string, - logger dex.Logger, blkFunc func(string, error)) (*btc.ExchangeWallet, *dex.ConnectionMaster) { + logger dex.Logger, blkFunc func(string, error), splitTx bool) (*btc.ExchangeWallet, *dex.ConnectionMaster) { + user, err := user.Current() if err != nil { t.Fatalf("error getting current user: %v", err) @@ -49,6 +52,9 @@ func tBackend(t *testing.T, ctx context.Context, newWallet WalletConstructor, sy t.Fatalf("error reading config options: %v", err) } settings["walletname"] = name + if splitTx { + settings["txsplit"] = "1" + } walletCfg := &asset.WalletConfig{ Settings: settings, TipChange: func(err error) { @@ -109,7 +115,7 @@ func randBytes(l int) []byte { return b } -func Run(t *testing.T, newWallet WalletConstructor, address string, dexAsset *dex.Asset) { +func Run(t *testing.T, newWallet WalletConstructor, address string, dexAsset *dex.Asset, splitTx bool) { tLogger := dex.StdOutLogger("TEST", dex.LevelTrace) tCtx, shutdown := context.WithCancel(context.Background()) defer shutdown() @@ -130,9 +136,9 @@ func Run(t *testing.T, newWallet WalletConstructor, address string, dexAsset *de backends: make(map[string]*btc.ExchangeWallet), connectionMasters: make(map[string]*dex.ConnectionMaster, 3), } - rig.backends["alpha"], rig.connectionMasters["alpha"] = tBackend(t, tCtx, newWallet, dexAsset.Symbol, "alpha", "", tLogger, blkFunc) - rig.backends["beta"], rig.connectionMasters["beta"] = tBackend(t, tCtx, newWallet, dexAsset.Symbol, "beta", "", tLogger, blkFunc) - rig.backends["gamma"], rig.connectionMasters["gamma"] = tBackend(t, tCtx, newWallet, dexAsset.Symbol, "alpha", "gamma", tLogger, blkFunc) + rig.backends["alpha"], rig.connectionMasters["alpha"] = tBackend(t, tCtx, newWallet, dexAsset.Symbol, "alpha", "", tLogger, blkFunc, splitTx) + rig.backends["beta"], rig.connectionMasters["beta"] = tBackend(t, tCtx, newWallet, dexAsset.Symbol, "beta", "", tLogger, blkFunc, splitTx) + rig.backends["gamma"], rig.connectionMasters["gamma"] = tBackend(t, tCtx, newWallet, dexAsset.Symbol, "alpha", "gamma", tLogger, blkFunc, splitTx) defer rig.close() contractValue := 2 * dexAsset.LotSize @@ -154,6 +160,13 @@ func Run(t *testing.T, newWallet WalletConstructor, address string, dexAsset *de tLogger.Debugf("%s %f available, %f immature, %f locked", name, float64(bal.Available)/1e8, float64(bal.Immature)/1e8, float64(bal.Locked)/1e8) } + + // Unlock the wallet for use. + err := rig.gamma().Unlock(walletPassword, time.Hour*24) + if err != nil { + t.Fatalf("error unlocking gamma wallet: %v", err) + } + // Gamma should only have 10 BTC utxos, so calling fund for less should only // return 1 utxo. utxos, err := rig.gamma().FundOrder(contractValue*3, dexAsset) @@ -167,12 +180,11 @@ func Run(t *testing.T, newWallet WalletConstructor, address string, dexAsset *de if inUTXOs(utxo, utxos) { t.Fatalf("received locked output") } - // Unlock rig.gamma().ReturnCoins([]asset.Coin{utxo}) rig.gamma().ReturnCoins(utxos) // Make sure we get the first utxo back with Fund. utxos, _ = rig.gamma().FundOrder(contractValue*3, dexAsset) - if !inUTXOs(utxo, utxos) { + if !splitTx && !inUTXOs(utxo, utxos) { t.Fatalf("unlocked output not returned") } rig.gamma().ReturnCoins(utxos) @@ -188,12 +200,6 @@ func Run(t *testing.T, newWallet WalletConstructor, address string, dexAsset *de t.Fatalf("error funding second contract: %v", err) } - // Unlock the wallet for use. - err = rig.gamma().Unlock(walletPassword, time.Hour*24) - if err != nil { - t.Fatalf("error unlocking gamma wallet: %v", err) - } - secretKey1 := randBytes(32) keyHash1 := sha256.Sum256(secretKey1) secretKey2 := randBytes(32) diff --git a/client/asset/btc/livetest/regnet_test.go b/client/asset/btc/livetest/regnet_test.go index 460dc990ad..27118dd5c0 100644 --- a/client/asset/btc/livetest/regnet_test.go +++ b/client/asset/btc/livetest/regnet_test.go @@ -3,6 +3,7 @@ package livetest import ( + "fmt" "testing" "decred.org/dcrdex/client/asset/btc" @@ -20,7 +21,7 @@ var ( Symbol: "btc", SwapSize: dexbtc.InitTxSize, SwapSizeBase: dexbtc.InitTxSizeBase, - MaxFeeRate: 2, + MaxFeeRate: 10, LotSize: 1e6, RateStep: 10, SwapConf: 1, @@ -28,5 +29,8 @@ var ( ) func TestWallet(t *testing.T) { - Run(t, btc.NewWallet, alphaAddress, tBTC) + fmt.Println("////////// WITHOUT SPLIT FUNDING TRANSACTIONS //////////") + Run(t, btc.NewWallet, alphaAddress, tBTC, false) + fmt.Println("////////// WITH SPLIT FUNDING TRANSACTIONS //////////") + Run(t, btc.NewWallet, alphaAddress, tBTC, true) } diff --git a/client/asset/dcr/dcr.go b/client/asset/dcr/dcr.go index c4c6206f2b..7d95224ef5 100644 --- a/client/asset/dcr/dcr.go +++ b/client/asset/dcr/dcr.go @@ -37,8 +37,13 @@ import ( ) const ( - BipID = 42 + // BipID is the BIP-0044 asset ID. + BipID = 42 + // defaultFee is the default value for the fallbackfee. defaultFee = 20 + // splitTxBaggage is the total number of additional bytes associated with + // using a split transaction to fund a swap. + splitTxBaggage = dexdcr.MsgTxOverhead + 1 + dexdcr.P2PKHInputSize + 2*(1+dexdcr.P2PKHOutputSize) ) var ( @@ -300,6 +305,7 @@ type ExchangeWallet struct { // shutdown. fundingMtx sync.RWMutex fundingCoins map[string]*fundingCoin + splitFunds map[string]asset.Coins } // Check that ExchangeWallet satisfies the Wallet interface. @@ -348,6 +354,7 @@ func unconnectedWallet(cfg *asset.WalletConfig, dcrCfg *Config, logger dex.Logge tradeChange: make(map[string]time.Time), tipChange: cfg.TipChange, fundingCoins: make(map[string]*fundingCoin), + splitFunds: make(map[string]asset.Coins), fallbackFeeRate: fallbackFeesPerByte, useSplitTx: dcrCfg.UseSplitTx, } @@ -489,20 +496,27 @@ func (dcr *ExchangeWallet) FundOrder(value uint64, nfo *dex.Asset) (asset.Coins, if value == 0 { return nil, fmt.Errorf("cannot fund value = 0") } + //oneInputSize := dexdcr.P2PKHInputSize enough := func(sum uint64, size uint32, unspent *compositeUTXO) bool { reqFunds := calc.RequiredOrderFunds(value, uint64(size+unspent.input.Size()), nfo) // needed fees are reqFunds - value return sum+toAtoms(unspent.rpc.Amount) >= reqFunds } - coins, err := dcr.fund(0, enough) + + coins, inputsSize, fundingCoins, err := dcr.fund(0, enough) if err != nil { return nil, err } + // Send a split, if preferred. + if dcr.useSplitTx { + return dcr.split(value, coins, inputsSize, fundingCoins, nfo) + } + dcr.log.Debugf("funding %d atom order with coins %v", value, coins) - return coins, err + return coins, nil } // unspents fetches unspent outputs for the ExchangeWallet account. @@ -527,21 +541,21 @@ func (dcr *ExchangeWallet) unspents() ([]walletjson.ListUnspentResult, error) { // check whether adding the provided output would be enough to satisfy the // needed value. func (dcr *ExchangeWallet) fund(confs uint32, - enough func(sum uint64, size uint32, unspent *compositeUTXO) bool) (asset.Coins, error) { + enough func(sum uint64, size uint32, unspent *compositeUTXO) bool) (asset.Coins, uint64, []*fundingCoin, error) { unspents, err := dcr.unspents() if err != nil { - return nil, err + return nil, 0, nil, err } // Sort in ascending order by amount (smallest first). sort.Slice(unspents, func(i, j int) bool { return unspents[i].Amount < unspents[j].Amount }) utxos, avail, err := dcr.spendableUTXOs(unspents, confs) if err != nil { - return nil, fmt.Errorf("error parsing unspent outputs: %v", err) + return nil, 0, nil, fmt.Errorf("error parsing unspent outputs: %v", err) } if len(utxos) == 0 { - return nil, fmt.Errorf("insufficient funds. %.8f available", float64(avail)/1e8) + return nil, 0, nil, fmt.Errorf("insufficient funds. %.8f available", float64(avail)/1e8) } var sum uint64 @@ -609,24 +623,97 @@ func (dcr *ExchangeWallet) fund(confs uint32, // First try with confs>0. ok, err := tryUTXOs(1) if err != nil { - return nil, err + return nil, 0, nil, err } // Fallback to allowing 0-conf outputs. if !ok { ok, err = tryUTXOs(0) if err != nil { - return nil, err + return nil, 0, nil, err } if !ok { - return nil, fmt.Errorf("not enough to cover requested funds") + return nil, 0, nil, fmt.Errorf("not enough to cover requested funds") } } err = dcr.lockFundingCoins(spents) if err != nil { - return nil, err + return nil, 0, nil, err } - return coins, nil + return coins, uint64(size), spents, nil +} + +// split will send a split transaction and return the sized output. If the +// split transaction is determined to be un-economical, it will not be sent, +// there is no error, and the input coins will be returned unmodified, but an +// info message will be logged. +// +// A split transaction nets 1 extra transaction, 1 extra signed p2pkh-spending +// input, and two additional p2pkh outputs. If the fees associated with this +// extra baggage are more than the excess amount that would be locked if a split +// transaction were not used, then the split transaction is pointless. This +// might be common, for instance, if an order is canceled partially filled, and +// then the remainder resubmitted. We would already have an output of just the +// right size, and that would be recognized here. +func (dcr *ExchangeWallet) split(value uint64, coins asset.Coins, inputsSize uint64, fundingCoins []*fundingCoin, nfo *dex.Asset) (asset.Coins, error) { + + // Calculate the extra fees associated with the additional inputs, outputs, + // and transaction overhead, and compare to the excess that would be locked. + baggageFees := nfo.MaxFeeRate * splitTxBaggage + + var coinSum uint64 + for _, coin := range coins { + coinSum += coin.Value() + } + + excess := coinSum - calc.RequiredOrderFunds(value, inputsSize, nfo) + if baggageFees > excess { + dcr.log.Infof("skipping split transaction because cost is greater than potential over-lock. %d > %d.", baggageFees, excess) + dcr.log.Debugf("funding %d atom order with coins %v", value, coins) + return coins, nil + } + + // Use an internal address for the sized output. + addr, err := dcr.node.GetRawChangeAddress(dcr.acct, chainParams) + if err != nil { + return nil, fmt.Errorf("error creating split transaction address: %v", err) + } + + reqFunds := calc.RequiredOrderFunds(value, dexdcr.P2PKHInputSize, nfo) + + msgTx, net, err := dcr.sendCoins(addr, coins, reqFunds, dcr.feeRateWithFallback(), false) + if err != nil { + return nil, fmt.Errorf("error sending split transaction: %v", err) + } + + op := newOutput(dcr.node, msgTx.CachedTxHash(), 0, net, wire.TxTreeRegular, nil) + + // Lock the funding coin. + err = dcr.lockFundingCoins([]*fundingCoin{{ + op: op, + addr: addr.String(), + }}) + if err != nil { + dcr.log.Errorf("error locking funding coin from split transaction %s", op) + } + + // NOTE: Can't return coins yet, because dcrwallet doesn't recognize them as + // spent immediately, so subsequent calls to FundOrder might result in a + // `-4: rejected transaction: transaction in the pool already spends the + // same coins` error. + // // Unlock the spent coins. + // err = dcr.ReturnCoins(coins) + // if err != nil { + // dcr.log.Errorf("error locking coins spent in split transaction %v", coins) + // } + + dcr.logSplitFunds(op, append(coins, op)) + + dcr.log.Debugf("funding %d atom order with split output coin %v from original coins %v", value, op, coins) + dcr.log.Infof("sent split transaction %s to accommodate swap of size %d + fees = %d", op.txHash, value, reqFunds) + + return asset.Coins{op}, nil + } // lockFundingCoins locks the funding coins via RPC and stores them in the map. @@ -647,6 +734,17 @@ func (dcr *ExchangeWallet) lockFundingCoins(fCoins []*fundingCoin) error { return nil } +// logSplitFunds associates the funding coins with the split tx output, so that +// the coins can be unlocked when the swap is sent. The helps to deal with a +// timing issue with dcrwallet where listunspent might still return outputs that were +// just spent in a transaction broadcast with sendrawtransaction. Instead, we'll +// lock them until the split output is spent. +func (dcr *ExchangeWallet) logSplitFunds(op *output, coins asset.Coins) { + dcr.fundingMtx.Lock() + defer dcr.fundingMtx.Unlock() + dcr.splitFunds[op.String()] = coins +} + // ReturnCoins unlocks coins. This would be necessary in the case of a // canceled order. func (dcr *ExchangeWallet) ReturnCoins(unspents asset.Coins) error { @@ -665,6 +763,17 @@ func (dcr *ExchangeWallet) ReturnCoins(unspents asset.Coins) error { } ops = append(ops, wire.NewOutPoint(&op.txHash, op.vout, op.tree)) delete(dcr.fundingCoins, outpointID(&op.txHash, op.vout)) + splitFunds, found := dcr.splitFunds[unspent.String()] + if found { + for _, c := range splitFunds { + delete(dcr.splitFunds, c.String()) + op, err := dcr.convertCoin(c) + if err != nil { + return fmt.Errorf("error converting split funds coin: %v", err) + } + ops = append(ops, wire.NewOutPoint(&op.txHash, op.vout, op.tree)) + } + } } return dcr.node.LockUnspent(true, ops) } @@ -750,7 +859,7 @@ func (dcr *ExchangeWallet) Swap(swaps *asset.Swaps) ([]asset.Receipt, asset.Coin // Start with an empty MsgTx. baseTx := wire.NewMsgTx() // Add the funding utxos. - totalIn, wireOPs, err := dcr.addInputCoins(baseTx, swaps.Inputs) + totalIn, err := dcr.addInputCoins(baseTx, swaps.Inputs) if err != nil { return nil, nil, err } @@ -805,19 +914,12 @@ func (dcr *ExchangeWallet) Swap(swaps *asset.Swaps) ([]asset.Receipt, asset.Coin return nil, nil, err } - // Unlock the spent outputs. - err = dcr.node.LockUnspent(true, wireOPs) + // Return spent outputs. + err = dcr.ReturnCoins(swaps.Inputs) if err != nil { - dcr.log.Errorf("failed to unlock spent coins %v", wireOPs) + dcr.log.Errorf("error unlocking swapped coins", swaps.Inputs) } - // Delete the utxos from the cache. - dcr.fundingMtx.Lock() - for _, coin := range swaps.Inputs { - delete(dcr.fundingCoins, coin.String()) - } - dcr.fundingMtx.Unlock() - // Lock the change coin, if requested. if swaps.LockChange { dcr.log.Debugf("locking change coin %s", change) @@ -1350,24 +1452,22 @@ func (dcr *ExchangeWallet) Confirmations(id dex.Bytes) (uint32, error) { } // addInputCoins adds inputs to the MsgTx to spend the specified outputs. -func (dcr *ExchangeWallet) addInputCoins(msgTx *wire.MsgTx, coins asset.Coins) (uint64, []*wire.OutPoint, error) { +func (dcr *ExchangeWallet) addInputCoins(msgTx *wire.MsgTx, coins asset.Coins) (uint64, error) { var totalIn uint64 - wireOPs := make([]*wire.OutPoint, 0, len(coins)) for _, coin := range coins { output, err := dcr.convertCoin(coin) if err != nil { - return 0, nil, err + return 0, err } if output.value == 0 { - return 0, nil, fmt.Errorf("zero-valued output detected for %s:%d", output.txHash, output.vout) + return 0, fmt.Errorf("zero-valued output detected for %s:%d", output.txHash, output.vout) } totalIn += output.value prevOut := wire.NewOutPoint(&output.txHash, output.vout, output.tree) - wireOPs = append(wireOPs, prevOut) txIn := wire.NewTxIn(prevOut, int64(output.value), []byte{}) msgTx.AddTxIn(txIn) } - return totalIn, wireOPs, nil + return totalIn, nil } // Shutdown down the rpcclient.Client. @@ -1510,7 +1610,7 @@ func (dcr *ExchangeWallet) sendMinusFees(addr dcrutil.Address, val, feeRate uint enough := func(sum uint64, size uint32, unspent *compositeUTXO) bool { return sum+toAtoms(unspent.rpc.Amount) >= val } - coins, err := dcr.fund(0, enough) + coins, _, _, err := dcr.fund(0, enough) if err != nil { return nil, 0, err } @@ -1525,7 +1625,7 @@ func (dcr *ExchangeWallet) sendRegFee(addr dcrutil.Address, regfee, netFeeRate u txFee := uint64(size+unspent.input.Size()) * netFeeRate return sum+toAtoms(unspent.rpc.Amount) >= regfee+txFee } - coins, err := dcr.fund(0, enough) + coins, _, _, err := dcr.fund(0, enough) if err != nil { return nil, 0, err } @@ -1534,10 +1634,11 @@ func (dcr *ExchangeWallet) sendRegFee(addr dcrutil.Address, regfee, netFeeRate u // sendCoins sends the amount to the address as the zeroth output, spending the // specified coins. If subtract is true, the transaction fees will be taken from -// the sent value, otherwise it will taken from the change output. +// the sent value, otherwise it will taken from the change output. If there is +// change, it will be at index 1. func (dcr *ExchangeWallet) sendCoins(addr dcrutil.Address, coins asset.Coins, val, feeRate uint64, subtract bool) (*wire.MsgTx, uint64, error) { baseTx := wire.NewMsgTx() - totalIn, _, err := dcr.addInputCoins(baseTx, coins) + totalIn, err := dcr.addInputCoins(baseTx, coins) if err != nil { return nil, 0, err } @@ -1558,6 +1659,7 @@ func (dcr *ExchangeWallet) sendCoins(addr dcrutil.Address, coins asset.Coins, va if subtract { subtractee = txOut } + tx, _, err := dcr.sendWithReturn(baseTx, changeAddr, totalIn, val, feeRate, subtractee) return tx, uint64(txOut.Value), err } @@ -1694,7 +1796,7 @@ func (dcr *ExchangeWallet) sendWithReturn(baseTx *wire.MsgTx, addr dcrutil.Addre // admittedly un-scientific. This should account for any signature length // related variation as well as a potential dust change output with no // subtractee specified, in which case the dust goes to the miner. - if checkRate > feeRate*3 { + if changeAdded && checkRate > feeRate*3 { return nil, nil, fmt.Errorf("final fee rate for %s, %d, is seemingly outrageous, target = %d, raw tx = %x", msgTx.CachedTxHash(), checkRate, feeRate, dcr.wireBytes(msgTx)) } diff --git a/client/asset/dcr/dcr_test.go b/client/asset/dcr/dcr_test.go index 4fa263fc0c..33feadd9c6 100644 --- a/client/asset/dcr/dcr_test.go +++ b/client/asset/dcr/dcr_test.go @@ -15,6 +15,7 @@ import ( "decred.org/dcrdex/client/asset" "decred.org/dcrdex/dex" + "decred.org/dcrdex/dex/calc" dexdcr "decred.org/dcrdex/dex/networks/dcr" "github.com/decred/dcrd/chaincfg/chainhash" "github.com/decred/dcrd/chaincfg/v2" @@ -118,6 +119,7 @@ func tNewWallet() (*ExchangeWallet, *tRPCClient, func()) { type tRPCClient struct { sendRawHash *chainhash.Hash sendRawErr error + sentRawTx *wire.MsgTx txOutRes map[string]*chainjson.GetTxOutResult txOutErr error bestBlockHash *chainhash.Hash @@ -170,6 +172,7 @@ func (c *tRPCClient) EstimateSmartFee(confirmations int64, mode chainjson.Estima } func (c *tRPCClient) SendRawTransaction(tx *wire.MsgTx, allowHighFees bool) (*chainhash.Hash, error) { + c.sentRawTx = tx if c.sendRawErr == nil && c.sendRawHash == nil { h := tx.TxHash() return &h, nil @@ -232,7 +235,9 @@ func (c *tRPCClient) ListUnspentMin(int) ([]walletjson.ListUnspentResult, error) } func (c *tRPCClient) LockUnspent(unlock bool, ops []*wire.OutPoint) error { - c.lockedCoins = ops + if unlock == false { + c.lockedCoins = ops + } return c.lockUnspentErr } @@ -316,27 +321,26 @@ func TestAvailableFund(t *testing.T) { t.Fatalf("expected unconf = 0, got %d", bal.Immature) } - littleBit := tLotSize * 6 + littleOrder := tLotSize * 6 + littleFunds := calc.RequiredOrderFunds(littleOrder, dexdcr.P2PKHInputSize, tDCR) + littleUTXO := walletjson.ListUnspentResult{ + TxID: tTxID, + Address: tPKHAddr.String(), + Amount: float64(littleFunds) / 1e8, + Confirmations: 0, + ScriptPubKey: hex.EncodeToString(tP2PKHScript), + } + node.unspent = []walletjson.ListUnspentResult{littleUTXO} balanceResult = &walletjson.GetBalanceResult{ Balances: []walletjson.GetAccountBalanceResult{ { AccountName: wallet.acct, - Spendable: float64(littleBit) / 1e8, + Spendable: float64(littleFunds) / 1e8, }, }, } node.balanceResult = balanceResult - - littleUnspent := walletjson.ListUnspentResult{ - TxID: tTxID, - Address: tPKHAddr.String(), - Amount: float64(littleBit) / 1e8, - Confirmations: 0, - ScriptPubKey: hex.EncodeToString(tP2PKHScript), - } - node.unspent = []walletjson.ListUnspentResult{littleUnspent} - node.lluCoins = []*wire.OutPoint{ { Hash: *tTxHash, @@ -348,8 +352,8 @@ func TestAvailableFund(t *testing.T) { if err != nil { t.Fatalf("error for 1 utxo: %v", err) } - if bal.Available != littleBit { - t.Fatalf("expected available = %d for confirmed utxos, got %d", littleBit, bal.Available) + if bal.Available != littleFunds { + t.Fatalf("expected available = %d for confirmed utxos, got %d", littleFunds, bal.Available) } if bal.Immature != 0 { t.Fatalf("expected immature = %d, got %d", 0, bal.Immature) @@ -358,30 +362,31 @@ func TestAvailableFund(t *testing.T) { t.Fatalf("expected locked = %d, got %d", tLotSize, bal.Locked) } - lottaBit := tLotSize * 100 - balanceResult.Balances[0].Spendable += float64(lottaBit) / 1e8 - unspents = []walletjson.ListUnspentResult{littleUnspent, { + lottaOrder := tLotSize * 100 + // Add funding for an extra input to accommodate the later combined tests. + lottaFunds := calc.RequiredOrderFunds(lottaOrder, 2*dexdcr.P2PKHInputSize, tDCR) + balanceResult.Balances[0].Spendable += float64(lottaFunds) / 1e8 + lottaUTXO := walletjson.ListUnspentResult{ TxID: tTxID, Address: tPKHAddr.String(), - Amount: float64(lottaBit) / 1e8, + Amount: float64(lottaFunds) / 1e8, Confirmations: 1, Vout: 1, ScriptPubKey: hex.EncodeToString(tP2PKHScript), - }} + } + unspents = []walletjson.ListUnspentResult{littleUTXO, lottaUTXO} node.unspent = unspents bal, err = wallet.Balance() if err != nil { t.Fatalf("error for 2 utxos: %v", err) } - if bal.Available != littleBit+lottaBit { - t.Fatalf("expected available = %d for 2 outputs, got %d", littleBit+lottaBit, bal.Available) + if bal.Available != littleFunds+lottaFunds { + t.Fatalf("expected available = %d for 2 outputs, got %d", littleFunds+lottaFunds, bal.Available) } if bal.Immature != 0 { t.Fatalf("expected unconf = 0 for 2 outputs, got %d", bal.Immature) } - fundLittle := littleBit - tLotSize - // Zero value _, err = wallet.FundOrder(0, tDCR) if err == nil { @@ -390,7 +395,7 @@ func TestAvailableFund(t *testing.T) { // Nothing to spend node.unspent = nil - _, err = wallet.FundOrder(fundLittle, tDCR) + _, err = wallet.FundOrder(littleOrder, tDCR) if err == nil { t.Fatalf("no error for zero utxos") } @@ -398,7 +403,7 @@ func TestAvailableFund(t *testing.T) { // RPC error node.unspentErr = tErr - _, err = wallet.FundOrder(fundLittle, tDCR) + _, err = wallet.FundOrder(littleOrder, tDCR) if err == nil { t.Fatalf("no funding error for rpc error") } @@ -406,14 +411,14 @@ func TestAvailableFund(t *testing.T) { // Negative response when locking outputs. node.lockUnspentErr = tErr - _, err = wallet.FundOrder(fundLittle, tDCR) + _, err = wallet.FundOrder(littleOrder, tDCR) if err == nil { t.Fatalf("no error for lockunspent result = false: %v", err) } node.lockUnspentErr = nil - // Fund a little bit, but small output is unconfirmed. - spendables, err := wallet.FundOrder(fundLittle, tDCR) + // Fund a little bit. + spendables, err := wallet.FundOrder(littleOrder, tDCR) if err != nil { t.Fatalf("error funding small amount: %v", err) } @@ -421,14 +426,14 @@ func TestAvailableFund(t *testing.T) { t.Fatalf("expected 1 spendable, got %d", len(spendables)) } v := spendables[0].Value() - if v != lottaBit { - t.Fatalf("expected spendable of value %d, got %d", lottaBit, v) + if v != lottaFunds { + t.Fatalf("expected spendable of value %d, got %d", lottaFunds, v) } // Now confirm the little bit and have it selected. unspents[0].Confirmations++ - littleUnspent.Confirmations++ // for consistency (no indirection from unspents) - spendables, err = wallet.FundOrder(fundLittle, tDCR) + littleUTXO.Confirmations++ // for consistency (no indirection from unspents) + spendables, err = wallet.FundOrder(littleOrder, tDCR) if err != nil { t.Fatalf("error funding small amount: %v", err) } @@ -436,12 +441,12 @@ func TestAvailableFund(t *testing.T) { t.Fatalf("expected 1 spendable, got %d", len(spendables)) } v = spendables[0].Value() - if v != littleBit { - t.Fatalf("expected spendable of value %d, got %d", littleBit, v) + if v != littleFunds { + t.Fatalf("expected spendable of value %d, got %d", littleFunds, v) } // Fund a lotta bit. - spendables, err = wallet.FundOrder(lottaBit-tLotSize, tDCR) + spendables, err = wallet.FundOrder(lottaOrder, tDCR) if err != nil { t.Fatalf("error funding large amount: %v", err) } @@ -449,24 +454,84 @@ func TestAvailableFund(t *testing.T) { t.Fatalf("expected 1 spendable, got %d", len(spendables)) } v = spendables[0].Value() - if v != lottaBit { - t.Fatalf("expected spendable of value %d, got %d", lottaBit, v) + if v != lottaFunds { + t.Fatalf("expected spendable of value %d, got %d", lottaFunds, v) } // Not enough to cover transaction fees. - _, err = wallet.FundOrder(lottaBit+littleBit-1, tDCR) + littleUTXO.Amount -= 1e-7 + node.unspent = []walletjson.ListUnspentResult{littleUTXO, lottaUTXO} + _, err = wallet.FundOrder(lottaOrder+littleOrder, tDCR) if err == nil { t.Fatalf("no error when not enough to cover tx fees") } + littleUTXO.Amount += 1e-7 + node.unspent = []walletjson.ListUnspentResult{littleUTXO, lottaUTXO} + + // Prepare for a split transaction. + baggageFees := tDCR.MaxFeeRate * splitTxBaggage + node.changeAddr = tPKHAddr + extraLottaOrder := littleOrder + lottaOrder + wallet.useSplitTx = true + // No split performed due to economics is not an error. + coins, err := wallet.FundOrder(extraLottaOrder, tDCR) + if err != nil { + t.Fatalf("error for no-split split: %v", err) + } + + // Should be both coins. + if len(coins) != 2 { + t.Fatalf("no-split split didn't return both coins") + } + + // With a little more locked, the split should be performed. + lottaUTXO.Amount += float64(baggageFees) / 1e8 + node.unspent = []walletjson.ListUnspentResult{littleUTXO, lottaUTXO} + coins, err = wallet.FundOrder(extraLottaOrder, tDCR) + if err != nil { + t.Fatalf("error for split tx: %v", err) + } + + // Should be just one coin. + if len(coins) != 1 { + t.Fatalf("split failed - coin count != 1") + } + if node.sentRawTx == nil { + t.Fatalf("split failed - no tx sent") + } + + // Hit some error paths. + + // GetRawChangeAddress error + node.changeAddrErr = tErr + _, err = wallet.FundOrder(extraLottaOrder, tDCR) + if err == nil { + t.Fatalf("no error for split tx change addr error") + } + node.changeAddrErr = nil + + // SendRawTx error + node.sendRawErr = tErr + _, err = wallet.FundOrder(extraLottaOrder, tDCR) + if err == nil { + t.Fatalf("no error for split tx send error") + } + node.sendRawErr = nil + + // Success again. + _, err = wallet.FundOrder(extraLottaOrder, tDCR) + if err != nil { + t.Fatalf("error for split tx recovery run") + } // Not enough funds, because littleUnspent is a different account. - unspents[0].Account = "wrong account" - littleUnspent.Account = "wrong account" // for consistency - _, err = wallet.FundOrder(lottaBit+littleBit-tLotSize, tDCR) + littleUTXO.Account = "wrong account" // for consistency + node.unspent = []walletjson.ListUnspentResult{littleUTXO, lottaUTXO} + _, err = wallet.FundOrder(extraLottaOrder, tDCR) if err == nil { t.Fatalf("no error for wrong account") } - + littleUTXO.Account = "" } // Since ReturnCoins takes the wallet.Coin interface, make sure any interface diff --git a/client/asset/dcr/simnet_test.go b/client/asset/dcr/simnet_test.go index 7a50508dd4..7eefcaffba 100644 --- a/client/asset/dcr/simnet_test.go +++ b/client/asset/dcr/simnet_test.go @@ -148,6 +148,13 @@ func TestMain(m *testing.M) { } func TestWallet(t *testing.T) { + tLogger.Infof("////////// WITHOUT SPLIT FUNDING TRANSACTIONS //////////") + runTest(t, false) + tLogger.Infof("////////// WITH SPLIT FUNDING TRANSACTIONS //////////") + runTest(t, true) +} + +func runTest(t *testing.T, splitTx bool) { blockReported := false rig := newTestRig(t, func(name string, err error) { blockReported = true @@ -173,7 +180,15 @@ func TestWallet(t *testing.T) { } tLogger.Debugf("%s %f available, %f unconfirmed, %f locked", name, float64(bal.Available)/1e8, float64(bal.Immature)/1e8, float64(bal.Locked)/1e8) + wallet.useSplitTx = splitTx + } + + // Unlock the wallet for use. + err := rig.beta().Unlock(walletPassword, time.Hour*24) + if err != nil { + t.Fatalf("error unlocking beta wallet: %v", err) } + // Grab some coins. utxos, err := rig.beta().FundOrder(contractValue*3, tDCR) if err != nil { @@ -183,15 +198,14 @@ func TestWallet(t *testing.T) { // Coins should be locked utxos, _ = rig.beta().FundOrder(contractValue*3, tDCR) - if inUTXOs(utxo, utxos) { + if !splitTx && inUTXOs(utxo, utxos) { t.Fatalf("received locked output") } - // Unlock rig.beta().ReturnCoins([]asset.Coin{utxo}) rig.beta().ReturnCoins(utxos) // Make sure we get the first utxo back with Fund. utxos, _ = rig.beta().FundOrder(contractValue*3, tDCR) - if !inUTXOs(utxo, utxos) { + if !splitTx && !inUTXOs(utxo, utxos) { t.Fatalf("unlocked output not returned") } rig.beta().ReturnCoins(utxos) @@ -207,12 +221,6 @@ func TestWallet(t *testing.T) { t.Fatalf("error funding second contract: %v", err) } - // Unlock the wallet for use. - err = rig.beta().Unlock(walletPassword, time.Hour*24) - if err != nil { - t.Fatalf("error unlocking beta wallet: %v", err) - } - secretKey1 := randBytes(32) keyHash1 := sha256.Sum256(secretKey1) secretKey2 := randBytes(32) diff --git a/client/asset/ltc/regnet_test.go b/client/asset/ltc/regnet_test.go index 341b681eef..7e1323e26f 100644 --- a/client/asset/ltc/regnet_test.go +++ b/client/asset/ltc/regnet_test.go @@ -36,5 +36,5 @@ var tLTC = &dex.Asset{ } func TestWallet(t *testing.T) { - livetest.Run(t, NewWallet, alphaAddress, tLTC) + livetest.Run(t, NewWallet, alphaAddress, tLTC, false) } From 418f215fcda5c8909f6b26f5b1f5592a528d21de Mon Sep 17 00:00:00 2001 From: buck54321 Date: Tue, 28 Jul 2020 15:02:02 -0500 Subject: [PATCH 2/5] skip split for immediate orders --- client/asset/btc/btc.go | 21 ++++++----- client/asset/btc/btc_test.go | 53 ++++++++++++++++----------- client/asset/btc/livetest/livetest.go | 12 +++--- client/asset/dcr/dcr.go | 21 ++++++----- client/asset/dcr/dcr_test.go | 43 +++++++++++++--------- client/asset/dcr/simnet_test.go | 12 +++--- client/asset/interface.go | 2 +- client/asset/ltc/ltc.go | 15 +++++--- client/core/core.go | 5 ++- client/core/core_test.go | 2 +- 10 files changed, 108 insertions(+), 78 deletions(-) diff --git a/client/asset/btc/btc.go b/client/asset/btc/btc.go index 8ace555544..da00344806 100644 --- a/client/asset/btc/btc.go +++ b/client/asset/btc/btc.go @@ -90,12 +90,15 @@ var ( Description: "Bitcoin's 'fallbackfee' rate. Units: BTC/kB", DefaultValue: defaultFee * 1000 / 1e8, }, - // { - // Key: "txsplit", - // DisplayName: "Pre-split funding inputs", - // Description: "Pre-split funding inputs to prevent locking funds into an order for which a change output may not be immediately available. Only used for standing-type orders.", - // IsBoolean: true, - // }, + { + Key: "txsplit", + DisplayName: "Pre-split funding inputs", + Description: "When placing an order, create a \"split\" transaction to fund the order without locking more of the wallet balance than " + + "necessary. Otherwise, excess funds may be reserved to fund the order until the first swap contract is broadcast " + + "during match settlement, or the order is canceled. This an extra transaction for which network mining fees are paid. " + + "Used only for standing-type orders, e.g. limit orders without immediate time-in-force.", + IsBoolean: true, + }, } // walletInfo defines some general information about a Bitcoin wallet. WalletInfo = &asset.WalletInfo{ @@ -498,7 +501,7 @@ func (btc *ExchangeWallet) feeRateWithFallback() uint64 { // FundOrder selects utxos (as asset.Coin) for use in an order. Any Coins // returned will be locked. Part of the asset.Wallet interface. -func (btc *ExchangeWallet) FundOrder(value uint64, nfo *dex.Asset) (asset.Coins, error) { +func (btc *ExchangeWallet) FundOrder(value uint64, immediate bool, nfo *dex.Asset) (asset.Coins, error) { if value == 0 { return nil, fmt.Errorf("cannot fund value = 0") } @@ -557,7 +560,7 @@ out: utxos = utxos[:len(utxos)-1] } - if btc.useSplitTx { + if btc.useSplitTx && !immediate { return btc.split(value, spents, uint64(size), fundingCoins, nfo) } @@ -800,7 +803,7 @@ func (btc *ExchangeWallet) fundedTx(coins asset.Coins) (*wire.MsgTx, uint64, []s // Swap sends the swap contracts and prepares the receipts. func (btc *ExchangeWallet) Swap(swaps *asset.Swaps) ([]asset.Receipt, asset.Coin, error) { - var contracts [][]byte + contracts := make([][]byte, 0, len(swaps.Contracts)) var totalOut uint64 // Start with an empty MsgTx. baseTx, totalIn, opIDs, err := btc.fundedTx(swaps.Inputs) diff --git a/client/asset/btc/btc_test.go b/client/asset/btc/btc_test.go index a5e49df474..f91d90b8c2 100644 --- a/client/asset/btc/btc_test.go +++ b/client/asset/btc/btc_test.go @@ -397,14 +397,14 @@ func TestAvailableFund(t *testing.T) { } // Zero value - _, err = wallet.FundOrder(0, tBTC) + _, err = wallet.FundOrder(0, false, tBTC) if err == nil { t.Fatalf("no funding error for zero value") } // Nothing to spend node.rawRes[methodListUnspent] = mustMarshal(t, []struct{}{}) - _, err = wallet.FundOrder(littleOrder, tBTC) + _, err = wallet.FundOrder(littleOrder, false, tBTC) if err == nil { t.Fatalf("no error for zero utxos") } @@ -412,7 +412,7 @@ func TestAvailableFund(t *testing.T) { // RPC error node.rawErr[methodListUnspent] = tErr - _, err = wallet.FundOrder(littleOrder, tBTC) + _, err = wallet.FundOrder(littleOrder, false, tBTC) if err == nil { t.Fatalf("no funding error for rpc error") } @@ -420,7 +420,7 @@ func TestAvailableFund(t *testing.T) { // Negative response when locking outputs. node.rawRes[methodLockUnspent] = []byte(`false`) - _, err = wallet.FundOrder(littleOrder, tBTC) + _, err = wallet.FundOrder(littleOrder, false, tBTC) if err == nil { t.Fatalf("no error for lockunspent result = false: %v", err) } @@ -430,7 +430,7 @@ func TestAvailableFund(t *testing.T) { littleUTXO.Safe = false littleUTXO.Confirmations = 0 node.rawRes[methodListUnspent] = mustMarshal(t, unspents) - spendables, err := wallet.FundOrder(littleOrder, tBTC) + spendables, err := wallet.FundOrder(littleOrder, false, tBTC) if err != nil { t.Fatalf("error funding small amount: %v", err) } @@ -445,7 +445,7 @@ func TestAvailableFund(t *testing.T) { // Now with safe unconfirmed littleOrder. littleUTXO.Safe = true node.rawRes[methodListUnspent] = mustMarshal(t, unspents) - spendables, err = wallet.FundOrder(littleOrder, tBTC) + spendables, err = wallet.FundOrder(littleOrder, false, tBTC) if err != nil { t.Fatalf("error funding small amount: %v", err) } @@ -458,7 +458,7 @@ func TestAvailableFund(t *testing.T) { } // Fund a lotta bit, covered by just the lottaOrder UTXO. - spendables, err = wallet.FundOrder(lottaOrder /* hefty fees for 95 lots */, tBTC) + spendables, err = wallet.FundOrder(lottaOrder /* hefty fees for 95 lots */, false, tBTC) if err != nil { t.Fatalf("error funding large amount: %v", err) } @@ -472,7 +472,7 @@ func TestAvailableFund(t *testing.T) { // lotta bit++ to require both spendables extraLottaOrder := littleOrder + lottaOrder - spendables, err = wallet.FundOrder(extraLottaOrder, tBTC) + spendables, err = wallet.FundOrder(extraLottaOrder, false, tBTC) if err != nil { t.Fatalf("error funding large amount: %v", err) } @@ -487,7 +487,7 @@ func TestAvailableFund(t *testing.T) { // Not enough to cover transaction fees. lottaUTXO.Amount -= 1e-6 node.rawRes[methodListUnspent] = mustMarshal(t, unspents) - _, err = wallet.FundOrder(lottaOrder+littleOrder, tBTC) + _, err = wallet.FundOrder(lottaOrder+littleOrder, false, tBTC) if err == nil { t.Fatalf("no error when not enough to cover tx fees") } @@ -499,7 +499,7 @@ func TestAvailableFund(t *testing.T) { node.rawRes[methodChangeAddress] = mustMarshal(t, tP2WPKHAddr) wallet.useSplitTx = true // No error when no split performed cuz math. - coins, err := wallet.FundOrder(extraLottaOrder, tBTC) + coins, err := wallet.FundOrder(extraLottaOrder, false, tBTC) if err != nil { t.Fatalf("error for no-split split: %v", err) } @@ -509,13 +509,22 @@ func TestAvailableFund(t *testing.T) { t.Fatalf("no-split split didn't return both coins") } + // No split because not standing order. + coins, err = wallet.FundOrder(extraLottaOrder, true, tBTC) + if err != nil { + t.Fatalf("error for no-split split: %v", err) + } + if len(coins) != 2 { + t.Fatalf("no-split split didn't return both coins") + } + // With a little more locked, the split should be performed. node.signFunc = func(params []json.RawMessage) (json.RawMessage, error) { return signFunc(t, params, 0, true) } lottaUTXO.Amount += float64(baggageFees) / 1e8 node.rawRes[methodListUnspent] = mustMarshal(t, unspents) - coins, err = wallet.FundOrder(extraLottaOrder, tBTC) + coins, err = wallet.FundOrder(extraLottaOrder, false, tBTC) if err != nil { t.Fatalf("error for split tx: %v", err) } @@ -532,7 +541,7 @@ func TestAvailableFund(t *testing.T) { // GetRawChangeAddress error node.rawErr[methodChangeAddress] = tErr - _, err = wallet.FundOrder(extraLottaOrder, tBTC) + _, err = wallet.FundOrder(extraLottaOrder, false, tBTC) if err == nil { t.Fatalf("no error for split tx change addr error") } @@ -540,14 +549,14 @@ func TestAvailableFund(t *testing.T) { // SendRawTx error node.sendErr = tErr - _, err = wallet.FundOrder(extraLottaOrder, tBTC) + _, err = wallet.FundOrder(extraLottaOrder, false, tBTC) if err == nil { t.Fatalf("no error for split tx send error") } node.sendErr = nil // Success again. - _, err = wallet.FundOrder(extraLottaOrder, tBTC) + _, err = wallet.FundOrder(extraLottaOrder, false, tBTC) if err != nil { t.Fatalf("error for split tx recovery run") } @@ -696,7 +705,7 @@ func TestFundEdges(t *testing.T) { } unspents := []*ListUnspentResult{p2pkhUnspent} node.rawRes[methodListUnspent] = mustMarshal(t, unspents) - _, err := wallet.FundOrder(swapVal, tBTC) + _, err := wallet.FundOrder(swapVal, false, tBTC) if err == nil { t.Fatalf("no error when not enough funds in single p2pkh utxo") } @@ -704,7 +713,7 @@ func TestFundEdges(t *testing.T) { p2pkhUnspent.Amount = float64(swapVal+backingFees) / 1e8 node.rawRes[methodListUnspent] = mustMarshal(t, unspents) node.rawRes[methodLockUnspent] = mustMarshal(t, true) - _, err = wallet.FundOrder(swapVal, tBTC) + _, err = wallet.FundOrder(swapVal, false, tBTC) if err != nil { t.Fatalf("error when should be enough funding in single p2pkh utxo: %v", err) } @@ -732,13 +741,13 @@ func TestFundEdges(t *testing.T) { p2pkhUnspent.Amount = float64(halfSwap+backingFees-1) / 1e8 unspents = []*ListUnspentResult{p2pkhUnspent, p2shUnspent} node.rawRes[methodListUnspent] = mustMarshal(t, unspents) - _, err = wallet.FundOrder(swapVal, tBTC) + _, err = wallet.FundOrder(swapVal, false, tBTC) if err == nil { t.Fatalf("no error when not enough funds in two utxos") } p2pkhUnspent.Amount = float64(halfSwap+backingFees) / 1e8 node.rawRes[methodListUnspent] = mustMarshal(t, unspents) - _, err = wallet.FundOrder(swapVal, tBTC) + _, err = wallet.FundOrder(swapVal, false, tBTC) if err != nil { t.Fatalf("error when should be enough funding in two utxos: %v", err) } @@ -760,13 +769,13 @@ func TestFundEdges(t *testing.T) { } unspents = []*ListUnspentResult{p2wpkhUnspent} node.rawRes[methodListUnspent] = mustMarshal(t, unspents) - _, err = wallet.FundOrder(swapVal, tBTC) + _, err = wallet.FundOrder(swapVal, false, tBTC) if err == nil { t.Fatalf("no error when not enough funds in single p2wpkh utxo") } p2wpkhUnspent.Amount = float64(swapVal+backingFees) / 1e8 node.rawRes[methodListUnspent] = mustMarshal(t, unspents) - _, err = wallet.FundOrder(swapVal, tBTC) + _, err = wallet.FundOrder(swapVal, false, tBTC) if err != nil { t.Fatalf("error when should be enough funding in single p2wpkh utxo: %v", err) } @@ -792,13 +801,13 @@ func TestFundEdges(t *testing.T) { } unspents = []*ListUnspentResult{p2wpshUnspent} node.rawRes[methodListUnspent] = mustMarshal(t, unspents) - _, err = wallet.FundOrder(swapVal, tBTC) + _, err = wallet.FundOrder(swapVal, false, tBTC) if err == nil { t.Fatalf("no error when not enough funds in single p2wsh utxo") } p2wpshUnspent.Amount = float64(swapVal+backingFees) / 1e8 node.rawRes[methodListUnspent] = mustMarshal(t, unspents) - _, err = wallet.FundOrder(swapVal, tBTC) + _, err = wallet.FundOrder(swapVal, false, tBTC) if err != nil { t.Fatalf("error when should be enough funding in single p2wsh utxo: %v", err) } diff --git a/client/asset/btc/livetest/livetest.go b/client/asset/btc/livetest/livetest.go index 08536761fe..0276387b38 100644 --- a/client/asset/btc/livetest/livetest.go +++ b/client/asset/btc/livetest/livetest.go @@ -169,33 +169,33 @@ func Run(t *testing.T, newWallet WalletConstructor, address string, dexAsset *de // Gamma should only have 10 BTC utxos, so calling fund for less should only // return 1 utxo. - utxos, err := rig.gamma().FundOrder(contractValue*3, dexAsset) + utxos, err := rig.gamma().FundOrder(contractValue*3, false, dexAsset) if err != nil { t.Fatalf("Funding error: %v", err) } utxo := utxos[0] // UTXOs should be locked - utxos, _ = rig.gamma().FundOrder(contractValue*3, dexAsset) + utxos, _ = rig.gamma().FundOrder(contractValue*3, false, dexAsset) if inUTXOs(utxo, utxos) { t.Fatalf("received locked output") } rig.gamma().ReturnCoins([]asset.Coin{utxo}) rig.gamma().ReturnCoins(utxos) // Make sure we get the first utxo back with Fund. - utxos, _ = rig.gamma().FundOrder(contractValue*3, dexAsset) + utxos, _ = rig.gamma().FundOrder(contractValue*3, false, dexAsset) if !splitTx && !inUTXOs(utxo, utxos) { t.Fatalf("unlocked output not returned") } rig.gamma().ReturnCoins(utxos) // Get a separate set of UTXOs for each contract. - utxos1, err := rig.gamma().FundOrder(contractValue, dexAsset) + utxos1, err := rig.gamma().FundOrder(contractValue, false, dexAsset) if err != nil { t.Fatalf("error funding first contract: %v", err) } // Get a separate set of UTXOs for each contract. - utxos2, err := rig.gamma().FundOrder(contractValue*2, dexAsset) + utxos2, err := rig.gamma().FundOrder(contractValue*2, false, dexAsset) if err != nil { t.Fatalf("error funding second contract: %v", err) } @@ -325,7 +325,7 @@ func Run(t *testing.T, newWallet WalletConstructor, address string, dexAsset *de lockTime = time.Now().Add(-24 * time.Hour) // Have gamma send a swap contract to the alpha address. - utxos, _ = rig.gamma().FundOrder(contractValue, dexAsset) + utxos, _ = rig.gamma().FundOrder(contractValue, false, dexAsset) contract := &asset.Contract{ Address: address, Value: contractValue, diff --git a/client/asset/dcr/dcr.go b/client/asset/dcr/dcr.go index 7d95224ef5..08c78158e7 100644 --- a/client/asset/dcr/dcr.go +++ b/client/asset/dcr/dcr.go @@ -80,15 +80,18 @@ var ( { Key: fallbackFeeKey, DisplayName: "Fallback fee rate", - Description: "Decred's 'fallbackfee' rate. Units: DCR/kB", + Description: "The fee rate to use for fee payment and withdrawals when estimatesmartfee is not available. Units: DCR/kB", DefaultValue: defaultFee * 1000 / 1e8, }, - // { - // Key: "txsplit", - // DisplayName: "Pre-size funding inputs", - // Description: "Pre-size funding inputs to prevent locking funds into an order for which a change output may not be immediately available. Only used for standing-type orders.", - // IsBoolean: true, - // }, + { + Key: "txsplit", + DisplayName: "Pre-size funding inputs", + Description: "When placing an order, create a \"split\" transaction to fund the order without locking more of the wallet balance than " + + "necessary. Otherwise, excess funds may be reserved to fund the order until the first swap contract is broadcast " + + "during match settlement, or the order is canceled. This an extra transaction for which network mining fees are paid. " + + "Used only for standing-type orders, e.g. limit orders without immediate time-in-force.", + IsBoolean: true, + }, } // walletInfo defines some general information about a Decred wallet. WalletInfo = &asset.WalletInfo{ @@ -492,7 +495,7 @@ func (dcr *ExchangeWallet) feeRateWithFallback() uint64 { // FundOrder selects coins for use in an order. The coins will be locked, and // will not be returned in subsequent calls to Fund or calculated in calls to // Available, unless they are unlocked with ReturnCoins. -func (dcr *ExchangeWallet) FundOrder(value uint64, nfo *dex.Asset) (asset.Coins, error) { +func (dcr *ExchangeWallet) FundOrder(value uint64, immediate bool, nfo *dex.Asset) (asset.Coins, error) { if value == 0 { return nil, fmt.Errorf("cannot fund value = 0") } @@ -510,7 +513,7 @@ func (dcr *ExchangeWallet) FundOrder(value uint64, nfo *dex.Asset) (asset.Coins, } // Send a split, if preferred. - if dcr.useSplitTx { + if dcr.useSplitTx && !immediate { return dcr.split(value, coins, inputsSize, fundingCoins, nfo) } diff --git a/client/asset/dcr/dcr_test.go b/client/asset/dcr/dcr_test.go index 33feadd9c6..005ab13e75 100644 --- a/client/asset/dcr/dcr_test.go +++ b/client/asset/dcr/dcr_test.go @@ -388,14 +388,14 @@ func TestAvailableFund(t *testing.T) { } // Zero value - _, err = wallet.FundOrder(0, tDCR) + _, err = wallet.FundOrder(0, false, tDCR) if err == nil { t.Fatalf("no funding error for zero value") } // Nothing to spend node.unspent = nil - _, err = wallet.FundOrder(littleOrder, tDCR) + _, err = wallet.FundOrder(littleOrder, false, tDCR) if err == nil { t.Fatalf("no error for zero utxos") } @@ -403,7 +403,7 @@ func TestAvailableFund(t *testing.T) { // RPC error node.unspentErr = tErr - _, err = wallet.FundOrder(littleOrder, tDCR) + _, err = wallet.FundOrder(littleOrder, false, tDCR) if err == nil { t.Fatalf("no funding error for rpc error") } @@ -411,14 +411,14 @@ func TestAvailableFund(t *testing.T) { // Negative response when locking outputs. node.lockUnspentErr = tErr - _, err = wallet.FundOrder(littleOrder, tDCR) + _, err = wallet.FundOrder(littleOrder, false, tDCR) if err == nil { t.Fatalf("no error for lockunspent result = false: %v", err) } node.lockUnspentErr = nil // Fund a little bit. - spendables, err := wallet.FundOrder(littleOrder, tDCR) + spendables, err := wallet.FundOrder(littleOrder, false, tDCR) if err != nil { t.Fatalf("error funding small amount: %v", err) } @@ -433,7 +433,7 @@ func TestAvailableFund(t *testing.T) { // Now confirm the little bit and have it selected. unspents[0].Confirmations++ littleUTXO.Confirmations++ // for consistency (no indirection from unspents) - spendables, err = wallet.FundOrder(littleOrder, tDCR) + spendables, err = wallet.FundOrder(littleOrder, false, tDCR) if err != nil { t.Fatalf("error funding small amount: %v", err) } @@ -446,7 +446,7 @@ func TestAvailableFund(t *testing.T) { } // Fund a lotta bit. - spendables, err = wallet.FundOrder(lottaOrder, tDCR) + spendables, err = wallet.FundOrder(lottaOrder, false, tDCR) if err != nil { t.Fatalf("error funding large amount: %v", err) } @@ -461,7 +461,7 @@ func TestAvailableFund(t *testing.T) { // Not enough to cover transaction fees. littleUTXO.Amount -= 1e-7 node.unspent = []walletjson.ListUnspentResult{littleUTXO, lottaUTXO} - _, err = wallet.FundOrder(lottaOrder+littleOrder, tDCR) + _, err = wallet.FundOrder(lottaOrder+littleOrder, false, tDCR) if err == nil { t.Fatalf("no error when not enough to cover tx fees") } @@ -474,7 +474,7 @@ func TestAvailableFund(t *testing.T) { extraLottaOrder := littleOrder + lottaOrder wallet.useSplitTx = true // No split performed due to economics is not an error. - coins, err := wallet.FundOrder(extraLottaOrder, tDCR) + coins, err := wallet.FundOrder(extraLottaOrder, false, tDCR) if err != nil { t.Fatalf("error for no-split split: %v", err) } @@ -484,10 +484,19 @@ func TestAvailableFund(t *testing.T) { t.Fatalf("no-split split didn't return both coins") } + // No split because not standing order. + coins, err = wallet.FundOrder(extraLottaOrder, true, tDCR) + if err != nil { + t.Fatalf("error for no-split split: %v", err) + } + if len(coins) != 2 { + t.Fatalf("no-split split didn't return both coins") + } + // With a little more locked, the split should be performed. lottaUTXO.Amount += float64(baggageFees) / 1e8 node.unspent = []walletjson.ListUnspentResult{littleUTXO, lottaUTXO} - coins, err = wallet.FundOrder(extraLottaOrder, tDCR) + coins, err = wallet.FundOrder(extraLottaOrder, false, tDCR) if err != nil { t.Fatalf("error for split tx: %v", err) } @@ -504,7 +513,7 @@ func TestAvailableFund(t *testing.T) { // GetRawChangeAddress error node.changeAddrErr = tErr - _, err = wallet.FundOrder(extraLottaOrder, tDCR) + _, err = wallet.FundOrder(extraLottaOrder, false, tDCR) if err == nil { t.Fatalf("no error for split tx change addr error") } @@ -512,14 +521,14 @@ func TestAvailableFund(t *testing.T) { // SendRawTx error node.sendRawErr = tErr - _, err = wallet.FundOrder(extraLottaOrder, tDCR) + _, err = wallet.FundOrder(extraLottaOrder, false, tDCR) if err == nil { t.Fatalf("no error for split tx send error") } node.sendRawErr = nil // Success again. - _, err = wallet.FundOrder(extraLottaOrder, tDCR) + _, err = wallet.FundOrder(extraLottaOrder, false, tDCR) if err != nil { t.Fatalf("error for split tx recovery run") } @@ -693,14 +702,14 @@ func TestFundEdges(t *testing.T) { } node.unspent = []walletjson.ListUnspentResult{p2pkhUnspent} - _, err := wallet.FundOrder(swapVal, tDCR) + _, err := wallet.FundOrder(swapVal, false, tDCR) if err == nil { t.Fatalf("no error when not enough funds in single p2pkh utxo") } // Now add the needed atoms and try again. p2pkhUnspent.Amount = float64(swapVal+fees) / 1e8 node.unspent = []walletjson.ListUnspentResult{p2pkhUnspent} - _, err = wallet.FundOrder(swapVal, tDCR) + _, err = wallet.FundOrder(swapVal, false, tDCR) if err != nil { t.Fatalf("should be enough to fund with a single p2pkh utxo: %v", err) } @@ -731,13 +740,13 @@ func TestFundEdges(t *testing.T) { // } // p2pkhUnspent.Amount = float64(halfSwap+fees-1) / 1e8 // node.unspent = []walletjson.ListUnspentResult{p2pkhUnspent, p2shUnspent} - // _, err = wallet.FundOrder(swapVal, tDCR) + // _, err = wallet.FundOrder(swapVal, false, tDCR) // if err == nil { // t.Fatalf("no error when not enough funds in two utxos") // } // p2pkhUnspent.Amount = float64(halfSwap+fees) / 1e8 // node.unspent = []walletjson.ListUnspentResult{p2pkhUnspent, p2shUnspent} - // _, err = wallet.FundOrder(swapVal, tDCR) + // _, err = wallet.FundOrder(swapVal, false, tDCR) // if err != nil { // t.Fatalf("error when should be enough funding in two utxos: %v", err) // } diff --git a/client/asset/dcr/simnet_test.go b/client/asset/dcr/simnet_test.go index 7eefcaffba..bced3c641e 100644 --- a/client/asset/dcr/simnet_test.go +++ b/client/asset/dcr/simnet_test.go @@ -190,33 +190,33 @@ func runTest(t *testing.T, splitTx bool) { } // Grab some coins. - utxos, err := rig.beta().FundOrder(contractValue*3, tDCR) + utxos, err := rig.beta().FundOrder(contractValue*3, false, tDCR) if err != nil { t.Fatalf("Funding error: %v", err) } utxo := utxos[0] // Coins should be locked - utxos, _ = rig.beta().FundOrder(contractValue*3, tDCR) + utxos, _ = rig.beta().FundOrder(contractValue*3, false, tDCR) if !splitTx && inUTXOs(utxo, utxos) { t.Fatalf("received locked output") } rig.beta().ReturnCoins([]asset.Coin{utxo}) rig.beta().ReturnCoins(utxos) // Make sure we get the first utxo back with Fund. - utxos, _ = rig.beta().FundOrder(contractValue*3, tDCR) + utxos, _ = rig.beta().FundOrder(contractValue*3, false, tDCR) if !splitTx && !inUTXOs(utxo, utxos) { t.Fatalf("unlocked output not returned") } rig.beta().ReturnCoins(utxos) // Get a separate set of UTXOs for each contract. - utxos1, err := rig.beta().FundOrder(contractValue, tDCR) + utxos1, err := rig.beta().FundOrder(contractValue, false, tDCR) if err != nil { t.Fatalf("error funding first contract: %v", err) } // Get a separate set of UTXOs for each contract. - utxos2, err := rig.beta().FundOrder(contractValue*2, tDCR) + utxos2, err := rig.beta().FundOrder(contractValue*2, false, tDCR) if err != nil { t.Fatalf("error funding second contract: %v", err) } @@ -347,7 +347,7 @@ func runTest(t *testing.T, splitTx bool) { lockTime = time.Now().Add(-24 * time.Hour) // Have beta send a swap contract to the alpha address. - utxos, _ = rig.beta().FundOrder(contractValue, tDCR) + utxos, _ = rig.beta().FundOrder(contractValue, false, tDCR) contract := &asset.Contract{ Address: alphaAddress, Value: contractValue, diff --git a/client/asset/interface.go b/client/asset/interface.go index cbd720c73e..139cdfcb35 100644 --- a/client/asset/interface.go +++ b/client/asset/interface.go @@ -72,7 +72,7 @@ type Wallet interface { // Fund selects coins for use in an order. The coins will be locked, and will // not be returned in subsequent calls to Fund or calculated in calls to // Available, unless they are unlocked with ReturnCoins. - FundOrder(uint64, *dex.Asset) (Coins, error) + FundOrder(value uint64, immediate bool, nfo *dex.Asset) (Coins, error) // ReturnCoins unlocks coins. This would be necessary in the case of a // canceled order. ReturnCoins(Coins) error diff --git a/client/asset/ltc/ltc.go b/client/asset/ltc/ltc.go index 11c4a9c143..d23b1a155b 100644 --- a/client/asset/ltc/ltc.go +++ b/client/asset/ltc/ltc.go @@ -57,12 +57,15 @@ var ( Description: "Litecoin's 'fallbackfee' rate. Units: LTC/kB", DefaultValue: defaultFee * 1000 / 1e8, }, - // { - // Key: "txsplit", - // DisplayName: "Pre-split funding inputs", - // Description: "Pre-split funding inputs to prevent locking funds into an order for which a change output may not be immediately available. Only used for standing-type orders.", - // IsBoolean: true, - // }, + { + Key: "txsplit", + DisplayName: "Pre-split funding inputs", + Description: "When placing an order, create a \"split\" transaction to fund the order without locking more of the wallet balance than " + + "necessary. Otherwise, excess funds may be reserved to fund the order until the first swap contract is broadcast " + + "during match settlement, or the order is canceled. This an extra transaction for which network mining fees are paid. " + + "Used only for standing-type orders, e.g. limit orders without immediate time-in-force.", + IsBoolean: true, + }, } // WalletInfo defines some general information about a Litecoin wallet. WalletInfo = &asset.WalletInfo{ diff --git a/client/core/core.go b/client/core/core.go index 60f57d957d..bfd04055cb 100644 --- a/client/core/core.go +++ b/client/core/core.go @@ -1827,7 +1827,10 @@ func (c *Core) prepareTrackedTrade(dc *dexConnection, form *TradeForm, crypter e if form.IsLimit && !form.Sell { fundQty = calc.BaseToQuote(rate, fundQty) } - coins, err := fromWallet.FundOrder(fundQty, wallets.fromAsset) + + isImmediate := (!form.IsLimit || form.TifNow) + + coins, err := fromWallet.FundOrder(fundQty, isImmediate, wallets.fromAsset) if err != nil { return nil, 0, err } diff --git a/client/core/core_test.go b/client/core/core_test.go index 6ed0af5482..f5d105df91 100644 --- a/client/core/core_test.go +++ b/client/core/core_test.go @@ -513,7 +513,7 @@ func (w *TXCWallet) FeeRate() (uint64, error) { return 24, nil } -func (w *TXCWallet) FundOrder(v uint64, _ *dex.Asset) (asset.Coins, error) { +func (w *TXCWallet) FundOrder(v uint64, _ bool, _ *dex.Asset) (asset.Coins, error) { w.fundedVal = v return w.fundCoins, w.fundErr } From 6698f26385c50388eca03faada6c2499898f3113 Mon Sep 17 00:00:00 2001 From: buck54321 Date: Tue, 28 Jul 2020 18:44:15 -0500 Subject: [PATCH 3/5] fewer string keys --- client/asset/btc/btc.go | 183 ++++++++++++++--------------- client/asset/btc/btc_test.go | 10 +- client/asset/btc/walletclient.go | 4 +- client/asset/dcr/dcr.go | 190 ++++++++++++++++--------------- client/asset/dcr/dcr_test.go | 32 +++--- 5 files changed, 208 insertions(+), 211 deletions(-) diff --git a/client/asset/btc/btc.go b/client/asset/btc/btc.go index da00344806..52ab842d6c 100644 --- a/client/asset/btc/btc.go +++ b/client/asset/btc/btc.go @@ -50,7 +50,7 @@ const ( minProtocolVersion = 70015 // splitTxBaggage is the total number of additional bytes associated with // using a split transaction to fund a swap. - splitTxBaggage = dexbtc.MimimumTxOverhead + 1 + dexbtc.RedeemP2WPKHInputSize + 2*(1+dexbtc.P2PKHOutputSize) + splitTxBaggage = dexbtc.MimimumTxOverhead + dexbtc.RedeemP2WPKHInputSize + 2*dexbtc.P2PKHOutputSize ) var ( @@ -135,16 +135,18 @@ type BTCCloneCFG struct { DefaultFallbackFee uint64 // sats/byte } -// outpointID creates a unique string for a transaction output. -func outpointID(txid string, vout uint32) string { - return txid + ":" + strconv.Itoa(int(vout)) +// newOutPoint is the constructor for a new wire.OutPoint (non-pointer). +func newOutPoint(txHash *chainhash.Hash, vout uint32) wire.OutPoint { + return wire.OutPoint{ + Hash: *txHash, + Index: vout, + } } // output is information about a transaction output. output satisfies the // asset.Coin interface. type output struct { - txHash chainhash.Hash - vout uint32 + pt wire.OutPoint value uint64 redeem dex.Bytes node rpcClient // for calculating confirmations. @@ -153,8 +155,7 @@ type output struct { // newOutput is the constructor for an output. func newOutput(node rpcClient, txHash *chainhash.Hash, vout uint32, value uint64, redeem dex.Bytes) *output { return &output{ - txHash: *txHash, - vout: vout, + pt: newOutPoint(txHash, vout), value: value, redeem: redeem, node: node, @@ -171,7 +172,7 @@ func (op *output) Value() uint64 { // and will return an error if the output has been spent. Part of the // asset.Coin interface. func (op *output) Confirmations() (uint32, error) { - txOut, err := op.node.GetTxOut(&op.txHash, op.vout, true) + txOut, err := op.node.GetTxOut(op.txHash(), op.vout(), true) if err != nil { return 0, fmt.Errorf("error finding coin: %v", err) } @@ -184,12 +185,12 @@ func (op *output) Confirmations() (uint32, error) { // ID is the output's coin ID. Part of the asset.Coin interface. For BTC, the // coin ID is 36 bytes = 32 bytes tx hash + 4 bytes big-endian vout. func (op *output) ID() dex.Bytes { - return toCoinID(&op.txHash, op.vout) + return toCoinID(op.txHash(), op.vout()) } // String is a string representation of the coin. func (op *output) String() string { - return fmt.Sprintf("%s:%v", op.txHash, op.vout) + return op.pt.Hash.String() + ":" + strconv.Itoa(int(op.pt.Index)) } // Redeem is any known redeem script required to spend this output. Part of the @@ -198,6 +199,16 @@ func (op *output) Redeem() dex.Bytes { return op.redeem } +// txHash returns the pointer of the wire.OutPoint's Hash. +func (op *output) txHash() *chainhash.Hash { + return &op.pt.Hash +} + +// vout returns the wire.OutPoint's Index. +func (op *output) vout() uint32 { + return op.pt.Index +} + // auditInfo is information about a swap contract on that blockchain, not // necessarily created by this wallet, as would be returned from AuditContract. // auditInfo satisfies the asset.AuditInfo interface. @@ -300,16 +311,10 @@ type ExchangeWallet struct { fallbackFeeRate uint64 useSplitTx bool - // In the future, the client may wish to specify minimum confirmations for - // utxos to fund orders, and allowing change outputs from DEX-related swap - // transaction may be part of this client policy. - changeMtx sync.RWMutex - tradeChange map[string]time.Time // TODO: delete fully confirmed change - // Coins returned by Fund are cached for quick reference and for cleanup on // shutdown. fundingMtx sync.RWMutex - fundingCoins map[string]*compositeUTXO + fundingCoins map[wire.OutPoint]*compositeUTXO } // Check that ExchangeWallet satisfies the Wallet interface. @@ -395,9 +400,8 @@ func newWallet(cfg *BTCCloneCFG, btcCfg *dexbtc.Config, node rpcClient) *Exchang symbol: cfg.Symbol, chainParams: cfg.ChainParams, log: cfg.Logger, - tradeChange: make(map[string]time.Time), tipChange: cfg.WalletCFG.TipChange, - fundingCoins: make(map[string]*compositeUTXO), + fundingCoins: make(map[wire.OutPoint]*compositeUTXO), minNetworkVersion: cfg.MinNetworkVersion, fallbackFeeRate: fallbackFeesPerByte, useSplitTx: btcCfg.UseSplitTx, @@ -518,7 +522,7 @@ func (btc *ExchangeWallet) FundOrder(value uint64, immediate bool, nfo *dex.Asse var size uint32 var coins asset.Coins var spents []*output - fundingCoins := make(map[string]*compositeUTXO) + fundingCoins := make(map[wire.OutPoint]*compositeUTXO) // TODO: For the chained swaps, make sure that contract outputs are P2WSH, // and that change outputs that fund further swaps are P2WPKH. @@ -534,7 +538,7 @@ func (btc *ExchangeWallet) FundOrder(value uint64, immediate bool, nfo *dex.Asse coins = append(coins, op) spents = append(spents, op) size += unspent.input.VBytes() - fundingCoins[op.String()] = unspent + fundingCoins[op.pt] = unspent sum += v } @@ -570,8 +574,8 @@ out: } btc.fundingMtx.Lock() - for opID, utxo := range fundingCoins { - btc.fundingCoins[opID] = utxo + for pt, utxo := range fundingCoins { + btc.fundingCoins[pt] = utxo } btc.fundingMtx.Unlock() @@ -585,22 +589,28 @@ out: // there is no error, and the input coins will be returned unmodified, but an // info message will be logged. // -// A split transaction nets 1 extra transaction, 1 extra signed p2pkh-spending -// input, and two additional p2pkh outputs. If the fees associated with this -// extra baggage are more than the excess amount that would be locked if a split -// transaction were not used, then the split transaction is pointless. This -// might be common, for instance, if an order is canceled partially filled, and -// then the remainder resubmitted. We would already have an output of just the -// right size, and that would be recognized here. -func (btc *ExchangeWallet) split(value uint64, outputs []*output, inputsSize uint64, fundingCoins map[string]*compositeUTXO, nfo *dex.Asset) (asset.Coins, error) { +// A split transaction nets additional network bytes consisting of +// - overhead from 1 transaction +// - 1 extra signed p2wpkh-spending input. The split tx has the fundingCoins as +// inputs now, but we'll add the input that spends the sized coin that will go +// into the first swap +// - 2 additional p2wpkh outputs for the split tx sized output and change +// +// If the fees associated with this extra baggage are more than the excess +// amount that would be locked if a split transaction were not used, then the +// split transaction is pointless. This might be common, for instance, if an +// order is canceled partially filled, and then the remainder resubmitted. We +// would already have an output of just the right size, and that would be +// recognized here. +func (btc *ExchangeWallet) split(value uint64, outputs []*output, inputsSize uint64, fundingCoins map[wire.OutPoint]*compositeUTXO, nfo *dex.Asset) (asset.Coins, error) { var err error defer func() { if err != nil { return } btc.fundingMtx.Lock() - for _, fCoin := range fundingCoins { - btc.fundingCoins[outpointID(fCoin.txHash.String(), fCoin.vout)] = fCoin + for pt, fCoin := range fundingCoins { + btc.fundingCoins[pt] = fCoin } btc.fundingMtx.Unlock() err = btc.wallet.LockUnspent(false, outputs) @@ -661,16 +671,16 @@ func (btc *ExchangeWallet) split(value uint64, outputs []*output, inputsSize uin if err != nil { return nil, fmt.Errorf("error reading asset info: %v", err) } - fundingCoins = map[string]*compositeUTXO{outpointID(txHash.String(), 0): { - txHash: &op.txHash, - vout: op.vout, + fundingCoins = map[wire.OutPoint]*compositeUTXO{op.pt: { + txHash: op.txHash(), + vout: op.vout(), address: addr.String(), redeemScript: nil, amount: reqFunds, input: spendInfo, }} - btc.log.Infof("sent split transaction %s to accommodate swap of size %d + fees = %d", op.txHash, value, reqFunds) + btc.log.Infof("sent split transaction %s to accommodate swap of size %d + fees = %d", op.txHash(), value, reqFunds) // Assign to coins so the deferred function will lock the output. outputs = []*output{op} @@ -693,7 +703,7 @@ func (btc *ExchangeWallet) ReturnCoins(unspents asset.Coins) error { return fmt.Errorf("error converting coin: %v", err) } ops = append(ops, op) - delete(btc.fundingCoins, outpointID(op.txHash.String(), op.vout)) + delete(btc.fundingCoins, op.pt) } return btc.wallet.LockUnspent(true, ops) } @@ -704,7 +714,7 @@ func (btc *ExchangeWallet) ReturnCoins(unspents asset.Coins) error { func (btc *ExchangeWallet) FundingCoins(ids []dex.Bytes) (asset.Coins, error) { // First check if we have the coins in cache. coins := make(asset.Coins, 0, len(ids)) - notFound := make(map[string]struct{}) + notFound := make(map[wire.OutPoint]struct{}) btc.fundingMtx.RLock() for _, id := range ids { txHash, vout, err := decodeCoinID(id) @@ -712,13 +722,13 @@ func (btc *ExchangeWallet) FundingCoins(ids []dex.Bytes) (asset.Coins, error) { btc.fundingMtx.RUnlock() return nil, err } - opID := outpointID(txHash.String(), vout) - fundingCoin, found := btc.fundingCoins[opID] + pt := newOutPoint(txHash, vout) + fundingCoin, found := btc.fundingCoins[pt] if found { coins = append(coins, newOutput(btc.node, txHash, vout, fundingCoin.amount, fundingCoin.redeemScript)) continue } - notFound[opID] = struct{}{} + notFound[pt] = struct{}{} } btc.fundingMtx.RUnlock() if len(notFound) == 0 { @@ -736,24 +746,24 @@ func (btc *ExchangeWallet) FundingCoins(ids []dex.Bytes) (asset.Coins, error) { if err != nil { return nil, err } - opID := outpointID(txHash.String(), vout) - utxo, found := utxoMap[opID] + pt := newOutPoint(txHash, vout) + utxo, found := utxoMap[pt] if !found { - return nil, fmt.Errorf("funding coin %s not found", opID) + return nil, fmt.Errorf("funding coin %s not found", pt.String()) } - btc.fundingCoins[opID] = utxo + btc.fundingCoins[pt] = utxo coin := newOutput(btc.node, utxo.txHash, utxo.vout, utxo.amount, utxo.redeemScript) coins = append(coins, coin) lockers = append(lockers, coin) - delete(notFound, opID) + delete(notFound, pt) if len(notFound) == 0 { break } } if len(notFound) != 0 { ids := make([]string, 0, len(notFound)) - for opID := range notFound { - ids = append(ids, opID) + for pt := range notFound { + ids = append(ids, pt.String()) } return nil, fmt.Errorf("coins not found: %s", strings.Join(ids, ", ")) } @@ -779,26 +789,25 @@ func (btc *ExchangeWallet) Lock() error { } // fundedTx creates and returns a new MsgTx with the provided coins as inputs. -func (btc *ExchangeWallet) fundedTx(coins asset.Coins) (*wire.MsgTx, uint64, []string, error) { +func (btc *ExchangeWallet) fundedTx(coins asset.Coins) (*wire.MsgTx, uint64, []wire.OutPoint, error) { baseTx := wire.NewMsgTx(wire.TxVersion) var totalIn uint64 // Add the funding utxos. - opIDs := make([]string, 0, len(coins)) + pts := make([]wire.OutPoint, 0, len(coins)) for _, coin := range coins { op, err := btc.convertCoin(coin) if err != nil { return nil, 0, nil, fmt.Errorf("error converting coin: %v", err) } if op.value == 0 { - return nil, 0, nil, fmt.Errorf("zero-valued output detected for %s:%d", op.txHash, op.vout) + return nil, 0, nil, fmt.Errorf("zero-valued output detected for %s:%d", op.txHash(), op.vout()) } totalIn += op.value - prevOut := wire.NewOutPoint(&op.txHash, op.vout) - txIn := wire.NewTxIn(prevOut, []byte{}, nil) + txIn := wire.NewTxIn(&op.pt, []byte{}, nil) baseTx.AddTxIn(txIn) - opIDs = append(opIDs, outpointID(op.txHash.String(), op.vout)) + pts = append(pts, op.pt) } - return baseTx, totalIn, opIDs, nil + return baseTx, totalIn, pts, nil } // Swap sends the swap contracts and prepares the receipts. @@ -806,7 +815,7 @@ func (btc *ExchangeWallet) Swap(swaps *asset.Swaps) ([]asset.Receipt, asset.Coin contracts := make([][]byte, 0, len(swaps.Contracts)) var totalOut uint64 // Start with an empty MsgTx. - baseTx, totalIn, opIDs, err := btc.fundedTx(swaps.Inputs) + baseTx, totalIn, pts, err := btc.fundedTx(swaps.Inputs) if err != nil { return nil, nil, err } @@ -897,9 +906,9 @@ func (btc *ExchangeWallet) Swap(swaps *asset.Swaps) ([]asset.Receipt, asset.Coin return nil, nil, fmt.Errorf("error getting spend info: %v", err) } - btc.fundingCoins[outpointID(change.txHash.String(), change.vout)] = &compositeUTXO{ - txHash: &change.txHash, - vout: change.vout, + btc.fundingCoins[change.pt] = &compositeUTXO{ + txHash: change.txHash(), + vout: change.vout(), address: changeAddr.String(), redeemScript: nil, amount: change.value, @@ -908,8 +917,8 @@ func (btc *ExchangeWallet) Swap(swaps *asset.Swaps) ([]asset.Receipt, asset.Coin } // Delete the UTXOs from the cache. - for _, opID := range opIDs { - delete(btc.fundingCoins, opID) + for _, pt := range pts { + delete(btc.fundingCoins, pt) } return receipts, changeCoin, nil @@ -939,8 +948,7 @@ func (btc *ExchangeWallet) Redeem(redemptions []*asset.Redemption) ([]dex.Bytes, } addresses = append(addresses, receiver) contracts = append(contracts, cinfo.output.redeem) - prevOut := wire.NewOutPoint(&cinfo.output.txHash, cinfo.output.vout) - txIn := wire.NewTxIn(prevOut, []byte{}, nil) + txIn := wire.NewTxIn(&cinfo.output.pt, []byte{}, nil) // Enable locktime // https://github.com/bitcoin/bips/blob/master/bip-0125.mediawiki#Spending_wallet_policy txIn.Sequence = wire.MaxTxInSequenceNum - 1 @@ -994,7 +1002,6 @@ func (btc *ExchangeWallet) Redeem(redemptions []*asset.Redemption) ([]dex.Bytes, "expected %s, got %s", *txHash, checkHash) } // Log the change output. - btc.addChange(txHash.String(), 0) coinIDs := make([]dex.Bytes, 0, len(redemptions)) for i := range redemptions { coinIDs = append(coinIDs, toCoinID(txHash, uint32(i))) @@ -1011,7 +1018,7 @@ func (btc *ExchangeWallet) SignMessage(coin asset.Coin, msg dex.Bytes) (pubkeys, return nil, nil, fmt.Errorf("error converting coin: %v", err) } btc.fundingMtx.RLock() - utxo := btc.fundingCoins[outpointID(op.txHash.String(), op.vout)] + utxo := btc.fundingCoins[op.pt] btc.fundingMtx.RUnlock() if utxo == nil { return nil, nil, fmt.Errorf("no utxo found for %s", op) @@ -1527,7 +1534,6 @@ func (btc *ExchangeWallet) sendWithReturn(baseTx *wire.MsgTx, addr btcutil.Addre var change *output if changeAdded { - btc.addChange(txHash.String(), uint32(changeIdx)) change = newOutput(btc.node, txHash, uint32(changeIdx), uint64(changeOutput.Value), nil) } else { changeScript = nil @@ -1562,7 +1568,7 @@ type compositeUTXO struct { // spendableUTXOs filters the RPC utxos for those that are spendable with with // regards to the DEX's configuration, and considered safe to spend according to // confirmations and coin source. The UTXOs will be sorted by ascending value. -func (btc *ExchangeWallet) spendableUTXOs(confs uint32) ([]*compositeUTXO, map[string]*compositeUTXO, uint64, error) { +func (btc *ExchangeWallet) spendableUTXOs(confs uint32) ([]*compositeUTXO, map[wire.OutPoint]*compositeUTXO, uint64, error) { unspents, err := btc.wallet.ListUnspent() if err != nil { return nil, nil, 0, err @@ -1570,7 +1576,7 @@ func (btc *ExchangeWallet) spendableUTXOs(confs uint32) ([]*compositeUTXO, map[s sort.Slice(unspents, func(i, j int) bool { return unspents[i].Amount < unspents[j].Amount }) var sum uint64 utxos := make([]*compositeUTXO, 0, len(unspents)) - utxoMap := make(map[string]*compositeUTXO, len(unspents)) + utxoMap := make(map[wire.OutPoint]*compositeUTXO, len(unspents)) for _, txout := range unspents { if txout.Confirmations >= confs && txout.Safe { nfo, err := dexbtc.InputInfo(txout.ScriptPubKey, txout.RedeemScript, btc.chainParams) @@ -1590,7 +1596,7 @@ func (btc *ExchangeWallet) spendableUTXOs(confs uint32) ([]*compositeUTXO, map[s input: nfo, } utxos = append(utxos, utxo) - utxoMap[outpointID(txout.TxID, txout.Vout)] = utxo + utxoMap[newOutPoint(txHash, txout.Vout)] = utxo sum += toSatoshi(txout.Amount) } } @@ -1606,24 +1612,24 @@ func (btc *ExchangeWallet) lockedSats() (uint64, error) { var sum uint64 btc.fundingMtx.Lock() defer btc.fundingMtx.Unlock() - for _, outPoint := range lockedOutpoints { - opID := outpointID(outPoint.TxID, outPoint.Vout) - utxo, found := btc.fundingCoins[opID] + for _, rpcOP := range lockedOutpoints { + txHash, err := chainhash.NewHashFromStr(rpcOP.TxID) + if err != nil { + return 0, err + } + pt := newOutPoint(txHash, rpcOP.Vout) + utxo, found := btc.fundingCoins[pt] if found { sum += utxo.amount continue } - txHash, err := chainhash.NewHashFromStr(outPoint.TxID) - if err != nil { - return 0, err - } - txOut, err := btc.node.GetTxOut(txHash, outPoint.Vout, true) + txOut, err := btc.node.GetTxOut(txHash, rpcOP.Vout, true) if err != nil { return 0, err } if txOut == nil { // Must be spent now? - btc.log.Debugf("ignoring output from listlockunspent that wasn't found with gettxout. %s", opID) + btc.log.Debugf("ignoring output from listlockunspent that wasn't found with gettxout. %s", pt) continue } sum += toSatoshi(txOut.Value) @@ -1631,23 +1637,6 @@ func (btc *ExchangeWallet) lockedSats() (uint64, error) { return sum, nil } -// addChange adds the output to the list of DEX-trade change outputs. These -// outputs are tracked because the client may wish to exempt change outputs from -// funding coin confirmation requirements. -func (btc *ExchangeWallet) addChange(txid string, vout uint32) { - btc.changeMtx.Lock() - defer btc.changeMtx.Unlock() - btc.tradeChange[outpointID(txid, vout)] = time.Now() -} - -// isDEXChange checks whether the specified output is change from a DEX trade. -func (btc *ExchangeWallet) isDEXChange(txid string, vout uint32) bool { - btc.changeMtx.RLock() - _, found := btc.tradeChange[outpointID(txid, vout)] - btc.changeMtx.RUnlock() - return found -} - // wireBytes dumps the serialized transaction bytes. func (btc *ExchangeWallet) wireBytes(tx *wire.MsgTx) []byte { buf := bytes.NewBuffer(make([]byte, 0, tx.SerializeSize())) diff --git a/client/asset/btc/btc_test.go b/client/asset/btc/btc_test.go index f91d90b8c2..7fdb65ee21 100644 --- a/client/asset/btc/btc_test.go +++ b/client/asset/btc/btc_test.go @@ -643,7 +643,7 @@ func TestFundingCoins(t *testing.T) { ensureErr := func(tag string) { // Clear the cache. - wallet.fundingCoins = make(map[string]*compositeUTXO) + wallet.fundingCoins = make(map[wire.OutPoint]*compositeUTXO) _, err := wallet.FundingCoins(coinIDs) if err == nil { t.Fatalf("%s: no error", tag) @@ -1048,9 +1048,9 @@ func TestSignMessage(t *testing.T) { } sig := signature.Serialize() - opID := outpointID(tTxID, vout) + pt := newOutPoint(tTxHash, vout) utxo := &compositeUTXO{address: tP2PKHAddr} - wallet.fundingCoins[opID] = utxo + wallet.fundingCoins[pt] = utxo node.rawRes[methodPrivKeyForAddress] = mustMarshal(t, wif.String()) node.signMsgFunc = func(params []json.RawMessage) (json.RawMessage, error) { if len(params) != 2 { @@ -1094,12 +1094,12 @@ func TestSignMessage(t *testing.T) { } // Unknown UTXO - delete(wallet.fundingCoins, opID) + delete(wallet.fundingCoins, pt) _, _, err = wallet.SignMessage(coin, msg) if err == nil { t.Fatalf("no error for unknown utxo") } - wallet.fundingCoins[opID] = utxo + wallet.fundingCoins[pt] = utxo // dumpprivkey error node.rawErr[methodPrivKeyForAddress] = tErr diff --git a/client/asset/btc/walletclient.go b/client/asset/btc/walletclient.go index aed7f7e512..9ecad8daf1 100644 --- a/client/asset/btc/walletclient.go +++ b/client/asset/btc/walletclient.go @@ -73,8 +73,8 @@ func (wc *walletClient) LockUnspent(unlock bool, ops []*output) error { var rpcops []*RPCOutpoint // To clear all, this must be nil, not empty slice. for _, op := range ops { rpcops = append(rpcops, &RPCOutpoint{ - TxID: op.txHash.String(), - Vout: op.vout, + TxID: op.txHash().String(), + Vout: op.vout(), }) } var success bool diff --git a/client/asset/dcr/dcr.go b/client/asset/dcr/dcr.go index 08c78158e7..e12f06b480 100644 --- a/client/asset/dcr/dcr.go +++ b/client/asset/dcr/dcr.go @@ -43,7 +43,7 @@ const ( defaultFee = 20 // splitTxBaggage is the total number of additional bytes associated with // using a split transaction to fund a swap. - splitTxBaggage = dexdcr.MsgTxOverhead + 1 + dexdcr.P2PKHInputSize + 2*(1+dexdcr.P2PKHOutputSize) + splitTxBaggage = dexdcr.MsgTxOverhead + dexdcr.P2PKHInputSize + 2*dexdcr.P2PKHOutputSize ) var ( @@ -126,16 +126,29 @@ type rpcClient interface { Disconnected() bool } -// outpointID creates a unique string for a transaction output. -func outpointID(txHash *chainhash.Hash, vout uint32) string { - return txHash.String() + ":" + strconv.Itoa(int(vout)) +// outPoint is the hash and output index of a transaction output. +type outPoint struct { + txHash chainhash.Hash + vout uint32 +} + +// newOutPoint is the constructor for a new outPoint. +func newOutPoint(txHash *chainhash.Hash, vout uint32) outPoint { + return outPoint{ + txHash: *txHash, + vout: vout, + } +} + +// String is a human-readable string representation of the outPoint. +func (pt *outPoint) String() string { + return pt.txHash.String() + ":" + strconv.Itoa(int(pt.vout)) } // output is information about a transaction output. output satisfies the // asset.Coin interface. type output struct { - txHash chainhash.Hash - vout uint32 + pt outPoint value uint64 redeem dex.Bytes tree int8 @@ -145,8 +158,10 @@ type output struct { // newOutput is the constructor for an output. func newOutput(node rpcClient, txHash *chainhash.Hash, vout uint32, value uint64, tree int8, redeem dex.Bytes) *output { return &output{ - txHash: *txHash, - vout: vout, + pt: outPoint{ + txHash: *txHash, + vout: vout, + }, value: value, redeem: redeem, tree: tree, @@ -164,7 +179,7 @@ func (op *output) Value() uint64 { // and will return an error if the output has been spent. Part of the // asset.Coin interface. func (op *output) Confirmations() (uint32, error) { - txOut, err := op.node.GetTxOut(&op.txHash, op.vout, true) + txOut, err := op.node.GetTxOut(op.txHash(), op.vout(), true) if err != nil { return 0, fmt.Errorf("error finding unspent contract: %v", err) } @@ -177,12 +192,12 @@ func (op *output) Confirmations() (uint32, error) { // ID is the output's coin ID. Part of the asset.Coin interface. For DCR, the // coin ID is 36 bytes = 32 bytes tx hash + 4 bytes big-endian vout. func (op *output) ID() dex.Bytes { - return toCoinID(&op.txHash, op.vout) + return toCoinID(op.txHash(), op.vout()) } // String is a string representation of the coin. func (op *output) String() string { - return outpointID(&op.txHash, op.vout) + return op.pt.String() } // Redeem is any known redeem script required to spend this output. Part of the @@ -191,6 +206,21 @@ func (op *output) Redeem() dex.Bytes { return op.redeem } +// txHash returns the pointer of the outPoint's txHash. +func (op *output) txHash() *chainhash.Hash { + return &op.pt.txHash +} + +// vout returns the outPoint's vout. +func (op *output) vout() uint32 { + return op.pt.vout +} + +// wireOutPoint creates and returns a new *wire.OutPoint for the output. +func (op *output) wireOutPoint() *wire.OutPoint { + return wire.NewOutPoint(op.txHash(), op.vout(), op.tree) +} + // 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. @@ -298,17 +328,11 @@ type ExchangeWallet struct { fallbackFeeRate uint64 useSplitTx bool - // In the future, the client may wish to specify minimum confirmations for - // utxos to fund orders, and allowing change outputs from DEX-related swap - // transaction may be part of this client policy. - changeMtx sync.RWMutex - tradeChange map[string]time.Time // TODO: delete fully confirmed change - // Coins returned by Fund are cached for quick reference and for cleanup on // shutdown. fundingMtx sync.RWMutex - fundingCoins map[string]*fundingCoin - splitFunds map[string]asset.Coins + fundingCoins map[outPoint]*fundingCoin + splitFunds map[outPoint][]*fundingCoin } // Check that ExchangeWallet satisfies the Wallet interface. @@ -354,10 +378,9 @@ func unconnectedWallet(cfg *asset.WalletConfig, dcrCfg *Config, logger dex.Logge return &ExchangeWallet{ log: logger, acct: cfg.Settings["account"], - tradeChange: make(map[string]time.Time), tipChange: cfg.TipChange, - fundingCoins: make(map[string]*fundingCoin), - splitFunds: make(map[string]asset.Coins), + fundingCoins: make(map[outPoint]*fundingCoin), + splitFunds: make(map[outPoint][]*fundingCoin), fallbackFeeRate: fallbackFeesPerByte, useSplitTx: dcrCfg.UseSplitTx, } @@ -651,13 +674,19 @@ func (dcr *ExchangeWallet) fund(confs uint32, // there is no error, and the input coins will be returned unmodified, but an // info message will be logged. // -// A split transaction nets 1 extra transaction, 1 extra signed p2pkh-spending -// input, and two additional p2pkh outputs. If the fees associated with this -// extra baggage are more than the excess amount that would be locked if a split -// transaction were not used, then the split transaction is pointless. This -// might be common, for instance, if an order is canceled partially filled, and -// then the remainder resubmitted. We would already have an output of just the -// right size, and that would be recognized here. +// A split transaction nets additional network bytes consisting of +// - overhead from 1 transaction +// - 1 extra signed p2pkh-spending input. The split tx has the fundingCoins as +// inputs now, but we'll add the input that spends the sized coin that will go +// into the first swap +// - 2 additional p2pkh outputs for the split tx sized output and change +// +// If the fees associated with this extra baggage are more than the excess +// amount that would be locked if a split transaction were not used, then the +// split transaction is pointless. This might be common, for instance, if an +// order is canceled partially filled, and then the remainder resubmitted. We +// would already have an output of just the right size, and that would be +// recognized here. func (dcr *ExchangeWallet) split(value uint64, coins asset.Coins, inputsSize uint64, fundingCoins []*fundingCoin, nfo *dex.Asset) (asset.Coins, error) { // Calculate the extra fees associated with the additional inputs, outputs, @@ -689,6 +718,10 @@ func (dcr *ExchangeWallet) split(value uint64, coins asset.Coins, inputsSize uin return nil, fmt.Errorf("error sending split transaction: %v", err) } + if net != reqFunds { + dcr.log.Errorf("split - total sent %d does not match expected %d", net, reqFunds) + } + op := newOutput(dcr.node, msgTx.CachedTxHash(), 0, net, wire.TxTreeRegular, nil) // Lock the funding coin. @@ -710,10 +743,10 @@ func (dcr *ExchangeWallet) split(value uint64, coins asset.Coins, inputsSize uin // dcr.log.Errorf("error locking coins spent in split transaction %v", coins) // } - dcr.logSplitFunds(op, append(coins, op)) + dcr.logSplitFunds(op, fundingCoins) dcr.log.Debugf("funding %d atom order with split output coin %v from original coins %v", value, op, coins) - dcr.log.Infof("sent split transaction %s to accommodate swap of size %d + fees = %d", op.txHash, value, reqFunds) + dcr.log.Infof("sent split transaction %s to accommodate swap of size %d + fees = %d", op.txHash(), value, reqFunds) return asset.Coins{op}, nil @@ -723,7 +756,7 @@ func (dcr *ExchangeWallet) split(value uint64, coins asset.Coins, inputsSize uin func (dcr *ExchangeWallet) lockFundingCoins(fCoins []*fundingCoin) error { wireOPs := make([]*wire.OutPoint, 0, len(fCoins)) for _, c := range fCoins { - wireOPs = append(wireOPs, wire.NewOutPoint(&c.op.txHash, c.op.vout, c.op.tree)) + wireOPs = append(wireOPs, wire.NewOutPoint(c.op.txHash(), c.op.vout(), c.op.tree)) } err := dcr.node.LockUnspent(false, wireOPs) if err != nil { @@ -732,7 +765,7 @@ func (dcr *ExchangeWallet) lockFundingCoins(fCoins []*fundingCoin) error { dcr.fundingMtx.Lock() defer dcr.fundingMtx.Unlock() for _, c := range fCoins { - dcr.fundingCoins[c.op.String()] = c + dcr.fundingCoins[c.op.pt] = c } return nil } @@ -741,11 +774,11 @@ func (dcr *ExchangeWallet) lockFundingCoins(fCoins []*fundingCoin) error { // the coins can be unlocked when the swap is sent. The helps to deal with a // timing issue with dcrwallet where listunspent might still return outputs that were // just spent in a transaction broadcast with sendrawtransaction. Instead, we'll -// lock them until the split output is spent. -func (dcr *ExchangeWallet) logSplitFunds(op *output, coins asset.Coins) { +// keep them locked until the split output is spent. +func (dcr *ExchangeWallet) logSplitFunds(op *output, fCoins []*fundingCoin) { dcr.fundingMtx.Lock() defer dcr.fundingMtx.Unlock() - dcr.splitFunds[op.String()] = coins + dcr.splitFunds[op.pt] = fCoins } // ReturnCoins unlocks coins. This would be necessary in the case of a @@ -764,18 +797,14 @@ func (dcr *ExchangeWallet) ReturnCoins(unspents asset.Coins) error { if err != nil { return fmt.Errorf("error converting coin: %v", err) } - ops = append(ops, wire.NewOutPoint(&op.txHash, op.vout, op.tree)) - delete(dcr.fundingCoins, outpointID(&op.txHash, op.vout)) - splitFunds, found := dcr.splitFunds[unspent.String()] + ops = append(ops, wire.NewOutPoint(op.txHash(), op.vout(), op.tree)) + delete(dcr.fundingCoins, op.pt) + splitFunds, found := dcr.splitFunds[op.pt] if found { for _, c := range splitFunds { - delete(dcr.splitFunds, c.String()) - op, err := dcr.convertCoin(c) - if err != nil { - return fmt.Errorf("error converting split funds coin: %v", err) - } - ops = append(ops, wire.NewOutPoint(&op.txHash, op.vout, op.tree)) + ops = append(ops, c.op.wireOutPoint()) } + delete(dcr.splitFunds, op.pt) } } return dcr.node.LockUnspent(true, ops) @@ -787,7 +816,7 @@ func (dcr *ExchangeWallet) ReturnCoins(unspents asset.Coins) error { func (dcr *ExchangeWallet) FundingCoins(ids []dex.Bytes) (asset.Coins, error) { // First check if we have the coins in cache. coins := make(asset.Coins, 0, len(ids)) - notFound := make(map[string]bool) + notFound := make(map[outPoint]bool) dcr.fundingMtx.RLock() for _, id := range ids { txHash, vout, err := decodeCoinID(id) @@ -795,13 +824,13 @@ func (dcr *ExchangeWallet) FundingCoins(ids []dex.Bytes) (asset.Coins, error) { dcr.fundingMtx.RUnlock() return nil, err } - opID := outpointID(txHash, vout) - fundingCoin, found := dcr.fundingCoins[opID] + pt := newOutPoint(txHash, vout) + fundingCoin, found := dcr.fundingCoins[pt] if found { coins = append(coins, fundingCoin.op) continue } - notFound[opID] = true + notFound[pt] = true } dcr.fundingMtx.RUnlock() if len(notFound) == 0 { @@ -819,8 +848,8 @@ func (dcr *ExchangeWallet) FundingCoins(ids []dex.Bytes) (asset.Coins, error) { if err != nil { return nil, fmt.Errorf("error decoding txid from rpc server %s: %v", txout.TxID, err) } - opID := outpointID(txHash, txout.Vout) - if notFound[opID] { + pt := newOutPoint(txHash, txout.Vout) + if notFound[pt] { redeemScript, err := hex.DecodeString(txout.RedeemScript) if err != nil { return nil, fmt.Errorf("error decoding redeem script for %s, script = %s: %v", txout.TxID, txout.RedeemScript, err) @@ -828,11 +857,11 @@ func (dcr *ExchangeWallet) FundingCoins(ids []dex.Bytes) (asset.Coins, error) { lockers = append(lockers, wire.NewOutPoint(txHash, txout.Vout, txout.Tree)) coin := newOutput(dcr.node, txHash, txout.Vout, toAtoms(txout.Amount), txout.Tree, redeemScript) coins = append(coins, coin) - dcr.fundingCoins[opID] = &fundingCoin{ + dcr.fundingCoins[pt] = &fundingCoin{ op: coin, addr: txout.Address, } - delete(notFound, opID) + delete(notFound, pt) if len(notFound) == 0 { break } @@ -840,8 +869,8 @@ func (dcr *ExchangeWallet) FundingCoins(ids []dex.Bytes) (asset.Coins, error) { } if len(notFound) != 0 { ids := make([]string, 0, len(notFound)) - for opID := range notFound { - ids = append(ids, opID) + for pt := range notFound { + ids = append(ids, pt.String()) } return nil, fmt.Errorf("coins not found: %s", strings.Join(ids, ", ")) } @@ -977,7 +1006,7 @@ func (dcr *ExchangeWallet) Redeem(redemptions []*asset.Redemption) ([]dex.Bytes, } addresses = append(addresses, receiver) contracts = append(contracts, cinfo.output.redeem) - prevOut := wire.NewOutPoint(&cinfo.output.txHash, cinfo.output.vout, wire.TxTreeRegular) + prevOut := cinfo.output.wireOutPoint() txIn := wire.NewTxIn(prevOut, int64(cinfo.output.value), []byte{}) // Sequence = 0xffffffff - 1 is special value that marks the transaction as // irreplaceable and enables the use of lock time. @@ -1038,8 +1067,6 @@ func (dcr *ExchangeWallet) Redeem(redemptions []*asset.Redemption) ([]dex.Bytes, coinIDs = append(coinIDs, toCoinID(txHash, uint32(i))) } - // Log the change output. - dcr.addChange(txHash, 0) return coinIDs, newOutput(dcr.node, txHash, 0, uint64(txOut.Value), wire.TxTreeRegular, nil), nil } @@ -1047,7 +1074,7 @@ func (dcr *ExchangeWallet) Redeem(redemptions []*asset.Redemption) ([]dex.Bytes, // specified funding Coin. A slice of pubkeys required to spend the Coin and a // signature for each pubkey are returned. func (dcr *ExchangeWallet) SignMessage(coin asset.Coin, msg dex.Bytes) (pubkeys, sigs []dex.Bytes, err error) { - output, err := dcr.convertCoin(coin) + op, err := dcr.convertCoin(coin) if err != nil { return nil, nil, fmt.Errorf("error converting coin: %v", err) } @@ -1055,14 +1082,14 @@ func (dcr *ExchangeWallet) SignMessage(coin asset.Coin, msg dex.Bytes) (pubkeys, // First check if we have the funding coin cached. If so, grab the address // from there. dcr.fundingMtx.RLock() - fCoin, found := dcr.fundingCoins[coin.String()] + fCoin, found := dcr.fundingCoins[op.pt] dcr.fundingMtx.RUnlock() var addr string if found { addr = fCoin.addr } else { // Check if we can get the address from gettxout. - txOut, err := dcr.node.GetTxOut(&output.txHash, output.vout, true) + txOut, err := dcr.node.GetTxOut(op.txHash(), op.vout(), true) if err == nil && txOut != nil { addrs := txOut.ScriptPubKey.Addresses if len(addrs) != 1 { @@ -1458,16 +1485,16 @@ func (dcr *ExchangeWallet) Confirmations(id dex.Bytes) (uint32, error) { func (dcr *ExchangeWallet) addInputCoins(msgTx *wire.MsgTx, coins asset.Coins) (uint64, error) { var totalIn uint64 for _, coin := range coins { - output, err := dcr.convertCoin(coin) + op, err := dcr.convertCoin(coin) if err != nil { return 0, err } - if output.value == 0 { - return 0, fmt.Errorf("zero-valued output detected for %s:%d", output.txHash, output.vout) + if op.value == 0 { + return 0, fmt.Errorf("zero-valued output detected for %s:%d", op.txHash(), op.vout()) } - totalIn += output.value - prevOut := wire.NewOutPoint(&output.txHash, output.vout, output.tree) - txIn := wire.NewTxIn(prevOut, int64(output.value), []byte{}) + totalIn += op.value + prevOut := op.wireOutPoint() + txIn := wire.NewTxIn(prevOut, int64(op.value), []byte{}) msgTx.AddTxIn(txIn) } return totalIn, nil @@ -1538,20 +1565,20 @@ func (dcr *ExchangeWallet) lockedAtoms() (uint64, error) { var sum uint64 dcr.fundingMtx.Lock() defer dcr.fundingMtx.Unlock() - for _, outPoint := range lockedOutpoints { - opID := outpointID(&outPoint.Hash, outPoint.Index) - utxo, found := dcr.fundingCoins[opID] + for _, wireOP := range lockedOutpoints { + pt := newOutPoint(&wireOP.Hash, wireOP.Index) + utxo, found := dcr.fundingCoins[pt] if found { sum += utxo.op.value continue } - txOut, err := dcr.node.GetTxOut(&outPoint.Hash, outPoint.Index, true) + txOut, err := dcr.node.GetTxOut(&wireOP.Hash, wireOP.Index, true) if err != nil { return 0, err } if txOut == nil { // Must be spent now? - dcr.log.Debugf("ignoring output from listlockunspent that wasn't found with gettxout. %s", opID) + dcr.log.Debugf("ignoring output from listlockunspent that wasn't found with gettxout. %s", pt) continue } sum += toAtoms(txOut.Value) @@ -1559,23 +1586,6 @@ func (dcr *ExchangeWallet) lockedAtoms() (uint64, error) { return sum, nil } -// addChange adds the output to the list of DEX-trade change outputs. These -// outputs are tracked because the client may wish to exempt change outputs from -// funding coin confirmation requirements. -func (dcr *ExchangeWallet) addChange(txHash *chainhash.Hash, vout uint32) { - dcr.changeMtx.Lock() - defer dcr.changeMtx.Unlock() - dcr.tradeChange[outpointID(txHash, vout)] = time.Now() -} - -// isDEXChange checks whether the specified output is change from a DEX trade. -func (dcr *ExchangeWallet) isDEXChange(txHash *chainhash.Hash, vout uint32) bool { - dcr.changeMtx.RLock() - defer dcr.changeMtx.RUnlock() - _, found := dcr.tradeChange[outpointID(txHash, vout)] - return found -} - // convertCoin converts the asset.Coin to an unspent output. func (dcr *ExchangeWallet) convertCoin(coin asset.Coin) (*output, error) { op, _ := coin.(*output) @@ -1704,7 +1714,6 @@ func (dcr *ExchangeWallet) sendWithReturn(baseTx *wire.MsgTx, addr dcrutil.Addre if err != nil { return nil, nil, fmt.Errorf("error creating change script for address '%s': %v", addr, err) } - changeIdx := len(baseTx.TxOut) changeOutput := wire.NewTxOut(int64(remaining), changeScript) // The reservoir indicates the amount available to draw upon for fees. @@ -1819,7 +1828,6 @@ func (dcr *ExchangeWallet) sendWithReturn(baseTx *wire.MsgTx, addr dcrutil.Addre var change *output if changeAdded { - dcr.addChange(txHash, uint32(changeIdx)) change = newOutput(dcr.node, txHash, uint32(len(msgTx.TxOut)-1), uint64(changeOutput.Value), wire.TxTreeRegular, nil) } return msgTx, change, nil diff --git a/client/asset/dcr/dcr_test.go b/client/asset/dcr/dcr_test.go index 005ab13e75..fe39864a8f 100644 --- a/client/asset/dcr/dcr_test.go +++ b/client/asset/dcr/dcr_test.go @@ -120,7 +120,7 @@ type tRPCClient struct { sendRawHash *chainhash.Hash sendRawErr error sentRawTx *wire.MsgTx - txOutRes map[string]*chainjson.GetTxOutResult + txOutRes map[outPoint]*chainjson.GetTxOutResult txOutErr error bestBlockHash *chainhash.Hash bestBlockHeight int64 @@ -156,7 +156,7 @@ type tRPCClient struct { func newTRPCClient() *tRPCClient { return &tRPCClient{ - txOutRes: make(map[string]*chainjson.GetTxOutResult), + txOutRes: make(map[outPoint]*chainjson.GetTxOutResult), verboseBlocks: make(map[string]*chainjson.GetBlockVerboseResult), mainchain: make(map[int64]*chainhash.Hash), bestHash: chainhash.Hash{}, @@ -181,7 +181,7 @@ func (c *tRPCClient) SendRawTransaction(tx *wire.MsgTx, allowHighFees bool) (*ch } func (c *tRPCClient) GetTxOut(txHash *chainhash.Hash, vout uint32, mempool bool) (*chainjson.GetTxOutResult, error) { - return c.txOutRes[outpointID(txHash, vout)], c.txOutErr + return c.txOutRes[newOutPoint(txHash, vout)], c.txOutErr } func (c *tRPCClient) GetBestBlock() (*chainhash.Hash, int64, error) { @@ -346,7 +346,7 @@ func TestAvailableFund(t *testing.T) { Hash: *tTxHash, }, } - node.txOutRes[outpointID(tTxHash, 0)] = makeGetTxOutRes(1, 1, tP2PKHScript) + node.txOutRes[newOutPoint(tTxHash, 0)] = makeGetTxOutRes(1, 1, tP2PKHScript) bal, err = wallet.Balance() if err != nil { @@ -536,7 +536,7 @@ func TestAvailableFund(t *testing.T) { // Not enough funds, because littleUnspent is a different account. littleUTXO.Account = "wrong account" // for consistency node.unspent = []walletjson.ListUnspentResult{littleUTXO, lottaUTXO} - _, err = wallet.FundOrder(extraLottaOrder, tDCR) + _, err = wallet.FundOrder(extraLottaOrder, false, tDCR) if err == nil { t.Fatalf("no error for wrong account") } @@ -603,7 +603,7 @@ func TestReturnCoins(t *testing.T) { t.Fatalf("no error for missing txout") } - node.txOutRes[outpointID(tTxHash, 0)] = makeGetTxOutRes(1, 1, tP2PKHScript) + node.txOutRes[newOutPoint(tTxHash, 0)] = makeGetTxOutRes(1, 1, tP2PKHScript) err = wallet.ReturnCoins(coins) if err != nil { t.Fatalf("error with custom coin type: %v", err) @@ -640,7 +640,7 @@ func TestFundingCoins(t *testing.T) { // Clear the RPC coins, but add a coin to the cache. node.unspent = nil - opID := outpointID(tTxHash, vout) + opID := newOutPoint(tTxHash, vout) wallet.fundingCoins[opID] = &fundingCoin{ op: newOutput(node, tTxHash, vout, 0, 0, nil), } @@ -1006,14 +1006,14 @@ func TestSignMessage(t *testing.T) { node.privWIF = dcrutil.NewWIF(privKey, chainParams.PrivateKeyID, dcrec.STEcdsaSecp256k1) - var coin asset.Coin = newOutput(node, tTxHash, vout, 5e7, wire.TxTreeRegular, nil) + op := newOutput(node, tTxHash, vout, 5e7, wire.TxTreeRegular, nil) - wallet.fundingCoins[coin.String()] = &fundingCoin{ + wallet.fundingCoins[op.pt] = &fundingCoin{ addr: tPKHAddr.String(), } check := func() { - pubkeys, sigs, err := wallet.SignMessage(coin, msg) + pubkeys, sigs, err := wallet.SignMessage(op, msg) if err != nil { t.Fatalf("SignMessage error: %v", err) } @@ -1032,15 +1032,15 @@ func TestSignMessage(t *testing.T) { } check() - delete(wallet.fundingCoins, coin.String()) + delete(wallet.fundingCoins, op.pt) txOut := makeGetTxOutRes(1, 5, nil) txOut.ScriptPubKey.Addresses = []string{tPKHAddr.String()} - node.txOutRes[outpointID(tTxHash, vout)] = txOut + node.txOutRes[newOutPoint(tTxHash, vout)] = txOut check() // gettransaction error node.txOutErr = tErr - _, _, err = wallet.SignMessage(coin, msg) + _, _, err = wallet.SignMessage(op, msg) if err == nil { t.Fatalf("no error for gettxout rpc error") } @@ -1048,7 +1048,7 @@ func TestSignMessage(t *testing.T) { // dumpprivkey error node.privWIFErr = tErr - _, _, err = wallet.SignMessage(coin, msg) + _, _, err = wallet.SignMessage(op, msg) if err == nil { t.Fatalf("no error for dumpprivkey rpc error") } @@ -1076,7 +1076,7 @@ func TestAuditContract(t *testing.T) { addr, _ := dcrutil.NewAddressScriptHash(contract, chainParams) pkScript, _ := txscript.PayToAddrScript(addr) - node.txOutRes[outpointID(tTxHash, vout)] = makeGetTxOutRes(1, 5, pkScript) + node.txOutRes[newOutPoint(tTxHash, vout)] = makeGetTxOutRes(1, 5, pkScript) audit, err := wallet.AuditContract(toCoinID(tTxHash, vout), contract) if err != nil { @@ -1249,7 +1249,7 @@ func TestRefund(t *testing.T) { } bigTxOut := makeGetTxOutRes(2, 5, nil) - bigOutID := outpointID(tTxHash, 0) + bigOutID := newOutPoint(tTxHash, 0) node.txOutRes[bigOutID] = bigTxOut node.changeAddr = tPKHAddr node.newAddr = tPKHAddr From d55fbc16c12e823b9be89da4b6b3d5d104dcc348 Mon Sep 17 00:00:00 2001 From: buck54321 Date: Fri, 31 Jul 2020 10:17:33 -0500 Subject: [PATCH 4/5] add max-width for tooltip --- client/webserver/site/src/css/main.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/client/webserver/site/src/css/main.scss b/client/webserver/site/src/css/main.scss index 2e36635426..ce6e9ceb62 100644 --- a/client/webserver/site/src/css/main.scss +++ b/client/webserver/site/src/css/main.scss @@ -328,6 +328,7 @@ hr.dashed { background-color: #040012; border: 1px solid #333; color: white; + max-width: 500px; } .dynamicopts { From 47f798901e5917aa1c1771c1f21e8d5c55c0907a Mon Sep 17 00:00:00 2001 From: buck54321 Date: Tue, 4 Aug 2020 13:14:39 -0500 Subject: [PATCH 5/5] pointerless btc outPoint structure --- client/asset/btc/btc.go | 58 +++++++++++++++++++++++------------- client/asset/btc/btc_test.go | 2 +- 2 files changed, 38 insertions(+), 22 deletions(-) diff --git a/client/asset/btc/btc.go b/client/asset/btc/btc.go index 52ab842d6c..025da157b0 100644 --- a/client/asset/btc/btc.go +++ b/client/asset/btc/btc.go @@ -135,18 +135,29 @@ type BTCCloneCFG struct { DefaultFallbackFee uint64 // sats/byte } -// newOutPoint is the constructor for a new wire.OutPoint (non-pointer). -func newOutPoint(txHash *chainhash.Hash, vout uint32) wire.OutPoint { - return wire.OutPoint{ - Hash: *txHash, - Index: vout, +// outPoint is the hash and output index of a transaction output. +type outPoint struct { + txHash chainhash.Hash + vout uint32 +} + +// newOutPoint is the constructor for a new outPoint. +func newOutPoint(txHash *chainhash.Hash, vout uint32) outPoint { + return outPoint{ + txHash: *txHash, + vout: vout, } } +// String is a string representation of the outPoint. +func (pt *outPoint) String() string { + return pt.txHash.String() + ":" + strconv.Itoa(int(pt.vout)) +} + // output is information about a transaction output. output satisfies the // asset.Coin interface. type output struct { - pt wire.OutPoint + pt outPoint value uint64 redeem dex.Bytes node rpcClient // for calculating confirmations. @@ -190,7 +201,7 @@ func (op *output) ID() dex.Bytes { // String is a string representation of the coin. func (op *output) String() string { - return op.pt.Hash.String() + ":" + strconv.Itoa(int(op.pt.Index)) + return op.pt.String() } // Redeem is any known redeem script required to spend this output. Part of the @@ -201,12 +212,17 @@ func (op *output) Redeem() dex.Bytes { // txHash returns the pointer of the wire.OutPoint's Hash. func (op *output) txHash() *chainhash.Hash { - return &op.pt.Hash + return &op.pt.txHash } // vout returns the wire.OutPoint's Index. func (op *output) vout() uint32 { - return op.pt.Index + return op.pt.vout +} + +// wireOutPoint creates and returns a new *wire.OutPoint for the output. +func (op *output) wireOutPoint() *wire.OutPoint { + return wire.NewOutPoint(op.txHash(), op.vout()) } // auditInfo is information about a swap contract on that blockchain, not @@ -314,7 +330,7 @@ type ExchangeWallet struct { // Coins returned by Fund are cached for quick reference and for cleanup on // shutdown. fundingMtx sync.RWMutex - fundingCoins map[wire.OutPoint]*compositeUTXO + fundingCoins map[outPoint]*compositeUTXO } // Check that ExchangeWallet satisfies the Wallet interface. @@ -401,7 +417,7 @@ func newWallet(cfg *BTCCloneCFG, btcCfg *dexbtc.Config, node rpcClient) *Exchang chainParams: cfg.ChainParams, log: cfg.Logger, tipChange: cfg.WalletCFG.TipChange, - fundingCoins: make(map[wire.OutPoint]*compositeUTXO), + fundingCoins: make(map[outPoint]*compositeUTXO), minNetworkVersion: cfg.MinNetworkVersion, fallbackFeeRate: fallbackFeesPerByte, useSplitTx: btcCfg.UseSplitTx, @@ -522,7 +538,7 @@ func (btc *ExchangeWallet) FundOrder(value uint64, immediate bool, nfo *dex.Asse var size uint32 var coins asset.Coins var spents []*output - fundingCoins := make(map[wire.OutPoint]*compositeUTXO) + fundingCoins := make(map[outPoint]*compositeUTXO) // TODO: For the chained swaps, make sure that contract outputs are P2WSH, // and that change outputs that fund further swaps are P2WPKH. @@ -602,7 +618,7 @@ out: // order is canceled partially filled, and then the remainder resubmitted. We // would already have an output of just the right size, and that would be // recognized here. -func (btc *ExchangeWallet) split(value uint64, outputs []*output, inputsSize uint64, fundingCoins map[wire.OutPoint]*compositeUTXO, nfo *dex.Asset) (asset.Coins, error) { +func (btc *ExchangeWallet) split(value uint64, outputs []*output, inputsSize uint64, fundingCoins map[outPoint]*compositeUTXO, nfo *dex.Asset) (asset.Coins, error) { var err error defer func() { if err != nil { @@ -671,7 +687,7 @@ func (btc *ExchangeWallet) split(value uint64, outputs []*output, inputsSize uin if err != nil { return nil, fmt.Errorf("error reading asset info: %v", err) } - fundingCoins = map[wire.OutPoint]*compositeUTXO{op.pt: { + fundingCoins = map[outPoint]*compositeUTXO{op.pt: { txHash: op.txHash(), vout: op.vout(), address: addr.String(), @@ -714,7 +730,7 @@ func (btc *ExchangeWallet) ReturnCoins(unspents asset.Coins) error { func (btc *ExchangeWallet) FundingCoins(ids []dex.Bytes) (asset.Coins, error) { // First check if we have the coins in cache. coins := make(asset.Coins, 0, len(ids)) - notFound := make(map[wire.OutPoint]struct{}) + notFound := make(map[outPoint]struct{}) btc.fundingMtx.RLock() for _, id := range ids { txHash, vout, err := decodeCoinID(id) @@ -789,11 +805,11 @@ func (btc *ExchangeWallet) Lock() error { } // fundedTx creates and returns a new MsgTx with the provided coins as inputs. -func (btc *ExchangeWallet) fundedTx(coins asset.Coins) (*wire.MsgTx, uint64, []wire.OutPoint, error) { +func (btc *ExchangeWallet) fundedTx(coins asset.Coins) (*wire.MsgTx, uint64, []outPoint, error) { baseTx := wire.NewMsgTx(wire.TxVersion) var totalIn uint64 // Add the funding utxos. - pts := make([]wire.OutPoint, 0, len(coins)) + pts := make([]outPoint, 0, len(coins)) for _, coin := range coins { op, err := btc.convertCoin(coin) if err != nil { @@ -803,7 +819,7 @@ func (btc *ExchangeWallet) fundedTx(coins asset.Coins) (*wire.MsgTx, uint64, []w return nil, 0, nil, fmt.Errorf("zero-valued output detected for %s:%d", op.txHash(), op.vout()) } totalIn += op.value - txIn := wire.NewTxIn(&op.pt, []byte{}, nil) + txIn := wire.NewTxIn(op.wireOutPoint(), []byte{}, nil) baseTx.AddTxIn(txIn) pts = append(pts, op.pt) } @@ -948,7 +964,7 @@ func (btc *ExchangeWallet) Redeem(redemptions []*asset.Redemption) ([]dex.Bytes, } addresses = append(addresses, receiver) contracts = append(contracts, cinfo.output.redeem) - txIn := wire.NewTxIn(&cinfo.output.pt, []byte{}, nil) + txIn := wire.NewTxIn(cinfo.output.wireOutPoint(), []byte{}, nil) // Enable locktime // https://github.com/bitcoin/bips/blob/master/bip-0125.mediawiki#Spending_wallet_policy txIn.Sequence = wire.MaxTxInSequenceNum - 1 @@ -1568,7 +1584,7 @@ type compositeUTXO struct { // spendableUTXOs filters the RPC utxos for those that are spendable with with // regards to the DEX's configuration, and considered safe to spend according to // confirmations and coin source. The UTXOs will be sorted by ascending value. -func (btc *ExchangeWallet) spendableUTXOs(confs uint32) ([]*compositeUTXO, map[wire.OutPoint]*compositeUTXO, uint64, error) { +func (btc *ExchangeWallet) spendableUTXOs(confs uint32) ([]*compositeUTXO, map[outPoint]*compositeUTXO, uint64, error) { unspents, err := btc.wallet.ListUnspent() if err != nil { return nil, nil, 0, err @@ -1576,7 +1592,7 @@ func (btc *ExchangeWallet) spendableUTXOs(confs uint32) ([]*compositeUTXO, map[w sort.Slice(unspents, func(i, j int) bool { return unspents[i].Amount < unspents[j].Amount }) var sum uint64 utxos := make([]*compositeUTXO, 0, len(unspents)) - utxoMap := make(map[wire.OutPoint]*compositeUTXO, len(unspents)) + utxoMap := make(map[outPoint]*compositeUTXO, len(unspents)) for _, txout := range unspents { if txout.Confirmations >= confs && txout.Safe { nfo, err := dexbtc.InputInfo(txout.ScriptPubKey, txout.RedeemScript, btc.chainParams) diff --git a/client/asset/btc/btc_test.go b/client/asset/btc/btc_test.go index 7fdb65ee21..57e2d22702 100644 --- a/client/asset/btc/btc_test.go +++ b/client/asset/btc/btc_test.go @@ -643,7 +643,7 @@ func TestFundingCoins(t *testing.T) { ensureErr := func(tag string) { // Clear the cache. - wallet.fundingCoins = make(map[wire.OutPoint]*compositeUTXO) + wallet.fundingCoins = make(map[outPoint]*compositeUTXO) _, err := wallet.FundingCoins(coinIDs) if err == nil { t.Fatalf("%s: no error", tag)