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 93 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 77 commits
Commits
Show all changes
93 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
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
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
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
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
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
12 changes: 10 additions & 2 deletions services/rfq/api/model/request.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package model

import "time"
import (
"fmt"
"time"
)

// PutRelayerQuoteRequest contains the schema for a PUT /quote request.
type PutRelayerQuoteRequest struct {
Expand Down Expand Up @@ -55,8 +58,12 @@ type QuoteData struct {
DestChainID int `json:"dest_chain_id"`
OriginTokenAddr string `json:"origin_token_addr"`
DestTokenAddr string `json:"dest_token_addr"`
OriginAmountExact string `json:"origin_amount_exact"`
ExpirationWindow int64 `json:"expiration_window"`
ZapData string `json:"zap_data"`
ZapNative string `json:"zap_native"`
OriginAmountExact string `json:"origin_amount_exact"`
OriginSender string `json:"origin_sender"`
DestRecipient string `json:"dest_recipient"`
Comment on lines +61 to +65
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Input validation needed for new FastBridgeV2 fields

Based on the codebase analysis, there is only basic validation for ZapNative (checking if it's a valid big.Int and non-zero), but no validation for:

  • ZapData format and content
  • OriginSender and DestRecipient address format
  • OriginAmountExact numeric format

Suggested validation points:

  • Ensure ZapData is valid hex-encoded calldata
  • Verify OriginSender and DestRecipient are valid Ethereum addresses
  • Validate OriginAmountExact is a valid numeric string
🔗 Analysis chain

Verify input validation for new fields

The new fields support FastBridgeV2's arbitrary call functionality, but there's no apparent validation for these fields. Consider adding validation to ensure:

  • ZapData contains valid calldata
  • ZapNative contains a valid amount
  • OriginSender and DestRecipient are valid addresses
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if validation exists for the new fields
rg -A 5 "func.*validate.*Quote" 
ast-grep --pattern 'if $COND {
  $$$
  $ERR = $$$
  $$$
}'

Length of output: 66715


Script:

#!/bin/bash
# Search for validation functions in the codebase
rg -A 10 "func.*validate.*Quote|func.*validate.*Request" services/rfq/api/

# Search for validation patterns in request.go
rg -A 5 "if.*invalid|if.*err.*=.*validate" services/rfq/api/model/request.go

# Look for validation in handler code
rg -A 10 "func.*Handle.*Quote.*Request" services/rfq/api/rest/

Length of output: 2919


Script:

#!/bin/bash
# Search for validation in the request handling flow
rg -A 10 "func.*PutQuote|func.*PostQuote" services/rfq/api/rest/

# Look for validation in the model package
rg -A 10 "func.*Validate|func.*Check" services/rfq/api/model/

# Search for any validation related to zap or bridge data
rg -A 5 "zap|bridge.*valid" services/rfq/api/

Length of output: 2876

DestAmount *string `json:"dest_amount"`
RelayerAddress *string `json:"relayer_address"`
QuoteID *string `json:"quote_id"`
Expand All @@ -78,6 +85,7 @@ type SubscribeActiveRFQRequest struct {

// NewWsRFQRequest creates a new WsRFQRequest.
func NewWsRFQRequest(data QuoteData, requestID string) *WsRFQRequest {
fmt.Printf("NewWsRFQRequest with data: %+v\n", data)
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

Replace fmt.Printf with structured logging

Using fmt.Printf for debugging can expose sensitive data and doesn't provide proper log levels or structured output.

-    fmt.Printf("NewWsRFQRequest with data: %+v\n", data)
+    logger.Debug("Creating new WsRFQRequest",
+        "origin_chain_id", data.OriginChainID,
+        "dest_chain_id", data.DestChainID,
+        "origin_token_addr", data.OriginTokenAddr,
+        "dest_token_addr", data.DestTokenAddr)

Committable suggestion skipped: line range outside the PR's diff.

return &WsRFQRequest{
RequestID: requestID,
Data: data,
Expand Down
8 changes: 5 additions & 3 deletions services/rfq/api/model/response.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,12 @@ type PutRelayAckResponse struct {
RelayerAddress string `json:"relayer_address"`
}

// GetContractsResponse contains the schema for a GET /contract response.
// GetContractsResponse contains the schema for a GET /contracts response.
type GetContractsResponse struct {
// Contracts is a map of chain id to contract address
Contracts map[uint32]string `json:"contracts"`
// ContractsV1 is a map of chain id to contract address for v1 fast bridge contracts
ContractsV1 map[uint32]string `json:"contracts_v1"`
// ContractsV2 is a map of chain id to contract address for v2 fast bridge contracts
ContractsV2 map[uint32]string `json:"contracts_v2"`
Comment on lines +44 to +49
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Breaking change confirmed: Multiple impacted components require updates

The breaking change impacts several components that need to be updated:

  • contrib/restclient/client.gen.go: Currently uses an outdated model with a single Contracts map
  • services/rfq/api/client/client.go: Client implementation needs to handle the new V1/V2 contract structure
  • services/rfq/api/rest/handler.go: Handler has been updated correctly with the new structure
  • API documentation files (swagger.json, swagger.yaml) need to be regenerated
🔗 Analysis chain

Breaking Change: GetContractsResponse structure has been modified

The separation of contracts into V1 and V2 maps is a breaking change that will require updates from API consumers. Ensure this change is properly documented in the API changelog and communicated to consumers.

Let's verify if there are any direct API consumers that need to be updated:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for direct usage of the GetContractsResponse struct
rg -l "GetContractsResponse" | grep -v "response.go"

Length of output: 281


Script:

#!/bin/bash
# Let's check the usage of GetContractsResponse in these files to understand the impact
echo "=== Checking client.go ==="
rg -A 5 "GetContractsResponse" services/rfq/api/client/client.go

echo -e "\n=== Checking handler.go ==="
rg -A 5 "GetContractsResponse" services/rfq/api/rest/handler.go

echo -e "\n=== Checking client.gen.go ==="
rg -A 5 "GetContractsResponse" contrib/restclient/client.gen.go

Length of output: 3607

}

// ActiveRFQMessage represents the general structure of WebSocket messages for Active RFQ.
Expand Down
41 changes: 32 additions & 9 deletions services/rfq/api/rest/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strconv"
"time"

"github.com/ethereum/go-ethereum/common"
"github.com/synapsecns/sanguine/services/rfq/api/config"

"github.com/gin-gonic/gin"
Expand Down Expand Up @@ -69,7 +70,7 @@ func (h *Handler) ModifyQuote(c *gin.Context) {
return
}

dbQuote, err := parseDBQuote(*putRequest, relayerAddr)
dbQuote, err := parseDBQuote(h.cfg, *putRequest, relayerAddr)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
return
Expand Down Expand Up @@ -117,7 +118,7 @@ func (h *Handler) ModifyBulkQuotes(c *gin.Context) {

dbQuotes := []*db.Quote{}
for _, quoteReq := range putRequest.Quotes {
dbQuote, err := parseDBQuote(quoteReq, relayerAddr)
dbQuote, err := parseDBQuote(h.cfg, quoteReq, relayerAddr)
if err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid quote request"})
return
Expand All @@ -134,7 +135,7 @@ func (h *Handler) ModifyBulkQuotes(c *gin.Context) {
}

//nolint:gosec
func parseDBQuote(putRequest model.PutRelayerQuoteRequest, relayerAddr interface{}) (*db.Quote, error) {
func parseDBQuote(cfg config.Config, putRequest model.PutRelayerQuoteRequest, relayerAddr interface{}) (*db.Quote, error) {
destAmount, err := decimal.NewFromString(putRequest.DestAmount)
if err != nil {
return nil, fmt.Errorf("invalid DestAmount")
Expand All @@ -147,6 +148,12 @@ func parseDBQuote(putRequest model.PutRelayerQuoteRequest, relayerAddr interface
if err != nil {
return nil, fmt.Errorf("invalid FixedFee")
}

err = validateFastBridgeAddresses(cfg, putRequest)
if err != nil {
return nil, fmt.Errorf("invalid fast bridge addresses: %w", err)
}

// nolint: forcetypeassert
return &db.Quote{
OriginChainID: uint64(putRequest.OriginChainID),
Expand All @@ -163,6 +170,24 @@ func parseDBQuote(putRequest model.PutRelayerQuoteRequest, relayerAddr interface
}, nil
}

//nolint:gosec
func validateFastBridgeAddresses(cfg config.Config, putRequest model.PutRelayerQuoteRequest) error {
// Check V1 contracts
isV1Origin := common.HexToAddress(cfg.FastBridgeContractsV1[uint32(putRequest.OriginChainID)]) == common.HexToAddress(putRequest.OriginFastBridgeAddress)
isV1Dest := common.HexToAddress(cfg.FastBridgeContractsV1[uint32(putRequest.DestChainID)]) == common.HexToAddress(putRequest.DestFastBridgeAddress)

// Check V2 contracts
isV2Origin := common.HexToAddress(cfg.FastBridgeContractsV2[uint32(putRequest.OriginChainID)]) == common.HexToAddress(putRequest.OriginFastBridgeAddress)
isV2Dest := common.HexToAddress(cfg.FastBridgeContractsV2[uint32(putRequest.DestChainID)]) == common.HexToAddress(putRequest.DestFastBridgeAddress)

// Valid if both addresses match either V1 or V2
if (isV1Origin && isV1Dest) || (isV2Origin && isV2Dest) {
return nil
}

return fmt.Errorf("origin and destination fast bridge addresses must match either V1 or V2")
}
Comment on lines +173 to +189
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

Add zero address validation and improve error messages

While the contract version validation is good, consider these improvements:

  1. Add zero address validation
  2. Make error messages more specific by indicating which addresses don't match

Here's the suggested implementation:

 func validateFastBridgeAddresses(cfg config.Config, putRequest model.PutRelayerQuoteRequest) error {
+    // Validate non-zero addresses
+    if common.HexToAddress(putRequest.OriginFastBridgeAddress) == common.Address{} ||
+       common.HexToAddress(putRequest.DestFastBridgeAddress) == common.Address{} {
+        return fmt.Errorf("fast bridge addresses cannot be zero")
+    }
+
     // Check V1 contracts
     isV1Origin := common.HexToAddress(cfg.FastBridgeContractsV1[uint32(putRequest.OriginChainID)]) == common.HexToAddress(putRequest.OriginFastBridgeAddress)
     isV1Dest := common.HexToAddress(cfg.FastBridgeContractsV1[uint32(putRequest.DestChainID)]) == common.HexToAddress(putRequest.DestFastBridgeAddress)

     // Check V2 contracts
     isV2Origin := common.HexToAddress(cfg.FastBridgeContractsV2[uint32(putRequest.OriginChainID)]) == common.HexToAddress(putRequest.OriginFastBridgeAddress)
     isV2Dest := common.HexToAddress(cfg.FastBridgeContractsV2[uint32(putRequest.DestChainID)]) == common.HexToAddress(putRequest.DestFastBridgeAddress)

     // Valid if both addresses match either V1 or V2
     if (isV1Origin && isV1Dest) || (isV2Origin && isV2Dest) {
         return nil
     }

-    return fmt.Errorf("origin and destination fast bridge addresses must match either V1 or V2")
+    return fmt.Errorf("mismatched fast bridge versions: origin=%s, destination=%s", 
+        putRequest.OriginFastBridgeAddress, putRequest.DestFastBridgeAddress)
 }
📝 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
//nolint:gosec
func validateFastBridgeAddresses(cfg config.Config, putRequest model.PutRelayerQuoteRequest) error {
// Check V1 contracts
isV1Origin := common.HexToAddress(cfg.FastBridgeContractsV1[uint32(putRequest.OriginChainID)]) == common.HexToAddress(putRequest.OriginFastBridgeAddress)
isV1Dest := common.HexToAddress(cfg.FastBridgeContractsV1[uint32(putRequest.DestChainID)]) == common.HexToAddress(putRequest.DestFastBridgeAddress)
// Check V2 contracts
isV2Origin := common.HexToAddress(cfg.FastBridgeContractsV2[uint32(putRequest.OriginChainID)]) == common.HexToAddress(putRequest.OriginFastBridgeAddress)
isV2Dest := common.HexToAddress(cfg.FastBridgeContractsV2[uint32(putRequest.DestChainID)]) == common.HexToAddress(putRequest.DestFastBridgeAddress)
// Valid if both addresses match either V1 or V2
if (isV1Origin && isV1Dest) || (isV2Origin && isV2Dest) {
return nil
}
return fmt.Errorf("origin and destination fast bridge addresses must match either V1 or V2")
}
//nolint:gosec
func validateFastBridgeAddresses(cfg config.Config, putRequest model.PutRelayerQuoteRequest) error {
// Validate non-zero addresses
if common.HexToAddress(putRequest.OriginFastBridgeAddress) == common.Address{} ||
common.HexToAddress(putRequest.DestFastBridgeAddress) == common.Address{} {
return fmt.Errorf("fast bridge addresses cannot be zero")
}
// Check V1 contracts
isV1Origin := common.HexToAddress(cfg.FastBridgeContractsV1[uint32(putRequest.OriginChainID)]) == common.HexToAddress(putRequest.OriginFastBridgeAddress)
isV1Dest := common.HexToAddress(cfg.FastBridgeContractsV1[uint32(putRequest.DestChainID)]) == common.HexToAddress(putRequest.DestFastBridgeAddress)
// Check V2 contracts
isV2Origin := common.HexToAddress(cfg.FastBridgeContractsV2[uint32(putRequest.OriginChainID)]) == common.HexToAddress(putRequest.OriginFastBridgeAddress)
isV2Dest := common.HexToAddress(cfg.FastBridgeContractsV2[uint32(putRequest.DestChainID)]) == common.HexToAddress(putRequest.DestFastBridgeAddress)
// Valid if both addresses match either V1 or V2
if (isV1Origin && isV1Dest) || (isV2Origin && isV2Dest) {
return nil
}
return fmt.Errorf("mismatched fast bridge versions: origin=%s, destination=%s",
putRequest.OriginFastBridgeAddress, putRequest.DestFastBridgeAddress)
}


//nolint:gosec
func quoteResponseFromDBQuote(dbQuote *db.Quote) *model.GetQuoteResponse {
return &model.GetQuoteResponse{
Expand Down Expand Up @@ -301,12 +326,10 @@ func dbActiveQuoteRequestToModel(dbQuote *db.ActiveQuoteRequest) *model.GetOpenQ
// @Header 200 {string} X-Api-Version "API Version Number - See docs for more info"
// @Router /contracts [get].
func (h *Handler) GetContracts(c *gin.Context) {
// Convert quotes from db model to api model
contracts := make(map[uint32]string)
for chainID, address := range h.cfg.Bridges {
contracts[chainID] = address
}
c.JSON(http.StatusOK, model.GetContractsResponse{Contracts: contracts})
c.JSON(http.StatusOK, model.GetContractsResponse{
ContractsV1: h.cfg.FastBridgeContractsV1,
ContractsV2: h.cfg.FastBridgeContractsV2,
})
}

func filterQuoteAge(cfg config.Config, dbQuotes []*db.Quote) []*db.Quote {
Expand Down
2 changes: 2 additions & 0 deletions services/rfq/api/rest/rfq.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
const collectionTimeout = 1 * time.Minute

func (r *QuoterAPIServer) handleActiveRFQ(ctx context.Context, request *model.PutRFQRequest, requestID string) (quote *model.QuoteData) {
fmt.Printf("handleActiveRFQ with request data: %+v\n", request.Data)
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

Use structured logging consistently

Debug logging should use the application's logger with appropriate log levels and structured fields.

-    fmt.Printf("handleActiveRFQ with request data: %+v\n", request.Data)
+    logger.Debug("Handling active RFQ request",
+        "request_id", requestID,
+        "origin_chain_id", request.Data.OriginChainID,
+        "dest_chain_id", request.Data.DestChainID)

     // ... existing code ...

-    fmt.Printf("parsed websocket request: %+v\n", relayerReq.Data)
+    logger.Debug("Parsed websocket request",
+        "request_id", relayerReq.RequestID,
+        "origin_chain_id", relayerReq.Data.OriginChainID,
+        "dest_chain_id", relayerReq.Data.DestChainID)

Also applies to: 34-34

ctx, span := r.handler.Tracer().Start(ctx, "handleActiveRFQ", trace.WithAttributes(
attribute.String("user_address", request.UserAddress),
attribute.String("request_id", requestID),
Expand All @@ -30,6 +31,7 @@ func (r *QuoterAPIServer) handleActiveRFQ(ctx context.Context, request *model.Pu

// publish the quote request to all connected clients
relayerReq := model.NewWsRFQRequest(request.Data, requestID)
fmt.Printf("parsed websocket request: %+v\n", relayerReq.Data)
r.wsClients.Range(func(relayerAddr string, client WsClient) bool {
sendCtx, sendSpan := r.handler.Tracer().Start(ctx, "sendQuoteRequest", trace.WithAttributes(
attribute.String("relayer_address", relayerAddr),
Expand Down
3 changes: 3 additions & 0 deletions services/rfq/api/rest/rfq_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,9 @@ func (c *ServerSuite) TestActiveRFQFallbackToPassive() {
c.Assert().Equal("998000", userQuoteResp.DestAmount) // destAmount is quote destAmount minus fixed fee
c.Assert().Equal(c.relayerWallets[0].Address().Hex(), userQuoteResp.RelayerAddress)

// Submit a user quote request with zap data
userQuoteReq.Data.ZapData = "abc"
userQuoteReq.Data.ZapNative = "100"
// Submit the user quote request with a large origin amount, expect no quotes will be found
userQuoteReq.Data.OriginAmountExact = big.NewInt(1e18).String()
userQuoteResp, err = userClient.PutRFQRequest(c.GetTestContext(), userQuoteReq)
Expand Down
Loading
Loading