From fede2272617850062939d7017822f3e94a80065b Mon Sep 17 00:00:00 2001 From: Chris Schinnerl Date: Wed, 13 Dec 2023 14:16:03 +0100 Subject: [PATCH 1/5] bus: discard full txnSet and not just last txn --- api/wallet.go | 6 ++++++ autopilot/autopilot.go | 2 +- bus/bus.go | 8 ++++---- bus/client/wallet.go | 11 +++++++---- wallet/wallet.go | 8 +++++--- worker/rhpv3.go | 2 +- worker/worker.go | 20 +++++++++++--------- 7 files changed, 35 insertions(+), 22 deletions(-) diff --git a/api/wallet.go b/api/wallet.go index 576a2da99..cbfc0c270 100644 --- a/api/wallet.go +++ b/api/wallet.go @@ -11,6 +11,12 @@ import ( ) type ( + // WalletDiscardRequest is the request type for the /wallet/discard + // endpoint. + WalletDiscardRequest struct { + TransactionSet []types.Transaction `json:"transactionSet"` + } + // WalletFundRequest is the request type for the /wallet/fund endpoint. WalletFundRequest struct { Transaction types.Transaction `json:"transaction"` diff --git a/autopilot/autopilot.go b/autopilot/autopilot.go index 43b54023d..d1ca3cd59 100644 --- a/autopilot/autopilot.go +++ b/autopilot/autopilot.go @@ -84,7 +84,7 @@ type Bus interface { // wallet Wallet(ctx context.Context) (api.WalletResponse, error) - WalletDiscard(ctx context.Context, txn types.Transaction) error + WalletDiscard(ctx context.Context, txnSet ...types.Transaction) error WalletOutputs(ctx context.Context) (resp []wallet.SiacoinElement, err error) WalletPending(ctx context.Context) (resp []types.Transaction, err error) WalletRedistribute(ctx context.Context, outputs int, amount types.Currency) (id types.TransactionID, err error) diff --git a/bus/bus.go b/bus/bus.go index abde20e79..1443b8fa0 100644 --- a/bus/bus.go +++ b/bus/bus.go @@ -84,7 +84,7 @@ type ( FundTransaction(cs consensus.State, txn *types.Transaction, amount types.Currency, useUnconfirmedTxns bool) ([]types.Hash256, error) Height() uint64 Redistribute(cs consensus.State, outputs int, amount, feePerByte types.Currency, pool []types.Transaction) (types.Transaction, []types.Hash256, error) - ReleaseInputs(txn types.Transaction) + ReleaseInputs(txnSete ...types.Transaction) SignTransaction(cs consensus.State, txn *types.Transaction, toSign []types.Hash256, cf types.CoveredFields) error Transactions(before, since time.Time, offset, limit int) ([]wallet.Transaction, error) UnspentOutputs() ([]wallet.SiacoinElement, error) @@ -621,9 +621,9 @@ func (b *bus) walletRedistributeHandler(jc jape.Context) { } func (b *bus) walletDiscardHandler(jc jape.Context) { - var txn types.Transaction - if jc.Decode(&txn) == nil { - b.w.ReleaseInputs(txn) + var wdr api.WalletDiscardRequest + if jc.Decode(&wdr) == nil { + b.w.ReleaseInputs(wdr.TransactionSet...) } } diff --git a/bus/client/wallet.go b/bus/client/wallet.go index 7630654f6..8a5720c21 100644 --- a/bus/client/wallet.go +++ b/bus/client/wallet.go @@ -26,16 +26,17 @@ func (c *Client) SendSiacoins(ctx context.Context, scos []types.SiacoinOutput, u if err != nil { return err } + txnSet := append(parents, txn) defer func() { if err != nil { - _ = c.WalletDiscard(ctx, txn) + _ = c.WalletDiscard(ctx, txnSet...) } }() err = c.WalletSign(ctx, &txn, toSign, types.CoveredFields{WholeTransaction: true}) if err != nil { return err } - return c.BroadcastTransaction(ctx, append(parents, txn)) + return c.BroadcastTransaction(ctx, txnSet) } // Wallet calls the /wallet endpoint on the bus. @@ -46,8 +47,10 @@ func (c *Client) Wallet(ctx context.Context) (resp api.WalletResponse, err error // WalletDiscard discards the provided txn, make its inputs usable again. This // should only be called on transactions that will never be broadcast. -func (c *Client) WalletDiscard(ctx context.Context, txn types.Transaction) error { - return c.c.WithContext(ctx).POST("/wallet/discard", txn, nil) +func (c *Client) WalletDiscard(ctx context.Context, txn ...types.Transaction) error { + return c.c.WithContext(ctx).POST("/wallet/discard", api.WalletDiscardRequest{ + TransactionSet: txn, + }, nil) } // WalletFund funds txn using inputs controlled by the wallet. diff --git a/wallet/wallet.go b/wallet/wallet.go index 6882d9163..c02f1facd 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -270,9 +270,11 @@ func (w *SingleAddressWallet) FundTransaction(cs consensus.State, txn *types.Tra // ReleaseInputs is a helper function that releases the inputs of txn for use in // other transactions. It should only be called on transactions that are invalid // or will never be broadcast. -func (w *SingleAddressWallet) ReleaseInputs(txn types.Transaction) { - for _, in := range txn.SiacoinInputs { - delete(w.lastUsed, types.Hash256(in.ParentID)) +func (w *SingleAddressWallet) ReleaseInputs(txnSet ...types.Transaction) { + for _, txn := range txnSet { + for _, in := range txn.SiacoinInputs { + delete(w.lastUsed, types.Hash256(in.ParentID)) + } } } diff --git a/worker/rhpv3.go b/worker/rhpv3.go index c822e3e05..9fcb96527 100644 --- a/worker/rhpv3.go +++ b/worker/rhpv3.go @@ -1234,7 +1234,7 @@ func RPCRenew(ctx context.Context, rrr api.RHPRenewRequest, bus Bus, t *transpor } // Starting from here, we need to make sure to release the txn on error. - defer discardTxnOnErr(ctx, bus, l, wprr.TransactionSet[len(wprr.TransactionSet)-1], "RPCRenew", &err) + defer discardTxnSetOnErr(ctx, bus, l, wprr.TransactionSet, "RPCRenew", &err) txnSet := wprr.TransactionSet parents, txn := txnSet[:len(txnSet)-1], txnSet[len(txnSet)-1] diff --git a/worker/worker.go b/worker/worker.go index 56df000e7..a04a0132c 100644 --- a/worker/worker.go +++ b/worker/worker.go @@ -98,7 +98,7 @@ type ( MultipartUpload(ctx context.Context, uploadID string) (resp api.MultipartUpload, err error) PackedSlabsForUpload(ctx context.Context, lockingDuration time.Duration, minShards, totalShards uint8, set string, limit int) ([]api.PackedSlab, error) - WalletDiscard(ctx context.Context, txn types.Transaction) error + WalletDiscard(ctx context.Context, txnSet ...types.Transaction) error WalletFund(ctx context.Context, txn *types.Transaction, amount types.Currency, useUnconfirmedTxns bool) ([]types.Hash256, []types.Transaction, error) WalletPrepareForm(ctx context.Context, renterAddress types.Address, renterKey types.PublicKey, renterFunds, hostCollateral types.Currency, hostKey types.PublicKey, hostSettings rhpv2.HostSettings, endHeight uint64) (txns []types.Transaction, err error) WalletPrepareRenew(ctx context.Context, revision types.FileContractRevision, hostAddress, renterAddress types.Address, renterKey types.PrivateKey, renterFunds, minNewCollateral types.Currency, pt rhpv3.HostPriceTable, endHeight, windowSize, expectedStorage uint64) (api.WalletPrepareRenewResponse, error) @@ -373,8 +373,8 @@ func (w *worker) rhpPriceTableHandler(jc jape.Context) { jc.Encode(hpt) } -func (w *worker) discardTxnOnErr(ctx context.Context, txn types.Transaction, errContext string, err *error) { - discardTxnOnErr(ctx, w.bus, w.logger, txn, errContext, err) +func (w *worker) discardTxnSetOnErr(ctx context.Context, txnSet []types.Transaction, errContext string, err *error) { + discardTxnSetOnErr(ctx, w.bus, w.logger, txnSet, errContext, err) } func (w *worker) rhpFormHandler(jc jape.Context) { @@ -429,7 +429,7 @@ func (w *worker) rhpFormHandler(jc jape.Context) { if err != nil { return err } - defer w.discardTxnOnErr(ctx, renterTxnSet[len(renterTxnSet)-1], "rhpFormHandler", &err) + defer w.discardTxnSetOnErr(ctx, renterTxnSet, "rhpFormHandler", &err) contract, txnSet, err = RPCFormContract(ctx, t, renterKey, renterTxnSet) if err != nil { @@ -492,19 +492,21 @@ func (w *worker) rhpBroadcastHandler(jc jape.Context) { if jc.Check("failed to fund transaction", err) != nil { return } + // Sign the txn. err = w.bus.WalletSign(ctx, &txn, toSign, types.CoveredFields{ WholeTransaction: true, }) + + txnSet := append(parents, txn) if jc.Check("failed to sign transaction", err) != nil { - _ = w.bus.WalletDiscard(ctx, txn) + _ = w.bus.WalletDiscard(ctx, txnSet...) return } // Broadcast the txn. - txnSet := append(parents, txn) err = w.bus.BroadcastTransaction(ctx, txnSet) if jc.Check("failed to broadcast transaction", err) != nil { - _ = w.bus.WalletDiscard(ctx, txn) + _ = w.bus.WalletDiscard(ctx, txnSet...) return } } @@ -1409,7 +1411,7 @@ func (w *worker) scanHost(ctx context.Context, hostKey types.PublicKey, hostIP s return } -func discardTxnOnErr(ctx context.Context, bus Bus, l *zap.SugaredLogger, txn types.Transaction, errContext string, err *error) { +func discardTxnSetOnErr(ctx context.Context, bus Bus, l *zap.SugaredLogger, txnSet []types.Transaction, errContext string, err *error) { if *err == nil { return } @@ -1419,7 +1421,7 @@ func discardTxnOnErr(ctx context.Context, bus Bus, l *zap.SugaredLogger, txn typ timeoutCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() timeoutCtx = trace.ContextWithSpan(timeoutCtx, span) - if err := bus.WalletDiscard(timeoutCtx, txn); err != nil { + if err := bus.WalletDiscard(timeoutCtx, txnSet...); err != nil { l.Errorf("%v: failed to discard txn: %v", err) } } From 1a736ceb2c95091f244b5997f4751b500a40df1a Mon Sep 17 00:00:00 2001 From: Chris Schinnerl Date: Wed, 13 Dec 2023 14:39:40 +0100 Subject: [PATCH 2/5] testing: fix TestBlocklist --- bus/client/wallet.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bus/client/wallet.go b/bus/client/wallet.go index 8a5720c21..51bfba833 100644 --- a/bus/client/wallet.go +++ b/bus/client/wallet.go @@ -32,7 +32,7 @@ func (c *Client) SendSiacoins(ctx context.Context, scos []types.SiacoinOutput, u _ = c.WalletDiscard(ctx, txnSet...) } }() - err = c.WalletSign(ctx, &txn, toSign, types.CoveredFields{WholeTransaction: true}) + err = c.WalletSign(ctx, &txnSet[len(txnSet)-1], toSign, types.CoveredFields{WholeTransaction: true}) if err != nil { return err } From 1902b67e37ab8282a54f77c95781cdde9c13b7ff Mon Sep 17 00:00:00 2001 From: Chris Schinnerl Date: Wed, 13 Dec 2023 14:47:46 +0100 Subject: [PATCH 3/5] autopilot: errLargeTransaction --- autopilot/contractor.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/autopilot/contractor.go b/autopilot/contractor.go index 161a1ba67..2a7dd9fae 100644 --- a/autopilot/contractor.go +++ b/autopilot/contractor.go @@ -85,6 +85,10 @@ const ( timeoutPruneContract = 10 * time.Minute ) +var ( + errLargeTransaction = errors.New("transaction is too large for this transaction pool") +) + type ( contractor struct { ap *Autopilot @@ -1384,7 +1388,9 @@ func (c *contractor) renewContract(ctx context.Context, w Worker, ci contractInf "renterFunds", renterFunds, "expectedNewStorage", expectedNewStorage, ) - if strings.Contains(err.Error(), wallet.ErrInsufficientBalance.Error()) { + if strings.Contains(err.Error(), errLargeTransaction.Error()) { + return api.ContractMetadata{}, true, err + } else if strings.Contains(err.Error(), wallet.ErrInsufficientBalance.Error()) { return api.ContractMetadata{}, false, err } return api.ContractMetadata{}, true, err @@ -1462,7 +1468,9 @@ func (c *contractor) refreshContract(ctx context.Context, w Worker, ci contractI resp, err := w.RHPRenew(ctx, contract.ID, contract.EndHeight(), hk, contract.SiamuxAddr, settings.Address, state.address, renterFunds, minNewCollateral, expectedStorage, settings.WindowSize) if err != nil { c.logger.Errorw("refresh failed", zap.Error(err), "hk", hk, "fcid", fcid) - if strings.Contains(err.Error(), wallet.ErrInsufficientBalance.Error()) { + if strings.Contains(err.Error(), errLargeTransaction.Error()) { + return api.ContractMetadata{}, true, err + } else if strings.Contains(err.Error(), wallet.ErrInsufficientBalance.Error()) { return api.ContractMetadata{}, false, err } return api.ContractMetadata{}, true, err @@ -1535,7 +1543,9 @@ func (c *contractor) formContract(ctx context.Context, w Worker, host hostdb.Hos if err != nil { // TODO: keep track of consecutive failures and break at some point c.logger.Errorw(fmt.Sprintf("contract formation failed, err: %v", err), "hk", hk) - if strings.Contains(err.Error(), wallet.ErrInsufficientBalance.Error()) { + if strings.Contains(err.Error(), errLargeTransaction.Error()) { + return api.ContractMetadata{}, true, err + } else if strings.Contains(err.Error(), wallet.ErrInsufficientBalance.Error()) { return api.ContractMetadata{}, false, err } return api.ContractMetadata{}, true, err From 7c2f729a55e4900156ff99919e8307336c603c43 Mon Sep 17 00:00:00 2001 From: Chris Schinnerl Date: Wed, 13 Dec 2023 15:49:32 +0100 Subject: [PATCH 4/5] Revert "autopilot: errLargeTransaction" This reverts commit 1902b67e37ab8282a54f77c95781cdde9c13b7ff. --- autopilot/contractor.go | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/autopilot/contractor.go b/autopilot/contractor.go index 2a7dd9fae..161a1ba67 100644 --- a/autopilot/contractor.go +++ b/autopilot/contractor.go @@ -85,10 +85,6 @@ const ( timeoutPruneContract = 10 * time.Minute ) -var ( - errLargeTransaction = errors.New("transaction is too large for this transaction pool") -) - type ( contractor struct { ap *Autopilot @@ -1388,9 +1384,7 @@ func (c *contractor) renewContract(ctx context.Context, w Worker, ci contractInf "renterFunds", renterFunds, "expectedNewStorage", expectedNewStorage, ) - if strings.Contains(err.Error(), errLargeTransaction.Error()) { - return api.ContractMetadata{}, true, err - } else if strings.Contains(err.Error(), wallet.ErrInsufficientBalance.Error()) { + if strings.Contains(err.Error(), wallet.ErrInsufficientBalance.Error()) { return api.ContractMetadata{}, false, err } return api.ContractMetadata{}, true, err @@ -1468,9 +1462,7 @@ func (c *contractor) refreshContract(ctx context.Context, w Worker, ci contractI resp, err := w.RHPRenew(ctx, contract.ID, contract.EndHeight(), hk, contract.SiamuxAddr, settings.Address, state.address, renterFunds, minNewCollateral, expectedStorage, settings.WindowSize) if err != nil { c.logger.Errorw("refresh failed", zap.Error(err), "hk", hk, "fcid", fcid) - if strings.Contains(err.Error(), errLargeTransaction.Error()) { - return api.ContractMetadata{}, true, err - } else if strings.Contains(err.Error(), wallet.ErrInsufficientBalance.Error()) { + if strings.Contains(err.Error(), wallet.ErrInsufficientBalance.Error()) { return api.ContractMetadata{}, false, err } return api.ContractMetadata{}, true, err @@ -1543,9 +1535,7 @@ func (c *contractor) formContract(ctx context.Context, w Worker, host hostdb.Hos if err != nil { // TODO: keep track of consecutive failures and break at some point c.logger.Errorw(fmt.Sprintf("contract formation failed, err: %v", err), "hk", hk) - if strings.Contains(err.Error(), errLargeTransaction.Error()) { - return api.ContractMetadata{}, true, err - } else if strings.Contains(err.Error(), wallet.ErrInsufficientBalance.Error()) { + if strings.Contains(err.Error(), wallet.ErrInsufficientBalance.Error()) { return api.ContractMetadata{}, false, err } return api.ContractMetadata{}, true, err From 0751c8ad77e0864e6e252882bced34f045b96ea2 Mon Sep 17 00:00:00 2001 From: Chris Schinnerl Date: Wed, 13 Dec 2023 16:05:08 +0100 Subject: [PATCH 5/5] bus: fix spelling --- bus/bus.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bus/bus.go b/bus/bus.go index 1443b8fa0..0bf680ca4 100644 --- a/bus/bus.go +++ b/bus/bus.go @@ -84,7 +84,7 @@ type ( FundTransaction(cs consensus.State, txn *types.Transaction, amount types.Currency, useUnconfirmedTxns bool) ([]types.Hash256, error) Height() uint64 Redistribute(cs consensus.State, outputs int, amount, feePerByte types.Currency, pool []types.Transaction) (types.Transaction, []types.Hash256, error) - ReleaseInputs(txnSete ...types.Transaction) + ReleaseInputs(txnSet ...types.Transaction) SignTransaction(cs consensus.State, txn *types.Transaction, toSign []types.Hash256, cf types.CoveredFields) error Transactions(before, since time.Time, offset, limit int) ([]wallet.Transaction, error) UnspentOutputs() ([]wallet.SiacoinElement, error)