-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[goreleaser] Adding new rfq deployments to explorer #2909
Conversation
WalkthroughThe recent updates introduce the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Need clarification on asset support before merging |
Bundle ReportChanges will decrease total bundle size by 5.87MB ⬇️
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2909 +/- ##
===================================================
+ Coverage 25.38240% 25.39958% +0.01717%
===================================================
Files 785 785
Lines 56551 56560 +9
Branches 80 80
===================================================
+ Hits 14354 14366 +12
+ Misses 40717 40715 -2
+ Partials 1480 1479 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
Added support for new RFQ deployments, including the 'Linea' chain, across various components and services.
- ChainChart Update: Added 'linea' data key to
packages/explorer-ui/components/ChainChart/index.tsx
. - GraphQL Queries: Included 'linea' in
DAILY_STATISTICS_BY_CHAIN
inpackages/explorer-ui/graphql/queries/index.ts
. - Chain Constants: Added 'linea' and 'blast' chain IDs and contract addresses in
packages/synapse-constants/constants/chains/index.ts
. - Explorer Service: Updated GraphQL client and server to handle 'linea' chain in
services/explorer/graphql/client/client.go
andservices/explorer/graphql/server/graph/schema/types.graphql
. - Token Mappings: Added 'SPEC' token mappings and updated USDC/ETH for 'linea' in
packages/synapse-constants/constants/tokens/bridgeMap.ts
andbridgeable.ts
.
18 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (2)
packages/synapse-constants/constants/assets/chains/linea.svg
is excluded by!**/*.svg
packages/synapse-constants/constants/assets/explorer/linea.svg
is excluded by!**/*.svg
Files selected for processing (20)
- packages/explorer-ui/components/ChainChart/index.tsx (1 hunks)
- packages/explorer-ui/graphql/queries/index.ts (1 hunks)
- packages/synapse-constants/constants/assets/chains/index.ts (1 hunks)
- packages/synapse-constants/constants/assets/explorer/index.ts (1 hunks)
- packages/synapse-constants/constants/bridgeMap.ts (2 hunks)
- packages/synapse-constants/constants/chains/index.ts (2 hunks)
- packages/synapse-constants/constants/chains/master.ts (3 hunks)
- packages/synapse-constants/constants/tokens/bridgeMap.ts (2 hunks)
- packages/synapse-constants/constants/tokens/bridgeable.ts (3 hunks)
- packages/synapse-constants/constants/tokens/deprecated.ts (1 hunks)
- packages/synapse-constants/constants/tokens/swapMaster.ts (1 hunks)
- packages/synapse-constants/package.json (1 hunks)
- services/explorer/api/server_test.go (1 hunks)
- services/explorer/graphql/client/client.go (2 hunks)
- services/explorer/graphql/client/queries/queries.graphql (1 hunks)
- services/explorer/graphql/server/graph/model/models_gen.go (1 hunks)
- services/explorer/graphql/server/graph/partials.go (6 hunks)
- services/explorer/graphql/server/graph/resolver/server.go (6 hunks)
- services/explorer/graphql/server/graph/schema/types.graphql (1 hunks)
- services/explorer/static/chainIDs.yaml (1 hunks)
Files skipped from review due to trivial changes (6)
- packages/synapse-constants/constants/assets/chains/index.ts
- packages/synapse-constants/constants/assets/explorer/index.ts
- packages/synapse-constants/constants/tokens/deprecated.ts
- packages/synapse-constants/constants/tokens/swapMaster.ts
- packages/synapse-constants/package.json
- services/explorer/static/chainIDs.yaml
Additional comments not posted (33)
packages/explorer-ui/graphql/queries/index.ts (1)
213-213
: LGTM! But verify the usage of the new fieldlinea
.The code changes are approved.
However, ensure that the new field
linea
is correctly integrated and used in the application.services/explorer/graphql/server/graph/schema/types.graphql (1)
182-182
: LGTM! But verify the usage of the new fieldlinea
.The code changes are approved.
However, ensure that the new field
linea
is correctly integrated and used in the application.packages/synapse-constants/constants/chains/index.ts (2)
78-78
: LGTM! But verify the usage of the new chain identifierLINEA
.The code changes are approved.
However, ensure that the new chain identifier
LINEA
is correctly integrated and used in the application.Verification successful
The new chain identifier
LINEA
is correctly integrated and used in the codebase.The identifier is found in multiple files and contexts, indicating proper usage and integration:
packages/synapse-constants/constants/chains/master.ts
: Defines theLINEA
chain properties.packages/synapse-constants/constants/chains/index.ts
: Associates theLINEA
chain ID.packages/synapse-constants/constants/tokens/bridgeable.ts
: Configures bridgeable tokens for theLINEA
chain.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new chain identifier `LINEA` in the codebase. # Test: Search for the usage of the `LINEA` chain identifier in the codebase. Expect: Correct integration and usage. rg --type js -A 5 $'LINEA'Length of output: 25
Script:
#!/bin/bash # Description: Verify the usage of the new chain identifier `LINEA` in the codebase. # Test: Search for the usage of the `LINEA` chain identifier in the codebase across all file types. Expect: Correct integration and usage. rg -A 5 'LINEA'Length of output: 3095
128-130
: LGTM! But verify the usage of the new entries forLINEA
andBLAST
.The code changes are approved.
However, ensure that the new entries for
LINEA
andBLAST
are correctly integrated and used in the application.services/explorer/api/server_test.go (2)
60-60
: LGTM!The addition of the new field
Linea
is correctly integrated into the struct.
60-60
: Verify the new field integration.Ensure that the new field
Linea
is correctly integrated and used in the test.Verification successful
Verify the new field integration.
The new field
Linea
is correctly integrated and used in the following files:
services/explorer/graphql/client/client.go
services/explorer/graphql/server/graph/model/models_gen.go
services/explorer/graphql/server/graph/resolver/server.go
Please review these files to ensure the
Linea
field is used appropriately.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of the new field `Linea` in the codebase. # Test: Search for the usage of the new field `Linea`. Expect: Occurrences of the new field in relevant files. rg --type go --pattern 'Linea' --glob '!*.test.go'Length of output: 462
Script:
#!/bin/bash # Description: Verify the integration of the new field `Linea` in the codebase. # Test: Search for the usage of the new field `Linea`. Expect: Occurrences of the new field in relevant files. rg 'Linea' --type go --glob '!*.test.go'Length of output: 755
services/explorer/graphql/client/queries/queries.graphql (2)
157-157
: LGTM!The addition of the new field
linea
is correctly integrated into the query.
157-157
: Verify the new field integration.Ensure that the new field
linea
is correctly integrated and used in the query.Verification successful
Verify the new field integration.
The new field
linea
is correctly integrated in the following locations:
services/explorer/graphql/client/queries/queries.graphql
services/explorer/graphql/server/graph/schema/types.graphql
Ensure that the new field
linea
is correctly integrated and used in the query.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of the new field `linea` in the codebase. # Test: Search for the usage of the new field `linea`. Expect: Occurrences of the new field in relevant files. rg --type graphql --pattern 'linea' --glob '!*.test.graphql'Length of output: 472
Script:
#!/bin/bash # Description: Verify the integration of the new field `linea` in the codebase. # Test: Search for the usage of the new field `linea`. Expect: Occurrences of the new field in relevant files. rg 'linea' --type graphql --glob '!*.test.graphql'Length of output: 82
Script:
#!/bin/bash # Description: Verify the integration of the new field `linea` in the codebase. # Test: Search for the usage of the new field `linea`. Expect: Occurrences of the new field in relevant files. rg 'linea' --glob '*.graphql' --glob '!*.test.graphql'Length of output: 196
packages/explorer-ui/components/ChainChart/index.tsx (2)
317-322
: LGTM!The addition of the new
<Bar>
component withdataKey="linea"
is correctly integrated into the chart.
317-322
: Verify the new component integration.Ensure that the new
<Bar>
component withdataKey="linea"
is correctly integrated and used in the chart.Verification successful
The new
<Bar>
component withdataKey="linea"
is correctly integrated and does not conflict with otherdataKey
attributes in the file.
- The
dataKey="linea"
is added alongside other blockchain names.- There are no conflicts with existing
dataKey
attributes.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of the new `<Bar>` component with `dataKey="linea"` in the codebase. # Test: Search for the usage of the new dataKey `linea`. Expect: Occurrences of the new dataKey in relevant files. rg --type tsx --pattern 'dataKey="linea"'Length of output: 455
Script:
#!/bin/bash # Description: Verify the integration of the new `<Bar>` component with `dataKey="linea"` in the codebase. # Test: Search for the usage of the new dataKey `linea`. Expect: Occurrences of the new dataKey in relevant files. rg 'dataKey="linea"' --type tsxLength of output: 61
Script:
#!/bin/bash # Description: Verify the integration of the new `<Bar>` component with `dataKey="linea"` in the codebase. # Test: Search for the usage of the new dataKey `linea`. Expect: Occurrences of the new dataKey in relevant files. rg 'dataKey="linea"' --glob '*.tsx'Length of output: 122
Script:
#!/bin/bash # Description: Verify the integration of the new `<Bar>` component with `dataKey="linea"` in the codebase. # Test: Search for other `dataKey` attributes in the same file to ensure there are no conflicts. rg 'dataKey="' packages/explorer-ui/components/ChainChart/index.tsxLength of output: 887
packages/synapse-constants/constants/chains/master.ts (2)
22-22
: LGTM! Import statements are consistent.The import statements for
lineaImg
andlineaExplorerImg
are consistent with the existing pattern.Also applies to: 44-44
484-506
: LGTM! TheLINEA
entity is well-defined.The
LINEA
entity follows the existing pattern and includes all necessary properties for the new blockchain.services/explorer/graphql/server/graph/model/models_gen.go (1)
106-106
: LGTM! The new fieldLinea
is well-integrated.The new field
Linea
in theDateResultByChain
struct is consistent with the existing fields and enhances the struct's capability.services/explorer/graphql/client/client.go (2)
131-131
: LGTM! The new fieldLinea
is well-integrated.The new field
Linea
in theGetDailyStatisticsByChain
struct is consistent with the existing fields and enhances the struct's capability.
504-504
: LGTM! The query document is updated correctly.The update to include the
linea
field in theGetDailyStatisticsByChainDocument
query selection set is consistent with the existing pattern.services/explorer/graphql/server/graph/partials.go (6)
403-403
: LGTM! Addition oflinea
column is consistent.The addition of
results[59144] AS linea
is consistent with the existing columns and does not affect the query structure.
447-447
: LGTM! Addition oflinea
column is consistent.The addition of
results[59144] AS linea
is consistent with the existing columns and does not affect the query structure.
543-543
: LGTM! Addition oflinea
column is consistent.The addition of
results[59144] AS linea
is consistent with the existing columns and does not affect the query structure.
645-645
: LGTM! Addition oflinea
column is consistent.The addition of
results[59144] AS linea
is consistent with the existing columns and does not affect the query structure.
676-676
: LGTM! Addition oflinea
column is consistent.The addition of
results[59144] AS linea
is consistent with the existing columns and does not affect the query structure.
708-708
: LGTM! Addition oflinea
column is consistent.The addition of
results[59144] AS linea
is consistent with the existing columns and does not affect the query structure.packages/synapse-constants/constants/tokens/bridgeable.ts (2)
415-415
: LGTM! Addition ofLINEA
chain support forUSDC
.The addition of
CHAINS.LINEA.id
in both theaddresses
anddecimals
objects is consistent with other chain entries and enhances compatibility with theLINEA
blockchain.Also applies to: 432-432
790-790
: LGTM! Addition ofLINEA
chain support forETH
.The addition of
CHAINS.LINEA.id
in theaddresses
object is consistent with other chain entries and enhances compatibility with theLINEA
blockchain.packages/synapse-constants/constants/bridgeMap.ts (2)
170-176
: New entry for0xAdF7C35560035944e805D98fF17d58CDe2449389
looks good.The properties
decimals
,symbol
,origin
,destination
, andswappable
are correctly defined and consistent with other entries.
1243-1249
: New entry for0x96419929d7949D6A801A6909c145C8EEf6A40431
looks good.The properties
decimals
,symbol
,origin
,destination
, andswappable
are correctly defined and consistent with other entries.packages/synapse-constants/constants/tokens/bridgeMap.ts (2)
170-176
: LGTM!The new token mapping for the address
0xAdF7C35560035944e805D98fF17d58CDe2449389
is consistent with the existing mappings.
1243-1249
: LGTM!The new token mapping for the address
0x96419929d7949D6A801A6909c145C8EEf6A40431
is consistent with the existing mappings.services/explorer/graphql/server/graph/resolver/server.go (6)
119-119
: Addition ofLinea
field inComplexityRoot
struct looks good.The new field is consistent with the existing fields and correctly integrated.
607-613
: Addition ofDateResultByChain.linea
case inComplexity
method looks good.The new case is correctly integrated and follows the existing pattern.
1666-1666
: Addition oflinea
field inDateResultByChain
type looks good.The new field is consistent with the existing fields and correctly integrated.
4794-4833
: Addition of_DateResultByChain_linea
resolver function looks good.The new resolver function is correctly implemented and handles potential errors gracefully.
7191-7192
: Addition of field context forlinea
looks good.The new field context is correctly integrated and follows the existing pattern.
10435-10436
: Addition of_DateResultByChain_linea
in switch case looks good.The new case is correctly integrated and follows the existing pattern.
Deploying sanguine-fe with Cloudflare Pages
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
Added new RFQ deployments to the explorer, including support for the LINEA
blockchain, and updated various components and services.
- Token Support: Updated
packages/synapse-constants/constants/tokens/bridgeable.ts
to includeLINEA
blockchain support for WETH. - GraphQL Schema: Enhanced schema in
services/explorer/graphql/server/graph/schema/types.graphql
to includelinea
field. - Explorer UI: Modified
packages/explorer-ui/graphql/queries/index.ts
to addlinea
field in daily statistics query. - Chain Constants: Added
LINEA
chain ID and contract addresses inpackages/synapse-constants/constants/chains/index.ts
. - Version Update: Bumped
synapse-constants
package version to 1.3.21.
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- packages/synapse-constants/constants/tokens/bridgeable.ts (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/synapse-constants/constants/tokens/bridgeable.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
Added new RFQ deployments to the explorer, including support for the SCROLL blockchain and version updates.
- Token Support: Updated
packages/synapse-constants/constants/tokens/bridgeable.ts
to include SCROLL blockchain support for USDT. - Version Update: Incremented
synapse-constants
package version to 1.3.22 inpackages/synapse-constants/package.json
. - GraphQL Schema: Enhanced schema in
services/explorer/graphql/server/graph/schema/types.graphql
to includelinea
field. - Explorer UI: Modified
packages/explorer-ui/graphql/queries/index.ts
to addlinea
field in daily statistics query. - Chain Constants: Added
LINEA
chain ID and contract addresses inpackages/synapse-constants/constants/chains/index.ts
.
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- packages/synapse-constants/constants/tokens/bridgeable.ts (5 hunks)
- packages/synapse-constants/package.json (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- packages/synapse-constants/constants/tokens/bridgeable.ts
- packages/synapse-constants/package.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
The latest changes focus on adding new RFQ deployments and integrating the LINEA blockchain into the explorer.
- Token Removal: Removed LINEA blockchain entry for WETH in
packages/synapse-constants/constants/tokens/bridgeable.ts
. - GraphQL Schema: Enhanced schema in
services/explorer/graphql/server/graph/schema/types.graphql
to includelinea
field. - Explorer UI: Modified
packages/explorer-ui/pages/chain/[chainId].tsx
to support new RFQ deployments and LINEA blockchain. - GraphQL Query: Updated
services/explorer/graphql/client/queries/queries.graphql
to includelinea
field in daily statistics query. - Version Update: Incremented
synapse-constants
package version to 1.3.22 inpackages/synapse-constants/package.json
.
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- packages/synapse-constants/constants/tokens/bridgeable.ts (4 hunks)
Additional comments not posted (4)
packages/synapse-constants/constants/tokens/bridgeable.ts (4)
415-415
: Add the new address entry forUSDC
on theLINEA
chain.The address for
USDC
on theLINEA
chain is correctly added.
432-432
: Add the new decimal entry forUSDC
on theLINEA
chain.The decimal value for
USDC
on theLINEA
chain is correctly added.
492-492
: Add the new address entry forUSDT
on theSCROLL
chain.The address for
USDT
on theSCROLL
chain is correctly added.
791-791
: Add the new address entry forETH
on theLINEA
chain.The address for
ETH
on theLINEA
chain is correctly added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requesting a few changes, other than that LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Outside diff range, codebase verification and nitpick comments (3)
packages/synapse-constants/scripts/utils/fetchRfqData.js (1)
1-2
: Consider using a configuration file for the RFQ URL.Hardcoding URLs can make maintenance difficult. Using a configuration file or environment variables would be more flexible.
packages/synapse-constants/scripts/generateMaps.js (2)
462-463
: Add a docstring to thegetRFQSymbol
function.Adding a docstring can help in understanding the purpose and usage of the function.
/** * Generates an RFQ symbol by prefixing the given symbol with 'RFQ.'. * @param {string} symbol - The original token symbol. * @returns {string} The RFQ symbol. */ const getRFQSymbol = (symbol) => { return `RFQ.${symbol}` }
4-4
: The RFQ URL is incorrect or the endpoint is down.The URL
https://rfq-api.omnirpc.io/quotes
returned an HTTP 404 status, indicating that it is not found. Please verify the correct URL or check the endpoint's availability.
packages/synapse-constants/scripts/generateMaps.js
, line 4Analysis chain
Verify the RFQ URL.
Ensure that the RFQ URL is correct and accessible.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the RFQ URL is correct and accessible. # Test: Check if the URL is accessible. Expect: HTTP 200 OK. curl -I https://rfq-api.omnirpc.io/quotesLength of output: 790
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (6)
- packages/synapse-constants/constants/bridgeMap.ts (38 hunks)
- packages/synapse-constants/constants/tokens/bridgeMap.ts (39 hunks)
- packages/synapse-constants/constants/tokens/bridgeable.ts (5 hunks)
- packages/synapse-constants/scripts/generateMaps.js (9 hunks)
- packages/synapse-constants/scripts/utils/fetchRfqData.js (1 hunks)
- packages/synapse-constants/scripts/utils/prettyPrintTs.js (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- packages/synapse-constants/constants/bridgeMap.ts
- packages/synapse-constants/constants/tokens/bridgeMap.ts
- packages/synapse-constants/constants/tokens/bridgeable.ts
Additional comments not posted (1)
packages/synapse-constants/scripts/generateMaps.js (1)
43-43
: Verify the list of allowed chain IDs for RFQ.Ensure that the list of allowed chain IDs for RFQ is complete and accurate.
const response = await fetch(RFQ_URL) | ||
if (!response.ok) { | ||
throw new Error(`HTTP error! status: ${response.status}`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Log the response status for better debugging.
Logging the response status can help in diagnosing issues with the API call.
- if (!response.ok) {
+ if (!response.ok) {
+ console.error(`HTTP error! status: ${response.status}`)
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.
const response = await fetch(RFQ_URL) | |
if (!response.ok) { | |
throw new Error(`HTTP error! status: ${response.status}`) | |
const response = await fetch(RFQ_URL) | |
if (!response.ok) { | |
console.error(`HTTP error! status: ${response.status}`) | |
throw new Error(`HTTP error! status: ${response.status}`) |
} catch (error) { | ||
console.error('Failed to fetch RFQ data:', error) | ||
return [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a more descriptive error message.
Including the URL in the error message can help in identifying the source of the error.
- console.error('Failed to fetch RFQ data:', error)
+ console.error(`Failed to fetch RFQ data from ${RFQ_URL}:`, error)
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.
} catch (error) { | |
console.error('Failed to fetch RFQ data:', error) | |
return [] | |
} catch (error) { | |
console.error(`Failed to fetch RFQ data from ${RFQ_URL}:`, error) | |
return [] |
const json = JSON.stringify(map) | ||
fs.writeFileSync(fn, `export const ${mapName} = ${json}`) | ||
// Run prettier on the file using terminal command: | ||
execSync(`npx prettier --write ${fn}`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling for the Prettier command.
Currently, there is no error handling for the execSync
operation. Consider adding a try-catch block to handle potential errors.
- execSync(`npx prettier --write ${fn}`)
+ try {
+ execSync(`npx prettier --write ${fn}`)
+ } catch (error) {
+ console.error(`Failed to format file ${fn} with Prettier:`, error)
+ }
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.
execSync(`npx prettier --write ${fn}`) | |
try { | |
execSync(`npx prettier --write ${fn}`) | |
} catch (error) { | |
console.error(`Failed to format file ${fn} with Prettier:`, error) | |
} |
const prettyPrintTS = (map, mapName, fn) => { | ||
console.log(`Writing ${mapName} to ${fn}`) | ||
const json = JSON.stringify(map) | ||
fs.writeFileSync(fn, `export const ${mapName} = ${json}`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling for file write operations.
Currently, there is no error handling for the fs.writeFileSync
operation. Consider adding a try-catch block to handle potential errors.
- fs.writeFileSync(fn, `export const ${mapName} = ${json}`)
+ try {
+ fs.writeFileSync(fn, `export const ${mapName} = ${json}`)
+ } catch (error) {
+ console.error(`Failed to write to file ${fn}:`, error)
+ }
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.
fs.writeFileSync(fn, `export const ${mapName} = ${json}`) | |
try { | |
fs.writeFileSync(fn, `export const ${mapName} = ${json}`) | |
} catch (error) { | |
console.error(`Failed to write to file ${fn}:`, error) | |
} |
const printMaps = async () => { | ||
const bridgeMap = {} | ||
const bridgeSymbolsMap = {} | ||
console.log('Starting on chains: ', Object.keys(providers)) | ||
|
||
const rfqResponse = await fetchRfqData() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling for the RFQ data fetch operation.
Currently, there is no error handling for the fetchRfqData
operation. Consider adding a try-catch block to handle potential errors.
- const rfqResponse = await fetchRfqData()
+ let rfqResponse = []
+ try {
+ rfqResponse = await fetchRfqData()
+ } catch (error) {
+ console.error('Failed to fetch RFQ data:', error)
+ }
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.
const rfqResponse = await fetchRfqData() | |
let rfqResponse = [] | |
try { | |
rfqResponse = await fetchRfqData() | |
} catch (error) { | |
console.error('Failed to fetch RFQ data:', error) | |
} |
if (allowedChainIdsForRfq.includes(Number(chainId))) { | ||
Object.keys(originMap).forEach((token) => { | ||
if (originMap[token].has('nETH')) { | ||
originMap[token].add('RFQ.ETH') | ||
} | ||
if (originMap[token].has('CCTP.USDC')) { | ||
originMap[token].add('RFQ.USDC') | ||
} | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add logging for RFQ data integration.
Adding logging can help in diagnosing issues with the RFQ data integration.
- if (allowedChainIdsForRfq.includes(Number(chainId))) {
- Object.keys(originMap).forEach((token) => {
- if (originMap[token].has('nETH')) {
- originMap[token].add('RFQ.ETH')
- }
- if (originMap[token].has('CCTP.USDC')) {
- originMap[token].add('RFQ.USDC')
- }
- })
+ if (allowedChainIdsForRfq.includes(Number(chainId))) {
+ console.log(`Integrating RFQ data for chain ID: ${chainId}`)
+ Object.keys(originMap).forEach((token) => {
+ if (originMap[token].has('nETH')) {
+ originMap[token].add('RFQ.ETH')
+ console.log(`Added RFQ.ETH to token: ${token}`)
+ }
+ if (originMap[token].has('CCTP.USDC')) {
+ originMap[token].add('RFQ.USDC')
+ console.log(`Added RFQ.USDC to token: ${token}`)
+ }
+ })
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.
if (allowedChainIdsForRfq.includes(Number(chainId))) { | |
Object.keys(originMap).forEach((token) => { | |
if (originMap[token].has('nETH')) { | |
originMap[token].add('RFQ.ETH') | |
} | |
if (originMap[token].has('CCTP.USDC')) { | |
originMap[token].add('RFQ.USDC') | |
} | |
}) | |
} | |
if (allowedChainIdsForRfq.includes(Number(chainId))) { | |
console.log(`Integrating RFQ data for chain ID: ${chainId}`) | |
Object.keys(originMap).forEach((token) => { | |
if (originMap[token].has('nETH')) { | |
originMap[token].add('RFQ.ETH') | |
console.log(`Added RFQ.ETH to token: ${token}`) | |
} | |
if (originMap[token].has('CCTP.USDC')) { | |
originMap[token].add('RFQ.USDC') | |
console.log(`Added RFQ.USDC to token: ${token}`) | |
} | |
}) | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
(updates since last review)
The latest changes focus on adding new RFQ deployments and integrating the LINEA blockchain into the explorer.
- Token Mappings: Updated
packages/synapse-constants/constants/bridgeMap.ts
andpackages/synapse-constants/constants/tokens/bridgeMap.ts
to include new RFQ token mappings and support for the LINEA blockchain. - SCROLL Blockchain: Added SCROLL blockchain support for
USDT
inpackages/synapse-constants/constants/tokens/bridgeable.ts
. - ABI File Relocation: Moved several ABI files to
packages/synapse-constants/scripts/abi/
for better organization. - RFQ Data Fetching: Added
fetchRfqData.js
inpackages/synapse-constants/scripts/utils
to fetch RFQ data. - GraphQL Enhancements: Enhanced GraphQL schema and queries to include
linea
field for daily statistics.
16 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings
const json = JSON.stringify(map) | ||
fs.writeFileSync(fn, `export const ${mapName} = ${json}`) | ||
// Run prettier on the file using terminal command: | ||
execSync(`npx prettier --write ${fn}`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style: Consider handling errors for the execSync
call to prevent the script from crashing if Prettier fails.
const prettyPrintTS = (map, mapName, fn) => { | ||
console.log(`Writing ${mapName} to ${fn}`) | ||
const json = JSON.stringify(map) | ||
fs.writeFileSync(fn, `export const ${mapName} = ${json}`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic: Ensure the mapName
and fn
parameters are sanitized to avoid potential security risks from command injection.
Adding new rfq deployments to explorer and the following areas:
0f57507: explorer-ui preview link
Summary by CodeRabbit
New Features
<Bar>
component in theChainChart
for enhanced data representation.linea
field to the GraphQL query for daily statistics, providing additional metrics.LINEA
blockchain, including new asset and token mappings forUSDC
,USDT
, andETH
.LINEA
blockchain in the constants and mapping files.linea
in theDateResultByChain
type for more detailed data representation.Chores
synapse-constants
package to 1.3.22.04741ef: explorer-ui preview link
c02b4a3: explorer-ui preview link
7438b20: explorer-ui preview link
abb2c29: explorer-ui preview link