Skip to content

Commit

Permalink
client: Validate known deposit address after wallet connection
Browse files Browse the repository at this point in the history
  • Loading branch information
amass01 authored and chappjc committed Jan 7, 2021
1 parent 53194c6 commit c3990c7
Show file tree
Hide file tree
Showing 9 changed files with 136 additions and 39 deletions.
9 changes: 9 additions & 0 deletions client/asset/btc/btc.go
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,15 @@ func (btc *ExchangeWallet) SyncStatus() (bool, float32, error) {
return true, 1, nil
}

// OwnsAddress indicates if an address belongs to the wallet.
func (btc *ExchangeWallet) OwnsAddress(address string) (bool, error) {
ai, err := btc.wallet.GetAddressInfo(address)
if err != nil {
return false, err
}
return ai.IsMine, nil
}

// Balance returns the total available funds in the wallet. Part of the
// asset.Wallet interface.
func (btc *ExchangeWallet) Balance() (*asset.Balance, error) {
Expand Down
10 changes: 9 additions & 1 deletion client/asset/btc/walletclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const (
methodSendToAddress = "sendtoaddress"
methodSetTxFee = "settxfee"
methodGetWalletInfo = "getwalletinfo"
methodGetAddressInfo = "getaddressinfo"
)

// walletClient is a bitcoind wallet RPC client that uses rpcclient.Client's
Expand Down Expand Up @@ -225,7 +226,14 @@ func (wc *walletClient) GetWalletInfo() (*GetWalletInfoResult, error) {
return wi, wc.call(methodGetWalletInfo, nil, wi)
}

// call is used internally to marshal parmeters and send requests to the RPC
// GetAddressInfo gets information about the given address by calling
// getaddressinfo RPC command.
func (wc *walletClient) GetAddressInfo(address string) (*GetAddressInfoResult, error) {
ai := new(GetAddressInfoResult)
return ai, wc.call(methodGetAddressInfo, anylist{address}, ai)
}

// call is used internally to marshal parmeters and send requests to the RPC
// server via (*rpcclient.Client).RawRequest. If `thing` is non-nil, the result
// will be marshaled into `thing`.
func (wc *walletClient) call(method string, args anylist, thing interface{}) error {
Expand Down
5 changes: 5 additions & 0 deletions client/asset/btc/wallettypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,8 @@ type GetWalletInfoResult struct {
// Progress float32 `json:"progress"`
// } `json:"scanning"`
}

// GetAddressInfoResult models the data from the getaddressinfo command.
type GetAddressInfoResult struct {
IsMine bool `json:"ismine"`
}
14 changes: 14 additions & 0 deletions client/asset/dcr/dcr.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ type rpcClient interface {
Disconnected() bool
RawRequest(method string, params []json.RawMessage) (json.RawMessage, error)
WalletInfo() (*walletjson.WalletInfoResult, error)
ValidateAddress(address dcrutil.Address) (*walletjson.ValidateAddressWalletResult, error)
}

// outPoint is the hash and output index of a transaction output.
Expand Down Expand Up @@ -531,6 +532,19 @@ func (dcr *ExchangeWallet) Connect(ctx context.Context) (*sync.WaitGroup, error)
return &wg, nil
}

// OwnsAddress indicates if an address belongs to the wallet.
func (dcr *ExchangeWallet) OwnsAddress(address string) (bool, error) {
a, err := dcrutil.DecodeAddress(address, chainParams)
if err != nil {
return false, err
}
va, err := dcr.node.ValidateAddress(a)
if err != nil {
return false, err
}
return va.IsMine, nil
}

// Balance should return the total available funds in the wallet. Note that
// after calling Fund, the amount returned by Balance may change by more than
// the value funded. Part of the asset.Wallet interface. TODO: Since this
Expand Down
6 changes: 6 additions & 0 deletions client/asset/dcr/dcr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,12 @@ func (c *tRPCClient) WalletInfo() (*walletjson.WalletInfoResult, error) {
}, nil
}

func (c *tRPCClient) ValidateAddress(address dcrutil.Address) (*walletjson.ValidateAddressWalletResult, error) {
return &walletjson.ValidateAddressWalletResult{
IsMine: true,
}, nil
}

func (c *tRPCClient) Disconnected() bool {
return c.disconnected
}
Expand Down
2 changes: 2 additions & 0 deletions client/asset/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ type Wallet interface {
Refund(coinID, contract dex.Bytes) (dex.Bytes, error)
// Address returns an address for the exchange wallet.
Address() (string, error)
// OwnsAddress indicates if an address belongs to the wallet.
OwnsAddress(address string) (bool, error)
// Unlock unlocks the exchange wallet.
Unlock(pw string) error
// Lock locks the exchange wallet.
Expand Down
120 changes: 83 additions & 37 deletions client/core/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -1035,7 +1035,7 @@ func (c *Core) connectedWallet(assetID uint32) (*xcWallet, error) {
}
if !wallet.connected() {
c.log.Infof("Connecting wallet for %s", unbip(assetID))
err := c.connectWallet(wallet)
_, err := c.connectWallet(wallet)
if err != nil {
return nil, err // core.Error with code connectWalletErr
}
Expand All @@ -1051,10 +1051,40 @@ func (c *Core) connectedWallet(assetID uint32) (*xcWallet, error) {
return wallet, nil
}

func (c *Core) connectWallet(w *xcWallet) error {
err := w.Connect(c.ctx)
// connectWallet connects to wallet and validates the known deposit address
// after successful connection.
// If address found & it does not belong to wallet, it generates new address,
// then updates xcWallet and dbWallet.
// If connectWallet generates new address then first returned bool will
// be set to true.
func (c *Core) connectWallet(w *xcWallet) (generatedNewAddr bool, err error) {
err = w.Connect(c.ctx)
if err != nil {
return codedError(connectWalletErr, err)
return false, codedError(connectWalletErr, err)
}
// If xcWallet has deposit address ensure that it belongs to connected
// wallet.
w.mtx.RLock()
addr := w.address
w.mtx.RUnlock()
var mine bool

if addr != "" {
mine, err = w.OwnsAddress(addr)
if err != nil {
return generatedNewAddr, err
}
// If Existing address doesn't belong to connected wallet,
// generate new one.
if !mine {
nAddr, err := c.newDepositAddress(w)
if err != nil {
return generatedNewAddr, err
}
generatedNewAddr = true
c.log.Warnf("[%v]: Deposit address %v does not belong to connected wallet"+
", generated new address: %v", unbip(w.AssetID), addr, nAddr)
}
}
// If the wallet is not synced, start a loop to check the sync status until
// it is.
Expand Down Expand Up @@ -1093,14 +1123,14 @@ func (c *Core) connectWallet(w *xcWallet) error {
}
}()
}
return nil
return generatedNewAddr, nil
}

// Connect to the wallet if not already connected. Unlock the wallet if not
// already unlocked.
func (c *Core) connectAndUnlock(crypter encrypt.Crypter, wallet *xcWallet) error {
if !wallet.connected() {
err := c.connectWallet(wallet)
_, err := c.connectWallet(wallet)
if err != nil {
return err
}
Expand Down Expand Up @@ -1249,6 +1279,19 @@ func (c *Core) refreshUser() {
c.userMtx.Unlock()
}

// setUserWalletState updates wallet state on current User.
func (c *Core) setUserWalletState(state *WalletState) {
c.userMtx.Lock()
defer c.userMtx.Unlock()

sa, found := c.user.Assets[state.AssetID]
if !found {
c.log.Errorf("Unknown asset %d", state.AssetID)
return
}
sa.Wallet = state
}

// CreateWallet creates a new exchange wallet.
func (c *Core) CreateWallet(appPW, walletPW []byte, form *WalletForm) error {

Expand Down Expand Up @@ -1304,7 +1347,7 @@ func (c *Core) CreateWallet(appPW, walletPW []byte, form *WalletForm) error {
return fmt.Errorf("error loading wallet for %d -> %s: %v", assetID, symbol, err)
}

err = c.connectWallet(wallet)
_, err = c.connectWallet(wallet)
if err != nil {
return err
}
Expand Down Expand Up @@ -1508,6 +1551,7 @@ func (c *Core) ReconfigureWallet(appPW []byte, assetID uint32, cfg map[string]st
AssetID: oldWallet.AssetID,
Settings: cfg,
EncryptedPW: oldWallet.encPW,
Address: oldWallet.address,
}
if oldWallet.balance != nil {
dbWallet.Balance = oldWallet.balance.Balance
Expand All @@ -1517,27 +1561,25 @@ func (c *Core) ReconfigureWallet(appPW []byte, assetID uint32, cfg map[string]st
if err != nil {
return newError(walletErr, "error loading wallet for %d -> %s: %v", assetID, unbip(assetID), err)
}

// Must connect to ensure settings are good.
err = c.connectWallet(wallet)
generatedNewAddress, err := c.connectWallet(wallet)
if err != nil {
return err
}
// Get a new address. Definitely want this when the account changes, and
// maybe other settings as well.
addr, err := wallet.Address()
if err != nil {
wallet.Disconnect()
return newError(addrErr, "error getting wallet address: %v", err)
}
dbWallet.Address = addr
wallet.address = addr
if oldWallet.unlocked() {
err := unlockWallet(wallet, crypter)
if err != nil {
wallet.Disconnect()
return newError(walletAuthErr, "wallet successfully connected, but errored unlocking. reconfiguration not saved: %v", err)
}
}

// If connectWallet generated new xcWallet address store it on dbWallet.
if generatedNewAddress {
dbWallet.Address = wallet.address
}

err = c.db.UpdateWallet(dbWallet)
if err != nil {
wallet.Disconnect()
Expand Down Expand Up @@ -1616,7 +1658,7 @@ func (c *Core) SetWalletPassword(appPW []byte, assetID uint32, newPW []byte) err
// Connect if necessary.
wasConnected := wallet.connected()
if !wasConnected {
if err = c.connectWallet(wallet); err != nil {
if _, err = c.connectWallet(wallet); err != nil {
return newError(connectionErr, "SetWalletPassword connection error: %v", err)
}
}
Expand Down Expand Up @@ -1659,26 +1701,19 @@ func (c *Core) SetWalletPassword(appPW []byte, assetID uint32, newPW []byte) err
return nil
}

// NewDepositAddress retrieves a new deposit address from the specified wallet
// and saves it to the database.
func (c *Core) NewDepositAddress(assetID uint32) (string, error) {
w, exists := c.wallet(assetID)
if !exists {
return "", newError(missingWalletErr, "no wallet found for %s", unbip(assetID))
}

// newDepositAddress retrieves a new deposit address from given xcWallet.
func (c *Core) newDepositAddress(w *xcWallet) (string, error) {
if !w.connected() {
return "", fmt.Errorf("cannot get address from unconnected %s wallet", unbip(assetID))
return "", fmt.Errorf("cannot get address from unconnected %s wallet",
unbip(w.AssetID))
}

addr, err := w.Address()
if err != nil {
return "", fmt.Errorf("%s Wallet.Address error: %w", unbip(assetID), err)
return "", fmt.Errorf("%s Wallet.Address error: %w", unbip(w.AssetID), err)
}

c.walletMtx.Lock()
w.address = addr
c.walletMtx.Unlock()
// Set xcWallet's new address
w.setAddress(addr)

dbWallet, err := c.db.Wallet(w.dbID)
if err != nil {
Expand All @@ -1687,14 +1722,25 @@ func (c *Core) NewDepositAddress(assetID uint32) (string, error) {
dbWallet.Address = addr
err = c.db.UpdateWallet(dbWallet)
if err != nil {
return "", fmt.Errorf("UpdateWallet error for %s: %w", unbip(assetID), err)
return "", fmt.Errorf("UpdateWallet error for %s: %w", unbip(w.AssetID), err)
}
// Update wallet state on user struct
walletState := w.state()
c.setUserWalletState(walletState)
c.notify(newWalletStateNote(walletState))

c.refreshUser()
return addr, nil
}

c.notify(newWalletStateNote(w.state()))
// NewDepositAddress retrieves a new deposit address from the specified asset's
// wallet and saves it to the database.
func (c *Core) NewDepositAddress(assetID uint32) (string, error) {
w, exists := c.wallet(assetID)
if !exists {
return "", newError(missingWalletErr, "no wallet found for %s", unbip(assetID))
}

return addr, nil
return c.newDepositAddress(w)
}

// AutoWalletConfig attempts to load setting from a wallet package's
Expand Down Expand Up @@ -2032,7 +2078,7 @@ func (c *Core) Login(pw []byte) (*LoginResult, error) {
go func(wallet *xcWallet) {
defer wg.Done()
if !wallet.connected() {
err := c.connectWallet(wallet)
_, err := c.connectWallet(wallet)
if err != nil {
c.log.Errorf("Unable to connect to %s wallet (start and sync wallets BEFORE starting dex!): %v",
unbip(wallet.AssetID), err)
Expand Down
6 changes: 5 additions & 1 deletion client/core/core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,10 @@ func (w *TXCWallet) Info() *asset.WalletInfo {
return &asset.WalletInfo{}
}

func (w *TXCWallet) OwnsAddress(address string) (bool, error) {
return true, nil
}

func (w *TXCWallet) Connect(ctx context.Context) (*sync.WaitGroup, error) {
return w.connectWG, w.connectErr
}
Expand Down Expand Up @@ -5661,7 +5665,7 @@ func TestWalletSyncing(t *testing.T) {
return false, progress, nil
}

err := tCore.connectWallet(dcrWallet)
_, err := tCore.connectWallet(dcrWallet)
if err != nil {
t.Fatalf("connectWallet error: %v", err)
}
Expand Down
3 changes: 3 additions & 0 deletions client/webserver/site/src/html/wallets.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@
{{template "walletConfigTemplates"}}
</div>
<hr class="dashed my-2 mx-4">
<div class="px-4 my-2">
Note: Changing to a different wallet while having active trades might cause funds to be lost.
</div>
<div class="d-flex px-4 mt-1">
<div class="col-12 p-0">
<label for="reconfigPW" class="pl-1 mb-1">App Password</label>
Expand Down

0 comments on commit c3990c7

Please sign in to comment.