Skip to content

Commit

Permalink
Added EstimatedGasBuffer and addressed feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
amit-momin committed Aug 14, 2024
1 parent e09a7ca commit 4305ee3
Show file tree
Hide file tree
Showing 26 changed files with 362 additions and 59 deletions.
1 change: 1 addition & 0 deletions common/fee/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ var (
ErrBumpFeeExceedsLimit = errors.New("fee bump exceeds limit")
ErrBump = errors.New("fee bump failed")
ErrConnectivity = errors.New("transaction propagation issue: transactions are not being mined")
ErrFeeLimitTooLow = errors.New("provided fee limit too low")
)

func IsBumpErr(err error) bool {
Expand Down
7 changes: 6 additions & 1 deletion common/txmgr/broadcaster.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/smartcontractkit/chainlink-common/pkg/utils"

"github.com/smartcontractkit/chainlink/v2/common/client"
commonfee "github.com/smartcontractkit/chainlink/v2/common/fee"
feetypes "github.com/smartcontractkit/chainlink/v2/common/fee/types"
txmgrtypes "github.com/smartcontractkit/chainlink/v2/common/txmgr/types"
"github.com/smartcontractkit/chainlink/v2/common/types"
Expand Down Expand Up @@ -434,7 +435,11 @@ func (eb *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) hand
}

attempt, _, _, retryable, err := eb.NewTxAttempt(ctx, *etx, eb.lggr)
if err != nil {
// Mark transaction as fatal if provided gas limit is set too low
if errors.Is(err, commonfee.ErrFeeLimitTooLow) {
etx.Error = null.StringFrom(commonfee.ErrFeeLimitTooLow.Error())
return eb.saveFatallyErroredTransaction(eb.lggr, etx), false
} else if err != nil {
return fmt.Errorf("processUnstartedTxs failed on NewAttempt: %w", err), retryable
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,7 @@ func (g *TestGasEstimatorConfig) PriceMaxKey(addr common.Address) *assets.Wei {
return assets.GWei(1)
}
func (g *TestGasEstimatorConfig) EstimateGasLimit() bool { return false }
func (g *TestGasEstimatorConfig) EstimatedGasBuffer() float32 { return 1.25}

func (e *TestEvmConfig) GasEstimator() evmconfig.GasEstimator {
return &TestGasEstimatorConfig{bumpThreshold: e.BumpThreshold}
Expand Down
5 changes: 5 additions & 0 deletions core/chains/evm/config/chain_scoped_gas_estimator.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ func (g *gasEstimatorConfig) EstimateGasLimit() bool {
return *g.c.EstimateGasLimit
}

func (g *gasEstimatorConfig) EstimatedGasBuffer() float32 {
f, _ := g.c.EstimatedGasBuffer.BigFloat().Float32()
return f
}

type limitJobTypeConfig struct {
c toml.GasLimitJobType
}
Expand Down
1 change: 1 addition & 0 deletions core/chains/evm/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ type GasEstimator interface {
Mode() string
PriceMaxKey(gethcommon.Address) *assets.Wei
EstimateGasLimit() bool
EstimatedGasBuffer() float32
}

type LimitJobType interface {
Expand Down
45 changes: 45 additions & 0 deletions core/chains/evm/config/mocks/gas_estimator.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 10 additions & 6 deletions core/chains/evm/config/toml/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -541,12 +541,13 @@ type GasEstimator struct {
PriceMax *assets.Wei
PriceMin *assets.Wei

LimitDefault *uint64
LimitMax *uint64
LimitMultiplier *decimal.Decimal
LimitTransfer *uint64
LimitJobType GasLimitJobType `toml:",omitempty"`
EstimateGasLimit *bool
LimitDefault *uint64
LimitMax *uint64
LimitMultiplier *decimal.Decimal
LimitTransfer *uint64
LimitJobType GasLimitJobType `toml:",omitempty"`
EstimateGasLimit *bool
EstimatedGasBuffer *decimal.Decimal

BumpMin *assets.Wei
BumpPercent *uint16
Expand Down Expand Up @@ -637,6 +638,9 @@ func (e *GasEstimator) setFrom(f *GasEstimator) {
if v := f.EstimateGasLimit; v != nil {
e.EstimateGasLimit = v
}
if v := f.EstimatedGasBuffer; v != nil {
e.EstimatedGasBuffer = v
}
if v := f.PriceDefault; v != nil {
e.PriceDefault = v
}
Expand Down
1 change: 1 addition & 0 deletions core/chains/evm/config/toml/defaults/fallback.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ FeeCapDefault = '100 gwei'
TipCapDefault = '1'
TipCapMin = '1'
EstimateGasLimit = false
EstimatedGasBuffer = 1.25

[GasEstimator.BlockHistory]
BatchSize = 25
Expand Down
5 changes: 5 additions & 0 deletions core/chains/evm/gas/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ type MockGasEstimatorConfig struct {
LimitMaxF uint64
ModeF string
EstimateGasLimitF bool
EstimatedGasBufferF float32
}

func NewMockGasConfig() *MockGasEstimatorConfig {
Expand Down Expand Up @@ -219,3 +220,7 @@ func (m *MockGasEstimatorConfig) Mode() string {
func (m *MockGasEstimatorConfig) EstimateGasLimit() bool {
return m.EstimateGasLimitF
}

func (m *MockGasEstimatorConfig) EstimatedGasBuffer() float32 {
return m.EstimatedGasBufferF
}
4 changes: 2 additions & 2 deletions core/chains/evm/gas/mocks/evm_fee_estimator.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

84 changes: 59 additions & 25 deletions core/chains/evm/gas/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type EvmFeeEstimator interface {

// L1Oracle returns the L1 gas price oracle only if the chain has one, e.g. OP stack L2s and Arbitrum.
L1Oracle() rollups.L1Oracle
GetFee(ctx context.Context, calldata []byte, feeLimit uint64, maxFeePrice *assets.Wei, toAddress *common.Address, opts ...feetypes.Opt) (fee EvmFee, chainSpecificFeeLimit uint64, err error)
GetFee(ctx context.Context, calldata []byte, feeLimit uint64, maxFeePrice *assets.Wei, toAddress *common.Address, opts ...feetypes.Opt) (fee EvmFee, estimatedFeeLimit uint64, err error)
BumpFee(ctx context.Context, originalFee EvmFee, feeLimit uint64, maxFeePrice *assets.Wei, attempts []EvmPriorAttempt) (bumpedFee EvmFee, chainSpecificFeeLimit uint64, err error)

// GetMaxCost returns the total value = max price x fee units + transferred value
Expand Down Expand Up @@ -71,6 +71,8 @@ func NewEstimator(lggr logger.Logger, ethClient feeEstimatorClient, cfg Config,
"tipCapMin", geCfg.TipCapMin(),
"priceMax", geCfg.PriceMax(),
"priceMin", geCfg.PriceMin(),
"estimateGasLimit", geCfg.EstimateGasLimit(),
"estimatedGasBuffer", geCfg.EstimatedGasBuffer(),
)
df := geCfg.EIP1559DynamicFees()

Expand Down Expand Up @@ -264,13 +266,10 @@ func (e *evmFeeEstimator) L1Oracle() rollups.L1Oracle {
return e.EvmEstimator.L1Oracle()
}

func (e *evmFeeEstimator) GetFee(ctx context.Context, calldata []byte, feeLimit uint64, maxFeePrice *assets.Wei, toAddress *common.Address, opts ...feetypes.Opt) (fee EvmFee, chainSpecificFeeLimit uint64, err error) {
// Create call msg for gas limit estimation, if needed
callMsg := ethereum.CallMsg{
To: toAddress,
Data: calldata,
}

// GetFee returns an initial estimated gas price and gas limit for a transaction
// The gas limit provided by the caller can be adjusted by gas estimation or for 2D fees
func (e *evmFeeEstimator) GetFee(ctx context.Context, calldata []byte, feeLimit uint64, maxFeePrice *assets.Wei, toAddress *common.Address, opts ...feetypes.Opt) (fee EvmFee, estimatedFeeLimit uint64, err error) {
var chainSpecificFeeLimit uint64
// get dynamic fee
if e.EIP1559Enabled {
var dynamicFee DynamicFee
Expand All @@ -281,31 +280,15 @@ func (e *evmFeeEstimator) GetFee(ctx context.Context, calldata []byte, feeLimit
fee.DynamicFeeCap = dynamicFee.FeeCap
fee.DynamicTipCap = dynamicFee.TipCap
chainSpecificFeeLimit = feeLimit
// Set call msg fields with dynamic fee fields for gas limit estimation, if needed
callMsg.GasFeeCap = fee.DynamicFeeCap.ToInt()
callMsg.GasTipCap = fee.DynamicTipCap.ToInt()
} else {
// get legacy fee
fee.Legacy, chainSpecificFeeLimit, err = e.EvmEstimator.GetLegacyGas(ctx, calldata, feeLimit, maxFeePrice, opts...)
if err != nil {
return
}
// Set call msg fields with legacy fee field for gas limit estimation, if needed
callMsg.GasPrice = fee.Legacy.ToInt()
}

if e.geCfg.EstimateGasLimit() {
callMsg.Gas = chainSpecificFeeLimit
estimatedGasLimit, estimateErr := e.ethClient.EstimateGas(ctx, callMsg)
if estimateErr != nil {
// Do not return error if estimating gas failed, we can still use the provided limit instead since it is an upper limit
e.lggr.Errorw("failed to estimate gas limit. falling back to provided gas limit.", "toAddress", toAddress, "data", calldata, "gasLimit", chainSpecificFeeLimit, "error", estimateErr)
} else {
chainSpecificFeeLimit = estimatedGasLimit
}
}

chainSpecificFeeLimit, err = commonfee.ApplyMultiplier(chainSpecificFeeLimit, e.geCfg.LimitMultiplier())
estimatedFeeLimit, err = e.estimateFeeLimit(ctx, fee, chainSpecificFeeLimit, calldata, toAddress)
return
}

Expand Down Expand Up @@ -361,6 +344,56 @@ func (e *evmFeeEstimator) BumpFee(ctx context.Context, originalFee EvmFee, feeLi
return
}

func (e *evmFeeEstimator) estimateFeeLimit(ctx context.Context, fee EvmFee, feeLimit uint64, calldata []byte, toAddress *common.Address) (estimatedFeeLimit uint64, err error) {
// Use provided fee limit by default is EstimateGasLimit is disabled
if !e.geCfg.EstimateGasLimit() {
return commonfee.ApplyMultiplier(feeLimit, e.geCfg.LimitMultiplier())
}

// Create call msg for gas limit estimation
// Skip setting Gas to avoid capping the results of the estimation
callMsg := ethereum.CallMsg{
To: toAddress,
Data: calldata,
}
if e.EIP1559Enabled {
callMsg.GasFeeCap = fee.DynamicFeeCap.ToInt()
callMsg.GasTipCap = fee.DynamicTipCap.ToInt()
} else {
// Set call msg fields with legacy fee field for gas limit estimation, if needed
callMsg.GasPrice = fee.Legacy.ToInt()
}
e.lggr.Debugw("estimating gas limit", "callMsg", callMsg)
gasUsed, estimateErr := e.ethClient.EstimateGas(ctx, callMsg)
if estimateErr != nil {
// Do not return error if estimate gas failed, we can still use the provided limit instead since it is an upper limit
e.lggr.Errorw("failed to estimate gas limit. falling back to provided gas limit.", "callMsg", callMsg, "providedGasLimit", feeLimit, "error", estimateErr)
estimatedFeeLimit, err = commonfee.ApplyMultiplier(feeLimit, e.geCfg.LimitMultiplier())
return
}
// Return error if estimated gas without the buffer exceeds the provided gas limit
// Transaction is destined to run out of gas and fail
if gasUsed > feeLimit {
e.lggr.Errorw("estimated gas limit exceeds provided limit", "estimatedGasLimit", gasUsed, "providedGasLimit", feeLimit)
return estimatedFeeLimit, commonfee.ErrFeeLimitTooLow
}
// Apply EstimatedGasBuffer multiplier to the estimated gas limit
estimatedFeeLimit, err = commonfee.ApplyMultiplier(gasUsed, e.geCfg.EstimatedGasBuffer())
if err != nil {
return
}

// Fallback to the provided gas limit if the buffer causes the estimated gas limit to exceed it
// The provided gas limit should be used as an upper bound to avoid unexpected behavior for products
if estimatedFeeLimit > feeLimit {
e.lggr.Debugw("estimated gas limit with buffer exceeds provided limit. falling back to the provided gas limit", "estimatedGasLimit", estimatedFeeLimit, "providedGasLimit", feeLimit)
estimatedFeeLimit = feeLimit
}

estimatedFeeLimit, err = commonfee.ApplyMultiplier(estimatedFeeLimit, e.geCfg.LimitMultiplier())
return
}

// Config defines an interface for configuration in the gas package
type Config interface {
ChainType() chaintype.ChainType
Expand All @@ -383,6 +416,7 @@ type GasEstimatorConfig interface {
PriceMax() *assets.Wei
Mode() string
EstimateGasLimit() bool
EstimatedGasBuffer() float32
}

type BlockHistoryConfig interface {
Expand Down
Loading

0 comments on commit 4305ee3

Please sign in to comment.