feat(rfq-relayer): highly concurrent e2e test #3042
Conversation
|
Warning Rate limit exceeded@dwasse has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 35 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe integration suite has been enhanced with a new test function, Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Deploying sanguine-fe with
|
| Latest commit: |
829e597
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0215ac6d.sanguine-fe.pages.dev |
| Branch Preview URL: | https://feat-e2e-multi-test.sanguine-fe.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- services/rfq/e2e/rfq_test.go (3 hunks)
Additional context used
GitHub Check: Lint (services/rfq)
services/rfq/e2e/rfq_test.go
[failure] 476-476:
cognitive complexity 42 of func(*IntegrationSuite).TestMultipleBridgesis high (> 30) (gocognit)
[failure] 624-624:
unnecessary trailing newline (whitespace)
services/rfq/e2e/rfq_test.go
Outdated
| parser, err := fastbridge.NewParser(originFastBridge.Address()) | ||
| i.NoError(err) | ||
|
|
||
| txIDs := [][32]byte{} | ||
| txMux := sync.Mutex{} | ||
| sendBridgeReq := func(nonce *big.Int) (*types.Transaction, error) { | ||
| txMux.Lock() | ||
| auth.TransactOpts.Nonce = nonce | ||
| defer txMux.Unlock() | ||
| tx, err = originFastBridge.Bridge(auth.TransactOpts, fastbridge.IFastBridgeBridgeParams{ | ||
| DstChainId: uint32(i.destBackend.GetChainID()), | ||
| To: i.userWallet.Address(), | ||
| OriginToken: originUSDC.Address(), | ||
| SendChainGas: true, | ||
| DestToken: destUSDC.Address(), | ||
| OriginAmount: realRFQAmount, | ||
| DestAmount: new(big.Int).Sub(realRFQAmount, big.NewInt(5_000_000)), | ||
| Deadline: new(big.Int).SetInt64(time.Now().Add(time.Hour * 24).Unix()), | ||
| }) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to send bridge request: %w", err) | ||
| } | ||
| return tx, nil | ||
| } | ||
|
|
||
| // send several txs at once and record txids | ||
| numTxs := 100 | ||
| txIDMux := sync.Mutex{} | ||
| g, ctx := errgroup.WithContext(i.GetTestContext()) | ||
| for k := 0; k < numTxs; k++ { | ||
| nonce := big.NewInt(int64(k)) | ||
| g.Go(func() error { | ||
| tx, err := sendBridgeReq(nonce) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to send bridge request: %w", err) | ||
| } | ||
|
|
||
| i.originBackend.WaitForConfirmation(ctx, tx) | ||
| receipt, err := i.originBackend.TransactionReceipt(ctx, tx.Hash()) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get receipt: %w", err) | ||
| } | ||
| for _, log := range receipt.Logs { | ||
| _, parsedEvent, ok := parser.ParseEvent(*log) | ||
| if !ok { | ||
| continue | ||
| } | ||
| event, ok := parsedEvent.(*fastbridge.FastBridgeBridgeRequested) | ||
| if ok { | ||
| txIDMux.Lock() | ||
| txIDs = append(txIDs, event.TransactionId) | ||
| txIDMux.Unlock() | ||
| return nil | ||
| } | ||
| } | ||
| return nil | ||
| }) | ||
| } | ||
| err = g.Wait() | ||
| i.NoError(err) | ||
| i.Equal(numTxs, len(txIDs)) | ||
|
|
||
| // 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) | ||
| } | ||
| } | ||
|
|
||
| }() | ||
|
|
||
| // verify that each tx is relayed | ||
| i.Eventually(func() bool { | ||
| for _, txID := range txIDs { | ||
| result, err := i.store.GetQuoteRequestByID(i.GetTestContext(), txID) | ||
| if err != nil { | ||
| return false | ||
| } | ||
| fmt.Printf("result: %+v\n", result) | ||
| if result.Status <= reldb.ProvePosted { | ||
| return false | ||
| } | ||
| } | ||
| return true | ||
| }) | ||
| } |
There was a problem hiding this comment.
Refactor to reduce cognitive complexity.
The function TestMultipleBridges has a cognitive complexity of 42, which is above the recommended threshold of 30. Consider breaking down the function into smaller, more manageable pieces to improve readability and maintainability.
Address the TODO comment for time manipulation.
The comment suggests that the time manipulation logic should be cleaned up. Consider refactoring this part of the code to make it more concise and maintainable.
Remove unnecessary trailing newline.
There is an unnecessary trailing newline at the end of the function. Removing it will improve code cleanliness.
- })
+ })Committable suggestion was skipped due to low confidence.
Tools
GitHub Check: Lint (services/rfq)
[failure] 476-476:
cognitive complexity 42 of func(*IntegrationSuite).TestMultipleBridgesis high (> 30) (gocognit)
[failure] 624-624:
unnecessary trailing newline (whitespace)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3042 +/- ##
===================================================
- Coverage 23.40235% 16.88980% -6.51255%
===================================================
Files 644 139 -505
Lines 50183 10681 -39502
Branches 82 0 -82
===================================================
- Hits 11744 1804 -9940
+ Misses 37274 8629 -28645
+ Partials 1165 248 -917
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- services/rfq/e2e/rfq_test.go (3 hunks)
Additional context used
GitHub Check: Lint (services/rfq)
services/rfq/e2e/rfq_test.go
[failure] 476-476:
cognitive complexity 42 of func(*IntegrationSuite).TestConcurrentBridgesis high (> 30) (gocognit)
[failure] 624-624:
unnecessary trailing newline (whitespace)
services/rfq/e2e/rfq_test.go
Outdated
| auth := i.originBackend.GetTxContext(i.GetTestContext(), i.userWallet.AddressPtr()) | ||
| parser, err := fastbridge.NewParser(originFastBridge.Address()) | ||
| i.NoError(err) | ||
|
|
||
| txIDs := [][32]byte{} | ||
| txMux := sync.Mutex{} | ||
| sendBridgeReq := func(nonce *big.Int) (*types.Transaction, error) { | ||
| txMux.Lock() | ||
| auth.TransactOpts.Nonce = nonce | ||
| defer txMux.Unlock() | ||
| tx, err = originFastBridge.Bridge(auth.TransactOpts, fastbridge.IFastBridgeBridgeParams{ | ||
| DstChainId: uint32(i.destBackend.GetChainID()), | ||
| To: i.userWallet.Address(), | ||
| OriginToken: originUSDC.Address(), | ||
| SendChainGas: true, | ||
| DestToken: destUSDC.Address(), | ||
| OriginAmount: realRFQAmount, | ||
| DestAmount: new(big.Int).Sub(realRFQAmount, big.NewInt(5_000_000)), | ||
| Deadline: new(big.Int).SetInt64(time.Now().Add(time.Hour * 24).Unix()), | ||
| }) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to send bridge request: %w", err) | ||
| } | ||
| return tx, nil | ||
| } | ||
|
|
||
| // send several txs at once and record txids | ||
| numTxs := 100 | ||
| txIDMux := sync.Mutex{} | ||
| g, ctx := errgroup.WithContext(i.GetTestContext()) | ||
| for k := 0; k < numTxs; k++ { | ||
| nonce := big.NewInt(int64(k)) | ||
| g.Go(func() error { | ||
| tx, err := sendBridgeReq(nonce) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to send bridge request: %w", err) | ||
| } | ||
|
|
||
| i.originBackend.WaitForConfirmation(ctx, tx) | ||
| receipt, err := i.originBackend.TransactionReceipt(ctx, tx.Hash()) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get receipt: %w", err) | ||
| } | ||
| for _, log := range receipt.Logs { | ||
| _, parsedEvent, ok := parser.ParseEvent(*log) | ||
| if !ok { | ||
| continue | ||
| } | ||
| event, ok := parsedEvent.(*fastbridge.FastBridgeBridgeRequested) | ||
| if ok { | ||
| txIDMux.Lock() | ||
| txIDs = append(txIDs, event.TransactionId) | ||
| txIDMux.Unlock() | ||
| return nil | ||
| } | ||
| } | ||
| return nil | ||
| }) | ||
| } | ||
| err = g.Wait() | ||
| i.NoError(err) | ||
| i.Equal(numTxs, len(txIDs)) | ||
|
|
||
| // 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) | ||
| } | ||
| } | ||
|
|
||
| }() | ||
|
|
||
| // verify that each tx is relayed | ||
| i.Eventually(func() bool { | ||
| for _, txID := range txIDs { | ||
| result, err := i.store.GetQuoteRequestByID(i.GetTestContext(), txID) | ||
| if err != nil { | ||
| return false | ||
| } | ||
| if result.Status <= reldb.ProvePosted { | ||
| return false | ||
| } | ||
| } | ||
| return true | ||
| }) | ||
| } |
There was a problem hiding this comment.
Refactor to reduce cognitive complexity.
The function TestConcurrentBridges has a cognitive complexity of 42, which is above the recommended threshold of 30. Consider breaking down the function into smaller, more manageable pieces to improve readability and maintainability.
Refactor the function by creating helper functions for distinct operations such as token setup, transaction sending, and verification.
Address the TODO comment for time manipulation.
The comment suggests that the time manipulation logic should be cleaned up. Consider refactoring this part of the code to make it more concise and maintainable.
Refactor the time manipulation logic into a separate function or utility to improve clarity and reusability.
Remove unnecessary trailing newline.
There is an unnecessary trailing newline at the end of the function. Removing it will improve code cleanliness.
Remove the trailing newline as follows:
- })
+ })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.
| func (i *IntegrationSuite) TestConcurrentBridges() { | |
| // start the relayer and guard | |
| go func() { | |
| _ = i.relayer.Start(i.GetTestContext()) | |
| }() | |
| go func() { | |
| _ = i.guard.Start(i.GetTestContext()) | |
| }() | |
| // load token contracts | |
| const startAmount = 10000 | |
| const rfqAmount = 10 | |
| opts := i.destBackend.GetTxContext(i.GetTestContext(), nil) | |
| destUSDC, destUSDCHandle := i.cctpDeployManager.GetMockMintBurnTokenType(i.GetTestContext(), i.destBackend) | |
| realStartAmount, err := testutil.AdjustAmount(i.GetTestContext(), big.NewInt(startAmount), destUSDC.ContractHandle()) | |
| i.NoError(err) | |
| realRFQAmount, err := testutil.AdjustAmount(i.GetTestContext(), big.NewInt(rfqAmount), destUSDC.ContractHandle()) | |
| i.NoError(err) | |
| // add initial usdc to relayer on destination | |
| tx, err := destUSDCHandle.MintPublic(opts.TransactOpts, i.relayerWallet.Address(), realStartAmount) | |
| i.Nil(err) | |
| i.destBackend.WaitForConfirmation(i.GetTestContext(), tx) | |
| i.Approve(i.destBackend, destUSDC, i.relayerWallet) | |
| // add initial USDC to relayer on origin | |
| optsOrigin := i.originBackend.GetTxContext(i.GetTestContext(), nil) | |
| originUSDC, originUSDCHandle := i.cctpDeployManager.GetMockMintBurnTokenType(i.GetTestContext(), i.originBackend) | |
| tx, err = originUSDCHandle.MintPublic(optsOrigin.TransactOpts, i.relayerWallet.Address(), realStartAmount) | |
| i.Nil(err) | |
| i.originBackend.WaitForConfirmation(i.GetTestContext(), tx) | |
| i.Approve(i.originBackend, originUSDC, i.relayerWallet) | |
| // add initial USDC to user on origin | |
| tx, err = originUSDCHandle.MintPublic(optsOrigin.TransactOpts, i.userWallet.Address(), realStartAmount) | |
| i.Nil(err) | |
| i.originBackend.WaitForConfirmation(i.GetTestContext(), tx) | |
| i.Approve(i.originBackend, originUSDC, i.userWallet) | |
| // non decimal adjusted user want amount | |
| // now our friendly user is going to check the quote and send us some USDC 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.GetTestContext()) | |
| i.NoError(err) | |
| // let's figure out the amount of usdc we need | |
| for _, quote := range allQuotes { | |
| if common.HexToAddress(quote.DestTokenAddr) == destUSDC.Address() { | |
| destAmountBigInt, _ := new(big.Int).SetString(quote.DestAmount, 10) | |
| if destAmountBigInt.Cmp(realRFQAmount) > 0 { | |
| // we found our quote! | |
| // now we can move on | |
| return true | |
| } | |
| } | |
| } | |
| return false | |
| }) | |
| _, originFastBridge := i.manager.GetFastBridge(i.GetTestContext(), i.originBackend) | |
| auth := i.originBackend.GetTxContext(i.GetTestContext(), i.userWallet.AddressPtr()) | |
| parser, err := fastbridge.NewParser(originFastBridge.Address()) | |
| i.NoError(err) | |
| txIDs := [][32]byte{} | |
| txMux := sync.Mutex{} | |
| sendBridgeReq := func(nonce *big.Int) (*types.Transaction, error) { | |
| txMux.Lock() | |
| auth.TransactOpts.Nonce = nonce | |
| defer txMux.Unlock() | |
| tx, err = originFastBridge.Bridge(auth.TransactOpts, fastbridge.IFastBridgeBridgeParams{ | |
| DstChainId: uint32(i.destBackend.GetChainID()), | |
| To: i.userWallet.Address(), | |
| OriginToken: originUSDC.Address(), | |
| SendChainGas: true, | |
| DestToken: destUSDC.Address(), | |
| OriginAmount: realRFQAmount, | |
| DestAmount: new(big.Int).Sub(realRFQAmount, big.NewInt(5_000_000)), | |
| Deadline: new(big.Int).SetInt64(time.Now().Add(time.Hour * 24).Unix()), | |
| }) | |
| if err != nil { | |
| return nil, fmt.Errorf("failed to send bridge request: %w", err) | |
| } | |
| return tx, nil | |
| } | |
| // send several txs at once and record txids | |
| numTxs := 100 | |
| txIDMux := sync.Mutex{} | |
| g, ctx := errgroup.WithContext(i.GetTestContext()) | |
| for k := 0; k < numTxs; k++ { | |
| nonce := big.NewInt(int64(k)) | |
| g.Go(func() error { | |
| tx, err := sendBridgeReq(nonce) | |
| if err != nil { | |
| return fmt.Errorf("failed to send bridge request: %w", err) | |
| } | |
| i.originBackend.WaitForConfirmation(ctx, tx) | |
| receipt, err := i.originBackend.TransactionReceipt(ctx, tx.Hash()) | |
| if err != nil { | |
| return fmt.Errorf("failed to get receipt: %w", err) | |
| } | |
| for _, log := range receipt.Logs { | |
| _, parsedEvent, ok := parser.ParseEvent(*log) | |
| if !ok { | |
| continue | |
| } | |
| event, ok := parsedEvent.(*fastbridge.FastBridgeBridgeRequested) | |
| if ok { | |
| txIDMux.Lock() | |
| txIDs = append(txIDs, event.TransactionId) | |
| txIDMux.Unlock() | |
| return nil | |
| } | |
| } | |
| return nil | |
| }) | |
| } | |
| err = g.Wait() | |
| i.NoError(err) | |
| i.Equal(numTxs, len(txIDs)) | |
| // 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) | |
| } | |
| } | |
| }() | |
| // verify that each tx is relayed | |
| i.Eventually(func() bool { | |
| for _, txID := range txIDs { | |
| result, err := i.store.GetQuoteRequestByID(i.GetTestContext(), txID) | |
| if err != nil { | |
| return false | |
| } | |
| if result.Status <= reldb.ProvePosted { | |
| return false | |
| } | |
| } | |
| return true | |
| }) | |
| } | |
| }) |
Tools
GitHub Check: Lint (services/rfq)
[failure] 476-476:
cognitive complexity 42 of func(*IntegrationSuite).TestConcurrentBridgesis high (> 30) (gocognit)
[failure] 624-624:
unnecessary trailing newline (whitespace)
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- services/rfq/e2e/rfq_test.go (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- services/rfq/e2e/rfq_test.go
Summary by CodeRabbit