From 3fa59cf4dad60a991346795da25666f9bd215af2 Mon Sep 17 00:00:00 2001 From: JoeGruffins <34998433+JoeGruffins@users.noreply.github.com> Date: Wed, 12 May 2021 23:10:31 +0900 Subject: [PATCH] core: Check can trade with new wallet If a user has active trades, they must be settled by a wallet that can complete those trades. Disallow changing to a new wallet that cannot do this. --- client/asset/bch/bch.go | 9 +++++ client/asset/btc/btc.go | 9 +++++ client/asset/dcr/dcr.go | 9 +++++ client/asset/interface.go | 2 ++ client/core/core.go | 72 +++++++++++++++++++++++++++++++++++++++ client/core/core_test.go | 49 ++++++++++++++++++++++---- 6 files changed, 144 insertions(+), 6 deletions(-) diff --git a/client/asset/bch/bch.go b/client/asset/bch/bch.go index eb96f7f421..462ff84492 100644 --- a/client/asset/bch/bch.go +++ b/client/asset/bch/bch.go @@ -210,6 +210,15 @@ func (bch *BCHWallet) AuditContract(coinID, contract, txData dex.Bytes) (*asset. return ai, nil } +// RefundAddress extracts and returns the refund address from a contract. +func (bch *BCHWallet) RefundAddress(contract dex.Bytes) (string, error) { + addr, err := bch.ExchangeWallet.RefundAddress(contract) + if err != nil { + return "", err + } + return dexbch.RecodeCashAddress(addr, bch.Net()) +} + // rawTxSigner signs the transaction using Bitcoin Cash's custom signature // hash and signing algorithm. func rawTxInSigner(btcTx *wire.MsgTx, idx int, subScript []byte, hashType txscript.SigHashType, btcKey *btcec.PrivateKey, val uint64) ([]byte, error) { diff --git a/client/asset/btc/btc.go b/client/asset/btc/btc.go index ac95ef5518..257f653861 100644 --- a/client/asset/btc/btc.go +++ b/client/asset/btc/btc.go @@ -1687,6 +1687,15 @@ func (btc *ExchangeWallet) AuditContract(coinID, contract, txData dex.Bytes) (*a }, nil } +// RefundAddress extracts and returns the refund address from a contract. +func (btc *ExchangeWallet) RefundAddress(contract dex.Bytes) (string, error) { + sender, _, _, _, err := dexbtc.ExtractSwapDetails(contract, btc.segwit, btc.chainParams) + if err != nil { + return "", fmt.Errorf("error extracting refund address: %w", err) + } + return sender.String(), nil +} + // LocktimeExpired returns true if the specified contract's locktime has // expired, making it possible to issue a Refund. func (btc *ExchangeWallet) LocktimeExpired(contract dex.Bytes) (bool, time.Time, error) { diff --git a/client/asset/dcr/dcr.go b/client/asset/dcr/dcr.go index f0557b19a8..2fb31a5a6e 100644 --- a/client/asset/dcr/dcr.go +++ b/client/asset/dcr/dcr.go @@ -1631,6 +1631,15 @@ func (dcr *ExchangeWallet) AuditContract(coinID, contract, txData dex.Bytes) (*a }, nil } +// RefundAddress extracts and returns the refund address from a contract. +func (dcr *ExchangeWallet) RefundAddress(contract dex.Bytes) (string, error) { + sender, _, _, _, err := dexdcr.ExtractSwapDetails(contract, dcr.chainParams) + if err != nil { + return "", fmt.Errorf("error extracting refund address: %w", err) + } + return sender.String(), nil +} + // LocktimeExpired returns true if the specified contract's locktime has // expired, making it possible to issue a Refund. func (dcr *ExchangeWallet) LocktimeExpired(contract dex.Bytes) (bool, time.Time, error) { diff --git a/client/asset/interface.go b/client/asset/interface.go index 19ca512703..383eb34ad7 100644 --- a/client/asset/interface.go +++ b/client/asset/interface.go @@ -180,6 +180,8 @@ type Wallet interface { ValidateSecret(secret, secretHash []byte) bool // SyncStatus is information about the blockchain sync status. SyncStatus() (synced bool, progress float32, err error) + // RefundAddress extracts and returns the refund address from a contract. + RefundAddress(contract dex.Bytes) (string, error) } // Balance is categorized information about a wallet's balance. diff --git a/client/core/core.go b/client/core/core.go index 506d0d034f..9c25fb7c7e 100644 --- a/client/core/core.go +++ b/client/core/core.go @@ -1902,6 +1902,78 @@ func (c *Core) ReconfigureWallet(appPW, newWalletPW []byte, assetID uint32, cfg return err } + // If there are active trades, make sure they can be settled by the + // keys held within the new wallet. + sameWallet := func() error { + ownsAddr := func(addr string) error { + owns, err := wallet.OwnsAddress(addr) + if err != nil { + return err + } + if !owns { + return fmt.Errorf("new wallet does not own address found in active trades: %v", addr) + } + return nil + } + for _, dc := range c.dexConnections() { + maybeDifferentWallet := false + for _, trade := range dc.trackedTrades() { + if !trade.isActive() { + continue + } + waID := wallet.AssetID + // If the to asset, check if we own an + // contract.Address. + if trade.wallets.toAsset.ID == waID { + if err := ownsAddr(trade.Trade().SwapAddress()); err != nil { + return err + } + // Assume all trade addresses are + // owned by the new wallet if one is. + return nil + } + // If the from asset, check if we own a + // refund address for a match if any exist. + if trade.wallets.fromAsset.ID == waID { + for _, match := range trade.matches { + script := match.MetaData.Proof.Script + if len(script) == 0 { + continue + } + addr, err := wallet.RefundAddress(script) + if err != nil { + return err + } + if err := ownsAddr(addr); err != nil { + return err + } + // Assume all refund addresses + // are owned by the new + // wallet if one is. + return nil + } + // If we did not find a refund address, + // we cannot be sure that this is the + // same wallet. + // + // TODO: Implement a way to check that + // these accounts are the same or not. + maybeDifferentWallet = true + } + } + if maybeDifferentWallet { + return errors.New("unable to change wallets with active trades. " + + "trades with this wallet must match or be canceled in " + + "order to change settings") + } + } + return nil + } + if err := sameWallet(); err != nil { + wallet.Disconnect() + return newError(walletErr, "new wallet cannot be used with current active trades: %v", err) + } + // If newWalletPW is non-nil, update the wallet's password. if newWalletPW != nil { // includes empty non-nil slice err = c.setWalletPassword(wallet, newWalletPW, crypter) diff --git a/client/core/core_test.go b/client/core/core_test.go index 625b605dd5..c84b69d941 100644 --- a/client/core/core_test.go +++ b/client/core/core_test.go @@ -530,14 +530,17 @@ type TXCWallet struct { preSwap *asset.PreSwap preRedeemForm *asset.PreRedeemForm preRedeem *asset.PreRedeem + ownsAddress bool + ownsAddressErr error } func newTWallet(assetID uint32) (*xcWallet, *TXCWallet) { w := &TXCWallet{ - changeCoin: &tCoin{id: encode.RandomBytes(36)}, - syncStatus: func() (synced bool, progress float32, err error) { return true, 1, nil }, - confs: make(map[string]uint32), - confsErr: make(map[string]error), + changeCoin: &tCoin{id: encode.RandomBytes(36)}, + syncStatus: func() (synced bool, progress float32, err error) { return true, 1, nil }, + confs: make(map[string]uint32), + confsErr: make(map[string]error), + ownsAddress: true, } xcWallet := &xcWallet{ Wallet: w, @@ -561,7 +564,7 @@ func (w *TXCWallet) Info() *asset.WalletInfo { } func (w *TXCWallet) OwnsAddress(address string) (bool, error) { - return true, nil + return w.ownsAddress, w.ownsAddressErr } func (w *TXCWallet) Connect(ctx context.Context) (*sync.WaitGroup, error) { @@ -668,6 +671,10 @@ func (w *TXCWallet) AuditContract(coinID, contract, txData dex.Bytes) (*asset.Au return w.auditInfo, w.auditErr } +func (w *TXCWallet) RefundAddress(contract dex.Bytes) (string, error) { + return "", nil +} + func (w *TXCWallet) LocktimeExpired(contract dex.Bytes) (bool, time.Time, error) { return true, time.Now().Add(-time.Minute), nil } @@ -5203,14 +5210,26 @@ func TestReconfigureWallet(t *testing.T) { // For the last success, make sure that we also clear any related // tickGovernors. + matchID := ordertest.RandomMatchID() match := &matchTracker{ suspectSwap: true, tickGovernor: time.NewTimer(time.Hour), + MetaMatch: db.MetaMatch{ + MetaData: &db.MatchMetaData{ + Proof: db.MatchProof{ + Script: dex.Bytes{0}, + }, + }, + UserMatch: &order.UserMatch{ + MatchID: matchID, + }, + }, } tCore.conns[tDexHost].trades[order.OrderID{}] = &trackedTrade{ Order: &order.LimitOrder{ P: order.Prefix{ - BaseAsset: assetID, + BaseAsset: assetID, + ServerTime: time.Now(), }, }, wallets: &walletSet{ @@ -5222,7 +5241,25 @@ func TestReconfigureWallet(t *testing.T) { matches: map[order.MatchID]*matchTracker{ {}: match, }, + metaData: &db.OrderMetaData{}, + dc: rig.dc, + } + + // Error checking if wallet owns address. + tXyzWallet.ownsAddressErr = tErr + err = tCore.ReconfigureWallet(tPW, nil, assetID, newSettings) + if !errorHasCode(err, walletErr) { + t.Fatalf("wrong error when expecting ownsAddress wallet error: %v", err) + } + tXyzWallet.ownsAddressErr = nil + + // Wallet doesn't own address. + tXyzWallet.ownsAddress = false + err = tCore.ReconfigureWallet(tPW, nil, assetID, newSettings) + if !errorHasCode(err, walletErr) { + t.Fatalf("wrong error when expecting not owned wallet error: %v", err) } + tXyzWallet.ownsAddress = true // Success updating settings. err = tCore.ReconfigureWallet(tPW, nil, assetID, newSettings)