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

Add native ETH support to relayer #1840

Merged
merged 67 commits into from Feb 1, 2024
Merged

Add native ETH support to relayer #1840

merged 67 commits into from Feb 1, 2024

Conversation

dwasse
Copy link
Collaborator

@dwasse dwasse commented Jan 12, 2024

Description
Enables native ETH support in the RFQ relayer by setting message.value equal to the destination amount.

  • Add MinGasToken param to relayer config
  • Quoter quotes ETH balance minus MinGasToken
  • ShouldProcess() validation function in quoter now checks that at least MinGasToken is leftover before processing ETH quotes

Note that integration tests TestUSDCToUSDC and TestETHtoETH work locally but are currently disabled in CI. #1955 was created to address this.

Metadata

Summary by CodeRabbit

  • New Features
    • Added support for ETH as a quotable token, including integration tests for ETH-to-ETH transactions.
    • Introduced a new REST API handler for managing quote requests and transaction retries in the RFQ relayer service.
    • Implemented a new blockchain interaction interface, enhancing the service's capability to interact with blockchain networks.
  • Refactor
    • Renamed and updated various components to better reflect their purpose, such as changing APIServer to QuoterAPIServer.
    • Enhanced the quote generation logic to support native tokens and improved error handling.
    • Updated the token initialization and balance refreshing logic to distinguish between native and non-native tokens.
  • Bug Fixes
    • Corrected the error message in GetQuoteRequestByID method to accurately reflect the error condition.
  • Tests
    • Expanded testing suites to cover new functionalities, including tests for native token quote generation.
  • Chores
    • Adjusted CI/CD pipeline settings to refine the build and test process, including modifications for retry behavior and the addition of a new setup job.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between ae965b4 and ee72578.
Files selected for processing (1)
  • services/rfq/relayer/chain/util.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • services/rfq/relayer/chain/util.go

* Cleanup: Commitable -> Committable

* Feat: add gasBalances tracking

* Feat: fallback to fetching from gasBalances map

* Feat: add hasSufficientGas check

* WIP: export HasSufficientGas, add gasMiddleware

* WIP: move HasSufficientGas into inventory manager

* Feat: inventory manager now uses ClientFetcher instead of omni client

* Feat: add mock inventory manager

* Fix: tests

* Feat: add sufficient gas clause to TestShouldProcess

* Cleanup: tests

* Feat: add clause testing 0 quote amount

* [goreleaser] Fix: handle ETH in getDecimals()

* Fix: don't approve ETH
@github-actions github-actions bot added the M-ci Module: CI label Jan 30, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between ee72578 and f687e5d.
Files selected for processing (1)
  • .github/workflows/go.yml (2 hunks)
Files skipped from review due to trivial changes (1)
  • .github/workflows/go.yml

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between f687e5d and a4a5c22.
Files selected for processing (1)
  • .github/workflows/go.yml (2 hunks)
Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/go.yml

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 8

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between a4a5c22 and 31d98fa63f5485c7687527b6f3d74a7915cf28f6.
Files selected for processing (3)
  • ethergo/backends/anvil/anvil.go (3 hunks)
  • ethergo/backends/base/base.go (2 hunks)
  • ethergo/deployer/registry.go (2 hunks)
Additional comments: 3
ethergo/deployer/registry.go (1)
  • 5-6: The addition of fmt and sync imports indicates new functionality related to printing and synchronization. Ensure that these imports are utilized effectively within the file.
ethergo/backends/base/base.go (1)
  • 13-13: The addition of the github.com/synapsecns/sanguine/ethergo/signer/wallet import suggests new functionality related to wallet management. Verify that this import is utilized appropriately within the file.
ethergo/backends/anvil/anvil.go (1)
  • 13-15: The addition of github.com/ipfs/go-log, github.com/lmittmann/w3/w3types, and github.com/ory/dockertest/v3 imports suggests enhancements related to logging, web3 types handling, and Docker test setup. Ensure these imports are utilized effectively within the file.

@@ -151,9 +153,11 @@ func (c *contractRegistryImpl) Deploy(ctx context.Context, contractType contract
deploymentHandle := c.deployers[contractType.ID()]
c.structMux.RUnlock()

fmt.Printf("Deploying on handle on %d\n", c.backend.GetChainID())
Copy link
Contributor

Choose a reason for hiding this comment

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

The fmt.Printf statement is used for logging deployment information. Consider using a structured logging framework instead of fmt.Printf for better log management and filtering capabilities.

- fmt.Printf("Deploying on handle on %d\n", c.backend.GetChainID())
+ logger.Infof("Deploying on handle on %d", c.backend.GetChainID())

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.

Suggested change
fmt.Printf("Deploying on handle on %d\n", c.backend.GetChainID())
logger.Infof("Deploying on handle on %d", c.backend.GetChainID())

deployedContract, err := deploymentHandle.Deploy(ctx)
require.Nil(c.tb, err)

fmt.Printf("waiting for confirmation of deploytx %v on %d\n", deployedContract.DeployTx().Hash().String(), c.backend.GetChainID())
Copy link
Contributor

Choose a reason for hiding this comment

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

The fmt.Printf statement is used for logging the wait for transaction confirmation. As with the previous comment, recommend using a structured logging approach for consistency and enhanced log management.

- fmt.Printf("waiting for confirmation of deploytx %v on %d\n", deployedContract.DeployTx().Hash().String(), c.backend.GetChainID())
+ logger.Infof("waiting for confirmation of deploytx %v on %d", deployedContract.DeployTx().Hash().String(), c.backend.GetChainID())

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.

Suggested change
fmt.Printf("waiting for confirmation of deploytx %v on %d\n", deployedContract.DeployTx().Hash().String(), c.backend.GetChainID())
logger.Infof("waiting for confirmation of deploytx %v on %d", deployedContract.DeployTx().Hash().String(), c.backend.GetChainID())

@@ -225,25 +226,31 @@ type ConfirmationClient interface {
// it blocks until the transaction is confirmed.
// nolint: cyclop
func WaitForConfirmation(ctx context.Context, client ConfirmationClient, transaction *types.Transaction, timeout time.Duration) {
fmt.Printf("WaitForConfirmation [base] on tx: %v\n", transaction.Hash())
Copy link
Contributor

Choose a reason for hiding this comment

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

The fmt.Printf statement is used for logging the start of the confirmation wait process. Recommend using a structured logging framework for consistency across the codebase.

- fmt.Printf("WaitForConfirmation [base] on tx: %v\n", transaction.Hash())
+ logger.Infof("WaitForConfirmation [base] on tx: %v", transaction.Hash())

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.

Suggested change
fmt.Printf("WaitForConfirmation [base] on tx: %v\n", transaction.Hash())
logger.Infof("WaitForConfirmation [base] on tx: %v", transaction.Hash())

// if tx is nil , we should panic here so we can see the call context
_ = transaction.Hash()

txConfirmedCtx, cancel := context.WithCancel(ctx)
var logOnce sync.Once
wait.UntilWithContext(txConfirmedCtx, func(ctx context.Context) {
fmt.Printf("WaitForConfirmation until with cotext [base] on tx: %v\n", transaction.Hash())
Copy link
Contributor

Choose a reason for hiding this comment

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

The fmt.Printf statement within the loop logs each check for transaction confirmation. This could potentially flood the logs. Consider logging this at a debug level or removing it if not necessary.

- fmt.Printf("WaitForConfirmation until with cotext [base] on tx: %v\n", transaction.Hash())
+ logger.Debugf("WaitForConfirmation until with context [base] on tx: %v", transaction.Hash())

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.

Suggested change
fmt.Printf("WaitForConfirmation until with cotext [base] on tx: %v\n", transaction.Hash())
logger.Debugf("WaitForConfirmation until with context [base] on tx: %v", transaction.Hash())

Comment on lines 244 to 245
fmt.Printf("Got receipt for %v with status: %v\n", transaction.Hash(), receipt.Status)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The fmt.Printf statement for logging the receipt status should be replaced with structured logging for better log management.

- fmt.Printf("Got receipt for %v with status: %v\n", transaction.Hash(), receipt.Status)
+ logger.Infof("Got receipt for %v with status: %v", transaction.Hash(), receipt.Status)

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.

Suggested change
fmt.Printf("Got receipt for %v with status: %v\n", transaction.Hash(), receipt.Status)
}
logger.Infof("Got receipt for %v with status: %v", transaction.Hash(), receipt.Status)
}

if err != nil {
if receipt.Status == types.ReceiptStatusFailed {
rawJSON, _ := transaction.MarshalJSON()
logger.Errorf("transaction %s with body %s reverted", transaction, string(rawJSON))
}
}

fmt.Printf("Canceling txConfirmedCtx [base] on tx: %v\n", transaction.Hash())
Copy link
Contributor

Choose a reason for hiding this comment

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

The fmt.Printf statement for canceling the context should be considered for removal or replacement with debug-level logging, as it might not provide significant value in production logs.

- fmt.Printf("Canceling txConfirmedCtx [base] on tx: %v\n", transaction.Hash())
+ logger.Debugf("Canceling txConfirmedCtx [base] on tx: %v", transaction.Hash())

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.

Suggested change
fmt.Printf("Canceling txConfirmedCtx [base] on tx: %v\n", transaction.Hash())
logger.Debugf("Canceling txConfirmedCtx [base] on tx: %v", transaction.Hash())

@@ -325,6 +326,7 @@ func (f *Backend) FundAccount(ctx context.Context, address common.Address, amoun
// WaitForConfirmation checks confirmation if the transaction is signed.
// nolint: cyclop
func (f *Backend) WaitForConfirmation(ctx context.Context, tx *types.Transaction) {
fmt.Printf("WaitForConfirmation [anvil] called on tx %s\n", tx.Hash().String())
Copy link
Contributor

Choose a reason for hiding this comment

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

The fmt.Printf statement is used for logging the call to WaitForConfirmation. Recommend using a structured logging framework for consistency across the codebase.

- fmt.Printf("WaitForConfirmation [anvil] called on tx %s\n", tx.Hash().String())
+ logger.Infof("WaitForConfirmation [anvil] called on tx %s", tx.Hash().String())

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.

Suggested change
fmt.Printf("WaitForConfirmation [anvil] called on tx %s\n", tx.Hash().String())
logger.Infof("WaitForConfirmation [anvil] called on tx %s", tx.Hash().String())

@@ -335,6 +337,7 @@
return
}

fmt.Printf("WaitForConfirmation [anvil] tx %s is signed\n", tx.Hash().String())
Copy link
Contributor

Choose a reason for hiding this comment

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

The fmt.Printf statement for indicating a signed transaction in WaitForConfirmation should be replaced with structured logging for better log management.

- fmt.Printf("WaitForConfirmation [anvil] tx %s is signed\n", tx.Hash().String())
+ logger.Infof("WaitForConfirmation [anvil] tx %s is signed", tx.Hash().String())

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.

Suggested change
fmt.Printf("WaitForConfirmation [anvil] tx %s is signed\n", tx.Hash().String())
logger.Infof("WaitForConfirmation [anvil] tx %s is signed", tx.Hash().String())

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 31d98fa63f5485c7687527b6f3d74a7915cf28f6 and 2e343033156ad4f14935750a87014f2e6ea54a8e.
Files selected for processing (2)
  • ethergo/manager/manager.go (2 hunks)
  • services/rfq/e2e/setup_test.go (5 hunks)
Additional comments: 3
ethergo/manager/manager.go (1)
  • 9-10: The reordering of import statements improves readability and consistency. This change aligns with best practices for organizing imports in Go, separating standard library imports from third-party packages.
services/rfq/e2e/setup_test.go (2)
  • 34-34: The addition of the chain package from the relayer module is appropriate for accessing Ethereum-specific constants and utilities, such as chain.EthAddress. This change is necessary for the integration of native Ethereum support in the RFQ service.
  • 284-289: Adding Ethereum (ETH) as a quotable token from origin to destination enhances the RFQ service's functionality, allowing for direct ETH-to-ETH quotes. This change aligns with the PR's objective of integrating native Ethereum support. Ensure that the rest of the system, including the quoting and transaction processing mechanisms, is updated to handle these cases correctly.

Comment on lines 93 to 102
// BulkDeploySerial deploys a set of contracts in serial to ensure ordering.
func (d *DeployerManager) BulkDeploySerial(ctx context.Context, testBackends []backends.SimulatedTestBackend, contracts ...contracts.ContractType) {
for _, backend := range testBackends {
cr := d.GetContractRegistry(backend)

for _, contract := range contracts {
cr.Get(ctx, contract)
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The BulkDeploySerial function correctly implements serial deployment of contracts, ensuring that contracts are deployed in the specified order. This is particularly useful when contract deployment order matters due to dependencies or initialization requirements. However, consider adding error handling within the loop to catch and handle any issues that arise during the deployment process. Currently, if cr.Get(ctx, contract) encounters an error, it's not handled, which could lead to partial deployments without clear feedback to the caller.

for _, contract := range contracts {
-   cr.Get(ctx, contract)
+   if err := cr.Get(ctx, contract); err != nil {
+       // Handle error, possibly accumulating errors to return after all attempts
+   }
}

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.

Suggested change
// BulkDeploySerial deploys a set of contracts in serial to ensure ordering.
func (d *DeployerManager) BulkDeploySerial(ctx context.Context, testBackends []backends.SimulatedTestBackend, contracts ...contracts.ContractType) {
for _, backend := range testBackends {
cr := d.GetContractRegistry(backend)
for _, contract := range contracts {
cr.Get(ctx, contract)
}
}
}
// BulkDeploySerial deploys a set of contracts in serial to ensure ordering.
func (d *DeployerManager) BulkDeploySerial(ctx context.Context, testBackends []backends.SimulatedTestBackend, contracts ...contracts.ContractType) {
for _, backend := range testBackends {
cr := d.GetContractRegistry(backend)
for _, contract := range contracts {
if err := cr.Get(ctx, contract); err != nil {
// Handle error, possibly accumulating errors to return after all attempts
}
}
}
}

Comment on lines 212 to 227
Bridge: i.manager.Get(i.GetTestContext(), i.destBackend, testutil.FastBridgeType).Address().String(),
Confirmations: 0,
Tokens: map[string]relconfig.TokenConfig{
"MATIC": {
PriceUSD: 0.5,
"ETH": {
Address: chain.EthAddress.String(),
PriceUSD: 2000,
Decimals: 18,
},
},
NativeToken: "MATIC",
NativeToken: "ETH",
},
},
OmniRPCURL: i.omniServer,
Copy link
Contributor

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [204-217]

The configuration for the "ETH" token within the setupRelayer function correctly sets the Address field using chain.EthAddress.String(), ensuring that the native Ethereum token is recognized and handled properly by the relayer. This change is crucial for supporting native Ethereum transactions without requiring wrapping into ERC-20 tokens. However, ensure that the PriceUSD and Decimals values are accurate and reflect the current standards for Ethereum. Additionally, consider externalizing these values to a configuration file or environment variables for easier updates and maintenance.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 4

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between f687e5d and 644d58c.
Files selected for processing (1)
  • services/rfq/e2e/rfq_test.go (6 hunks)
Additional comments: 1
services/rfq/e2e/rfq_test.go (1)
  • 20-20: The addition of the chain package import is appropriate for the new functionality being tested, specifically the native Ethereum support.

Comment on lines +66 to +68
if core.GetEnvBool("CI", false) {
return
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The conditional check to return early from SetupTest when running in a CI environment is a temporary workaround. Ensure that the underlying issues with Anvil in CI are resolved promptly to avoid skipping important setup steps in tests.

Comment on lines +93 to +95
if core.GetEnvBool("CI", false) {
i.T().Skip("skipping until anvil issues are fixed in CI")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Skipping the TestUSDCtoUSDC test in CI environments due to Anvil issues is a temporary measure. It's crucial to address these issues to ensure comprehensive testing in CI pipelines.

Comment on lines +203 to +205
if core.GetEnvBool("CI", false) {
i.T().Skip("skipping until anvil issues are fixed in CI")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly, skipping the TestETHtoETH test in CI environments is not ideal. Work towards resolving the Anvil CI issues to enable full test coverage.

Comment on lines +202 to +303
i.T().Skip("skipping until anvil issues are fixed in CI")
}
// Send ETH to the relayer on destination
const initialBalance = 10
i.destBackend.FundAccount(i.GetTestContext(), i.relayerWallet.Address(), *big.NewInt(initialBalance))

// let's give the user some money as well
const userWantAmount = 1
i.originBackend.FundAccount(i.GetTestContext(), i.userWallet.Address(), *big.NewInt(userWantAmount))

// non decimal adjusted user want amount
realWantAmount := new(big.Int).Mul(big.NewInt(userWantAmount), big.NewInt(1e18))

// now our friendly user is going to check the quote and send us some ETH on the origin chain.
i.Eventually(func() bool {
// first he's gonna check the quotes.
userAPIClient, err := client.NewAuthenticatedClient(metrics.Get(), i.apiServer, localsigner.NewSigner(i.userWallet.PrivateKey()))
i.NoError(err)

allQuotes, err := userAPIClient.GetAllQuotes()
i.NoError(err)

// let's figure out the amount of ETH we need
for _, quote := range allQuotes {
if common.HexToAddress(quote.DestTokenAddr) == chain.EthAddress {
destAmountBigInt, _ := new(big.Int).SetString(quote.DestAmount, 10)
if destAmountBigInt.Cmp(realWantAmount) > 0 {
// we found our quote!
// now we can move on
return true
}
}
}
return false
})

// now we can send the money
_, originFastBridge := i.manager.GetFastBridge(i.GetTestContext(), i.originBackend)
auth := i.originBackend.GetTxContext(i.GetTestContext(), i.userWallet.AddressPtr())
auth.TransactOpts.Value = realWantAmount
// we want 499 ETH for 500 requested within a day
tx, err := originFastBridge.Bridge(auth.TransactOpts, fastbridge.IFastBridgeBridgeParams{
DstChainId: uint32(i.destBackend.GetChainID()),
To: i.userWallet.Address(),
OriginToken: chain.EthAddress,
SendChainGas: true,
DestToken: chain.EthAddress,
OriginAmount: realWantAmount,
DestAmount: new(big.Int).Sub(realWantAmount, big.NewInt(1e17)),
Deadline: new(big.Int).SetInt64(time.Now().Add(time.Hour * 24).Unix()),
})
i.NoError(err)
i.originBackend.WaitForConfirmation(i.GetTestContext(), tx)

// TODO: this, but cleaner
anvilClient, err := anvil.Dial(i.GetTestContext(), i.originBackend.RPCAddress())
i.NoError(err)

go func() {
for {
select {
case <-i.GetTestContext().Done():
return
case <-time.After(time.Second * 4):
// increase time by 30 mintutes every second, should be enough to get us a fastish e2e test
// we don't need to worry about deadline since we're only doing this on origin
err = anvilClient.IncreaseTime(i.GetTestContext(), 60*30)
i.NoError(err)

// because can claim works on last block timestamp, we need to do something
err = anvilClient.Mine(i.GetTestContext(), 1)
i.NoError(err)
}
}
}()

// since relayer started w/ 0 ETH, once they're offering the inventory up on origin chain we know the workflow completed
i.Eventually(func() bool {
// first he's gonna check the quotes.
relayerAPIClient, err := client.NewAuthenticatedClient(metrics.Get(), i.apiServer, localsigner.NewSigner(i.relayerWallet.PrivateKey()))
i.NoError(err)

allQuotes, err := relayerAPIClient.GetAllQuotes()
i.NoError(err)

// let's figure out the amount of ETH we need
for _, quote := range allQuotes {
if common.HexToAddress(quote.DestTokenAddr) == chain.EthAddress && quote.DestChainID == originBackendChainID {
// we should now have some ETH on the origin chain since we claimed
// this should be offered up as inventory
destAmountBigInt, _ := new(big.Int).SetString(quote.DestAmount, 10)
if destAmountBigInt.Cmp(realWantAmount) > 0 {
// we found our quote!
// now we can move on
return true
}
}
}
return false
})
Copy link
Contributor

Choose a reason for hiding this comment

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

The TestETHtoETH function correctly implements a test for Ethereum to Ethereum transactions, aligning with the PR's objective to introduce native Ethereum support. However, ensure that:

  • The hardcoded values such as initialBalance and userWantAmount are justified and documented.
  • Error handling is consistent, especially after external calls like i.destBackend.FundAccount and originFastBridge.Bridge.
  • The use of i.Eventually for asynchronous operations is appropriate, but consider adding comments to clarify the expectations and the rationale behind the chosen timeouts and intervals.
- // Send ETH to the relayer on destination
+ // Send 10 ETH to the relayer on the destination chain to simulate initial funding.
- // let's give the user some money as well
+ // Fund the user's wallet with 1 ETH on the origin chain for testing.
- // non decimal adjusted user want amount
+ // Convert the user's want amount to Wei for accurate transaction handling.
- // now we can send the money
+ // Execute the bridge transaction to send ETH from the user to the destination.

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.

Suggested change
func (i *IntegrationSuite) TestETHtoETH() {
if core.GetEnvBool("CI", false) {
i.T().Skip("skipping until anvil issues are fixed in CI")
}
// Send ETH to the relayer on destination
const initialBalance = 10
i.destBackend.FundAccount(i.GetTestContext(), i.relayerWallet.Address(), *big.NewInt(initialBalance))
// let's give the user some money as well
const userWantAmount = 1
i.originBackend.FundAccount(i.GetTestContext(), i.userWallet.Address(), *big.NewInt(userWantAmount))
// non decimal adjusted user want amount
realWantAmount := new(big.Int).Mul(big.NewInt(userWantAmount), big.NewInt(1e18))
// now our friendly user is going to check the quote and send us some ETH on the origin chain.
i.Eventually(func() bool {
// first he's gonna check the quotes.
userAPIClient, err := client.NewAuthenticatedClient(metrics.Get(), i.apiServer, localsigner.NewSigner(i.userWallet.PrivateKey()))
i.NoError(err)
allQuotes, err := userAPIClient.GetAllQuotes()
i.NoError(err)
// let's figure out the amount of ETH we need
for _, quote := range allQuotes {
if common.HexToAddress(quote.DestTokenAddr) == chain.EthAddress {
destAmountBigInt, _ := new(big.Int).SetString(quote.DestAmount, 10)
if destAmountBigInt.Cmp(realWantAmount) > 0 {
// we found our quote!
// now we can move on
return true
}
}
}
return false
})
// now we can send the money
_, originFastBridge := i.manager.GetFastBridge(i.GetTestContext(), i.originBackend)
auth := i.originBackend.GetTxContext(i.GetTestContext(), i.userWallet.AddressPtr())
auth.TransactOpts.Value = realWantAmount
// we want 499 ETH for 500 requested within a day
tx, err := originFastBridge.Bridge(auth.TransactOpts, fastbridge.IFastBridgeBridgeParams{
DstChainId: uint32(i.destBackend.GetChainID()),
To: i.userWallet.Address(),
OriginToken: chain.EthAddress,
SendChainGas: true,
DestToken: chain.EthAddress,
OriginAmount: realWantAmount,
DestAmount: new(big.Int).Sub(realWantAmount, big.NewInt(1e17)),
Deadline: new(big.Int).SetInt64(time.Now().Add(time.Hour * 24).Unix()),
})
i.NoError(err)
i.originBackend.WaitForConfirmation(i.GetTestContext(), tx)
// TODO: this, but cleaner
anvilClient, err := anvil.Dial(i.GetTestContext(), i.originBackend.RPCAddress())
i.NoError(err)
go func() {
for {
select {
case <-i.GetTestContext().Done():
return
case <-time.After(time.Second * 4):
// increase time by 30 mintutes every second, should be enough to get us a fastish e2e test
// we don't need to worry about deadline since we're only doing this on origin
err = anvilClient.IncreaseTime(i.GetTestContext(), 60*30)
i.NoError(err)
// because can claim works on last block timestamp, we need to do something
err = anvilClient.Mine(i.GetTestContext(), 1)
i.NoError(err)
}
}
}()
// since relayer started w/ 0 ETH, once they're offering the inventory up on origin chain we know the workflow completed
i.Eventually(func() bool {
// first he's gonna check the quotes.
relayerAPIClient, err := client.NewAuthenticatedClient(metrics.Get(), i.apiServer, localsigner.NewSigner(i.relayerWallet.PrivateKey()))
i.NoError(err)
allQuotes, err := relayerAPIClient.GetAllQuotes()
i.NoError(err)
// let's figure out the amount of ETH we need
for _, quote := range allQuotes {
if common.HexToAddress(quote.DestTokenAddr) == chain.EthAddress && quote.DestChainID == originBackendChainID {
// we should now have some ETH on the origin chain since we claimed
// this should be offered up as inventory
destAmountBigInt, _ := new(big.Int).SetString(quote.DestAmount, 10)
if destAmountBigInt.Cmp(realWantAmount) > 0 {
// we found our quote!
// now we can move on
return true
}
}
}
return false
})
func (i *IntegrationSuite) TestETHtoETH() {
if core.GetEnvBool("CI", false) {
i.T().Skip("skipping until anvil issues are fixed in CI")
}
// Send 10 ETH to the relayer on the destination chain to simulate initial funding.
const initialBalance = 10
i.destBackend.FundAccount(i.GetTestContext(), i.relayerWallet.Address(), *big.NewInt(initialBalance))
// Fund the user's wallet with 1 ETH on the origin chain for testing.
const userWantAmount = 1
i.originBackend.FundAccount(i.GetTestContext(), i.userWallet.Address(), *big.NewInt(userWantAmount))
// Convert the user's want amount to Wei for accurate transaction handling.
realWantAmount := new(big.Int).Mul(big.NewInt(userWantAmount), big.NewInt(1e18))
// now our friendly user is going to check the quote and send us some ETH on the origin chain.
i.Eventually(func() bool {
// first he's gonna check the quotes.
userAPIClient, err := client.NewAuthenticatedClient(metrics.Get(), i.apiServer, localsigner.NewSigner(i.userWallet.PrivateKey()))
i.NoError(err)
allQuotes, err := userAPIClient.GetAllQuotes()
i.NoError(err)
// let's figure out the amount of ETH we need
for _, quote := range allQuotes {
if common.HexToAddress(quote.DestTokenAddr) == chain.EthAddress {
destAmountBigInt, _ := new(big.Int).SetString(quote.DestAmount, 10)
if destAmountBigInt.Cmp(realWantAmount) > 0 {
// we found our quote!
// now we can move on
return true
}
}
}
return false
})
// Execute the bridge transaction to send ETH from the user to the destination.
_, originFastBridge := i.manager.GetFastBridge(i.GetTestContext(), i.originBackend)
auth := i.originBackend.GetTxContext(i.GetTestContext(), i.userWallet.AddressPtr())
auth.TransactOpts.Value = realWantAmount
// we want 499 ETH for 500 requested within a day
tx, err := originFastBridge.Bridge(auth.TransactOpts, fastbridge.IFastBridgeBridgeParams{
DstChainId: uint32(i.destBackend.GetChainID()),
To: i.userWallet.Address(),
OriginToken: chain.EthAddress,
SendChainGas: true,
DestToken: chain.EthAddress,
OriginAmount: realWantAmount,
DestAmount: new(big.Int).Sub(realWantAmount, big.NewInt(1e17)),
Deadline: new(big.Int).SetInt64(time.Now().Add(time.Hour * 24).Unix()),
})
i.NoError(err)
i.originBackend.WaitForConfirmation(i.GetTestContext(), tx)
// TODO: this, but cleaner
anvilClient, err := anvil.Dial(i.GetTestContext(), i.originBackend.RPCAddress())
i.NoError(err)
go func() {
for {
select {
case <-i.GetTestContext().Done():
return
case <-time.After(time.Second * 4):
// increase time by 30 mintutes every second, should be enough to get us a fastish e2e test
// we don't need to worry about deadline since we're only doing this on origin
err = anvilClient.IncreaseTime(i.GetTestContext(), 60*30)
i.NoError(err)
// because can claim works on last block timestamp, we need to do something
err = anvilClient.Mine(i.GetTestContext(), 1)
i.NoError(err)
}
}
}()
// since relayer started w/ 0 ETH, once they're offering the inventory up on origin chain we know the workflow completed
i.Eventually(func() bool {
// first he's gonna check the quotes.
relayerAPIClient, err := client.NewAuthenticatedClient(metrics.Get(), i.apiServer, localsigner.NewSigner(i.relayerWallet.PrivateKey()))
i.NoError(err)
allQuotes, err := relayerAPIClient.GetAllQuotes()
i.NoError(err)
// let's figure out the amount of ETH we need
for _, quote := range allQuotes {
if common.HexToAddress(quote.DestTokenAddr) == chain.EthAddress && quote.DestChainID == originBackendChainID {
// we should now have some ETH on the origin chain since we claimed
// this should be offered up as inventory
destAmountBigInt, _ := new(big.Int).SetString(quote.DestAmount, 10)
if destAmountBigInt.Cmp(realWantAmount) > 0 {
// we found our quote!
// now we can move on
return true
}
}
}
return false
})

@dwasse dwasse merged commit 9187ace into master Feb 1, 2024
29 of 30 checks passed
@dwasse dwasse deleted the feat/rfq-eth branch February 1, 2024 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code size/m
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFQ] ETH Support
3 participants