FE Release 2024-08-27 #3071
FE Release 2024-08-27 #3071
Conversation
- @synapsecns/[email protected]
* add test for multiple incorrect auth submissions in a row * - Separate cache check/population from role verification * rm duplicate import
[ci skip]
* Feat: add listener name field * Feat: store listenerName in store struct * Fix: build * [goreleaser] * Feat: make listener name an option * [goreleaser] * Fix: listener name indexing * [goreleaser] * Fix: remove listener name from db constructor * Feat: set listener name on relayer and guard * [goreleaser] * Fix: use where statements instead of attempted composite index * [goreleaser] * Fix: introduce new unique index * [goreleaser]
* Feat: add balanceMtx * [goreleaser] * Feat: check lock in handleNotEnoughInventory * [goreleaser] * Feat: add commitPendingBalance * [goreleaser]
* initial impl * Better * logic fix * change param * only unique addresses * add relayer balances * [goreleaser] * lint * [goreleaser] * fix config lint [goreleaser] * lint [goreleaser]
- @synapsecns/[email protected]
* add usdc to balance fetching * [goreleaser]
* Fix: listener query * [goreleaser] * Feat: add multi chain case to db test
- @synapsecns/[email protected]
Co-authored-by: Trajan0x <[email protected]>
- @synapsecns/[email protected]
* WIP: parallel test * Feat: working serial * Feat: working concurrent requests * Cleanup: revert unnecessary changes * Cleanup: test name * Cleanup: lint * Cleanup: lint
* Revert "revert(relayer): potential deadlock (#3036)" to reinstate #2994 Co-authored-by: Daniel Wasserman <[email protected]>
* Add METIS token constant * Generate mapping * Use zeroAddress constant to signify gas * fix: reconcile WMETIS to Metis in bridge map
- @synapsecns/[email protected]
* first pass * extra comments * fixing internal link * fix build
- @synapsecns/[email protected]
- @synapsecns/[email protected]
[ci skip]
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
services/rfq/relayer/inventory/manager_test.go (2)
Line range hint
16-56: Add assertions and remove the unused variable.The function is correctly setting up the test environment, but it can be improved:
- Add assertions to test the behavior of the inventory manager after it is created.
- Remove the assignment to the blank identifier
_ = imas the inventory manager is not being used.
Line range hint
58-125: Use the chain IDs from the test suite.The function is correctly testing the
HasSufficientGasmethod with different scenarios. However, it can be improved by using the chain IDs from the test suite instead of hardcoding them.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (14)
- .golangci.yml (2 hunks)
- services/rfq/e2e/rfq_test.go (4 hunks)
- services/rfq/relayer/inventory/circle.go (1 hunks)
- services/rfq/relayer/inventory/export_test.go (1 hunks)
- services/rfq/relayer/inventory/manager.go (10 hunks)
- services/rfq/relayer/inventory/manager_test.go (1 hunks)
- services/rfq/relayer/inventory/mocks/manager.go (1 hunks)
- services/rfq/relayer/inventory/rebalance.go (2 hunks)
- services/rfq/relayer/inventory/rebalance_test.go (6 hunks)
- services/rfq/relayer/inventory/scroll.go (1 hunks)
- services/rfq/relayer/inventory/synapse.go (1 hunks)
- services/rfq/relayer/reldb/base/model.go (3 hunks)
- services/rfq/relayer/reldb/db.go (1 hunks)
- services/rfq/relayer/service/handlers.go (6 hunks)
Files skipped from review as they are similar to previous changes (2)
- .golangci.yml
- services/rfq/e2e/rfq_test.go
Additional comments not posted (33)
services/rfq/relayer/inventory/export_test.go (1)
11-12: LGTM! Verify the usage of the internalgetRebalancesfunction.The code changes are approved. The new
GetRebalancesfunction follows the best practices for context usage in Go and consolidates the functionality of the removedGetRebalanceandGetRebalanceMetadatasfunctions.Ensure that the internal
getRebalancesfunction is implemented correctly and handles the context parameter appropriately.Run the following script to verify the implementation of the
getRebalancesfunction:Verification successful
The internal
getRebalancesfunction is implemented correctly.The
getRebalancesfunction inservices/rfq/relayer/inventory/rebalance.gois correctly implemented, handling the context parameter and following best practices for error handling and data processing. The function's logic aligns with the expected behavior of consolidating rebalance operations. No issues were found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of the `getRebalances` function. # Test: Search for the function implementation. Expect: The function signature matches the one in the `GetRebalances` function. ast-grep --lang go --pattern $'func getRebalances(ctx context.Context, cfg relconfig.Config, inv map[int]map[common.Address]*TokenMetadata) (rebalances map[string]*RebalanceData, err error) { $$$ }'Length of output: 1920
services/rfq/relayer/inventory/mocks/manager.go (1)
Line range hint
139-149: Verify the impact of the function signature change on the codebase.The
Rebalancefunction signature has been modified to remove thechainIDandtokenparameters. This change indicates a shift in how the function is intended to be used, likely reflecting a change in the underlying logic or requirements for the mock behavior.Run the following script to verify the function usage:
Verification successful
Function Signature Change Verified
The
Rebalancefunction's signature change has been successfully integrated into the codebase. All instances of the function call match the new signature, which only takesctxas a parameter. No issues were found with the integration of this change.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `Rebalance` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'Rebalance'Length of output: 129253
services/rfq/relayer/inventory/synapse.go (1)
166-171: LGTM! The additional fields enhance the rebalance data model.The inclusion of
OriginTokenAddrandTokenNamefields in themodelstructure improves the tracking and management of token transactions by storing more detailed information about the origin. This change enhances the functionality related to rebalance operations while maintaining the overall logic of the method.services/rfq/relayer/reldb/db.go (1)
188-188: LGTM!The addition of the
TokenNamefield to theRebalancestruct is a valid enhancement. It allows storing the name of the token involved in the rebalance action, providing more detailed information. The field name is clear, follows naming conventions, and thestringtype is appropriate for token names.services/rfq/relayer/reldb/base/model.go (3)
110-110: LGTM!The addition of the
TokenNamefield to theRebalancestruct is approved. This field enhances the data structure by allowing it to store more detailed information about the rebalance operation.
160-160: LGTM!The update to the
FromRebalancefunction to include theTokenNamefield when converting areldb.Rebalanceobject to aRebalanceobject is approved. This ensures that the new field is populated correctly during the conversion process.
246-246: LGTM!The update to the
ToRebalancemethod to include theTokenNamefield when converting aRebalanceobject to areldb.Rebalanceobject is approved. This ensures that the new field is included when converting back to thereldb.Rebalanceobject.services/rfq/relayer/inventory/rebalance.go (5)
31-51: LGTM!The code changes are approved. The function is well-structured and modularized, delegating responsibilities to separate functions, which improves readability and maintainability.
54-81: LGTM!The code changes are approved. The function is well-structured and follows the Single Responsibility Principle by focusing on getting the rebalance candidates. It delegates the responsibility of getting the best rebalance for each method to a separate function, which improves readability and maintainability.
86-123: LGTM!The code changes are approved. The function is well-structured and modularized, delegating responsibilities to separate functions, which improves readability and maintainability. The function is quite long and has a high cyclomatic complexity due to the nested loops and conditional statements. However, the complexity is justified as it is a core part of the rebalance logic.
Line range hint
178-272: LGTM!The code changes are approved. The function is well-documented with comments explaining the high-level steps. The function is quite long and has a high cyclomatic complexity due to the multiple conditional statements. However, the complexity is justified as it is a core part of the rebalance logic. The function could be further modularized by extracting some of the logic into separate functions, but it is not a blocker for this review.
292-300: LGTM!The code changes are approved. The function is well-structured and follows the Single Responsibility Principle by focusing on checking if a token supports a specific rebalance method. It has a clear and concise implementation.
services/rfq/relayer/inventory/circle.go (2)
245-250: LGTM!The code changes to include
OriginTokenAddrandTokenNamein therebalanceModelstruct are approved. The additional fields provide more context about the rebalance operation, which can be beneficial for tracking and auditing purposes.
Line range hint
1-244: No further comments.The code changes are limited to the
Executemethod, and the remaining code is unaffected. No further review comments are necessary.Also applies to: 251-500
services/rfq/relayer/inventory/rebalance_test.go (9)
Line range hint
17-56: LGTM!The subtest correctly asserts that the
GetRebalancesfunction returns an empty map of rebalances and no error when no rebalance methods are configured for the tokens.
Line range hint
59-101: LGTM!The subtest correctly asserts that the
GetRebalancesfunction returns a nil rebalance for the token and no error when incompatible rebalance methods are configured for the tokens.
Line range hint
103-155: LGTM!The subtest correctly asserts that the
GetRebalancesfunction returns the correct rebalance data for the token and no error when a single rebalance method is configured for the tokens.
157-203: LGTM!The subtest correctly asserts that the
GetRebalancesfunction returns a nil rebalance for the token and no error when a single rebalance method is configured for the tokens and the origin token balance is above the maintenance balance percentage.
Line range hint
205-257: LGTM!The subtest correctly asserts that the
GetRebalancesfunction returns the correct rebalance data for the token to bring the origin token balance up to the initial balance percentage and no error when a single rebalance method is configured for the tokens and the origin token balance is below the initial balance percentage.
259-311: LGTM!The subtest correctly asserts that the
GetRebalancesfunction returns the correct rebalance data for the token to bring the destination token balance up to the initial balance percentage and no error when a single rebalance method is configured for the tokens and the destination token balance is below the initial balance percentage.
313-370: LGTM!The subtest correctly asserts that the
GetRebalancesfunction returns the correct rebalance data for the token with the rebalance amount capped at the maximum rebalance amount and no error when a single rebalance method is configured for the tokens and the rebalance amount is above the maximum rebalance amount.
Line range hint
372-442: LGTM!The subtest correctly asserts that the
GetRebalancesfunction returns the correct rebalance data for the token using the first compatible rebalance method and no error when multiple rebalance methods are configured for the tokens.
444-557: LGTM!The subtest correctly asserts that the
GetRebalancesfunction returns the correct rebalance data for each token using the first compatible rebalance method and no error when multiple rebalance methods are configured for multiple tokens.services/rfq/relayer/service/handlers.go (5)
165-190: LGTM!The code changes are approved. The introduction of the
commitPendingBalancemethod and the corresponding updates to thehandleSeenmethod improve the balance checking and committing process before forwarding the request.
Line range hint
192-222: LGTM!The code changes are approved. The
commitPendingBalancemethod introduces proper locking and balance checking mechanisms to handle concurrent requests and ensure the integrity of the balance.
392-420: LGTM!The code changes are approved. The updates to the
handleRelayCompletedmethod improve the reliability of the prove process by ensuring that the relay transaction has been finalized. The handling of reorg scenarios is also a valuable addition.
441-476: LGTM!The code changes are approved. The
getRelayBlockNumbermethod is a useful addition that fetches the block number of the relay transaction reliably by checking for the presence of theFastBridgeBridgeRelayedevent in the transaction receipt.
Line range hint
588-610: LGTM!The code changes are approved. The updates to the
handleNotEnoughInventorymethod improve the handling of concurrent requests by acquiring a balance lock and updating the request status based on the sufficiency of the balance.services/rfq/relayer/inventory/manager.go (4)
51-52: IMPORTANT: Verify the impact of the breaking change to theRebalancemethod signature.The
Rebalancemethod signature has been modified and no longer acceptschainIDandtokenparameters. This is a breaking change.Please ensure that all implementations and usages of the
Managerinterface have been updated to match the new signature. Additionally, verify that the new signature is sufficient for the rebalancing functionality and doesn't lose any necessary flexibility.
473-496: LGTM!The changes to the
Rebalancemethod look good:
- The method signature has been updated to match the
Managerinterface.- The new implementation efficiently handles rebalances across all supported tokens and chains by iterating over the results of
getRebalancesand executing each rebalance usingtryExecuteRebalance.This is an improvement over the previous version that only handled a single token and chain.
Line range hint
498-540: LGTM!The new
tryExecuteRebalancemethod looks good:
- It provides a clean separation of concerns by handling the execution of a single rebalance.
- The check for pending rebalances helps avoid conflicts.
- Retrieving the rebalance manager based on the rebalance method allows for flexibility in supporting different rebalance implementations.
297-301: LGTM!The changes to the
ApproveAllTokensmethod look good:
- Using a named return improves error handling.
- Logging gateway addresses enhances observability.
These are minor improvements and don't affect the core functionality of the method.
Also applies to: 341-345, 354-354, 363-367, 376-376
services/rfq/relayer/inventory/scroll.go (1)
419-419: LGTM!The addition of the
TokenNamefield to therebalanceModelstruct is a good change. It captures additional context about the token name associated with the rebalance operation, which can improve the clarity and utility of the stored rebalance data.
abtestingalpha
temporarily deployed
to
github-pages
- @synapsecns/[email protected]
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
packages/synapse-interface/CHANGELOG.md (1)
33-33: Fix heading levels.The following lines have heading level increment issues:
- Line 33
- Line 68
- Line 87
Heading levels should only increment by one level at a time for proper document structure. Change the
###to##to fix this.Apply this diff to fix the heading levels:
-### Features +## FeaturesDo you want me to submit a PR with this change?
Also applies to: 68-68, 87-87
Tools
Markdownlint
33-33: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
packages/synapse-interface/assets/explorer/etherscan.svgis excluded by!**/*.svg
Files selected for processing (2)
- packages/synapse-interface/CHANGELOG.md (1 hunks)
- packages/synapse-interface/package.json (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/synapse-interface/package.json
Additional context used
Markdownlint
packages/synapse-interface/CHANGELOG.md
33-33: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
68-68: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
87-87: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time(MD001, heading-increment)
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- services/rfq/api/rest/auth.go (1 hunks)
- services/rfq/api/rest/server_test.go (2 hunks)
Additional comments not posted (2)
services/rfq/api/rest/auth.go (1)
57-71: LGTM! The changes enhance the handling of the recovery ID in Ethereum signatures.The code segment correctly identifies the byte position of the recovery ID within the signature and normalizes it to either 0 or 1, which is the expected format for the
crypto.SigToPubfunction. The normalization process accounts for common values of 27, 28, 0, and 1, improving the robustness of the signature verification process.Additionally, the error handling for unexpected recovery ID values, along with the appropriate JSON response and status code, enhances the overall functionality of the authentication process.
services/rfq/api/rest/server_test.go (1)
242-284: The existing review comments are still valid and applicable to the current code segment. Skipping generation of similar comments.
[ci skip]
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
assets/merge-example.pngis excluded by!**/*.png
Files selected for processing (1)
- CONTRIBUTING.md (3 hunks)
Additional context used
LanguageTool
CONTRIBUTING.md
[style] ~15-~15: Try using a synonym here to strengthen your wording.
Context: ...ng the review process, you might notice comments about code coverage changes. While we g...(COMMENT_REMARK)
[typographical] ~51-~51: When punctuating reported speech in US English, the comma is usually placed before the closing quotation mark.
Context: ...example.png) By using "Squash and Merge", we maintain a linear and clean history ...(QUOTATION_MARKS_COMMA)
[grammar] ~74-~74: You’ve repeated a verb. Did you mean to only write one of them?
Context: ...l. # Go ## Linting Linting for go is used using [golangci-lint](https://golangci-lint.r...(REPEATED_VERBS)
Additional comments not posted (5)
CONTRIBUTING.md (5)
5-24: Clarification and Enhancement of PR Submission GuidelinesThe new section "Submitting a PR" provides clear and structured guidelines for contributors, emphasizing the use of a default PR template and the inclusion of Notion IDs for better tracking. The addition of an example image is a practical visual aid that complements the textual instructions well.
- Visual Aids: The use of an example image (line 13) is a good practice as it helps in visual learning and provides a clear example of the expected format.
- Code Coverage: The explanation regarding code coverage fluctuations (lines 15-16) is reassuring and helps set realistic expectations for contributors.
- Automated Tools: The mention of CodeRabbit (lines 17-18) and its role in the review process helps contributors understand the context and limitations of automated reviews.
Overall, these changes make the PR submission process more transparent and accessible to new contributors.
Tools
LanguageTool
[style] ~15-~15: Try using a synonym here to strengthen your wording.
Context: ...ng the review process, you might notice comments about code coverage changes. While we g...(COMMENT_REMARK)
19-22: Handling Specific Labels:needs-go-generateandneeds-yarn-lockThe instructions for handling the
needs-go-generateandneeds-yarn-locklabels (lines 19-22) are clear and actionable. These additions are practical for contributors, guiding them on how to resolve common issues related to auto-generated code and dependencies.
- Actionable Steps: The steps provided are straightforward and easy to follow, which will likely reduce the number of questions or errors related to these common PR issues.
- Contextual Help: Including these instructions directly in the contributing guidelines ensures that contributors have immediate access to the necessary actions without needing to search through external documentation or forums.
This enhancement aids in maintaining a smooth workflow and consistency in handling project dependencies and generated code.
27-48: Detailed Review Process and Merge StrategyThe sections "Carefully Reviewing PRs" and "Using Squash and Merge" (lines 27-48) provide comprehensive guidelines on the review and merge processes. These sections are well-structured and cover crucial aspects of code quality, functionality, tests, documentation, and performance.
- Review Checklist: The checklist (lines 31-35) is thorough, covering all critical areas that need attention before merging a PR.
- Merge Strategy: The explanation and steps for using the "Squash and Merge" strategy (lines 41-47) are clear and justify the choice of this method to maintain a clean commit history.
These guidelines are essential for ensuring high standards are maintained in the project's codebase and that the history remains manageable and meaningful.
70-76: Guidelines for Adding a New Go ModuleThe updated guidelines for adding a new Go module (lines 70-76) are detailed and provide a clear step-by-step process. This section is particularly useful for contributors who are unfamiliar with the specifics of Go module management.
- Detailed Instructions: The steps include creating directories, setting up
go.mod, and managing dependencies, which are crucial for correctly setting up a new Go module.- Best Practices: Emphasizing the use of the
go.workfile and the importance of keeping entries in alphabetical order (line 76) promotes best practices and consistency across the project.This update enhances the clarity and usability of the guidelines, making it easier for contributors to correctly add new Go modules to the project.
Tools
LanguageTool
[grammar] ~74-~74: You’ve repeated a verb. Did you mean to only write one of them?
Context: ...l. # Go ## Linting Linting for go is used using [golangci-lint](https://golangci-lint.r...(REPEATED_VERBS)
342-342: Optimization of GitHub Actions for Shallow ClonesThe recommendation to use
fetch-depth: 2for shallow clones in GitHub Actions (line 342) is a subtle yet important optimization. It allows certain actions to determine the correct SHA, which can be crucial for CI processes that depend on accurate commit history.
- Technical Insight: This change shows an understanding of CI optimizations and the nuances of how GitHub Actions interacts with the repository's commit history.
- Practical Impact: While this is a small change, it can significantly impact the efficiency and accuracy of CI workflows.
This adjustment is a good example of continuous improvement in the project's CI/CD practices.
Can undo after Rookout/GoSDK#15 Co-authored-by: Trajan0x <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
core/metrics/rookout.go (1)
Line range hint
18-33: Enhance error handling in the initialization function.The
initfunction configures the Rookout SDK and handles errors by logging them. Consider enhancing error handling by adding retry mechanisms or escalating the error if it persists, to ensure the SDK is properly initialized.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- core/metrics/rookout.go (1 hunks)
Additional comments not posted (3)
core/metrics/rookout.go (3)
1-1: Verify the build constraint for Go versions.The build constraint
//go:build go1.16 && !go1.23ensures compatibility with specific Go versions. Please confirm that the exclusion of Go 1.23 is intentional and based on compatibility issues or deprecated features.
Line range hint
4-10: Check the necessity of imports.Ensure that all imported packages are utilized within the file. This helps in avoiding unnecessary dependencies and keeping the code clean.
Line range hint
14-16: Good practice in constant declaration.The declaration of
DefaultGitRepoas a constant, with the possibility of being overridden by an ldflag, is a good practice. It allows for flexibility in configuration without changing the codebase.
- @synapsecns/[email protected]
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- docs/bridge/docs/rfq/Relayer/Relayer.md (4 hunks)
Additional context used
LanguageTool
docs/bridge/docs/rfq/Relayer/Relayer.md
[style] ~7-~7: Consider a shorter alternative to avoid wordiness.
Context: ...- :::note Relayers must be whitelisted in order to fill bridgeRequests. ::: At a high le...(IN_ORDER_TO_PREMIUM)
[uncategorized] ~39-~39: Possible missing article found.
Context: ...e amount by computing the delta between current balance and the initial threshold on or...(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~49-~49: Possible wrong verb form detected. Did you mean “been” or “being”?
Context: ...The rebalance amount would be initially be targeted as 600 since this would take c...(BE_WITH_WRONG_VERB_FORM)
[grammar] ~54-~54: Probably a preposition is missing after ‘continue’.
Context: ...is below themaintenance_balance_pct, continue 2. Calculate the amount to rebalance by taking the d...(ATD_VERBS_TO_COLLOCATION)
[style] ~56-~56: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ebalance to themax_rebalance_amount. If the amount to rebalance is less than th...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~59-~59: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...lementation for certain native bridges (e.g Scroll) is also supported. It works sli...(E_G)
[uncategorized] ~59-~59: Possible missing comma found.
Context: ...l) is also supported. It works slightly differently as flows are only supported between Scr...(AI_HYDRA_LEO_MISSING_COMMA)
Markdownlint
docs/bridge/docs/rfq/Relayer/Relayer.md
136-136: null
Bare URL used(MD034, no-bare-urls)
222-222: null
Bare URL used(MD034, no-bare-urls)
| Relayers must be whitelisted in order to fill bridgeRequests. | ||
|
|
||
| ::: |
There was a problem hiding this comment.
Clarify the whitelisting requirement.
The note about relayers needing to be whitelisted to fill bridge requests is critical. Consider expanding this note to include how one can become whitelisted or where to find more information about the whitelisting process.
Tools
LanguageTool
[style] ~7-~7: Consider a shorter alternative to avoid wordiness.
Context: ...- :::note Relayers must be whitelisted in order to fill bridgeRequests. ::: At a high le...(IN_ORDER_TO_PREMIUM)
| @@ -104,7 +133,7 @@ | |||
|
|
|||
| screener_api_url: 'http://screener-url' # can be left blank | |||
| rfq_url: 'http://rfq-api' # url of the rfq api backend. | |||
| omnirpc_url: 'http://omnirpc' # url of the omnirpc instance | |||
| omnirpc_url: 'http://omnirpc' # url of the omnirpc instance, please reference the Omnirpc section under Services for proper configuration | |||
There was a problem hiding this comment.
Address bare URL issues in the configuration section.
The use of bare URLs in the configuration section should be addressed by providing a more descriptive hyperlink text. This not only enhances readability but also improves accessibility.
- omnirpc_url: 'http://omnirpc' # url of the omnirpc instance, please reference the Omnirpc section under Services for proper configuration
+ omnirpc_url: '[Omnirpc Service](http://omnirpc)' # url of the omnirpc instance, please reference the Omnirpc section under Services for proper configuration
- base_omnirpc_url: "http://omnirpc" # Make sure this is configured properly
+ base_omnirpc_url: "[Base Omnirpc URL](http://omnirpc)" # Make sure this is configured properlyAlso applies to: 222-222
Tools
Markdownlint
136-136: null
Bare URL used(MD034, no-bare-urls)
| The implementation for certain native bridges (e.g Scroll) is also supported. It works slightly differently as flows are only supported between Scroll and Mainnet. At a high level, the rebalancer checks inventory on Scroll versus other chains, and if imbalanced, initiates a bridge to mainnet, allowing the CCTP relayer to rebalance funds where needed. | ||
|
|
There was a problem hiding this comment.
Clarify implementation details for native bridges.
The section on native bridges, such as Scroll, mentions different operational flows. It would be beneficial to provide more specific examples or a deeper explanation of how these flows differ from the standard operations. Also, correct the abbreviation "e.g." to "e.g.," and consider adding a comma after "differently" for better readability.
- The implementation for certain native bridges (e.g Scroll) is also supported. It works slightly differently as flows are only supported between Scroll and Mainnet.
+ The implementation for certain native bridges (e.g., Scroll) is also supported. It works slightly differently, as flows are only supported between Scroll and Mainnet.Committable suggestion was skipped due to low confidence.
Tools
LanguageTool
[uncategorized] ~59-~59: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...lementation for certain native bridges (e.g Scroll) is also supported. It works sli...(E_G)
[uncategorized] ~59-~59: Possible missing comma found.
Context: ...l) is also supported. It works slightly differently as flows are only supported between Scr...(AI_HYDRA_LEO_MISSING_COMMA)
| Automated rebalancing is configurable by token and currently supports CCTP routes as well as native bridging in the case of Scroll. Rebalancing evaluation follows this logic on every `rebalance_interval`: | ||
|
|
||
| - For every supported rebalance method, compute the 'total balance' of each token as the sum of all balances across chains that support the given rebalance method. | ||
| - For every supported token, compute the 'maintenance threshold' as the product of the `maintenance_balance_pct` and the 'total balance'. A token is eligible for rebalancing if the current balance is below the 'maintenance threshold'. | ||
|
|
||
| To limit the amount of inventory that is inflight, only one rebalance can be pending at a time for each token. As such, we select the 'best' rebalance candidate as the rebalance with the largest delta between origin balance and destination balance. | ||
|
|
||
| The final step in evaluating a rebalance is determining the rebalance amount: | ||
|
|
||
| - Arrive at an initial rebalance amount by computing the delta between current balance and the initial threshold on origin. | ||
| - Check if this rebalance amount is too much, i.e. it would take the destination balance above its initial threshold. If so, clip the rebalance amount to target the destination initial threshold. | ||
| - Filter the rebalance amount by the configured min and max. | ||
|
|
||
| An example of this process is given below: | ||
|
|
||
| We are dealing with chains 1, 2, and 3. Chains 1 and 2 support USDC with CCTP, and chain 3 supports USDC but does not support CCTP. Each chain has a `maintenance_pct` of 20%, while chains 1 and 2 have `initial_pct` of 40% and chain 3 has 20%. Assume chain 1 has a balance of 100 USDC, chain 2 has 900 USDC, and chain 3 has 2000 USDC. | ||
|
|
||
| The system would trigger a rebalance from chain 2 to chain 1, since the total balance of CCTP-supported chains is 1000 USDC, and chain 1 is below the 20% maintenance threshold. Chain 3 is not considered for rebalance since it does not support CCTP. | ||
|
|
||
| The rebalance amount would be initially be targeted as 600 since this would take chain 2 to its 40% initial threshold. However, since chain 1 only needs 300 to reach its initial 40% threshold, the rebalance amount is clipped at 300. | ||
|
|
||
| So, the final result is a rebalance of 300 USDC from chain 2 to chain 1, leading to chain 1 having 400 USDC, chain 2 having 600 USDC, and chain 3 having 2000 USDC. | ||
|
|
There was a problem hiding this comment.
Detailed review of the rebalancing process.
The expanded explanation of the rebalancing process is clear and detailed, especially with the inclusion of an illustrative example. This should greatly aid users in understanding how rebalancing works across multiple chains. However, consider addressing the following:
- Line 49 has a grammatical error with the phrase "would be initially be targeted." It should be corrected to "would initially be targeted."
- Ensure that all technical terms used in the example are defined somewhere in the document or linked to a glossary for clarity.
- The rebalance amount would be initially be targeted as 600 since this would take chain 2 to its 40% initial threshold.
+ The rebalance amount would initially be targeted as 600 since this would take chain 2 to its 40% initial threshold.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.
| Automated rebalancing is configurable by token and currently supports CCTP routes as well as native bridging in the case of Scroll. Rebalancing evaluation follows this logic on every `rebalance_interval`: | |
| - For every supported rebalance method, compute the 'total balance' of each token as the sum of all balances across chains that support the given rebalance method. | |
| - For every supported token, compute the 'maintenance threshold' as the product of the `maintenance_balance_pct` and the 'total balance'. A token is eligible for rebalancing if the current balance is below the 'maintenance threshold'. | |
| To limit the amount of inventory that is inflight, only one rebalance can be pending at a time for each token. As such, we select the 'best' rebalance candidate as the rebalance with the largest delta between origin balance and destination balance. | |
| The final step in evaluating a rebalance is determining the rebalance amount: | |
| - Arrive at an initial rebalance amount by computing the delta between current balance and the initial threshold on origin. | |
| - Check if this rebalance amount is too much, i.e. it would take the destination balance above its initial threshold. If so, clip the rebalance amount to target the destination initial threshold. | |
| - Filter the rebalance amount by the configured min and max. | |
| An example of this process is given below: | |
| We are dealing with chains 1, 2, and 3. Chains 1 and 2 support USDC with CCTP, and chain 3 supports USDC but does not support CCTP. Each chain has a `maintenance_pct` of 20%, while chains 1 and 2 have `initial_pct` of 40% and chain 3 has 20%. Assume chain 1 has a balance of 100 USDC, chain 2 has 900 USDC, and chain 3 has 2000 USDC. | |
| The system would trigger a rebalance from chain 2 to chain 1, since the total balance of CCTP-supported chains is 1000 USDC, and chain 1 is below the 20% maintenance threshold. Chain 3 is not considered for rebalance since it does not support CCTP. | |
| The rebalance amount would be initially be targeted as 600 since this would take chain 2 to its 40% initial threshold. However, since chain 1 only needs 300 to reach its initial 40% threshold, the rebalance amount is clipped at 300. | |
| So, the final result is a rebalance of 300 USDC from chain 2 to chain 1, leading to chain 1 having 400 USDC, chain 2 having 600 USDC, and chain 3 having 2000 USDC. | |
| Automated rebalancing is configurable by token and currently supports CCTP routes as well as native bridging in the case of Scroll. Rebalancing evaluation follows this logic on every `rebalance_interval`: | |
| - For every supported rebalance method, compute the 'total balance' of each token as the sum of all balances across chains that support the given rebalance method. | |
| - For every supported token, compute the 'maintenance threshold' as the product of the `maintenance_balance_pct` and the 'total balance'. A token is eligible for rebalancing if the current balance is below the 'maintenance threshold'. | |
| To limit the amount of inventory that is inflight, only one rebalance can be pending at a time for each token. As such, we select the 'best' rebalance candidate as the rebalance with the largest delta between origin balance and destination balance. | |
| The final step in evaluating a rebalance is determining the rebalance amount: | |
| - Arrive at an initial rebalance amount by computing the delta between current balance and the initial threshold on origin. | |
| - Check if this rebalance amount is too much, i.e. it would take the destination balance above its initial threshold. If so, clip the rebalance amount to target the destination initial threshold. | |
| - Filter the rebalance amount by the configured min and max. | |
| An example of this process is given below: | |
| We are dealing with chains 1, 2, and 3. Chains 1 and 2 support USDC with CCTP, and chain 3 supports USDC but does not support CCTP. Each chain has a `maintenance_pct` of 20%, while chains 1 and 2 have `initial_pct` of 40% and chain 3 has 20%. Assume chain 1 has a balance of 100 USDC, chain 2 has 900 USDC, and chain 3 has 2000 USDC. | |
| The system would trigger a rebalance from chain 2 to chain 1, since the total balance of CCTP-supported chains is 1000 USDC, and chain 1 is below the 20% maintenance threshold. Chain 3 is not considered for rebalance since it does not support CCTP. | |
| The rebalance amount would initially be targeted as 600 since this would take chain 2 to its 40% initial threshold. However, since chain 1 only needs 300 to reach its initial 40% threshold, the rebalance amount is clipped at 300. | |
| So, the final result is a rebalance of 300 USDC from chain 2 to chain 1, leading to chain 1 having 400 USDC, chain 2 having 600 USDC, and chain 3 having 2000 USDC. |
Tools
LanguageTool
[uncategorized] ~39-~39: Possible missing article found.
Context: ...e amount by computing the delta between current balance and the initial threshold on or...(AI_HYDRA_LEO_MISSING_THE)
[uncategorized] ~49-~49: Possible wrong verb form detected. Did you mean “been” or “being”?
Context: ...The rebalance amount would be initially be targeted as 600 since this would take c...(BE_WITH_WRONG_VERB_FORM)
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- docs/bridge/CHANGELOG.md (1 hunks)
- docs/bridge/package.json (1 hunks)
Files skipped from review due to trivial changes (1)
- docs/bridge/CHANGELOG.md
Files skipped from review as they are similar to previous changes (1)
- docs/bridge/package.json
null
4282f2f: docs preview link
4282f2f: explorer-ui preview link
4282f2f: synapse-interface preview link
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
2dbf484: docs preview link
2dbf484: synapse-interface preview link
2dbf484: explorer-ui preview link
f3e91da: docs preview link
f3e91da: synapse-interface preview link
f3e91da: explorer-ui preview link
980c2e2: docs preview link
980c2e2: synapse-interface preview link
980c2e2: explorer-ui preview link
f42374e: docs preview link
f42374e: synapse-interface preview link
f42374e: explorer-ui preview link
e3766d3: docs preview link
e3766d3: synapse-interface preview link
e3766d3: explorer-ui preview link
bffffab: docs preview link
bffffab: explorer-ui preview link
bffffab: synapse-interface preview link
4877ae3: docs preview link
4877ae3: synapse-interface preview link
4877ae3: explorer-ui preview link
0818d01: docs preview link
0818d01: synapse-interface preview link
0818d01: explorer-ui preview link
a36f25f: docs preview link
a36f25f: synapse-interface preview link
a36f25f: explorer-ui preview link
26d884f: docs preview link
26d884f: synapse-interface preview link
26d884f: explorer-ui preview link
4bfcb76: docs preview link
4bfcb76: synapse-interface preview link
4bfcb76: explorer-ui preview link
19761d4: docs preview link
19761d4: synapse-interface preview link
19761d4: explorer-ui preview link
2214e74: docs preview link
2214e74: synapse-interface preview link
2214e74: explorer-ui preview link
a65ab50: docs preview link
a65ab50: synapse-interface preview link
a65ab50: explorer-ui preview link
41e6a04: docs preview link
41e6a04: synapse-interface preview link
41e6a04: explorer-ui preview link
6864f8a: docs preview link
6864f8a: synapse-interface preview link
6864f8a: explorer-ui preview link
db54e7b: docs preview link
db54e7b: synapse-interface preview link
db54e7b: explorer-ui preview link
613e2b6: docs preview link
613e2b6: explorer-ui preview link
613e2b6: synapse-interface preview link
c451e18: docs preview link
c451e18: synapse-interface preview link
c451e18: explorer-ui preview link
c365f96: docs preview link
c365f96: synapse-interface preview link
c365f96: explorer-ui preview link