From a6ea5710d9d0629cab1990e444bf47bc41dc2343 Mon Sep 17 00:00:00 2001 From: Philemon Ukane Date: Tue, 18 Jul 2023 19:06:13 +0100 Subject: [PATCH] client/core: check if wallet is synchronized for spend actions --- client/core/bond.go | 13 +++++++------ client/core/core.go | 31 +++++++++++++++++++++++++++---- client/core/core_test.go | 7 +++++++ client/core/trade.go | 8 ++------ client/core/wallet.go | 15 ++++++++++++--- 5 files changed, 55 insertions(+), 19 deletions(-) diff --git a/client/core/bond.go b/client/core/bond.go index 5f922eb390..428bc86093 100644 --- a/client/core/bond.go +++ b/client/core/bond.go @@ -446,10 +446,10 @@ func (c *Core) postRequiredBonds(dc *dexConnection, cfg *dexBondCfg, state *dexA c.log.Errorf("failed to unlock bond asset wallet %v: %v", unbip(state.bondAssetID), err) return } - if !wallet.synchronized() { - c.log.Warnf("Wallet %v is not yet synchronized with the network. Cannot post new bonds yet.", - unbip(state.bondAssetID)) - return // otherwise we might double spend if the wallet keys were used elsewhere + err = wallet.checkPeersAndSyncStatus() + if err != nil { + c.log.Errorf("Cannot post new bonds yet. %v", err) + return } // For the max bonded limit, we'll normalize all bonds to the @@ -1075,8 +1075,9 @@ func (c *Core) PostBond(form *PostBondForm) (*PostBondResult, error) { if _, ok := wallet.Wallet.(asset.Bonder); !ok { // will fail in MakeBondTx, but assert early return nil, fmt.Errorf("wallet %v is not an asset.Bonder", bondAssetSymbol) } - if !wallet.synchronized() { // otherwise we might double spend if the wallet keys were used elsewhere - return nil, fmt.Errorf("wallet %v is not synchronized", unbip(bondAssetID)) + err = wallet.checkPeersAndSyncStatus() + if err != nil { + return nil, err } // Check the app password. diff --git a/client/core/core.go b/client/core/core.go index e24caf0edb..3e01f8cfc4 100644 --- a/client/core/core.go +++ b/client/core/core.go @@ -5324,6 +5324,10 @@ func (c *Core) Send(pw []byte, assetID uint32, value uint64, address string, sub return nil, err } + if err = wallet.checkPeersAndSyncStatus(); err != nil { + return nil, err + } + var coin asset.Coin feeSuggestion := c.feeSuggestionAny(assetID) if !subtract { @@ -5379,6 +5383,11 @@ func (c *Core) ApproveToken(appPW []byte, assetID uint32, dexAddr string, onConf return "", err } + err = wallet.checkPeersAndSyncStatus() + if err != nil { + return "", err + } + dex, connected, err := c.dex(dexAddr) if err != nil { return "", err @@ -5424,6 +5433,11 @@ func (c *Core) UnapproveToken(appPW []byte, assetID uint32, version uint32) (str return "", err } + err = wallet.checkPeersAndSyncStatus() + if err != nil { + return "", err + } + onConfirm := func() { go c.notify(newTokenApprovalNote(wallet.state())) } @@ -10358,15 +10372,24 @@ func (c *Core) saveDisabledRateSources() { } } -func (c *Core) shieldedWallet(assetID uint32) (asset.ShieldedWallet, error) { +func (c *Core) shieldedWallet(assetID uint32, forFundTransfer ...bool) (asset.ShieldedWallet, error) { w, found := c.wallet(assetID) if !found { return nil, fmt.Errorf("no %s wallet", unbip(assetID)) } + sw, is := w.Wallet.(asset.ShieldedWallet) if !is { return nil, fmt.Errorf("%s wallet is not a shielded wallet", unbip(assetID)) } + + // Check if this wallet can send funds at the moment. + if len(forFundTransfer) > 0 && forFundTransfer[0] { + if err := w.checkPeersAndSyncStatus(); err != nil { + return nil, err + } + } + return sw, nil } @@ -10393,7 +10416,7 @@ func (c *Core) NewShieldedAddress(assetID uint32) (string, error) { // ShieldFunds moves funds from the transparent account to the shielded account. func (c *Core) ShieldFunds(assetID uint32, amt uint64) ([]byte, error) { - sw, err := c.shieldedWallet(assetID) + sw, err := c.shieldedWallet(assetID, true) if err != nil { return nil, err } @@ -10403,7 +10426,7 @@ func (c *Core) ShieldFunds(assetID uint32, amt uint64) ([]byte, error) { // UnshieldFunds moves funds from the shielded account to the transparent // account. func (c *Core) UnshieldFunds(assetID uint32, amt uint64) ([]byte, error) { - sw, err := c.shieldedWallet(assetID) + sw, err := c.shieldedWallet(assetID, true) if err != nil { return nil, err } @@ -10419,7 +10442,7 @@ func (c *Core) SendShielded(appPW []byte, assetID uint32, toAddr string, amt uin return nil, fmt.Errorf("password error: %w", err) } - sw, err := c.shieldedWallet(assetID) + sw, err := c.shieldedWallet(assetID, true) if err != nil { return nil, err } diff --git a/client/core/core_test.go b/client/core/core_test.go index 7a74f3516c..4b75fb45bc 100644 --- a/client/core/core_test.go +++ b/client/core/core_test.go @@ -2779,6 +2779,13 @@ func TestSend(t *testing.T) { if tWallet.sendFeeSuggestion != feeRate { t.Fatalf("unexpected fee rate from FeeRater. wanted %d, got %d", feeRate, tWallet.sendFeeSuggestion) } + + // wallet is not synced + wallet.synced = false + _, err = tCore.Send(tPW, tUTXOAssetA.ID, 1e8, address, false) + if err == nil { + t.Fatalf("Expected error for a non-synchronized wallet") + } } func trade(t *testing.T, async bool) { diff --git a/client/core/trade.go b/client/core/trade.go index ef9c22be22..dc20b0c616 100644 --- a/client/core/trade.go +++ b/client/core/trade.go @@ -2903,12 +2903,8 @@ func (t *trackedTrade) confirmRedemption(match *matchTracker) (bool, error) { // In some cases the wallet will need to send a new redeem transaction. toWallet := t.wallets.toWallet - if toWallet.peerCount < 1 { - return false, fmt.Errorf("%s wallet has no peers", unbip(toWallet.AssetID)) - } - - if !toWallet.synchronized() { - return false, fmt.Errorf("%s still syncing", unbip(toWallet.AssetID)) + if err := toWallet.checkPeersAndSyncStatus(); err != nil { + return false, err } didUnlock, err := toWallet.refreshUnlock() diff --git a/client/core/wallet.go b/client/core/wallet.go index 1e009e6982..f3e3d314ab 100644 --- a/client/core/wallet.go +++ b/client/core/wallet.go @@ -365,11 +365,20 @@ func (w *xcWallet) connected() bool { return w.hookedUp } -// synchronized is true if the wallet had been synchronized with the network. -func (w *xcWallet) synchronized() bool { +// checkPeersAndSyncStatus checks that the wallet is synced, and has peers +// otherwise we might double spend if the wallet keys were used elsewhere. This +// should be checked before attempting to send funds but does not replace any +// other checks that may be required. +func (w *xcWallet) checkPeersAndSyncStatus() error { w.mtx.RLock() defer w.mtx.RUnlock() - return w.synced + if w.peerCount < 1 { + return fmt.Errorf("%s wallet has no connected peers", unbip(w.AssetID)) + } + if !w.synced { + return fmt.Errorf("%s wallet is not synchronized", unbip(w.AssetID)) + } + return nil } // Connect calls the dex.Connector's Connect method, sets the xcWallet.hookedUp