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): add MaxRelayAmount #3259

Merged
merged 7 commits into from
Oct 14, 2024
Merged

feat(rfq-relayer): add MaxRelayAmount #3259

merged 7 commits into from
Oct 14, 2024

Conversation

dwasse
Copy link
Collaborator

@dwasse dwasse commented Oct 9, 2024

Summary by CodeRabbit

  • New Features

    • Introduced a maximum quote amount feature to ensure quotes adhere to specified limits.
    • Added new configuration options for maximum quote and relay amounts in the token settings.
  • Bug Fixes

    • Enhanced logic for calculating quote amounts, ensuring accurate deductions for gas costs.
  • Documentation

    • Clarified comments related to quote amount calculations for better understanding.
  • Tests

    • Updated test cases to validate new quote parameters and configurations.

Copy link
Contributor

coderabbitai bot commented Oct 9, 2024

Walkthrough

The pull request introduces several modifications across multiple files to enhance the quote handling functionality in the RFQ service. Key changes include the addition of a maxQuoteAmount variable in the getOriginAmount method to enforce maximum constraints, the introduction of a new quoteParams struct for streamlined parameter handling in tests, and the addition of MaxQuoteAmount and MaxRelayAmount fields in the TokenConfig struct. Additional methods and variables are introduced to improve configuration management.

Changes

File Path Change Summary
services/rfq/relayer/quoter/quoter.go Modified ShouldProcess and getOriginAmount methods to include maxQuoteAmount for clipping quote amounts. Updated method signatures.
services/rfq/relayer/quoter/quoter_test.go Introduced quoteParams struct for handling quote parameters. Updated setQuoteParams function to accept quoteParams. Added new test cases for GetOriginAmount.
services/rfq/relayer/relconfig/config.go Added MaxQuoteAmount and MaxRelayAmount fields to TokenConfig struct for specifying maximum amounts for quotes and relays.
services/rfq/relayer/relconfig/getters.go Introduced defaultMaxQuoteAmount and defaultMaxRelayAmount variables, along with GetMaxQuoteAmount and GetMaxRelayAmount methods in Config struct. Updated existing methods for default value handling.

Possibly related PRs

Suggested labels

size/m

Suggested reviewers

  • aureliusbtc
  • trajan0x
  • ChiTimesChi

🐰 In the meadow where quotes do bloom,
A new max amount makes room!
With params wrapped in a cozy nest,
Our quoting logic is now the best!
So hop along, let’s cheer and play,
For better quotes are here to stay! 🌼


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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 using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added go Pull requests that update Go code size/s labels Oct 9, 2024
Copy link

codecov bot commented Oct 9, 2024

Codecov Report

Attention: Patch coverage is 36.84211% with 24 lines in your changes missing coverage. Please review.

Please upload report for BASE (master@e74153c). Learn more about missing BASE report.
Report is 61 commits behind head on master.

Files with missing lines Patch % Lines
services/rfq/relayer/relconfig/getters.go 0.00000% 24 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master       #3259   +/-   ##
============================================
  Coverage          ?   24.98855%           
============================================
  Files             ?         198           
  Lines             ?       13106           
  Branches          ?          82           
============================================
  Hits              ?        3275           
  Misses            ?        9549           
  Partials          ?         282           
Flag Coverage Δ
opbot 0.48870% <ø> (?)
promexporter 6.81642% <ø> (?)
rfq 24.60395% <36.84211%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (3)
services/rfq/relayer/quoter/quoter_test.go (1)

Line range hint 184-196: LGTM with a minor suggestion: Consider clarifying the function name

The updated setQuoteParams function now uses the new quoteParams struct, which improves consistency and readability. However, the function name might not fully convey that it's setting parameters for both destination and origin tokens. Consider renaming it to setQuoteParamsForBothChains or adding a comment to clarify its behavior.

services/rfq/relayer/relconfig/getters.go (2)

749-750: Consider initializing defaultMaxQuoteAmount

The defaultMaxQuoteAmount is declared but not initialized. While nil is a valid default for this case (signifying no maximum limit), it might be beneficial to explicitly initialize it for clarity.

Consider modifying the declaration as follows:

-var defaultMaxQuoteAmount *big.Int // nil
+var defaultMaxQuoteAmount *big.Int = nil // Explicitly set to nil, signifying no maximum limit

751-782: Consider adding error logging

While the function handles error cases gracefully by returning the default value, it might be beneficial to log these errors for debugging purposes.

Consider adding error logging in cases where the configuration is not found or parsing fails. For example:

 if !ok {
+    log.Printf("Error: Chain configuration not found for chainID: %d", chainID)
     return defaultMaxQuoteAmount
 }
 if !ok {
+    log.Printf("Error: Failed to parse MaxQuoteAmount for chainID: %d, token: %s", chainID, addr.Hex())
     return defaultMaxQuoteAmount
 }
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between e74153c and 1f0b71c.

📒 Files selected for processing (4)
  • services/rfq/relayer/quoter/quoter.go (2 hunks)
  • services/rfq/relayer/quoter/quoter_test.go (2 hunks)
  • services/rfq/relayer/relconfig/config.go (1 hunks)
  • services/rfq/relayer/relconfig/getters.go (1 hunks)
🧰 Additional context used
🔇 Additional comments (6)
services/rfq/relayer/quoter/quoter_test.go (3)

176-182: LGTM: Introduction of quoteParams struct improves code organization

The new quoteParams struct effectively groups related quote parameters, enhancing the readability and maintainability of the test cases.


214-219: LGTM: Well-structured test cases using the new quoteParams struct

The updated test cases effectively utilize the new quoteParams struct, improving readability and maintainability. They cover a good range of scenarios, including edge cases, with clear expected results and assertions.

Also applies to: 226-231, 238-243, 250-255, 262-267


273-284: LGTM: New test case for MaxQuoteAmount

The new test case effectively verifies the behavior of the MaxQuoteAmount parameter. It's a valuable addition to ensure the new functionality is working as expected and maintains consistency with the existing test style.

services/rfq/relayer/relconfig/getters.go (1)

751-782: LGTM: New GetMaxQuoteAmount function looks good

The new GetMaxQuoteAmount function is well-implemented and follows the existing patterns in the file. It correctly handles various edge cases and scales the quote amount by the token decimals.

A few observations:

  1. The function properly checks for the existence of chain and token configurations.
  2. It correctly handles cases where the max quote amount is not set or is invalid.
  3. The scaling of the quote amount by token decimals is implemented correctly.
services/rfq/relayer/quoter/quoter.go (2)

Line range hint 716-723: LGTM: Clipping quoteAmount by Destination Balance

The code correctly ensures that quoteAmount does not exceed the available DestBalance, preventing over-allocation of funds beyond what is currently available on the destination chain.


725-733: LGTM: Adding maxQuoteAmount Clipping for Maximum Limit Enforcement

The new logic appropriately limits quoteAmount by maxQuoteAmount as defined in the configuration. This enhances the robustness of the quote generation by enforcing an upper bound on the quote amounts.

Comment on lines 127 to 128
// MaxQuoteAmount is the maximum amount to quote for this token in human-readable units.
MaxQuoteAmount string `yaml:"max_quote_amount"`
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

Action Required: MaxQuoteAmount is not properly utilized in GetMaxQuoteAmount.

The GetMaxQuoteAmount function currently returns defaultMaxQuoteAmount without using the configured MaxQuoteAmount. This means the maximum quote limit is not being enforced as intended.

Files to address:

  • services/rfq/relayer/relconfig/getters.go
  • services/rfq/relayer/quoter/quoter.go
🔗 Analysis chain

LGTM! Consider updating related quote generation logic.

The addition of the MaxQuoteAmount field to the TokenConfig struct is a good improvement. It complements the existing MinQuoteAmount field and provides more control over the quoting process. The field is consistently named and tagged for YAML serialization.

To ensure this change is properly integrated, please verify that the quote generation logic is updated to respect this new maximum limit. You can use the following script to check for potential usage:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for files that might need updating to use MaxQuoteAmount
echo "Files potentially needing updates for MaxQuoteAmount:"
rg -l "MinQuoteAmount" --type go | grep -v "config.go"

Length of output: 292


Script:

#!/bin/bash
# Search for 'MaxQuoteAmount' usage in identified files
rg 'MaxQuoteAmount' services/rfq/relayer/relconfig/getters.go services/rfq/relayer/quoter/quoter_test.go services/rfq/relayer/quoter/quoter.go

Length of output: 1329


Script:

#!/bin/bash
# Extract the implementation of GetMaxQuoteAmount from getters.go
ast-grep --lang go --pattern 'func (c Config) GetMaxQuoteAmount($_, $_) $_ { $$$ }' services/rfq/relayer/relconfig/getters.go

Length of output: 2397

Comment on lines 751 to 782
// GetMaxQuoteAmount returns the quote amount for the given chain and address.
// Note that this getter returns the value in native token decimals.
func (c Config) GetMaxQuoteAmount(chainID int, addr common.Address) *big.Int {
chainCfg, ok := c.Chains[chainID]
if !ok {
return defaultMaxQuoteAmount
}

var tokenCfg *TokenConfig
for _, cfg := range chainCfg.Tokens {
if common.HexToAddress(cfg.Address).Hex() == addr.Hex() {
cfgCopy := cfg
tokenCfg = &cfgCopy
break
}
}
if tokenCfg == nil {
return defaultMaxQuoteAmount
}
quoteAmountFlt, ok := new(big.Float).SetString(tokenCfg.MaxQuoteAmount)
if !ok {
return defaultMaxQuoteAmount
}
if quoteAmountFlt.Cmp(big.NewFloat(0)) <= 0 {
return defaultMaxQuoteAmount
}

// Scale the minQuoteAmount by the token decimals.
denomDecimalsFactor := new(big.Int).Exp(big.NewInt(10), big.NewInt(int64(tokenCfg.Decimals)), nil)
quoteAmountScaled, _ := new(big.Float).Mul(quoteAmountFlt, new(big.Float).SetInt(denomDecimalsFactor)).Int(nil)
return quoteAmountScaled
}
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

Impact of the new GetMaxQuoteAmount function confirmed

The addition of the GetMaxQuoteAmount function affects multiple parts of the codebase where maximum quote amounts are utilized. Ensure the following areas are reviewed and updated accordingly:

  • services/rfq/api/rest/handler.go
  • services/rfq/api/db/api_db_test.go
  • services/rfq/relayer/relconfig/getters.go
  • services/rfq/relayer/relconfig/config.go
  • services/rfq/relayer/quoter/quoter.go
  • services/rfq/relayer/quoter/quoter_test.go
🔗 Analysis chain

Verify the impact of the new GetMaxQuoteAmount function

The addition of this new function might require updates in other parts of the codebase where maximum quote amounts are used.

Let's check for any existing usages of maximum quote amounts that might need to be updated:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for potential usages of maximum quote amounts
rg -i "max.*quote.*amount" --type go

Length of output: 2676

Copy link

cloudflare-workers-and-pages bot commented Oct 9, 2024

Deploying sanguine-fe with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2639cd2
Status: ✅  Deploy successful!
Preview URL: https://3769f41f.sanguine-fe.pages.dev
Branch Preview URL: https://feat-max-quote-amount.sanguine-fe.pages.dev

View logs

@dwasse dwasse changed the title feat(rfq-relayer): add MaxQuoteAmount feat(rfq-relayer): add MaxRelayAmount Oct 10, 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.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (5)
services/rfq/relayer/quoter/quoter_test.go (3)

139-145: LGTM! Consider adding an additional test case.

The new test case for max relay amount is well-implemented and checks the correct behavior. It's a good addition to ensure the quoter respects the max relay amount constraint.

Consider adding another test case where the max relay amount is greater than the balance to ensure the quoter behaves correctly in both scenarios.


Line range hint 191-203: Effective update of setQuoteParams function.

The setQuoteParams function has been successfully updated to use the new quoteParams struct. It correctly sets the configuration for both destination and origin tokens, improving the test setup process.

Consider adding a comment explaining the purpose of this function, as it plays a crucial role in setting up the test environment.


221-291: Comprehensive test cases for GetOriginAmount.

The new test cases for GetOriginAmount are well-implemented and cover a wide range of scenarios. They effectively test different combinations of quote percentages, offsets, and constraints, ensuring the function behaves correctly under various conditions.

To improve test readability, consider adding brief comments before each test case explaining the specific scenario being tested. This would make it easier for other developers to understand the purpose of each test at a glance.

services/rfq/relayer/relconfig/getters.go (2)

751-782: Minor improvements for GetMaxRelayAmount function

The implementation of GetMaxRelayAmount is well-structured and consistent with other getter functions. However, there are a few minor issues that could be addressed:

  1. The comment on line 752 should be updated to mention "relay amount" instead of "quote amount".
  2. The variable name quoteAmountFlt on line 770 could be more specific, e.g., maxRelayAmountFlt.
  3. The comment on line 778 should be updated to mention "maxRelayAmount" instead of "minQuoteAmount".

Consider applying these changes to improve clarity and consistency:

-// Note that this getter returns the value in native token decimals.
+// Note that this getter returns the max relay amount in native token decimals.
 func (c Config) GetMaxRelayAmount(chainID int, addr common.Address) *big.Int {
   // ...
-  quoteAmountFlt, ok := new(big.Float).SetString(tokenCfg.MaxRelayAmount)
+  maxRelayAmountFlt, ok := new(big.Float).SetString(tokenCfg.MaxRelayAmount)
   if !ok {
     return defaultMaxRelayAmount
   }
-  if quoteAmountFlt.Cmp(big.NewFloat(0)) <= 0 {
+  if maxRelayAmountFlt.Cmp(big.NewFloat(0)) <= 0 {
     return defaultMaxRelayAmount
   }

-  // Scale the minQuoteAmount by the token decimals.
+  // Scale the maxRelayAmount by the token decimals.
   denomDecimalsFactor := new(big.Int).Exp(big.NewInt(10), big.NewInt(int64(tokenCfg.Decimals)), nil)
-  quoteAmountScaled, _ := new(big.Float).Mul(quoteAmountFlt, new(big.Float).SetInt(denomDecimalsFactor)).Int(nil)
+  maxRelayAmountScaled, _ := new(big.Float).Mul(maxRelayAmountFlt, new(big.Float).SetInt(denomDecimalsFactor)).Int(nil)
-  return quoteAmountScaled
+  return maxRelayAmountScaled
 }

These changes will improve the readability and maintainability of the code.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 754-755: services/rfq/relayer/relconfig/getters.go#L754-L755
Added lines #L754 - L755 were not covered by tests


[warning] 757-757: services/rfq/relayer/relconfig/getters.go#L757
Added line #L757 was not covered by tests


[warning] 759-764: services/rfq/relayer/relconfig/getters.go#L759-L764
Added lines #L759 - L764 were not covered by tests


[warning] 767-767: services/rfq/relayer/relconfig/getters.go#L767
Added line #L767 was not covered by tests


[warning] 769-769: services/rfq/relayer/relconfig/getters.go#L769
Added line #L769 was not covered by tests


[warning] 771-771: services/rfq/relayer/relconfig/getters.go#L771
Added line #L771 was not covered by tests


[warning] 773-774: services/rfq/relayer/relconfig/getters.go#L773-L774
Added lines #L773 - L774 were not covered by tests


[warning] 776-776: services/rfq/relayer/relconfig/getters.go#L776
Added line #L776 was not covered by tests


[warning] 779-781: services/rfq/relayer/relconfig/getters.go#L779-L781
Added lines #L779 - L781 were not covered by tests


751-782: Improve test coverage for GetMaxRelayAmount function

The static analysis tool indicates that several lines in the GetMaxRelayAmount function are not covered by tests. To ensure the reliability and correctness of this new function, it's important to add comprehensive unit tests that cover various scenarios, including:

  1. Retrieving a valid max relay amount
  2. Handling cases where the chain configuration is not found
  3. Handling cases where the token configuration is not found
  4. Dealing with invalid MaxRelayAmount values
  5. Verifying the correct scaling of the amount based on token decimals

Would you like assistance in generating unit tests for the GetMaxRelayAmount function to improve code coverage?

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 754-755: services/rfq/relayer/relconfig/getters.go#L754-L755
Added lines #L754 - L755 were not covered by tests


[warning] 757-757: services/rfq/relayer/relconfig/getters.go#L757
Added line #L757 was not covered by tests


[warning] 759-764: services/rfq/relayer/relconfig/getters.go#L759-L764
Added lines #L759 - L764 were not covered by tests


[warning] 767-767: services/rfq/relayer/relconfig/getters.go#L767
Added line #L767 was not covered by tests


[warning] 769-769: services/rfq/relayer/relconfig/getters.go#L769
Added line #L769 was not covered by tests


[warning] 771-771: services/rfq/relayer/relconfig/getters.go#L771
Added line #L771 was not covered by tests


[warning] 773-774: services/rfq/relayer/relconfig/getters.go#L773-L774
Added lines #L773 - L774 were not covered by tests


[warning] 776-776: services/rfq/relayer/relconfig/getters.go#L776
Added line #L776 was not covered by tests


[warning] 779-781: services/rfq/relayer/relconfig/getters.go#L779-L781
Added lines #L779 - L781 were not covered by tests

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 1f0b71c and 2639cd2.

📒 Files selected for processing (4)
  • services/rfq/relayer/quoter/quoter.go (3 hunks)
  • services/rfq/relayer/quoter/quoter_test.go (3 hunks)
  • services/rfq/relayer/relconfig/config.go (1 hunks)
  • services/rfq/relayer/relconfig/getters.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • services/rfq/relayer/relconfig/config.go
🧰 Additional context used
🪛 GitHub Check: codecov/patch
services/rfq/relayer/relconfig/getters.go

[warning] 754-755: services/rfq/relayer/relconfig/getters.go#L754-L755
Added lines #L754 - L755 were not covered by tests


[warning] 757-757: services/rfq/relayer/relconfig/getters.go#L757
Added line #L757 was not covered by tests


[warning] 759-764: services/rfq/relayer/relconfig/getters.go#L759-L764
Added lines #L759 - L764 were not covered by tests


[warning] 767-767: services/rfq/relayer/relconfig/getters.go#L767
Added line #L767 was not covered by tests


[warning] 769-769: services/rfq/relayer/relconfig/getters.go#L769
Added line #L769 was not covered by tests


[warning] 771-771: services/rfq/relayer/relconfig/getters.go#L771
Added line #L771 was not covered by tests


[warning] 773-774: services/rfq/relayer/relconfig/getters.go#L773-L774
Added lines #L773 - L774 were not covered by tests


[warning] 776-776: services/rfq/relayer/relconfig/getters.go#L776
Added line #L776 was not covered by tests


[warning] 779-781: services/rfq/relayer/relconfig/getters.go#L779-L781
Added lines #L779 - L781 were not covered by tests

🔇 Additional comments (5)
services/rfq/relayer/quoter/quoter_test.go (2)

183-189: Well-structured quoteParams struct.

The introduction of the quoteParams struct is a good design choice. It encapsulates related quote parameters, improving code organization and readability. This will make it easier to manage and update quote parameters in the test cases.


294-299: Good addition of MaxBalance test case.

The new test case effectively checks the behavior of GetOriginAmount when a MaxBalance constraint is applied. It ensures that the quoter correctly respects the MaxBalance limit, even when it's less than the actual balance. This is an important edge case to test.

services/rfq/relayer/relconfig/getters.go (1)

749-750: LGTM: Appropriate default value for defaultMaxRelayAmount

The declaration of defaultMaxRelayAmount as nil is consistent with other similar default variables in the file. This is an appropriate default value, as it effectively represents the absence of a maximum relay amount limit.

services/rfq/relayer/quoter/quoter.go (2)

Line range hint 725-732: Properly clipping quoteAmount by destination balance

Clipping the quoteAmount by input.DestBalance ensures that the quoted amount does not exceed the available destination balance. This is a correct implementation that prevents overcommitting funds and potential failures during transaction execution.


734-743: Adding maximum quote amount constraint enhances control

Introducing the clipping of quoteAmount by maxQuoteAmount adds a crucial constraint that prevents quoting amounts higher than a predefined maximum. This enhancement improves the robustness and compliance of the quote generation logic by enforcing configuration-defined limits.

Comment on lines +211 to +217
maxRelayAmount := m.config.GetMaxRelayAmount(int(quote.Transaction.OriginChainId), quote.Transaction.OriginToken)
if maxRelayAmount != nil {
if quote.Transaction.OriginAmount.Cmp(maxRelayAmount) > 0 {
span.AddEvent("origin amount is greater than max relay amount")
return false, nil
}
}
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

Potential integer overflow when casting OriginChainId to int

In line 211, you are casting quote.Transaction.OriginChainId to an int using int(quote.Transaction.OriginChainId). If OriginChainId is a uint32, casting it directly to int could lead to incorrect negative values on systems where int is a 32-bit signed integer and the OriginChainId exceeds 2,147,483,647. This may cause unexpected behavior when fetching the maxRelayAmount.

Consider using uint32 consistently or adjusting the GetMaxRelayAmount method to accept the appropriate unsigned integer type to prevent potential overflows.

@dwasse dwasse merged commit 93f9886 into master Oct 14, 2024
42 checks passed
@dwasse dwasse deleted the feat/max-quote-amount branch October 14, 2024 15:18
golangisfun123 added a commit that referenced this pull request Oct 16, 2024
…nds (#3287)

* feat(rfq-relayer): add MaxRelayAmount (#3259)

* Feat: add quoteParams helper for test

* Feat: add MaxQuoteAmount to relconfig

* Feat: use MaxQuoteAmount

* Feat: handle MaxQuoteAmount in quoter test

* Replace: MaxQuoteAmount -> MaxRelayAmount

* Feat: shouldProcess() returns false if max relay amount exceeded

* Feat: add test for MaxRelayAmount

* add request field for refunds

* adding to events typing

---------

Co-authored-by: dwasse <wassermandaniel8@gmail.com>
Co-authored-by: defi-moses <jakedinero@protonmail.com>
Defi-Moses added a commit that referenced this pull request Oct 29, 2024
* explorer UI updated

* rfq-indexer update

* explorer backend update

* [goreleaser] trigger explorer version bump

* rfq indexer with the right contracts

* [goreleaser] adding catch

* response error fixes and wld decimals

* adding address

* feat(rfq-indexer): add `request` column to `BridgeRequested` for refunds (#3287)

* feat(rfq-relayer): add MaxRelayAmount (#3259)

* Feat: add quoteParams helper for test

* Feat: add MaxQuoteAmount to relconfig

* Feat: use MaxQuoteAmount

* Feat: handle MaxQuoteAmount in quoter test

* Replace: MaxQuoteAmount -> MaxRelayAmount

* Feat: shouldProcess() returns false if max relay amount exceeded

* Feat: add test for MaxRelayAmount

* add request field for refunds

* adding to events typing

---------

Co-authored-by: dwasse <wassermandaniel8@gmail.com>
Co-authored-by: defi-moses <jakedinero@protonmail.com>

* fix api docs

* linting fixes

* fixing irrelavent files

---------

Co-authored-by: vro <168573323+golangisfun123@users.noreply.github.com>
Co-authored-by: dwasse <wassermandaniel8@gmail.com>
golangisfun123 added a commit that referenced this pull request Nov 7, 2024
* use gql api

* remove old test

* [goreleaser]

* fix lint[goreleaser]

* [goreleaser]

* fix lint [goreleaser]

* explorer UI updated

* rfq-indexer update

* explorer backend update

* [goreleaser] trigger explorer version bump

* rfq indexer with the right contracts

* fix logic

* [goreleaser]

* fix error msg

* [goreleaser] adding catch

* remove function

* response error fixes and wld decimals

* origin tx hash

* adding address

* feat(rfq-indexer): add `request` column to `BridgeRequested` for refunds (#3287)

* feat(rfq-relayer): add MaxRelayAmount (#3259)

* Feat: add quoteParams helper for test

* Feat: add MaxQuoteAmount to relconfig

* Feat: use MaxQuoteAmount

* Feat: handle MaxQuoteAmount in quoter test

* Replace: MaxQuoteAmount -> MaxRelayAmount

* Feat: shouldProcess() returns false if max relay amount exceeded

* Feat: add test for MaxRelayAmount

* add request field for refunds

* adding to events typing

---------

Co-authored-by: dwasse <wassermandaniel8@gmail.com>
Co-authored-by: defi-moses <jakedinero@protonmail.com>

* idr

* merge

* refund from rfq indexer

* add request field

* yarn lcok

* Revert "yarn lcok"

This reverts commit 54b1391.

* Revert "add request field"

This reverts commit 111d862.

* add new fiedl

* fix retry logic

* [goreleaser]

* max attempt time [goreleaser]

* fix route [goreleaser]

* add linea explorer

* remove slash

* add worldchain explorer

* fix  lint

* remove 0x [goreleaser]

* revert quoter test

* fix lint

---------

Co-authored-by: defi-moses <jakedinero@protonmail.com>
Co-authored-by: dwasse <wassermandaniel8@gmail.com>
Defi-Moses added a commit that referenced this pull request Nov 18, 2024
* explorer UI updated

* rfq-indexer update

* explorer backend update

* [goreleaser] trigger explorer version bump

* rfq indexer with the right contracts

* [goreleaser] adding catch

* response error fixes and wld decimals

* adding address

* feat(rfq-indexer): add `request` column to `BridgeRequested` for refunds (#3287)

* feat(rfq-relayer): add MaxRelayAmount (#3259)

* Feat: add quoteParams helper for test

* Feat: add MaxQuoteAmount to relconfig

* Feat: use MaxQuoteAmount

* Feat: handle MaxQuoteAmount in quoter test

* Replace: MaxQuoteAmount -> MaxRelayAmount

* Feat: shouldProcess() returns false if max relay amount exceeded

* Feat: add test for MaxRelayAmount

* add request field for refunds

* adding to events typing

---------

Co-authored-by: dwasse <wassermandaniel8@gmail.com>
Co-authored-by: defi-moses <jakedinero@protonmail.com>

* fix api docs

* linting fixes

* fixing irrelavent files

* wc fixes and tests solved

* fix linting errors

* [goreleaser]

* refining tests

* adding bridge Module to graphql

* Adding to all bridge watcher queries

* adding worldchain to the chart

* adding other rfq events to the rfq events db

* fixing wld coingecko ticker [goreleaser]

* [goreleaser] coingecko update and new topic parsing

* small lint fix

* [goreleaser] linting fixes

* fix(rfq-indexer): add missing fields (#3309)

* add missing fields

---------

Co-authored-by: abtestingalpha <abtestingalpha@gmail.com>

---------

Co-authored-by: vro <168573323+golangisfun123@users.noreply.github.com>
Co-authored-by: dwasse <wassermandaniel8@gmail.com>
Co-authored-by: abtestingalpha <abtestingalpha@gmail.com>
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 needs-go-generate-services/rfq size/s
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant