From cad8b73571ad2e9006ee6d30b03ceedd4064daa5 Mon Sep 17 00:00:00 2001 From: sbackend Date: Wed, 6 May 2026 23:46:17 +0200 Subject: [PATCH 1/7] feat: gas fee limit draft implementation (refactoring) --- cmd/bee/cmd/cmd.go | 4 + cmd/bee/cmd/start.go | 2 + pkg/api/api_test.go | 6 +- pkg/node/node.go | 8 +- pkg/storageincentives/agent.go | 50 ++- pkg/storageincentives/agent_test.go | 6 +- pkg/storageincentives/metrics.go | 9 + .../redistribution/redistribution.go | 394 ++++++++++++++++-- .../redistribution/redistribution_test.go | 80 +++- pkg/storageincentives/redistributionstate.go | 33 ++ pkg/transaction/mock/transaction.go | 17 + pkg/transaction/transaction.go | 32 ++ 12 files changed, 590 insertions(+), 51 deletions(-) diff --git a/cmd/bee/cmd/cmd.go b/cmd/bee/cmd/cmd.go index b4ece436d8d..8163997d9e4 100644 --- a/cmd/bee/cmd/cmd.go +++ b/cmd/bee/cmd/cmd.go @@ -82,6 +82,8 @@ const ( optionSkipPostageSnapshot = "skip-postage-snapshot" optionNameMinimumGasTipCap = "minimum-gas-tip-cap" optionNameGasLimitFallback = "gas-limit-fallback" + optionNameMaxTxCost = "max-tx-cost" + optionNameMaxTxCostTolerancePercent = "max-tx-cost-tolerance-percent" optionNameP2PWSSEnable = "p2p-wss-enable" optionP2PWSSAddr = "p2p-wss-addr" optionNATWSSAddr = "nat-wss-addr" @@ -328,6 +330,8 @@ func (c *command) setAllFlags(cmd *cobra.Command) { cmd.Flags().Bool(optionSkipPostageSnapshot, false, "skip postage snapshot") cmd.Flags().Uint64(optionNameMinimumGasTipCap, 0, "minimum gas tip cap in wei for transactions, 0 means use suggested gas tip cap") cmd.Flags().Uint64(optionNameGasLimitFallback, 500_000, "gas limit fallback when estimation fails for contract transactions") + cmd.Flags().Uint64(optionNameMaxTxCost, 0, "maximum total cost in wei per redistribution transaction, 0 means no limit") + cmd.Flags().Uint64(optionNameMaxTxCostTolerancePercent, 5, "percentage above max-tx-cost within which the transaction is still allowed") cmd.Flags().Bool(optionNameP2PWSSEnable, false, "Enable Secure WebSocket P2P connections") cmd.Flags().String(optionP2PWSSAddr, ":1635", "p2p wss address") cmd.Flags().String(optionNATWSSAddr, "", "WSS NAT exposed address") diff --git a/cmd/bee/cmd/start.go b/cmd/bee/cmd/start.go index 5773c4af3e3..95c00eeb8a3 100644 --- a/cmd/bee/cmd/start.go +++ b/cmd/bee/cmd/start.go @@ -301,6 +301,8 @@ func buildBeeNode(ctx context.Context, c *command, cmd *cobra.Command, logger lo Logger: logger, MinimumGasTipCap: c.config.GetUint64(optionNameMinimumGasTipCap), GasLimitFallback: c.config.GetUint64(optionNameGasLimitFallback), + MaxTxCost: c.config.GetUint64(optionNameMaxTxCost), + MaxTxCostTolerancePercent: c.config.GetUint64(optionNameMaxTxCostTolerancePercent), MinimumStorageRadius: c.config.GetUint(optionMinimumStorageRadius), MutexProfile: c.config.GetBool(optionNamePProfMutex), NATAddr: c.config.GetString(optionNameNATAddr), diff --git a/pkg/api/api_test.go b/pkg/api/api_test.go index 4dd7aa1d3f7..2bc5f105a64 100644 --- a/pkg/api/api_test.go +++ b/pkg/api/api_test.go @@ -752,7 +752,7 @@ func (m *mockContract) IsWinner(context.Context) (bool, error) { return false, nil } -func (m *mockContract) Claim(context.Context, redistribution.ChunkInclusionProofs) (common.Hash, error) { +func (m *mockContract) Claim(context.Context, redistribution.ChunkInclusionProofs, *redistribution.ClaimOpts) (common.Hash, error) { m.mtx.Lock() defer m.mtx.Unlock() m.callsList = append(m.callsList, claimCall) @@ -773,6 +773,10 @@ func (m *mockContract) Reveal(context.Context, uint8, []byte, []byte) (common.Ha return common.Hash{}, nil } +func (m *mockContract) ExpectedReward(context.Context) (*big.Int, error) { + return big.NewInt(1_000_000), nil +} + type mockHealth struct{} func (m *mockHealth) IsHealthy() bool { return true } diff --git a/pkg/node/node.go b/pkg/node/node.go index 35b78d4dc05..db31b8f80ac 100644 --- a/pkg/node/node.go +++ b/pkg/node/node.go @@ -193,6 +193,8 @@ type Options struct { WarmupTime time.Duration WelcomeMessage string WhitelistedWithdrawalAddress []string + MaxTxCost uint64 + MaxTxCostTolerancePercent uint64 } const ( @@ -1201,7 +1203,11 @@ func NewBee( redistributionContractAddress = common.HexToAddress(o.RedistributionContractAddress) } - redistributionContract := redistribution.New(swarmAddress, overlayEthAddress, logger, transactionService, redistributionContractAddress, abiutil.MustParseABI(chainCfg.RedistributionABI), contractGasLimit) + var redistributionOpts []redistribution.Option + if o.MaxTxCost > 0 { + redistributionOpts = append(redistributionOpts, redistribution.WithMaxTxCost(o.MaxTxCost, o.MaxTxCostTolerancePercent)) + } + redistributionContract := redistribution.New(swarmAddress, overlayEthAddress, logger, transactionService, redistributionContractAddress, abiutil.MustParseABI(chainCfg.RedistributionABI), postageStampContractAddress, postageStampContractABI, contractGasLimit, redistributionOpts...) isFullySynced := func() bool { reserveThreshold := reserveCapacity * 5 / 10 diff --git a/pkg/storageincentives/agent.go b/pkg/storageincentives/agent.go index 5142e97836e..2116033ff53 100644 --- a/pkg/storageincentives/agent.go +++ b/pkg/storageincentives/agent.go @@ -41,6 +41,10 @@ const ( // average tx gas used by transactions issued from agent avgTxGas = 250_000 + + // forceClaimBlocksBeforeEnd is how many blocks before round end claim may + // bypass max-tx-cost when economics justify it (see redistribution.ClaimOpts). + forceClaimBlocksBeforeEnd = 10 ) type ChainBackend interface { @@ -59,6 +63,7 @@ type Agent struct { metrics metrics backend ChainBackend blocksPerRound uint64 + blockTime time.Duration contract redistribution.Contract batchExpirer postagecontract.PostageBatchExpirer redistributionStatuser staking.RedistributionStatuser @@ -102,6 +107,7 @@ func New(overlay swarm.Address, store: store, fullSyncedFunc: fullSyncedFunc, blocksPerRound: blocksPerRound, + blockTime: blockTime, quit: make(chan struct{}), redistributionStatuser: redistributionStatuser, health: health, @@ -116,7 +122,7 @@ func New(overlay swarm.Address, a.state = state a.wg.Add(1) - go a.start(blockTime, a.blocksPerRound, blocksPerPhase) + go a.start(a.blockTime, a.blocksPerRound, blocksPerPhase) return a, nil } @@ -280,6 +286,11 @@ func (a *Agent) handleCommit(ctx context.Context, round uint64) error { err := a.commit(ctx, sample, round) if err != nil { + if errors.Is(err, redistribution.ErrMaxTxCostExceeded) { + a.logger.Info("skipping commit: tx cost exceeds configured max", "round", round) + a.metrics.SkippedExpensivePhase.Inc() + return nil + } return err } @@ -311,7 +322,7 @@ func (a *Agent) handleReveal(ctx context.Context, round uint64) error { a.metrics.ErrReveal.Inc() return err } - a.state.AddFee(ctx, txHash) + a.state.AddRoundFee(ctx, round, txHash) a.state.SetHasRevealed(round) @@ -353,7 +364,7 @@ func (a *Agent) handleClaim(ctx context.Context, round uint64) error { errBalance := a.state.SetBalance(ctx) if errBalance != nil { - a.logger.Info("could not set balance", "err", err) + a.logger.Info("could not set balance", "err", errBalance) } sampleData, exists := a.state.SampleData(round - 1) @@ -371,8 +382,33 @@ func (a *Agent) handleClaim(ctx context.Context, round uint64) error { return fmt.Errorf("making inclusion proofs: %w", err) } - txHash, err := a.contract.Claim(ctx, proofs) + claimCtx := ctx + phaseEndBlock := (round+1)*a.blocksPerRound - 1 + if rem := int64(phaseEndBlock) - int64(a.state.currentBlock()); rem > 0 { + var cancel context.CancelFunc + claimCtx, cancel = context.WithDeadline(ctx, time.Now().Add(time.Duration(rem)*a.blockTime)) + defer cancel() + } + + reward, err := a.contract.ExpectedReward(ctx) if err != nil { + a.logger.Warning("could not estimate claim reward, override will be disabled", "error", err) + } + + opts := &redistribution.ClaimOpts{ + OverrideAfterBlock: (round+1)*a.blocksPerRound - forceClaimBlocksBeforeEnd, + CurrentBlockFn: func() uint64 { return a.state.currentBlock() }, + ExpectedReward: reward, + RoundFees: a.state.RoundFees(round), + } + + txHash, err := a.contract.Claim(claimCtx, proofs, opts) + if err != nil { + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + a.logger.Info("claim aborted by context", "round", round, "err", err) + a.metrics.SkippedExpensivePhase.Inc() + return nil + } a.metrics.ErrClaim.Inc() return fmt.Errorf("claiming win: %w", err) } @@ -382,11 +418,11 @@ func (a *Agent) handleClaim(ctx context.Context, round uint64) error { if errBalance == nil { errReward := a.state.CalculateWinnerReward(ctx) if errReward != nil { - a.logger.Info("calculate winner reward", "err", err) + a.logger.Info("calculate winner reward", "err", errReward) } } - a.state.AddFee(ctx, txHash) + a.state.AddRoundFee(ctx, round, txHash) return nil } @@ -539,7 +575,7 @@ func (a *Agent) commit(ctx context.Context, sample SampleData, round uint64) err a.metrics.ErrCommit.Inc() return err } - a.state.AddFee(ctx, txHash) + a.state.AddRoundFee(ctx, round, txHash) a.state.SetCommitKey(round, key) diff --git a/pkg/storageincentives/agent_test.go b/pkg/storageincentives/agent_test.go index 6449ede9059..53078bef3fb 100644 --- a/pkg/storageincentives/agent_test.go +++ b/pkg/storageincentives/agent_test.go @@ -307,7 +307,7 @@ func (m *mockContract) IsWinner(context.Context) (bool, error) { return false, nil } -func (m *mockContract) Claim(context.Context, redistribution.ChunkInclusionProofs) (common.Hash, error) { +func (m *mockContract) Claim(context.Context, redistribution.ChunkInclusionProofs, *redistribution.ClaimOpts) (common.Hash, error) { m.mtx.Lock() defer m.mtx.Unlock() m.callsList = append(m.callsList, claimCall) @@ -333,6 +333,10 @@ func (m *mockContract) Reveal(_ context.Context, r uint8, _ []byte, _ []byte) (c return common.Hash{}, nil } +func (m *mockContract) ExpectedReward(context.Context) (*big.Int, error) { + return big.NewInt(1_000_000), nil +} + type mockHealth struct{} func (m *mockHealth) IsHealthy() bool { return true } diff --git a/pkg/storageincentives/metrics.go b/pkg/storageincentives/metrics.go index b376d9d20b2..3db40b39caa 100644 --- a/pkg/storageincentives/metrics.go +++ b/pkg/storageincentives/metrics.go @@ -31,6 +31,9 @@ type metrics struct { ErrClaim prometheus.Counter ErrWinner prometheus.Counter ErrCheckIsPlaying prometheus.Counter + + // cost control metrics + SkippedExpensivePhase prometheus.Counter } func newMetrics() metrics { @@ -137,6 +140,12 @@ func newMetrics() metrics { Name: "is_playing_errors", Help: "total neighborhood selected errors while processing", }), + SkippedExpensivePhase: prometheus.NewCounter(prometheus.CounterOpts{ + Namespace: m.Namespace, + Subsystem: subsystem, + Name: "skipped_expensive_phase", + Help: "Count of phases skipped because estimated tx cost exceeded configured limit.", + }), } } diff --git a/pkg/storageincentives/redistribution/redistribution.go b/pkg/storageincentives/redistribution/redistribution.go index 77013a8a990..71456d0e4cf 100644 --- a/pkg/storageincentives/redistribution/redistribution.go +++ b/pkg/storageincentives/redistribution/redistribution.go @@ -6,8 +6,11 @@ package redistribution import ( "context" + "errors" "fmt" "math/big" + "strings" + "time" "github.com/ethereum/go-ethereum/accounts/abi" "github.com/ethereum/go-ethereum/common" @@ -20,15 +23,33 @@ import ( const ( loggerName = "redistributionContract" BoostTipPercent = 50 + + retryBaseDelay = 5 * time.Second + claimRetryInterval = 15 * time.Second ) +// ErrMaxTxCostExceeded is returned when the upper-bound cost of a redistribution +// transaction (gas limit × max fee per gas) would exceed the configured limit. +var ErrMaxTxCostExceeded = errors.New("redistribution tx cost exceeds max tx cost limit") + +// ClaimOpts configures optional claim behaviour: after OverrideAfterBlock (absolute +// chain block number), if ExpectedReward covers upper-bound claim cost plus +// RoundFees, the max-tx-cost limit is bypassed for that send attempt. +type ClaimOpts struct { + OverrideAfterBlock uint64 + CurrentBlockFn func() uint64 + ExpectedReward *big.Int + RoundFees *big.Int +} + type Contract interface { ReserveSalt(context.Context) ([]byte, error) IsPlaying(context.Context, uint8) (bool, error) IsWinner(context.Context) (bool, error) - Claim(context.Context, ChunkInclusionProofs) (common.Hash, error) + Claim(context.Context, ChunkInclusionProofs, *ClaimOpts) (common.Hash, error) Commit(context.Context, []byte, uint64) (common.Hash, error) Reveal(context.Context, uint8, []byte, []byte) (common.Hash, error) + ExpectedReward(ctx context.Context) (*big.Int, error) } type contract struct { @@ -38,9 +59,29 @@ type contract struct { txService transaction.Service incentivesContractAddress common.Address incentivesContractABI abi.ABI + postageContractAddress common.Address + postageContractABI abi.ABI + maxTxCost *big.Int + maxTxCostTolerancePercent uint64 gasLimit uint64 } +// Option configures the redistribution contract wrapper. +type Option func(*contract) + +// WithMaxTxCost sets the maximum total wei the node is willing to spend on a +// single redistribution transaction (gas limit × max fee per gas). When wei is +// zero, no limit is enforced. tolerancePercent expands the limit (e.g. 5 means +// allow up to 105% of wei); use 0 for a strict cap. +func WithMaxTxCost(wei uint64, tolerancePercent uint64) Option { + return func(c *contract) { + if wei > 0 { + c.maxTxCost = new(big.Int).SetUint64(wei) + c.maxTxCostTolerancePercent = tolerancePercent + } + } +} + func New( overlay swarm.Address, owner common.Address, @@ -48,17 +89,26 @@ func New( txService transaction.Service, incentivesContractAddress common.Address, incentivesContractABI abi.ABI, + postageContractAddress common.Address, + postageContractABI abi.ABI, gasLimit uint64, + opts ...Option, ) Contract { - return &contract{ + c := &contract{ overlay: overlay, owner: owner, logger: logger.WithName(loggerName).Register(), txService: txService, incentivesContractAddress: incentivesContractAddress, incentivesContractABI: incentivesContractABI, + postageContractAddress: postageContractAddress, + postageContractABI: postageContractABI, gasLimit: gasLimit, } + for _, o := range opts { + o(c) + } + return c } // IsPlaying checks if the overlay is participating in the upcoming round. @@ -100,27 +150,102 @@ func (c *contract) IsWinner(ctx context.Context) (isWinner bool, err error) { return results[0].(bool), nil } -// Claim sends a transaction to blockchain if a win is claimed. -func (c *contract) Claim(ctx context.Context, proofs ChunkInclusionProofs) (common.Hash, error) { +// Claim sends a transaction to blockchain if a win is claimed. When the +// configured max-tx-cost is exceeded, Claim waits claimRetryInterval and +// retries instead of returning ErrMaxTxCostExceeded. After OverrideAfterBlock +// (see ClaimOpts), if economics justify it, the limit is bypassed for one send. +func (c *contract) Claim(ctx context.Context, proofs ChunkInclusionProofs, opts *ClaimOpts) (common.Hash, error) { callData, err := c.incentivesContractABI.Pack("claim", proofs.A, proofs.B, proofs.C) if err != nil { return common.Hash{}, err } - request := &transaction.TxRequest{ - To: &c.incentivesContractAddress, - Data: callData, - GasPrice: sctx.GetGasPrice(ctx), - GasLimit: max(sctx.GetGasLimit(ctx), c.gasLimit), - MinEstimatedGasLimit: 500_000, - Value: big.NewInt(0), - Description: "claim win transaction", + + for { + sendCtx, prepErr := c.prepareSendCtx(ctx, BoostTipPercent) + if prepErr == nil { + request := c.newTxRequest(sendCtx, callData, "claim win transaction") + txHash, sendErr := c.sendAndWaitWithRetry(sendCtx, request, BoostTipPercent) + if sendErr != nil { + return txHash, fmt.Errorf("claim: %w", sendErr) + } + return txHash, nil + } + + if !errors.Is(prepErr, ErrMaxTxCostExceeded) { + return common.Hash{}, fmt.Errorf("claim: %w", prepErr) + } + + if c.tryClaimOverride(opts) { + gasFeeCap, ok := c.canOverrideClaim(ctx, opts) + if ok { + c.logger.Warning("claim: max-tx-cost overridden", + "expected_reward", opts.ExpectedReward, + "round_fees", opts.RoundFees, + "override_after_block", opts.OverrideAfterBlock, + ) + overrideCtx := sctx.SetGasPrice(ctx, gasFeeCap) + request := c.newTxRequest(overrideCtx, callData, "claim win transaction (override)") + txHash, sendErr := c.sendAndWaitWithRetry(overrideCtx, request, BoostTipPercent) + if sendErr != nil { + return txHash, fmt.Errorf("claim: %w", sendErr) + } + return txHash, nil + } + } + + c.logger.Info("claim: tx cost exceeds limit, waiting", "retry_in", claimRetryInterval) + select { + case <-ctx.Done(): + return common.Hash{}, ctx.Err() + case <-time.After(claimRetryInterval): + } + } +} + +func (c *contract) tryClaimOverride(opts *ClaimOpts) bool { + if opts == nil || opts.OverrideAfterBlock == 0 || opts.CurrentBlockFn == nil { + return false + } + return opts.CurrentBlockFn() >= opts.OverrideAfterBlock +} + +func (c *contract) canOverrideClaim(ctx context.Context, opts *ClaimOpts) (*big.Int, bool) { + if opts.ExpectedReward == nil { + reward, err := c.ExpectedReward(ctx) + if err != nil { + c.logger.Warning("error getting expected claim reward", "error", err) + } + opts.ExpectedReward = reward + } + + if opts.ExpectedReward == nil || opts.RoundFees == nil { + return nil, false + } + if opts.ExpectedReward.Sign() <= 0 { + return nil, false } - txHash, err := c.sendAndWait(ctx, request, BoostTipPercent) + + gasUnits := int64(max(sctx.GetGasLimit(ctx), c.gasLimit)) + if gasUnits <= 0 { + gasUnits = 500_000 + } + + estimated, gasFeeCap, err := c.txService.EstimateTxCost(ctx, gasUnits, BoostTipPercent) if err != nil { - return txHash, fmt.Errorf("claim: %w", err) + c.logger.Warning("claim override: estimate failed", "error", err) + return nil, false } - return txHash, nil + totalSpent := new(big.Int).Add(estimated, opts.RoundFees) + if opts.ExpectedReward.Cmp(totalSpent) < 0 { + c.logger.Info("claim override: reward does not cover upper-bound cost", + "estimated", estimated, + "round_fees", opts.RoundFees, + "expected_reward", opts.ExpectedReward, + ) + return nil, false + } + return gasFeeCap, true } // Commit submits the obfusHash hash by sending a transaction to the blockchain. @@ -129,16 +254,23 @@ func (c *contract) Commit(ctx context.Context, obfusHash []byte, round uint64) ( if err != nil { return common.Hash{}, err } - request := &transaction.TxRequest{ - To: &c.incentivesContractAddress, - Data: callData, - GasPrice: sctx.GetGasPrice(ctx), - GasLimit: max(sctx.GetGasLimit(ctx), c.gasLimit), - MinEstimatedGasLimit: 500_000, - Value: big.NewInt(0), - Description: "commit transaction", + for { + ctx, err = c.prepareSendCtx(ctx, BoostTipPercent) + if err == nil { + break + } + + select { + case <-ctx.Done(): + return common.Hash{}, err + case <-time.After(retryBaseDelay): + continue + } } - txHash, err := c.sendAndWait(ctx, request, BoostTipPercent) + + request := c.newTxRequest(ctx, callData, "commit transaction") + + txHash, err := c.sendAndWaitWithRetry(ctx, request, BoostTipPercent) if err != nil { return txHash, fmt.Errorf("commit: obfusHash %v: %w", common.BytesToHash(obfusHash), err) } @@ -152,16 +284,21 @@ func (c *contract) Reveal(ctx context.Context, storageDepth uint8, reserveCommit if err != nil { return common.Hash{}, err } - request := &transaction.TxRequest{ - To: &c.incentivesContractAddress, - Data: callData, - GasPrice: sctx.GetGasPrice(ctx), - GasLimit: max(sctx.GetGasLimit(ctx), c.gasLimit), - MinEstimatedGasLimit: 500_000, - Value: big.NewInt(0), - Description: "reveal transaction", + for { + ctx, err = c.prepareSendCtx(ctx, BoostTipPercent) + if err == nil { + break + } + + select { + case <-ctx.Done(): + return common.Hash{}, err + case <-time.After(retryBaseDelay): + continue + } } - txHash, err := c.sendAndWait(ctx, request, BoostTipPercent) + request := c.newTxRequest(ctx, callData, "reveal transaction") + txHash, err := c.sendAndWaitWithRetry(ctx, request, BoostTipPercent) if err != nil { return txHash, fmt.Errorf("reveal: storageDepth %d reserveCommitmentHash %v RandomNonce %v: %w", storageDepth, common.BytesToHash(reserveCommitmentHash), common.BytesToHash(RandomNonce), err) } @@ -189,7 +326,7 @@ func (c *contract) ReserveSalt(ctx context.Context) ([]byte, error) { return salt[:], nil } -func (c *contract) sendAndWait(ctx context.Context, request *transaction.TxRequest, boostPercent int) (txHash common.Hash, err error) { +func (c *contract) sendAndWaitWithRetry(ctx context.Context, request *transaction.TxRequest, boostPercent int) (txHash common.Hash, err error) { defer func() { err = c.txService.UnwrapABIError( ctx, @@ -199,21 +336,138 @@ func (c *contract) sendAndWait(ctx context.Context, request *transaction.TxReque ) }() - txHash, err = c.txService.Send(ctx, request, boostPercent) - if err != nil { - return txHash, err + for { + txHash, err = c.txService.Send(ctx, request, boostPercent) + if err == nil { + break + } + + if isCritical(err) { + return txHash, err + } + + c.logger.Warning("send failed, will retry", "error", err, "description", request.Description) + + if len(txHash.Bytes()) == 0 { + select { + case <-ctx.Done(): + return txHash, ctx.Err() + case <-time.After(retryBaseDelay): + continue + } + } + + txHash, err = c.resendWithRetry(ctx, txHash) + if err != nil { + return common.Hash{}, err + } + break } + receipt, err := c.txService.WaitForReceipt(ctx, txHash) if err != nil { - return txHash, err + return common.Hash{}, err } if receipt.Status == 0 { return txHash, transaction.ErrTransactionReverted } + return txHash, nil } +func (c *contract) resendWithRetry(ctx context.Context, txHash common.Hash) (common.Hash, error) { + for { + err := c.txService.ResendTransaction(ctx, txHash) + if err != nil { + if isCritical(err) { + return txHash, err + } + select { + case <-ctx.Done(): + return txHash, ctx.Err() + case <-time.After(retryBaseDelay): + continue + } + } + return txHash, nil + } +} + +func isCritical(err error) bool { + if errors.Is(err, transaction.ErrTransactionReverted) { + return true + } + if errors.Is(err, transaction.ErrTransactionCancelled) { + return true + } + if errors.Is(err, transaction.ErrFeeCapExceeded) { + return true + } + + s := err.Error() + nonRetryable := []string{ + "specified gas price", + "below current base fee", + "AlreadyCommitted", + "AlreadyRevealed", + "AlreadyClaimed", + "NotCommitPhase", + "NotRevealPhase", + "NotClaimPhase", + "CommitRoundOver", + "CommitRoundNotStarted", + "PhaseLastBlock", + "OutOfDepth", + "OutOfDepthReveal", + "OutOfDepthClaim", + "NotStaked", + "MustStake2Rounds", + "NoReveals", + "NoCommitsReceived", + "execution reverted", + "insufficient funds", + } + for _, sub := range nonRetryable { + if strings.Contains(s, sub) { + return true + } + } + return false +} + +// ExpectedReward returns the current pot value from the PostageStamp contract via eth_call. +func (c *contract) ExpectedReward(ctx context.Context) (*big.Int, error) { + callData, err := c.postageContractABI.Pack("totalPot") + if err != nil { + return nil, fmt.Errorf("pack totalPot: %w", err) + } + + result, err := c.txService.Call(ctx, &transaction.TxRequest{ + To: &c.postageContractAddress, + Data: callData, + }) + if err != nil { + return nil, fmt.Errorf("call totalPot: %w", err) + } + + results, err := c.postageContractABI.Unpack("totalPot", result) + if err != nil { + return nil, fmt.Errorf("unpack totalPot: %w", err) + } + + if len(results) == 0 { + return nil, fmt.Errorf("totalPot returned no results") + } + + pot, ok := results[0].(*big.Int) + if !ok { + return nil, fmt.Errorf("totalPot unexpected type %T", results[0]) + } + + return pot, nil +} + // callTx simulates a transaction based on tx request. func (c *contract) callTx(ctx context.Context, callData []byte) ([]byte, error) { result, err := c.txService.Call(ctx, &transaction.TxRequest{ @@ -225,3 +479,67 @@ func (c *contract) callTx(ctx context.Context, callData []byte) ([]byte, error) } return result, nil } + +// prepareSendCtx enforces max tx cost (when configured) and pins maxFeePerGas +// into ctx so the subsequent Send cannot use a higher fee than checked. +func (c *contract) prepareSendCtx(ctx context.Context, boostPercent int) (context.Context, error) { + if c.maxTxCost == nil { + return ctx, nil + } + ok, _, gasFeeCap, err := c.isTxCostAcceptable(ctx, boostPercent) + if err != nil { + return ctx, err + } + if !ok { + return ctx, ErrMaxTxCostExceeded + } + if gasFeeCap != nil { + ctx = sctx.SetGasPrice(ctx, gasFeeCap) + } + return ctx, nil +} + +func (c *contract) newTxRequest(ctx context.Context, callData []byte, description string) *transaction.TxRequest { + pinnedGasFeeCap := sctx.GetGasPrice(ctx) + return &transaction.TxRequest{ + To: &c.incentivesContractAddress, + Data: callData, + GasPrice: pinnedGasFeeCap, + GasFeeCap: pinnedGasFeeCap, + GasLimit: max(sctx.GetGasLimit(ctx), c.gasLimit), + MinEstimatedGasLimit: 500_000, + Value: big.NewInt(0), + Description: description, + } +} + +func (c *contract) isTxCostAcceptable(ctx context.Context, tip int) (ok bool, estimated *big.Int, gasFeeCap *big.Int, err error) { + gasUnits := int64(max(sctx.GetGasLimit(ctx), c.gasLimit)) + if gasUnits <= 0 { + gasUnits = 500_000 + } + + // If the caller pinned maxFeePerGas in context, use it for total-cost estimate + // since that is the actual cap used when creating the transaction request. + if pinnedGasFeeCap := sctx.GetGasPrice(ctx); pinnedGasFeeCap != nil { + if pinnedGasFeeCap.Sign() <= 0 { + return false, nil, nil, errors.New("gas price must be greater than zero") + } + gasFeeCap = new(big.Int).Set(pinnedGasFeeCap) + estimated = new(big.Int).Mul(big.NewInt(gasUnits), gasFeeCap) + } else { + estimated, gasFeeCap, err = c.txService.EstimateTxCost(ctx, gasUnits, tip) + if err != nil { + return false, nil, nil, err + } + } + + if c.maxTxCost == nil { + return true, estimated, gasFeeCap, nil + } + + tol := c.maxTxCostTolerancePercent + threshold := new(big.Int).Mul(c.maxTxCost, big.NewInt(int64(100+tol))) + threshold.Div(threshold, big.NewInt(100)) + return estimated.Cmp(threshold) <= 0, estimated, gasFeeCap, nil +} diff --git a/pkg/storageincentives/redistribution/redistribution_test.go b/pkg/storageincentives/redistribution/redistribution_test.go index b48e9014b02..08f5c5a1364 100644 --- a/pkg/storageincentives/redistribution/redistribution_test.go +++ b/pkg/storageincentives/redistribution/redistribution_test.go @@ -11,7 +11,9 @@ import ( "errors" "fmt" "math/big" + "sync/atomic" "testing" + "time" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" @@ -26,7 +28,11 @@ import ( "github.com/ethersphere/bee/v2/pkg/util/testutil" ) -var redistributionContractABI = abiutil.MustParseABI(chaincfg.Testnet.RedistributionABI) +var ( + redistributionContractABI = abiutil.MustParseABI(chaincfg.Testnet.RedistributionABI) + postageContractABI = abiutil.MustParseABI(chaincfg.Testnet.PostageStampABI) + postageContractAddress = common.HexToAddress("eeee") +) func randChunkInclusionProof(t *testing.T) redistribution.ChunkInclusionProof { t.Helper() @@ -88,6 +94,8 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, + postageContractAddress, + postageContractABI, 0, ) @@ -120,6 +128,8 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, + postageContractAddress, + postageContractABI, 0, ) @@ -150,6 +160,8 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, + postageContractAddress, + postageContractABI, 0, ) @@ -177,6 +189,8 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, + postageContractAddress, + postageContractABI, 0, ) @@ -223,10 +237,12 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, + postageContractAddress, + postageContractABI, 0, ) - _, err = contract.Claim(ctx, proofs) + _, err = contract.Claim(ctx, proofs, nil) if err != nil { t.Fatal(err) } @@ -265,10 +281,12 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, + postageContractAddress, + postageContractABI, 0, ) - _, err = contract.Claim(ctx, proofs) + _, err = contract.Claim(ctx, proofs, nil) if !errors.Is(err, transaction.ErrTransactionReverted) { t.Fatal(err) } @@ -308,6 +326,8 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, + postageContractAddress, + postageContractABI, 0, ) @@ -353,6 +373,8 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, + postageContractAddress, + postageContractABI, 0, ) @@ -379,6 +401,8 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, + postageContractAddress, + postageContractABI, 0, ) @@ -409,6 +433,8 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, + postageContractAddress, + postageContractABI, 0, ) @@ -442,6 +468,8 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, + postageContractAddress, + postageContractABI, 0, ) @@ -451,3 +479,49 @@ func TestRedistribution(t *testing.T) { } }) } + +func TestRedistribution_MaxTxCostWaitsUntilContextDone(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), 80*time.Millisecond) + defer cancel() + owner := common.HexToAddress("abcd") + overlay := swarm.NewAddress(common.HexToHash("cbd").Bytes()) + redistributionContractAddress := common.HexToAddress("ffff") + var sendCalled atomic.Bool + + proofs := randChunkInclusionProofs(t) + + contract := redistribution.New( + overlay, + owner, + log.Noop, + transactionMock.New( + transactionMock.WithEstimateTxCostFunc(func(_ context.Context, gasUnits int64, _ int) (*big.Int, *big.Int, error) { + gasFeeCap := big.NewInt(10) + return new(big.Int).Mul(big.NewInt(gasUnits), gasFeeCap), gasFeeCap, nil + }), + transactionMock.WithSendFunc(func(context.Context, *transaction.TxRequest, int) (common.Hash, error) { + sendCalled.Store(true) + return common.Hash{}, errors.New("send should not be called") + }), + transactionMock.WithWaitForReceiptFunc(func(context.Context, common.Hash) (*types.Receipt, error) { + return nil, errors.New("unexpected wait") + }), + ), + redistributionContractAddress, + redistributionContractABI, + postageContractAddress, + postageContractABI, + 100_000, + redistribution.WithMaxTxCost(500_000, 0), + ) + + _, err := contract.Claim(ctx, proofs, nil) + if !errors.Is(err, context.DeadlineExceeded) && !errors.Is(err, context.Canceled) { + t.Fatalf("want context deadline or cancel, got %v", err) + } + if sendCalled.Load() { + t.Fatal("Send must not be called when cost exceeds limit") + } +} diff --git a/pkg/storageincentives/redistributionstate.go b/pkg/storageincentives/redistributionstate.go index 9929a9e7831..c74a44464e3 100644 --- a/pkg/storageincentives/redistributionstate.go +++ b/pkg/storageincentives/redistributionstate.go @@ -61,6 +61,7 @@ type RoundData struct { CommitKey []byte SampleData *SampleData HasRevealed bool + RoundFees *big.Int } type SampleData struct { @@ -198,6 +199,38 @@ func (r *RedistributionState) AddFee(ctx context.Context, txHash common.Hash) { r.save() } +// AddRoundFee tracks fees spent in a specific round. +func (r *RedistributionState) AddRoundFee(ctx context.Context, round uint64, txHash common.Hash) { + fee, err := r.txService.TransactionFee(ctx, txHash) + if err != nil { + return + } + + r.mtx.Lock() + defer r.mtx.Unlock() + + rd := r.status.RoundData[round] + if rd.RoundFees == nil { + rd.RoundFees = new(big.Int) + } + rd.RoundFees.Add(rd.RoundFees, fee) + r.status.RoundData[round] = rd + r.status.Fees.Add(r.status.Fees, fee) + r.save() +} + +// RoundFees returns the total fees spent in a given round. +func (r *RedistributionState) RoundFees(round uint64) *big.Int { + r.mtx.Lock() + defer r.mtx.Unlock() + + rd, ok := r.status.RoundData[round] + if !ok || rd.RoundFees == nil { + return new(big.Int) + } + return new(big.Int).Set(rd.RoundFees) +} + // CalculateWinnerReward calculates the reward for the winner func (r *RedistributionState) CalculateWinnerReward(ctx context.Context) error { currentBalance, err := r.erc20Service.BalanceOf(ctx, r.ethAddress) diff --git a/pkg/transaction/mock/transaction.go b/pkg/transaction/mock/transaction.go index 072f47cf8f2..501f51dead7 100644 --- a/pkg/transaction/mock/transaction.go +++ b/pkg/transaction/mock/transaction.go @@ -18,6 +18,7 @@ import ( ) type transactionServiceMock struct { + estimateTxCost func(ctx context.Context, gasUnits int64, tip int) (cost *big.Int, gasFeeCap *big.Int, err error) send func(ctx context.Context, request *transaction.TxRequest, boost int) (txHash common.Hash, err error) waitForReceipt func(ctx context.Context, txHash common.Hash) (receipt *types.Receipt, err error) watchSentTransaction func(txHash common.Hash) (chan types.Receipt, chan error, error) @@ -29,6 +30,16 @@ type transactionServiceMock struct { transactionFee func(ctx context.Context, txHash common.Hash) (*big.Int, error) } +func (m *transactionServiceMock) EstimateTxCost(ctx context.Context, gasUnits int64, tip int) (cost *big.Int, gasFeeCap *big.Int, err error) { + if m.estimateTxCost != nil { + return m.estimateTxCost(ctx, gasUnits, tip) + } + // Default: small fee so callers that enable max-tx-cost checks can override via WithEstimateTxCostFunc. + gasFeeCap = big.NewInt(1) + cost = new(big.Int).Mul(big.NewInt(gasUnits), gasFeeCap) + return cost, gasFeeCap, nil +} + func (m *transactionServiceMock) Send(ctx context.Context, request *transaction.TxRequest, boostPercent int) (txHash common.Hash, err error) { if m.send != nil { return m.send(ctx, request, boostPercent) @@ -110,6 +121,12 @@ type optionFunc func(*transactionServiceMock) func (f optionFunc) apply(r *transactionServiceMock) { f(r) } +func WithEstimateTxCostFunc(f func(ctx context.Context, gasUnits int64, tip int) (cost *big.Int, gasFeeCap *big.Int, err error)) Option { + return optionFunc(func(s *transactionServiceMock) { + s.estimateTxCost = f + }) +} + func WithSendFunc(f func(context.Context, *transaction.TxRequest, int) (txHash common.Hash, err error)) Option { return optionFunc(func(s *transactionServiceMock) { s.send = f diff --git a/pkg/transaction/transaction.go b/pkg/transaction/transaction.go index 68fc74d6f53..ed1969ceb89 100644 --- a/pkg/transaction/transaction.go +++ b/pkg/transaction/transaction.go @@ -40,6 +40,7 @@ var ( ErrTransactionReverted = errors.New("transaction reverted") ErrUnknownTransaction = errors.New("unknown transaction") ErrAlreadyImported = errors.New("already imported") + ErrFeeCapExceeded = errors.New("suggested fee cap exceeds request fee cap") ) const ( @@ -81,6 +82,7 @@ type StoredTransaction struct { // limit and nonce management. type Service interface { io.Closer + EstimateTxCost(ctx context.Context, gasUnits int64, tip int) (cost *big.Int, gasFeeCap *big.Int, err error) // Send creates a transaction based on the request (with gasprice increased by provided percentage) and sends it. Send(ctx context.Context, request *TxRequest, tipCapBoostPercent int) (txHash common.Hash, err error) // Call simulate a transaction based on the request. @@ -172,6 +174,15 @@ func (t *transactionService) waitForAllPendingTx() error { return nil } +func (t *transactionService) EstimateTxCost(ctx context.Context, gasUnits int64, tip int) (cost *big.Int, gasFeeCap *big.Int, err error) { + gasFeeCap, _, err = t.backend.SuggestedFeeAndTip(ctx, nil, tip) + if err != nil { + return nil, nil, err + } + cost = new(big.Int).Mul(big.NewInt(gasUnits), gasFeeCap) + return cost, gasFeeCap, nil +} + // Send creates and signs a transaction based on the request and sends it. func (t *transactionService) Send(ctx context.Context, request *TxRequest, boostPercent int) (txHash common.Hash, err error) { loggerV1 := t.logger.V(1).Register() @@ -349,6 +360,18 @@ func (t *transactionService) prepareTransaction(ctx context.Context, request *Tx if err != nil { return nil, err } + if request.GasFeeCap != nil { + if request.GasFeeCap.Sign() <= 0 { + return nil, errors.New("gas fee cap must be greater than zero") + } + if gasFeeCap.Cmp(request.GasFeeCap) > 0 { + return nil, fmt.Errorf("%w: suggested=%s requested=%s", ErrFeeCapExceeded, gasFeeCap, request.GasFeeCap) + } + gasFeeCap = new(big.Int).Set(request.GasFeeCap) + if gasTipCap.Cmp(gasFeeCap) > 0 { + gasTipCap = new(big.Int).Set(gasFeeCap) + } + } t.logger.Debug("prepared transaction", "to", request.To, @@ -495,6 +518,15 @@ func (t *transactionService) ResendTransaction(ctx context.Context, txHash commo if err != nil { return err } + if storedTransaction.GasFeeCap != nil && gasFeeCap.Cmp(storedTransaction.GasFeeCap) > 0 { + gasFeeCap = new(big.Int).Set(storedTransaction.GasFeeCap) + } + if storedTransaction.GasTipCap != nil && gasTipCap.Cmp(storedTransaction.GasTipCap) > 0 { + gasTipCap = new(big.Int).Set(storedTransaction.GasTipCap) + } + if gasTipCap.Cmp(gasFeeCap) > 0 { + gasTipCap = new(big.Int).Set(gasFeeCap) + } tx := types.NewTx(&types.DynamicFeeTx{ Nonce: storedTransaction.Nonce, From 8032e8b2cf772a577b646068d05328a9f8e86e1c Mon Sep 17 00:00:00 2001 From: sbackend Date: Thu, 7 May 2026 13:58:07 +0200 Subject: [PATCH 2/7] fix: clean up --- pkg/storageincentives/agent.go | 5 - .../redistribution/redistribution.go | 117 ++++++++++-------- pkg/storageincentives/redistributionstate.go | 14 --- .../redistributionstate_test.go | 9 +- 4 files changed, 68 insertions(+), 77 deletions(-) diff --git a/pkg/storageincentives/agent.go b/pkg/storageincentives/agent.go index 2116033ff53..6e153ade98e 100644 --- a/pkg/storageincentives/agent.go +++ b/pkg/storageincentives/agent.go @@ -286,11 +286,6 @@ func (a *Agent) handleCommit(ctx context.Context, round uint64) error { err := a.commit(ctx, sample, round) if err != nil { - if errors.Is(err, redistribution.ErrMaxTxCostExceeded) { - a.logger.Info("skipping commit: tx cost exceeds configured max", "round", round) - a.metrics.SkippedExpensivePhase.Inc() - return nil - } return err } diff --git a/pkg/storageincentives/redistribution/redistribution.go b/pkg/storageincentives/redistribution/redistribution.go index 71456d0e4cf..5546b01580f 100644 --- a/pkg/storageincentives/redistribution/redistribution.go +++ b/pkg/storageincentives/redistribution/redistribution.go @@ -24,13 +24,16 @@ const ( loggerName = "redistributionContract" BoostTipPercent = 50 - retryBaseDelay = 5 * time.Second - claimRetryInterval = 15 * time.Second + claimRetryInterval = 15 * time.Second + minEstimatedGasLimit = 500_000 ) -// ErrMaxTxCostExceeded is returned when the upper-bound cost of a redistribution -// transaction (gas limit × max fee per gas) would exceed the configured limit. -var ErrMaxTxCostExceeded = errors.New("redistribution tx cost exceeds max tx cost limit") +var ( + // ErrMaxTxCostExceeded is returned when the upper-bound cost of a redistribution + // transaction (gas limit × max fee per gas) would exceed the configured limit. + ErrMaxTxCostExceeded = errors.New("redistribution tx cost exceeds max tx cost limit") + ErrZeroGasPrice = errors.New("gas price must be greater than zero") +) // ClaimOpts configures optional claim behaviour: after OverrideAfterBlock (absolute // chain block number), if ExpectedReward covers upper-bound claim cost plus @@ -64,6 +67,7 @@ type contract struct { maxTxCost *big.Int maxTxCostTolerancePercent uint64 gasLimit uint64 + blockTime time.Duration } // Option configures the redistribution contract wrapper. @@ -92,6 +96,7 @@ func New( postageContractAddress common.Address, postageContractABI abi.ABI, gasLimit uint64, + blockTime time.Duration, opts ...Option, ) Contract { c := &contract{ @@ -103,6 +108,7 @@ func New( incentivesContractABI: incentivesContractABI, postageContractAddress: postageContractAddress, postageContractABI: postageContractABI, + blockTime: blockTime, gasLimit: gasLimit, } for _, o := range opts { @@ -161,55 +167,52 @@ func (c *contract) Claim(ctx context.Context, proofs ChunkInclusionProofs, opts } for { - sendCtx, prepErr := c.prepareSendCtx(ctx, BoostTipPercent) - if prepErr == nil { - request := c.newTxRequest(sendCtx, callData, "claim win transaction") - txHash, sendErr := c.sendAndWaitWithRetry(sendCtx, request, BoostTipPercent) - if sendErr != nil { - return txHash, fmt.Errorf("claim: %w", sendErr) - } - return txHash, nil + ctx, err = c.prepareSendCtx(ctx, BoostTipPercent) + if err == nil { + break } - if !errors.Is(prepErr, ErrMaxTxCostExceeded) { - return common.Hash{}, fmt.Errorf("claim: %w", prepErr) + if errors.Is(err, ErrZeroGasPrice) { + return common.Hash{}, err } - if c.tryClaimOverride(opts) { - gasFeeCap, ok := c.canOverrideClaim(ctx, opts) - if ok { - c.logger.Warning("claim: max-tx-cost overridden", - "expected_reward", opts.ExpectedReward, - "round_fees", opts.RoundFees, - "override_after_block", opts.OverrideAfterBlock, - ) - overrideCtx := sctx.SetGasPrice(ctx, gasFeeCap) - request := c.newTxRequest(overrideCtx, callData, "claim win transaction (override)") - txHash, sendErr := c.sendAndWaitWithRetry(overrideCtx, request, BoostTipPercent) - if sendErr != nil { - return txHash, fmt.Errorf("claim: %w", sendErr) - } - return txHash, nil + gasFeeCap, ok := c.canOverrideClaim(ctx, opts) + if !ok { + c.logger.Info("claim: tx cost exceeds limit, waiting", "retry_in", claimRetryInterval) + select { + case <-ctx.Done(): + return common.Hash{}, ctx.Err() + case <-time.After(claimRetryInterval): + continue } } - c.logger.Info("claim: tx cost exceeds limit, waiting", "retry_in", claimRetryInterval) - select { - case <-ctx.Done(): - return common.Hash{}, ctx.Err() - case <-time.After(claimRetryInterval): - } + ctx = sctx.SetGasPrice(ctx, gasFeeCap) + c.logger.Warning("claim: max-tx-cost overridden", + "expected_reward", opts.ExpectedReward, + "round_fees", opts.RoundFees, + "override_after_block", opts.OverrideAfterBlock, + ) + break } -} - -func (c *contract) tryClaimOverride(opts *ClaimOpts) bool { - if opts == nil || opts.OverrideAfterBlock == 0 || opts.CurrentBlockFn == nil { - return false + request := c.newTxRequest(ctx, callData, "claim win transaction") + txHash, sendErr := c.sendAndWaitWithRetry(ctx, request, BoostTipPercent) + if sendErr != nil { + return txHash, fmt.Errorf("claim: %w", sendErr) } - return opts.CurrentBlockFn() >= opts.OverrideAfterBlock + return txHash, nil + } func (c *contract) canOverrideClaim(ctx context.Context, opts *ClaimOpts) (*big.Int, bool) { + if opts == nil || opts.OverrideAfterBlock == 0 || opts.CurrentBlockFn == nil || opts.RoundFees == nil { + return nil, false + } + + if opts.CurrentBlockFn() < opts.OverrideAfterBlock { + return nil, false + } + if opts.ExpectedReward == nil { reward, err := c.ExpectedReward(ctx) if err != nil { @@ -218,16 +221,13 @@ func (c *contract) canOverrideClaim(ctx context.Context, opts *ClaimOpts) (*big. opts.ExpectedReward = reward } - if opts.ExpectedReward == nil || opts.RoundFees == nil { - return nil, false - } - if opts.ExpectedReward.Sign() <= 0 { + if opts.ExpectedReward == nil || opts.ExpectedReward.Sign() <= 0 { return nil, false } gasUnits := int64(max(sctx.GetGasLimit(ctx), c.gasLimit)) if gasUnits <= 0 { - gasUnits = 500_000 + gasUnits = minEstimatedGasLimit } estimated, gasFeeCap, err := c.txService.EstimateTxCost(ctx, gasUnits, BoostTipPercent) @@ -254,16 +254,21 @@ func (c *contract) Commit(ctx context.Context, obfusHash []byte, round uint64) ( if err != nil { return common.Hash{}, err } + for { ctx, err = c.prepareSendCtx(ctx, BoostTipPercent) if err == nil { break } + if errors.Is(err, ErrZeroGasPrice) { + return common.Hash{}, err + } + select { case <-ctx.Done(): return common.Hash{}, err - case <-time.After(retryBaseDelay): + case <-time.After(c.blockTime): continue } } @@ -290,10 +295,14 @@ func (c *contract) Reveal(ctx context.Context, storageDepth uint8, reserveCommit break } + if errors.Is(err, ErrZeroGasPrice) { + return common.Hash{}, err + } + select { case <-ctx.Done(): return common.Hash{}, err - case <-time.After(retryBaseDelay): + case <-time.After(c.blockTime): continue } } @@ -352,7 +361,7 @@ func (c *contract) sendAndWaitWithRetry(ctx context.Context, request *transactio select { case <-ctx.Done(): return txHash, ctx.Err() - case <-time.After(retryBaseDelay): + case <-time.After(c.blockTime): continue } } @@ -386,7 +395,7 @@ func (c *contract) resendWithRetry(ctx context.Context, txHash common.Hash) (com select { case <-ctx.Done(): return txHash, ctx.Err() - case <-time.After(retryBaseDelay): + case <-time.After(c.blockTime): continue } } @@ -507,7 +516,7 @@ func (c *contract) newTxRequest(ctx context.Context, callData []byte, descriptio GasPrice: pinnedGasFeeCap, GasFeeCap: pinnedGasFeeCap, GasLimit: max(sctx.GetGasLimit(ctx), c.gasLimit), - MinEstimatedGasLimit: 500_000, + MinEstimatedGasLimit: minEstimatedGasLimit, Value: big.NewInt(0), Description: description, } @@ -516,14 +525,14 @@ func (c *contract) newTxRequest(ctx context.Context, callData []byte, descriptio func (c *contract) isTxCostAcceptable(ctx context.Context, tip int) (ok bool, estimated *big.Int, gasFeeCap *big.Int, err error) { gasUnits := int64(max(sctx.GetGasLimit(ctx), c.gasLimit)) if gasUnits <= 0 { - gasUnits = 500_000 + gasUnits = minEstimatedGasLimit } // If the caller pinned maxFeePerGas in context, use it for total-cost estimate // since that is the actual cap used when creating the transaction request. if pinnedGasFeeCap := sctx.GetGasPrice(ctx); pinnedGasFeeCap != nil { if pinnedGasFeeCap.Sign() <= 0 { - return false, nil, nil, errors.New("gas price must be greater than zero") + return false, nil, nil, ErrZeroGasPrice } gasFeeCap = new(big.Int).Set(pinnedGasFeeCap) estimated = new(big.Int).Mul(big.NewInt(gasUnits), gasFeeCap) diff --git a/pkg/storageincentives/redistributionstate.go b/pkg/storageincentives/redistributionstate.go index c74a44464e3..adbcbdcbcc3 100644 --- a/pkg/storageincentives/redistributionstate.go +++ b/pkg/storageincentives/redistributionstate.go @@ -185,20 +185,6 @@ func (r *RedistributionState) SetLastSelectedRound(round uint64) { r.save() } -// AddFee sets the internal node status -func (r *RedistributionState) AddFee(ctx context.Context, txHash common.Hash) { - fee, err := r.txService.TransactionFee(ctx, txHash) - if err != nil { - return - } - - r.mtx.Lock() - defer r.mtx.Unlock() - - r.status.Fees.Add(r.status.Fees, fee) - r.save() -} - // AddRoundFee tracks fees spent in a specific round. func (r *RedistributionState) AddRoundFee(ctx context.Context, round uint64, txHash common.Hash) { fee, err := r.txService.TransactionFee(ctx, txHash) diff --git a/pkg/storageincentives/redistributionstate_test.go b/pkg/storageincentives/redistributionstate_test.go index 9e2930642a4..318aca7e264 100644 --- a/pkg/storageincentives/redistributionstate_test.go +++ b/pkg/storageincentives/redistributionstate_test.go @@ -295,9 +295,10 @@ func TestReward(t *testing.T) { } } -// TestFee check if fees increments when called multiple times -func TestFee(t *testing.T) { +// TestRoundFee check if fees increments when called multiple times +func TestRoundFee(t *testing.T) { t.Parallel() + const round = 1 firstFee := big.NewInt(10) state := createRedistribution(t, nil, []transactionmock.Option{ transactionmock.WithTransactionFeeFunc(func(ctx context.Context, txHash common.Hash) (*big.Int, error) { @@ -305,7 +306,7 @@ func TestFee(t *testing.T) { }), }) ctx := context.Background() - state.AddFee(ctx, common.Hash{}) + state.AddRoundFee(ctx, round, common.Hash{}) gotFirstResult, err := state.Status() if err != nil { t.Fatal("failed to get status") @@ -320,7 +321,7 @@ func TestFee(t *testing.T) { }), }...) - state.AddFee(ctx, common.Hash{}) + state.AddRoundFee(ctx, round, common.Hash{}) gotSecondResult, err := state.Status() if err != nil { t.Fatal("failed to get status") From fff9a06e903316867d4106664aff003b82ee5a5e Mon Sep 17 00:00:00 2001 From: sbackend Date: Thu, 7 May 2026 22:40:14 +0200 Subject: [PATCH 3/7] fix: clean up + tests + metrics --- pkg/node/node.go | 3 +- .../redistribution/metrics.go | 163 +++++++++ .../redistribution/redistribution.go | 121 +++++- .../redistribution/redistribution_test.go | 346 ++++++++++++++++++ 4 files changed, 614 insertions(+), 19 deletions(-) create mode 100644 pkg/storageincentives/redistribution/metrics.go diff --git a/pkg/node/node.go b/pkg/node/node.go index db31b8f80ac..f6d5bf3235b 100644 --- a/pkg/node/node.go +++ b/pkg/node/node.go @@ -1207,7 +1207,7 @@ func NewBee( if o.MaxTxCost > 0 { redistributionOpts = append(redistributionOpts, redistribution.WithMaxTxCost(o.MaxTxCost, o.MaxTxCostTolerancePercent)) } - redistributionContract := redistribution.New(swarmAddress, overlayEthAddress, logger, transactionService, redistributionContractAddress, abiutil.MustParseABI(chainCfg.RedistributionABI), postageStampContractAddress, postageStampContractABI, contractGasLimit, redistributionOpts...) + redistributionContract := redistribution.New(swarmAddress, overlayEthAddress, logger, transactionService, redistributionContractAddress, abiutil.MustParseABI(chainCfg.RedistributionABI), postageStampContractAddress, postageStampContractABI, contractGasLimit, o.BlockTime, redistributionOpts...) isFullySynced := func() bool { reserveThreshold := reserveCapacity * 5 / 10 @@ -1292,6 +1292,7 @@ func NewBee( if agent != nil { apiService.MustRegisterMetrics(agent.Metrics()...) + apiService.MustRegisterMetrics(redistribution.Metrics()...) } apiService.MustRegisterMetrics(pushSyncProtocol.Metrics()...) diff --git a/pkg/storageincentives/redistribution/metrics.go b/pkg/storageincentives/redistribution/metrics.go new file mode 100644 index 00000000000..c79038742a2 --- /dev/null +++ b/pkg/storageincentives/redistribution/metrics.go @@ -0,0 +1,163 @@ +// Copyright 2026 The Swarm Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package redistribution + +import ( + "context" + "errors" + "strings" + + m "github.com/ethersphere/bee/v2/pkg/metrics" + "github.com/ethersphere/bee/v2/pkg/transaction" + "github.com/prometheus/client_golang/prometheus" +) + +// Tx kinds for metric labels (commit / reveal / claim). +const ( + TxKindCommit = "commit" + TxKindReveal = "reveal" + TxKindClaim = "claim" +) + +var ( + prepareSendRetries = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: m.Namespace, + Subsystem: "redistribution", + Name: "prepare_send_retries_total", + Help: "Waits before Send while estimated tx cost is above max-tx-cost (per tx kind).", + }, + []string{"tx_kind"}, + ) + + claimCostWaits = prometheus.NewCounter( + prometheus.CounterOpts{ + Namespace: m.Namespace, + Subsystem: "redistribution", + Name: "claim_cost_wait_iterations_total", + Help: "Iterations where Claim waited because estimated tx cost exceeded max-tx-cost (before override).", + }, + ) + + claimMaxTxCostOverrides = prometheus.NewCounter( + prometheus.CounterOpts{ + Namespace: m.Namespace, + Subsystem: "redistribution", + Name: "claim_max_tx_cost_overrides_total", + Help: "Times Claim bypassed max-tx-cost because expected reward covered estimated cost plus round fees.", + }, + ) + + phaseSkippedExpensive = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: m.Namespace, + Subsystem: "redistribution", + Name: "phase_skipped_expensive_total", + Help: "Phases stopped while fees exceeded user max-tx-cost (or claim context ended while waiting).", + }, + []string{"phase"}, + ) + + sendRetryStages = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: m.Namespace, + Subsystem: "redistribution", + Name: "send_retry_stages_total", + Help: "Send retry paths: empty_tx_hash_wait, resend_retry, resend_ok.", + }, + []string{"tx_kind", "stage"}, + ) + + txErrors = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: m.Namespace, + Subsystem: "redistribution", + Name: "errors_total", + Help: "Errors observed during redistribution txs (each failed Send/Resend/WaitForReceipt counts once).", + }, + []string{"tx_kind", "error_class"}, + ) +) + +func incPrepareSendRetry(txKind string) { + prepareSendRetries.WithLabelValues(txKind).Inc() +} + +func incClaimCostWait() { + claimCostWaits.Inc() +} + +func incClaimMaxTxCostOverride() { + claimMaxTxCostOverrides.Inc() +} + +func incPhaseSkippedExpensive(phase string) { + phaseSkippedExpensive.WithLabelValues(phase).Inc() +} + +func incSendRetryStage(txKind, stage string) { + sendRetryStages.WithLabelValues(txKind, stage).Inc() +} + +func incTxError(txKind string, err error) { + if err == nil { + return + } + txErrors.WithLabelValues(txKind, classifyErr(err)).Inc() +} + +// classifyErr maps errors to a small label set for dashboards (low cardinality). +func classifyErr(err error) string { + if err == nil { + return "none" + } + if errors.Is(err, context.Canceled) { + return "context_canceled" + } + if errors.Is(err, context.DeadlineExceeded) { + return "context_deadline" + } + if errors.Is(err, ErrMaxTxCostExceeded) { + return "max_tx_cost_exceeded" + } + if errors.Is(err, ErrZeroGasPrice) { + return "zero_gas_price" + } + if errors.Is(err, transaction.ErrFeeCapExceeded) { + return "fee_cap_exceeded" + } + if errors.Is(err, transaction.ErrTransactionReverted) { + return "tx_reverted" + } + if errors.Is(err, transaction.ErrTransactionCancelled) { + return "tx_cancelled" + } + + s := err.Error() + switch { + case strings.Contains(s, "below current base fee"): + return "below_base_fee" + case strings.Contains(s, "specified gas price"): + return "specified_gas_price" + case strings.Contains(s, "insufficient funds"): + return "insufficient_funds" + case strings.Contains(s, "execution reverted"): + return "execution_reverted" + default: + return "other" + } +} + +// Metrics returns Prometheus collectors for this package (register once). +func Metrics() []prometheus.Collector { + return []prometheus.Collector{ + prepareSendRetries, + claimCostWaits, + claimMaxTxCostOverrides, + phaseSkippedExpensive, + sendRetryStages, + txErrors, + } +} diff --git a/pkg/storageincentives/redistribution/redistribution.go b/pkg/storageincentives/redistribution/redistribution.go index 5546b01580f..0b6629e063b 100644 --- a/pkg/storageincentives/redistribution/redistribution.go +++ b/pkg/storageincentives/redistribution/redistribution.go @@ -160,12 +160,23 @@ func (c *contract) IsWinner(ctx context.Context) (isWinner bool, err error) { // configured max-tx-cost is exceeded, Claim waits claimRetryInterval and // retries instead of returning ErrMaxTxCostExceeded. After OverrideAfterBlock // (see ClaimOpts), if economics justify it, the limit is bypassed for one send. -func (c *contract) Claim(ctx context.Context, proofs ChunkInclusionProofs, opts *ClaimOpts) (common.Hash, error) { +func (c *contract) Claim(ctx context.Context, proofs ChunkInclusionProofs, opts *ClaimOpts) (txHash common.Hash, err error) { + defer func() { + if err == nil { + return + } + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) || errors.Is(err, ErrMaxTxCostExceeded) { + incPhaseSkippedExpensive(TxKindClaim) + } + }() + callData, err := c.incentivesContractABI.Pack("claim", proofs.A, proofs.B, proofs.C) if err != nil { + incTxError(TxKindClaim, err) return common.Hash{}, err } + var claimCostWait uint64 for { ctx, err = c.prepareSendCtx(ctx, BoostTipPercent) if err == nil { @@ -173,35 +184,46 @@ func (c *contract) Claim(ctx context.Context, proofs ChunkInclusionProofs, opts } if errors.Is(err, ErrZeroGasPrice) { + incTxError(TxKindClaim, err) return common.Hash{}, err } gasFeeCap, ok := c.canOverrideClaim(ctx, opts) if !ok { - c.logger.Info("claim: tx cost exceeds limit, waiting", "retry_in", claimRetryInterval) + claimCostWait++ + incClaimCostWait() + c.logger.Info("claim: tx cost exceeds limit, waiting", + "retry_in", claimRetryInterval, + "wait_iteration", claimCostWait, + "err", err, + ) select { case <-ctx.Done(): - return common.Hash{}, ctx.Err() + e := ctx.Err() + incTxError(TxKindClaim, e) + return common.Hash{}, e case <-time.After(claimRetryInterval): continue } } ctx = sctx.SetGasPrice(ctx, gasFeeCap) + incClaimMaxTxCostOverride() c.logger.Warning("claim: max-tx-cost overridden", "expected_reward", opts.ExpectedReward, "round_fees", opts.RoundFees, "override_after_block", opts.OverrideAfterBlock, + "current_block", opts.CurrentBlockFn(), + "pinned_max_fee_wei", gasFeeCap, ) break } request := c.newTxRequest(ctx, callData, "claim win transaction") - txHash, sendErr := c.sendAndWaitWithRetry(ctx, request, BoostTipPercent) + txHash, sendErr := c.sendAndWaitWithRetry(ctx, request, BoostTipPercent, TxKindClaim) if sendErr != nil { return txHash, fmt.Errorf("claim: %w", sendErr) } return txHash, nil - } func (c *contract) canOverrideClaim(ctx context.Context, opts *ClaimOpts) (*big.Int, bool) { @@ -249,12 +271,20 @@ func (c *contract) canOverrideClaim(ctx context.Context, opts *ClaimOpts) (*big. } // Commit submits the obfusHash hash by sending a transaction to the blockchain. -func (c *contract) Commit(ctx context.Context, obfusHash []byte, round uint64) (common.Hash, error) { +func (c *contract) Commit(ctx context.Context, obfusHash []byte, round uint64) (txHash common.Hash, err error) { + defer func() { + if err != nil && errors.Is(err, ErrMaxTxCostExceeded) { + incPhaseSkippedExpensive(TxKindCommit) + } + }() + callData, err := c.incentivesContractABI.Pack("commit", common.BytesToHash(obfusHash), round) if err != nil { + incTxError(TxKindCommit, err) return common.Hash{}, err } + var prepareAttempt uint64 for { ctx, err = c.prepareSendCtx(ctx, BoostTipPercent) if err == nil { @@ -262,11 +292,21 @@ func (c *contract) Commit(ctx context.Context, obfusHash []byte, round uint64) ( } if errors.Is(err, ErrZeroGasPrice) { + incTxError(TxKindCommit, err) return common.Hash{}, err } + prepareAttempt++ + incPrepareSendRetry(TxKindCommit) + c.logger.Debug("commit: prepare_send_ctx retry", + "attempt", prepareAttempt, + "wait", c.blockTime, + "err", err, + ) + select { case <-ctx.Done(): + incTxError(TxKindCommit, err) return common.Hash{}, err case <-time.After(c.blockTime): continue @@ -275,20 +315,27 @@ func (c *contract) Commit(ctx context.Context, obfusHash []byte, round uint64) ( request := c.newTxRequest(ctx, callData, "commit transaction") - txHash, err := c.sendAndWaitWithRetry(ctx, request, BoostTipPercent) + txHash, err = c.sendAndWaitWithRetry(ctx, request, BoostTipPercent, TxKindCommit) if err != nil { return txHash, fmt.Errorf("commit: obfusHash %v: %w", common.BytesToHash(obfusHash), err) } - return txHash, nil } // Reveal submits the storageDepth, reserveCommitmentHash and RandomNonce in a transaction to blockchain. -func (c *contract) Reveal(ctx context.Context, storageDepth uint8, reserveCommitmentHash []byte, RandomNonce []byte) (common.Hash, error) { +func (c *contract) Reveal(ctx context.Context, storageDepth uint8, reserveCommitmentHash []byte, RandomNonce []byte) (txHash common.Hash, err error) { + defer func() { + if err != nil && errors.Is(err, ErrMaxTxCostExceeded) { + incPhaseSkippedExpensive(TxKindReveal) + } + }() + callData, err := c.incentivesContractABI.Pack("reveal", storageDepth, common.BytesToHash(reserveCommitmentHash), common.BytesToHash(RandomNonce)) if err != nil { + incTxError(TxKindReveal, err) return common.Hash{}, err } + var prepareAttempt uint64 for { ctx, err = c.prepareSendCtx(ctx, BoostTipPercent) if err == nil { @@ -296,18 +343,28 @@ func (c *contract) Reveal(ctx context.Context, storageDepth uint8, reserveCommit } if errors.Is(err, ErrZeroGasPrice) { + incTxError(TxKindReveal, err) return common.Hash{}, err } + prepareAttempt++ + incPrepareSendRetry(TxKindReveal) + c.logger.Debug("reveal: prepare_send_ctx retry", + "attempt", prepareAttempt, + "wait", c.blockTime, + "err", err, + ) + select { case <-ctx.Done(): + incTxError(TxKindReveal, err) return common.Hash{}, err case <-time.After(c.blockTime): continue } } request := c.newTxRequest(ctx, callData, "reveal transaction") - txHash, err := c.sendAndWaitWithRetry(ctx, request, BoostTipPercent) + txHash, err = c.sendAndWaitWithRetry(ctx, request, BoostTipPercent, TxKindReveal) if err != nil { return txHash, fmt.Errorf("reveal: storageDepth %d reserveCommitmentHash %v RandomNonce %v: %w", storageDepth, common.BytesToHash(reserveCommitmentHash), common.BytesToHash(RandomNonce), err) } @@ -335,7 +392,7 @@ func (c *contract) ReserveSalt(ctx context.Context) ([]byte, error) { return salt[:], nil } -func (c *contract) sendAndWaitWithRetry(ctx context.Context, request *transaction.TxRequest, boostPercent int) (txHash common.Hash, err error) { +func (c *contract) sendAndWaitWithRetry(ctx context.Context, request *transaction.TxRequest, boostPercent int, txKind string) (txHash common.Hash, err error) { defer func() { err = c.txService.UnwrapABIError( ctx, @@ -345,56 +402,85 @@ func (c *contract) sendAndWaitWithRetry(ctx context.Context, request *transactio ) }() + var sendAttempt uint64 for { txHash, err = c.txService.Send(ctx, request, boostPercent) + sendAttempt++ if err == nil { break } + incTxError(txKind, err) + if isCritical(err) { return txHash, err } - c.logger.Warning("send failed, will retry", "error", err, "description", request.Description) + c.logger.Warning("send failed, will retry", + "tx_kind", txKind, + "attempt", sendAttempt, + "error", err, + "error_class", classifyErr(err), + "description", request.Description, + ) - if len(txHash.Bytes()) == 0 { + if txHash == (common.Hash{}) { + incSendRetryStage(txKind, "empty_tx_hash_wait") select { case <-ctx.Done(): - return txHash, ctx.Err() + e := ctx.Err() + incTxError(txKind, e) + return txHash, e case <-time.After(c.blockTime): continue } } - txHash, err = c.resendWithRetry(ctx, txHash) + txHash, err = c.resendWithRetry(ctx, txHash, txKind) if err != nil { return common.Hash{}, err } + incSendRetryStage(txKind, "resend_ok") break } receipt, err := c.txService.WaitForReceipt(ctx, txHash) if err != nil { + incTxError(txKind, err) return common.Hash{}, err } if receipt.Status == 0 { + incTxError(txKind, transaction.ErrTransactionReverted) return txHash, transaction.ErrTransactionReverted } return txHash, nil } -func (c *contract) resendWithRetry(ctx context.Context, txHash common.Hash) (common.Hash, error) { +func (c *contract) resendWithRetry(ctx context.Context, txHash common.Hash, txKind string) (common.Hash, error) { + var resendRound uint64 for { err := c.txService.ResendTransaction(ctx, txHash) if err != nil { + resendRound++ + incTxError(txKind, err) + incSendRetryStage(txKind, "resend_retry") + c.logger.Warning("resend transaction failed, will retry", + "tx_kind", txKind, + "round", resendRound, + "tx_hash", txHash, + "error", err, + "error_class", classifyErr(err), + ) if isCritical(err) { return txHash, err } select { case <-ctx.Done(): - return txHash, ctx.Err() + e := ctx.Err() + incTxError(txKind, e) + return txHash, e case <-time.After(c.blockTime): continue } @@ -417,7 +503,6 @@ func isCritical(err error) bool { s := err.Error() nonRetryable := []string{ "specified gas price", - "below current base fee", "AlreadyCommitted", "AlreadyRevealed", "AlreadyClaimed", diff --git a/pkg/storageincentives/redistribution/redistribution_test.go b/pkg/storageincentives/redistribution/redistribution_test.go index 08f5c5a1364..1253583d859 100644 --- a/pkg/storageincentives/redistribution/redistribution_test.go +++ b/pkg/storageincentives/redistribution/redistribution_test.go @@ -26,6 +26,7 @@ import ( transactionMock "github.com/ethersphere/bee/v2/pkg/transaction/mock" "github.com/ethersphere/bee/v2/pkg/util/abiutil" "github.com/ethersphere/bee/v2/pkg/util/testutil" + "github.com/stretchr/testify/assert" ) var ( @@ -97,6 +98,7 @@ func TestRedistribution(t *testing.T) { postageContractAddress, postageContractABI, 0, + 0, ) isPlaying, err := contract.IsPlaying(ctx, depth) @@ -131,6 +133,7 @@ func TestRedistribution(t *testing.T) { postageContractAddress, postageContractABI, 0, + 0, ) isPlaying, err := contract.IsPlaying(ctx, depth) @@ -163,6 +166,7 @@ func TestRedistribution(t *testing.T) { postageContractAddress, postageContractABI, 0, + 0, ) isWinner, err := contract.IsWinner(ctx) @@ -192,6 +196,7 @@ func TestRedistribution(t *testing.T) { postageContractAddress, postageContractABI, 0, + 0, ) isWinner, err := contract.IsWinner(ctx) @@ -240,6 +245,7 @@ func TestRedistribution(t *testing.T) { postageContractAddress, postageContractABI, 0, + 0, ) _, err = contract.Claim(ctx, proofs, nil) @@ -284,6 +290,7 @@ func TestRedistribution(t *testing.T) { postageContractAddress, postageContractABI, 0, + 0, ) _, err = contract.Claim(ctx, proofs, nil) @@ -329,6 +336,7 @@ func TestRedistribution(t *testing.T) { postageContractAddress, postageContractABI, 0, + 0, ) _, err = contract.Commit(ctx, testobfus, 0) @@ -376,6 +384,7 @@ func TestRedistribution(t *testing.T) { postageContractAddress, postageContractABI, 0, + 0, ) _, err = contract.Reveal(ctx, depth, common.Hex2Bytes("hash"), common.Hex2Bytes("nonce")) @@ -404,6 +413,7 @@ func TestRedistribution(t *testing.T) { postageContractAddress, postageContractABI, 0, + 0, ) salt, err := contract.ReserveSalt(ctx) @@ -436,6 +446,7 @@ func TestRedistribution(t *testing.T) { postageContractAddress, postageContractABI, 0, + 0, ) _, err := contract.IsPlaying(ctx, depth) @@ -471,6 +482,7 @@ func TestRedistribution(t *testing.T) { postageContractAddress, postageContractABI, 0, + 0, ) _, err = contract.Commit(ctx, common.Hex2Bytes("hash"), 0) @@ -514,6 +526,7 @@ func TestRedistribution_MaxTxCostWaitsUntilContextDone(t *testing.T) { postageContractAddress, postageContractABI, 100_000, + time.Millisecond, redistribution.WithMaxTxCost(500_000, 0), ) @@ -525,3 +538,336 @@ func TestRedistribution_MaxTxCostWaitsUntilContextDone(t *testing.T) { t.Fatal("Send must not be called when cost exceeds limit") } } + +const testShortBlockTime = 10 * time.Millisecond + +// 1. Commit waits while cost too high; after EstimateTxCost improves, tx is sent successfully. +func TestCommit_RetriesUntilCostAcceptableThenSuccess(t *testing.T) { + t.Parallel() + + ctx := context.Background() + owner := common.HexToAddress("abcd") + overlay := swarm.NewAddress(common.HexToHash("cbd").Bytes()) + redistributionContractAddress := common.HexToAddress("ffff") + testobfus := common.Hex2Bytes("hash") + var obfus [32]byte + copy(obfus[:], testobfus) + expectedHash := common.HexToHash("abc123") + + var estimateCalls atomic.Int32 + txSvc := transactionMock.New( + transactionMock.WithEstimateTxCostFunc(func(_ context.Context, gasUnits int64, _ int) (*big.Int, *big.Int, error) { + n := estimateCalls.Add(1) + var gasFeeCap *big.Int + if n == 1 { + gasFeeCap = big.NewInt(10) + } else { + gasFeeCap = big.NewInt(1) + } + cost := new(big.Int).Mul(big.NewInt(gasUnits), gasFeeCap) + return cost, gasFeeCap, nil + }), + transactionMock.WithSendFunc( + func(ctx context.Context, request *transaction.TxRequest, _ int) (common.Hash, error) { + assert.NotNil(t, request.GasFeeCap) + assert.Equal(t, 0, big.NewInt(1).Cmp(request.GasFeeCap), "fee cap must match last successful estimate") + + callData, err := redistributionContractABI.Pack("commit", obfus, uint64(0)) + assert.NoError(t, err) + assert.Equal(t, callData, request.Data) + return expectedHash, nil + }), + transactionMock.WithWaitForReceiptFunc(func(context.Context, common.Hash) (*types.Receipt, error) { + return &types.Receipt{Status: 1}, nil + }), + ) + + c := redistribution.New( + overlay, + owner, + log.Noop, + txSvc, + redistributionContractAddress, + redistributionContractABI, + postageContractAddress, + postageContractABI, + 0, + testShortBlockTime, + redistribution.WithMaxTxCost(500_000, 0), + ) + + h, err := c.Commit(ctx, testobfus, 0) + assert.NoError(t, err) + assert.Equal(t, expectedHash, h) + assert.EqualValues(t, 2, estimateCalls.Load(), "should re-estimate after blockTime wait") +} + +// 2. Commit sends a tx but receives a critical error → fails without endless retry. +func TestCommit_CriticalErrorFails(t *testing.T) { + t.Parallel() + + ctx := context.Background() + owner := common.HexToAddress("abcd") + overlay := swarm.NewAddress(common.HexToHash("cbd").Bytes()) + redistributionContractAddress := common.HexToAddress("ffff") + testobfus := common.Hex2Bytes("hash") + + txSvc := transactionMock.New( + transactionMock.WithSendFunc(func(context.Context, *transaction.TxRequest, int) (common.Hash, error) { + return common.Hash{}, transaction.ErrTransactionReverted + }), + ) + + c := redistribution.New( + overlay, + owner, + log.Noop, + txSvc, + redistributionContractAddress, + redistributionContractABI, + postageContractAddress, + postageContractABI, + 0, + testShortBlockTime, + ) + + _, err := c.Commit(ctx, testobfus, 0) + assert.Error(t, err) + assert.ErrorIs(t, err, transaction.ErrTransactionReverted) +} + +// First Send fails with non-critical error and zero tx hash; wait blockTime, retry Send -> success. +func TestCommit_RetriesAfterTransientSendFailure(t *testing.T) { + t.Parallel() + + ctx := context.Background() + owner := common.HexToAddress("abcd") + overlay := swarm.NewAddress(common.HexToHash("cbd").Bytes()) + redistributionContractAddress := common.HexToAddress("ffff") + testobfus := common.Hex2Bytes("hash") + expectedHash := common.HexToHash("deadbeef") + + var sendCalls atomic.Int32 + txSvc := transactionMock.New( + transactionMock.WithEstimateTxCostFunc(func(_ context.Context, gasUnits int64, _ int) (*big.Int, *big.Int, error) { + gasFeeCap := big.NewInt(2) + return new(big.Int).Mul(big.NewInt(gasUnits), gasFeeCap), gasFeeCap, nil + }), + transactionMock.WithSendFunc(func(context.Context, *transaction.TxRequest, int) (common.Hash, error) { + if sendCalls.Add(1) == 1 { + // Zero hash + non-critical error must trigger a second Send after blockTime. + return common.Hash{}, errors.New("temporary rpc failure") + } + return expectedHash, nil + }), + transactionMock.WithWaitForReceiptFunc(func(context.Context, common.Hash) (*types.Receipt, error) { + return &types.Receipt{Status: 1}, nil + }), + ) + + c := redistribution.New( + overlay, + owner, + log.Noop, + txSvc, + redistributionContractAddress, + redistributionContractABI, + postageContractAddress, + postageContractABI, + 0, + testShortBlockTime, + redistribution.WithMaxTxCost(2_000_000, 0), + ) + + h, err := c.Commit(ctx, testobfus, 0) + assert.NoError(t, err) + assert.Equal(t, expectedHash, h) + assert.EqualValues(t, 2, sendCalls.Load(), "second Send should happen after zero-hash failure") +} + +// 4. Commit never sends: cost stays above max-tx-cap until context is cancelled. +func TestCommit_contextCancelledWhileWaitingForAcceptableCost(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + owner := common.HexToAddress("abcd") + overlay := swarm.NewAddress(common.HexToHash("cbd").Bytes()) + redistributionContractAddress := common.HexToAddress("ffff") + + txSvc := transactionMock.New( + transactionMock.WithEstimateTxCostFunc(func(_ context.Context, gasUnits int64, _ int) (*big.Int, *big.Int, error) { + gasFeeCap := big.NewInt(100) + return new(big.Int).Mul(big.NewInt(gasUnits), gasFeeCap), gasFeeCap, nil + }), + transactionMock.WithSendFunc(func(context.Context, *transaction.TxRequest, int) (common.Hash, error) { + t.Fatal("Send must not be called") + return common.Hash{}, nil + }), + ) + + c := redistribution.New( + overlay, + owner, + log.Noop, + txSvc, + redistributionContractAddress, + redistributionContractABI, + postageContractAddress, + postageContractABI, + 0, + testShortBlockTime, + redistribution.WithMaxTxCost(500_000, 0), + ) + + _, err := c.Commit(ctx, common.Hex2Bytes("hash"), 0) + assert.ErrorIs(t, err, redistribution.ErrMaxTxCostExceeded) +} + +// No max-tx-cost limit: user pays whatever fee the network/request carries (nil caps on TxRequest). +func TestCommit_withoutMaxTxCostLeavesFeeCapsUnset(t *testing.T) { + t.Parallel() + + ctx := context.Background() + owner := common.HexToAddress("abcd") + overlay := swarm.NewAddress(common.HexToHash("cbd").Bytes()) + redistributionContractAddress := common.HexToAddress("ffff") + testobfus := common.Hex2Bytes("hash") + expectedHash := common.HexToHash("bbbb") + + txSvc := transactionMock.New( + transactionMock.WithSendFunc(func(_ context.Context, request *transaction.TxRequest, _ int) (common.Hash, error) { + assert.Nil(t, request.GasFeeCap) + assert.Nil(t, request.GasPrice) + return expectedHash, nil + }), + transactionMock.WithWaitForReceiptFunc(func(context.Context, common.Hash) (*types.Receipt, error) { + return &types.Receipt{Status: 1}, nil + }), + ) + + c := redistribution.New( + overlay, + owner, + log.Noop, + txSvc, + redistributionContractAddress, + redistributionContractABI, + postageContractAddress, + postageContractABI, + 0, + testShortBlockTime, + ) + + h, err := c.Commit(ctx, testobfus, 0) + assert.NoError(t, err) + assert.Equal(t, expectedHash, h) +} + +// 6. Claim hits max cost; there is 10 blocks remaining before phase end, expectedReward covers upper-bound cost + previous round fees → limit bypassed, tx sent. +func TestClaim_bypassesMaxTxCost(t *testing.T) { + t.Parallel() + + ctx := context.Background() + owner := common.HexToAddress("abcd") + overlay := swarm.NewAddress(common.HexToHash("cbd").Bytes()) + redistributionContractAddress := common.HexToAddress("ffff") + proofs := randChunkInclusionProofs(t) + expectedHash := common.HexToHash("cafe") + + var sendCalls atomic.Int32 + txSvc := transactionMock.New( + transactionMock.WithEstimateTxCostFunc(func(_ context.Context, gasUnits int64, _ int) (*big.Int, *big.Int, error) { + gasFeeCap := big.NewInt(20) + return new(big.Int).Mul(big.NewInt(gasUnits), gasFeeCap), gasFeeCap, nil + }), + transactionMock.WithSendFunc(func(_ context.Context, request *transaction.TxRequest, _ int) (common.Hash, error) { + sendCalls.Add(1) + callData, err := redistributionContractABI.Pack("claim", proofs.A, proofs.B, proofs.C) + assert.NoError(t, err) + assert.Equal(t, callData, request.Data) + assert.NotNil(t, request.GasFeeCap, "override should pin maxFeePerGas for send") + return expectedHash, nil + }), + transactionMock.WithWaitForReceiptFunc(func(context.Context, common.Hash) (*types.Receipt, error) { + return &types.Receipt{Status: 1}, nil + }), + ) + + c := redistribution.New( + overlay, + owner, + log.Noop, + txSvc, + redistributionContractAddress, + redistributionContractABI, + postageContractAddress, + postageContractABI, + 0, + testShortBlockTime, + redistribution.WithMaxTxCost(500_000, 0), + ) + + opts := &redistribution.ClaimOpts{ + OverrideAfterBlock: 100, + CurrentBlockFn: func() uint64 { return 110 }, + ExpectedReward: new(big.Int).Mul(big.NewInt(50), big.NewInt(1_000_000)), // >> estimated + fees + RoundFees: big.NewInt(100_000), + } + + h, err := c.Claim(ctx, proofs, opts) + assert.NoError(t, err) + assert.Equal(t, expectedHash, h) + assert.EqualValues(t, 1, sendCalls.Load()) +} + +// 7. Same high fees; expected reward does not cover cost — override denied; cancelled ctx exits without Send. +func TestClaim_noBypassWhenRewardTooSmall(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + owner := common.HexToAddress("abcd") + overlay := swarm.NewAddress(common.HexToHash("cbd").Bytes()) + redistributionContractAddress := common.HexToAddress("ffff") + proofs := randChunkInclusionProofs(t) + + var sendCalls atomic.Int32 + txSvc := transactionMock.New( + transactionMock.WithEstimateTxCostFunc(func(_ context.Context, gasUnits int64, _ int) (*big.Int, *big.Int, error) { + gasFeeCap := big.NewInt(50) + return new(big.Int).Mul(big.NewInt(gasUnits), gasFeeCap), gasFeeCap, nil + }), + transactionMock.WithSendFunc(func(context.Context, *transaction.TxRequest, int) (common.Hash, error) { + sendCalls.Add(1) + return common.Hash{}, errors.New("Send must not run") + }), + ) + + c := redistribution.New( + overlay, + owner, + log.Noop, + txSvc, + redistributionContractAddress, + redistributionContractABI, + postageContractAddress, + postageContractABI, + 0, + testShortBlockTime, + redistribution.WithMaxTxCost(500_000, 0), + ) + + opts := &redistribution.ClaimOpts{ + OverrideAfterBlock: 100, + CurrentBlockFn: func() uint64 { return 200 }, + ExpectedReward: big.NewInt(1000), + RoundFees: big.NewInt(1), + } + + _, err := c.Claim(ctx, proofs, opts) + assert.ErrorIs(t, err, context.Canceled) + assert.Zero(t, sendCalls.Load()) +} From a491b62b71688d34211c9348464c098902fdead2 Mon Sep 17 00:00:00 2001 From: sbackend Date: Tue, 12 May 2026 11:43:36 +0200 Subject: [PATCH 4/7] fix: testing --- cmd/bee/cmd/cmd.go | 17 +++++++++++++++-- go.mod | 2 +- go.sum | 4 ++-- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/cmd/bee/cmd/cmd.go b/cmd/bee/cmd/cmd.go index 8163997d9e4..1bf2dd20f4c 100644 --- a/cmd/bee/cmd/cmd.go +++ b/cmd/bee/cmd/cmd.go @@ -330,8 +330,21 @@ func (c *command) setAllFlags(cmd *cobra.Command) { cmd.Flags().Bool(optionSkipPostageSnapshot, false, "skip postage snapshot") cmd.Flags().Uint64(optionNameMinimumGasTipCap, 0, "minimum gas tip cap in wei for transactions, 0 means use suggested gas tip cap") cmd.Flags().Uint64(optionNameGasLimitFallback, 500_000, "gas limit fallback when estimation fails for contract transactions") - cmd.Flags().Uint64(optionNameMaxTxCost, 0, "maximum total cost in wei per redistribution transaction, 0 means no limit") - cmd.Flags().Uint64(optionNameMaxTxCostTolerancePercent, 5, "percentage above max-tx-cost within which the transaction is still allowed") + // Default 1.5e15 wei (= 0.0015 xDAI = 1.5 mxDAI) is calibrated against the + // upper-bound formula used by isTxCostAcceptable: + // estimated = gasUnits × (2·baseFee + boostedTip) + // where gasUnits = 500_000 (gas-limit-fallback when not overridden via ctx), + // boostedTip = suggestedTip × 1.5 (BoostTipPercent=50, ≈0.15 gwei on Gnosis). + // With max-tx-cost-tolerance-percent=10 the effective threshold is 1.65e15 wei, + // which permits a gasFeeCap of up to ≈3.3 gwei → roughly baseFee ≤ ~1.5 gwei. + // Observed network-wide effective gas prices over the last 10 rounds were + // 0.3–0.5 gwei in normal conditions and peaked at ≈1.5 gwei during a spike; + // this default keeps commit/reveal flowing through ordinary operation and + // only rejects sustained spikes above ≈1.5 gwei baseFee. Claim has its own + // override path (see ClaimOpts) and is unaffected by spikes when the + // expected reward covers the upper-bound cost. + cmd.Flags().Uint64(optionNameMaxTxCost, 1_500_000_000_000_000, "maximum total cost in wei per redistribution transaction (gas limit × max fee per gas); 0 means no limit. Default 1.5e15 wei (= 0.0015 xDAI = 1.5 mxDAI) is calibrated for typical Gnosis baseFee up to ~1.5 gwei with default gas-limit-fallback=500000 and 10% tolerance.") + cmd.Flags().Uint64(optionNameMaxTxCostTolerancePercent, 10, "percentage above max-tx-cost within which the transaction is still allowed (effective threshold = max-tx-cost × (1 + tol/100))") cmd.Flags().Bool(optionNameP2PWSSEnable, false, "Enable Secure WebSocket P2P connections") cmd.Flags().String(optionP2PWSSAddr, ":1635", "p2p wss address") cmd.Flags().String(optionNATWSSAddr, "", "WSS NAT exposed address") diff --git a/go.mod b/go.mod index 6885b1805d0..cdd45e7693e 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/ethereum/go-ethereum v1.16.9 github.com/ethersphere/batch-archive v0.0.6 github.com/ethersphere/go-price-oracle-abi v0.6.9 - github.com/ethersphere/go-storage-incentives-abi v0.9.4 + github.com/ethersphere/go-storage-incentives-abi v0.9.3-rc4 github.com/ethersphere/go-sw3-abi v0.6.9 github.com/ethersphere/langos v1.0.0 github.com/go-playground/validator/v10 v10.19.0 diff --git a/go.sum b/go.sum index d0fc255af2a..4b7ae9ac480 100644 --- a/go.sum +++ b/go.sum @@ -254,8 +254,8 @@ github.com/ethersphere/batch-archive v0.0.6 h1:Nt9mundj8CXT42qgGdq1sqKIVOk4KkKC4 github.com/ethersphere/batch-archive v0.0.6/go.mod h1:41BPb192NoK9CYjNB8BAE1J2MtiI/5aq0Wtas5O7A7Q= github.com/ethersphere/go-price-oracle-abi v0.6.9 h1:bseen6he3PZv5GHOm+KD6s4awaFmVSD9LFx+HpB6rCU= github.com/ethersphere/go-price-oracle-abi v0.6.9/go.mod h1:sI/Qj4/zJ23/b1enzwMMv0/hLTpPNVNacEwCWjo6yBk= -github.com/ethersphere/go-storage-incentives-abi v0.9.4 h1:mSIWXQXg5OQmH10QvXMV5w0vbSibFMaRlBL37gPLTM0= -github.com/ethersphere/go-storage-incentives-abi v0.9.4/go.mod h1:SXvJVtM4sEsaSKD0jc1ClpDLw8ErPoROZDme4Wrc/Nc= +github.com/ethersphere/go-storage-incentives-abi v0.9.3-rc4 h1:YK9FpiQz29ctU5V46CuwMt+4X5Xn8FTBwy6E2v/ix8s= +github.com/ethersphere/go-storage-incentives-abi v0.9.3-rc4/go.mod h1:SXvJVtM4sEsaSKD0jc1ClpDLw8ErPoROZDme4Wrc/Nc= github.com/ethersphere/go-sw3-abi v0.6.9 h1:TnWLnYkWE5UvC17mQBdUmdkzhPhO8GcqvWy4wvd1QJQ= github.com/ethersphere/go-sw3-abi v0.6.9/go.mod h1:BmpsvJ8idQZdYEtWnvxA8POYQ8Rl/NhyCdF0zLMOOJU= github.com/ethersphere/langos v1.0.0 h1:NBtNKzXTTRSue95uOlzPN4py7Aofs0xWPzyj4AI1Vcc= From 217c288f7768007d0b74ec1e3b59af32ec02dde5 Mon Sep 17 00:00:00 2001 From: sbackend Date: Thu, 21 May 2026 01:13:26 +0200 Subject: [PATCH 5/7] fix: integrate with issue 5114 --- pkg/api/api_test.go | 4 - pkg/node/node.go | 10 +- pkg/postage/postagecontract/contract.go | 14 + pkg/postage/postagecontract/mock/contract.go | 24 +- pkg/storageincentives/agent.go | 10 +- pkg/storageincentives/agent_test.go | 4 - .../redistribution/metrics.go | 163 ------- .../redistribution/redistribution.go | 401 ++---------------- .../redistribution/redistribution_test.go | 40 -- 9 files changed, 75 insertions(+), 595 deletions(-) delete mode 100644 pkg/storageincentives/redistribution/metrics.go diff --git a/pkg/api/api_test.go b/pkg/api/api_test.go index 2bc5f105a64..86fdc14c92b 100644 --- a/pkg/api/api_test.go +++ b/pkg/api/api_test.go @@ -773,10 +773,6 @@ func (m *mockContract) Reveal(context.Context, uint8, []byte, []byte) (common.Ha return common.Hash{}, nil } -func (m *mockContract) ExpectedReward(context.Context) (*big.Int, error) { - return big.NewInt(1_000_000), nil -} - type mockHealth struct{} func (m *mockHealth) IsHealthy() bool { return true } diff --git a/pkg/node/node.go b/pkg/node/node.go index 64ef664f89b..f2ba2ef9f68 100644 --- a/pkg/node/node.go +++ b/pkg/node/node.go @@ -199,8 +199,8 @@ type Options struct { WarmupTime time.Duration WelcomeMessage string WhitelistedWithdrawalAddress []string - MaxTxCost uint64 - MaxTxCostTolerancePercent uint64 + MaxTxCost uint64 + MaxTxCostTolerancePercent uint64 } func txRetryConfigFromOptions(o *Options) transaction.TransactionsRetryConfig { @@ -1267,11 +1267,7 @@ func NewBee( redistributionContractAddress = common.HexToAddress(o.RedistributionContractAddress) } - var redistributionOpts []redistribution.Option - if o.MaxTxCost > 0 { - redistributionOpts = append(redistributionOpts, redistribution.WithMaxTxCost(o.MaxTxCost, o.MaxTxCostTolerancePercent)) - } - redistributionContract := redistribution.New(swarmAddress, overlayEthAddress, logger, transactionService, redistributionContractAddress, abiutil.MustParseABI(chainCfg.RedistributionABI), postageStampContractAddress, postageStampContractABI, contractGasLimit, o.BlockTime, redistributionOpts...) + redistributionContract := redistribution.New(swarmAddress, overlayEthAddress, logger, transactionService, redistributionContractAddress, abiutil.MustParseABI(chainCfg.RedistributionABI), contractGasLimit) isFullySynced := func() bool { reserveThreshold := reserveCapacity * 5 / 10 diff --git a/pkg/postage/postagecontract/contract.go b/pkg/postage/postagecontract/contract.go index 62242c9630e..433469c6cb7 100644 --- a/pkg/postage/postagecontract/contract.go +++ b/pkg/postage/postagecontract/contract.go @@ -46,6 +46,7 @@ type Interface interface { TopUpBatch(ctx context.Context, batchID []byte, topupBalance *big.Int) (common.Hash, error) DiluteBatch(ctx context.Context, batchID []byte, newDepth uint8) (common.Hash, error) Paused(ctx context.Context) (bool, error) + ExpectedReward(ctx context.Context) (*big.Int, error) PostageBatchExpirer } @@ -338,6 +339,15 @@ func (c *postageContract) getProperty(ctx context.Context, propertyName string, return nil } +// ExpectedReward returns the current redistribution pot (totalPot) from the postage stamp contract. +func (c *postageContract) ExpectedReward(ctx context.Context) (*big.Int, error) { + pot := new(big.Int) + if err := c.getProperty(ctx, "totalPot", pot); err != nil { + return nil, fmt.Errorf("totalPot: %w", err) + } + return pot, nil +} + func (c *postageContract) getMinInitialBalance(ctx context.Context) (uint64, error) { var lastPrice uint64 err := c.getProperty(ctx, "lastPrice", &lastPrice) @@ -556,6 +566,10 @@ func (m *noOpPostageContract) ExpireBatches(context.Context) error { return ErrChainDisabled } +func (m *noOpPostageContract) ExpectedReward(context.Context) (*big.Int, error) { + return nil, ErrChainDisabled +} + func LookupERC20Address(ctx context.Context, transactionService transaction.Service, postageStampContractAddress common.Address, postageStampContractABI abi.ABI, chainEnabled bool) (common.Address, error) { if !chainEnabled { return common.Address{}, nil diff --git a/pkg/postage/postagecontract/mock/contract.go b/pkg/postage/postagecontract/mock/contract.go index c670cf62dce..fdaeceec0d1 100644 --- a/pkg/postage/postagecontract/mock/contract.go +++ b/pkg/postage/postagecontract/mock/contract.go @@ -13,11 +13,12 @@ import ( ) type contractMock struct { - createBatch func(ctx context.Context, initialBalance *big.Int, depth uint8, immutable bool, label string) (common.Hash, []byte, error) - topupBatch func(ctx context.Context, id []byte, amount *big.Int) (common.Hash, error) - diluteBatch func(ctx context.Context, id []byte, newDepth uint8) (common.Hash, error) - expireBatches func(ctx context.Context) error - paused func(ctx context.Context) (bool, error) + createBatch func(ctx context.Context, initialBalance *big.Int, depth uint8, immutable bool, label string) (common.Hash, []byte, error) + topupBatch func(ctx context.Context, id []byte, amount *big.Int) (common.Hash, error) + diluteBatch func(ctx context.Context, id []byte, newDepth uint8) (common.Hash, error) + expireBatches func(ctx context.Context) error + paused func(ctx context.Context) (bool, error) + expectedReward func(ctx context.Context) (*big.Int, error) } func (c *contractMock) CreateBatch(ctx context.Context, initialBalance *big.Int, depth uint8, immutable bool, label string) (common.Hash, []byte, error) { @@ -40,6 +41,13 @@ func (s *contractMock) Paused(ctx context.Context) (bool, error) { return s.paused(ctx) } +func (c *contractMock) ExpectedReward(ctx context.Context) (*big.Int, error) { + if c.expectedReward != nil { + return c.expectedReward(ctx) + } + return big.NewInt(1_000_000), nil +} + // Option is an option passed to New type Option func(*contractMock) @@ -83,3 +91,9 @@ func WithPaused(f func(ctx context.Context) (bool, error)) Option { mock.paused = f } } + +func WithExpectedRewardFunc(f func(ctx context.Context) (*big.Int, error)) Option { + return func(m *contractMock) { + m.expectedReward = f + } +} diff --git a/pkg/storageincentives/agent.go b/pkg/storageincentives/agent.go index 6e153ade98e..c76dee245b8 100644 --- a/pkg/storageincentives/agent.go +++ b/pkg/storageincentives/agent.go @@ -65,7 +65,7 @@ type Agent struct { blocksPerRound uint64 blockTime time.Duration contract redistribution.Contract - batchExpirer postagecontract.PostageBatchExpirer + postageContract postagecontract.Interface redistributionStatuser staking.RedistributionStatuser store storer.Reserve fullSyncedFunc func() bool @@ -83,7 +83,7 @@ func New(overlay swarm.Address, ethAddress common.Address, backend ChainBackend, contract redistribution.Contract, - batchExpirer postagecontract.PostageBatchExpirer, + postageContract postagecontract.Interface, redistributionStatuser staking.RedistributionStatuser, store storer.Reserve, fullSyncedFunc func() bool, @@ -103,7 +103,7 @@ func New(overlay swarm.Address, backend: backend, logger: logger.WithName(loggerName).Register(), contract: contract, - batchExpirer: batchExpirer, + postageContract: postageContract, store: store, fullSyncedFunc: fullSyncedFunc, blocksPerRound: blocksPerRound, @@ -350,7 +350,7 @@ func (a *Agent) handleClaim(ctx context.Context, round uint64) error { // In case when there are too many expired batches, Claim trx could runs out of gas. // To prevent this, node should first expire batches before Claiming a reward. - err = a.batchExpirer.ExpireBatches(ctx) + err = a.postageContract.ExpireBatches(ctx) if err != nil { a.logger.Info("expire batches failed", "err", err) // Even when error happens, proceed with claim handler @@ -385,7 +385,7 @@ func (a *Agent) handleClaim(ctx context.Context, round uint64) error { defer cancel() } - reward, err := a.contract.ExpectedReward(ctx) + reward, err := a.postageContract.ExpectedReward(ctx) if err != nil { a.logger.Warning("could not estimate claim reward, override will be disabled", "error", err) } diff --git a/pkg/storageincentives/agent_test.go b/pkg/storageincentives/agent_test.go index 53078bef3fb..cbc28d8a813 100644 --- a/pkg/storageincentives/agent_test.go +++ b/pkg/storageincentives/agent_test.go @@ -333,10 +333,6 @@ func (m *mockContract) Reveal(_ context.Context, r uint8, _ []byte, _ []byte) (c return common.Hash{}, nil } -func (m *mockContract) ExpectedReward(context.Context) (*big.Int, error) { - return big.NewInt(1_000_000), nil -} - type mockHealth struct{} func (m *mockHealth) IsHealthy() bool { return true } diff --git a/pkg/storageincentives/redistribution/metrics.go b/pkg/storageincentives/redistribution/metrics.go deleted file mode 100644 index c79038742a2..00000000000 --- a/pkg/storageincentives/redistribution/metrics.go +++ /dev/null @@ -1,163 +0,0 @@ -// Copyright 2026 The Swarm Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package redistribution - -import ( - "context" - "errors" - "strings" - - m "github.com/ethersphere/bee/v2/pkg/metrics" - "github.com/ethersphere/bee/v2/pkg/transaction" - "github.com/prometheus/client_golang/prometheus" -) - -// Tx kinds for metric labels (commit / reveal / claim). -const ( - TxKindCommit = "commit" - TxKindReveal = "reveal" - TxKindClaim = "claim" -) - -var ( - prepareSendRetries = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: m.Namespace, - Subsystem: "redistribution", - Name: "prepare_send_retries_total", - Help: "Waits before Send while estimated tx cost is above max-tx-cost (per tx kind).", - }, - []string{"tx_kind"}, - ) - - claimCostWaits = prometheus.NewCounter( - prometheus.CounterOpts{ - Namespace: m.Namespace, - Subsystem: "redistribution", - Name: "claim_cost_wait_iterations_total", - Help: "Iterations where Claim waited because estimated tx cost exceeded max-tx-cost (before override).", - }, - ) - - claimMaxTxCostOverrides = prometheus.NewCounter( - prometheus.CounterOpts{ - Namespace: m.Namespace, - Subsystem: "redistribution", - Name: "claim_max_tx_cost_overrides_total", - Help: "Times Claim bypassed max-tx-cost because expected reward covered estimated cost plus round fees.", - }, - ) - - phaseSkippedExpensive = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: m.Namespace, - Subsystem: "redistribution", - Name: "phase_skipped_expensive_total", - Help: "Phases stopped while fees exceeded user max-tx-cost (or claim context ended while waiting).", - }, - []string{"phase"}, - ) - - sendRetryStages = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: m.Namespace, - Subsystem: "redistribution", - Name: "send_retry_stages_total", - Help: "Send retry paths: empty_tx_hash_wait, resend_retry, resend_ok.", - }, - []string{"tx_kind", "stage"}, - ) - - txErrors = prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: m.Namespace, - Subsystem: "redistribution", - Name: "errors_total", - Help: "Errors observed during redistribution txs (each failed Send/Resend/WaitForReceipt counts once).", - }, - []string{"tx_kind", "error_class"}, - ) -) - -func incPrepareSendRetry(txKind string) { - prepareSendRetries.WithLabelValues(txKind).Inc() -} - -func incClaimCostWait() { - claimCostWaits.Inc() -} - -func incClaimMaxTxCostOverride() { - claimMaxTxCostOverrides.Inc() -} - -func incPhaseSkippedExpensive(phase string) { - phaseSkippedExpensive.WithLabelValues(phase).Inc() -} - -func incSendRetryStage(txKind, stage string) { - sendRetryStages.WithLabelValues(txKind, stage).Inc() -} - -func incTxError(txKind string, err error) { - if err == nil { - return - } - txErrors.WithLabelValues(txKind, classifyErr(err)).Inc() -} - -// classifyErr maps errors to a small label set for dashboards (low cardinality). -func classifyErr(err error) string { - if err == nil { - return "none" - } - if errors.Is(err, context.Canceled) { - return "context_canceled" - } - if errors.Is(err, context.DeadlineExceeded) { - return "context_deadline" - } - if errors.Is(err, ErrMaxTxCostExceeded) { - return "max_tx_cost_exceeded" - } - if errors.Is(err, ErrZeroGasPrice) { - return "zero_gas_price" - } - if errors.Is(err, transaction.ErrFeeCapExceeded) { - return "fee_cap_exceeded" - } - if errors.Is(err, transaction.ErrTransactionReverted) { - return "tx_reverted" - } - if errors.Is(err, transaction.ErrTransactionCancelled) { - return "tx_cancelled" - } - - s := err.Error() - switch { - case strings.Contains(s, "below current base fee"): - return "below_base_fee" - case strings.Contains(s, "specified gas price"): - return "specified_gas_price" - case strings.Contains(s, "insufficient funds"): - return "insufficient_funds" - case strings.Contains(s, "execution reverted"): - return "execution_reverted" - default: - return "other" - } -} - -// Metrics returns Prometheus collectors for this package (register once). -func Metrics() []prometheus.Collector { - return []prometheus.Collector{ - prepareSendRetries, - claimCostWaits, - claimMaxTxCostOverrides, - phaseSkippedExpensive, - sendRetryStages, - txErrors, - } -} diff --git a/pkg/storageincentives/redistribution/redistribution.go b/pkg/storageincentives/redistribution/redistribution.go index c12158f8cd1..159219d98db 100644 --- a/pkg/storageincentives/redistribution/redistribution.go +++ b/pkg/storageincentives/redistribution/redistribution.go @@ -6,11 +6,8 @@ package redistribution import ( "context" - "errors" "fmt" "math/big" - "strings" - "time" "github.com/ethereum/go-ethereum/accounts/abi" "github.com/ethereum/go-ethereum/common" @@ -21,21 +18,12 @@ import ( ) const ( - loggerName = "redistributionContract" - // BoostTipPercent is used where the node sends transactions without retry. + loggerName = "redistributionContract" BoostTipPercent = 50 - claimRetryInterval = 15 * time.Second minEstimatedGasLimit = 500_000 ) -var ( - // ErrMaxTxCostExceeded is returned when the upper-bound cost of a redistribution - // transaction (gas limit × max fee per gas) would exceed the configured limit. - ErrMaxTxCostExceeded = errors.New("redistribution tx cost exceeds max tx cost limit") - ErrZeroGasPrice = errors.New("gas price must be greater than zero") -) - // ClaimOpts configures optional claim behaviour: after OverrideAfterBlock (absolute // chain block number), if ExpectedReward covers upper-bound claim cost plus // RoundFees, the max-tx-cost limit is bypassed for that send attempt. @@ -53,7 +41,6 @@ type Contract interface { Claim(context.Context, ChunkInclusionProofs, *ClaimOpts) (common.Hash, error) Commit(context.Context, []byte, uint64) (common.Hash, error) Reveal(context.Context, uint8, []byte, []byte) (common.Hash, error) - ExpectedReward(ctx context.Context) (*big.Int, error) } type contract struct { @@ -63,28 +50,7 @@ type contract struct { txService transaction.Service incentivesContractAddress common.Address incentivesContractABI abi.ABI - postageContractAddress common.Address - postageContractABI abi.ABI - maxTxCost *big.Int - maxTxCostTolerancePercent uint64 gasLimit uint64 - blockTime time.Duration -} - -// Option configures the redistribution contract wrapper. -type Option func(*contract) - -// WithMaxTxCost sets the maximum total wei the node is willing to spend on a -// single redistribution transaction (gas limit × max fee per gas). When wei is -// zero, no limit is enforced. tolerancePercent expands the limit (e.g. 5 means -// allow up to 105% of wei); use 0 for a strict cap. -func WithMaxTxCost(wei uint64, tolerancePercent uint64) Option { - return func(c *contract) { - if wei > 0 { - c.maxTxCost = new(big.Int).SetUint64(wei) - c.maxTxCostTolerancePercent = tolerancePercent - } - } } func New( @@ -94,28 +60,17 @@ func New( txService transaction.Service, incentivesContractAddress common.Address, incentivesContractABI abi.ABI, - postageContractAddress common.Address, - postageContractABI abi.ABI, gasLimit uint64, - blockTime time.Duration, - opts ...Option, ) Contract { - c := &contract{ + return &contract{ overlay: overlay, owner: owner, logger: logger.WithName(loggerName).Register(), txService: txService, incentivesContractAddress: incentivesContractAddress, incentivesContractABI: incentivesContractABI, - postageContractAddress: postageContractAddress, - postageContractABI: postageContractABI, - blockTime: blockTime, gasLimit: gasLimit, } - for _, o := range opts { - o(c) - } - return c } // IsPlaying checks if the overlay is participating in the upcoming round. @@ -162,67 +117,22 @@ func (c *contract) IsWinner(ctx context.Context) (isWinner bool, err error) { // retries instead of returning ErrMaxTxCostExceeded. After OverrideAfterBlock // (see ClaimOpts), if economics justify it, the limit is bypassed for one send. func (c *contract) Claim(ctx context.Context, proofs ChunkInclusionProofs, opts *ClaimOpts) (txHash common.Hash, err error) { - defer func() { - if err == nil { - return - } - if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) || errors.Is(err, ErrMaxTxCostExceeded) { - incPhaseSkippedExpensive(TxKindClaim) - } - }() - callData, err := c.incentivesContractABI.Pack("claim", proofs.A, proofs.B, proofs.C) if err != nil { - incTxError(TxKindClaim, err) return common.Hash{}, err } - - var claimCostWait uint64 - for { - ctx, err = c.prepareSendCtx(ctx, BoostTipPercent) - if err == nil { - break - } - - if errors.Is(err, ErrZeroGasPrice) { - incTxError(TxKindClaim, err) - return common.Hash{}, err - } - - gasFeeCap, ok := c.canOverrideClaim(ctx, opts) - if !ok { - claimCostWait++ - incClaimCostWait() - c.logger.Info("claim: tx cost exceeds limit, waiting", - "retry_in", claimRetryInterval, - "wait_iteration", claimCostWait, - "err", err, - ) - select { - case <-ctx.Done(): - e := ctx.Err() - incTxError(TxKindClaim, e) - return common.Hash{}, e - case <-time.After(claimRetryInterval): - continue - } - } - - ctx = sctx.SetGasPrice(ctx, gasFeeCap) - incClaimMaxTxCostOverride() - c.logger.Warning("claim: max-tx-cost overridden", - "expected_reward", opts.ExpectedReward, - "round_fees", opts.RoundFees, - "override_after_block", opts.OverrideAfterBlock, - "current_block", opts.CurrentBlockFn(), - "pinned_max_fee_wei", gasFeeCap, - ) - break + request := &transaction.TxRequest{ + To: &c.incentivesContractAddress, + Data: callData, + GasPrice: sctx.GetGasPrice(ctx), + GasLimit: max(sctx.GetGasLimit(ctx), c.gasLimit), + MinEstimatedGasLimit: 500_000, + Value: big.NewInt(0), + Description: "claim win transaction", } - request := c.newTxRequest(ctx, callData, "claim win transaction") - txHash, sendErr := c.sendAndWaitWithRetry(ctx, request, BoostTipPercent, TxKindClaim) - if sendErr != nil { - return txHash, fmt.Errorf("claim: %w", sendErr) + txHash, err = c.sendAndWait(ctx, request) + if err != nil { + return txHash, fmt.Errorf("claim: %w", err) } return txHash, nil } @@ -236,15 +146,6 @@ func (c *contract) canOverrideClaim(ctx context.Context, opts *ClaimOpts) (*big. return nil, false } - if opts.ExpectedReward == nil { - reward, err := c.ExpectedReward(ctx) - if err != nil { - c.logger.Warning("error getting expected claim reward", "error", err) - } - opts.ExpectedReward = reward - } - txHash, err := c.sendAndWait(ctx, request) - if opts.ExpectedReward == nil || opts.ExpectedReward.Sign() <= 0 { return nil, false } @@ -273,101 +174,43 @@ func (c *contract) canOverrideClaim(ctx context.Context, opts *ClaimOpts) (*big. } // Commit submits the obfusHash hash by sending a transaction to the blockchain. -func (c *contract) Commit(ctx context.Context, obfusHash []byte, round uint64) (txHash common.Hash, err error) { - defer func() { - if err != nil && errors.Is(err, ErrMaxTxCostExceeded) { - incPhaseSkippedExpensive(TxKindCommit) - } - }() - +func (c *contract) Commit(ctx context.Context, obfusHash []byte, round uint64) (common.Hash, error) { callData, err := c.incentivesContractABI.Pack("commit", common.BytesToHash(obfusHash), round) if err != nil { - incTxError(TxKindCommit, err) return common.Hash{}, err } - - var prepareAttempt uint64 - for { - ctx, err = c.prepareSendCtx(ctx, BoostTipPercent) - if err == nil { - break - } - - if errors.Is(err, ErrZeroGasPrice) { - incTxError(TxKindCommit, err) - return common.Hash{}, err - } - - prepareAttempt++ - incPrepareSendRetry(TxKindCommit) - c.logger.Debug("commit: prepare_send_ctx retry", - "attempt", prepareAttempt, - "wait", c.blockTime, - "err", err, - ) - - select { - case <-ctx.Done(): - incTxError(TxKindCommit, err) - return common.Hash{}, err - case <-time.After(c.blockTime): - continue - } + request := &transaction.TxRequest{ + To: &c.incentivesContractAddress, + Data: callData, + GasPrice: sctx.GetGasPrice(ctx), + GasLimit: max(sctx.GetGasLimit(ctx), c.gasLimit), + MinEstimatedGasLimit: 500_000, + Value: big.NewInt(0), + Description: "commit transaction", } txHash, err := c.sendAndWait(ctx, request) - - request := c.newTxRequest(ctx, callData, "commit transaction") - - txHash, err = c.sendAndWaitWithRetry(ctx, request, BoostTipPercent, TxKindCommit) if err != nil { return txHash, fmt.Errorf("commit: obfusHash %v: %w", common.BytesToHash(obfusHash), err) } + return txHash, nil } // Reveal submits the storageDepth, reserveCommitmentHash and RandomNonce in a transaction to blockchain. -func (c *contract) Reveal(ctx context.Context, storageDepth uint8, reserveCommitmentHash []byte, RandomNonce []byte) (txHash common.Hash, err error) { - defer func() { - if err != nil && errors.Is(err, ErrMaxTxCostExceeded) { - incPhaseSkippedExpensive(TxKindReveal) - } - }() - +func (c *contract) Reveal(ctx context.Context, storageDepth uint8, reserveCommitmentHash []byte, RandomNonce []byte) (common.Hash, error) { callData, err := c.incentivesContractABI.Pack("reveal", storageDepth, common.BytesToHash(reserveCommitmentHash), common.BytesToHash(RandomNonce)) if err != nil { - incTxError(TxKindReveal, err) return common.Hash{}, err } - var prepareAttempt uint64 - for { - ctx, err = c.prepareSendCtx(ctx, BoostTipPercent) - if err == nil { - break - } - - if errors.Is(err, ErrZeroGasPrice) { - incTxError(TxKindReveal, err) - return common.Hash{}, err - } - - prepareAttempt++ - incPrepareSendRetry(TxKindReveal) - c.logger.Debug("reveal: prepare_send_ctx retry", - "attempt", prepareAttempt, - "wait", c.blockTime, - "err", err, - ) - - select { - case <-ctx.Done(): - incTxError(TxKindReveal, err) - return common.Hash{}, err - case <-time.After(c.blockTime): - continue - } + request := &transaction.TxRequest{ + To: &c.incentivesContractAddress, + Data: callData, + GasPrice: sctx.GetGasPrice(ctx), + GasLimit: max(sctx.GetGasLimit(ctx), c.gasLimit), + MinEstimatedGasLimit: 500_000, + Value: big.NewInt(0), + Description: "reveal transaction", } - request := c.newTxRequest(ctx, callData, "reveal transaction") - txHash, err = c.sendAndWaitWithRetry(ctx, request, BoostTipPercent, TxKindReveal) txHash, err := c.sendAndWait(ctx, request) if err != nil { return txHash, fmt.Errorf("reveal: storageDepth %d reserveCommitmentHash %v RandomNonce %v: %w", storageDepth, common.BytesToHash(reserveCommitmentHash), common.BytesToHash(RandomNonce), err) @@ -406,120 +249,8 @@ func (c *contract) sendAndWait(ctx context.Context, request *transaction.TxReque ) }() - txHash, receipt, err := c.txService.SendWithRetry(ctx, request) - if err != nil { - incTxError(txKind, err) - return common.Hash{}, err - } - if receipt == nil { - return txHash, fmt.Errorf("missing receipt after send with retry") - } - - return txHash, nil -} - -func (c *contract) resendWithRetry(ctx context.Context, txHash common.Hash, txKind string) (common.Hash, error) { - var resendRound uint64 - for { - err := c.txService.ResendTransaction(ctx, txHash) - if err != nil { - resendRound++ - incTxError(txKind, err) - incSendRetryStage(txKind, "resend_retry") - c.logger.Warning("resend transaction failed, will retry", - "tx_kind", txKind, - "round", resendRound, - "tx_hash", txHash, - "error", err, - "error_class", classifyErr(err), - ) - if isCritical(err) { - return txHash, err - } - select { - case <-ctx.Done(): - e := ctx.Err() - incTxError(txKind, e) - return txHash, e - case <-time.After(c.blockTime): - continue - } - } - return txHash, nil - } -} - -func isCritical(err error) bool { - if errors.Is(err, transaction.ErrTransactionReverted) { - return true - } - if errors.Is(err, transaction.ErrTransactionCancelled) { - return true - } - if errors.Is(err, transaction.ErrFeeCapExceeded) { - return true - } - - s := err.Error() - nonRetryable := []string{ - "specified gas price", - "AlreadyCommitted", - "AlreadyRevealed", - "AlreadyClaimed", - "NotCommitPhase", - "NotRevealPhase", - "NotClaimPhase", - "CommitRoundOver", - "CommitRoundNotStarted", - "PhaseLastBlock", - "OutOfDepth", - "OutOfDepthReveal", - "OutOfDepthClaim", - "NotStaked", - "MustStake2Rounds", - "NoReveals", - "NoCommitsReceived", - "execution reverted", - "insufficient funds", - } - for _, sub := range nonRetryable { - if strings.Contains(s, sub) { - return true - } - } - return false -} - -// ExpectedReward returns the current pot value from the PostageStamp contract via eth_call. -func (c *contract) ExpectedReward(ctx context.Context) (*big.Int, error) { - callData, err := c.postageContractABI.Pack("totalPot") - if err != nil { - return nil, fmt.Errorf("pack totalPot: %w", err) - } - - result, err := c.txService.Call(ctx, &transaction.TxRequest{ - To: &c.postageContractAddress, - Data: callData, - }) - if err != nil { - return nil, fmt.Errorf("call totalPot: %w", err) - } - - results, err := c.postageContractABI.Unpack("totalPot", result) - if err != nil { - return nil, fmt.Errorf("unpack totalPot: %w", err) - } - - if len(results) == 0 { - return nil, fmt.Errorf("totalPot returned no results") - } - - pot, ok := results[0].(*big.Int) - if !ok { - return nil, fmt.Errorf("totalPot unexpected type %T", results[0]) - } - - return pot, nil + txHash, _, err = c.txService.SendWithRetry(ctx, request) + return txHash, err } // callTx simulates a transaction based on tx request. @@ -533,67 +264,3 @@ func (c *contract) callTx(ctx context.Context, callData []byte) ([]byte, error) } return result, nil } - -// prepareSendCtx enforces max tx cost (when configured) and pins maxFeePerGas -// into ctx so the subsequent Send cannot use a higher fee than checked. -func (c *contract) prepareSendCtx(ctx context.Context, boostPercent int) (context.Context, error) { - if c.maxTxCost == nil { - return ctx, nil - } - ok, _, gasFeeCap, err := c.isTxCostAcceptable(ctx, boostPercent) - if err != nil { - return ctx, err - } - if !ok { - return ctx, ErrMaxTxCostExceeded - } - if gasFeeCap != nil { - ctx = sctx.SetGasPrice(ctx, gasFeeCap) - } - return ctx, nil -} - -func (c *contract) newTxRequest(ctx context.Context, callData []byte, description string) *transaction.TxRequest { - pinnedGasFeeCap := sctx.GetGasPrice(ctx) - return &transaction.TxRequest{ - To: &c.incentivesContractAddress, - Data: callData, - GasPrice: pinnedGasFeeCap, - GasFeeCap: pinnedGasFeeCap, - GasLimit: max(sctx.GetGasLimit(ctx), c.gasLimit), - MinEstimatedGasLimit: minEstimatedGasLimit, - Value: big.NewInt(0), - Description: description, - } -} - -func (c *contract) isTxCostAcceptable(ctx context.Context, tip int) (ok bool, estimated *big.Int, gasFeeCap *big.Int, err error) { - gasUnits := int64(max(sctx.GetGasLimit(ctx), c.gasLimit)) - if gasUnits <= 0 { - gasUnits = minEstimatedGasLimit - } - - // If the caller pinned maxFeePerGas in context, use it for total-cost estimate - // since that is the actual cap used when creating the transaction request. - if pinnedGasFeeCap := sctx.GetGasPrice(ctx); pinnedGasFeeCap != nil { - if pinnedGasFeeCap.Sign() <= 0 { - return false, nil, nil, ErrZeroGasPrice - } - gasFeeCap = new(big.Int).Set(pinnedGasFeeCap) - estimated = new(big.Int).Mul(big.NewInt(gasUnits), gasFeeCap) - } else { - estimated, gasFeeCap, err = c.txService.EstimateTxCost(ctx, gasUnits, tip) - if err != nil { - return false, nil, nil, err - } - } - - if c.maxTxCost == nil { - return true, estimated, gasFeeCap, nil - } - - tol := c.maxTxCostTolerancePercent - threshold := new(big.Int).Mul(c.maxTxCost, big.NewInt(int64(100+tol))) - threshold.Div(threshold, big.NewInt(100)) - return estimated.Cmp(threshold) <= 0, estimated, gasFeeCap, nil -} diff --git a/pkg/storageincentives/redistribution/redistribution_test.go b/pkg/storageincentives/redistribution/redistribution_test.go index c4b274055b0..ebe45d43a8e 100644 --- a/pkg/storageincentives/redistribution/redistribution_test.go +++ b/pkg/storageincentives/redistribution/redistribution_test.go @@ -31,8 +31,6 @@ import ( var ( redistributionContractABI = abiutil.MustParseABI(chaincfg.Testnet.RedistributionABI) - postageContractABI = abiutil.MustParseABI(chaincfg.Testnet.PostageStampABI) - postageContractAddress = common.HexToAddress("eeee") ) func randChunkInclusionProof(t *testing.T) redistribution.ChunkInclusionProof { @@ -95,8 +93,6 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, - postageContractAddress, - postageContractABI, 0, 0, ) @@ -130,8 +126,6 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, - postageContractAddress, - postageContractABI, 0, 0, ) @@ -163,8 +157,6 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, - postageContractAddress, - postageContractABI, 0, 0, ) @@ -193,8 +185,6 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, - postageContractAddress, - postageContractABI, 0, 0, ) @@ -234,8 +224,6 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, - postageContractAddress, - postageContractABI, 0, 0, ) @@ -271,8 +259,6 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, - postageContractAddress, - postageContractABI, 0, 0, ) @@ -309,8 +295,6 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, - postageContractAddress, - postageContractABI, 0, 0, ) @@ -349,8 +333,6 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, - postageContractAddress, - postageContractABI, 0, 0, ) @@ -378,8 +360,6 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, - postageContractAddress, - postageContractABI, 0, 0, ) @@ -411,8 +391,6 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, - postageContractAddress, - postageContractABI, 0, 0, ) @@ -450,8 +428,6 @@ func TestRedistribution(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, - postageContractAddress, - postageContractABI, 0, 0, ) @@ -494,8 +470,6 @@ func TestRedistribution_MaxTxCostWaitsUntilContextDone(t *testing.T) { ), redistributionContractAddress, redistributionContractABI, - postageContractAddress, - postageContractABI, 100_000, time.Millisecond, redistribution.WithMaxTxCost(500_000, 0), @@ -560,8 +534,6 @@ func TestCommit_RetriesUntilCostAcceptableThenSuccess(t *testing.T) { txSvc, redistributionContractAddress, redistributionContractABI, - postageContractAddress, - postageContractABI, 0, testShortBlockTime, redistribution.WithMaxTxCost(500_000, 0), @@ -596,8 +568,6 @@ func TestCommit_CriticalErrorFails(t *testing.T) { txSvc, redistributionContractAddress, redistributionContractABI, - postageContractAddress, - postageContractABI, 0, testShortBlockTime, ) @@ -643,8 +613,6 @@ func TestCommit_RetriesAfterTransientSendFailure(t *testing.T) { txSvc, redistributionContractAddress, redistributionContractABI, - postageContractAddress, - postageContractABI, 0, testShortBlockTime, redistribution.WithMaxTxCost(2_000_000, 0), @@ -685,8 +653,6 @@ func TestCommit_contextCancelledWhileWaitingForAcceptableCost(t *testing.T) { txSvc, redistributionContractAddress, redistributionContractABI, - postageContractAddress, - postageContractABI, 0, testShortBlockTime, redistribution.WithMaxTxCost(500_000, 0), @@ -725,8 +691,6 @@ func TestCommit_withoutMaxTxCostLeavesFeeCapsUnset(t *testing.T) { txSvc, redistributionContractAddress, redistributionContractABI, - postageContractAddress, - postageContractABI, 0, testShortBlockTime, ) @@ -773,8 +737,6 @@ func TestClaim_bypassesMaxTxCost(t *testing.T) { txSvc, redistributionContractAddress, redistributionContractABI, - postageContractAddress, - postageContractABI, 0, testShortBlockTime, redistribution.WithMaxTxCost(500_000, 0), @@ -824,8 +786,6 @@ func TestClaim_noBypassWhenRewardTooSmall(t *testing.T) { txSvc, redistributionContractAddress, redistributionContractABI, - postageContractAddress, - postageContractABI, 0, testShortBlockTime, redistribution.WithMaxTxCost(500_000, 0), From 01ded1ea238b63ce75aec4802e2b4955e1737638 Mon Sep 17 00:00:00 2001 From: sbackend Date: Thu, 21 May 2026 14:47:58 +0200 Subject: [PATCH 6/7] fix: refactoring, new tests --- pkg/storageincentives/agent.go | 2 +- .../redistribution/redistribution.go | 72 ++++---- pkg/transaction/export_test.go | 3 +- pkg/transaction/mock/transaction.go | 15 +- pkg/transaction/send_tx_with_retry.go | 89 +++++++-- pkg/transaction/send_tx_with_retry_test.go | 173 +++++++++++++++++- pkg/transaction/transaction.go | 5 +- 7 files changed, 303 insertions(+), 56 deletions(-) diff --git a/pkg/storageincentives/agent.go b/pkg/storageincentives/agent.go index c76dee245b8..bb9341851f3 100644 --- a/pkg/storageincentives/agent.go +++ b/pkg/storageincentives/agent.go @@ -387,7 +387,7 @@ func (a *Agent) handleClaim(ctx context.Context, round uint64) error { reward, err := a.postageContract.ExpectedReward(ctx) if err != nil { - a.logger.Warning("could not estimate claim reward, override will be disabled", "error", err) + a.logger.Warning("could not estimate claim reward, override max_tx_cost option will be disabled", "error", err) } opts := &redistribution.ClaimOpts{ diff --git a/pkg/storageincentives/redistribution/redistribution.go b/pkg/storageincentives/redistribution/redistribution.go index 159219d98db..8fb09b280cb 100644 --- a/pkg/storageincentives/redistribution/redistribution.go +++ b/pkg/storageincentives/redistribution/redistribution.go @@ -12,7 +12,6 @@ import ( "github.com/ethereum/go-ethereum/accounts/abi" "github.com/ethereum/go-ethereum/common" "github.com/ethersphere/bee/v2/pkg/log" - "github.com/ethersphere/bee/v2/pkg/sctx" "github.com/ethersphere/bee/v2/pkg/swarm" "github.com/ethersphere/bee/v2/pkg/transaction" ) @@ -21,7 +20,7 @@ const ( loggerName = "redistributionContract" BoostTipPercent = 50 - minEstimatedGasLimit = 500_000 + minEstimatedGasLimit = 250_000 ) // ClaimOpts configures optional claim behaviour: after OverrideAfterBlock (absolute @@ -112,10 +111,11 @@ func (c *contract) IsWinner(ctx context.Context) (isWinner bool, err error) { return results[0].(bool), nil } -// Claim sends a transaction to blockchain if a win is claimed. When the -// configured max-tx-cost is exceeded, Claim waits claimRetryInterval and -// retries instead of returning ErrMaxTxCostExceeded. After OverrideAfterBlock -// (see ClaimOpts), if economics justify it, the limit is bypassed for one send. +// Claim sends a transaction to blockchain if a win is claimed. When opts is +// non-nil and the configured max-tx-price would block the broadcast, +// canOverrideClaim is consulted: if the override block threshold has passed +// and ExpectedReward covers the estimated cost plus previous round fees, +// the price cap is bypassed. func (c *contract) Claim(ctx context.Context, proofs ChunkInclusionProofs, opts *ClaimOpts) (txHash common.Hash, err error) { callData, err := c.incentivesContractABI.Pack("claim", proofs.A, proofs.B, proofs.C) if err != nil { @@ -124,53 +124,59 @@ func (c *contract) Claim(ctx context.Context, proofs ChunkInclusionProofs, opts request := &transaction.TxRequest{ To: &c.incentivesContractAddress, Data: callData, - GasPrice: sctx.GetGasPrice(ctx), - GasLimit: max(sctx.GetGasLimit(ctx), c.gasLimit), - MinEstimatedGasLimit: 500_000, + GasLimit: c.gasLimit, + MinEstimatedGasLimit: minEstimatedGasLimit, Value: big.NewInt(0), Description: "claim win transaction", } - txHash, err = c.sendAndWait(ctx, request) + + retryOpts := []transaction.RetryOption{ + transaction.WithIgnoreMaxPrice(func(gasFeeCap *big.Int) bool { + return c.canOverrideClaim(opts, gasFeeCap) + }), + } + + txHash, err = c.sendAndWait(ctx, request, retryOpts...) if err != nil { return txHash, fmt.Errorf("claim: %w", err) } return txHash, nil } -func (c *contract) canOverrideClaim(ctx context.Context, opts *ClaimOpts) (*big.Int, bool) { +// canOverrideClaim decides whether the claim transaction should bypass the +// max-tx-price cap. gasFeeCap is the actual max fee per gas (wei) that the +// retry loop wants to use — it is provided by suggestGasFeeGasTipCapWithHistory +// so there is no redundant estimation. +func (c *contract) canOverrideClaim(opts *ClaimOpts, gasFeeCap *big.Int) bool { if opts == nil || opts.OverrideAfterBlock == 0 || opts.CurrentBlockFn == nil || opts.RoundFees == nil { - return nil, false + return false } if opts.CurrentBlockFn() < opts.OverrideAfterBlock { - return nil, false + return false } if opts.ExpectedReward == nil || opts.ExpectedReward.Sign() <= 0 { - return nil, false + return false } - gasUnits := int64(max(sctx.GetGasLimit(ctx), c.gasLimit)) + gasUnits := c.gasLimit if gasUnits <= 0 { gasUnits = minEstimatedGasLimit } - estimated, gasFeeCap, err := c.txService.EstimateTxCost(ctx, gasUnits, BoostTipPercent) - if err != nil { - c.logger.Warning("claim override: estimate failed", "error", err) - return nil, false - } - - totalSpent := new(big.Int).Add(estimated, opts.RoundFees) + txCost := new(big.Int).Mul(gasFeeCap, big.NewInt(int64(gasUnits))) + totalSpent := new(big.Int).Add(txCost, opts.RoundFees) if opts.ExpectedReward.Cmp(totalSpent) < 0 { - c.logger.Info("claim override: reward does not cover upper-bound cost", - "estimated", estimated, + c.logger.Info("claim override: reward does not cover cost", + "tx_cost", txCost, "round_fees", opts.RoundFees, + "total_spent", totalSpent, "expected_reward", opts.ExpectedReward, ) - return nil, false + return false } - return gasFeeCap, true + return true } // Commit submits the obfusHash hash by sending a transaction to the blockchain. @@ -182,9 +188,8 @@ func (c *contract) Commit(ctx context.Context, obfusHash []byte, round uint64) ( request := &transaction.TxRequest{ To: &c.incentivesContractAddress, Data: callData, - GasPrice: sctx.GetGasPrice(ctx), - GasLimit: max(sctx.GetGasLimit(ctx), c.gasLimit), - MinEstimatedGasLimit: 500_000, + GasLimit: c.gasLimit, + MinEstimatedGasLimit: minEstimatedGasLimit, Value: big.NewInt(0), Description: "commit transaction", } @@ -205,9 +210,8 @@ func (c *contract) Reveal(ctx context.Context, storageDepth uint8, reserveCommit request := &transaction.TxRequest{ To: &c.incentivesContractAddress, Data: callData, - GasPrice: sctx.GetGasPrice(ctx), - GasLimit: max(sctx.GetGasLimit(ctx), c.gasLimit), - MinEstimatedGasLimit: 500_000, + GasLimit: c.gasLimit, + MinEstimatedGasLimit: minEstimatedGasLimit, Value: big.NewInt(0), Description: "reveal transaction", } @@ -239,7 +243,7 @@ func (c *contract) ReserveSalt(ctx context.Context) ([]byte, error) { return salt[:], nil } -func (c *contract) sendAndWait(ctx context.Context, request *transaction.TxRequest) (txHash common.Hash, err error) { +func (c *contract) sendAndWait(ctx context.Context, request *transaction.TxRequest, opts ...transaction.RetryOption) (txHash common.Hash, err error) { defer func() { err = c.txService.UnwrapABIError( ctx, @@ -249,7 +253,7 @@ func (c *contract) sendAndWait(ctx context.Context, request *transaction.TxReque ) }() - txHash, _, err = c.txService.SendWithRetry(ctx, request) + txHash, _, err = c.txService.SendWithRetry(ctx, request, opts...) return txHash, err } diff --git a/pkg/transaction/export_test.go b/pkg/transaction/export_test.go index 8b6a9070fdb..4cf9af5246a 100644 --- a/pkg/transaction/export_test.go +++ b/pkg/transaction/export_test.go @@ -25,6 +25,7 @@ func SuggestGasFeeGasTipCapWithHistory( maxTxPrice *big.Int, ctx context.Context, prevGasTipCap *big.Int, + overrides *RetryOverrides, ) (gasFeeCap, gasTipCap *big.Int, err error) { svc := &transactionService{ logger: log.Noop, @@ -32,5 +33,5 @@ func SuggestGasFeeGasTipCapWithHistory( txRetryGasIncreasePercent: gasIncreasePercent, maxTxPrice: maxTxPrice, } - return svc.suggestGasFeeGasTipCapWithHistory(ctx, prevGasTipCap) + return svc.suggestGasFeeGasTipCapWithHistory(ctx, prevGasTipCap, overrides) } diff --git a/pkg/transaction/mock/transaction.go b/pkg/transaction/mock/transaction.go index 2d85b03a055..13886b74796 100644 --- a/pkg/transaction/mock/transaction.go +++ b/pkg/transaction/mock/transaction.go @@ -31,13 +31,20 @@ type transactionServiceMock struct { transactionFee func(ctx context.Context, txHash common.Hash) (*big.Int, error) } -func (m *transactionServiceMock) SendWithRetry(ctx context.Context, request *transaction.TxRequest) (common.Hash, *types.Receipt, error) { +func (m *transactionServiceMock) SendWithRetry(ctx context.Context, request *transaction.TxRequest, _ ...transaction.RetryOption) (common.Hash, *types.Receipt, error) { if m.sendWithRetry != nil { return m.sendWithRetry(ctx, request) } return common.Hash{}, nil, errors.New("not implemented") } +func (m *transactionServiceMock) EstimateTxCost(ctx context.Context, gasUnits int64, tip int) (*big.Int, *big.Int, error) { + if m.estimateTxCost != nil { + return m.estimateTxCost(ctx, gasUnits, tip) + } + return big.NewInt(0), big.NewInt(0), nil +} + func (m *transactionServiceMock) Send(ctx context.Context, request *transaction.TxRequest, boostPercent int) (txHash common.Hash, err error) { if m.send != nil { return m.send(ctx, request, boostPercent) @@ -125,6 +132,12 @@ func WithSendWithRetryFunc(f func(context.Context, *transaction.TxRequest) (comm }) } +func WithEstimateTxCostFunc(f func(context.Context, int64, int) (*big.Int, *big.Int, error)) Option { + return optionFunc(func(s *transactionServiceMock) { + s.estimateTxCost = f + }) +} + func WithSendFunc(f func(context.Context, *transaction.TxRequest, int) (txHash common.Hash, err error)) Option { return optionFunc(func(s *transactionServiceMock) { s.send = f diff --git a/pkg/transaction/send_tx_with_retry.go b/pkg/transaction/send_tx_with_retry.go index a7dcbfa0377..fda9936451e 100644 --- a/pkg/transaction/send_tx_with_retry.go +++ b/pkg/transaction/send_tx_with_retry.go @@ -19,6 +19,46 @@ import ( const retryStatePrefix = "transaction_retry_" +// RetryOverrides controls per-call behaviour overrides for SendWithRetry. +// Fields are optional; nil means "use default behaviour". +type RetryOverrides struct { + // IgnoreMaxPrice is called when maxTxPrice would block a broadcast. + // It receives the gasFeeCap (max fee per gas, wei) that would be used + // for this attempt. If it returns true, the price cap is bypassed. + IgnoreMaxPrice func(gasFeeCap *big.Int) bool + + // RetryDelay, if set, overrides the configured delay between attempts. + RetryDelay func(attempt int) time.Duration +} + +// RetryOption configures per-call overrides for SendWithRetry. +type RetryOption func(*RetryOverrides) + +// WithIgnoreMaxPrice returns a RetryOption that installs a predicate called +// whenever the configured maxTxPrice would block a broadcast. The predicate +// receives the gasFeeCap (max fee per gas, wei) that would be used. When fn +// returns true the price cap is bypassed for that attempt. +func WithIgnoreMaxPrice(fn func(gasFeeCap *big.Int) bool) RetryOption { + return func(o *RetryOverrides) { o.IgnoreMaxPrice = fn } +} + +// WithRetryDelay returns a RetryOption that overrides the configured +// per-attempt delay. +func WithRetryDelay(fn func(attempt int) time.Duration) RetryOption { + return func(o *RetryOverrides) { o.RetryDelay = fn } +} + +func applyRetryOptions(opts []RetryOption) *RetryOverrides { + if len(opts) == 0 { + return nil + } + var o RetryOverrides + for _, fn := range opts { + fn(&o) + } + return &o +} + // TransactionRetryState is persisted so transactions with retry can resume after a node restart. type TransactionRetryState struct { Nonce uint64 `json:"nonce"` @@ -43,13 +83,14 @@ func retryStateKey(nonce uint64) string { // SendWithRetry sends an EIP-1559 transaction using one eth_feeHistory snapshot for the initial tip, // then increases gas tip by gas_increase_percent after each unsuccessful wait, up to max_retries. -func (t *transactionService) SendWithRetry(ctx context.Context, request *TxRequest) (txHash common.Hash, receipt *types.Receipt, err error) { +// Optional RetryOption values can override per-call retry behaviour (e.g. bypass price cap). +func (t *transactionService) SendWithRetry(ctx context.Context, request *TxRequest, opts ...RetryOption) (txHash common.Hash, receipt *types.Receipt, err error) { if request.GasPrice != nil { err = errors.New("send txs with retry requires automatic gas pricing") t.recordRetryComplete(0, err) return common.Hash{}, nil, err } - return t.retry(ctx, "", request) + return t.retry(ctx, "", request, applyRetryOptions(opts)) } // escalateGasTip returns tip * (100+increasePct)/100 — a single escalation step. @@ -72,7 +113,9 @@ func escalateGasTip(tip *big.Int, increasePct int) *big.Int { // When maxTxPrice is set and 2*baseFee + escalated tip exceeds it, the function broadcasts with the // un-escalated previous tip (2*baseFee + prevGasTipCap) instead. If that fee cap still exceeds // maxTxPrice, it returns ErrTxMaxPriceExceeded. -func (t *transactionService) suggestGasFeeGasTipCapWithHistory(ctx context.Context, prevGasTipCap *big.Int) (gasFeeCap, gasTipCap *big.Int, err error) { +// +// When overrides.IgnoreMaxPrice is set and returns true, the maxTxPrice cap is bypassed. +func (t *transactionService) suggestGasFeeGasTipCapWithHistory(ctx context.Context, prevGasTipCap *big.Int, overrides *RetryOverrides) (gasFeeCap, gasTipCap *big.Int, err error) { header, err := t.backend.HeaderByNumber(ctx, nil) if err != nil { return nil, prevGasTipCap, err @@ -108,7 +151,18 @@ func (t *transactionService) suggestGasFeeGasTipCapWithHistory(ctx context.Conte "gas_fee_cap_with_previous_tip", gasFeeCapWithPreviousTip, "max_tx_price", t.maxTxPrice) + canOverride := func(feeCap *big.Int) bool { + return overrides != nil && overrides.IgnoreMaxPrice != nil && overrides.IgnoreMaxPrice(feeCap) + } + if t.maxTxPrice != nil && gasFeeCapWithEscalatedTip.Cmp(t.maxTxPrice) > 0 { + if canOverride(gasFeeCapWithEscalatedTip) { + t.logger.Info("max price override: bypassing limit", + "escalated_gas_fee_cap", gasFeeCapWithEscalatedTip, + "max_tx_price", t.maxTxPrice) + return gasFeeCapWithEscalatedTip, escalatedGasTip, nil + } + t.logger.Warning("gas cap fee with escalated gas tip is too high, fallback to previous gas tip", "escalated_gas_tip_cap", escalatedGasTip.String(), "escalated_gas_fee_cap", gasFeeCapWithEscalatedTip.String(), @@ -122,13 +176,13 @@ func (t *transactionService) suggestGasFeeGasTipCapWithHistory(ctx context.Conte return gasFeeCapWithEscalatedTip, escalatedGasTip, nil } -func (t *transactionService) prepareTransactionWithRetry(ctx context.Context, request *TxRequest, nonce uint64, prevGasTipCap *big.Int) (*types.Transaction, error) { +func (t *transactionService) prepareTransactionWithRetry(ctx context.Context, request *TxRequest, nonce uint64, prevGasTipCap *big.Int, overrides *RetryOverrides) (*types.Transaction, error) { gasLimit, err := t.estimateGasLimit(ctx, request) if err != nil { return nil, err } - gasFeeCap, newGasTipCap, err := t.suggestGasFeeGasTipCapWithHistory(ctx, prevGasTipCap) + gasFeeCap, newGasTipCap, err := t.suggestGasFeeGasTipCapWithHistory(ctx, prevGasTipCap, overrides) if err != nil { return nil, err } @@ -149,7 +203,7 @@ func (t *transactionService) prepareTransactionWithRetry(ctx context.Context, re // broadcastTx prepares, signs, and sends a transaction. // When fixedNonce is nil a new nonce is allocated (first attempt); // otherwise the supplied nonce is reused (replacement transaction). -func (t *transactionService) broadcastTx(ctx context.Context, request *TxRequest, fixedNonce *uint64, gasTipCap *big.Int, attempt int) (*types.Transaction, error) { +func (t *transactionService) broadcastTx(ctx context.Context, request *TxRequest, fixedNonce *uint64, gasTipCap *big.Int, attempt int, overrides *RetryOverrides) (*types.Transaction, error) { var nonce uint64 if fixedNonce != nil { @@ -164,7 +218,7 @@ func (t *transactionService) broadcastTx(ctx context.Context, request *TxRequest } nonce = n } - tx, err := t.prepareTransactionWithRetry(ctx, request, nonce, gasTipCap) + tx, err := t.prepareTransactionWithRetry(ctx, request, nonce, gasTipCap, overrides) if err != nil { return nil, err } @@ -205,7 +259,7 @@ func (t *transactionService) deleteRetryStateAndPending(retryKey string, state T _ = t.store.Delete(pendingTransactionKey(state.LastTxHash)) } } -func (t *transactionService) retry(ctx context.Context, txRetryKey string, request *TxRequest) (common.Hash, *types.Receipt, error) { +func (t *transactionService) retry(ctx context.Context, txRetryKey string, request *TxRequest, overrides *RetryOverrides) (common.Hash, *types.Receipt, error) { var ( txState TransactionRetryState nonce *uint64 @@ -238,12 +292,19 @@ func (t *transactionService) retry(ctx context.Context, txRetryKey string, reque "nonce_assigned", txState.NonceAssigned, "previous_tip", txState.PreviousTip) + retryDelay := func(attempt int) time.Duration { + if overrides != nil && overrides.RetryDelay != nil { + return overrides.RetryDelay(attempt) + } + return t.txRetryDelay + } + for attempt := txState.NextAttempt; attempt < t.txMaxRetries; attempt++ { if txState.NonceAssigned { nonce = &txState.Nonce } - signedTx, err := t.broadcastTx(ctx, request, nonce, txState.PreviousTip, attempt) + signedTx, err := t.broadcastTx(ctx, request, nonce, txState.PreviousTip, attempt, overrides) if err != nil { if isErrCritical(err) { t.logger.Error(err, @@ -277,22 +338,24 @@ func (t *transactionService) retry(ctx context.Context, txRetryKey string, reque "previous_tip", txState.PreviousTip, "description", request.Description) + delay := retryDelay(attempt) + if txState.LastTxHash == (common.Hash{}) { loggerV1.Debug("send with retry: no tx hash after broadcast failure, waiting before next attempt", "attempt", attempt, - "retry_delay", t.txRetryDelay, + "retry_delay", delay, "description", request.Description) select { case <-ctx.Done(): err := ctx.Err() t.recordRetryComplete(txState.NextAttempt, err) return common.Hash{}, nil, err - case <-time.After(t.txRetryDelay): + case <-time.After(delay): continue } } - waitCtx, cancel := context.WithTimeout(ctx, t.txRetryDelay) + waitCtx, cancel := context.WithTimeout(ctx, delay) rec, waitErr := t.WaitForReceipt(waitCtx, txState.LastTxHash) cancel() @@ -499,7 +562,7 @@ func (t *transactionService) resumeRetryTransactions() error { sk := key st := state t.wg.Go(func() { - if _, _, err := t.retry(t.ctx, sk, nil); err != nil { + if _, _, err := t.retry(t.ctx, sk, nil, nil); err != nil { t.logger.Error(err, "resumed transaction retry aborted", "nonce", st.Nonce, "description", st.Description) } }) diff --git a/pkg/transaction/send_tx_with_retry_test.go b/pkg/transaction/send_tx_with_retry_test.go index ecd7a2b3161..5a54f780600 100644 --- a/pkg/transaction/send_tx_with_retry_test.go +++ b/pkg/transaction/send_tx_with_retry_test.go @@ -67,7 +67,7 @@ func TestSuggestGasFeeGasTipCapWithHistory(t *testing.T) { backend := backendmock.New(headerOption(), feeHistoryOption(&feeHistoryCalls)) gasFeeCap, gasTipCap, err := transaction.SuggestGasFeeGasTipCapWithHistory( - backend, gasIncreasePct, nil, context.Background(), nil, + backend, gasIncreasePct, nil, context.Background(), nil, nil, ) require.NoError(t, err) @@ -83,7 +83,7 @@ func TestSuggestGasFeeGasTipCapWithHistory(t *testing.T) { backend := backendmock.New(headerOption(), feeHistoryOption(&feeHistoryCalls)) gasFeeCap, gasTipCap, err := transaction.SuggestGasFeeGasTipCapWithHistory( - backend, gasIncreasePct, nil, context.Background(), big.NewInt(prevTip), + backend, gasIncreasePct, nil, context.Background(), big.NewInt(prevTip), nil, ) require.NoError(t, err) @@ -101,7 +101,7 @@ func TestSuggestGasFeeGasTipCapWithHistory(t *testing.T) { backend := backendmock.New(headerOption()) gasFeeCap, gasTipCap, err := transaction.SuggestGasFeeGasTipCapWithHistory( - backend, gasIncreasePct, maxTxPrice, context.Background(), big.NewInt(prevTip), + backend, gasIncreasePct, maxTxPrice, context.Background(), big.NewInt(prevTip), nil, ) require.NoError(t, err) @@ -117,13 +117,56 @@ func TestSuggestGasFeeGasTipCapWithHistory(t *testing.T) { backend := backendmock.New(headerOption()) gasFeeCap, gasTipCap, err := transaction.SuggestGasFeeGasTipCapWithHistory( - backend, gasIncreasePct, maxTxPrice, context.Background(), big.NewInt(prevTip), + backend, gasIncreasePct, maxTxPrice, context.Background(), big.NewInt(prevTip), nil, ) assert.ErrorIs(t, err, transaction.ErrTxMaxPriceExceeded) assert.Nil(t, gasFeeCap) assert.Nil(t, gasTipCap) }) + + t.Run("IgnoreMaxPrice override bypasses limit", func(t *testing.T) { + t.Parallel() + + maxTxPrice := big.NewInt(baseFeeCap + prevTip - 1) + backend := backendmock.New(headerOption()) + + var receivedFeeCap *big.Int + overrides := &transaction.RetryOverrides{ + IgnoreMaxPrice: func(feeCap *big.Int) bool { + receivedFeeCap = feeCap + return true + }, + } + + gasFeeCap, gasTipCap, err := transaction.SuggestGasFeeGasTipCapWithHistory( + backend, gasIncreasePct, maxTxPrice, context.Background(), big.NewInt(prevTip), overrides, + ) + + require.NoError(t, err, "override should bypass ErrTxMaxPriceExceeded") + assert.Equal(t, escalatedTip, gasTipCap.Int64(), "must use escalated tip despite exceeding max") + assert.Equal(t, baseFeeCap+escalatedTip, gasFeeCap.Int64()) + assert.NotNil(t, receivedFeeCap, "IgnoreMaxPrice must receive gasFeeCap") + assert.Equal(t, gasFeeCap.Int64(), receivedFeeCap.Int64(), + "IgnoreMaxPrice must receive the actual gasFeeCap that would be used") + }) + + t.Run("IgnoreMaxPrice false does not bypass limit", func(t *testing.T) { + t.Parallel() + + maxTxPrice := big.NewInt(baseFeeCap + prevTip - 1) + backend := backendmock.New(headerOption()) + + overrides := &transaction.RetryOverrides{ + IgnoreMaxPrice: func(_ *big.Int) bool { return false }, + } + + _, _, err := transaction.SuggestGasFeeGasTipCapWithHistory( + backend, gasIncreasePct, maxTxPrice, context.Background(), big.NewInt(prevTip), overrides, + ) + + assert.ErrorIs(t, err, transaction.ErrTxMaxPriceExceeded) + }) } // capturedBroadcast records the parameters of a transaction as seen by SendTransaction. @@ -855,6 +898,128 @@ func TestSendWithRetry_MaxTxPriceCap(t *testing.T) { }) } +// WithIgnoreMaxPrice override allows transactions to be sent despite exceeding maxTxPrice. +func TestSendWithRetry_IgnoreMaxPriceOverride(t *testing.T) { + t.Parallel() + s := newRetryTestSetup() + store := storemock.NewStateStore() + testutil.CleanupCloser(t, store) + + marketTip := s.expectedMarketTip() + maxTxPrice := new(big.Int).Sub(s.expectedGasFeeCap(marketTip), big.NewInt(1)) + + cfg := s.retryConfig() + cfg.MaxTxPrice = maxTxPrice + + var broadcasts []capturedBroadcast + var overrideCalls atomic.Int32 + + svc, err := transaction.NewService(log.Noop, s.sender, + backendmock.New( + s.nonceOption(), + s.feeHistoryOption(nil), + s.headerOption(), + s.estimateGasOption(), + backendmock.WithSendTransactionFunc(func(ctx context.Context, tx *types.Transaction) error { + broadcasts = append(broadcasts, captureTx(tx)) + return nil + }), + ), + signermock.New(s.passThroughSigner(), s.signerAddr()), + store, + s.chainID, + monitormock.New( + monitormock.WithWatchTransactionFunc(func(txHash common.Hash, nonce uint64) (<-chan types.Receipt, <-chan error, error) { + ch := make(chan types.Receipt, 1) + ch <- types.Receipt{TxHash: txHash, Status: 1} + return ch, nil, nil + }), + ), + 0, + cfg, + ) + require.NoError(t, err) + testutil.CleanupCloser(t, svc) + + var receivedFeeCaps []*big.Int + txHash, receipt, err := svc.SendWithRetry(context.Background(), s.request(), + transaction.WithIgnoreMaxPrice(func(gasFeeCap *big.Int) bool { + overrideCalls.Add(1) + receivedFeeCaps = append(receivedFeeCaps, new(big.Int).Set(gasFeeCap)) + return true + }), + ) + + require.NoError(t, err) + assert.NotEqual(t, common.Hash{}, txHash) + require.NotNil(t, receipt) + assert.Equal(t, uint64(1), receipt.Status) + + require.Len(t, broadcasts, 1, "transaction should be sent despite exceeding maxTxPrice") + assert.Equal(t, marketTip.Int64(), broadcasts[0].GasTipCap.Int64()) + assert.GreaterOrEqual(t, int(overrideCalls.Load()), 1, "override function must have been called") + require.Len(t, receivedFeeCaps, 1, "override should have received gasFeeCap") + assert.Equal(t, broadcasts[0].GasFeeCap.Int64(), receivedFeeCaps[0].Int64(), + "override must receive the same gasFeeCap as the broadcast") +} + +// WithRetryDelay override changes the delay between retry attempts. +func TestSendWithRetry_RetryDelayOverride(t *testing.T) { + t.Parallel() + s := newRetryTestSetup() + store := storemock.NewStateStore() + testutil.CleanupCloser(t, store) + + cfg := s.retryConfig() + cfg.RetryDelay = 10 * time.Second + + var broadcastCount atomic.Int32 + + svc, err := transaction.NewService(log.Noop, s.sender, + backendmock.New( + s.nonceOption(), + s.feeHistoryOption(nil), + s.headerOption(), + s.estimateGasOption(), + backendmock.WithSendTransactionFunc(func(ctx context.Context, tx *types.Transaction) error { + broadcastCount.Add(1) + return nil + }), + ), + signermock.New(s.passThroughSigner(), s.signerAddr()), + store, + s.chainID, + monitormock.New( + monitormock.WithWatchTransactionFunc(func(txHash common.Hash, nonce uint64) (<-chan types.Receipt, <-chan error, error) { + if broadcastCount.Load() <= 1 { + return make(chan types.Receipt), make(chan error), nil + } + ch := make(chan types.Receipt, 1) + ch <- types.Receipt{TxHash: txHash, Status: 1} + return ch, nil, nil + }), + ), + 0, + cfg, + ) + require.NoError(t, err) + testutil.CleanupCloser(t, svc) + + start := time.Now() + _, receipt, err := svc.SendWithRetry(context.Background(), s.request(), + transaction.WithRetryDelay(func(attempt int) time.Duration { + return 50 * time.Millisecond + }), + ) + + require.NoError(t, err) + require.NotNil(t, receipt) + elapsed := time.Since(start) + assert.Less(t, elapsed, 2*time.Second, + "with overridden short delay, retry should complete much faster than the 10s default") + assert.GreaterOrEqual(t, int(broadcastCount.Load()), 2, "should have retried at least once") +} + // failOnNthPutStore wraps a StateStorer and fails the Nth Put call with putErr. type failOnNthPutStore struct { storage.StateStorer diff --git a/pkg/transaction/transaction.go b/pkg/transaction/transaction.go index 443311c1e48..0234724e1c7 100644 --- a/pkg/transaction/transaction.go +++ b/pkg/transaction/transaction.go @@ -117,7 +117,8 @@ type Service interface { // Send creates a transaction based on the request (with gasprice increased by provided percentage) and sends it. Send(ctx context.Context, request *TxRequest, tipCapBoostPercent int) (txHash common.Hash, err error) // SendWithRetry sends a transaction using fee-history tiers and automatic fee escalation; see send_tx_with_retry.go. - SendWithRetry(ctx context.Context, request *TxRequest) (txHash common.Hash, receipt *types.Receipt, err error) + // Optional RetryOption values can override per-call retry behaviour (e.g. bypass price cap). + SendWithRetry(ctx context.Context, request *TxRequest, opts ...RetryOption) (txHash common.Hash, receipt *types.Receipt, err error) // Call simulate a transaction based on the request. Call(ctx context.Context, request *TxRequest) (result []byte, err error) // WaitForReceipt waits until either the transaction with the given hash has been mined or the context is cancelled. @@ -442,7 +443,7 @@ func (t *transactionService) prepareTransaction(ctx context.Context, request *Tx return nil, errors.New("gas fee cap must be greater than zero") } if gasFeeCap.Cmp(request.GasFeeCap) > 0 { - return nil, fmt.Errorf("%w: suggested=%s requested=%s", ErrFeeCapExceeded, gasFeeCap, request.GasFeeCap) + return nil, fmt.Errorf("gas fee cap exceeded: suggested=%s requested=%s", gasFeeCap, request.GasFeeCap) } gasFeeCap = new(big.Int).Set(request.GasFeeCap) if gasTipCap.Cmp(gasFeeCap) > 0 { From a4e74cc547ebe98a17f219f939e7a3aae1cb7798 Mon Sep 17 00:00:00 2001 From: sbackend Date: Fri, 22 May 2026 12:16:18 +0200 Subject: [PATCH 7/7] fix: fix tests --- pkg/postage/postagecontract/contract_test.go | 12 +- .../redistribution/redistribution_test.go | 268 ++---------------- pkg/transaction/mock/transaction.go | 8 +- 3 files changed, 30 insertions(+), 258 deletions(-) diff --git a/pkg/postage/postagecontract/contract_test.go b/pkg/postage/postagecontract/contract_test.go index 6592a7f23a7..66d56f2c455 100644 --- a/pkg/postage/postagecontract/contract_test.go +++ b/pkg/postage/postagecontract/contract_test.go @@ -74,7 +74,7 @@ func TestCreateBatch(t *testing.T) { postageStampContractABI, bzzTokenAddress, transactionMock.New( - transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest) (common.Hash, *types.Receipt, error) { + transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest, _ ...transaction.RetryOption) (common.Hash, *types.Receipt, error) { switch *request.To { case bzzTokenAddress: return txHashApprove, &types.Receipt{Status: 1}, nil @@ -308,7 +308,7 @@ func TestTopUpBatch(t *testing.T) { postageStampContractABI, bzzTokenAddress, transactionMock.New( - transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest) (common.Hash, *types.Receipt, error) { + transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest, _ ...transaction.RetryOption) (common.Hash, *types.Receipt, error) { switch *request.To { case bzzTokenAddress: return txHashApprove, &types.Receipt{Status: 1}, nil @@ -468,7 +468,7 @@ func TestDiluteBatch(t *testing.T) { postageStampContractABI, bzzTokenAddress, transactionMock.New( - transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest) (common.Hash, *types.Receipt, error) { + transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest, _ ...transaction.RetryOption) (common.Hash, *types.Receipt, error) { if *request.To == postageStampAddress { if bytes.Equal(expectedCallDataForExpireLimitedBatches[:32], request.Data[:32]) { return txHashApprove, &types.Receipt{Status: 1}, nil @@ -630,7 +630,7 @@ func TestBatchExpirer(t *testing.T) { } } return nil, errors.New("unexpected call") - }), transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest) (common.Hash, *types.Receipt, error) { + }), transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest, _ ...transaction.RetryOption) (common.Hash, *types.Receipt, error) { return common.Hash{}, &types.Receipt{Status: 1}, nil }), ), @@ -768,7 +768,7 @@ func TestBatchExpirer(t *testing.T) { } } return nil, errors.New("unexpected call") - }), transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest) (common.Hash, *types.Receipt, error) { + }), transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest, _ ...transaction.RetryOption) (common.Hash, *types.Receipt, error) { if *request.To == postageContractAddress { if bytes.Equal(expectedCallDataForExpireLimitedBatches[:32], request.Data[:32]) { return common.Hash{}, nil, fmt.Errorf("some error") @@ -891,7 +891,7 @@ func TestBatchExpirer(t *testing.T) { } } return nil, errors.New("unexpected call") - }), transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest) (common.Hash, *types.Receipt, error) { + }), transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest, _ ...transaction.RetryOption) (common.Hash, *types.Receipt, error) { return common.Hash{}, &types.Receipt{Status: 0}, transaction.ErrTransactionReverted }), ), diff --git a/pkg/storageincentives/redistribution/redistribution_test.go b/pkg/storageincentives/redistribution/redistribution_test.go index ebe45d43a8e..e9092c3ee02 100644 --- a/pkg/storageincentives/redistribution/redistribution_test.go +++ b/pkg/storageincentives/redistribution/redistribution_test.go @@ -13,7 +13,6 @@ import ( "math/big" "sync/atomic" "testing" - "time" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" @@ -94,7 +93,6 @@ func TestRedistribution(t *testing.T) { redistributionContractAddress, redistributionContractABI, 0, - 0, ) isPlaying, err := contract.IsPlaying(ctx, depth) @@ -127,7 +125,6 @@ func TestRedistribution(t *testing.T) { redistributionContractAddress, redistributionContractABI, 0, - 0, ) isPlaying, err := contract.IsPlaying(ctx, depth) @@ -158,7 +155,6 @@ func TestRedistribution(t *testing.T) { redistributionContractAddress, redistributionContractABI, 0, - 0, ) isWinner, err := contract.IsWinner(ctx) @@ -186,7 +182,6 @@ func TestRedistribution(t *testing.T) { redistributionContractAddress, redistributionContractABI, 0, - 0, ) isWinner, err := contract.IsWinner(ctx) @@ -212,7 +207,7 @@ func TestRedistribution(t *testing.T) { owner, log.Noop, transactionMock.New( - transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest) (common.Hash, *types.Receipt, error) { + transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest, _ ...transaction.RetryOption) (common.Hash, *types.Receipt, error) { if *request.To == redistributionContractAddress { if !bytes.Equal(expectedCallData[:32], request.Data[:32]) { return common.Hash{}, nil, fmt.Errorf("got wrong call data. wanted %x, got %x", expectedCallData, request.Data) @@ -225,7 +220,6 @@ func TestRedistribution(t *testing.T) { redistributionContractAddress, redistributionContractABI, 0, - 0, ) _, err = contract.Claim(ctx, proofs, nil) @@ -247,7 +241,7 @@ func TestRedistribution(t *testing.T) { owner, log.Noop, transactionMock.New( - transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest) (common.Hash, *types.Receipt, error) { + transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest, _ ...transaction.RetryOption) (common.Hash, *types.Receipt, error) { if *request.To == redistributionContractAddress { if !bytes.Equal(expectedCallData[:32], request.Data[:32]) { return common.Hash{}, nil, fmt.Errorf("got wrong call data. wanted %x, got %x", expectedCallData, request.Data) @@ -260,7 +254,6 @@ func TestRedistribution(t *testing.T) { redistributionContractAddress, redistributionContractABI, 0, - 0, ) _, err = contract.Claim(ctx, proofs, nil) @@ -283,7 +276,7 @@ func TestRedistribution(t *testing.T) { owner, log.Noop, transactionMock.New( - transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest) (common.Hash, *types.Receipt, error) { + transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest, _ ...transaction.RetryOption) (common.Hash, *types.Receipt, error) { if *request.To == redistributionContractAddress { if !bytes.Equal(expectedCallData[:32], request.Data[:32]) { return common.Hash{}, nil, fmt.Errorf("got wrong call data. wanted %x, got %x", expectedCallData, request.Data) @@ -296,7 +289,6 @@ func TestRedistribution(t *testing.T) { redistributionContractAddress, redistributionContractABI, 0, - 0, ) _, err = contract.Commit(ctx, testobfus, 0) @@ -321,7 +313,7 @@ func TestRedistribution(t *testing.T) { owner, log.Noop, transactionMock.New( - transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest) (common.Hash, *types.Receipt, error) { + transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest, _ ...transaction.RetryOption) (common.Hash, *types.Receipt, error) { if *request.To == redistributionContractAddress { if !bytes.Equal(expectedCallData[:32], request.Data[:32]) { return common.Hash{}, nil, fmt.Errorf("got wrong call data. wanted %x, got %x", expectedCallData, request.Data) @@ -334,7 +326,6 @@ func TestRedistribution(t *testing.T) { redistributionContractAddress, redistributionContractABI, 0, - 0, ) _, err = contract.Reveal(ctx, depth, common.Hex2Bytes("hash"), common.Hex2Bytes("nonce")) @@ -361,7 +352,6 @@ func TestRedistribution(t *testing.T) { redistributionContractAddress, redistributionContractABI, 0, - 0, ) salt, err := contract.ReserveSalt(ctx) @@ -392,7 +382,6 @@ func TestRedistribution(t *testing.T) { redistributionContractAddress, redistributionContractABI, 0, - 0, ) _, err := contract.IsPlaying(ctx, depth) @@ -416,7 +405,7 @@ func TestRedistribution(t *testing.T) { owner, log.Noop, transactionMock.New( - transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest) (common.Hash, *types.Receipt, error) { + transactionMock.WithSendWithRetryFunc(func(ctx context.Context, request *transaction.TxRequest, _ ...transaction.RetryOption) (common.Hash, *types.Receipt, error) { if *request.To == redistributionContractAddress { if !bytes.Equal(expectedCallData, request.Data) { return common.Hash{}, nil, fmt.Errorf("got wrong call data. wanted %x, got %x", expectedCallData, request.Data) @@ -429,7 +418,6 @@ func TestRedistribution(t *testing.T) { redistributionContractAddress, redistributionContractABI, 0, - 0, ) _, err = contract.Commit(ctx, actualHash, 0) @@ -439,113 +427,6 @@ func TestRedistribution(t *testing.T) { }) } -func TestRedistribution_MaxTxCostWaitsUntilContextDone(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), 80*time.Millisecond) - defer cancel() - owner := common.HexToAddress("abcd") - overlay := swarm.NewAddress(common.HexToHash("cbd").Bytes()) - redistributionContractAddress := common.HexToAddress("ffff") - var sendCalled atomic.Bool - - proofs := randChunkInclusionProofs(t) - - contract := redistribution.New( - overlay, - owner, - log.Noop, - transactionMock.New( - transactionMock.WithEstimateTxCostFunc(func(_ context.Context, gasUnits int64, _ int) (*big.Int, *big.Int, error) { - gasFeeCap := big.NewInt(10) - return new(big.Int).Mul(big.NewInt(gasUnits), gasFeeCap), gasFeeCap, nil - }), - transactionMock.WithSendFunc(func(context.Context, *transaction.TxRequest, int) (common.Hash, error) { - sendCalled.Store(true) - return common.Hash{}, errors.New("send should not be called") - }), - transactionMock.WithWaitForReceiptFunc(func(context.Context, common.Hash) (*types.Receipt, error) { - return nil, errors.New("unexpected wait") - }), - ), - redistributionContractAddress, - redistributionContractABI, - 100_000, - time.Millisecond, - redistribution.WithMaxTxCost(500_000, 0), - ) - - _, err := contract.Claim(ctx, proofs, nil) - if !errors.Is(err, context.DeadlineExceeded) && !errors.Is(err, context.Canceled) { - t.Fatalf("want context deadline or cancel, got %v", err) - } - if sendCalled.Load() { - t.Fatal("Send must not be called when cost exceeds limit") - } -} - -const testShortBlockTime = 10 * time.Millisecond - -// 1. Commit waits while cost too high; after EstimateTxCost improves, tx is sent successfully. -func TestCommit_RetriesUntilCostAcceptableThenSuccess(t *testing.T) { - t.Parallel() - - ctx := context.Background() - owner := common.HexToAddress("abcd") - overlay := swarm.NewAddress(common.HexToHash("cbd").Bytes()) - redistributionContractAddress := common.HexToAddress("ffff") - testobfus := common.Hex2Bytes("hash") - var obfus [32]byte - copy(obfus[:], testobfus) - expectedHash := common.HexToHash("abc123") - - var estimateCalls atomic.Int32 - txSvc := transactionMock.New( - transactionMock.WithEstimateTxCostFunc(func(_ context.Context, gasUnits int64, _ int) (*big.Int, *big.Int, error) { - n := estimateCalls.Add(1) - var gasFeeCap *big.Int - if n == 1 { - gasFeeCap = big.NewInt(10) - } else { - gasFeeCap = big.NewInt(1) - } - cost := new(big.Int).Mul(big.NewInt(gasUnits), gasFeeCap) - return cost, gasFeeCap, nil - }), - transactionMock.WithSendFunc( - func(ctx context.Context, request *transaction.TxRequest, _ int) (common.Hash, error) { - assert.NotNil(t, request.GasFeeCap) - assert.Equal(t, 0, big.NewInt(1).Cmp(request.GasFeeCap), "fee cap must match last successful estimate") - - callData, err := redistributionContractABI.Pack("commit", obfus, uint64(0)) - assert.NoError(t, err) - assert.Equal(t, callData, request.Data) - return expectedHash, nil - }), - transactionMock.WithWaitForReceiptFunc(func(context.Context, common.Hash) (*types.Receipt, error) { - return &types.Receipt{Status: 1}, nil - }), - ) - - c := redistribution.New( - overlay, - owner, - log.Noop, - txSvc, - redistributionContractAddress, - redistributionContractABI, - 0, - testShortBlockTime, - redistribution.WithMaxTxCost(500_000, 0), - ) - - h, err := c.Commit(ctx, testobfus, 0) - assert.NoError(t, err) - assert.Equal(t, expectedHash, h) - assert.EqualValues(t, 2, estimateCalls.Load(), "should re-estimate after blockTime wait") -} - -// 2. Commit sends a tx but receives a critical error → fails without endless retry. func TestCommit_CriticalErrorFails(t *testing.T) { t.Parallel() @@ -556,8 +437,8 @@ func TestCommit_CriticalErrorFails(t *testing.T) { testobfus := common.Hex2Bytes("hash") txSvc := transactionMock.New( - transactionMock.WithSendFunc(func(context.Context, *transaction.TxRequest, int) (common.Hash, error) { - return common.Hash{}, transaction.ErrTransactionReverted + transactionMock.WithSendWithRetryFunc(func(_ context.Context, _ *transaction.TxRequest, _ ...transaction.RetryOption) (common.Hash, *types.Receipt, error) { + return common.Hash{}, nil, transaction.ErrTransactionReverted }), ) @@ -569,7 +450,6 @@ func TestCommit_CriticalErrorFails(t *testing.T) { redistributionContractAddress, redistributionContractABI, 0, - testShortBlockTime, ) _, err := c.Commit(ctx, testobfus, 0) @@ -577,93 +457,7 @@ func TestCommit_CriticalErrorFails(t *testing.T) { assert.ErrorIs(t, err, transaction.ErrTransactionReverted) } -// First Send fails with non-critical error and zero tx hash; wait blockTime, retry Send -> success. -func TestCommit_RetriesAfterTransientSendFailure(t *testing.T) { - t.Parallel() - - ctx := context.Background() - owner := common.HexToAddress("abcd") - overlay := swarm.NewAddress(common.HexToHash("cbd").Bytes()) - redistributionContractAddress := common.HexToAddress("ffff") - testobfus := common.Hex2Bytes("hash") - expectedHash := common.HexToHash("deadbeef") - - var sendCalls atomic.Int32 - txSvc := transactionMock.New( - transactionMock.WithEstimateTxCostFunc(func(_ context.Context, gasUnits int64, _ int) (*big.Int, *big.Int, error) { - gasFeeCap := big.NewInt(2) - return new(big.Int).Mul(big.NewInt(gasUnits), gasFeeCap), gasFeeCap, nil - }), - transactionMock.WithSendFunc(func(context.Context, *transaction.TxRequest, int) (common.Hash, error) { - if sendCalls.Add(1) == 1 { - // Zero hash + non-critical error must trigger a second Send after blockTime. - return common.Hash{}, errors.New("temporary rpc failure") - } - return expectedHash, nil - }), - transactionMock.WithWaitForReceiptFunc(func(context.Context, common.Hash) (*types.Receipt, error) { - return &types.Receipt{Status: 1}, nil - }), - ) - - c := redistribution.New( - overlay, - owner, - log.Noop, - txSvc, - redistributionContractAddress, - redistributionContractABI, - 0, - testShortBlockTime, - redistribution.WithMaxTxCost(2_000_000, 0), - ) - - h, err := c.Commit(ctx, testobfus, 0) - assert.NoError(t, err) - assert.Equal(t, expectedHash, h) - assert.EqualValues(t, 2, sendCalls.Load(), "second Send should happen after zero-hash failure") -} - -// 4. Commit never sends: cost stays above max-tx-cap until context is cancelled. -func TestCommit_contextCancelledWhileWaitingForAcceptableCost(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithCancel(context.Background()) - cancel() - - owner := common.HexToAddress("abcd") - overlay := swarm.NewAddress(common.HexToHash("cbd").Bytes()) - redistributionContractAddress := common.HexToAddress("ffff") - - txSvc := transactionMock.New( - transactionMock.WithEstimateTxCostFunc(func(_ context.Context, gasUnits int64, _ int) (*big.Int, *big.Int, error) { - gasFeeCap := big.NewInt(100) - return new(big.Int).Mul(big.NewInt(gasUnits), gasFeeCap), gasFeeCap, nil - }), - transactionMock.WithSendFunc(func(context.Context, *transaction.TxRequest, int) (common.Hash, error) { - t.Fatal("Send must not be called") - return common.Hash{}, nil - }), - ) - - c := redistribution.New( - overlay, - owner, - log.Noop, - txSvc, - redistributionContractAddress, - redistributionContractABI, - 0, - testShortBlockTime, - redistribution.WithMaxTxCost(500_000, 0), - ) - - _, err := c.Commit(ctx, common.Hex2Bytes("hash"), 0) - assert.ErrorIs(t, err, redistribution.ErrMaxTxCostExceeded) -} - -// No max-tx-cost limit: user pays whatever fee the network/request carries (nil caps on TxRequest). -func TestCommit_withoutMaxTxCostLeavesFeeCapsUnset(t *testing.T) { +func TestCommit_withoutGasFeeCapOnRequest(t *testing.T) { t.Parallel() ctx := context.Background() @@ -674,13 +468,10 @@ func TestCommit_withoutMaxTxCostLeavesFeeCapsUnset(t *testing.T) { expectedHash := common.HexToHash("bbbb") txSvc := transactionMock.New( - transactionMock.WithSendFunc(func(_ context.Context, request *transaction.TxRequest, _ int) (common.Hash, error) { + transactionMock.WithSendWithRetryFunc(func(_ context.Context, request *transaction.TxRequest, _ ...transaction.RetryOption) (common.Hash, *types.Receipt, error) { assert.Nil(t, request.GasFeeCap) assert.Nil(t, request.GasPrice) - return expectedHash, nil - }), - transactionMock.WithWaitForReceiptFunc(func(context.Context, common.Hash) (*types.Receipt, error) { - return &types.Receipt{Status: 1}, nil + return expectedHash, &types.Receipt{Status: 1}, nil }), ) @@ -692,7 +483,6 @@ func TestCommit_withoutMaxTxCostLeavesFeeCapsUnset(t *testing.T) { redistributionContractAddress, redistributionContractABI, 0, - testShortBlockTime, ) h, err := c.Commit(ctx, testobfus, 0) @@ -700,8 +490,7 @@ func TestCommit_withoutMaxTxCostLeavesFeeCapsUnset(t *testing.T) { assert.Equal(t, expectedHash, h) } -// 6. Claim hits max cost; there is 10 blocks remaining before phase end, expectedReward covers upper-bound cost + previous round fees → limit bypassed, tx sent. -func TestClaim_bypassesMaxTxCost(t *testing.T) { +func TestClaim_sendsWithRetryOptions(t *testing.T) { t.Parallel() ctx := context.Background() @@ -712,21 +501,15 @@ func TestClaim_bypassesMaxTxCost(t *testing.T) { expectedHash := common.HexToHash("cafe") var sendCalls atomic.Int32 + var retryOptsLen int txSvc := transactionMock.New( - transactionMock.WithEstimateTxCostFunc(func(_ context.Context, gasUnits int64, _ int) (*big.Int, *big.Int, error) { - gasFeeCap := big.NewInt(20) - return new(big.Int).Mul(big.NewInt(gasUnits), gasFeeCap), gasFeeCap, nil - }), - transactionMock.WithSendFunc(func(_ context.Context, request *transaction.TxRequest, _ int) (common.Hash, error) { + transactionMock.WithSendWithRetryFunc(func(_ context.Context, request *transaction.TxRequest, opts ...transaction.RetryOption) (common.Hash, *types.Receipt, error) { sendCalls.Add(1) + retryOptsLen = len(opts) callData, err := redistributionContractABI.Pack("claim", proofs.A, proofs.B, proofs.C) assert.NoError(t, err) assert.Equal(t, callData, request.Data) - assert.NotNil(t, request.GasFeeCap, "override should pin maxFeePerGas for send") - return expectedHash, nil - }), - transactionMock.WithWaitForReceiptFunc(func(context.Context, common.Hash) (*types.Receipt, error) { - return &types.Receipt{Status: 1}, nil + return expectedHash, &types.Receipt{Status: 1}, nil }), ) @@ -738,14 +521,12 @@ func TestClaim_bypassesMaxTxCost(t *testing.T) { redistributionContractAddress, redistributionContractABI, 0, - testShortBlockTime, - redistribution.WithMaxTxCost(500_000, 0), ) opts := &redistribution.ClaimOpts{ OverrideAfterBlock: 100, CurrentBlockFn: func() uint64 { return 110 }, - ExpectedReward: new(big.Int).Mul(big.NewInt(50), big.NewInt(1_000_000)), // >> estimated + fees + ExpectedReward: new(big.Int).Mul(big.NewInt(50), big.NewInt(1_000_000)), RoundFees: big.NewInt(100_000), } @@ -753,10 +534,10 @@ func TestClaim_bypassesMaxTxCost(t *testing.T) { assert.NoError(t, err) assert.Equal(t, expectedHash, h) assert.EqualValues(t, 1, sendCalls.Load()) + assert.Equal(t, 1, retryOptsLen, "Claim must pass WithIgnoreMaxPrice retry option") } -// 7. Same high fees; expected reward does not cover cost — override denied; cancelled ctx exits without Send. -func TestClaim_noBypassWhenRewardTooSmall(t *testing.T) { +func TestClaim_contextCanceled(t *testing.T) { t.Parallel() ctx, cancel := context.WithCancel(context.Background()) @@ -767,15 +548,9 @@ func TestClaim_noBypassWhenRewardTooSmall(t *testing.T) { redistributionContractAddress := common.HexToAddress("ffff") proofs := randChunkInclusionProofs(t) - var sendCalls atomic.Int32 txSvc := transactionMock.New( - transactionMock.WithEstimateTxCostFunc(func(_ context.Context, gasUnits int64, _ int) (*big.Int, *big.Int, error) { - gasFeeCap := big.NewInt(50) - return new(big.Int).Mul(big.NewInt(gasUnits), gasFeeCap), gasFeeCap, nil - }), - transactionMock.WithSendFunc(func(context.Context, *transaction.TxRequest, int) (common.Hash, error) { - sendCalls.Add(1) - return common.Hash{}, errors.New("Send must not run") + transactionMock.WithSendWithRetryFunc(func(ctx context.Context, _ *transaction.TxRequest, _ ...transaction.RetryOption) (common.Hash, *types.Receipt, error) { + return common.Hash{}, nil, ctx.Err() }), ) @@ -787,8 +562,6 @@ func TestClaim_noBypassWhenRewardTooSmall(t *testing.T) { redistributionContractAddress, redistributionContractABI, 0, - testShortBlockTime, - redistribution.WithMaxTxCost(500_000, 0), ) opts := &redistribution.ClaimOpts{ @@ -800,5 +573,4 @@ func TestClaim_noBypassWhenRewardTooSmall(t *testing.T) { _, err := c.Claim(ctx, proofs, opts) assert.ErrorIs(t, err, context.Canceled) - assert.Zero(t, sendCalls.Load()) } diff --git a/pkg/transaction/mock/transaction.go b/pkg/transaction/mock/transaction.go index 13886b74796..8491ca0fa49 100644 --- a/pkg/transaction/mock/transaction.go +++ b/pkg/transaction/mock/transaction.go @@ -20,7 +20,7 @@ import ( type transactionServiceMock struct { estimateTxCost func(ctx context.Context, gasUnits int64, tip int) (cost *big.Int, gasFeeCap *big.Int, err error) send func(ctx context.Context, request *transaction.TxRequest, boost int) (txHash common.Hash, err error) - sendWithRetry func(ctx context.Context, request *transaction.TxRequest) (txHash common.Hash, receipt *types.Receipt, err error) + sendWithRetry func(ctx context.Context, request *transaction.TxRequest, opts ...transaction.RetryOption) (txHash common.Hash, receipt *types.Receipt, err error) waitForReceipt func(ctx context.Context, txHash common.Hash) (receipt *types.Receipt, err error) watchSentTransaction func(txHash common.Hash) (chan types.Receipt, chan error, error) call func(ctx context.Context, request *transaction.TxRequest) (result []byte, err error) @@ -31,9 +31,9 @@ type transactionServiceMock struct { transactionFee func(ctx context.Context, txHash common.Hash) (*big.Int, error) } -func (m *transactionServiceMock) SendWithRetry(ctx context.Context, request *transaction.TxRequest, _ ...transaction.RetryOption) (common.Hash, *types.Receipt, error) { +func (m *transactionServiceMock) SendWithRetry(ctx context.Context, request *transaction.TxRequest, opts ...transaction.RetryOption) (common.Hash, *types.Receipt, error) { if m.sendWithRetry != nil { - return m.sendWithRetry(ctx, request) + return m.sendWithRetry(ctx, request, opts...) } return common.Hash{}, nil, errors.New("not implemented") } @@ -126,7 +126,7 @@ type optionFunc func(*transactionServiceMock) func (f optionFunc) apply(r *transactionServiceMock) { f(r) } -func WithSendWithRetryFunc(f func(context.Context, *transaction.TxRequest) (common.Hash, *types.Receipt, error)) Option { +func WithSendWithRetryFunc(f func(context.Context, *transaction.TxRequest, ...transaction.RetryOption) (common.Hash, *types.Receipt, error)) Option { return optionFunc(func(s *transactionServiceMock) { s.sendWithRetry = f })