-
Notifications
You must be signed in to change notification settings - Fork 33
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.
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
+dynamic_gas_unit_add_percentage flag #3494
Changes from all commits
dfbf6ba
93459f1
905a3a5
09816fb
63f730c
31ca3cf
d8481f2
d7f1cad
3d7eea2
4c11d33
145583d
2005d91
a8a48bb
75c9c44
96e16c3
dada2b0
7abfdb0
0e7c236
220e0d4
b995385
4782b44
d699cd0
04ccdc2
c8f2dfe
d199850
e8fd10e
ee9eb85
f6bfa43
c79a81b
82b4f99
5b02cca
965802d
7b62df2
73442fa
ab18ea9
cf43fdc
c24c568
0e35f40
8adb278
961fbfc
6586d73
528839f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -77,13 +77,15 @@ gas_bump_percentage: 10 | |
gas_estimate: 1000 | ||
is_l2: true | ||
dynamic_gas_estimate: true | ||
dynamic_gas_unit_add_percentage: 20 | ||
supports_eip_1559: true` | ||
var cfg config.Config | ||
err := yaml.Unmarshal([]byte(cfgStr), &cfg) | ||
assert.NoError(t, err) | ||
assert.Equal(t, big.NewInt(250000000000), cfg.MaxGasPrice) | ||
assert.Equal(t, 60, cfg.BumpIntervalSeconds) | ||
assert.Equal(t, 10, cfg.GasBumpPercentage) | ||
assert.Equal(t, 20, cfg.DynamicGasUnitAddPercentage) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance test coverage for the new configuration parameter. While the basic YAML parsing test is good, consider adding the following test cases:
Here's a suggested implementation: func TestGetters(t *testing.T) {
cfg := config.Config{
ChainConfig: config.ChainConfig{
MaxBatchSize: 5,
DoNotBatch: false,
MaxGasPrice: big.NewInt(250 * params.GWei),
+ DynamicGasUnitAddPercentage: 15,
},
Chains: map[int]config.ChainConfig{
MaxBatchSize: 8,
DoNotBatch: true,
MaxGasPrice: big.NewInt(300 * params.GWei),
+ DynamicGasUnitAddPercentage: 25,
},
MaxBatchSize: 0, // Should use global config value
},
},
}
// ... existing test cases ...
+ t.Run("GetDynamicGasUnitAddPercentage", func(t *testing.T) {
+ // Test chain-specific value
+ assert.Equal(t, 25, cfg.GetDynamicGasUnitAddPercentage(1))
+ // Test fallback to global value
+ assert.Equal(t, 15, cfg.GetDynamicGasUnitAddPercentage(2))
+ // Test nonexistent chain
+ assert.Equal(t, 15, cfg.GetDynamicGasUnitAddPercentage(999))
+
+ // Test default value
+ emptyCfg := config.Config{}
+ assert.Equal(t, config.DefaultDynamicGasUnitAddPercentage,
+ emptyCfg.GetDynamicGasUnitAddPercentage(1))
+
+ // Test validation (if implemented)
+ invalidCfg := config.Config{
+ ChainConfig: config.ChainConfig{
+ DynamicGasUnitAddPercentage: -1,
+ },
+ }
+ assert.Equal(t, config.DefaultDynamicGasUnitAddPercentage,
+ invalidCfg.GetDynamicGasUnitAddPercentage(1))
+ })
}
|
||
assert.Equal(t, uint64(1000), cfg.GasEstimate) | ||
assert.Equal(t, true, cfg.DynamicGasEstimate) | ||
assert.Equal(t, true, cfg.SupportsEIP1559(0)) | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -392,8 +392,23 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
span.AddEvent("could not set gas price", trace.WithAttributes(attribute.String("error", err.Error()))) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if !t.config.GetDynamicGasEstimate(int(chainID.Uint64())) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
transactor.GasLimit = t.config.GetGasEstimate(int(chainID.Uint64())) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
performSignature := func(address common.Address, transaction *types.Transaction) (_ *types.Transaction, err error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
newNonce, err := t.getNonce(ctx, chainID, address) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return nil, fmt.Errorf("could not sign tx: %w", err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
txType := t.txTypeForChain(chainID) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
transaction, err = util.CopyTX(transaction, util.WithNonce(newNonce), util.WithTxType(txType)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return nil, fmt.Errorf("could not copy tx: %w", err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
//nolint: wrapcheck | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return parentTransactor.Signer(address, transaction) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
transactor.Signer = func(address common.Address, transaction *types.Transaction) (_ *types.Transaction, err error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -406,25 +421,42 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
newNonce, err := t.getNonce(ctx, chainID, address) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return nil, fmt.Errorf("could not sign tx: %w", err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return performSignature(address, transaction) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
txType := t.txTypeForChain(chainID) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// if dynamic gas estimation is not enabled, use cfg var gas_estimate as a gas limit default and do not run a pre-flight simulation | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// since we do not need it to determine proper gas units | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if !t.config.GetDynamicGasEstimate(int(chainID.Uint64())) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
parodime marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
transactor.GasLimit = t.config.GetGasEstimate(int(chainID.Uint64())) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
parodime marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+429
to
+431
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Protect against integer overflow in chain ID conversion. The conversion of - if !t.config.GetDynamicGasEstimate(int(chainID.Uint64())) {
- transactor.GasLimit = t.config.GetGasEstimate(int(chainID.Uint64()))
+ chainIDInt := chainID.Int64()
+ if chainIDInt > math.MaxInt32 {
+ return 0, fmt.Errorf("chain ID %d exceeds maximum supported value", chainIDInt)
+ }
+ if !t.config.GetDynamicGasEstimate(int(chainIDInt)) {
+ transactor.GasLimit = t.config.GetGasEstimate(int(chainIDInt)) 📝 Committable suggestion
Suggested change
🧰 Tools🪛 golangci-lint (1.62.2)[high] 424-424: G115: integer overflow conversion uint64 -> int (gosec) [high] 425-425: G115: integer overflow conversion uint64 -> int (gosec) 426-426: unnecessary leading newline (whitespace) 🪛 GitHub Check: Lint (ethergo)[failure] 424-424: [failure] 425-425: [failure] 426-426: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
transaction, err = util.CopyTX(transaction, util.WithNonce(newNonce), util.WithTxType(txType)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// deepcopy the real transactor so we can use it for simulation | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
transactorForGasEstimate := copyTransactOpts(transactor) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// override the signer func for our simulation/estimation with a version that does not lock the nonce, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// which would othewrise cause a deadlock with the following *actual* transactor | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
transactorForGasEstimate.Signer = func(address common.Address, transaction *types.Transaction) (_ *types.Transaction, err error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return performSignature(address, transaction) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
txForGasEstimate, err := call(transactorForGasEstimate) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return nil, fmt.Errorf("could not copy tx: %w", err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
errMsg := util.FormatError(err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return 0, fmt.Errorf("err contract call for gas est: %s", errMsg) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
//nolint: wrapcheck | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return parentTransactor.Signer(address, transaction) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// with our gas limit now obtained from the simulation, apply this limit (plus any configured % modifier) to the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// gas limit of the actual transactor that is about to prepare the real transaction | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
gasLimitAddPercentage := t.config.GetDynamicGasUnitAddPercentage(int(chainID.Uint64())) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
transactor.GasLimit = txForGasEstimate.Gas() + (txForGasEstimate.Gas() * uint64(gasLimitAddPercentage) / 100) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Check failure on line 452 in ethergo/submitter/submitter.go
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+451
to
+452
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Protect against integer overflow in gas limit calculation. The gas limit percentage calculation could overflow when dealing with large gas values. - gasLimitAddPercentage := t.config.GetDynamicGasUnitAddPercentage(int(chainID.Uint64()))
- transactor.GasLimit = txForGasEstimate.Gas() + (txForGasEstimate.Gas() * uint64(gasLimitAddPercentage) / 100)
+ chainIDInt := chainID.Int64()
+ if chainIDInt > math.MaxInt32 {
+ return 0, fmt.Errorf("chain ID %d exceeds maximum supported value", chainIDInt)
+ }
+ gasLimitAddPercentage := t.config.GetDynamicGasUnitAddPercentage(int(chainIDInt))
+
+ // Use big.Int for safe calculation
+ gasLimit := new(big.Int).SetUint64(txForGasEstimate.Gas())
+ increase := new(big.Int).Mul(gasLimit, big.NewInt(int64(gasLimitAddPercentage)))
+ increase.Div(increase, big.NewInt(100))
+ if !increase.IsUint64() {
+ return 0, fmt.Errorf("gas limit increase exceeds uint64 max value")
+ }
+ transactor.GasLimit = txForGasEstimate.Gas() + increase.Uint64() 📝 Committable suggestion
Suggested change
🧰 Tools🪛 golangci-lint (1.62.2)[high] 460-460: G115: integer overflow conversion uint64 -> int (gosec) [high] 461-461: G115: integer overflow conversion int -> uint64 (gosec) 🪛 GitHub Check: codecov/patch[warning] 460-460: ethergo/submitter/submitter.go#L460 🪛 GitHub Check: Lint (ethergo)[failure] 460-460: [failure] 461-461:
Comment on lines
+427
to
+452
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Protect against multiple integer overflows. The code has several potential integer overflow issues:
Apply this diff to fix the overflow risks: + chainIDInt := chainID.Int64()
+ if chainIDInt > math.MaxInt32 {
+ return 0, fmt.Errorf("chain ID %d exceeds maximum supported value", chainIDInt)
+ }
- if !t.config.GetDynamicGasEstimate(int(chainID.Uint64())) {
- transactor.GasLimit = t.config.GetGasEstimate(int(chainID.Uint64()))
+ if !t.config.GetDynamicGasEstimate(int(chainIDInt)) {
+ transactor.GasLimit = t.config.GetGasEstimate(int(chainIDInt))
} else {
- gasLimitAddPercentage := t.config.GetDynamicGasUnitAddPercentage(int(chainID.Uint64()))
- transactor.GasLimit = txForGasEstimate.Gas() + (txForGasEstimate.Gas() * uint64(gasLimitAddPercentage) / 100)
+ gasLimitAddPercentage := t.config.GetDynamicGasUnitAddPercentage(int(chainIDInt))
+ gasLimit := new(big.Int).SetUint64(txForGasEstimate.Gas())
+ increase := new(big.Int).Mul(gasLimit, big.NewInt(int64(gasLimitAddPercentage)))
+ increase.Div(increase, big.NewInt(100))
+ if !increase.IsUint64() {
+ return 0, fmt.Errorf("gas limit increase exceeds uint64 max value")
+ }
+ transactor.GasLimit = txForGasEstimate.Gas() + increase.Uint64()
} 📝 Committable suggestion
Suggested change
🧰 Tools🪛 golangci-lint (1.62.2)[high] 429-429: G115: integer overflow conversion uint64 -> int (gosec) [high] 451-451: G115: integer overflow conversion uint64 -> int (gosec) [high] 452-452: G115: integer overflow conversion int -> uint64 (gosec) [high] 430-430: G115: integer overflow conversion uint64 -> int (gosec) 431-431: unnecessary leading newline (whitespace) 🪛 GitHub Check: codecov/patch[warning] 433-438: ethergo/submitter/submitter.go#L433-L438 [warning] 442-442: ethergo/submitter/submitter.go#L442 [warning] 444-446: ethergo/submitter/submitter.go#L444-L446 [warning] 451-452: ethergo/submitter/submitter.go#L451-L452 🪛 GitHub Check: Lint (ethergo)[failure] 429-429: [failure] 451-451: [failure] 452-452: [failure] 430-430: [failure] 431-431: 💡 Codebase verification Test coverage for dynamic gas estimation is indeed missing The codebase lacks proper test coverage for the dynamic gas estimation functionality. Critical areas requiring test coverage:
🔗 Analysis chainAdd test coverage for dynamic gas estimation. The new gas estimation logic lacks test coverage, particularly for:
Run this script to verify test coverage: 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check test coverage for gas estimation logic
echo "=== Checking test coverage ==="
go test -coverprofile=coverage.out ./ethergo/submitter/
go tool cover -func=coverage.out | grep "getGasEstimate"
echo -e "\n=== Searching for gas estimation tests ==="
rg -l "TestGetGasEstimate|TestDynamicGasEstimate" --type go --glob "*_test.go"
Length of output: 13718 Script: #!/bin/bash
# Search for test files and their content related to gas estimation
echo "=== Searching for gas estimation related test files ==="
rg -l "gas.*estimate" --type go --glob "*_test.go"
echo -e "\n=== Searching for gas estimation test cases ==="
rg "func.*[Tt]est.*[Gg]as.*[Ee]stimate" --type go --glob "*_test.go" -A 5
echo -e "\n=== Searching for dynamic gas configuration tests ==="
rg "DynamicGasEstimate|GetDynamicGasEstimate" --type go --glob "*_test.go" -A 5
Length of output: 868 🧰 Tools🪛 golangci-lint (1.62.2)[high] 429-429: G115: integer overflow conversion uint64 -> int (gosec) [high] 451-451: G115: integer overflow conversion uint64 -> int (gosec) [high] 452-452: G115: integer overflow conversion int -> uint64 (gosec) [high] 430-430: G115: integer overflow conversion uint64 -> int (gosec) 431-431: unnecessary leading newline (whitespace) 🪛 GitHub Check: codecov/patch[warning] 433-438: ethergo/submitter/submitter.go#L433-L438 [warning] 442-442: ethergo/submitter/submitter.go#L442 [warning] 444-446: ethergo/submitter/submitter.go#L444-L446 [warning] 451-452: ethergo/submitter/submitter.go#L451-L452 🪛 GitHub Check: Lint (ethergo)[failure] 429-429: [failure] 451-451: [failure] 452-452: [failure] 430-430: [failure] 431-431: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
tx, err := call(transactor) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return 0, fmt.Errorf("could not call contract: %w", err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return 0, fmt.Errorf("err contract call for tx: %w", err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
defer locker.Unlock() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// now that we've stored the tx | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -676,18 +708,23 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// getGasEstimate gets the gas estimate for the given transaction. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// TODO: handle l2s w/ custom gas pricing through contracts. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func (t *txSubmitterImpl) getGasEstimate(ctx context.Context, chainClient client.EVM, chainID int, tx *types.Transaction) (gasEstimate uint64, err error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func (t *txSubmitterImpl) getGasEstimate(ctx context.Context, chainClient client.EVM, chainID int, tx *types.Transaction) (gasLimit uint64, err error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// if dynamic gas estimation is not enabled, use cfg var gas_estimate as a default | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if !t.config.GetDynamicGasEstimate(chainID) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return t.config.GetGasEstimate(chainID), nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
gasUnitAddPercentage := t.config.GetDynamicGasUnitAddPercentage(chainID) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ctx, span := t.metrics.Tracer().Start(ctx, "submitter.getGasEstimate", trace.WithAttributes( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
attribute.Int(metrics.ChainID, chainID), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
attribute.String(metrics.TxHash, tx.Hash().String()), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
attribute.Int("gasUnitAddPercentage", gasUnitAddPercentage), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
parodime marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
defer func() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
span.AddEvent("estimated_gas", trace.WithAttributes(attribute.Int64("gas", int64(gasEstimate)))) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
span.AddEvent("estimated_gas", trace.WithAttributes(attribute.Int64("gas", int64(gasLimit)))) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Check failure on line 727 in ethergo/submitter/submitter.go
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Use string attributes for large gas values. Converting gas values to int64 in trace attributes could overflow. Apply this diff: - span.AddEvent("estimated_gas", trace.WithAttributes(attribute.Int64("gas", int64(gasLimit))))
+ span.AddEvent("estimated_gas", trace.WithAttributes(attribute.String("gas", fmt.Sprintf("%d", gasLimit)))) 📝 Committable suggestion
Suggested change
🧰 Tools🪛 golangci-lint (1.62.2)[high] 727-727: G115: integer overflow conversion uint64 -> int64 (gosec) 🪛 GitHub Check: codecov/patch[warning] 727-727: ethergo/submitter/submitter.go#L727 🪛 GitHub Check: Lint (ethergo)[failure] 727-727: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
metrics.EndSpanWithErr(span, err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -696,21 +733,21 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return 0, fmt.Errorf("could not convert tx to call: %w", err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// tmpdebug | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fmt.Printf("Debug Calling EstimateGas") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
gasEstimate, err = chainClient.EstimateGas(ctx, *call) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
gasLimitFromEstimate, err := chainClient.EstimateGas(ctx, *call) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
span.AddEvent("could not estimate gas", trace.WithAttributes(attribute.String("error", err.Error()))) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// tmpdebug | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fmt.Printf("Debug Default Gas Estimate: %d\n", t.config.GetGasEstimate(chainID)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// fallback to default | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// if we failed to est gas for any reason, use the default flat gas from config | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return t.config.GetGasEstimate(chainID), nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return gasEstimate, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// multiply the freshly simulated gasLimit by the configured gas unit add percentage | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
gasLimitFromEstimate += (gasLimitFromEstimate * uint64(gasUnitAddPercentage) / 100) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
gasLimit = gasLimitFromEstimate | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+746
to
+748
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Protect against integer overflow in gas limit adjustment. The gas limit percentage calculation needs overflow protection. - gasLimitFromEstimate += (gasLimitFromEstimate * uint64(gasUnitAddPercentage) / 100)
+ // Use big.Int for safe calculation
+ gasLimitBig := new(big.Int).SetUint64(gasLimitFromEstimate)
+ increase := new(big.Int).Mul(gasLimitBig, big.NewInt(int64(gasUnitAddPercentage)))
+ increase.Div(increase, big.NewInt(100))
+ if !increase.IsUint64() {
+ return 0, fmt.Errorf("gas limit increase exceeds uint64 max value")
+ }
+ gasLimitFromEstimate = gasLimitFromEstimate + increase.Uint64() 📝 Committable suggestion
Suggested change
🧰 Tools🪛 golangci-lint (1.62.2)[high] 756-756: G115: integer overflow conversion int -> uint64 (gosec) 🪛 GitHub Check: Lint (ethergo)[failure] 756-756: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return gasLimit, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
func (t *txSubmitterImpl) Address() common.Address { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,17 @@ | ||||||
package util | ||||||
|
||||||
import "strings" | ||||||
|
||||||
// FormatError applies custom formatting & noise reduction to error messages. Add more as needed. | ||||||
func FormatError(err error) string { | ||||||
if err == nil { | ||||||
return "" | ||||||
} | ||||||
errMsg := err.Error() | ||||||
|
||||||
//if an error message contains embedded HTML (eg: many RPC errors), strip it out to reduce noise. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix comment formatting. Add a space after - //if an error message contains embedded HTML (eg: many RPC errors), strip it out to reduce noise.
+ // if an error message contains embedded HTML (eg: many RPC errors), strip it out to reduce noise. 📝 Committable suggestion
Suggested change
🧰 Tools🪛 golangci-lint (1.62.2)12-12: commentFormatting: put a space between (gocritic) 🪛 GitHub Check: Lint (ethergo)[failure] 12-12: |
||||||
if strings.Contains(errMsg, "<!DOCTYPE html>") { | ||||||
errMsg = strings.Split(errMsg, "<!DOCTYPE html>")[0] + "<html portion of error removed>" | ||||||
} | ||||||
return errMsg | ||||||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,10 +86,19 @@ func (c Chain) SubmitRelay(ctx context.Context, request reldb.QuoteRequest) (uin | |
} | ||
} | ||
|
||
fmt.Printf( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we trace this instead? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i just added these to help provide some high-level context at a glance in the times that we're observing logs. Not likely to add many more statements like these & will continue to primarily trace everything |
||
"TxID 0x%x %7d.%s > %7d.%s : Submitting \033[32mRelay\033[0m\n", | ||
request.TransactionID, | ||
request.Transaction.OriginChainId, | ||
request.Transaction.OriginToken.Hex()[:6], | ||
request.Transaction.DestChainId, | ||
request.Transaction.DestToken.Hex()[:6]) | ||
|
||
nonce, err := c.SubmitTransaction(ctx, func(transactor *bind.TransactOpts) (tx *types.Transaction, err error) { | ||
transactor.Value = core.CopyBigInt(gasAmount) | ||
|
||
tx, err = c.Bridge.RelayV2(transactor, request.RawRequest, c.submitter.Address()) | ||
|
||
if err != nil { | ||
return nil, fmt.Errorf("could not relay: %w", err) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure we don't want to default to zero? This is fine with me, just making sure since may not be obvious to others
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with zaps we're going to need to start doing dynamic estimates all the time and relying on the simulation to give us enough limit. a small boost by default on top of the simulation is a good idea just to further reduce chance of OOG
also -- our flat limits that we use today, by comparison, give us a boost of like +5000% so +5% is pretty small by that measure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference, most wallets are using a 50% buffer on top of the estimated gas limit. So I'd recommend increasing the default value a bit here.