Skip to content

Commit

Permalink
most of wisdoms review
Browse files Browse the repository at this point in the history
  • Loading branch information
buck54321 committed Oct 18, 2021
1 parent 51cd3ea commit 5ce2e16
Show file tree
Hide file tree
Showing 10 changed files with 62 additions and 65 deletions.
4 changes: 1 addition & 3 deletions client/asset/bch/bch.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,7 @@ func (d *Driver) Info() *asset.WalletInfo {
}

// NewWallet is the exported constructor by which the DEX will import the
// exchange wallet. The wallet will shut down when the provided context is
// canceled. The configPath can be an empty string, in which case the standard
// system location of the daemon config file is assumed.
// exchange wallet.
func NewWallet(cfg *asset.WalletConfig, logger dex.Logger, network dex.Network) (asset.Wallet, error) {
var params *chaincfg.Params
switch network {
Expand Down
8 changes: 3 additions & 5 deletions client/asset/bch/regnet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ import (
dexbtc "decred.org/dcrdex/dex/networks/btc"
)

const alphaAddress = "bchreg:qqnm4z2tftyyeu3kvzzepmlp9mj3g6fvxgft570vll"

var (
tLotSize uint64 = 1e6
tRateStep uint64 = 10
Expand All @@ -41,8 +39,8 @@ var (

func TestWallet(t *testing.T) {
livetest.Run(t, &livetest.Config{
New: NewWallet,
LotSize: tLotSize,
Asset: tBCH,
NewWallet: NewWallet,
LotSize: tLotSize,
Asset: tBCH,
})
}
56 changes: 30 additions & 26 deletions client/asset/btc/btc.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ const (
splitTxBaggageSegwit = dexbtc.MinimumTxOverhead + 2*dexbtc.P2WPKHOutputSize +
dexbtc.RedeemP2WPKHInputSize + ((dexbtc.RedeemP2WPKHInputWitnessWeight + 2 + 3) / 4)

WalletTypeLegacy = ""
WalletTypeRPC = "bitcoindRPC"
WalletTypeSPV = "SPV"
walletTypeLegacy = ""
walletTypeRPC = "bitcoindRPC"
walletTypeSPV = "SPV"
)

var (
Expand Down Expand Up @@ -151,14 +151,14 @@ var (
},
}
rpcWalletDefinition = &asset.WalletDefinition{
Type: WalletTypeRPC,
Type: walletTypeRPC,
Tab: "External",
Description: "Connect to bitcoind",
DefaultConfigPath: dexbtc.SystemConfigPath("bitcoin"),
ConfigOpts: append(append([]*asset.ConfigOption(nil), rpcOpts...), commonOpts...),
ConfigOpts: append(rpcOpts, commonOpts...),
}
spvWalletDefinition = &asset.WalletDefinition{
Type: WalletTypeSPV,
Type: walletTypeSPV,
Tab: "Native",
Description: "Use the built-in SPV wallet",
ConfigOpts: commonOpts,
Expand Down Expand Up @@ -373,8 +373,8 @@ type WalletConfig struct {
Peer string `ini:"peer"` // SPV
}

// PrepareConfig parses the settings map into a *RPCWalletConfig.
func PrepareConfig(settings map[string]string, symbol string, net dex.Network, ports dexbtc.NetPorts) (cfg *RPCWalletConfig, err error) {
// readRPCWalletConfig parses the settings map into a *RPCWalletConfig.
func readRPCWalletConfig(settings map[string]string, symbol string, net dex.Network, ports dexbtc.NetPorts) (cfg *RPCWalletConfig, err error) {
cfg = new(RPCWalletConfig)
err = config.Unmapify(settings, &cfg.WalletConfig)
if err != nil {
Expand All @@ -388,14 +388,21 @@ func PrepareConfig(settings map[string]string, symbol string, net dex.Network, p
return
}

// ParseRPCWalletConfig parses a *RPCWalletConfig from the settings map and
// parseRPCWalletConfig parses a *RPCWalletConfig from the settings map and
// creates the unconnected *rpcclient.Client.
func ParseRPCWalletConfig(settings map[string]string, symbol string, net dex.Network, ports dexbtc.NetPorts) (*RPCWalletConfig, *rpcclient.Client, error) {
cfg, err := PrepareConfig(settings, symbol, net, ports)
func parseRPCWalletConfig(settings map[string]string, symbol string, net dex.Network, ports dexbtc.NetPorts) (*RPCWalletConfig, *rpcclient.Client, error) {
cfg, err := readRPCWalletConfig(settings, symbol, net, ports)
if err != nil {
return nil, nil, err
}
cl, err := newRPCWalletConnection(cfg)
endpoint := cfg.RPCBind + "/wallet/" + cfg.WalletName
cl, err := rpcclient.New(&rpcclient.ConnConfig{
HTTPPostMode: true,
DisableTLS: true,
Host: endpoint,
User: cfg.RPCUser,
Pass: cfg.RPCPass,
}, nil)
if err != nil {
return nil, nil, err
}
Expand All @@ -405,16 +412,15 @@ func ParseRPCWalletConfig(settings map[string]string, symbol string, net dex.Net
// Driver implements asset.Driver.
type Driver struct{}

// Check that Driver implements Driver, Creator, and Initializer.s
// Check that Driver implements Driver and Creator.
var _ asset.Driver = (*Driver)(nil)
var _ asset.Creator = (*Driver)(nil)
var _ asset.Initializer = (*Driver)(nil)

// Exists checks the existence of the wallet. For the RPC wallet, this attempts
// to connect and request getnetworkinfo to verify existence. For the SPV wallet,
// we ultimately just look for wallet files.
// Exists checks the existence of the wallet. Part of the Creator interface, so
// only used for wallets with WalletDefinition.Seeded = true.
func (d *Driver) Exists(walletType, dataDir string, settings map[string]string, net dex.Network) (bool, error) {
if walletType != WalletTypeSPV {
if walletType != walletTypeSPV {
return false, fmt.Errorf("no Bitcoin wallet of type %q available", walletType)
}

Expand All @@ -430,8 +436,8 @@ func (d *Driver) Exists(walletType, dataDir string, settings map[string]string,

// Create creates a new SPV wallet.
func (d *Driver) Create(params *asset.CreateWalletParams) error {
if params.Type != WalletTypeSPV {
return fmt.Errorf("SPV is the only seeded wallet type. required = %q, requested = %q", WalletTypeSPV, params.Type)
if params.Type != walletTypeSPV {
return fmt.Errorf("SPV is the only seeded wallet type. required = %q, requested = %q", walletTypeSPV, params.Type)
}
if len(params.Seed) == 0 {
return errors.New("wallet seed cannot be empty")
Expand Down Expand Up @@ -586,9 +592,7 @@ func parseChainParams(net dex.Network) (*chaincfg.Params, error) {
}

// NewWallet is the exported constructor by which the DEX will import the
// exchange wallet. The wallet will shut down when the provided context is
// canceled. The configPath can be an empty string, in which case the standard
// system location of the bitcoind config file is assumed.
// exchange wallet.
func NewWallet(cfg *asset.WalletConfig, logger dex.Logger, net dex.Network) (asset.Wallet, error) {
params, err := parseChainParams(net)
if err != nil {
Expand All @@ -610,12 +614,12 @@ func NewWallet(cfg *asset.WalletConfig, logger dex.Logger, net dex.Network) (ass
}

switch cfg.Type {
case WalletTypeSPV:
case walletTypeSPV:
if atomic.LoadUint32(&initialized) == 0 {
return nil, fmt.Errorf("must call asset.Initialize if you want to create an SPV wallet")
}
return openSPVWallet(cloneCFG)
case WalletTypeRPC, WalletTypeLegacy:
case walletTypeRPC, walletTypeLegacy:
return BTCCloneWallet(cloneCFG)
default:
return nil, fmt.Errorf("unknown wallet type %q", cfg.Type)
Expand All @@ -627,7 +631,7 @@ func NewWallet(cfg *asset.WalletConfig, logger dex.Logger, net dex.Network) (ass
// conjunction with ReadCloneParams, to create a ExchangeWallet for other assets
// with minimal coding.
func BTCCloneWallet(cfg *BTCCloneCFG) (*ExchangeWallet, error) {
clientCfg, client, err := ParseRPCWalletConfig(cfg.WalletCFG.Settings, cfg.Symbol, cfg.Network, cfg.Ports)
clientCfg, client, err := parseRPCWalletConfig(cfg.WalletCFG.Settings, cfg.Symbol, cfg.Network, cfg.Ports)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -729,7 +733,7 @@ func newUnconnectedWallet(cfg *BTCCloneCFG, walletCfg *WalletConfig) (*ExchangeW
return w, nil
}

// openSPVWallet opens the pre-existing native SPV wallet.
// openSPVWallet opens the previously created native SPV wallet.
func openSPVWallet(cfg *BTCCloneCFG) (*ExchangeWallet, error) {
walletCfg := new(WalletConfig)
err := config.Unmapify(cfg.WalletCFG.Settings, walletCfg)
Expand Down
26 changes: 13 additions & 13 deletions client/asset/btc/btc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -586,9 +586,9 @@ func tNewWallet(segwit bool, walletType string) (*ExchangeWallet, *testData, fun
var wallet *ExchangeWallet
var err error
switch walletType {
case WalletTypeRPC:
case walletTypeRPC:
wallet, err = newRPCWallet(&tRawRequester{data}, cfg, &WalletConfig{})
case WalletTypeSPV:
case walletTypeSPV:
wallet, err = newUnconnectedWallet(cfg, &WalletConfig{})
if err == nil {
neutrinoClient := &tNeutrinoClient{data}
Expand Down Expand Up @@ -653,13 +653,13 @@ type testFunc func(t *testing.T, segwit bool, walletType string)

func runRubric(t *testing.T, f testFunc) {
t.Run("rpc|segwit", func(t *testing.T) {
f(t, true, WalletTypeRPC)
f(t, true, walletTypeRPC)
})
t.Run("rpc|non-segwit", func(t *testing.T) {
f(t, false, WalletTypeRPC)
f(t, false, walletTypeRPC)
})
t.Run("spv|segwit", func(t *testing.T) {
f(t, true, WalletTypeSPV)
f(t, true, walletTypeSPV)
})
}

Expand Down Expand Up @@ -824,7 +824,7 @@ func testAvailableFund(t *testing.T, segwit bool, walletType string) {

// Negative response when locking outputs.
// There is no way to error locking outpoints in spv
if walletType != WalletTypeSPV {
if walletType != walletTypeSPV {
node.lockUnspentErr = tErr
_, _, err = wallet.FundOrder(ord)
if err == nil {
Expand Down Expand Up @@ -1035,7 +1035,7 @@ func (c *tCoin) String() string { return hex.EncodeToString(c.id) }
func (c *tCoin) Value() uint64 { return 100 }

func TestReturnCoins(t *testing.T) {
wallet, node, shutdown, err := tNewWallet(true, WalletTypeRPC)
wallet, node, shutdown, err := tNewWallet(true, walletTypeRPC)
defer shutdown()
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -1073,7 +1073,7 @@ func TestReturnCoins(t *testing.T) {

func TestFundingCoins(t *testing.T) {
// runRubric(t, testFundingCoins)
testFundingCoins(t, false, WalletTypeRPC)
testFundingCoins(t, false, walletTypeRPC)
}

func testFundingCoins(t *testing.T, segwit bool, walletType string) {
Expand Down Expand Up @@ -1194,7 +1194,7 @@ func checkSwapEstimate(t *testing.T, est *asset.SwapEstimate, lots, swapVal, max
}

func TestFundEdges(t *testing.T) {
wallet, node, shutdown, err := tNewWallet(false, WalletTypeRPC)
wallet, node, shutdown, err := tNewWallet(false, walletTypeRPC)
defer shutdown()
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -1414,7 +1414,7 @@ func TestFundEdges(t *testing.T) {
}

func TestFundEdgesSegwit(t *testing.T) {
wallet, node, shutdown, err := tNewWallet(true, WalletTypeRPC)
wallet, node, shutdown, err := tNewWallet(true, walletTypeRPC)
defer shutdown()
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -2258,7 +2258,7 @@ func testLockUnlock(t *testing.T, segwit bool, walletType string) {
}

// Locking can't error on SPV.
if walletType == WalletTypeRPC {
if walletType == walletTypeRPC {
err = wallet.Lock()
if err != nil {
t.Fatalf("lock error: %v", err)
Expand Down Expand Up @@ -2703,7 +2703,7 @@ func testPreRedeem(t *testing.T, segwit bool, walletType string) {

func TestTryRedemptionRequests(t *testing.T) {
// runRubric(t, testTryRedemptionRequests)
testTryRedemptionRequests(t, true, WalletTypeSPV)
testTryRedemptionRequests(t, true, walletTypeSPV)
}

func testTryRedemptionRequests(t *testing.T, segwit bool, walletType string) {
Expand Down Expand Up @@ -2911,7 +2911,7 @@ func testTryRedemptionRequests(t *testing.T, segwit bool, walletType string) {

for _, tt := range tests {
// Skip tests where we're expected to see mempool in SPV.
if walletType == WalletTypeSPV && isMempoolTest(tt) {
if walletType == walletTypeSPV && isMempoolTest(tt) {
continue
}

Expand Down
10 changes: 5 additions & 5 deletions client/asset/btc/livetest/livetest.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,11 @@ func randBytes(l int) []byte {
}

type Config struct {
New WalletConstructor
LotSize uint64
Asset *dex.Asset
SplitTx bool
SPV bool
NewWallet WalletConstructor
LotSize uint64
Asset *dex.Asset
SplitTx bool
SPV bool
}

func Run(t *testing.T, cfg *Config) {
Expand Down
8 changes: 4 additions & 4 deletions client/asset/btc/spv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ func (p *tSPVPeer) LastBlock() int32 {
}

func TestSwapConfirmations(t *testing.T) {
wallet, node, shutdown, _ := tNewWallet(true, WalletTypeSPV)
wallet, node, shutdown, _ := tNewWallet(true, walletTypeSPV)
defer shutdown()

spv := wallet.node.(*spvWallet)
Expand Down Expand Up @@ -451,7 +451,7 @@ func TestSwapConfirmations(t *testing.T) {
}

func TestFindBlockForTime(t *testing.T) {
wallet, node, shutdown, _ := tNewWallet(true, WalletTypeSPV)
wallet, node, shutdown, _ := tNewWallet(true, walletTypeSPV)
defer shutdown()
spv := wallet.node.(*spvWallet)

Expand Down Expand Up @@ -507,7 +507,7 @@ func TestFindBlockForTime(t *testing.T) {
}

func TestGetTxOut(t *testing.T) {
wallet, node, shutdown, _ := tNewWallet(true, WalletTypeSPV)
wallet, node, shutdown, _ := tNewWallet(true, walletTypeSPV)
defer shutdown()
spv := wallet.node.(*spvWallet)

Expand Down Expand Up @@ -611,7 +611,7 @@ func TestGetTxOut(t *testing.T) {
}

func TestSendWithSubtract(t *testing.T) {
wallet, node, shutdown, _ := tNewWallet(true, WalletTypeSPV)
wallet, node, shutdown, _ := tNewWallet(true, walletTypeSPV)
defer shutdown()
spv := wallet.node.(*spvWallet)

Expand Down
3 changes: 1 addition & 2 deletions client/asset/dcr/dcr.go
Original file line number Diff line number Diff line change
Expand Up @@ -482,8 +482,7 @@ func (cc *combinedClient) ValidateAddress(ctx context.Context, address stdaddr.A
var _ asset.Wallet = (*ExchangeWallet)(nil)

// NewWallet is the exported constructor by which the DEX will import the
// exchange wallet. The wallet will shut down when the provided context is
// canceled.
// exchange wallet.
func NewWallet(cfg *asset.WalletConfig, logger dex.Logger, network dex.Network) (*ExchangeWallet, error) {
// loadConfig will set fields if defaults are used and set the chainParams
// package variable.
Expand Down
2 changes: 1 addition & 1 deletion client/asset/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ var (

// CreateWalletParams are the parameters for internal wallet creation. The
// Settings provided should be the same wallet configuration settings passed to
// Create.
// OpenWallet.
type CreateWalletParams struct {
Type string
Seed []byte
Expand Down
4 changes: 1 addition & 3 deletions client/asset/ltc/ltc.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,7 @@ func (d *Driver) Info() *asset.WalletInfo {
}

// NewWallet is the exported constructor by which the DEX will import the
// exchange wallet. The wallet will shut down when the provided context is
// canceled. The configPath can be an empty string, in which case the standard
// system location of the litecoind config file is assumed.
// exchange wallet.
func NewWallet(cfg *asset.WalletConfig, logger dex.Logger, network dex.Network) (asset.Wallet, error) {
var params *chaincfg.Params
switch network {
Expand Down
6 changes: 3 additions & 3 deletions client/asset/ltc/regnet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ var (

func TestWallet(t *testing.T) {
livetest.Run(t, &livetest.Config{
New: NewWallet,
LotSize: tLotSize,
Asset: tLTC,
NewWallet: NewWallet,
LotSize: tLotSize,
Asset: tLTC,
})
}

0 comments on commit 5ce2e16

Please sign in to comment.