-
Notifications
You must be signed in to change notification settings - Fork 32
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
rfq: mimimal viable withdrawal api #2815
Changes from all commits
3b80d3f
9258369
a17db24
484a9a3
e83dd9d
a3c3516
84dcdd7
6d10be7
5ed6d9a
dce9713
2965bb7
5c4554a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,7 @@ | |
} | ||
|
||
// commands | ||
app.Commands = cli.Commands{runCommand} | ||
app.Commands = cli.Commands{runCommand, withdrawCommand} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure the new command The addition of the new command should be tested to ensure it works as expected. Do you want me to generate the unit testing code or open a GitHub issue to track this task? |
||
shellCommand := commandline.GenerateShellCommand(app.Commands) | ||
app.Commands = append(app.Commands, shellCommand) | ||
app.Action = shellCommand.Action | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,9 +4,11 @@ | |
import ( | ||
"fmt" | ||
|
||
"github.com/ethereum/go-ethereum/common" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure the new command The addition of the new command should be tested to ensure it works as expected. Do you want me to generate the unit testing code or open a GitHub issue to track this task? |
||
"github.com/synapsecns/sanguine/core" | ||
"github.com/synapsecns/sanguine/core/commandline" | ||
"github.com/synapsecns/sanguine/core/metrics" | ||
"github.com/synapsecns/sanguine/services/rfq/relayer/relapi" | ||
"github.com/synapsecns/sanguine/services/rfq/relayer/relconfig" | ||
"github.com/synapsecns/sanguine/services/rfq/relayer/service" | ||
"github.com/urfave/cli/v2" | ||
|
@@ -44,3 +46,79 @@ | |
return nil | ||
}, | ||
} | ||
|
||
var relayerURLFlag = &cli.StringFlag{ | ||
Name: "relayer-url", | ||
Usage: "relayer url", | ||
} | ||
|
||
var chainIDFlag = &cli.StringFlag{ | ||
Name: "chain-id", | ||
Usage: "chain id", | ||
} | ||
|
||
var amountFlag = &cli.StringFlag{ | ||
Name: "amount", | ||
Usage: "amount", | ||
} | ||
|
||
var tokenAddressFlag = &cli.StringFlag{ | ||
Name: "token-address", | ||
Usage: "token address", | ||
} | ||
|
||
var toFlag = &cli.StringFlag{ | ||
Name: "to", | ||
Usage: "to", | ||
} | ||
|
||
// runCommand runs the rfq relayer. | ||
var withdrawCommand = &cli.Command{ | ||
Name: "withdraw", | ||
Description: "run the withdrawal tool", | ||
Flags: []cli.Flag{relayerURLFlag, chainIDFlag, amountFlag, tokenAddressFlag, toFlag, &commandline.LogLevel}, | ||
Action: func(c *cli.Context) (err error) { | ||
if c.String(relayerURLFlag.Name) == "" { | ||
return fmt.Errorf("relayer URL is required") | ||
} | ||
Comment on lines
+81
to
+83
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add tests for Ensure that this validation is tested for various scenarios, such as missing or invalid URLs. Do you want me to generate the unit testing code or open a GitHub issue to track this task? |
||
|
||
client := relapi.NewRelayerClient(metrics.Get(), c.String(relayerURLFlag.Name)) | ||
if err != nil { | ||
return fmt.Errorf("could not create relayer: %w", err) | ||
} | ||
|
||
chainID := c.Uint(chainIDFlag.Name) | ||
if chainID == 0 { | ||
return fmt.Errorf("valid chain ID is required") | ||
} | ||
Comment on lines
+90
to
+93
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add tests for Ensure that this validation is tested for various scenarios, such as missing or invalid chain IDs. Do you want me to generate the unit testing code or open a GitHub issue to track this task? |
||
|
||
amount := c.String(amountFlag.Name) | ||
if amount == "" { | ||
return fmt.Errorf("amount is required") | ||
} | ||
Comment on lines
+95
to
+98
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add tests for Ensure that this validation is tested for various scenarios, such as missing or invalid amounts. Do you want me to generate the unit testing code or open a GitHub issue to track this task? |
||
|
||
tokenAddress := c.String(tokenAddressFlag.Name) | ||
if !common.IsHexAddress(tokenAddress) { | ||
return fmt.Errorf("valid token address is required") | ||
} | ||
Comment on lines
+100
to
+103
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add tests for Ensure that this validation is tested for various scenarios, such as missing or invalid token addresses. Do you want me to generate the unit testing code or open a GitHub issue to track this task? |
||
|
||
to := c.String(toFlag.Name) | ||
if !common.IsHexAddress(to) { | ||
return fmt.Errorf("valid recipient address is required") | ||
} | ||
Comment on lines
+105
to
+108
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add tests for Ensure that this validation is tested for various scenarios, such as missing or invalid recipient addresses. Do you want me to generate the unit testing code or open a GitHub issue to track this task? |
||
|
||
withdrawRequest := relapi.WithdrawRequest{ | ||
ChainID: uint32(chainID), | ||
Amount: amount, | ||
TokenAddress: common.HexToAddress(tokenAddress), | ||
To: common.HexToAddress(to), | ||
} | ||
|
||
_, err = client.Withdraw(c.Context, &withdrawRequest) | ||
if err != nil { | ||
return fmt.Errorf("could not start relayer: %w", err) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🪶 style: Error message should reflect the context of the operation, e.g., 'could not execute withdrawal'. |
||
} | ||
Comment on lines
+117
to
+120
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add tests for the withdrawal process. Ensure that the withdrawal process is tested for various scenarios, such as successful withdrawals, failed withdrawals, and edge cases. Do you want me to generate the unit testing code or open a GitHub issue to track this task? |
||
|
||
return nil | ||
}, | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package relapi | ||
|
||
import ( | ||
"github.com/ethereum/go-ethereum/common" | ||
"github.com/synapsecns/sanguine/services/rfq/relayer/relconfig" | ||
) | ||
|
||
// TokenIDExists checks if a token ID exists in the config. | ||
func TokenIDExists(cfg relconfig.Config, tokenAddress common.Address, chainID int) bool { | ||
return tokenIDExists(cfg, tokenAddress, chainID) | ||
} | ||
Comment on lines
+8
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add tests for Ensure that this function is tested for various scenarios, such as valid token addresses, invalid token addresses, and edge cases. Do you want me to generate the unit testing code or open a GitHub issue to track this task? |
||
|
||
// ToAddressIsWhitelisted checks if a to address is whitelisted. | ||
func ToAddressIsWhitelisted(cfg relconfig.Config, to common.Address) bool { | ||
return toAddressIsWhitelisted(cfg, to) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tip
Codebase Verification
No test coverage found for
withdrawCommand
.It appears that the
withdrawCommand
logic is not covered by any tests. Please ensure that appropriate tests are added to verify the functionality of this command.services/rfq/relayer/cmd/commands.go
: Definition ofwithdrawCommand
.Analysis chain
Verify the integration of the
withdrawCommand
.Ensure that the new
withdrawCommand
is correctly integrated into the CLI application.Verify the definition and coverage of
withdrawCommand
.Ensure that the
withdrawCommand
is correctly defined and that its logic is covered by tests.Verify the test coverage of
withdrawCommand
.Ensure that the
withdrawCommand
logic is covered by tests.Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 193
Script:
Length of output: 1169
Script:
Length of output: 49