Skip to content

Commit

Permalink
Read request body only once (#44)
Browse files Browse the repository at this point in the history
* Add KeepReqAlive option used to defer body closure

* resolve the linting issues

* Remove KeepReqAlive and read body only once

* Resubmit the vsp fee tx if previous tx failed, on clicking ticket details page
  • Loading branch information
dmigwi authored Aug 16, 2023
1 parent ac178b7 commit cd8d96a
Show file tree
Hide file tree
Showing 8 changed files with 90 additions and 77 deletions.
54 changes: 22 additions & 32 deletions libwallet/assets/dcr/ticket.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,20 +123,13 @@ func (asset *Asset) PurchaseTickets(account, numTickets int32, vspHost, passphra
}
defer asset.LockWallet()

// Use the user-specified instructions for processing fee payments
// for this ticket, rather than some default policy.
vspPolicy := vsp.Policy{
MaxFee: 0.2e8,
FeeAcct: uint32(account),
ChangeAcct: uint32(account),
}
request := &w.PurchaseTicketsRequest{
Count: int(numTickets),
SourceAccount: uint32(account),
MinConf: asset.RequiredConfirmations(),
VSPFeeProcess: vspClient.FeePercentage,
VSPFeePaymentProcess: func(ctx context.Context, ticketHash *chainhash.Hash, feeTx *wire.MsgTx) error {
return vspClient.Process(ctx, ticketHash, feeTx, vspPolicy)
return vspClient.Process(ctx, ticketHash, feeTx, asset.GetvspPolicy(account))
},
}

Expand All @@ -159,6 +152,17 @@ func (asset *Asset) PurchaseTickets(account, numTickets int32, vspHost, passphra
return ticketsResponse.TicketHashes, err
}

// GetvspPolicy creates the VSP policy using the account number provided.
// Uses the user-specified instructions for processing fee payments
// on a ticket, rather than some default policy.
func (asset *Asset) GetvspPolicy(account int32) vsp.Policy {
return vsp.Policy{
MaxFee: 0.2e8,
FeeAcct: uint32(account),
ChangeAcct: uint32(account),
}
}

// VSPTicketInfo returns vsp-related info for a given ticket. Returns an error
// if the ticket is not yet assigned to a VSP.
func (asset *Asset) VSPTicketInfo(hash string) (*VSPTicketInfo, error) {
Expand All @@ -175,6 +179,7 @@ func (asset *Asset) VSPTicketInfo(hash string) (*VSPTicketInfo, error) {
ctx, _ := asset.ShutdownContextWithCancel()
walletTicketInfo, err := asset.Internal().DCR.VSPTicketInfo(ctx, ticketHash)
if err != nil {
log.Warnf("unable to getWallet info using ticket: %s Error: %v", hash, err)
return nil, err
}

Expand All @@ -187,48 +192,33 @@ func (asset *Asset) VSPTicketInfo(hash string) (*VSPTicketInfo, error) {
// Cannot submit a TicketStatus api request to the VSP if
// the wallet is locked. Return just the wallet info.
if asset.IsLocked() {
log.Warnf("cannot submit a ticket status request when wallet is locked")
return ticketInfo, nil
}

vspClient, err := asset.VSPClient(walletTicketInfo.Host, walletTicketInfo.PubKey)
if err != nil {
log.Warnf("unable to get vsp ticket info for %s: %v", hash, err)
log.Warnf("unable to connect to host: %s Error: %v", walletTicketInfo.Host, err)
return ticketInfo, nil
}

ticketInfo.Client = vspClient

vspTicketStatus, err := vspClient.GetTicketStatus(ctx, ticketHash)
if err != nil {
log.Warnf("unable to get vsp ticket info for %s: %v", hash, err)
log.Warnf("unable to get vsp ticket: %s Error: %v", hash, err)
return ticketInfo, nil
}

// Parse the fee status returned by the vsp.
var vspFeeStatus VSPFeeStatus
switch vspTicketStatus.FeeTxStatus {
case "received": // received but not broadcast
vspFeeStatus = VSPFeeProcessStarted
case "broadcast": // broadcast but not confirmed
vspFeeStatus = VSPFeeProcessPaid
case "confirmed": // broadcast and confirmed
vspFeeStatus = VSPFeeProcessConfirmed
case "error":
vspFeeStatus = VSPFeeProcessErrored
default:
vspFeeStatus = VSPFeeProcessErrored
log.Warnf("VSP responded with %v for %v", vspTicketStatus.FeeTxStatus, ticketHash)
}

// Sanity check and log any observed discrepancies.
if ticketInfo.FeeTxHash != vspTicketStatus.FeeTxHash {
log.Warnf("wallet fee tx hash %s differs from vsp fee tx hash %s for ticket %s",
ticketInfo.FeeTxHash, vspTicketStatus.FeeTxHash, ticketHash)
ticketInfo.FeeTxHash = vspTicketStatus.FeeTxHash
}
if ticketInfo.FeeTxStatus != vspFeeStatus {
log.Warnf("wallet fee status %q differs from vsp fee status %q for ticket %s",
ticketInfo.FeeTxStatus, vspFeeStatus, ticketHash)
ticketInfo.FeeTxStatus = vspFeeStatus
}

ticketInfo.FeeTxHash = vspTicketStatus.FeeTxHash
ticketInfo.ConfirmedByVSP = vspTicketStatus.TicketConfirmed

return ticketInfo, nil
}

Expand Down
4 changes: 3 additions & 1 deletion libwallet/assets/dcr/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,12 @@ type VSPTicketInfo struct {
VSP string
FeeTxHash string
FeeTxStatus VSPFeeStatus
// Client defines the vsp client needed to process more tickets requests.
Client *vsp.Client
// ConfirmedByVSP is nil if the ticket status could not be obtained
// from the VSP, false if the VSP hasn't confirmed the fee and true
// if the VSP has fully registered the ticket.
ConfirmedByVSP *bool
ConfirmedByVSP bool
// VoteChoices is only set if the ticket status was obtained from the
// VSP.
VoteChoices map[string]string
Expand Down
2 changes: 1 addition & 1 deletion libwallet/assets/dcr/vsp.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func (asset *Asset) VSPClient(host string, pubKey []byte) (*vsp.Client, error) {

cfg := vsp.Config{
URL: host,
PubKey: base64.StdEncoding.EncodeToString(pubKey),
PubKey: pubKey,
Dialer: nil, // optional, but consider providing a value
Wallet: asset.Internal().DCR,
}
Expand Down
41 changes: 16 additions & 25 deletions libwallet/internal/vsp/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"encoding/base64"
"encoding/json"
"fmt"
"io"
"net/http"

"github.com/crypto-power/cryptopower/libwallet/utils"
Expand Down Expand Up @@ -48,8 +47,10 @@ func (c *client) do(ctx context.Context, method, path string, addr stdaddr.Addre
var err error
var sig []byte
reqConf := &utils.ReqConfig{
Method: method,
HTTPURL: c.url + path,
Method: method,
HTTPURL: c.url + path,
IsRetByte: true,
Headers: make(http.Header),
}

if method == http.MethodPost {
Expand All @@ -62,13 +63,11 @@ func (c *client) do(ctx context.Context, method, path string, addr stdaddr.Addre

// Add cookies.
if sig != nil {
reqConf.Cookies = append(reqConf.Cookies, &http.Cookie{
Name: "VSP-Client-Signature",
Value: base64.StdEncoding.EncodeToString(sig),
})
reqConf.Headers.Add("VSP-Client-Signature", base64.StdEncoding.EncodeToString(sig))
}

reply, err := utils.HTTPRequest(reqConf, &response)
respBytes := []byte{}
reply, err := utils.HTTPRequest(reqConf, &respBytes)
if err != nil && reply == nil {
// Status code errors are handled below.
return err
Expand All @@ -80,35 +79,27 @@ func (c *client) do(ctx context.Context, method, path string, addr stdaddr.Addre
if !(is200 || is4xx) {
return err
}
sigBase64 := reply.Header.Get("VSP-Server-Signature")
if sigBase64 == "" {
return fmt.Errorf("cannot authenticate server: no signature")

if err = json.Unmarshal(respBytes, response); err != nil {
return fmt.Errorf("could not pack response data: %w", err)
}

sigBase64 := reply.Header.Get("VSP-Server-Signature")
sig, err = base64.StdEncoding.DecodeString(sigBase64)
if err != nil {
return fmt.Errorf("cannot authenticate server: %w", err)
}
respBody, err := io.ReadAll(reply.Body)
if err != nil {
return fmt.Errorf("read response body: %w", err)
}
if !ed25519.Verify(c.pub, respBody, sig) {

if !ed25519.Verify(c.pub, respBytes, sig) {
return fmt.Errorf("cannot authenticate server: invalid signature")
}

var apiError *BadRequestError
if is4xx {
apiError = new(BadRequestError)
response = apiError
}
if response != nil {
err = json.Unmarshal(respBody, response)
if err != nil {
return fmt.Errorf("unmarshal respose body: %w", err)
}
}
if apiError != nil {
apiError.HTTPStatus = status
return apiError
}

return nil
}
12 changes: 4 additions & 8 deletions libwallet/internal/vsp/vsp.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package vsp

import (
"context"
"encoding/base64"
"fmt"
"net"
"net/http"
Expand Down Expand Up @@ -38,8 +37,8 @@ type Config struct {
// URL specifies the base URL of the VSP
URL string

// PubKey specifies the VSP's base64 encoded public key
PubKey string
// PubKey specifies the VSP public key in bytes.
PubKey []byte

// Dialer specifies an optional dialer when connecting to the VSP.
Dialer DialFunc
Expand All @@ -53,15 +52,12 @@ func New(cfg Config) (*Client, error) {
if err != nil {
return nil, err
}
pubKey, err := base64.StdEncoding.DecodeString(cfg.PubKey)
if err != nil {
return nil, err
}

if cfg.Wallet == nil {
return nil, fmt.Errorf("wallet option not set")
}

client := newClient(u.String(), pubKey, cfg.Wallet)
client := newClient(u.String(), cfg.PubKey, cfg.Wallet)
client.Transport = &http.Transport{
DialContext: cfg.Dialer,
}
Expand Down
6 changes: 6 additions & 0 deletions libwallet/utils/httpclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ type (
ReqConfig struct {
Payload interface{}
Cookies []*http.Cookie
Headers http.Header
Method string
HTTPURL string
// If IsRetByte is set to true, client.Do will delegate
Expand Down Expand Up @@ -165,6 +166,9 @@ func (c *Client) query(reqConfig *ReqConfig) (rawData []byte, resp *http.Respons
req.AddCookie(cookie)
}

// assign the headers.
req.Header = reqConfig.Headers

// Send request
resp, err = c.HTTPClient.Do(req)
if err != nil {
Expand All @@ -186,6 +190,8 @@ func (c *Client) query(reqConfig *ReqConfig) (rawData []byte, resp *http.Respons

// HTTPRequest queries the API provided in the ReqConfig object and converts
// the returned json(Byte data) into an respObj interface.
// Returned http response body is usually empty because the http stream
// cannot be read twice.
func HTTPRequest(reqConfig *ReqConfig, respObj interface{}) (*http.Response, error) {
// validate the API Url address
urlPath, err := url.ParseRequestURI(reqConfig.HTTPURL)
Expand Down
37 changes: 29 additions & 8 deletions ui/page/staking/stake_overview.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package staking
import (
"context"
"fmt"
"sync/atomic"

"gioui.org/layout"
"gioui.org/text"
Expand All @@ -20,6 +21,7 @@ import (
"github.com/crypto-power/cryptopower/ui/page/settings"
tpage "github.com/crypto-power/cryptopower/ui/page/transaction"
"github.com/crypto-power/cryptopower/ui/values"
"github.com/decred/dcrd/chaincfg/chainhash"
"github.com/decred/dcrd/dcrutil/v4"
)

Expand Down Expand Up @@ -62,6 +64,7 @@ type Page struct {
showMaterialLoader bool

navToSettingsBtn cryptomaterial.Button
processingTicket uint32

dcrImpl *dcr.Asset
}
Expand Down Expand Up @@ -328,18 +331,36 @@ func (pg *Page) HandleUserInteractions() {
// but where it is necessary to display vsp-stored info, the
// wallet passphrase should be requested and used to unlock
// the wallet before calling this method.
// TODO: Use log.Errorf and log.Warnf instead of fmt.Printf.
ticketInfo, err := pg.dcrImpl.VSPTicketInfo(ticketTx.Hash)
if err != nil {
log.Errorf("VSPTicketInfo error: %v\n", err)
log.Errorf("VSPTicketInfo error: %v", err)
} else {
if ticketInfo.FeeTxStatus != dcr.VSPFeeProcessConfirmed {
log.Errorf("[WARN] Ticket %s has unconfirmed fee tx %s with status %q, vsp %s \n",
ticketTx.Hash, ticketInfo.FeeTxHash, ticketInfo.FeeTxStatus.String(), ticketInfo.VSP)
if ticketInfo.FeeTxStatus != dcr.VSPFeeProcessConfirmed || !ticketInfo.ConfirmedByVSP {
log.Warnf("Ticket %s has unconfirmed fee tx with status %q, vsp %s",
ticketTx.Hash, ticketInfo.FeeTxStatus.String(), ticketInfo.VSP)
}
if ticketInfo.ConfirmedByVSP == nil || !*ticketInfo.ConfirmedByVSP {
log.Errorf("[WARN] Ticket %s is not confirmed by VSP %s. Fee tx %s, status %q \n",
ticketTx.Hash, ticketInfo.VSP, ticketInfo.FeeTxHash, ticketInfo.FeeTxStatus.String())

// Confirm that fee hasn't been paid, sender account exists, the wallet
// is unlocked and no previous ticket processing instance is running.
if ticketInfo.FeeTxStatus != dcr.VSPFeeProcessPaid && len(ticketTx.Inputs) == 1 &&
ticketInfo.Client != nil && atomic.CompareAndSwapUint32(&pg.processingTicket, 0, 1) {

log.Infof("Attempting to process the unconfirmed VSP fee for tx: %v", ticketTx.Hash)

txHash, err := chainhash.NewHashFromStr(ticketTx.Hash)
if err != nil {
log.Errorf("convert hex to hash failed: %v", ticketTx.Hash)
return
}

account := ticketTx.Inputs[0].AccountNumber
err = ticketInfo.Client.ProcessTicket(pg.ctx, txHash, pg.dcrImpl.GetvspPolicy(account))
if err != nil {
log.Errorf("processing the unconfirmed tx fee failed: %v", err)
}

// Reset the processing
atomic.StoreUint32(&pg.processingTicket, 0)
}
}
}
Expand Down
11 changes: 9 additions & 2 deletions ui/page/transaction/transaction_details_page.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ type TxDetailsPage struct {
txDestinationAddress string
title string
vspHost string
vspHostFees string

moreOptionIsOpen bool
}
Expand Down Expand Up @@ -215,13 +216,19 @@ func (pg *TxDetailsPage) OnNavigatedTo() {
go func() {
info, err := dcrImp.VSPTicketInfo(pg.transaction.Hash)
if err != nil {
log.Errorf("VSPTicketInfo error: %v\n", err)
log.Errorf("VSPTicketInfo error: %v", err)
}

pg.vspHost = values.String(values.StrNotAvailable)
if info != nil {
pg.vspHost = info.VSP
}

pg.vspHostFees = values.String(values.StrNotAvailable)
feeTx, _ := pg.wallet.GetTransactionRaw(info.FeeTxHash)
if feeTx != nil {
pg.vspHostFees = pg.wallet.ToAmount(feeTx.Amount).String()
}
}()
}
}
Expand Down Expand Up @@ -726,7 +733,7 @@ func (pg *TxDetailsPage) txnTypeAndID(gtx C) D {
return pg.keyValue(gtx, values.String(values.StrVsp), pg.Theme.Label(values.TextSize14, pg.vspHost).Layout)
}),
layout.Rigid(func(gtx C) D {
return pg.keyValue(gtx, values.String(values.StrVspFee), pg.Theme.Label(values.TextSize14, values.String(values.StrNotAvailable)).Layout)
return pg.keyValue(gtx, values.String(values.StrVspFee), pg.Theme.Label(values.TextSize14, pg.vspHostFees).Layout)
}),
)
}),
Expand Down

0 comments on commit cd8d96a

Please sign in to comment.