-
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
fe-release 12/8 #3445
fe-release 12/8 #3445
Conversation
- @synapsecns/[email protected]
* gitattribute foundry * vendored->generated --------- Co-authored-by: Trajan0x <[email protected]>
* chore: move mocks to their respecitve directory * chore: update imports * chore: also move UniversalToken tests * build: include harnesses and mocks in the flatten command
- @synapsecns/[email protected]
feat(docs): Add SYN to docs
- @synapsecns/[email protected]
feat(synapse-interface): Link to docs
- @synapsecns/[email protected]
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/synapse-interface/components/layouts/LandingPageWrapper/index.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Failed to load parser '@babel/eslint-parser' declared in 'packages/synapse-interface/.eslintrc.js': Cannot find module '@babel/core/package.json'
WalkthroughThe pull request includes updates to various files primarily focused on refining file type handling in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
|
- @synapsecns/[email protected]
Deploying sanguine-fe with Cloudflare Pages
|
Bundle ReportChanges will decrease total bundle size by 3.12MB (-8.74%) ⬇️. This is within the configured threshold ✅ Detailed changes
ℹ️ *Bundle size includes cached data from a previous commit |
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: 4
🧹 Outside diff range and nitpick comments (7)
packages/synapse-interface/pages/_app.tsx (1)
Line range hint
64-64
: Move Google Analytics ID to environment variablesThe Google Analytics ID is currently hardcoded. This should be moved to environment variables for better security and environment-specific configuration.
Consider this change:
- <GoogleAnalytics gaId={'G-BBC13LQXBD'} /> + <GoogleAnalytics gaId={process.env.NEXT_PUBLIC_GA_ID} />Don't forget to:
- Add this variable to your environment configuration
- Update deployment documentation
- Add the variable to CI/CD configuration
packages/contracts-rfq/package.json (1)
Line range hint
18-18
: Empty build script should be addressed.The build script is currently set to
" "
which might cause issues with dependent workflows or CI/CD pipelines.Please either:
- Remove the empty build script if it's not needed
- Implement the required build steps
- Add a comment explaining why it's intentionally empty
packages/synapse-interface/components/toast/index.tsx (1)
Line range hint
6-32
: Consider making toast duration configurableThe hard-coded 10-second duration (
duration: 10000
) might not be suitable for all notifications. Error messages might need to stay longer, while success messages could be shorter.Consider making the duration configurable based on the toast type:
- duration: 10000, + duration: (t) => { + switch(t.type) { + case 'error': return 15000; + case 'success': return 5000; + default: return 10000; + } + },packages/synapse-interface/components/toast/ToastContent.tsx (2)
Line range hint
8-8
: Add TypeScript interface for component propsThe component props should be properly typed for better maintainability and type safety.
Add a proper interface:
interface ToastContentProps { toastData: { id: string; type?: 'success' | 'error' | 'loading'; }; icon?: React.ReactNode; message: React.ReactNode; }
Line range hint
22-54
: Enhance accessibility of toast notificationsThe toast notifications should be accessible to screen readers and keyboard users.
Add appropriate ARIA attributes and roles:
<div className={` flex rounded items-center min-w-[300px] px-2 pt-1 pb-2 bg-bgBase text-white border ${borderColor} - `} + `} + role="alert" + aria-live="polite" >Also, consider adding a proper aria-label to the dismiss button:
<button className={` rounded-full h-6 w-6 mt-1.5 focus:outline-none active:outline-none hover:bg-gray-900 text-gray-400 hover:text-gray-300 `} onClick={() => toast.dismiss(toastData.id)} + aria-label="Dismiss notification" >
packages/synapse-interface/messages/ar.json (1)
Line range hint
1-500
: Consider RTL-specific UI adjustments for Arabic translations.While the translations are accurate, consider:
- Ensuring UI elements like arrows (▼) and ellipsis (…) are properly mirrored for RTL
- Verifying that placeholder positions work correctly in RTL context
- Testing that numerical values display correctly in RTL layout
docs/bridge/docs/01-About/04-SYN.md (1)
11-21
: Add blank lines around the table for better readability.According to markdown best practices, tables should be surrounded by blank lines.
Apply this diff:
Liquidity for the [$SYN](https://coinmarketcap.com/currencies/synapse-2/) token can be found here: + | Venue | Link | | -------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | Coinbase | `https://www.coinbase.com/price/synapse` [↗](https://www.coinbase.com/price/synapse) | | Sushi | `https://www.sushi.com/ethereum/pool/v2/0x4a86c01d67965f8cb3d0aaa2c655705e64097c31` [↗](https://www.sushi.com/ethereum/pool/v2/0x4a86c01d67965f8cb3d0aaa2c655705e64097c31) | | Revolut | `https://www.revolut.com/crypto/price/syn` [↗](https://www.revolut.com/crypto/price/syn) | | Binance (Spot) | `https://www.binance.com/en/trade/SYN_USDT?type=spot` [↗](https://www.binance.com/en/trade/SYN_USDT?type=spot) | | Binance (Perpetuals) | `https://www.binance.com/en/futures/SYNUSDT` [↗](https://www.binance.com/en/futures/SYNUSDT) | | Bybit (SYN/USDT) | `https://www.bybit.com/trade/usdt/SYNUSDT` [↗](https://www.bybit.com/trade/usdt/SYNUSDT) | | HTX | `https://www.htx.com/price/syn/` [↗](https://www.htx.com/price/syn/) | | Kraken | `https://www.kraken.com/prices/synapse` [↗](https://www.kraken.com/prices/synapse) | | KuCoin | `https://www.kucoin.com/price/SYN` [↗](https://www.kucoin.com/price/SYN) | +🧰 Tools
🪛 Markdownlint (0.35.0)
21-21: null
Tables should be surrounded by blank lines(MD058, blanks-around-tables)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (33)
.gitattributes
(1 hunks)docs/bridge/CHANGELOG.md
(1 hunks)docs/bridge/docs/01-About/04-SYN.md
(1 hunks)docs/bridge/docs/03-RFQ/index.md
(1 hunks)docs/bridge/docs/05-Contracts/09-SYN.md
(1 hunks)docs/bridge/package.json
(1 hunks)packages/contracts-rfq/.solhintignore
(1 hunks)packages/contracts-rfq/CHANGELOG.md
(1 hunks)packages/contracts-rfq/package.json
(2 hunks)packages/contracts-rfq/test/FastBridge.t.sol
(1 hunks)packages/contracts-rfq/test/FastBridgeV2.t.sol
(1 hunks)packages/contracts-rfq/test/harnesses/UniversalTokenLibHarness.sol
(1 hunks)packages/contracts-rfq/test/integration/MulticallTarget.t.sol
(1 hunks)packages/contracts-rfq/test/integration/TokenZapV1.t.sol
(1 hunks)packages/contracts-rfq/test/libs/UniversalTokenLib.t.sol
(1 hunks)packages/contracts-rfq/test/mocks/FastBridgeMock.sol
(1 hunks)packages/contracts-rfq/test/zaps/TokenZapV1.GasBench.t.sol
(1 hunks)packages/contracts-rfq/test/zaps/TokenZapV1.t.sol
(1 hunks)packages/synapse-interface/CHANGELOG.md
(1 hunks)packages/synapse-interface/components/layouts/LandingPageWrapper/index.tsx
(1 hunks)packages/synapse-interface/components/toast/ToastContent.tsx
(1 hunks)packages/synapse-interface/components/toast/index.tsx
(2 hunks)packages/synapse-interface/constants/routes.ts
(2 hunks)packages/synapse-interface/constants/urls/index.tsx
(1 hunks)packages/synapse-interface/messages/ar.json
(1 hunks)packages/synapse-interface/messages/en-US.json
(1 hunks)packages/synapse-interface/messages/es.json
(1 hunks)packages/synapse-interface/messages/fr.json
(1 hunks)packages/synapse-interface/messages/jp.json
(1 hunks)packages/synapse-interface/messages/tr.json
(1 hunks)packages/synapse-interface/messages/zh-CN.json
(1 hunks)packages/synapse-interface/package.json
(1 hunks)packages/synapse-interface/pages/_app.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (18)
- packages/contracts-rfq/test/harnesses/UniversalTokenLibHarness.sol
- docs/bridge/docs/05-Contracts/09-SYN.md
- packages/contracts-rfq/test/zaps/TokenZapV1.GasBench.t.sol
- packages/contracts-rfq/.solhintignore
- packages/synapse-interface/package.json
- packages/contracts-rfq/test/FastBridgeV2.t.sol
- packages/contracts-rfq/CHANGELOG.md
- docs/bridge/CHANGELOG.md
- packages/contracts-rfq/test/zaps/TokenZapV1.t.sol
- docs/bridge/package.json
- packages/synapse-interface/messages/en-US.json
- packages/contracts-rfq/test/integration/TokenZapV1.t.sol
- packages/contracts-rfq/test/integration/MulticallTarget.t.sol
- packages/contracts-rfq/test/libs/UniversalTokenLib.t.sol
- packages/contracts-rfq/test/mocks/FastBridgeMock.sol
- packages/contracts-rfq/test/FastBridge.t.sol
- docs/bridge/docs/03-RFQ/index.md
- packages/synapse-interface/CHANGELOG.md
🧰 Additional context used
🪛 Markdownlint (0.35.0)
docs/bridge/docs/01-About/04-SYN.md
21-21: null
Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🔇 Additional comments (13)
.gitattributes (2)
19-19
: LGTM! Good simplification of the linguist-generated attribute.
The simplified syntax is the preferred modern approach.
27-29
: LGTM! Verify deployment files coverage.
The pattern correctly marks Foundry deployment artifacts as generated files.
✅ Verification successful
All deployment JSON files are correctly covered by the pattern
The verification confirms that all deployment JSON files are located under the packages/**/deployments/
directory structure, and there are no deployment-related JSON files outside this pattern. The .gitattributes
rule packages/**/deployments/*.json linguist-generated
will correctly mark all these files as generated.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the coverage of deployment JSON files pattern
# Check if there are any deployment JSON files that would be covered by this rule
echo "Deployment JSON files that will be marked as generated:"
fd -e json . packages/ | grep "/deployments/"
# Verify there are no deployment files outside the specified pattern
echo -e "\nChecking for any deployment JSON files that might be missed:"
fd -e json . | grep -i "deploy" | grep -v "/deployments/"
Length of output: 2673
packages/synapse-interface/pages/_app.tsx (1)
Line range hint 57-77
: LGTM: Provider hierarchy is well-structured
The provider hierarchy is logically organized:
- WagmiProvider at the top for Web3 functionality
- Query and UI providers (RainbowKit) next
- Application-specific providers
- Analytics within the authentication context
This structure ensures proper dependency flow and context availability.
packages/contracts-rfq/package.json (1)
4-4
: Version bump looks good, please verify changelog.
The version bump from 0.14.6 to 0.14.7 is consistent with semantic versioning for a patch release.
✅ Verification successful
Version bump is correctly documented in the changelog
The changelog entry for version 0.14.7 exists and correctly indicates this is a version bump only release, which is consistent with the changes observed in the package.json file.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify changelog entry exists for version 0.14.7
# Check for changelog file and version entry
fd -t f "CHANGELOG.md" packages/contracts-rfq/ --exec grep -A 5 "## \[0.14.7\]" {}
Length of output: 295
packages/synapse-interface/constants/urls/index.tsx (1)
28-28
: LGTM: New documentation URL constant follows conventions
The addition of SYN_TOKEN_LINK
is well-structured and consistent with other documentation URLs in the file.
packages/synapse-interface/components/layouts/LandingPageWrapper/index.tsx (1)
214-216
: LGTM: Navigation item update is consistent with new documentation link
The changes to the MoreInfoItem component properly integrate the new SYN token documentation link with an appropriate description.
packages/synapse-interface/messages/zh-CN.json (2)
347-347
: LGTM: Added Chinese translation for SYN token navigation
The translation for "$SYN" maintains consistency across the interface.
Line range hint 391-396
: Verify the accuracy of security warning translations
The translations for critical security warnings about bridge exploits and paused transactions need verification to ensure they accurately convey the severity of the risks.
✅ Verification successful
Security warning translations accurately convey risks across all language files
The verification of security warning translations across all language files (ar, en-US, es, fr, jp, tr, zh-CN) shows that:
- All translations maintain consistent severity indicators ("Warning!", "Alert")
- Critical terms about exploits and risks are accurately translated
- The cautionary messages about bridge exploits (Harmony, Fantom) are properly conveyed
- Paused transaction warnings for Dogechain are clearly communicated
- Risk acknowledgment requirements are consistently emphasized
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Compare security warning translations across different language files
# to ensure consistent messaging about risks
# Check for warning messages across all language files
for lang_file in $(fd -e json . packages/synapse-interface/messages/); do
echo "=== $lang_file ==="
jq -r '.Warning | select(. != null)' "$lang_file"
done
Length of output: 7023
packages/synapse-interface/messages/jp.json (1)
Line range hint 1-500
: LGTM! Translations are consistent and well-structured.
The Japanese translations:
- Maintain proper placeholder variables (e.g., {chainName}, {symbol})
- Preserve the meaning while being culturally appropriate
- Include all security warnings with proper emphasis
packages/synapse-interface/messages/tr.json (1)
Line range hint 1-500
: LGTM! Translations are consistent and well-structured.
The Turkish translations:
- Maintain proper placeholder variables (e.g., {chainName}, {symbol})
- Preserve the meaning while being culturally appropriate
- Include all security warnings with proper emphasis
packages/synapse-interface/messages/es.json (1)
347-347
: LGTM! Consistent token branding.
The $SYN token name is correctly kept untranslated, maintaining brand consistency across languages.
packages/synapse-interface/messages/fr.json (1)
347-347
: LGTM! Consistent token branding.
The $SYN token name is correctly kept untranslated, maintaining brand consistency across languages.
docs/bridge/docs/01-About/04-SYN.md (1)
13-21
: Verify the external URLs for security.
Please verify that all exchange URLs are correct and point to legitimate platforms to prevent potential phishing risks.
Run the following script to verify the URLs:
🧰 Tools
🪛 Markdownlint (0.35.0)
21-21: null
Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
{/* @ts-ignore */} | ||
<WagmiProvider config={wagmiConfig}> | ||
<QueryClientProvider client={queryClient}> |
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.
Remove @ts-ignore and fix type issues properly
The @ts-ignore
comment suggests underlying type compatibility issues with the WagmiProvider configuration. This could lead to runtime errors and makes the code less maintainable.
Please investigate and fix the type issues instead of suppressing them. This might involve:
- Updating the wagmi configuration types
- Ensuring proper type exports from wagmiConfig
- Updating the wagmi package if needed
@@ -26,7 +26,7 @@ | |||
"lint": "forge fmt && npm run solhint", | |||
"lint:check": "forge fmt --check && npm run solhint:check", | |||
"ci:lint": "npm run lint:check", | |||
"build:go": "./flatten.sh contracts/*.sol test/*.sol", | |||
"build:go": "./flatten.sh contracts/*.sol test/harnesses/*.sol test/mocks/*.sol", |
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.
💡 Codebase verification
Additional test directories need coverage in build:go script
The build:go script pattern test/harnesses/*.sol test/mocks/*.sol
only covers files in the harnesses and mocks directories. However, there are Solidity files in additional test directories that need to be included:
- test/integration/*.sol
- test/libs/*.sol
- test/zaps/*.sol
- test/*.sol (root test directory)
The script should be updated to include all test directories containing .sol files.
🔗 Analysis chain
Verify test file coverage after directory restructure.
The build:go script has been updated to reflect the new test directory structure. Let's ensure all test files are covered by the new paths.
The previous script had a syntax error in the shell condition. This new approach will help us identify all test files and their locations to verify coverage against the build:go script patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all .sol files in test directories are covered by the build:go script patterns
# Find all .sol files in test directory tree
echo "All .sol files in test directories:"
fd -e sol . packages/contracts-rfq/test/
# Verify no files are missed by the new patterns
echo -e "\nFiles not covered by new patterns:"
fd -e sol . packages/contracts-rfq/test/ --exec sh -c '
if ! [[ "{}" =~ ^packages/contracts-rfq/test/(harnesses|mocks)/ ]]; then
echo "{}"
fi
'
Length of output: 6409
Script:
#!/bin/bash
# Let's try a different approach to check for files not in harnesses or mocks directories
echo "Files in test directory that are not in harnesses or mocks:"
fd -e sol . packages/contracts-rfq/test/ | grep -v "/test/\(harnesses\|mocks\)/"
echo -e "\nDirectories containing .sol files:"
fd -e sol . packages/contracts-rfq/test/ | sed 's!/[^/]*$!!' | sort -u
Length of output: 2721
@@ -1,7 +1,9 @@ | |||
// @ts-nocheck |
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.
Remove @ts-nocheck and fix type issues properly
Using @ts-nocheck
disables all TypeScript type checking for the file, which reduces code reliability and maintainability. Instead, address type issues explicitly.
Remove this directive and properly type the component props and children.
SYN: { | ||
path: SYN_TOKEN_LINK, | ||
text: '$SYN', | ||
match: null, |
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.
💡 Codebase verification
The removal of INTERCHAIN_LINK from routes.ts requires additional changes
The INTERCHAIN_LINK constant is still being used in other parts of the codebase:
packages/explorer-ui/utils/urls/index.ts
defines and uses INTERCHAIN_PATHpackages/explorer-ui/components/layouts/MainLayout/index.tsx
uses INTERCHAIN_PATH for navigationpackages/synapse-interface/constants/urls/index.tsx
still defines INTERCHAIN_LINK
Please ensure these references are updated or removed to maintain consistency with the routes.ts changes.
🔗 Analysis chain
Verify the removal of INTERCHAIN_LINK
While adding the SYN token link is good, please ensure that removing INTERCHAIN_LINK doesn't break any existing functionality or user journeys.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to INTERCHAIN_LINK
rg "INTERCHAIN_LINK" --type ts --type tsx
Length of output: 69
Script:
#!/bin/bash
# Search for any remaining references to INTERCHAIN_LINK with ripgrep
# Note: Using just --type=ts since tsx wasn't recognized
rg "INTERCHAIN_LINK" --type=ts
# Also search with ast-grep to catch any potential TypeScript/JavaScript usage
ast-grep --pattern 'INTERCHAIN_LINK'
# Let's also check for any potential route references
rg "interchain" -i --type=ts
Length of output: 1437
Description
A clear and concise description of the features you're adding in this pull request.
Additional context
Add any other context about the problem you're solving.
Metadata
82438c0: docs preview link
82438c0: synapse-interface preview link
Summary by CodeRabbit
Release Notes
New Features
ToastContent
component for enhanced user notifications.CustomToaster
component for improved type safety and organization.Bug Fixes
Localization Updates
Chores