From d7f30a949f122847c5de9b8f7581829a112a561e Mon Sep 17 00:00:00 2001 From: jholdstock Date: Tue, 17 Sep 2024 11:53:23 +0100 Subject: [PATCH 1/2] Move VSP client code to wallet package. This moves the VSP client code from its own package into the wallet package. Moving this code should have minimal impact on consumers of the vsp package because they will have been importing the wallet package already due to the fact that a VSP client could not be created without an instance of wallet. The benefits of this are: - vsp client creation now exists only in the wallet package, rather than being scattered across 3 packages. - vsp client loading funcs and variables are no longer package global. - simplifies the creation of a vsp client because the wallet no longer needs to be passed into the func as a param. - no longer necessary to abstract the vsp client away by referring to its methods, the client can just be used directly. --- dcrwallet.go | 11 +-- internal/rpc/jsonrpc/methods.go | 30 +++----- internal/rpc/rpcserver/server.go | 74 ++++++++----------- ticketbuyer/tb.go | 11 +-- wallet/createtx.go | 12 +-- {vsp => wallet}/feepayment.go | 43 ++++++----- {vsp => wallet}/vsp.go | 68 +++++++---------- .../vsp.go => wallet/vspclientloader.go | 42 ++++------- wallet/wallet.go | 17 ++--- 9 files changed, 123 insertions(+), 185 deletions(-) rename {vsp => wallet}/feepayment.go (93%) rename {vsp => wallet}/vsp.go (85%) rename internal/loader/vsp.go => wallet/vspclientloader.go (52%) diff --git a/dcrwallet.go b/dcrwallet.go index 59b98847b..af39ced09 100644 --- a/dcrwallet.go +++ b/dcrwallet.go @@ -28,7 +28,6 @@ import ( "decred.org/dcrwallet/v5/spv" "decred.org/dcrwallet/v5/ticketbuyer" "decred.org/dcrwallet/v5/version" - "decred.org/dcrwallet/v5/vsp" "decred.org/dcrwallet/v5/wallet" "github.com/decred/dcrd/addrmgr/v2" "github.com/decred/dcrd/wire" @@ -190,7 +189,7 @@ func run(ctx context.Context) error { }() // Open the wallet when --noinitialload was not set. - var vspClient *vsp.Client + var vspClient *wallet.VSPClient passphrase := []byte{} if !cfg.NoInitialLoad { walletPass := []byte(cfg.WalletPass) @@ -271,19 +270,17 @@ func run(ctx context.Context) error { cfg.PurchaseAccount, err) return err } - vspCfg := vsp.Config{ + vspCfg := wallet.VSPClientConfig{ URL: cfg.VSPOpts.URL, PubKey: cfg.VSPOpts.PubKey, Dialer: cfg.dial, - Wallet: w, - Policy: &vsp.Policy{ + Policy: &wallet.VSPPolicy{ MaxFee: cfg.VSPOpts.MaxFee.Amount, FeeAcct: purchaseAcct, ChangeAcct: changeAcct, }, - Params: w.ChainParams(), } - vspClient, err = ldr.VSP(vspCfg) + vspClient, err = w.VSP(vspCfg) if err != nil { log.Errorf("vsp: %v", err) return err diff --git a/internal/rpc/jsonrpc/methods.go b/internal/rpc/jsonrpc/methods.go index ad349e6dc..03a117c8d 100644 --- a/internal/rpc/jsonrpc/methods.go +++ b/internal/rpc/jsonrpc/methods.go @@ -22,13 +22,11 @@ import ( "decred.org/dcrwallet/v5/chain" "decred.org/dcrwallet/v5/errors" - "decred.org/dcrwallet/v5/internal/loader" "decred.org/dcrwallet/v5/p2p" "decred.org/dcrwallet/v5/rpc/client/dcrd" "decred.org/dcrwallet/v5/rpc/jsonrpc/types" "decred.org/dcrwallet/v5/spv" "decred.org/dcrwallet/v5/version" - "decred.org/dcrwallet/v5/vsp" "decred.org/dcrwallet/v5/wallet" "decred.org/dcrwallet/v5/wallet/txauthor" "decred.org/dcrwallet/v5/wallet/txrules" @@ -3344,21 +3342,19 @@ func (s *Server) purchaseTicket(ctx context.Context, icmd any) (any, error) { } } - var vspClient *vsp.Client + var vspClient *wallet.VSPClient if s.cfg.VSPHost != "" { - cfg := vsp.Config{ + cfg := wallet.VSPClientConfig{ URL: s.cfg.VSPHost, PubKey: s.cfg.VSPPubKey, Dialer: s.cfg.Dial, - Wallet: w, - Policy: &vsp.Policy{ + Policy: &wallet.VSPPolicy{ MaxFee: s.cfg.VSPMaxFee, FeeAcct: account, ChangeAcct: changeAccount, }, - Params: w.ChainParams(), } - vspClient, err = loader.VSP(cfg) + vspClient, err = w.VSP(cfg) if err != nil { return nil, rpcErrorf(dcrjson.ErrRPCMisc, "VSP Server instance failed to start: %v", err) @@ -3378,11 +3374,8 @@ func (s *Server) purchaseTicket(ctx context.Context, icmd any) (any, error) { MixedAccountBranch: mixedAccountBranch, MixedSplitAccount: mixedSplitAccount, ChangeAccount: changeAccount, - } - if vspClient != nil { - request.VSPFeePaymentProcess = vspClient.Process - request.VSPFeePercent = vspClient.FeePercentage + VSPClient: vspClient, } ticketsResponse, err := w.PurchaseTickets(ctx, n, request) @@ -3447,16 +3440,17 @@ func (s *Server) processUnmanagedTicket(ctx context.Context, icmd any) (any, err if vspHost == "" { return nil, rpcErrorf(dcrjson.ErrRPCInvalidParameter, "vsphost must be set in options") } - vspClient, err := loader.LookupVSP(vspHost) - if err != nil { - return nil, err - } w, ok := s.walletLoader.LoadedWallet() if !ok { return nil, errUnloadedWallet } + vspClient, err := w.LookupVSP(vspHost) + if err != nil { + return nil, err + } + ticket, err := w.NewVSPTicket(ctx, hash) if err != nil { return nil, err @@ -4703,7 +4697,7 @@ func (s *Server) updateVSPVoteChoices(ctx context.Context, w *wallet.Wallet, tic } return err } - vspClient, err := loader.LookupVSP(vspHost) + vspClient, err := w.LookupVSP(vspHost) if err != nil { return err } @@ -4726,7 +4720,7 @@ func (s *Server) updateVSPVoteChoices(ctx context.Context, w *wallet.Wallet, tic } return err } - vspClient, err := loader.LookupVSP(vspHost) + vspClient, err := w.LookupVSP(vspHost) if err != nil { return err } diff --git a/internal/rpc/rpcserver/server.go b/internal/rpc/rpcserver/server.go index 7846c0fc6..dd9caf1e7 100644 --- a/internal/rpc/rpcserver/server.go +++ b/internal/rpc/rpcserver/server.go @@ -39,7 +39,6 @@ import ( pb "decred.org/dcrwallet/v5/rpc/walletrpc" "decred.org/dcrwallet/v5/spv" "decred.org/dcrwallet/v5/ticketbuyer" - "decred.org/dcrwallet/v5/vsp" "decred.org/dcrwallet/v5/wallet" "decred.org/dcrwallet/v5/wallet/txauthor" "decred.org/dcrwallet/v5/wallet/txrules" @@ -1805,7 +1804,7 @@ func (s *walletServer) PurchaseTickets(ctx context.Context, // new vspd request var vspHost string var vspPubKey string - var vspClient *vsp.Client + var vspClient *wallet.VSPClient if req.VspHost != "" || req.VspPubkey != "" { vspHost = req.VspHost vspPubKey = req.VspPubkey @@ -1815,19 +1814,17 @@ func (s *walletServer) PurchaseTickets(ctx context.Context, if vspHost == "" { return nil, status.Errorf(codes.InvalidArgument, "vsp host can not be null") } - cfg := vsp.Config{ + cfg := wallet.VSPClientConfig{ URL: vspHost, PubKey: vspPubKey, Dialer: nil, - Wallet: s.wallet, - Policy: &vsp.Policy{ + Policy: &wallet.VSPPolicy{ MaxFee: 0.1e8, FeeAcct: req.Account, ChangeAcct: req.ChangeAccount, }, - Params: s.wallet.ChainParams(), } - vspClient, err = loader.VSP(cfg) + vspClient, err = s.wallet.VSP(cfg) if err != nil { return nil, status.Errorf(codes.Unknown, "VSP Server instance failed to start: %v", err) } @@ -1894,11 +1891,8 @@ func (s *walletServer) PurchaseTickets(ctx context.Context, MixedAccountBranch: mixedAccountBranch, MixedSplitAccount: mixedSplitAccount, ChangeAccount: changeAccount, - } - if vspClient != nil { - request.VSPFeePaymentProcess = vspClient.Process - request.VSPFeePercent = vspClient.FeePercentage + VSPClient: vspClient, } // If dontSignTx is false we unlock the wallet so we can sign the tx. @@ -2608,7 +2602,7 @@ func StartTicketBuyerService(server *grpc.Server, loader *loader.Loader) { // RunTicketBuyer starts the automatic ticket buyer. func (t *ticketbuyerServer) RunTicketBuyer(req *pb.RunTicketBuyerRequest, svr pb.TicketBuyerService_RunTicketBuyerServer) error { - wallet, ok := t.loader.LoadedWallet() + w, ok := t.loader.LoadedWallet() if !ok { return status.Errorf(codes.FailedPrecondition, "Wallet has not been loaded") } @@ -2619,7 +2613,7 @@ func (t *ticketbuyerServer) RunTicketBuyer(req *pb.RunTicketBuyerRequest, svr pb var err error var vspHost string var vspPubKey string - var vspClient *vsp.Client + var vspClient *wallet.VSPClient if req.VspHost != "" || req.VspPubkey != "" { vspHost = req.VspHost vspPubKey = req.VspPubkey @@ -2629,19 +2623,17 @@ func (t *ticketbuyerServer) RunTicketBuyer(req *pb.RunTicketBuyerRequest, svr pb if vspHost == "" { return status.Errorf(codes.InvalidArgument, "vsp host can not be null") } - cfg := vsp.Config{ + cfg := wallet.VSPClientConfig{ URL: vspHost, PubKey: vspPubKey, Dialer: nil, - Wallet: wallet, - Policy: &vsp.Policy{ + Policy: &wallet.VSPPolicy{ MaxFee: 0.1e8, FeeAcct: req.Account, ChangeAcct: req.Account, }, - Params: wallet.ChainParams(), } - vspClient, err = loader.VSP(cfg) + vspClient, err = w.VSP(cfg) if err != nil { return status.Errorf(codes.Unknown, "TicketBuyer instance failed to start. Error: %v", err) } @@ -2659,7 +2651,7 @@ func (t *ticketbuyerServer) RunTicketBuyer(req *pb.RunTicketBuyerRequest, svr pb if req.EnableMixing { mixedChange = true mixedAccount = req.MixedAccount - _, err = wallet.AccountName(ctx, mixedAccount) + _, err = w.AccountName(ctx, mixedAccount) if err != nil { return status.Errorf(codes.InvalidArgument, "Mixing requested, but error on mixed account: %v", err) @@ -2670,12 +2662,12 @@ func (t *ticketbuyerServer) RunTicketBuyer(req *pb.RunTicketBuyerRequest, svr pb "MixedAccountBranch should be 0 or 1.") } mixedSplitAccount = req.MixedSplitAccount - _, err = wallet.AccountName(ctx, mixedSplitAccount) + _, err = w.AccountName(ctx, mixedSplitAccount) if err != nil { return status.Errorf(codes.InvalidArgument, "Mixing requested, but error on mixedSplitAccount: %v", err) } - _, err = wallet.AccountName(ctx, changeAccount) + _, err = w.AccountName(ctx, changeAccount) if err != nil { return status.Errorf(codes.InvalidArgument, "Mixing requested, but error on changeAccount: %v", err) @@ -2686,7 +2678,7 @@ func (t *ticketbuyerServer) RunTicketBuyer(req *pb.RunTicketBuyerRequest, svr pb // is defaulted to 20. limit := int(req.Limit) - tb := ticketbuyer.New(wallet, ticketbuyer.Config{ + tb := ticketbuyer.New(w, ticketbuyer.Config{ BuyTickets: true, Account: req.Account, VotingAccount: req.VotingAccount, @@ -2709,7 +2701,7 @@ func (t *ticketbuyerServer) RunTicketBuyer(req *pb.RunTicketBuyerRequest, svr pb zero(req.Passphrase) } - err = wallet.Unlock(svr.Context(), req.Passphrase, lock) + err = w.Unlock(svr.Context(), req.Passphrase, lock) if err != nil { return translateError(err) } @@ -4112,20 +4104,18 @@ func (s *walletServer) SyncVSPFailedTickets(ctx context.Context, req *pb.SyncVSP if vspHost == "" { return nil, status.Errorf(codes.InvalidArgument, "vsp host can not be null") } - policy := &vsp.Policy{ + policy := &wallet.VSPPolicy{ MaxFee: 0.1e8, FeeAcct: req.Account, ChangeAcct: req.ChangeAccount, } - cfg := vsp.Config{ + cfg := wallet.VSPClientConfig{ URL: vspHost, PubKey: vspPubKey, Dialer: nil, - Wallet: s.wallet, Policy: policy, - Params: s.wallet.ChainParams(), } - vspClient, err := loader.VSP(cfg) + vspClient, err := s.wallet.VSP(cfg) if err != nil { return nil, status.Errorf(codes.Unknown, "TicketBuyer instance failed to start. Error: %v", err) } @@ -4161,20 +4151,18 @@ func (s *walletServer) ProcessManagedTickets(ctx context.Context, req *pb.Proces if vspHost == "" { return nil, status.Errorf(codes.InvalidArgument, "vsp host can not be null") } - policy := &vsp.Policy{ + policy := &wallet.VSPPolicy{ MaxFee: 0.1e8, FeeAcct: req.FeeAccount, ChangeAcct: req.ChangeAccount, } - cfg := vsp.Config{ + cfg := wallet.VSPClientConfig{ URL: vspHost, PubKey: vspPubKey, Dialer: nil, - Wallet: s.wallet, Policy: policy, - Params: s.wallet.ChainParams(), } - vspClient, err := loader.VSP(cfg) + vspClient, err := s.wallet.VSP(cfg) if err != nil { return nil, status.Errorf(codes.Unknown, "VSPClient instance failed to start. Error: %v", err) } @@ -4203,20 +4191,18 @@ func (s *walletServer) ProcessUnmanagedTickets(ctx context.Context, req *pb.Proc if vspHost == "" { return nil, status.Errorf(codes.InvalidArgument, "vsp host can not be null") } - policy := &vsp.Policy{ + policy := &wallet.VSPPolicy{ MaxFee: 0.1e8, FeeAcct: req.FeeAccount, ChangeAcct: req.ChangeAccount, } - cfg := vsp.Config{ + cfg := wallet.VSPClientConfig{ URL: vspHost, PubKey: vspPubKey, Dialer: nil, - Wallet: s.wallet, Policy: policy, - Params: s.wallet.ChainParams(), } - vspClient, err := loader.VSP(cfg) + vspClient, err := s.wallet.VSP(cfg) if err != nil { return nil, status.Errorf(codes.Unknown, "VSPClient instance failed to start. Error: %v", err) } @@ -4242,20 +4228,18 @@ func (s *walletServer) SetVspdVoteChoices(ctx context.Context, req *pb.SetVspdVo if vspHost == "" { return nil, status.Errorf(codes.InvalidArgument, "vsp host can not be null") } - policy := &vsp.Policy{ + policy := &wallet.VSPPolicy{ MaxFee: 0.1e8, FeeAcct: req.FeeAccount, ChangeAcct: req.ChangeAccount, } - cfg := vsp.Config{ + cfg := wallet.VSPClientConfig{ URL: vspHost, PubKey: vspPubKey, Dialer: nil, - Wallet: s.wallet, Policy: policy, - Params: s.wallet.ChainParams(), } - vspClient, err := loader.VSP(cfg) + vspClient, err := s.wallet.VSP(cfg) if err != nil { return nil, status.Errorf(codes.Unknown, "VSPClient instance failed to start. Error: %v", err) } @@ -4320,7 +4304,7 @@ func (s *walletServer) SetVspdVoteChoices(ctx context.Context, req *pb.SetVspdVo return &pb.SetVspdVoteChoicesResponse{}, nil } -func marshalVSPTrackedTickets(tickets []*vsp.TicketInfo) []*pb.GetTrackedVSPTicketsResponse_Ticket { +func marshalVSPTrackedTickets(tickets []*wallet.VSPTicketInfo) []*pb.GetTrackedVSPTicketsResponse_Ticket { res := make([]*pb.GetTrackedVSPTicketsResponse_Ticket, len(tickets)) for i, ticket := range tickets { res[i] = &pb.GetTrackedVSPTicketsResponse_Ticket{ @@ -4336,7 +4320,7 @@ func marshalVSPTrackedTickets(tickets []*vsp.TicketInfo) []*pb.GetTrackedVSPTick } func (s *walletServer) GetTrackedVSPTickets(ctx context.Context, req *pb.GetTrackedVSPTicketsRequest) (*pb.GetTrackedVSPTicketsResponse, error) { - vspClients := loader.AllVSPs() + vspClients := s.wallet.AllVSPs() res := &pb.GetTrackedVSPTicketsResponse{ Vsps: make([]*pb.GetTrackedVSPTicketsResponse_VSP, 0, len(vspClients)), } diff --git a/ticketbuyer/tb.go b/ticketbuyer/tb.go index 37629768c..b74321fe5 100644 --- a/ticketbuyer/tb.go +++ b/ticketbuyer/tb.go @@ -10,7 +10,6 @@ import ( "sync" "decred.org/dcrwallet/v5/errors" - "decred.org/dcrwallet/v5/vsp" "decred.org/dcrwallet/v5/wallet" "github.com/decred/dcrd/dcrutil/v4" "github.com/decred/dcrd/wire" @@ -43,7 +42,7 @@ type Config struct { MixChange bool // VSP client - VSP *vsp.Client + VSP *wallet.VSPClient } // TB is an automated ticket buyer, buying as many tickets as possible given an @@ -293,12 +292,10 @@ func (tb *TB) buy(ctx context.Context, passphrase []byte, tip *wire.BlockHeader, MixedAccountBranch: mixedBranch, MixedSplitAccount: splitAccount, ChangeAccount: changeAccount, + + VSPClient: tb.cfg.VSP, } - // If VSP is configured, we need to set the methods for vsp fee processment. - if tb.cfg.VSP != nil { - purchaseTicketReq.VSPFeePaymentProcess = tb.cfg.VSP.Process - purchaseTicketReq.VSPFeePercent = tb.cfg.VSP.FeePercentage - } + tix, err := w.PurchaseTickets(ctx, n, purchaseTicketReq) if tix != nil { for _, hash := range tix.TicketHashes { diff --git a/wallet/createtx.go b/wallet/createtx.go index e44c01758..8c80ddd9e 100644 --- a/wallet/createtx.go +++ b/wallet/createtx.go @@ -1261,12 +1261,8 @@ func (w *Wallet) purchaseTickets(ctx context.Context, op errors.Op, } return } - if req.VSPFeePaymentProcess != nil { - if req.VSPFeePercent == nil { - return nil, errors.E(op, errors.Bug, "VSPFeeProcess "+ - "may not be nil if VSPServerProcess is non-nil") - } - feePrice, err := req.VSPFeePercent(ctx) + if req.VSPClient != nil { + feePrice, err := req.VSPClient.FeePercentage(ctx) if err != nil { return nil, err } @@ -1639,7 +1635,7 @@ func (w *Wallet) purchaseTickets(ctx context.Context, op errors.Op, log.Infof("Published ticket purchase %v", ticket.TxHash()) // Pay VSP fee when configured to do so. - if req.VSPFeePaymentProcess == nil { + if req.VSPClient == nil { continue } unlockCredits = false @@ -1664,7 +1660,7 @@ func (w *Wallet) purchaseTickets(ctx context.Context, op errors.Op, continue } - err = req.VSPFeePaymentProcess(ctx, ticket, feeTx) + err = req.VSPClient.Process(ctx, ticket, feeTx) if err != nil { unlock() continue diff --git a/vsp/feepayment.go b/wallet/feepayment.go similarity index 93% rename from vsp/feepayment.go rename to wallet/feepayment.go index 7d70e29af..1ca94958a 100644 --- a/vsp/feepayment.go +++ b/wallet/feepayment.go @@ -2,7 +2,7 @@ // Use of this source code is governed by an ISC // license that can be found in the LICENSE file. -package vsp +package wallet import ( "bytes" @@ -13,7 +13,6 @@ import ( "sync" "time" - "decred.org/dcrwallet/v5/wallet" "github.com/decred/dcrd/chaincfg/chainhash" "github.com/decred/dcrd/chaincfg/v3" "github.com/decred/dcrd/crypto/rand" @@ -36,13 +35,13 @@ const ( unminedJitter = 2 * time.Minute ) -type feePayment struct { - client *Client +type vspFeePayment struct { + client *VSPClient ctx context.Context // Set at feePayment creation and never changes - ticket *wallet.VSPTicket - policy *Policy + ticket *VSPTicket + policy *VSPPolicy // Requires locking for all access outside of Client.feePayment mu sync.Mutex @@ -69,7 +68,7 @@ const ( TicketSpent ) -func (fp *feePayment) removedExpiredOrSpent() bool { +func (fp *vspFeePayment) removedExpiredOrSpent() bool { var reason string switch { case fp.ticket.Expired(fp.ctx): @@ -85,7 +84,7 @@ func (fp *feePayment) removedExpiredOrSpent() bool { return false } -func (fp *feePayment) remove(reason string) { +func (fp *vspFeePayment) remove(reason string) { fp.stop() fp.client.log.Infof("Ticket %v is %s; removing from VSP client", fp.ticket, reason) fp.client.mu.Lock() @@ -95,7 +94,7 @@ func (fp *feePayment) remove(reason string) { // feePayment returns an existing managed fee payment, or creates and begins // processing a fee payment for a ticket. -func (c *Client) feePayment(ctx context.Context, ticket *wallet.VSPTicket, paidConfirmed bool) (fp *feePayment) { +func (c *VSPClient) feePayment(ctx context.Context, ticket *VSPTicket, paidConfirmed bool) (fp *vspFeePayment) { ticketHash := ticket.Hash() c.mu.Lock() fp = c.jobs[*ticketHash] @@ -124,12 +123,12 @@ func (c *Client) feePayment(ctx context.Context, ticket *wallet.VSPTicket, paidC } }() - fp = &feePayment{ + fp = &vspFeePayment{ client: c, ctx: context.Background(), ticket: ticket, policy: c.policy, - params: c.params, + params: c.wallet.chainParams, } // No VSP interaction is required for spent tickets. @@ -176,7 +175,7 @@ func (c *Client) feePayment(ctx context.Context, ticket *wallet.VSPTicket, paidC // Schedule a method to be executed. // Any currently-scheduled method is replaced. -func (fp *feePayment) schedule(name string, method func() error) { +func (fp *vspFeePayment) schedule(name string, method func() error) { var delay time.Duration if method != nil { delay = fp.next() @@ -194,7 +193,7 @@ func (fp *feePayment) schedule(name string, method func() error) { } } -func (fp *feePayment) next() time.Duration { +func (fp *vspFeePayment) next() time.Duration { w := fp.client.wallet _, tipHeight := w.MainChainTip(fp.ctx) @@ -221,7 +220,7 @@ func (fp *feePayment) next() time.Duration { // task returns a function running a feePayment method. // If the method errors, the error is logged, and the payment is put // in an errored state and may require manual processing. -func (fp *feePayment) task(name string, method func() error) func() { +func (fp *vspFeePayment) task(name string, method func() error) func() { return func() { err := method() fp.mu.Lock() @@ -233,11 +232,11 @@ func (fp *feePayment) task(name string, method func() error) func() { } } -func (fp *feePayment) stop() { +func (fp *vspFeePayment) stop() { fp.schedule("", nil) } -func (fp *feePayment) receiveFeeAddress() error { +func (fp *vspFeePayment) receiveFeeAddress() error { ctx := fp.ctx // stop processing if ticket is expired or spent @@ -297,7 +296,7 @@ func (fp *feePayment) receiveFeeAddress() error { // // If tx is nil, fp.feeTx may be assigned or modified, but the pointer will not // be dereferenced. -func (fp *feePayment) makeFeeTx(tx *wire.MsgTx) error { +func (fp *vspFeePayment) makeFeeTx(tx *wire.MsgTx) error { ctx := fp.ctx w := fp.client.wallet @@ -360,7 +359,7 @@ func (fp *feePayment) makeFeeTx(tx *wire.MsgTx) error { return nil } -func (c *Client) status(ctx context.Context, ticket *wallet.VSPTicket) (*types.TicketStatusResponse, error) { +func (c *VSPClient) status(ctx context.Context, ticket *VSPTicket) (*types.TicketStatusResponse, error) { req := types.TicketStatusRequest{ TicketHash: ticket.Hash().String(), @@ -376,7 +375,7 @@ func (c *Client) status(ctx context.Context, ticket *wallet.VSPTicket) (*types.T return resp, nil } -func (c *Client) setVoteChoices(ctx context.Context, ticket *wallet.VSPTicket, +func (c *VSPClient) setVoteChoices(ctx context.Context, ticket *VSPTicket, choices map[string]string, tspendPolicy map[string]string, treasuryPolicy map[string]string) error { req := types.SetVoteChoicesRequest{ @@ -397,7 +396,7 @@ func (c *Client) setVoteChoices(ctx context.Context, ticket *wallet.VSPTicket, return nil } -func (fp *feePayment) reconcilePayment() error { +func (fp *vspFeePayment) reconcilePayment() error { ctx := fp.ctx w := fp.client.wallet @@ -490,7 +489,7 @@ func (fp *feePayment) reconcilePayment() error { */ } -func (fp *feePayment) submitPayment() (err error) { +func (fp *vspFeePayment) submitPayment() (err error) { ctx := fp.ctx w := fp.client.wallet @@ -563,7 +562,7 @@ func (fp *feePayment) submitPayment() (err error) { // reached sufficient confirmations, and reschedule itself if the fee is not // confirmed yet. If the fee tx is ever removed from the wallet, this will // schedule another reconcile. -func (fp *feePayment) confirmPayment() (err error) { +func (fp *vspFeePayment) confirmPayment() (err error) { ctx := fp.ctx // stop processing if ticket is expired or spent diff --git a/vsp/vsp.go b/wallet/vsp.go similarity index 85% rename from vsp/vsp.go rename to wallet/vsp.go index 800920316..07506c1de 100644 --- a/vsp/vsp.go +++ b/wallet/vsp.go @@ -2,7 +2,7 @@ // Use of this source code is governed by an ISC // license that can be found in the LICENSE file. -package vsp +package wallet import ( "context" @@ -14,10 +14,8 @@ import ( "sync" "decred.org/dcrwallet/v5/errors" - "decred.org/dcrwallet/v5/wallet" "decred.org/dcrwallet/v5/wallet/udb" "github.com/decred/dcrd/chaincfg/chainhash" - "github.com/decred/dcrd/chaincfg/v3" "github.com/decred/dcrd/dcrutil/v4" "github.com/decred/dcrd/txscript/v4/stdaddr" "github.com/decred/dcrd/wire" @@ -27,25 +25,24 @@ import ( type DialFunc func(ctx context.Context, network, addr string) (net.Conn, error) -type Policy struct { +type VSPPolicy struct { MaxFee dcrutil.Amount ChangeAcct uint32 // to derive fee addresses FeeAcct uint32 // to pay fees from, if inputs are not provided to Process } -type Client struct { - wallet *wallet.Wallet - policy *Policy +type VSPClient struct { + wallet *Wallet + policy *VSPPolicy *vspd.Client mu sync.Mutex - jobs map[chainhash.Hash]*feePayment + jobs map[chainhash.Hash]*vspFeePayment - log slog.Logger - params *chaincfg.Params + log slog.Logger } -type Config struct { +type VSPClientConfig struct { // URL specifies the base URL of the VSP URL string @@ -55,17 +52,12 @@ type Config struct { // Dialer specifies an optional dialer when connecting to the VSP. Dialer DialFunc - // Wallet specifies a loaded wallet. - Wallet *wallet.Wallet - // Default policy for fee payments unless another is provided by the // caller. - Policy *Policy - - Params *chaincfg.Params + Policy *VSPPolicy } -func New(cfg Config, log slog.Logger) (*Client, error) { +func (w *Wallet) NewVSPClient(cfg VSPClientConfig, log slog.Logger) (*VSPClient, error) { u, err := url.Parse(cfg.URL) if err != nil { return nil, err @@ -74,36 +66,28 @@ func New(cfg Config, log slog.Logger) (*Client, error) { if err != nil { return nil, err } - if cfg.Wallet == nil { - return nil, fmt.Errorf("wallet option not set") - } - - if cfg.Params == nil { - return nil, fmt.Errorf("params option not set") - } client := &vspd.Client{ URL: u.String(), PubKey: pubKey, - Sign: cfg.Wallet.SignMessage, + Sign: w.SignMessage, Log: log, } client.Transport = &http.Transport{ DialContext: cfg.Dialer, } - v := &Client{ - wallet: cfg.Wallet, + v := &VSPClient{ + wallet: w, policy: cfg.Policy, Client: client, - jobs: make(map[chainhash.Hash]*feePayment), + jobs: make(map[chainhash.Hash]*vspFeePayment), log: log, - params: cfg.Params, } return v, nil } -func (c *Client) FeePercentage(ctx context.Context) (float64, error) { +func (c *VSPClient) FeePercentage(ctx context.Context) (float64, error) { resp, err := c.Client.VspInfo(ctx) if err != nil { return -1, err @@ -113,7 +97,7 @@ func (c *Client) FeePercentage(ctx context.Context) (float64, error) { // ProcessUnprocessedTickets adds the provided tickets to the client. Noop if // a given ticket is already added. -func (c *Client) ProcessUnprocessedTickets(ctx context.Context, tickets []*wallet.VSPTicket) { +func (c *VSPClient) ProcessUnprocessedTickets(ctx context.Context, tickets []*VSPTicket) { var wg sync.WaitGroup for _, ticket := range tickets { @@ -127,7 +111,7 @@ func (c *Client) ProcessUnprocessedTickets(ctx context.Context, tickets []*walle // Start processing in the background. wg.Add(1) - go func(t *wallet.VSPTicket) { + go func(t *VSPTicket) { defer wg.Done() err := c.Process(ctx, t, nil) if err != nil { @@ -143,7 +127,7 @@ func (c *Client) ProcessUnprocessedTickets(ctx context.Context, tickets []*walle // their fee payment process. Noop if a given ticket is already added, or if the // ticket is not registered with the VSP. This is used to recover VSP tracking // after seed restores. -func (c *Client) ProcessManagedTickets(ctx context.Context, tickets []*wallet.VSPTicket) error { +func (c *VSPClient) ProcessManagedTickets(ctx context.Context, tickets []*VSPTicket) error { for _, ticket := range tickets { hash := ticket.Hash() c.mu.Lock() @@ -203,7 +187,7 @@ func (c *Client) ProcessManagedTickets(ctx context.Context, tickets []*wallet.VS // the inputs and the fee and change outputs before returning without an error. // The fee transaction is also recorded as unpublised in the wallet, and the fee // hash is associated with the ticket. -func (c *Client) Process(ctx context.Context, ticket *wallet.VSPTicket, feeTx *wire.MsgTx) error { +func (c *VSPClient) Process(ctx context.Context, ticket *VSPTicket, feeTx *wire.MsgTx) error { vspTicket, err := ticket.VSPTicketInfo(ctx) if err != nil && !errors.Is(err, errors.NotExist) { return err @@ -275,7 +259,7 @@ func (c *Client) Process(ctx context.Context, ticket *wallet.VSPTicket, feeTx *w // preferences, and checks if they match the status of the specified ticket from // the connected VSP. The status provides the current voting preferences so we // can just update from there if need be. -func (c *Client) SetVoteChoice(ctx context.Context, ticket *wallet.VSPTicket, +func (c *VSPClient) SetVoteChoice(ctx context.Context, ticket *VSPTicket, choices map[string]string, tspendPolicy map[string]string, treasuryPolicy map[string]string) error { // Retrieve current voting preferences from VSP. @@ -344,8 +328,8 @@ func (c *Client) SetVoteChoice(ctx context.Context, ticket *wallet.VSPTicket, return nil } -// TicketInfo stores per-ticket info tracked by a VSP Client instance. -type TicketInfo struct { +// VSPTicketInfo stores per-ticket info tracked by a VSP Client instance. +type VSPTicketInfo struct { TicketHash chainhash.Hash CommitmentAddr stdaddr.StakeAddress VotingAddr stdaddr.StakeAddress @@ -361,19 +345,19 @@ type TicketInfo struct { // // Currently this returns only info about tickets which fee hasn't been paid or // confirmed at enough depth to be considered committed to. -func (c *Client) TrackedTickets() []*TicketInfo { +func (c *VSPClient) TrackedTickets() []*VSPTicketInfo { // Collect all jobs first, to avoid working under two different locks. c.mu.Lock() - jobs := make([]*feePayment, 0, len(c.jobs)) + jobs := make([]*vspFeePayment, 0, len(c.jobs)) for _, job := range c.jobs { jobs = append(jobs, job) } c.mu.Unlock() - tickets := make([]*TicketInfo, 0, len(jobs)) + tickets := make([]*VSPTicketInfo, 0, len(jobs)) for _, job := range jobs { job.mu.Lock() - tickets = append(tickets, &TicketInfo{ + tickets = append(tickets, &VSPTicketInfo{ TicketHash: *job.ticket.Hash(), CommitmentAddr: job.ticket.CommitmentAddr(), VotingAddr: job.ticket.VotingAddr(), diff --git a/internal/loader/vsp.go b/wallet/vspclientloader.go similarity index 52% rename from internal/loader/vsp.go rename to wallet/vspclientloader.go index 7dd6adb39..a0f667c64 100644 --- a/internal/loader/vsp.go +++ b/wallet/vspclientloader.go @@ -1,45 +1,35 @@ -package loader +package wallet import ( - "sync" - "decred.org/dcrwallet/v5/errors" "decred.org/dcrwallet/v5/internal/loggers" - "decred.org/dcrwallet/v5/vsp" ) -var vspClients = struct { - mu sync.Mutex - clients map[string]*vsp.Client -}{ - clients: make(map[string]*vsp.Client), -} - // VSP loads or creates a package-global instance of the VSP client for a host. // This allows clients to be created and reused across various subsystems. -func VSP(cfg vsp.Config) (*vsp.Client, error) { +func (w *Wallet) VSP(cfg VSPClientConfig) (*VSPClient, error) { key := cfg.URL - vspClients.mu.Lock() - defer vspClients.mu.Unlock() - client, ok := vspClients.clients[key] + w.vspClientsMu.Lock() + defer w.vspClientsMu.Unlock() + client, ok := w.vspClients[key] if ok { return client, nil } - client, err := vsp.New(cfg, loggers.VspcLog) + client, err := w.NewVSPClient(cfg, loggers.VspcLog) if err != nil { return nil, err } - vspClients.clients[key] = client + w.vspClients[key] = client return client, nil } // LookupVSP returns a previously-configured VSP client, if one has been created // and registered with the VSP function. Otherwise, a NotExist error is // returned. -func LookupVSP(host string) (*vsp.Client, error) { - vspClients.mu.Lock() - defer vspClients.mu.Unlock() - client, ok := vspClients.clients[host] +func (w *Wallet) LookupVSP(host string) (*VSPClient, error) { + w.vspClientsMu.Lock() + defer w.vspClientsMu.Unlock() + client, ok := w.vspClients[host] if !ok { err := errors.Errorf("VSP client for %q not found", host) return nil, errors.E(errors.NotExist, err) @@ -48,12 +38,12 @@ func LookupVSP(host string) (*vsp.Client, error) { } // AllVSPs returns the list of all currently registered VSPs. -func AllVSPs() map[string]*vsp.Client { +func (w *Wallet) AllVSPs() map[string]*VSPClient { // Create a copy to avoid callers mutating the list. - vspClients.mu.Lock() - defer vspClients.mu.Unlock() - res := make(map[string]*vsp.Client, len(vspClients.clients)) - for host, client := range vspClients.clients { + w.vspClientsMu.Lock() + defer w.vspClientsMu.Unlock() + res := make(map[string]*VSPClient, len(w.vspClients)) + for host, client := range w.vspClients { res[host] = client } return res diff --git a/wallet/wallet.go b/wallet/wallet.go index 9a2200e2d..8d44fe45b 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -170,6 +170,9 @@ type Wallet struct { deploymentsByID map[string]*chaincfg.ConsensusDeployment minTestNetTarget *big.Int minTestNetDiffBits uint32 + + vspClientsMu sync.Mutex + vspClients map[string]*VSPClient } // Config represents the configuration options needed to initialize a wallet. @@ -1556,15 +1559,7 @@ type PurchaseTicketsRequest struct { MixedSplitAccount uint32 ChangeAccount uint32 - // VSPServer methods - // XXX this should be an interface - - // VSPFeePercent returns the VSPs fee as a percentage of the vote reward. - VSPFeePercent func(context.Context) (float64, error) - // VSPFeePaymentProcess checks the fee payment status for the specified - // ticket and, if necessary, starts a long-lived handler to process the fee - // payment. - VSPFeePaymentProcess func(context.Context, *VSPTicket, *wire.MsgTx) error + VSPClient *VSPClient // extraSplitOutput is an additional transaction output created during // UTXO contention, to be used as the input to pay a VSP fee @@ -1605,7 +1600,7 @@ func (w *Wallet) PurchaseTickets(ctx context.Context, n NetworkBackend, return nil, errors.E(op, errors.InsufficientBalance) } - feePercent, err := req.VSPFeePercent(ctx) + feePercent, err := req.VSPClient.FeePercentage(ctx) if err != nil { return nil, err } @@ -5409,6 +5404,8 @@ func Open(ctx context.Context, cfg *Config) (*Wallet, error) { mixSems: newMixSemaphores(cfg.MixSplitLimit), mixing: !cfg.DisableMixing, + + vspClients: make(map[string]*VSPClient), } // Open database managers From 53cd8211b1f27915c2200f186e4152c5dbad96b4 Mon Sep 17 00:00:00 2001 From: jholdstock Date: Wed, 18 Sep 2024 16:00:34 +0100 Subject: [PATCH 2/2] Make VSP clients respect tor config. Ensure dialer is set correctly so VSPs are reached over tor if requested in config. This also has a nice side-effect of simplifying VSP client creation. --- dcrwallet.go | 3 +-- internal/loader/loader.go | 7 ++++++- internal/rpc/jsonrpc/methods.go | 1 - internal/rpc/rpcserver/server.go | 8 -------- wallet/vsp.go | 7 ++----- wallet/vspclientloader.go | 2 +- wallet/wallet.go | 6 ++++++ walletsetup.go | 2 +- 8 files changed, 17 insertions(+), 19 deletions(-) diff --git a/dcrwallet.go b/dcrwallet.go index af39ced09..7013e9462 100644 --- a/dcrwallet.go +++ b/dcrwallet.go @@ -169,7 +169,7 @@ func run(ctx context.Context) error { loader := ldr.NewLoader(activeNet.Params, dbDir, cfg.EnableVoting, cfg.GapLimit, cfg.WatchLast, cfg.AllowHighFees, cfg.RelayFee.Amount, cfg.AccountGapLimit, cfg.DisableCoinTypeUpgrades, !cfg.Mixing, - cfg.ManualTickets, cfg.MixSplitLimit) + cfg.ManualTickets, cfg.MixSplitLimit, cfg.dial) // Stop any services started by the loader after the shutdown procedure is // initialized and this function returns. @@ -273,7 +273,6 @@ func run(ctx context.Context) error { vspCfg := wallet.VSPClientConfig{ URL: cfg.VSPOpts.URL, PubKey: cfg.VSPOpts.PubKey, - Dialer: cfg.dial, Policy: &wallet.VSPPolicy{ MaxFee: cfg.VSPOpts.MaxFee.Amount, FeeAcct: purchaseAcct, diff --git a/internal/loader/loader.go b/internal/loader/loader.go index 8ddac7839..24c5a78a4 100644 --- a/internal/loader/loader.go +++ b/internal/loader/loader.go @@ -47,6 +47,7 @@ type Loader struct { manualTickets bool relayFee dcrutil.Amount mixSplitLimit int + dialer wallet.DialFunc mu sync.Mutex } @@ -54,7 +55,7 @@ type Loader struct { // NewLoader constructs a Loader. func NewLoader(chainParams *chaincfg.Params, dbDirPath string, votingEnabled bool, gapLimit uint32, watchLast uint32, allowHighFees bool, relayFee dcrutil.Amount, accountGapLimit int, - disableCoinTypeUpgrades bool, disableMixing bool, manualTickets bool, mixSplitLimit int) *Loader { + disableCoinTypeUpgrades bool, disableMixing bool, manualTickets bool, mixSplitLimit int, dialer wallet.DialFunc) *Loader { return &Loader{ chainParams: chainParams, @@ -69,6 +70,7 @@ func NewLoader(chainParams *chaincfg.Params, dbDirPath string, votingEnabled boo manualTickets: manualTickets, relayFee: relayFee, mixSplitLimit: mixSplitLimit, + dialer: dialer, } } @@ -176,6 +178,7 @@ func (l *Loader) CreateWatchingOnlyWallet(ctx context.Context, extendedPubKey st RelayFee: l.relayFee, MixSplitLimit: l.mixSplitLimit, Params: l.chainParams, + Dialer: l.dialer, } w, err = wallet.Open(ctx, cfg) if err != nil { @@ -262,6 +265,7 @@ func (l *Loader) CreateNewWallet(ctx context.Context, pubPassphrase, privPassphr AllowHighFees: l.allowHighFees, RelayFee: l.relayFee, Params: l.chainParams, + Dialer: l.dialer, } w, err = wallet.Open(ctx, cfg) if err != nil { @@ -319,6 +323,7 @@ func (l *Loader) OpenExistingWallet(ctx context.Context, pubPassphrase []byte) ( RelayFee: l.relayFee, MixSplitLimit: l.mixSplitLimit, Params: l.chainParams, + Dialer: l.dialer, } w, err = wallet.Open(ctx, cfg) if err != nil { diff --git a/internal/rpc/jsonrpc/methods.go b/internal/rpc/jsonrpc/methods.go index 03a117c8d..53b17f5c8 100644 --- a/internal/rpc/jsonrpc/methods.go +++ b/internal/rpc/jsonrpc/methods.go @@ -3347,7 +3347,6 @@ func (s *Server) purchaseTicket(ctx context.Context, icmd any) (any, error) { cfg := wallet.VSPClientConfig{ URL: s.cfg.VSPHost, PubKey: s.cfg.VSPPubKey, - Dialer: s.cfg.Dial, Policy: &wallet.VSPPolicy{ MaxFee: s.cfg.VSPMaxFee, FeeAcct: account, diff --git a/internal/rpc/rpcserver/server.go b/internal/rpc/rpcserver/server.go index dd9caf1e7..d66eb4fe6 100644 --- a/internal/rpc/rpcserver/server.go +++ b/internal/rpc/rpcserver/server.go @@ -321,8 +321,6 @@ func (*versionServer) Version(ctx context.Context, req *pb.VersionRequest) (*pb. }, nil } -type dialFunc func(ctx context.Context, network, addr string) (net.Conn, error) - // StartWalletService starts the WalletService. func StartWalletService(server *grpc.Server, wallet *wallet.Wallet) { if walletService.ready.Swap(1) != 0 { @@ -1817,7 +1815,6 @@ func (s *walletServer) PurchaseTickets(ctx context.Context, cfg := wallet.VSPClientConfig{ URL: vspHost, PubKey: vspPubKey, - Dialer: nil, Policy: &wallet.VSPPolicy{ MaxFee: 0.1e8, FeeAcct: req.Account, @@ -2626,7 +2623,6 @@ func (t *ticketbuyerServer) RunTicketBuyer(req *pb.RunTicketBuyerRequest, svr pb cfg := wallet.VSPClientConfig{ URL: vspHost, PubKey: vspPubKey, - Dialer: nil, Policy: &wallet.VSPPolicy{ MaxFee: 0.1e8, FeeAcct: req.Account, @@ -4112,7 +4108,6 @@ func (s *walletServer) SyncVSPFailedTickets(ctx context.Context, req *pb.SyncVSP cfg := wallet.VSPClientConfig{ URL: vspHost, PubKey: vspPubKey, - Dialer: nil, Policy: policy, } vspClient, err := s.wallet.VSP(cfg) @@ -4159,7 +4154,6 @@ func (s *walletServer) ProcessManagedTickets(ctx context.Context, req *pb.Proces cfg := wallet.VSPClientConfig{ URL: vspHost, PubKey: vspPubKey, - Dialer: nil, Policy: policy, } vspClient, err := s.wallet.VSP(cfg) @@ -4199,7 +4193,6 @@ func (s *walletServer) ProcessUnmanagedTickets(ctx context.Context, req *pb.Proc cfg := wallet.VSPClientConfig{ URL: vspHost, PubKey: vspPubKey, - Dialer: nil, Policy: policy, } vspClient, err := s.wallet.VSP(cfg) @@ -4236,7 +4229,6 @@ func (s *walletServer) SetVspdVoteChoices(ctx context.Context, req *pb.SetVspdVo cfg := wallet.VSPClientConfig{ URL: vspHost, PubKey: vspPubKey, - Dialer: nil, Policy: policy, } vspClient, err := s.wallet.VSP(cfg) diff --git a/wallet/vsp.go b/wallet/vsp.go index 07506c1de..1cc1b0b6f 100644 --- a/wallet/vsp.go +++ b/wallet/vsp.go @@ -49,15 +49,12 @@ type VSPClientConfig struct { // PubKey specifies the VSP's base64 encoded public key PubKey string - // Dialer specifies an optional dialer when connecting to the VSP. - Dialer DialFunc - // Default policy for fee payments unless another is provided by the // caller. Policy *VSPPolicy } -func (w *Wallet) NewVSPClient(cfg VSPClientConfig, log slog.Logger) (*VSPClient, error) { +func (w *Wallet) NewVSPClient(cfg VSPClientConfig, log slog.Logger, dialer DialFunc) (*VSPClient, error) { u, err := url.Parse(cfg.URL) if err != nil { return nil, err @@ -74,7 +71,7 @@ func (w *Wallet) NewVSPClient(cfg VSPClientConfig, log slog.Logger) (*VSPClient, Log: log, } client.Transport = &http.Transport{ - DialContext: cfg.Dialer, + DialContext: dialer, } v := &VSPClient{ diff --git a/wallet/vspclientloader.go b/wallet/vspclientloader.go index a0f667c64..d8dd5b62b 100644 --- a/wallet/vspclientloader.go +++ b/wallet/vspclientloader.go @@ -15,7 +15,7 @@ func (w *Wallet) VSP(cfg VSPClientConfig) (*VSPClient, error) { if ok { return client, nil } - client, err := w.NewVSPClient(cfg, loggers.VspcLog) + client, err := w.NewVSPClient(cfg, loggers.VspcLog, w.dialer) if err != nil { return nil, err } diff --git a/wallet/wallet.go b/wallet/wallet.go index 8d44fe45b..581148d61 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -173,6 +173,8 @@ type Wallet struct { vspClientsMu sync.Mutex vspClients map[string]*VSPClient + + dialer DialFunc } // Config represents the configuration options needed to initialize a wallet. @@ -194,6 +196,8 @@ type Config struct { AllowHighFees bool RelayFee dcrutil.Amount Params *chaincfg.Params + + Dialer DialFunc } // DisapprovePercent returns the wallet's block disapproval percentage. @@ -5406,6 +5410,8 @@ func Open(ctx context.Context, cfg *Config) (*Wallet, error) { mixing: !cfg.DisableMixing, vspClients: make(map[string]*VSPClient), + + dialer: cfg.Dialer, } // Open database managers diff --git a/walletsetup.go b/walletsetup.go index 8ef21723f..a42474612 100644 --- a/walletsetup.go +++ b/walletsetup.go @@ -115,7 +115,7 @@ func createWallet(ctx context.Context, cfg *config) error { loader := loader.NewLoader(activeNet.Params, dbDir, cfg.EnableVoting, cfg.GapLimit, cfg.WatchLast, cfg.AllowHighFees, cfg.RelayFee.Amount, cfg.AccountGapLimit, cfg.DisableCoinTypeUpgrades, !cfg.Mixing, - cfg.ManualTickets, cfg.MixSplitLimit) + cfg.ManualTickets, cfg.MixSplitLimit, cfg.dial) var privPass, pubPass, seed []byte var imported bool