feat(rfq-guard): v2 guard logic [SLT-387] #3324
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -63,6 +63,10 @@ | |||||
| originBackendChainID: i.manager.Get(i.GetTestContext(), i.originBackend, testutil.FastBridgeType).Address().String(), | ||||||
| destBackendChainID: i.manager.Get(i.GetTestContext(), i.destBackend, testutil.FastBridgeType).Address().String(), | ||||||
| }, | ||||||
| FastBridgeContractsV2: map[uint32]string{ | ||||||
| originBackendChainID: i.manager.Get(i.GetTestContext(), i.originBackend, testutil.FastBridgeV2Type).Address().String(), | ||||||
| destBackendChainID: i.manager.Get(i.GetTestContext(), i.destBackend, testutil.FastBridgeV2Type).Address().String(), | ||||||
| }, | ||||||
| Port: strconv.Itoa(apiPort), | ||||||
| } | ||||||
| api, err := rest.NewAPI(i.GetTestContext(), apiCfg, i.metrics, i.omniClient, apiStore) | ||||||
|
|
@@ -147,7 +151,7 @@ | |||||
| // but this way we can do something while we're waiting for the other backend to startup. | ||||||
| // no need to wait for these to deploy since they can happen in background as soon as the backend is up. | ||||||
| predeployTokens := []contracts.ContractType{testutil.DAIType, testutil.USDTType, testutil.WETH9Type} | ||||||
| predeploys := append(predeployTokens, testutil.FastBridgeV2Type) | ||||||
|
There was a problem hiding this comment. Ensure both FastBridge versions are deployed in Currently, only Apply this diff to include both versions: predeployTokens := []contracts.ContractType{testutil.DAIType, testutil.USDTType, testutil.WETH9Type}
- predeploys := append(predeployTokens, testutil.FastBridgeV2Type)
+ predeploys := append(predeployTokens, testutil.FastBridgeType, testutil.FastBridgeV2Type)📝 Committable suggestion
Suggested change
|
||||||
| slices.Reverse(predeploys) // return fast bridge first | ||||||
|
|
||||||
| ethAmount := *new(big.Int).Mul(big.NewInt(params.Ether), big.NewInt(10)) | ||||||
|
|
@@ -267,15 +271,41 @@ | |||||
|
|
||||||
| erc20, err := ierc20.NewIERC20(token.Address(), backend) | ||||||
| i.Require().NoError(err, "Failed to get erc20") | ||||||
|
|
||||||
| // approve fastbridgev1 | ||||||
| _, fastBridgeV1 := i.manager.GetFastBridge(i.GetTestContext(), backend) | ||||||
| allowance, err := erc20.Allowance(&bind.CallOpts{Context: i.GetTestContext()}, user.Address(), fastBridgeV1.Address()) | ||||||
| i.Require().NoError(err, "Failed to get allowance") | ||||||
|
|
||||||
| if allowance.Cmp(big.NewInt(0)) == 0 { | ||||||
| err = retry.WithBackoff(i.GetTestContext(), func(ctx context.Context) error { | ||||||
| txOpts := backend.GetTxContext(ctx, user.AddressPtr()) | ||||||
| tx, err := erc20.Approve(txOpts.TransactOpts, fastBridgeV1.Address(), core.CopyBigInt(abi.MaxUint256)) | ||||||
| if err != nil { | ||||||
| return fmt.Errorf("failed to approve: %w", err) | ||||||
| } | ||||||
| backend.WaitForConfirmation(ctx, tx) | ||||||
| return nil | ||||||
| }) | ||||||
| i.Require().NoError(err, "Failed to approve") | ||||||
| } | ||||||
|
|
||||||
| // approve fastbridgev2 | ||||||
| _, fastBridgeV2 := i.manager.GetFastBridgeV2(i.GetTestContext(), backend) | ||||||
| allowance, err = erc20.Allowance(&bind.CallOpts{Context: i.GetTestContext()}, user.Address(), fastBridgeV2.Address()) | ||||||
| i.Require().NoError(err, "Failed to get allowance") | ||||||
|
|
||||||
| if allowance.Cmp(big.NewInt(0)) == 0 { | ||||||
| err = retry.WithBackoff(i.GetTestContext(), func(ctx context.Context) error { | ||||||
| txOpts := backend.GetTxContext(ctx, user.AddressPtr()) | ||||||
| tx, err := erc20.Approve(txOpts.TransactOpts, fastBridgeV2.Address(), core.CopyBigInt(abi.MaxUint256)) | ||||||
| if err != nil { | ||||||
| return fmt.Errorf("failed to approve: %w", err) | ||||||
| } | ||||||
| backend.WaitForConfirmation(ctx, tx) | ||||||
| return nil | ||||||
| }) | ||||||
| i.Require().NoError(err, "Failed to approve") | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -286,11 +316,14 @@ | |||||
| dsn := filet.TmpDir(i.T(), "") | ||||||
| cctpContractOrigin, _ := i.cctpDeployManager.GetSynapseCCTP(i.GetTestContext(), i.originBackend) | ||||||
| cctpContractDest, _ := i.cctpDeployManager.GetSynapseCCTP(i.GetTestContext(), i.destBackend) | ||||||
| rfqAddressV1Origin := i.manager.Get(i.GetTestContext(), i.originBackend, testutil.FastBridgeType).Address().String() | ||||||
| rfqAddressV1Dest := i.manager.Get(i.GetTestContext(), i.destBackend, testutil.FastBridgeType).Address().String() | ||||||
| return relconfig.Config{ | ||||||
| // generated ex-post facto | ||||||
| Chains: map[int]relconfig.ChainConfig{ | ||||||
| originBackendChainID: { | ||||||
| RFQAddress: i.manager.Get(i.GetTestContext(), i.originBackend, testutil.FastBridgeV2Type).Address().String(), | ||||||
| RFQAddressV1: &rfqAddressV1Origin, | ||||||
| RebalanceConfigs: relconfig.RebalanceConfigs{ | ||||||
| Synapse: &relconfig.SynapseCCTPRebalanceConfig{ | ||||||
| SynapseCCTPAddress: cctpContractOrigin.Address().Hex(), | ||||||
|
|
@@ -307,7 +340,8 @@ | |||||
| NativeToken: "ETH", | ||||||
| }, | ||||||
| destBackendChainID: { | ||||||
| RFQAddress: i.manager.Get(i.GetTestContext(), i.destBackend, testutil.FastBridgeV2Type).Address().String(), | ||||||
| RFQAddressV1: &rfqAddressV1Dest, | ||||||
| RebalanceConfigs: relconfig.RebalanceConfigs{ | ||||||
| Synapse: &relconfig.SynapseCCTPRebalanceConfig{ | ||||||
| SynapseCCTPAddress: cctpContractDest.Address().Hex(), | ||||||
|
|
@@ -362,26 +396,38 @@ | |||||
| defer wg.Done() | ||||||
|
|
||||||
| err := retry.WithBackoff(i.GetTestContext(), func(ctx context.Context) error { | ||||||
| metadataV1, rfqContractV1 := i.manager.GetFastBridge(i.GetTestContext(), backend) | ||||||
| txContextV1 := backend.GetTxContext(i.GetTestContext(), metadataV1.OwnerPtr()) | ||||||
|
|
||||||
| relayerRole, err := rfqContractV1.RELAYERROLE(&bind.CallOpts{Context: i.GetTestContext()}) | ||||||
| proverRole, err := rfqContractV1.RELAYERROLE(&bind.CallOpts{Context: i.GetTestContext()}) | ||||||
| if err != nil { | ||||||
| return fmt.Errorf("could not get prover role: %w", err) | ||||||
| } | ||||||
| tx, err := rfqContractV1.GrantRole(txContextV1.TransactOpts, relayerRole, i.relayerWallet.Address()) | ||||||
| if err != nil { | ||||||
| return fmt.Errorf("could not grant relayer role: %w", err) | ||||||
| } | ||||||
| backend.WaitForConfirmation(i.GetTestContext(), tx) | ||||||
|
|
||||||
| metadataV2, rfqContractV2 := i.manager.GetFastBridgeV2(i.GetTestContext(), backend) | ||||||
| txContextV2 := backend.GetTxContext(i.GetTestContext(), metadataV2.OwnerPtr()) | ||||||
|
|
||||||
| proverRole, err = rfqContractV2.PROVERROLE(&bind.CallOpts{Context: i.GetTestContext()}) | ||||||
| if err != nil { | ||||||
| return fmt.Errorf("could not get prover role: %w", err) | ||||||
| } | ||||||
| tx, err = rfqContractV2.GrantRole(txContextV2.TransactOpts, proverRole, i.relayerWallet.Address()) | ||||||
| if err != nil { | ||||||
| return fmt.Errorf("could not grant prover role: %w", err) | ||||||
| } | ||||||
| backend.WaitForConfirmation(i.GetTestContext(), tx) | ||||||
|
|
||||||
| quoterRole, err := rfqContractV2.QUOTERROLE(&bind.CallOpts{Context: i.GetTestContext()}) | ||||||
| if err != nil { | ||||||
| return fmt.Errorf("could not get quoter role: %w", err) | ||||||
| } | ||||||
| tx, err = rfqContractV2.GrantRole(txContextV2.TransactOpts, quoterRole, i.relayerWallet.Address()) | ||||||
| if err != nil { | ||||||
| return fmt.Errorf("could not grant quoter role: %w", err) | ||||||
| } | ||||||
|
|
@@ -488,13 +534,24 @@ | |||||
| go func(backend backends.SimulatedTestBackend) { | ||||||
| defer wg.Done() | ||||||
|
|
||||||
| metadataV1, rfqContractV1 := i.manager.GetFastBridge(i.GetTestContext(), backend) | ||||||
|
|
||||||
| txContext := backend.GetTxContext(i.GetTestContext(), metadataV1.OwnerPtr()) | ||||||
| guardRole, err := rfqContractV1.GUARDROLE(&bind.CallOpts{Context: i.GetTestContext()}) | ||||||
| i.NoError(err) | ||||||
|
|
||||||
| tx, err := rfqContractV1.GrantRole(txContext.TransactOpts, guardRole, i.guardWallet.Address()) | ||||||
| i.NoError(err) | ||||||
|
|
||||||
| backend.WaitForConfirmation(i.GetTestContext(), tx) | ||||||
|
|
||||||
| metadataV2, rfqContractV2 := i.manager.GetFastBridgeV2(i.GetTestContext(), backend) | ||||||
|
|
||||||
| txContext = backend.GetTxContext(i.GetTestContext(), metadataV2.OwnerPtr()) | ||||||
| guardRole, err = rfqContractV2.GUARDROLE(&bind.CallOpts{Context: i.GetTestContext()}) | ||||||
| i.NoError(err) | ||||||
|
|
||||||
| tx, err = rfqContractV2.GrantRole(txContext.TransactOpts, guardRole, i.guardWallet.Address()) | ||||||
| i.NoError(err) | ||||||
|
|
||||||
| backend.WaitForConfirmation(i.GetTestContext(), tx) | ||||||
|
|
||||||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -34,7 +34,9 @@ type Config struct { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // ChainConfig represents the configuration for a chain. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type ChainConfig struct { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // RFQAddressV1 is the legacy V1 rfq bridge contract address. OPTIONAL. Only populate if also guarding a deprecated V1 contract. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RFQAddressV1 *string `yaml:"rfq_address_v1"` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // RFQAddress is the current/latest rfq bridge contract address. REQUIRED. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RFQAddress string `yaml:"rfq_address"` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Confirmations is the number of required confirmations. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Confirmations uint64 `yaml:"confirmations"` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -66,12 +68,19 @@ func LoadConfig(path string) (config Config, err error) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Validate validates the config. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (c Config) Validate() (err error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for chainID := range c.Chains { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| addrV1, err := c.GetRFQAddressV1(chainID) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return fmt.Errorf("could not get v1 rfq address: %w", err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if addrV1 != nil && !common.IsHexAddress(*addrV1) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return fmt.Errorf("invalid rfq v1 address: %s", *addrV1) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| addrV2, err := c.GetRFQAddressV2(chainID) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return fmt.Errorf("could not get v2 rfq address: %w", err) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if !common.IsHexAddress(addrV2) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return fmt.Errorf("invalid rfq v2 address: %s", addrV2) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+71
to
+83
There was a problem hiding this comment. Add test coverage for address validation logic The validation logic for both V1 and V2 addresses lacks test coverage. Consider adding test cases for:
Would you like me to help create comprehensive test cases for the validation logic? Additionally, consider making error messages more descriptive: -return fmt.Errorf("could not get v1 rfq address: %w", err)
+return fmt.Errorf("failed to retrieve RFQ V1 address for chain %d: %w", chainID, err)
-return fmt.Errorf("invalid rfq v1 address: %s", *addrV1)
+return fmt.Errorf("invalid RFQ V1 address format for chain %d: %s", chainID, *addrV1)
-return fmt.Errorf("could not get v2 rfq address: %w", err)
+return fmt.Errorf("failed to retrieve RFQ V2 address for chain %d: %w", chainID, err)
-return fmt.Errorf("invalid rfq v2 address: %s", addrV2)
+return fmt.Errorf("invalid RFQ V2 address format for chain %d: %s", chainID, addrV2)📝 Committable suggestion
Suggested change
🧰 Tools🪛 GitHub Check: codecov/patch[warning] 71-71: services/rfq/guard/guardconfig/config.go#L71 [warning] 75-75: services/rfq/guard/guardconfig/config.go#L75 [warning] 77-79: services/rfq/guard/guardconfig/config.go#L77-L79 [warning] 81-82: services/rfq/guard/guardconfig/config.go#L81-L82 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -83,11 +92,20 @@ func (c Config) GetChains() map[int]ChainConfig { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return c.Chains | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // GetRFQAddressV1 returns the RFQ address for the given chain. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (c Config) GetRFQAddressV1(chainID int) (*string, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| chainCfg, ok := c.Chains[chainID] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if !ok { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil, fmt.Errorf("v1 chain config not found for chain %d", chainID) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return chainCfg.RFQAddressV1, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // GetRFQAddressV2 returns the RFQ address for the given chain. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func (c Config) GetRFQAddressV2(chainID int) (string, error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| chainCfg, ok := c.Chains[chainID] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if !ok { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "", fmt.Errorf("v2 chain config not found for chain %d", chainID) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return chainCfg.RFQAddress, nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -115,6 +133,7 @@ func NewGuardConfigFromRelayer(relayerCfg relconfig.Config) Config { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for chainID, chainCfg := range relayerCfg.GetChains() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cfg.Chains[chainID] = ChainConfig{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RFQAddressV1: chainCfg.RFQAddressV1, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RFQAddress: chainCfg.RFQAddress, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Confirmations: chainCfg.FinalityConfirmations, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+136
to
139
There was a problem hiding this comment. Add test coverage for guard config creation The NewGuardConfigFromRelayer function's handling of the new RFQAddressV1 field is untested. Consider adding test cases to verify:
Would you like me to help create test cases for the config creation logic? 🧰 Tools🪛 GitHub Check: codecov/patch[warning] 136-136: services/rfq/guard/guardconfig/config.go#L136 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Potential mismatch in parser usage: using
fastbridge.NewParserwith FastBridgeV2In
TestDisputeV2, you're creating a parser withfastbridge.NewParser(originFastBridge.Address()), butoriginFastBridgeis a FastBridgeV2 instance. This may lead to incorrect event parsing. Consider usingfastbridgev2.NewParserinstead to match the contract version.Apply this diff to use the correct parser:
📝 Committable suggestion