Skip to content
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

feat(rfq-relayer): support FastBridgeV2 with arbitrary calls [SLT-320] #3258

Open
wants to merge 103 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
103 commits
Select commit Hold shift + click to select a range
36a42bd
feat: add `callValue` to bridge params
ChiTimesChi Oct 2, 2024
919a0a9
feat: add `callValue` to bridge tx V2
ChiTimesChi Oct 2, 2024
867d5d9
test: add cases for SRC calls with `callValue`
ChiTimesChi Oct 3, 2024
c93c403
refactor: simplify tests further
ChiTimesChi Oct 3, 2024
9575246
test: add cases for DST relays with `callValue`
ChiTimesChi Oct 3, 2024
7872f2a
feat: support `callValue` in `bridge()`
ChiTimesChi Oct 3, 2024
ad7dd4c
feat: support `callValue` in `relay()`
ChiTimesChi Oct 3, 2024
5ca5ad1
test: update revert message for failed ETH transfers
ChiTimesChi Oct 3, 2024
e8355c3
refactor: always forward full msg.value for the hook call
ChiTimesChi Oct 3, 2024
eb2bbb8
refactor: use `_pullToken` only in `bridge()`
ChiTimesChi Oct 3, 2024
c50042a
refactor: isolate validation of relay params
ChiTimesChi Oct 3, 2024
9fb4461
refactor: isolate validation of the bridge params
ChiTimesChi Oct 3, 2024
156e333
docs: could -> can
ChiTimesChi Oct 8, 2024
2b77b4a
test: enable the backwards compatibility encoding test
ChiTimesChi Oct 8, 2024
271f59d
fix: getBridgeTransaction partial support for V2 structs
ChiTimesChi Oct 8, 2024
2317a58
test: add clarifications about expected reverts
ChiTimesChi Oct 8, 2024
c6a1fdc
Feat: initial fastbridgev2 bindings
dwasse Oct 8, 2024
e5e8646
Feat: abigen helpers
dwasse Oct 8, 2024
ab63286
Fix: generate
dwasse Oct 8, 2024
93d9b7d
Fix: bridge enum test
dwasse Oct 8, 2024
e34a08b
Feat: relayer integrates fastbridgev2
dwasse Oct 8, 2024
1056ef1
Feat: testutils uses v2
dwasse Oct 8, 2024
1301270
Fix: deployer uses v2
dwasse Oct 8, 2024
1081e0a
Fix: current e2e tests with v2 bridge
dwasse Oct 8, 2024
467d5c7
Feat: add recipient mock test contract
dwasse Oct 9, 2024
f456bb7
Feat: add TestUSDCtoUSDCWithCallData
dwasse Oct 9, 2024
de621a7
Rename: TestArbitraryCall
dwasse Oct 9, 2024
e3c1604
Feat: add TransactionV1 in QuoteRequest struct for access to SendChai…
dwasse Oct 9, 2024
bff4f72
Cleanup: bridge tx fetching
dwasse Oct 9, 2024
4ed813b
Cleanup: remove guard check for now
dwasse Oct 9, 2024
5b6ec12
Feat: add v2 of fastbridgemock
dwasse Oct 11, 2024
d3dbeb0
Fix: build
dwasse Oct 11, 2024
0bc22c5
Cleanup: lint
dwasse Oct 11, 2024
117ce59
Cleanup: remove unnecessary test
dwasse Oct 11, 2024
6dee3d1
Fix: mock fast bridge deployer
dwasse Oct 11, 2024
f57312b
Revert "Cleanup: remove unnecessary test"
dwasse Oct 11, 2024
4b8dc68
Fix: flatten all files
dwasse Oct 11, 2024
b24d31d
Revert "Fix: flatten all files"
dwasse Oct 11, 2024
91a6b8f
Feat: flatten mocks
dwasse Oct 11, 2024
b6a4609
Fix: tests
dwasse Oct 11, 2024
05ab3dd
Fix: test
dwasse Oct 11, 2024
94ee810
Fix: use nativeTokenDecimals instead of origin decimals for call value
dwasse Oct 15, 2024
337782c
Merge branch 'master' into feat/relayer-arb-call
dwasse Oct 15, 2024
8759318
Update abigen
dwasse Oct 15, 2024
f44c7ae
Fix: use native token decimals for CallValue
dwasse Oct 16, 2024
978313d
Feat: relayer uses v2 for relay() / prove()
dwasse Oct 25, 2024
95ea295
Merge branch 'master' into feat/relayer-arb-call
dwasse Nov 14, 2024
23d4a7a
feat(rfq-relayer): regenerate with updated fastbridgev2 and zap termi…
dwasse Nov 14, 2024
83cef1d
feat(rfq-api): don't respond to passive quotes for requests with zap …
dwasse Nov 14, 2024
14ed93d
Merge branch 'master' into feat/relayer-arb-call
dwasse Nov 18, 2024
9dd3998
feat(rfq-api): add v2 contracts to rfq api endpoint [SLT-429] (#3387)
dwasse Nov 20, 2024
8ece825
Merge branch 'master' into feat/relayer-arb-call
dwasse Nov 20, 2024
d3be2ff
Fix: tests
dwasse Nov 20, 2024
8edcb7b
Fix: tests
dwasse Nov 20, 2024
884d9ae
feat(rfq-api): check both relayer role and quoter role (#3399)
dwasse Nov 22, 2024
5a6cb3c
Merge branch 'master' into feat/relayer-arb-call
dwasse Nov 25, 2024
aac6885
[goreleaser]
dwasse Nov 25, 2024
8f9d8ec
Regen contracts w/ function renaming
dwasse Nov 26, 2024
2132369
[goreleaser]
dwasse Nov 26, 2024
57efbd0
feat(rfq-guard): v2 guard logic [SLT-387] (#3324)
dwasse Nov 26, 2024
fab0b9e
[goreleaser]
dwasse Dec 4, 2024
c1f57ef
feat(rfq-relayer): fee pricer considers v2 CallValue and CallParams […
dwasse Dec 6, 2024
e2dfc5a
[goreleaser]
parodime Dec 6, 2024
731a83c
pr 3299 comments. modify Zap checks
parodime Dec 7, 2024
a7bb3a3
[goreleaser]
parodime Dec 7, 2024
1add668
Merge branch 'master' into feat/relayer-arb-call
ChiTimesChi Dec 9, 2024
ac79f4d
[goreleaser]
parodime Dec 9, 2024
6dac46b
zap data nil check & test fixes
parodime Dec 9, 2024
9ff5bf3
[goreleaser]
parodime Dec 9, 2024
231bbea
[goreleaser] add debugging
parodime Dec 12, 2024
1a12f35
[goreleaser] span switch
parodime Dec 12, 2024
bf8b443
[goreleaser] switch to console prints
parodime Dec 12, 2024
f113504
[goreleaser] more debugggggg
parodime Dec 12, 2024
bff2348
[goreleaser] debug
parodime Dec 12, 2024
442be53
quote data logging
dwasse Dec 12, 2024
96055a7
[goreleaser]
dwasse Dec 12, 2024
b7c4f20
[goreleaser] more debug
parodime Dec 12, 2024
ba5afb9
Fix: use hexutil.Decode() for str -> byte
dwasse Dec 12, 2024
3ba6dda
[goreleaser]
dwasse Dec 12, 2024
e160c45
More logs
dwasse Dec 12, 2024
ac4c1a0
[goreleaser]
dwasse Dec 12, 2024
d80847e
Try panic on ws cancel
dwasse Dec 12, 2024
62e17de
[goreleaser]
dwasse Dec 12, 2024
15858f7
Add chan buffers, panics
dwasse Dec 12, 2024
cd6015f
[goreleaser]
dwasse Dec 12, 2024
a90924e
Cleanup: remove logs
dwasse Dec 12, 2024
67f7e14
fix: correct Tx.Value for `EstimateGas`
ChiTimesChi Dec 18, 2024
19b24c6
[goreleaser]
ChiTimesChi Dec 18, 2024
551083b
Fix: integration tests
dwasse Dec 23, 2024
d4b7d23
Merge branch 'master' into feat/relayer-arb-call
dwasse Dec 24, 2024
df72ef2
Merge branch 'master' into feat/relayer-arb-call
dwasse Dec 30, 2024
0aaf37e
Cleanup: lint
dwasse Dec 30, 2024
15c2598
Cleanup: lint
dwasse Jan 2, 2025
7913f76
[goreleaser]
parodime Jan 13, 2025
ae150b3
Fix: panic is now a log
dwasse Jan 13, 2025
46de8c5
[goreleaser]
dwasse Jan 13, 2025
a718e52
[goreleaser] debug tmp logging
parodime Jan 13, 2025
1b86ea5
Fix: suppress error in generateActiveRFQ call
dwasse Jan 13, 2025
d99fbcf
[goreleaser]
dwasse Jan 13, 2025
f43aca7
Merge branch 'feat/relayer-arb-call' of github.com:synapsecns/sanguin…
dwasse Jan 13, 2025
60f0e4c
[goreleaser]
dwasse Jan 13, 2025
f584479
+dynamic_gas_unit_add_percentage flag (#3494)
parodime Jan 31, 2025
cb110a0
[goreleaser]
parodime Jan 31, 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
7 changes: 7 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,10 @@ main

# golang-ci-lint binary
contrib/golang-ci-lint/golang-ci-lint

*signer*.txt

# ignore any rfq config files that are not the template
services/rfq/**/config*.yaml
services/rfq/**/config*.yml
!services/rfq/**/config.yml
3 changes: 2 additions & 1 deletion contrib/opbot/botmd/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,8 @@
return nil, fmt.Errorf("error getting chain client for chain ID %d: %w", chainID, err)
}

contractAddress, ok := contracts.Contracts[chainID]
// TODO: handle v2 contract if specified
contractAddress, ok := contracts.ContractsV1[chainID]

Check warning on line 382 in contrib/opbot/botmd/commands.go

View check run for this annotation

Codecov / codecov/patch

contrib/opbot/botmd/commands.go#L382

Added line #L382 was not covered by tests
Comment on lines +381 to +382
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

TODO comment needs to be addressed before merging

The TODO comment indicates that v2 contract handling is missing, which is a critical component given that this PR is titled "support FastBridgeV2". This functionality should be implemented before merging.

Would you like me to help implement the v2 contract handling logic?

if !ok {
return nil, fmt.Errorf("no contract address for chain ID")
}
Expand Down
3 changes: 3 additions & 0 deletions docs/bridge/docs/06-Services/04-Submitter.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ submitter_config:
dynamic_gas_estimate: true
# SupportsEIP1559 is whether or not this chain supports EIP1559.
supports_eip_1559: true
# DynamicGasUnitAddPercentage - increase gas unit limit (ie: "gas" field on a typical tx) by X% from what dynamic gas estimate returns
# Has no effect if dynamic gas estimation is not also enabled.
dynamic_gas_unit_add_percentage: 5
43114:
max_gas_price: 100000000000 # 100 Gwei
10:
Expand Down
13 changes: 10 additions & 3 deletions ethergo/backends/anvil/anvil.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
"github.com/synapsecns/sanguine/core/dockerutil"
"github.com/synapsecns/sanguine/core/mapmutex"
"github.com/synapsecns/sanguine/core/processlog"
"github.com/synapsecns/sanguine/core/retry"
"github.com/synapsecns/sanguine/ethergo/backends"
"github.com/synapsecns/sanguine/ethergo/backends/base"
"github.com/synapsecns/sanguine/ethergo/chain"
Expand Down Expand Up @@ -386,9 +387,15 @@
var acct *keystore.Key
// TODO handle storing accounts to conform to get tx context
if address != nil {
acct = f.GetAccount(*address)
if acct == nil {
f.T().Errorf("could not get account %s", address.String())
err := retry.WithBackoff(ctx, func(_ context.Context) error {
acct = f.GetAccount(*address)
if acct == nil {
return fmt.Errorf("could not get account %s", address.String())
}
return nil

Check warning on line 395 in ethergo/backends/anvil/anvil.go

View check run for this annotation

Codecov / codecov/patch

ethergo/backends/anvil/anvil.go#L390-L395

Added lines #L390 - L395 were not covered by tests
}, retry.WithMaxTotalTime(10*time.Second))
if err != nil {
f.T().Errorf("could not get account %s: %v", address.String(), err)

Check warning on line 398 in ethergo/backends/anvil/anvil.go

View check run for this annotation

Codecov / codecov/patch

ethergo/backends/anvil/anvil.go#L397-L398

Added lines #L397 - L398 were not covered by tests
return res
}
} else {
Expand Down
28 changes: 26 additions & 2 deletions ethergo/submitter/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@
DynamicGasEstimate bool `yaml:"dynamic_gas_estimate"`
// SupportsEIP1559 is whether or not this chain supports EIP1559
SupportsEIP1559 bool `yaml:"supports_eip_1559"`
// DynamicGasUnitAddPercentage - increase gas unit limit (ie: "gas" field on a typical tx) by X% from what dynamic gas estimate returns
// Has no effect if dynamic gas estimation is not also enabled.
DynamicGasUnitAddPercentage int `yaml:"dynamic_gas_unit_add_percentage"`
}

const (
Expand All @@ -64,6 +67,9 @@

// DefaultGasEstimate is the default gas estimate to use for transactions.
DefaultGasEstimate = uint64(1200000)

// DefaultDynamicGasUnitAddPercentage is the default percentage to bump the gas limit by.
DefaultDynamicGasUnitAddPercentage = 5
)

// DefaultMaxPrice is the default max price of a tx.
Expand Down Expand Up @@ -188,19 +194,37 @@
return gasBumpPercentage
}

// GetDynamicGasUnitAddPercentage returns the percentage to bump the gas limit by
func (c *Config) GetDynamicGasUnitAddPercentage(chainID int) (dynamicGasUnitAddPercentage int) {
chainConfig, ok := c.Chains[chainID]
if ok {
dynamicGasUnitAddPercentage = chainConfig.DynamicGasUnitAddPercentage
}

Check warning on line 202 in ethergo/submitter/config/config.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/config/config.go#L198-L202

Added lines #L198 - L202 were not covered by tests
// if dynamicGasUnitAddPercentage is not set for the chain, use the global config
if dynamicGasUnitAddPercentage == 0 {
dynamicGasUnitAddPercentage = c.DynamicGasUnitAddPercentage
}

Check warning on line 206 in ethergo/submitter/config/config.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/config/config.go#L204-L206

Added lines #L204 - L206 were not covered by tests

// if the dynamicGasUnitAddPercentage isn't set at all, use the default
if dynamicGasUnitAddPercentage == 0 {
dynamicGasUnitAddPercentage = DefaultDynamicGasUnitAddPercentage
}
return dynamicGasUnitAddPercentage

Check warning on line 212 in ethergo/submitter/config/config.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/config/config.go#L209-L212

Added lines #L209 - L212 were not covered by tests
}

// GetGasEstimate returns the gas estimate to use for transactions
// TODO: test this method.
func (c *Config) GetGasEstimate(chainID int) (gasEstimate uint64) {
chainConfig, ok := c.Chains[chainID]
if ok {
gasEstimate = chainConfig.GasEstimate
}
// if gasBumpPercentage is not set for the chain, use the global config
// if gasEstimate is not set for the chain, use the global config
if gasEstimate == 0 {
gasEstimate = c.GasEstimate
}

// if the gasBumpPercentage isn't set at all, use the default
// if the gasEstimate isn't set at all, use the default
if gasEstimate == 0 {
gasEstimate = DefaultGasEstimate
}
Expand Down
2 changes: 2 additions & 0 deletions ethergo/submitter/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
assert.Equal(t, uint64(1000), cfg.GasEstimate)
assert.Equal(t, true, cfg.DynamicGasEstimate)
assert.Equal(t, true, cfg.SupportsEIP1559(0))
Expand Down
2 changes: 2 additions & 0 deletions ethergo/submitter/config/iconfig_generated.go

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

77 changes: 60 additions & 17 deletions ethergo/submitter/submitter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {

Check failure on line 396 in ethergo/submitter/submitter.go

View workflow job for this annotation

GitHub Actions / Lint (ethergo)

unnecessary leading newline (whitespace)

newNonce, err := t.getNonce(ctx, chainID, address)
if err != nil {
return nil, fmt.Errorf("could not sign tx: %w", err)
}

Check warning on line 401 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L400-L401

Added lines #L400 - L401 were not covered by tests

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)
}

Check warning on line 408 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L407-L408

Added lines #L407 - L408 were not covered by tests

//nolint: wrapcheck
return parentTransactor.Signer(address, transaction)
}

transactor.Signer = func(address common.Address, transaction *types.Transaction) (_ *types.Transaction, err error) {
Expand All @@ -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())) {

Check failure on line 429 in ethergo/submitter/submitter.go

View workflow job for this annotation

GitHub Actions / Lint (ethergo)

G115: integer overflow conversion uint64 -> int (gosec)
transactor.GasLimit = t.config.GetGasEstimate(int(chainID.Uint64()))

Check failure on line 430 in ethergo/submitter/submitter.go

View workflow job for this annotation

GitHub Actions / Lint (ethergo)

G115: integer overflow conversion uint64 -> int (gosec)
} else {

Check failure on line 431 in ethergo/submitter/submitter.go

View workflow job for this annotation

GitHub Actions / Lint (ethergo)

unnecessary leading newline (whitespace)

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)
}

Check warning on line 440 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L433-L440

Added lines #L433 - L440 were not covered by tests

txForGasEstimate, err := call(transactorForGasEstimate)

Check warning on line 442 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L442

Added line #L442 was not covered by tests
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)

Check warning on line 446 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L444-L446

Added lines #L444 - L446 were not covered by tests
}

//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()))

Check failure on line 451 in ethergo/submitter/submitter.go

View workflow job for this annotation

GitHub Actions / Lint (ethergo)

G115: integer overflow conversion uint64 -> int (gosec)
transactor.GasLimit = txForGasEstimate.Gas() + (txForGasEstimate.Gas() * uint64(gasLimitAddPercentage) / 100)

Check warning on line 452 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L451-L452

Added lines #L451 - L452 were not covered by tests

Check failure on line 452 in ethergo/submitter/submitter.go

View workflow job for this annotation

GitHub Actions / Lint (ethergo)

G115: integer overflow conversion int -> uint64 (gosec)
Comment on lines +427 to +452
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure safe gas limit calculations.

The gas limit calculation with percentage adjustments could overflow for large gas values.

Add overflow checks and use safe math:

+func safeGasLimitAdjustment(gasLimit uint64, percentage int) (uint64, error) {
+    if percentage < 0 {
+        return 0, fmt.Errorf("negative percentage not allowed")
+    }
+    adjustment := new(big.Int).Mul(
+        new(big.Int).SetUint64(gasLimit),
+        new(big.Int).SetInt64(int64(percentage)),
+    )
+    adjustment.Div(adjustment, big.NewInt(100))
+    if !adjustment.IsUint64() {
+        return 0, fmt.Errorf("gas limit adjustment overflow")
+    }
+    total := new(big.Int).Add(
+        new(big.Int).SetUint64(gasLimit),
+        adjustment,
+    )
+    if !total.IsUint64() {
+        return 0, fmt.Errorf("final gas limit overflow")
+    }
+    return total.Uint64(), nil
+}

-transactor.GasLimit = txForGasEstimate.Gas() + (txForGasEstimate.Gas() * uint64(gasLimitAddPercentage) / 100)
+transactor.GasLimit, err = safeGasLimitAdjustment(txForGasEstimate.Gas(), gasLimitAddPercentage)
+if err != nil {
+    return 0, fmt.Errorf("could not calculate gas limit: %w", err)
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// 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())) {
transactor.GasLimit = t.config.GetGasEstimate(int(chainID.Uint64()))
} else {
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)
// 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())) {
transactor.GasLimit = t.config.GetGasEstimate(int(chainID.Uint64()))
} else {
// 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 {
errMsg := util.FormatError(err)
return 0, fmt.Errorf("err contract call for gas est: %s", errMsg)
}
// 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()))
func safeGasLimitAdjustment(gasLimit uint64, percentage int) (uint64, error) {
if percentage < 0 {
return 0, fmt.Errorf("negative percentage not allowed")
}
adjustment := new(big.Int).Mul(
new(big.Int).SetUint64(gasLimit),
new(big.Int).SetInt64(int64(percentage)),
)
adjustment.Div(adjustment, big.NewInt(100))
if !adjustment.IsUint64() {
return 0, fmt.Errorf("gas limit adjustment overflow")
}
total := new(big.Int).Add(
new(big.Int).SetUint64(gasLimit),
adjustment,
)
if !total.IsUint64() {
return 0, fmt.Errorf("final gas limit overflow")
}
return total.Uint64(), nil
}
transactor.GasLimit, err = safeGasLimitAdjustment(txForGasEstimate.Gas(), gasLimitAddPercentage)
if err != nil {
return 0, fmt.Errorf("could not calculate gas limit: %w", err)
}
🧰 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: Lint (ethergo)

[failure] 429-429:
G115: integer overflow conversion uint64 -> int (gosec)


[failure] 451-451:
G115: integer overflow conversion uint64 -> int (gosec)


[failure] 452-452:
G115: integer overflow conversion int -> uint64 (gosec)


[failure] 430-430:
G115: integer overflow conversion uint64 -> int (gosec)


[failure] 431-431:
unnecessary leading newline (whitespace)

}

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)

Check warning on line 457 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L457

Added line #L457 was not covered by tests
}

defer locker.Unlock()

// now that we've stored the tx
Expand Down Expand Up @@ -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) {

Check failure on line 711 in ethergo/submitter/submitter.go

View workflow job for this annotation

GitHub Actions / Lint (ethergo)

unnecessary leading newline (whitespace)

// if dynamic gas estimation is not enabled, use cfg var gas_estimate as a default

Check warning on line 713 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L711-L713

Added lines #L711 - L713 were not covered by tests
if !t.config.GetDynamicGasEstimate(chainID) {
return t.config.GetGasEstimate(chainID), nil
}

gasUnitAddPercentage := t.config.GetDynamicGasUnitAddPercentage(chainID)

Check warning on line 719 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L718-L719

Added lines #L718 - L719 were not covered by tests
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),

Check warning on line 723 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L723

Added line #L723 was not covered by tests
))

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 warning on line 727 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L727

Added line #L727 was not covered by tests

Check failure on line 727 in ethergo/submitter/submitter.go

View workflow job for this annotation

GitHub Actions / Lint (ethergo)

G115: integer overflow conversion uint64 -> int64 (gosec)
metrics.EndSpanWithErr(span, err)
}()

Expand All @@ -697,14 +734,20 @@
return 0, fmt.Errorf("could not convert tx to call: %w", err)
}

gasEstimate, err = chainClient.EstimateGas(ctx, *call)
gasLimitFromEstimate, err := chainClient.EstimateGas(ctx, *call)

Check warning on line 738 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L737-L738

Added lines #L737 - L738 were not covered by tests
if err != nil {
span.AddEvent("could not estimate gas", trace.WithAttributes(attribute.String("error", err.Error())))
// fallback to default

// if we failed to est gas for any reason, use the default flat gas from config

Check warning on line 742 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L741-L742

Added lines #L741 - L742 were not covered by tests
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)

Check failure on line 747 in ethergo/submitter/submitter.go

View workflow job for this annotation

GitHub Actions / Lint (ethergo)

G115: integer overflow conversion int -> uint64 (gosec)
gasLimit = gasLimitFromEstimate

return gasLimit, nil

Check warning on line 750 in ethergo/submitter/submitter.go

View check run for this annotation

Codecov / codecov/patch

ethergo/submitter/submitter.go#L747-L750

Added lines #L747 - L750 were not covered by tests
}

func (t *txSubmitterImpl) Address() common.Address {
Expand Down
17 changes: 17 additions & 0 deletions ethergo/util/util.go
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.

Check failure on line 12 in ethergo/util/util.go

View workflow job for this annotation

GitHub Actions / Lint (ethergo)

commentFormatting: put a space between `//` and comment text (gocritic)
if strings.Contains(errMsg, "<!DOCTYPE html>") {
errMsg = strings.Split(errMsg, "<!DOCTYPE html>")[0] + "<html portion of error removed>"
}
return errMsg

Check warning on line 16 in ethergo/util/util.go

View check run for this annotation

Codecov / codecov/patch

ethergo/util/util.go#L6-L16

Added lines #L6 - L16 were not covered by tests
}
7 changes: 4 additions & 3 deletions services/omnirpc/proxy/forward.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
"context"
"crypto/sha256"
"fmt"
goHTTP "net/http"
"strings"

"github.com/ImVexed/fasturl"
"github.com/goccy/go-json"
"github.com/jftuga/ellipsis"
Expand All @@ -14,8 +17,6 @@
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"
"golang.org/x/exp/slices"
goHTTP "net/http"
"strings"
)

type rawResponse struct {
Expand Down Expand Up @@ -56,7 +57,7 @@

standardizedResponse, err = standardizeResponse(ctx, &f.rpcRequest[0], rpcMessage)
if err != nil {
return nil, fmt.Errorf("could not standardize response: %w", err)
return nil, fmt.Errorf("could not standardize response from body %s: %w", ellipsis.Shorten(string(body), 200), err)

Check warning on line 60 in services/omnirpc/proxy/forward.go

View check run for this annotation

Codecov / codecov/patch

services/omnirpc/proxy/forward.go#L60

Added line #L60 was not covered by tests
}
}

Expand Down
12 changes: 8 additions & 4 deletions services/rfq/api/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ import (

var logger = log.Logger("rfq-client")

const pingPeriod = 20 * time.Second
const (
pingPeriod = 20 * time.Second
chanBuffer = 1000
)

// AuthenticatedClient is an interface for the RFQ API.
// It provides methods for creating, retrieving and updating quotes.
Expand Down Expand Up @@ -208,7 +211,7 @@ func (c *clientImpl) SubscribeActiveQuotes(ctx context.Context, req *model.Subsc
return nil, fmt.Errorf("subscription failed")
}

respChan = make(chan *model.ActiveRFQMessage)
respChan = make(chan *model.ActiveRFQMessage, chanBuffer)
go func() {
wsErr := c.processWebsocket(ctx, conn, reqChan, respChan)
if wsErr != nil {
Expand Down Expand Up @@ -264,16 +267,17 @@ func (c *clientImpl) processWebsocket(ctx context.Context, conn *websocket.Conn,
if err != nil {
logger.Warnf("error closing websocket connection: %v", err)
}
logger.Warnf("processWebsocket exited")
}()

readChan := make(chan []byte)
readChan := make(chan []byte, chanBuffer)
go c.listenWsMessages(ctx, conn, readChan)
go c.sendPings(ctx, reqChan)

for {
select {
case <-ctx.Done():
return nil
return ctx.Err()
case msg, ok := <-reqChan:
if !ok {
return fmt.Errorf("error reading from reqChan: %w", ctx.Err())
Expand Down
6 changes: 5 additions & 1 deletion services/rfq/api/client/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,11 @@ func (c *ClientSuite) SetupTest() {
DSN: filet.TmpFile(c.T(), "", "").Name(),
},
OmniRPCURL: testOmnirpc,
Bridges: map[uint32]string{
FastBridgeContractsV1: map[uint32]string{
1: ethFastBridgeAddress.Hex(),
42161: arbFastBridgeAddress.Hex(),
},
FastBridgeContractsV2: map[uint32]string{
1: ethFastBridgeAddress.Hex(),
42161: arbFastBridgeAddress.Hex(),
},
Expand Down
14 changes: 7 additions & 7 deletions services/rfq/api/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ type DatabaseConfig struct {

// Config is the configuration for the RFQ Quoter.
type Config struct {
Database DatabaseConfig `yaml:"database"`
OmniRPCURL string `yaml:"omnirpc_url"`
// bridges is a map of chainid->address
Bridges map[uint32]string `yaml:"bridges"`
Port string `yaml:"port"`
RelayAckTimeout time.Duration `yaml:"relay_ack_timeout"`
MaxQuoteAge time.Duration `yaml:"max_quote_age"`
Database DatabaseConfig `yaml:"database"`
OmniRPCURL string `yaml:"omnirpc_url"`
FastBridgeContractsV1 map[uint32]string `yaml:"fast_bridge_contracts_v1"`
FastBridgeContractsV2 map[uint32]string `yaml:"fast_bridge_contracts_v2"`
Port string `yaml:"port"`
RelayAckTimeout time.Duration `yaml:"relay_ack_timeout"`
MaxQuoteAge time.Duration `yaml:"max_quote_age"`
}

const defaultRelayAckTimeout = 30 * time.Second
Expand Down
Loading
Loading