From fe5145770c2f9ed5d8fca1293046f5e352e67843 Mon Sep 17 00:00:00 2001 From: martonp Date: Fri, 18 Nov 2022 14:07:30 -0500 Subject: [PATCH] Chappjc reivew --- client/asset/bch/spv.go | 10 ++-- client/asset/btc/btc.go | 2 +- client/asset/btc/spv.go | 6 +-- client/asset/btc/spv_peer_manager.go | 73 +++++++++++++--------------- client/asset/btc/spv_wrapper.go | 6 ++- client/asset/ltc/spv.go | 10 ++-- 6 files changed, 58 insertions(+), 49 deletions(-) diff --git a/client/asset/bch/spv.go b/client/asset/bch/spv.go index c7f45d5617..80b7aa5773 100644 --- a/client/asset/bch/spv.go +++ b/client/asset/bch/spv.go @@ -91,7 +91,7 @@ var _ btc.BTCWallet = (*bchSPVWallet)(nil) // openSPVWallet creates a bchSPVWallet, but does not Start. // Satisfies btc.BTCWalletConstructor. -func openSPVWallet(dir string, cfg *btc.WalletConfig, btcParams *chaincfg.Params, log dex.Logger) (btc.BTCWallet, error) { +func openSPVWallet(dir string, cfg *btc.WalletConfig, btcParams *chaincfg.Params, log dex.Logger) btc.BTCWallet { var bchParams *bchchaincfg.Params switch btcParams.Name { case dexbch.MainNetParams.Name: @@ -110,7 +110,7 @@ func openSPVWallet(dir string, cfg *btc.WalletConfig, btcParams *chaincfg.Params allowAutomaticRescan: !cfg.ActivelyUsed, } w.birthdayV.Store(cfg.AdjustedBirthday()) - return w, nil + return w } // createSPVWallet creates a new SPV wallet. @@ -245,7 +245,7 @@ func (w *bchSPVWallet) Start() (btc.SPVService, error) { // Add the address for a local bchd testnet4 node. defaultPeers = []string{} case bchwire.TestNet, bchwire.SimNet: // plain "wire.TestNet" is regnet! - defaultPeers = []string{"localhost:21577"} + defaultPeers = []string{"127.0.0.1:21577"} } peerManager := btc.NewSPVPeerManager(&spvService{w.cl}, defaultPeers, w.dir, w.log, w.chainParams.DefaultPort) w.peerManager = peerManager @@ -848,6 +848,10 @@ func (s *spvService) AddPeer(addr string) error { return s.ChainService.ConnectNode(addr, true) } +func (s *spvService) RemovePeer(addr string) error { + return s.ChainService.RemoveNodeByAddr(addr) +} + func (s *spvService) GetBlockHeight(h *chainhash.Hash) (int32, error) { return s.ChainService.GetBlockHeight((*bchchainhash.Hash)(h)) } diff --git a/client/asset/btc/btc.go b/client/asset/btc/btc.go index 1a4d664d16..afa656de63 100644 --- a/client/asset/btc/btc.go +++ b/client/asset/btc/btc.go @@ -1230,7 +1230,7 @@ func OpenSPVWallet(cfg *BTCCloneCFG, walletConstructor BTCWalletConstructor) (*E decodeAddr: btc.decodeAddr, } - spvw.wallet, err = walletConstructor(spvw.dir, spvw.cfg, spvw.chainParams, spvw.log) + spvw.wallet = walletConstructor(spvw.dir, spvw.cfg, spvw.chainParams, spvw.log) if err != nil { return nil, err } diff --git a/client/asset/btc/spv.go b/client/asset/btc/spv.go index 1743a9506f..f439bb7fb8 100644 --- a/client/asset/btc/spv.go +++ b/client/asset/btc/spv.go @@ -116,7 +116,7 @@ func createSPVWallet(privPass []byte, seed []byte, bday time.Time, dataDir strin // openSPVWallet is the BTCWalletConstructor for Bitcoin. func openSPVWallet(dir string, cfg *WalletConfig, - chainParams *chaincfg.Params, log dex.Logger) (BTCWallet, error) { + chainParams *chaincfg.Params, log dex.Logger) BTCWallet { w := &btcSPVWallet{ dir: dir, @@ -125,7 +125,7 @@ func openSPVWallet(dir string, cfg *WalletConfig, allowAutomaticRescan: !cfg.ActivelyUsed, } w.birthdayV.Store(cfg.AdjustedBirthday()) - return w, nil + return w } func (w *btcSPVWallet) Birthday() time.Time { @@ -204,7 +204,7 @@ func (w *btcSPVWallet) Start() (SPVService, error) { case wire.TestNet3: defaultPeers = []string{"dex-test.ssgen.io:18333"} case wire.TestNet, wire.SimNet: // plain "wire.TestNet" is regnet! - defaultPeers = []string{"localhost:20575"} + defaultPeers = []string{"127.0.0.1:20575"} } peerManager := NewSPVPeerManager(&btcChainService{w.cl}, defaultPeers, w.dir, w.log, w.chainParams.DefaultPort) w.peerManager = peerManager diff --git a/client/asset/btc/spv_peer_manager.go b/client/asset/btc/spv_peer_manager.go index 2d29ae6e47..1efa81794f 100644 --- a/client/asset/btc/spv_peer_manager.go +++ b/client/asset/btc/spv_peer_manager.go @@ -10,7 +10,6 @@ import ( "net" "os" "path/filepath" - "strconv" "strings" "sync" @@ -44,7 +43,7 @@ type walletPeer struct { // to communicate with a chain service. type PeerManagerChainService interface { AddPeer(addr string) error - RemoveNodeByAddr(addr string) error + RemovePeer(addr string) error Peers() []SPVPeer } @@ -65,9 +64,21 @@ type SPVPeerManager struct { log dex.Logger } -func (w *SPVPeerManager) connectedPeers() map[string]interface{} { +// NewSPVPeerManager creates a new SPVPeerManager. +func NewSPVPeerManager(cs PeerManagerChainService, defaultPeers []string, dir string, log dex.Logger, defaultPort string) *SPVPeerManager { + return &SPVPeerManager{ + cs: cs, + defaultPeers: defaultPeers, + peers: make(map[string]*walletPeer), + savedPeersFilePath: filepath.Join(dir, "dexc-peers.json"), // peers.json is used by neutrino + log: log, + defaultPort: defaultPort, + } +} + +func (w *SPVPeerManager) connectedPeers() map[string]struct{} { peers := w.cs.Peers() - connectedPeers := make(map[string]interface{}) + connectedPeers := make(map[string]struct{}, len(peers)) for _, peer := range peers { connectedPeers[peer.Addr()] = struct{}{} } @@ -127,29 +138,24 @@ func (w *SPVPeerManager) resolveAddress(addr string) (string, error) { // Tor addresses cannot be resolved to an IP, so just return onionAddr // instead. if strings.HasSuffix(host, ".onion") { - return host, nil - } - - // Attempt to look up an IP address associated with the parsed host. - ips, err := net.LookupIP(host) - if err != nil { - return "", err + return addr, nil } - if len(ips) == 0 { - return "", fmt.Errorf("no addresses found for %s", host) - } - - port, err := strconv.Atoi(strPort) - if err != nil { - return "", err + var ip string + if host == "localhost" { + ip = "127.0.0.1" + } else { + ips, err := net.LookupIP(host) + if err != nil { + return "", err + } + if len(ips) == 0 { + return "", fmt.Errorf("no addresses found for %s", host) + } + ip = ips[0].String() } - return (&net.TCPAddr{ - IP: ips[0], - Port: port, - }).String(), nil - + return net.JoinHostPort(ip, strPort), nil } // peerWithResolvedAddress checks to see if there is a peer with a resolved @@ -204,7 +210,7 @@ func (w *SPVPeerManager) addPeer(addr string, source peerSource, initialLoad boo // to the user. If a user previously added a peer that originally connected // but now the address cannot be resolved to an IP, it should be displayed // that the wallet was unable to connect to that peer. - w.peers[addr] = &walletPeer{source: source, resolvedName: resolvedAddr} + w.peers[addr] = &walletPeer{source: source} } return fmt.Errorf("failed to resolve address: %v", err) } @@ -268,7 +274,7 @@ func (w *SPVPeerManager) RemovePeer(addr string) error { connectedPeers := w.connectedPeers() _, connected := connectedPeers[peer.resolvedName] if connected { - return w.cs.RemoveNodeByAddr(peer.resolvedName) + return w.cs.RemovePeer(peer.resolvedName) } return nil @@ -278,7 +284,10 @@ func (w *SPVPeerManager) RemovePeer(addr string) error { // that were added by the user and persisted in the db. func (w *SPVPeerManager) ConnectToInitialWalletPeers() { for _, peer := range w.defaultPeers { - w.addPeer(peer, defaultPeer, true) + err := w.addPeer(peer, defaultPeer, true) + if err != nil { + w.log.Errorf("failed to add default peer %s: %v", peer, err) + } } savedPeers, err := w.loadSavedPeersFromFile() @@ -294,15 +303,3 @@ func (w *SPVPeerManager) ConnectToInitialWalletPeers() { } } } - -// NewSPVPeerManager creates a new SPVPeerManager. -func NewSPVPeerManager(cs PeerManagerChainService, defaultPeers []string, dir string, log dex.Logger, defaultPort string) *SPVPeerManager { - return &SPVPeerManager{ - cs: cs, - defaultPeers: defaultPeers, - peers: make(map[string]*walletPeer), - savedPeersFilePath: filepath.Join(dir, "dexc-peers.json"), // peers.json is used by neutrino - log: log, - defaultPort: defaultPort, - } -} diff --git a/client/asset/btc/spv_wrapper.go b/client/asset/btc/spv_wrapper.go index bbf0c852ec..64dc2e12e8 100644 --- a/client/asset/btc/spv_wrapper.go +++ b/client/asset/btc/spv_wrapper.go @@ -167,10 +167,14 @@ func (s *btcChainService) AddPeer(addr string) error { return s.ChainService.ConnectNode(addr, true) } +func (s *btcChainService) RemovePeer(addr string) error { + return s.ChainService.RemoveNodeByAddr(addr) +} + var _ SPVService = (*btcChainService)(nil) // BTCWalletConstructor is a function to construct a BTCWallet. -type BTCWalletConstructor func(dir string, cfg *WalletConfig, chainParams *chaincfg.Params, log dex.Logger) (BTCWallet, error) +type BTCWalletConstructor func(dir string, cfg *WalletConfig, chainParams *chaincfg.Params, log dex.Logger) BTCWallet func extendAddresses(extIdx, intIdx uint32, btcw *wallet.Wallet) error { scopedKeyManager, err := btcw.Manager.FetchScopedKeyManager(waddrmgr.KeyScopeBIP0084) diff --git a/client/asset/ltc/spv.go b/client/asset/ltc/spv.go index 36dca9bc20..22bba54e45 100644 --- a/client/asset/ltc/spv.go +++ b/client/asset/ltc/spv.go @@ -98,7 +98,7 @@ type ltcSPVWallet struct { var _ btc.BTCWallet = (*ltcSPVWallet)(nil) // openSPVWallet creates a ltcSPVWallet, but does not Start. -func openSPVWallet(dir string, cfg *btc.WalletConfig, btcParams *chaincfg.Params, log dex.Logger) (btc.BTCWallet, error) { +func openSPVWallet(dir string, cfg *btc.WalletConfig, btcParams *chaincfg.Params, log dex.Logger) btc.BTCWallet { var ltcParams *ltcchaincfg.Params switch btcParams.Name { case dexltc.MainNetParams.Name: @@ -116,7 +116,7 @@ func openSPVWallet(dir string, cfg *btc.WalletConfig, btcParams *chaincfg.Params allowAutomaticRescan: !cfg.ActivelyUsed, } w.birthdayV.Store(cfg.AdjustedBirthday()) - return w, nil + return w } // createSPVWallet creates a new SPV wallet. @@ -272,7 +272,7 @@ func (w *ltcSPVWallet) Start() (btc.SPVService, error) { defaultPeers = append(defaultPeers, addr.String()) } case ltcwire.TestNet, ltcwire.SimNet: // plain "wire.TestNet" is regnet! - defaultPeers = []string{"localhost:20585"} + defaultPeers = []string{"127.0.0.1:20585"} } peerManager := btc.NewSPVPeerManager(&spvService{w.cl}, defaultPeers, w.dir, w.log, w.chainParams.DefaultPort) w.peerManager = peerManager @@ -889,6 +889,10 @@ func (s *spvService) AddPeer(addr string) error { return s.ChainService.ConnectNode(addr, true) } +func (s *spvService) RemovePeer(addr string) error { + return s.ChainService.RemoveNodeByAddr(addr) +} + func (s *spvService) GetBlockHeight(h *chainhash.Hash) (int32, error) { return s.ChainService.GetBlockHeight((*ltcchainhash.Hash)(h)) }