FE release 2025-02-27 #3539
FE release 2025-02-27 #3539
Conversation
Adding Audits to Github
* fix: correct fee calculation in the tests * fix: apply fixed fee to the dest amount * chore: add logging for testing [REVERT LATER] * Revert "chore: add logging for testing [REVERT LATER]" This reverts commit 1fd4064.
- @synapsecns/[email protected] - @synapsecns/[email protected] - @synapsecns/[email protected] - @synapsecns/[email protected]
fixing rest-api constants
- @synapsecns/[email protected]
ChiTimesChi
requested review from
abtestingalpha,
aureliusbtc,
trajan0x and
Defi-Moses
as code owners
WalkthroughThis pull request introduces various updates across multiple packages, including documentation enhancements, changelog entries, package version increments, and additions of new constants and interfaces. Notably, it adds a hyperlink and a detailed audit draft in documentation, updates version numbers and dependencies, introduces new router address mappings, and modifies the logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant applyQuote
Caller->>applyQuote: Call with originAmount, quote
alt originAmount is 0 or exceeds maxOriginAmount
applyQuote-->>Caller: Return Zero
else
applyQuote->>applyQuote: Calculate destAmount = originAmount * quote.destAmount / quote.maxOriginAmount
alt destAmount less than fixedFee
applyQuote-->>Caller: Return Zero
else
applyQuote-->>Caller: Return (destAmount - quote.fixedFee)
end
end
Possibly related PRs
Suggested labels
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
![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
🧹 Nitpick comments (4)
assets/sanguine-audits/Synapse Fast Bridge Review - Aleph_v.md (1)
1-1: Format the bare URL as a proper markdown link.The URL should be formatted using proper markdown syntax for better readability and to comply with markdown best practices.
-https://hackmd.io/@geistermeister/rJvvSfDQJe +[Synapse Fast Bridge Review](https://hackmd.io/@geistermeister/rJvvSfDQJe)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
1-1: Bare URL used
null(MD034, no-bare-urls)
assets/sanguine-audits/chainlight.md (3)
4-50: Consider proper numbering for each separate audit item.Currently, multiple items are labeled as “1.” (e.g.,
[SYNAPSE-001],[SYNAPSE-002]...). For textual clarity, increment them consistently or use auto-numbering in Markdown.🧰 Tools
🪛 LanguageTool
[style] ~22-~22: ‘take into account’ might be wordy. Consider a shorter alternative.
Context: ...onvertRemoteValueToLocalUnitsdoes not take into account the decimal difference -_remoteGa...(EN_WORDINESS_PREMIUM_TAKE_INTO_ACCOUNT)
[style] ~24-~24: ‘take into account’ might be wordy. Consider a shorter alternative.
Context: ...onvertRemoteValueToLocalUnits` does not take into account the decimal difference between the two ...(EN_WORDINESS_PREMIUM_TAKE_INTO_ACCOUNT)
[uncategorized] ~27-~27: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...cols (i.e., Layer Zero) support message rate limiting feature. - Consider implementing ra...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~39-~39: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...ditional gas limits. - It should be very careful not to use the existing app balance to ...(EN_WEAK_ADJECTIVE)
[style] ~41-~41: This phrase is redundant. Consider writing “points” or “times”.
Context: ...d be significantly different at the two points in time, which could lead to uncertain results ...(MOMENT_IN_TIME)
[uncategorized] ~41-~41: A comma may be missing after the conjunctive/linking adverb ‘Therefore’.
Context: ...tain results and cause security issues. Therefore it is recommended to add a timeout to t...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
🪛 markdownlint-cli2 (0.17.2)
29-29: Bare URL used
null(MD034, no-bare-urls)
33-33: Bare URL used
null(MD034, no-bare-urls)
34-34: Bare URL used
null(MD034, no-bare-urls)
35-35: Bare URL used
null(MD034, no-bare-urls)
36-36: Bare URL used
null(MD034, no-bare-urls)
22-41: Refine wording and punctuation for clarity.
- “Take into account” (lines 22, 24) can be shortened to “consider” or “account for” to reduce wordiness.
- “It should be very careful...” (line 39) could remove “very” for a crisper tone.
- “Therefore it is recommended...” (line 41) typically needs a comma after “Therefore.”
🧰 Tools
🪛 LanguageTool
[style] ~22-~22: ‘take into account’ might be wordy. Consider a shorter alternative.
Context: ...onvertRemoteValueToLocalUnitsdoes not take into account the decimal difference -_remoteGa...(EN_WORDINESS_PREMIUM_TAKE_INTO_ACCOUNT)
[style] ~24-~24: ‘take into account’ might be wordy. Consider a shorter alternative.
Context: ...onvertRemoteValueToLocalUnits` does not take into account the decimal difference between the two ...(EN_WORDINESS_PREMIUM_TAKE_INTO_ACCOUNT)
[uncategorized] ~27-~27: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...cols (i.e., Layer Zero) support message rate limiting feature. - Consider implementing ra...(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~39-~39: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...ditional gas limits. - It should be very careful not to use the existing app balance to ...(EN_WEAK_ADJECTIVE)
[style] ~41-~41: This phrase is redundant. Consider writing “points” or “times”.
Context: ...d be significantly different at the two points in time, which could lead to uncertain results ...(MOMENT_IN_TIME)
[uncategorized] ~41-~41: A comma may be missing after the conjunctive/linking adverb ‘Therefore’.
Context: ...tain results and cause security issues. Therefore it is recommended to add a timeout to t...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
🪛 markdownlint-cli2 (0.17.2)
29-29: Bare URL used
null(MD034, no-bare-urls)
33-33: Bare URL used
null(MD034, no-bare-urls)
34-34: Bare URL used
null(MD034, no-bare-urls)
35-35: Bare URL used
null(MD034, no-bare-urls)
36-36: Bare URL used
null(MD034, no-bare-urls)
29-36: Address minor Markdown issues.
- Bare URLs (lines 29, 33, 34, 35, 36) can be enclosed in angle brackets or turned into clickable links as per Markdown best practices.
- Verify there are no unintended spaces inside emphasis markers (line 55). Maintaining consistent formatting improves readability.
Also applies to: 55-55
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
29-29: Bare URL used
null(MD034, no-bare-urls)
33-33: Bare URL used
null(MD034, no-bare-urls)
34-34: Bare URL used
null(MD034, no-bare-urls)
35-35: Bare URL used
null(MD034, no-bare-urls)
36-36: Bare URL used
null(MD034, no-bare-urls)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
assets/sanguine-audits/Sanguine - Zellic Audit Report (1).pdfis excluded by!**/*.pdfassets/sanguine-audits/Trail of Bits - Synapse Protocol Comprehensive Report (2).pdfis excluded by!**/*.pdfassets/sanguine-audits/Zellic Security Assessment Report.pdfis excluded by!**/*.pdf
📒 Files selected for processing (14)
assets/sanguine-audits/Synapse Fast Bridge Review - Aleph_v.md(1 hunks)assets/sanguine-audits/chainlight.md(1 hunks)packages/rest-api/CHANGELOG.md(1 hunks)packages/rest-api/package.json(2 hunks)packages/rest-api/src/constants/index.ts(1 hunks)packages/sdk-router/CHANGELOG.md(1 hunks)packages/sdk-router/package.json(1 hunks)packages/sdk-router/src/rfq/quote.test.ts(9 hunks)packages/sdk-router/src/rfq/quote.ts(1 hunks)packages/synapse-interface/CHANGELOG.md(1 hunks)packages/synapse-interface/package.json(2 hunks)packages/synapse-interface/public/blacklist.json(1 hunks)packages/widget/CHANGELOG.md(1 hunks)packages/widget/package.json(2 hunks)
✅ Files skipped from review due to trivial changes (6)
- packages/sdk-router/package.json
- packages/widget/CHANGELOG.md
- packages/widget/package.json
- packages/synapse-interface/package.json
- packages/synapse-interface/CHANGELOG.md
- packages/rest-api/CHANGELOG.md
🧰 Additional context used
🧠 Learnings (1)
packages/rest-api/src/constants/index.ts (1)
Learnt from: abtestingalpha
PR: synapsecns/sanguine#3307
File: packages/rest-api/src/constants/index.ts:33-43
Timestamp: 2024-11-12T03:37:08.148Z
Learning: In `packages/rest-api/src/constants/index.ts`, recognize that `FAST_BRIDGE_ROUTER_ADDRESS_MAP` contains identical and correctly checksummed addresses.
🪛 LanguageTool
assets/sanguine-audits/chainlight.md
[style] ~22-~22: ‘take into account’ might be wordy. Consider a shorter alternative.
Context: ...onvertRemoteValueToLocalUnitsdoes not take into account the decimal difference -_remoteGa...
(EN_WORDINESS_PREMIUM_TAKE_INTO_ACCOUNT)
[style] ~24-~24: ‘take into account’ might be wordy. Consider a shorter alternative.
Context: ...onvertRemoteValueToLocalUnits` does not take into account the decimal difference between the two ...
(EN_WORDINESS_PREMIUM_TAKE_INTO_ACCOUNT)
[uncategorized] ~27-~27: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...cols (i.e., Layer Zero) support message rate limiting feature. - Consider implementing ra...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[style] ~39-~39: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...ditional gas limits. - It should be very careful not to use the existing app balance to ...
(EN_WEAK_ADJECTIVE)
[style] ~41-~41: This phrase is redundant. Consider writing “points” or “times”.
Context: ...d be significantly different at the two points in time, which could lead to uncertain results ...
(MOMENT_IN_TIME)
[uncategorized] ~41-~41: A comma may be missing after the conjunctive/linking adverb ‘Therefore’.
Context: ...tain results and cause security issues. Therefore it is recommended to add a timeout to t...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
🪛 markdownlint-cli2 (0.17.2)
assets/sanguine-audits/chainlight.md
29-29: Bare URL used
null
(MD034, no-bare-urls)
33-33: Bare URL used
null
(MD034, no-bare-urls)
34-34: Bare URL used
null
(MD034, no-bare-urls)
35-35: Bare URL used
null
(MD034, no-bare-urls)
36-36: Bare URL used
null
(MD034, no-bare-urls)
55-55: Spaces inside emphasis markers
null
(MD037, no-space-in-emphasis)
assets/sanguine-audits/Synapse Fast Bridge Review - Aleph_v.md
1-1: Bare URL used
null
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Deploy to Vercel (synapse-interface)
- GitHub Check: lint
- GitHub Check: test
- GitHub Check: Cloudflare Pages
🔇 Additional comments (24)
packages/sdk-router/CHANGELOG.md (1)
6-14: Changelog entry looks good!The changelog entry correctly documents the bug fix for fee calculation with proper references to the issue number and PR.
packages/rest-api/package.json (1)
3-3: Version updates are correctly aligned.The package version update and the sdk-router dependency update are properly synchronized, ensuring compatibility with the bug fix for fee calculation implemented in sdk-router v0.11.11.
Also applies to: 25-25
packages/rest-api/src/constants/index.ts (1)
43-44:✅ Verification successful
Added support for two new chains in Fast Bridge Router.
The addition of chain IDs 130 and 80094 to the FAST_BRIDGE_ROUTER_ADDRESS_MAP expands the support for the Fast Bridge Router functionality to these networks.
🏁 Script executed:
#!/bin/bash # Find what networks these chain IDs correspond to rg -B 1 -A 1 "id: 130" --glob "*.ts" rg -B 1 -A 1 "id: 80094" --glob "*.ts"Length of output: 1822
Attention: Fast Bridge Router Chain Mapping Update
The FAST_BRIDGE_ROUTER_ADDRESS_MAP now correctly includes support for Unichain (Chain ID: 130) and Berachain (Chain ID: 80094). Our investigation confirms that these IDs are consistently defined across the codebase (e.g., in
packages/rest-api/src/constants/chains.tsand other related files) with the respective network names. Please verify that the address0x00cD000000003f7F682BE4813200893d4e690000is the intended router address for both networks.packages/sdk-router/src/rfq/quote.ts (5)
77-79: Ensure out-of-range amounts are handled as intended.Returning zero when
originAmountis zero or exceedsquote.maxOriginAmountis a straightforward safeguard. Confirm that any downstream code callingapplyQuoteexpects a zero return value in these out-of-range scenarios.
80-80: No concerns with this explanatory comment.
81-83: Double-check precision in proportional calculation.The proportional calculation
(originAmount * destAmount) / maxOriginAmountcan introduce rounding differences. Ensure that downstream logic tolerates this rounding and that finite precision is acceptable for your use case.
85-85: Validate the fixed fee against small transfers.When
destAmountis smaller than thefixedFee, the function returns zero. Confirm that this intended behavior—which essentially discards small transfers—aligns with business requirements.
88-88: Good subtraction logic.Subtracting the
fixedFeefromdestAmountafter confirming it’s sufficiently large is correct.packages/synapse-interface/public/blacklist.json (1)
662-686: Removal and addition of blacklist entries.The removal of
"0xbca02b395747d62626a65016f2e64a20bd254a39"and the addition of multiple addresses appear consistent. Verify that each newly added address is confirmed malicious or disallowed, and that removing the old entry still aligns with your security policies.packages/sdk-router/src/rfq/quote.test.ts (15)
2-2: Good addition of Zero constant.Using the Zero constant from @ethersproject/constants improves code consistency and readability compared to BigNumber.from(0).
12-18: Well-designed helper functions for BigNumber comparison.These utility functions ensure consistent BigNumber comparisons by converting to strings. This is a safer approach than direct object comparison and improves test reliability.
22-24: Good refactoring to use the new utility functions.Proper use of expectEqualBigNumbers with Zero constant improves test readability and maintainability.
26-29: Improved test case clarity.The test case logic is clearer now, using the Zero constant and expectEqualBigNumbers for consistent comparisons.
31-36: Clearer logic for fixed fee handling.The test case properly checks if the destination amount is lower than the fixed fee, which is a valid edge case to test.
41-41: Good use of negative assertion.Using expectNotEqualBigNumbers is appropriate here to verify the quote returns a non-zero value.
67-67: Important change to fixed fee decimals.The fixed fee is now using destination decimals instead of origin decimals, which aligns with changes in the applyQuote function implementation.
87-92: Updated calculation reflects fee changes.The comment and expected value properly reflect the new calculation logic where the fee is subtracted after conversion, resulting in 9.0010 destination tokens.
104-109: Consistent application of fee logic.The comment and test case correctly implement the same fee logic for the 1:0.9999 price ratio scenario.
187-189: Improved clarity with calculation comment.Adding the calculation formula in the comment helps understand the expected value and makes the test more maintainable.
199-201: Consistent approach for different decimal cases.The test properly handles the case where origin has fewer decimals than destination, with appropriate comments explaining the calculation.
211-213: Correct decimal handling for smaller destination decimals.The test correctly truncates the result to match the destination decimal precision (6 decimals).
215-222: Good edge case testing for zero maxOriginAmount.This test properly validates that the applyQuote function returns Zero when maxOriginAmount is Zero, regardless of the input amount.
224-231: Proper handling of zero destAmount edge case.The test correctly verifies that applyQuote returns Zero when destAmount is Zero, which is important for avoiding division by zero errors.
233-241: Comprehensive edge case testing.Testing when both maxOriginAmount and destAmount are Zero ensures complete coverage of potential edge cases.
* feat: scaffold GasZip module * refactor: isolate logging utils * refactor: isolate API utils * feat: bridge, status first impl * feat: supported chain ID check * feat: `isSameAddress` * feat: gas.zip quotes * feat: no-op slippage * feat: add gas.zip module * fix: sanitize expected amount from gas.zip * fix: bridge module name * feat: prioritize gas.zip quotes for testing [REVERT LATER] * feat: regenerate bridge map with gas.zip [SYN-38] * feat: don't show slippage for gas.zip for now [SYN-39] * feat: add BNB, BERA and other native tokens * feat: add HyperEVM to the list of chains * feat: add HYPE to bridge map * chore: add hyperEVM to spellcheck * chore: clean up TODOs * fix: remove trailing slash * fix: remove unsupported gas.zip assets [SYN-53] * Revert "feat: prioritize gas.zip quotes for testing [REVERT LATER]" This reverts commit 3072d75. * feat: track gas.zip refund status * fix: gas.zip refund tracking * refactor: logs cleanup, better typing
Deploying sanguine-fe with
|
| Latest commit: |
531916b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f78111e0.sanguine-fe.pages.dev |
| Branch Preview URL: | https://master.sanguine-fe.pages.dev |
github-actions
bot
added
javascript
ChiTimesChi
temporarily deployed
to
github-pages
- @synapsecns/[email protected] - @synapsecns/[email protected] - @synapsecns/[email protected] - @synapsecns/[email protected]
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (20)
packages/sdk-router/src/utils/RouterCache.ts (1)
61-73: Enhanced error logging with structured contextImproved error logging by replacing console.error with structured logging that includes comprehensive context (target, args, method name, chain details, etc.). This will make debugging much easier when errors occur.
You might want to extract the error context object creation to a separate function since similar structured context might be useful in other parts of the codebase:
+function createErrorContext(target, args, propertyKey, instance) { + return { + target, + args, + propertyKey, + name: target.constructor.name, + address: instance.address, + chainId: instance.chainId, + }; +} // Then in the catch block: .catch((error) => { logger.error( - { - target, - args, - propertyKey, - name: target.constructor.name, - address: this.address, - chainId: this.chainId, - error, - }, + {...createErrorContext(target, args, propertyKey, this), error}, '[SynapseSDK: RouterCache] Error' ) })packages/synapse-interface/components/_Transaction/_Transaction.tsx (1)
79-80: Consider addressing the TODO comment.This TODO comment suggests that the code could be refactored for better clarity. Consider revisiting this section to improve code organization and readability.
packages/synapse-interface/scripts/generateMaps.js (1)
267-274: Assess future extensibility.
This new function creates a simple mapping for native ETH if the chain is Gas.zip supported. If you envision Gas.zip expanding beyond ETH, consider a more flexible approach to allow additional tokens.packages/sdk-router/src/gaszip/gasZipModule.ts (1)
24-54: Bridge method logic is clear but could use additional documentation.
Your checks for native-only tokens and the comparison withdestQuery.rawParamsappear correct. Consider clarifying the usage ofdestQuery.rawParamsin a docstring to avoid confusion.packages/sdk-router/src/utils/api.ts (3)
3-39: Consider surfacing errors instead of returningnull.When a response is not OK or an error occurs, returning
nullmay obscure the underlying issue for upstream callers. You might consider throwing a custom error or returning a structured error object to allow callers to distinguish timeout scenarios from server or network errors.
27-29: Avoid partially handling abort errors in catch block.Currently, the code logs the abort scenario but does not differentiate between user-initiated aborts and real timeouts beyond just logging. Providing more context (e.g., a custom message or code) could help identify the cause of the abort more clearly in upstream error handling.
74-89: Check for edge cases inputWithTimeout.Similar to
postWithTimeout, ensureparamsare always serializable to JSON. Additionally, consider returning a specialized error if the server rejects PUT requests with large or malformed data, rather than relying on null returns only.packages/synapse-interface/constants/bridgeMap.ts (3)
715-720: Review'POL'bridging with'Gas.zip'and'MATIC'.Polygon’s native token is typically
MATIC. If'POL'is an official rebrand, ensure all references toPOLvsMATICare coherent in the user interface and bridging flows.
1582-1583: Synchronize multi-ETH references.For
'ETH'on chain'8453', multiple references'Gas.zip','RFQ.ETH','nETH'can be confusing. Confirm your DApp properly detects these token representations.
1988-1989: Consolidate'ETH'bridging references for chain 81457.Again,
'Gas.zip','RFQ.ETH', and'nETH'might overlap in functionality. Document or unify these references to minimize confusion.packages/sdk-router/src/gaszip/api.ts (3)
10-16: ExpandTransactionandTransactionStatusDatawith clearer status fields.While these interfaces define base properties, consider detailing or enumerating possible error codes or partial statuses for more robust client handling and logging.
45-53: RefineGasZipQuotestructure for clarity.Because
amountOutis aBigNumberandcalldatais a hex string, you may want to store additional metadata about quotes. Doing so can help disambiguate partial or invalid quotes.
55-66:getGasZipTxStatus: Evaluate partial confirmations.Returning only
booleanmight lose nuance about intermediate or pending statuses. Consider returning an enumeration or a structured object so upstream layers can more intelligently handle “in-progress” states.packages/synapse-interface/components/_Transaction/helpers/useTxRefundStatus.ts (5)
20-20: Check for overlap between'PRIORITY'and'PENDING'statuses.If
'PRIORITY'is just a variant of'PENDING', consider combining them or clarifying usage. This avoids potential confusion in the UI layer.
27-39: Consider more explicit naming inDeposit.
valueis a string, andusdis a number. Potentially rename them toamountValueandusdValuefor clarity, especially if you plan additional fields later (likegasCost).
41-53: SynchronizeTransactioninterface with upstream.The
statusproperty is typed asStatus, butvalueis typed asnumberwhile deposit uses astring. If they represent the same concept (like tokens or native currency), unify their data types to prevent confusion.
62-63: EvaluateGAS_ZIP_API_URLandGAS_ZIP_DEPOSIT_ADDRESSusage.These constants tie the logic to a particular backend and deposit address. If you have multiple environments (dev, staging, production), consider parameterizing these values or injecting them at build time.
66-66:txIdparameter made mandatory.Consider fallback handling if
txIdis not provided or is invalid. A guard clause can provide immediate user feedback rather than waiting for an external call to fail.packages/sdk-router/src/gaszip/gasZipModuleSet.ts (2)
19-20: Consider making the estimated time configurable.Currently,
MEDIAN_TIME_GAS_ZIPis hard-coded to 30 seconds. It may be beneficial to expose this value as a configurable parameter so it can be adjusted as gas.zip mechanics or network conditions evolve.
120-136: Clarify zero-fee design.
getFeeData()hard-codes a fee of zero. While this is presumably by design for gas.zip, other modules might incorporate dynamic fees. Consider clearly documenting the rationale for having no fees, to help developers maintain or extend the code.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
cspell.json(1 hunks)packages/sdk-router/src/gaszip/api.ts(1 hunks)packages/sdk-router/src/gaszip/gasZipModule.ts(1 hunks)packages/sdk-router/src/gaszip/gasZipModuleSet.ts(1 hunks)packages/sdk-router/src/gaszip/index.ts(1 hunks)packages/sdk-router/src/rfq/api.ts(3 hunks)packages/sdk-router/src/router/router.ts(3 hunks)packages/sdk-router/src/router/routerSet.ts(2 hunks)packages/sdk-router/src/sdk.ts(3 hunks)packages/sdk-router/src/utils/RouterCache.ts(2 hunks)packages/sdk-router/src/utils/addressUtils.test.ts(1 hunks)packages/sdk-router/src/utils/addressUtils.ts(1 hunks)packages/sdk-router/src/utils/api.ts(1 hunks)packages/sdk-router/src/utils/logger.ts(1 hunks)packages/synapse-interface/components/StateManagedBridge/BridgeExchangeRateInfo.tsx(1 hunks)packages/synapse-interface/components/_Transaction/_Transaction.tsx(3 hunks)packages/synapse-interface/components/_Transaction/helpers/useTxRefundStatus.ts(4 hunks)packages/synapse-interface/constants/bridgeMap.ts(19 hunks)packages/synapse-interface/constants/chains/extraWagmiChains.ts(1 hunks)packages/synapse-interface/constants/chains/index.tsx(1 hunks)packages/synapse-interface/constants/chains/master.tsx(3 hunks)packages/synapse-interface/constants/chains/supportedChains.ts(2 hunks)packages/synapse-interface/constants/tokens/bridgeable.ts(3 hunks)packages/synapse-interface/scripts/data/providerOverrides.json(1 hunks)packages/synapse-interface/scripts/generateMaps.js(6 hunks)packages/synapse-interface/scripts/utils/fetchGasZipData.js(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/sdk-router/src/gaszip/index.ts
- packages/synapse-interface/scripts/data/providerOverrides.json
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Deploy to Vercel (synapse-interface)
- GitHub Check: Deploy to Vercel (synapse-interface)
- GitHub Check: lint
- GitHub Check: test
- GitHub Check: SonarCloud
- GitHub Check: test
- GitHub Check: lint
- GitHub Check: Analyze go
- GitHub Check: Cloudflare Pages
🔇 Additional comments (77)
cspell.json (1)
47-47: Addition of "hyperevm" to recognized words looks good.The term "hyperevm" has been added to the spell checker dictionary, which aligns with the introduction of the hyperEVM blockchain configuration elsewhere in the codebase.
packages/sdk-router/src/utils/addressUtils.ts (1)
1-3: Clean implementation of address comparison utility.This utility function correctly handles Ethereum address comparison by:
- Converting both addresses to lowercase (Ethereum addresses are case-insensitive)
- Safely handling undefined/null values with the double bang (!!) operator
This is a useful addition that will help maintain consistent address comparison logic throughout the codebase.
packages/synapse-interface/constants/chains/index.tsx (1)
47-47: New hyperEVM chain ID addition looks good.The addition of
HYPEREVM: 999to the ChainId object is consistent with the rest of the file structure and naming conventions.packages/sdk-router/src/router/routerSet.ts (2)
20-20: Good addition of structured logging.Adding the logger import enhances the error logging capabilities of the module.
144-155: Improved error logging with structured context.The migration from console.error to structured logging is a good improvement. The new implementation:
- Provides comprehensive context for debugging (all relevant parameters)
- Uses consistent error message formatting
- Makes logs more searchable and filterable
This change will make troubleshooting bridge route calculation issues significantly easier.
packages/synapse-interface/constants/chains/master.tsx (3)
657-657: Formatting change looks good.The rpcUrl has been reformatted to be on a single line, maintaining consistency with other chain definitions.
683-683: Formatting change looks good.The rpcUrl has been reformatted to be on a single line, maintaining consistency with other chain definitions.
701-726:✅ Verification successful
New HyperEVM chain added with a TODO for icon replacement.
The HyperEVM chain configuration is well-structured and contains all the required properties. However, there's a TODO comment indicating that a different icon might be needed instead of reusing the hyperliquidImg.
Please ensure that the chain ID and RPC endpoints are correct and functional:
🏁 Script executed:
#!/bin/bash # Test if RPC endpoint is functional curl -s -X POST \ -H "Content-Type: application/json" \ --data '{"jsonrpc":"2.0","method":"eth_chainId","params":[],"id":1}' \ https://rpc.hyperliquid.xyz/evmLength of output: 196
HyperEVM Chain Configuration Verified and Approved
- The RPC endpoint (https://rpc.hyperliquid.xyz/evm) returns a chain ID of "0x3e7" (which is 999 in decimal), matching the configuration.
- All required properties are correctly set.
- The TODO for using a different icon remains noted for future update.
packages/synapse-interface/scripts/utils/fetchGasZipData.js (1)
1-10:✅ Verification successful
New utility function to fetch Gas.zip data.
The function retrieves chain data from Gas.zip API and extracts chain identifiers. The TODO comment indicates a need to filter out chains without direct deposit support in the future.
Let's verify the API endpoint:
🏁 Script executed:
#!/bin/bash # Check if the Gas.zip API endpoint is accessible response=$(curl -s -o /dev/null -w "%{http_code}" https://backend.gas.zip/v2/chains) echo "HTTP Status Code: $response" # If accessible, check the response structure if [ "$response" -eq 200 ]; then curl -s https://backend.gas.zip/v2/chains | jq -r '.chains[0:2]' fiLength of output: 776
Verified API and Data Structure – No Further Changes Needed
The new utility function correctly fetches chain data from the Gas.zip API. Our verification confirms that the endpoint (https://backend.gas.zip/v2/chains) returns an HTTP 200 status with the expected JSON structure (an array of chain objects), and the function appropriately extracts the chain identifiers. The existing TODO for filtering out chains without direct deposit support remains noted for future work.packages/sdk-router/src/sdk.ts (4)
4-4: New module import added for Gas.zip integration.Adding the GasZipModuleSet import to support Gas.zip functionality.
21-21: New public property for Gas.zip module.Added the gasZipModuleSet property to the SynapseSDK class.
50-50: Initializing the Gas.zip module set.The gasZipModuleSet is properly initialized with chainProviders, consistent with other module sets.
55-55: Added Gas.zip module to allModuleSets array.This ensures the Gas.zip module will be included in operations that iterate through all modules.
packages/synapse-interface/constants/chains/extraWagmiChains.ts (1)
148-173:✅ Verification successful
New hyperEVM chain configuration added.
The configuration includes all necessary properties: id, name, network, nativeCurrency details, RPC URLs, block explorer information, and multicall3 contract address. This aligns with the corresponding HYPEREVM chain definition in master.tsx.
Make sure the multicall3 contract address and blockCreated value are correct:
🏁 Script executed:
#!/bin/bash # Check if the multicall3 contract exists at the specified address on hyperEVM curl -s -X POST \ -H "Content-Type: application/json" \ --data '{"jsonrpc":"2.0","method":"eth_getCode","params":["0xca11bde05977b3631167028862be2a173976ca11", "latest"],"id":1}' \ https://rpc.hyperliquid.xyz/evm | jq .Length of output: 7883
HyperEVM configuration verified and approved.
The new chain configuration in
packages/synapse-interface/constants/chains/extraWagmiChains.tsincludes all the required properties. Our RPC query confirms that the multicall3 contract exists at the specified address (0xca11bde05977b3631167028862be2a173976ca11) by returning valid bytecode. Although the block creation number (13051) wasn’t directly verified via the RPC call, there are no discrepancies, and the configuration aligns with the corresponding HYPEREVM definition in master.tsx.packages/synapse-interface/components/StateManagedBridge/BridgeExchangeRateInfo.tsx (3)
66-66: Added TODO for future enhancementGood documentation of a future requirement to handle slippage between different asset types.
68-68: Destructured bridgeModuleName from bridgeQuoteCorrectly added
bridgeModuleNameto the destructured properties to support the conditional rendering in line 76.
76-76: Enhanced conditional rendering for Gas.zip moduleThe slippage percentage will now be hidden when using the 'Gas.zip' bridge module, maintaining consistent UI behavior for different module types.
packages/sdk-router/src/utils/logger.ts (2)
1-2: Created centralized logger interfaceGood practice to establish a central logging interface that can be consistently used across the codebase.
3-18: Added execution time logging decoratorGreat implementation of a reusable decorator that measures and logs execution time for methods. The decorator properly handles async functions and provides useful context in the logs.
Example usage would be:
class SomeService { @logExecutionTime('fetchData') async fetchData() { // Method implementation } }packages/synapse-interface/constants/chains/supportedChains.ts (2)
30-30: Added hyperEVM to importsSuccessfully imported the hyperEVM chain configuration.
60-60:❓ Verification inconclusive
Added hyperEVM to supported chains
Added hyperEVM to the list of supported chains, which will allow users to interact with this chain through the interface.
Make sure to verify that all required configurations for hyperEVM are properly set up in the related chain constants files.
🏁 Script executed:
#!/bin/bash # Check if hyperEVM is properly configured in chain constants # Check for hyperEVM definition in extraWagmiChains echo "Checking hyperEVM definition in extraWagmiChains.ts:" rg -A 10 "export const hyperEVM" packages/synapse-interface/constants/chains/extraWagmiChains.ts # Check for hyperEVM in ChainId enum/object echo -e "\nChecking for hyperEVM in ChainId definition:" rg "HYPEREVM" packages/synapse-interface/constants/chains/index.tsx # Check if there are any router addresses configured for hyperEVM echo -e "\nChecking for router addresses for hyperEVM:" rg "999" packages/synapse-interface/constants/bridgeLength of output: 877
Action: Verify hyperEVM router addresses configuration
- The hyperEVM chain is properly defined in
packages/synapse-interface/constants/chains/extraWagmiChains.tsand the corresponding ChainId mapping (i.e.HYPEREVM: 999,) is present inpackages/synapse-interface/constants/chains/index.tsx.- However, the search for router addresses (expected under
packages/synapse-interface/constants/bridge) did not locate the relevant configuration file. Please verify whether:
- Router addresses for hyperEVM are required.
- They are maintained under a different path or intentionally omitted.
packages/sdk-router/src/utils/RouterCache.ts (1)
5-6: Imported centralized loggerProperly imported the logger from the centralized logging utility.
packages/synapse-interface/components/_Transaction/_Transaction.tsx (3)
80-88: LGTM! Changed fromconsttoletto allow reassignment.Changing the destructuring declaration from
consttoletis appropriate since theisCheckTxForRefundvariable needs to be reassigned later in the code.
89-91: Gas.zip refund check implementation looks good.The implementation correctly sets
isCheckTxForRefundto the value ofisEstimatedTimeReachedspecifically for Gas.zip bridge transactions, supporting the refund check functionality for this new bridge module.
109-115: Correctly extended transaction refund check to support Gas.zip.The condition for checking if a transaction is refunded has been appropriately expanded to include both 'SynapseRFQ' and 'Gas.zip' bridge modules, ensuring consistent refund status functionality across different bridge types.
packages/sdk-router/src/utils/addressUtils.test.ts (5)
1-2: Well-structured import for the test file.The import is clean and focused on the specific functionality being tested.
3-11: Good test setup with clear test data.The test setup is well-organized with constant definitions for different address formats (lowercase, checksummed, uppercase) for two distinct addresses, which helps make the test cases clear and maintainable.
12-48: Comprehensive test coverage for matching addresses.This test section thoroughly checks the equality functionality across various address formats, including every possible combination of lowercase, checksummed, and uppercase addresses. The test cases are well-organized and clearly labeled.
50-80: Thorough negative test cases for non-matching addresses.The test section thoroughly verifies that the function correctly returns false for different addresses in all format combinations, ensuring robust behavior when addresses don't match.
82-113: Good edge case handling for special values.The tests correctly verify the function's behavior with edge cases like undefined, empty strings, and null values, which is important for ensuring robust handling of potentially problematic inputs.
packages/synapse-interface/constants/tokens/bridgeable.ts (5)
5-5: Remember to follow up on the TODO comment for token logos.The comment indicates that the logos for the native tokens need to be corrected. Consider prioritizing this to ensure consistent visual representation of tokens in the UI.
Also applies to: 46-49
1263-1277: BERA token implementation looks good.The BERA token is properly defined with all required properties, including setting up the chain address, decimals, name, symbol, and other required token attributes following the established pattern.
1279-1293: BNB token implementation looks good.The BNB token implementation follows the same consistent pattern as other native tokens in the file, with appropriate properties set for UI visualization and routing.
1295-1309: CRO token implementation looks good.The CRO token implementation is correctly structured with all necessary properties, maintaining consistency with other token definitions.
1311-1325: HYPE token implementation looks good.The HYPE token implementation for HyperEVM follows the established pattern with all required properties correctly defined.
packages/sdk-router/src/rfq/api.ts (4)
1-2: Good replacement of custom fetch with reusable utility.The imports for
getWithTimeoutandloggerimprove code maintainability by leveraging centralized, reusable utilities instead of maintaining local implementations.
18-27: Improved API call implementation with better error handling.The refactored API call uses a centralized timeout utility and checks for a falsy response, which is a more robust approach than the previous implementation.
35-35: Enhanced error logging with structured context.Using the logger with structured context provides more useful error information compared to simple console logs, making debugging easier in production environments.
41-41: Consistent structured logging for error handling.The error logging for failed fetch operations now includes the error context in a structured format, consistent with the logging pattern used elsewhere in the function.
packages/synapse-interface/scripts/generateMaps.js (6)
6-6: Consider validating fetched data.
Fetching external data can fail, return empty objects, or otherwise cause unexpected issues. Consider adding error handling or default fallbacks in casefetchGasZipDatareturns invalid or missing data.
12-12: No immediate concerns.
Introduced usage ofproviderOverrides. Ensure that the JSON structure inproviderOverrides.jsonremains consistent to avoid runtime exceptions.
58-58: Fallback logic confirmed.
UsingproviderOverrides[chainId] || process.env.RPC_URLis a safe approach for provider URLs. No issues here sinceRPC_URLusage is already validated above.
384-384: Check error handling forgasZipChains.
When callingfetchGasZipData(), ensure you handle scenarios where the list is empty or contains unexpected values.
401-404: Merging Gas.zip origin map.
Successfully integrates the Gas.zip origin map by merging sets viaaddSetToMap. No further issues identified.
433-437: Consistent destination extension for Gas.zip.
Appending'Gas.zip'for ETH on supported chains aligns with the origin map logic. This is a clear, straightforward approach.packages/sdk-router/src/router/router.ts (3)
11-11: Structured logger import approved.
Replacingconsole.errorwith the centralized logger enhances consistency and can facilitate better log management.
121-124: Improved logging context.
Including{ tokenIn, tokenSymbols, amountIn, error }in the log object provides valuable debugging context. Confirm that no sensitive user data is inadvertently logged.
145-148: Extended error logging for destination queries.
Similarly, adding{ requests, tokenOut, error }ensures thorough debugging information. Again, confirm it doesn't expose sensitive details.packages/sdk-router/src/gaszip/gasZipModule.ts (4)
1-10: Imports and module structure look good.
All referenced utility functions and constants (isNativeToken,getGasZipQuote, etc.) are logically grouped, improving clarity.
11-22: Constructor validation is appropriate.
Usinginvariantto ensure valid chain ID and provider is a good safeguard against incomplete instantiation.
56-61: Simple pass-through ID retrieval.
Returning the transaction hash unmodified is acceptable. If you plan to store or transform these IDs, add relevant documentation or validations.
63-69: Bridge transaction status retrieval.
Delegating togetGasZipTxStatussimplifies the module’s responsibilities. Consider handling exceptions ifgetGasZipTxStatusfails or returns unexpected data.packages/sdk-router/src/utils/api.ts (2)
41-55: Reinforce type-checking for query parameters ingetWithTimeout.Passing
paramsdirectly intoURLSearchParamsworks well, but ensure that theparamsobject always maps to stringifiable values, to avoid runtime type conversion issues. A brief type guard or runtime check could prevent unexpected failures.
57-72: Validate JSON payload size inpostWithTimeout.While
JSON.stringify(params)is straightforward, large payloads might degrade performance or cause memory overhead. Consider validating or limiting the size ofparamsif you expect potential large data structures.
[performance]packages/synapse-interface/constants/bridgeMap.ts (12)
200-200: Confirm addition of'RFQ.USDC'as a destination.Including
'RFQ.USDC'expands the bridging routes for USDC on chain ID '1'. Verify upstream usage to prevent misrouting or unexpected bridging paths.
242-243: Review synergy of'Gas.zip','RFQ.ETH', and'nETH'as origins and destinations for ETH.Having multiple protocol representations for the same asset can become confusing if not well documented. Confirm these entries are aligned with your bridging logic and test coverage.
444-445: Align'Gas.zip','RFQ.ETH', and'nETH'mapping details with chain usage.Like the previous lines, ensure consumers of this bridging map handle all references consistently to avoid cross-chain confusion or partial bridging issues.
467-473: ValidateCROtoken details.For
'CRO'under0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE, confirm the decimals (18) and chain references are correct.CROtokens can vary across chain environments, so ensure these details match the target network.
518-524: EnsureETHon BSC remains accessible.This entry sets
destination: []for the'0x2170Ed...'address on chain 56. Verify if this is intentional or temporary. Having an emptydestinationarray could block bridging to other networks.
572-578: Check BNB bridging approach.
BNBis listed only for'Gas.zip'origin/destination. Confirm user flows that rely on direct bridging to or fromBNBdo not break if they expect standard BSC bridging. You might need to add or remove references toRFQ.ETHornETHfor consistency.
824-825: Revisit dual origins'FTM'and'Gas.zip'.When bridging FTM, double-check that combining
'FTM'and'Gas.zip'flows is correct. Evaluate user flows to ensure no collisions occur when bridging from gas-based solutions vs. standard bridging.
1011-1012: Validate'Metis'origins.
'Gas.zip'plus'Metis'might be a specialized bridging flow. Confirm necessary off-chain or aggregator components are set up to handle these transitions.
1604-1604: Check newly assigned symbol'USD₮0'.Renaming Tether to
'USD₮0'might confuse users used to'USDT'. Confirm user-facing labels stay consistent unless there is a business reason for this naming change.
1776-1777: Confirm'AVAX'bridging with'Gas.zip'.The new bridging approach for AVAX includes
'Gas.zip'in both origin and destination. Verify that user flows do not inadvertently route AVAX incorrectly across different chain IDs.
1945-1951: Review'BERA'token usage.
'BERA'references'Gas.zip'only for origin/destination, with no additional swap pairs. Confirm that no standard bridging route is needed if'BERA'is meant to be Gas.zip-exclusive.
2024-2025: Evaluate'ETH'bridging references for chain 534352.Similar to prior comments, having
'Gas.zip'plus'RFQ.ETH'might require ensuring correct bridging logic. Thoroughly test both in a staging environment.packages/sdk-router/src/gaszip/api.ts (4)
18-32: Validate chain data inChainsinterface.
gas,gwei, andbalare typed asstring. If you rely on numeric comparisons, ensure they are properly parsed downstream. Also confirm thatpriceremains accurate over time.
34-43: Check shape ofCalldataQuoteResponse.This interface looks good, but confirm that additional fields (e.g., error messages) from the Gas.Zip API are not missed if the backend evolves. Mismatched fields could cause parse errors.
68-79: Add caching or rate-limiting forgetChainIds.Repeated calls to the same endpoint can introduce unnecessary overhead. You might consider caching these chain IDs or controlling request frequency.
[performance]
81-108: ProtectgetGasZipQuotefrom invalidamountinputs.
BigNumber.fromcan throw ifamountis not a valid numeric string. Validate or sanitizeamountbefore constructing the request to avert run-time exceptions.packages/synapse-interface/components/_Transaction/helpers/useTxRefundStatus.ts (3)
22-25: EnsureTransactionStatusDatais consistent with GasZip API.The property
depositmight differ from the shapes used in the router or the GasZip backend. Confirm that the deposit schema is stable and recognized across all usage points.
55-60: Verify new statuses inGasZipStatus.
CONFIRMED,CANCELLED,REFUNDED,OTHERshould map precisely to the actual progression states in the GasZip system. Re-check the backend docs to confirm all states are covered.
74-84: Check for collisions in GasZip deposit detection.This logic checks
routerAddressagainst a single deposit address. Confirm no other deposit addresses exist or that this isn’t environment-specific, which could cause false positives or missed matches.packages/sdk-router/src/gaszip/gasZipModuleSet.ts (4)
21-33: Clean approach to storing modules and providers.Defining
modulesandprovidersas dictionaries keyed by chain ID cleanly ties each chain to its own logic and provider. This design is straightforward and enhances maintainability.
34-43: Validate the constructor's error-handling strategy.While the constructor correctly initializes
modulesandproviders, there is no handling for potential provider creation failures or invalid chain IDs. Please ensure that unexpected states are surfaced or handled gracefully, e.g., by throwing descriptive errors or defaulting to safe fallbacks.
138-150: Confirm that zero deadlines are intended.
getDefaultPeriods()returns zero for origin and destination periods. If there's ever a need to implement transaction timeouts or deadlines in gas.zip flows, this method might need to be updated. Confirm that this is indeed the desired behavior.
152-164: Slippage passing is straightforward.
applySlippagesimply returns the provided queries. This is consistent with the documentation stating that slippage is not supported by gas.zip. If future changes introduce partial slippage support, be sure to update this method accordingly.
| const checkGasZipTxStatus = async (txId: string): Promise<GasZipStatus> => { | ||
| try { | ||
| const res = await fetch(`${GAS_ZIP_API_URL}/${txId}`, { method: 'GET' }) | ||
| const data = (await res.json()) as TransactionStatusData | ||
| if (!data.txs || !data.txs.length) { | ||
| return GasZipStatus.OTHER | ||
| } | ||
| if ( | ||
| data.txs[0].status === GasZipStatus.CANCELLED || | ||
| data.txs[0].cancelled | ||
| ) { | ||
| // Check if there is a refund tx in the list | ||
| return data.txs.find((tx) => tx.refund) | ||
| ? GasZipStatus.REFUNDED | ||
| : GasZipStatus.CANCELLED | ||
| } | ||
| if (data.txs[0].status === GasZipStatus.CONFIRMED) { | ||
| return GasZipStatus.CONFIRMED | ||
| } | ||
| return GasZipStatus.OTHER | ||
| } catch (error) { | ||
| throw new Error(error) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
checkGasZipTxStatus logic assumes first transaction is the latest.
Using data.txs[0] for final status may be risky if the array is not guaranteed to be sorted. You might want to sort by time or clarify if the backend always returns the latest transaction first.
| public async getBridgeRoutes( | ||
| originChainId: number, | ||
| destChainId: number, | ||
| tokenIn: string, | ||
| tokenOut: string, | ||
| amountIn: BigintIsh, | ||
| originUserAddress?: string | ||
| ): Promise<BridgeRoute[]> { | ||
| // Check that both chains are supported by gas.zip | ||
| const supportedChainIds = await this.getChainIds() | ||
| if ( | ||
| !supportedChainIds.includes(originChainId) || | ||
| !supportedChainIds.includes(destChainId) | ||
| ) { | ||
| return [] | ||
| } | ||
| // Check that both tokens are native assets | ||
| if (!isNativeToken(tokenIn) || !isNativeToken(tokenOut)) { | ||
| return [] | ||
| } | ||
| const user = originUserAddress ?? AddressZero | ||
| const quote = await getGasZipQuote( | ||
| originChainId, | ||
| destChainId, | ||
| amountIn, | ||
| user, | ||
| user | ||
| ) | ||
| // Check that non-zero amount is returned | ||
| if (quote.amountOut.eq(Zero)) { | ||
| return [] | ||
| } | ||
| // Save user address in the origin query raw params | ||
| const originQuery = createNoSwapQuery(tokenIn, BigNumber.from(amountIn)) | ||
| originQuery.rawParams = quote.calldata | ||
| const destQuery = createNoSwapQuery(tokenOut, quote.amountOut) | ||
| destQuery.rawParams = user | ||
| const route: BridgeRoute = { | ||
| originChainId, | ||
| destChainId, | ||
| originQuery, | ||
| destQuery, | ||
| bridgeToken: { | ||
| symbol: 'NATIVE', | ||
| token: tokenIn, | ||
| }, | ||
| bridgeModuleName: this.bridgeModuleName, | ||
| } | ||
| return [route] | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure robust error handling for the gas.zip quote.
In getBridgeRoutes, if getGasZipQuote fails or rejects, it could cause unhandled promise rejections. Consider wrapping getGasZipQuote in a try/catch or validating the returned data so that the method gracefully handles error cases and avoids returning partial or inconsistent route information.
| private async getChainIds(): Promise<number[]> { | ||
| if (this.cachedChainIds.length === 0) { | ||
| this.cachedChainIds = await getChainIds() | ||
| } | ||
| return this.cachedChainIds | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Mitigate potential concurrency calls to getChainIds.
getChainIds() checks for an empty cachedChainIds array and then fetches chain IDs from getChainIds() only once. However, if this method is called multiple times concurrently before the first fetch completes, multiple requests could be triggered. Cache the in-flight promise to ensure only one request is made:
private cachedChainIds: number[] = []
+private chainIdsPromise: Promise<number[]> | null = null
private async getChainIds(): Promise<number[]> {
- if (this.cachedChainIds.length === 0) {
- this.cachedChainIds = await getChainIds()
+ if (!this.chainIdsPromise) {
+ this.chainIdsPromise = getChainIds().then(ids => {
+ this.cachedChainIds = ids
+ return ids
+ })
}
- return this.cachedChainIds
+ return this.chainIdsPromise
}📝 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.
| private async getChainIds(): Promise<number[]> { | |
| if (this.cachedChainIds.length === 0) { | |
| this.cachedChainIds = await getChainIds() | |
| } | |
| return this.cachedChainIds | |
| } | |
| } | |
| private cachedChainIds: number[] = [] | |
| private chainIdsPromise: Promise<number[]> | null = null | |
| private async getChainIds(): Promise<number[]> { | |
| if (!this.chainIdsPromise) { | |
| this.chainIdsPromise = getChainIds().then(ids => { | |
| this.cachedChainIds = ids | |
| return ids | |
| }) | |
| } | |
| return this.chainIdsPromise | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/synapse-interface/CHANGELOG.md (1)
6-9: Ensure Consistent Changelog Header LevelsThe new version entry for 0.42.0 on line 6 uses a single “#” header. For a well-structured changelog, the top-level title ("Change Log") should be level 1, while all version entries should be level 2 (i.e. using “##”). This change will ensure that subsequent subsections (such as “### Features” on line 9) increment by one level only, which complies with markdownlint’s expectations (MD001: heading-increment). Please update line 6 by replacing “#” with “##” to maintain the proper header hierarchy.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
9-9: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3(MD001, heading-increment)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/rest-api/CHANGELOG.md(1 hunks)packages/rest-api/package.json(2 hunks)packages/sdk-router/CHANGELOG.md(1 hunks)packages/sdk-router/package.json(1 hunks)packages/synapse-interface/CHANGELOG.md(1 hunks)packages/synapse-interface/package.json(2 hunks)packages/widget/CHANGELOG.md(1 hunks)packages/widget/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/synapse-interface/package.json
- packages/sdk-router/package.json
- packages/rest-api/package.json
- packages/widget/CHANGELOG.md
- packages/widget/package.json
- packages/rest-api/CHANGELOG.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
packages/synapse-interface/CHANGELOG.md
9-9: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
packages/sdk-router/CHANGELOG.md (2)
6-12: New Version Entry for v0.12.0 Added
The new section for version 0.12.0 introduces the Gas.zip module feature ([SYN-37]) and includes proper reference links as well as a clear release date (2025-02-27). The heading format and commit reference (34db356) maintain consistency with our changelog guidelines.
17-23: Bug Fix Documentation for v0.11.11
The updated entry for version 0.11.11 documents the bug fix for fee calculation ([SYN-49]), complete with the corresponding issue ([#3533]) and commit link (5b49675). This change is clearly presented, and the format is in line with previous entries. Consider verifying that this update aligns with the technical details of the fix elsewhere in the project.
Description
Summary by CodeRabbit