Skip to content

Add metric to count iterations taken for eth_estimateGas #725

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
m-Peter merged 2 commits into from
Jan 28, 2025
Merged

Add metric to count iterations taken for eth_estimateGas #725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
00384a8
Add metric to count iterations taken for eth_estimateGas
m-Peter Jan 15, 2025
b49b795
Simplify dry-run of tx in EstimateGas
m-Peter Jan 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 33 additions & 20 deletions metrics/collector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,24 @@ type Collector interface {
MeasureRequestDuration(start time.Time, method string)
OperatorBalance(account *flow.Account)
AvailableSigningKeys(count int)
GasEstimationIterations(count int)
}

var _ Collector = &DefaultCollector{}

type DefaultCollector struct {
// TODO: for now we cannot differentiate which api request failed number of times
apiErrorsCounter prometheus.Counter
serverPanicsCounters *prometheus.CounterVec
cadenceBlockHeight prometheus.Gauge
evmBlockHeight prometheus.Gauge
evmBlockIndexedCounter prometheus.Counter
evmTxIndexedCounter prometheus.Counter
operatorBalance prometheus.Gauge
evmAccountCallCounters *prometheus.CounterVec
requestDurations *prometheus.HistogramVec
availableSigningkeys prometheus.Gauge
apiErrorsCounter prometheus.Counter
serverPanicsCounters *prometheus.CounterVec
cadenceBlockHeight prometheus.Gauge
evmBlockHeight prometheus.Gauge
evmBlockIndexedCounter prometheus.Counter
evmTxIndexedCounter prometheus.Counter
operatorBalance prometheus.Gauge
evmAccountCallCounters *prometheus.CounterVec
requestDurations *prometheus.HistogramVec
availableSigningkeys prometheus.Gauge
gasEstimationIterations prometheus.Gauge
}

func NewCollector(logger zerolog.Logger) Collector {
Expand Down Expand Up @@ -90,6 +92,11 @@ func NewCollector(logger zerolog.Logger) Collector {
Help: "Number of keys available for transaction signing",
})

gasEstimationIterations := prometheus.NewGauge(prometheus.GaugeOpts{
Name: prefixedName("gas_estimation_iterations"),
Help: "Number of iterations taken to estimate the gas of a EVM call/tx",
})

metrics := []prometheus.Collector{
apiErrors,
serverPanicsCounters,
Expand All @@ -101,23 +108,25 @@ func NewCollector(logger zerolog.Logger) Collector {
evmAccountCallCounters,
requestDurations,
availableSigningKeys,
gasEstimationIterations,
}
if err := registerMetrics(logger, metrics...); err != nil {
logger.Info().Msg("using noop collector as metric register failed")
return NopCollector
}

return &DefaultCollector{
apiErrorsCounter: apiErrors,
serverPanicsCounters: serverPanicsCounters,
cadenceBlockHeight: cadenceBlockHeight,
evmBlockHeight: evmBlockHeight,
evmBlockIndexedCounter: evmBlockIndexedCounter,
evmTxIndexedCounter: evmTxIndexedCounter,
evmAccountCallCounters: evmAccountCallCounters,
requestDurations: requestDurations,
operatorBalance: operatorBalance,
availableSigningkeys: availableSigningKeys,
apiErrorsCounter: apiErrors,
serverPanicsCounters: serverPanicsCounters,
cadenceBlockHeight: cadenceBlockHeight,
evmBlockHeight: evmBlockHeight,
evmBlockIndexedCounter: evmBlockIndexedCounter,
evmTxIndexedCounter: evmTxIndexedCounter,
evmAccountCallCounters: evmAccountCallCounters,
requestDurations: requestDurations,
operatorBalance: operatorBalance,
availableSigningkeys: availableSigningKeys,
gasEstimationIterations: gasEstimationIterations,
}
}

Expand Down Expand Up @@ -172,6 +181,10 @@ func (c *DefaultCollector) AvailableSigningKeys(count int) {
c.availableSigningkeys.Set(float64(count))
}

func (c *DefaultCollector) GasEstimationIterations(count int) {
c.gasEstimationIterations.Set(float64(count))
}

func prefixedName(name string) string {
return fmt.Sprintf("evm_gateway_%s", name)
}
1 change: 1 addition & 0 deletions metrics/nop.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ func (c *nopCollector) EVMAccountInteraction(string) {}
func (c *nopCollector) MeasureRequestDuration(time.Time, string) {}
func (c *nopCollector) OperatorBalance(*flow.Account) {}
func (c *nopCollector) AvailableSigningKeys(count int) {}
func (c *nopCollector) GasEstimationIterations(count int) {}
25 changes: 19 additions & 6 deletions services/requester/requester.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,15 @@ func (e *EVM) EstimateGas(
height uint64,
stateOverrides *ethTypes.StateOverride,
) (uint64, error) {
iterations := 0

dryRun := func(gasLimit uint64) (*evmTypes.Result, error) {
tx.Gas = gasLimit
result, err := e.dryRunTx(tx, from, height, stateOverrides, nil)
iterations += 1
return result, err
}

Copy link
Copy Markdown
Contributor

@janezpodhostnik janezpodhostnik Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe might be easier to read if you introduce a closure here:

dryRun := func(gasLimit ??) (??, error) {
   tx.Gas = gasLimit
   result, err := e.dryRunTx(tx, from, height, stateOverrides, nil)
   iterations += 1
   return result, err
}

Copy link
Copy Markdown
Collaborator Author

@m-Peter m-Peter Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's neat 💯 💯 💯 Updated in c9cb612.

// Note: The following algorithm, is largely inspired from
// https://github.com/onflow/go-ethereum/blob/master/eth/gasestimator/gasestimator.go#L49-L192,
// and adapted to fit our use-case.
Expand All @@ -323,10 +332,10 @@ func (e *EVM) EstimateGas(
if tx.Gas >= gethParams.TxGas {
passingGasLimit = tx.Gas
}
tx.Gas = passingGasLimit

// We first execute the transaction at the highest allowable gas limit,
// since if this fails we can return the error immediately.
result, err := e.dryRunTx(tx, from, height, stateOverrides, nil)
result, err := dryRun(passingGasLimit)
if err != nil {
return 0, err
}
Expand All @@ -338,6 +347,12 @@ func (e *EVM) EstimateGas(
return 0, errs.NewFailedTransactionError(resultSummary.ErrorMessage)
}

// We do not want to report iterations for calls/transactions
// that errored out or had their execution reverted.
defer func() {
e.collector.GasEstimationIterations(iterations)
}()

// For almost any transaction, the gas consumed by the unconstrained execution
// above lower-bounds the gas limit required for it to succeed. One exception
// is those that explicitly check gas remaining in order to execute within a
Expand All @@ -350,8 +365,7 @@ func (e *EVM) EstimateGas(
// Explicitly check that gas amount and use as a limit for the binary search.
optimisticGasLimit := (result.GasConsumed + result.GasRefund + gethParams.CallStipend) * 64 / 63
if optimisticGasLimit < passingGasLimit {
tx.Gas = optimisticGasLimit
result, err = e.dryRunTx(tx, from, height, stateOverrides, nil)
result, err := dryRun(optimisticGasLimit)
if err != nil {
// This should not happen under normal conditions since if we make it this far the
// transaction had run without error at least once before.
Expand Down Expand Up @@ -380,8 +394,7 @@ func (e *EVM) EstimateGas(
// range here is skewed to favor the low side.
mid = failingGasLimit * 2
}
tx.Gas = mid
result, err = e.dryRunTx(tx, from, height, stateOverrides, nil)
result, err := dryRun(mid)
if err != nil {
return 0, err
}
Expand Down