Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

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

Adding ETH on BNB Chain [SLT-398] #3338

Merged
merged 10 commits into from
Oct 29, 2024
Merged

Adding ETH on BNB Chain [SLT-398] #3338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2acfada
first pass
Defi-Moses Oct 27, 2024
2ad4f4a
img url update
Defi-Moses Oct 27, 2024
6852b2b
small nits
Defi-Moses Oct 28, 2024
52902fd
use ZeroAddress
abtestingalpha Oct 28, 2024
b80adfc
removes widget changes
abtestingalpha Oct 28, 2024
45ef845
adding bnb logo
Defi-Moses Oct 28, 2024
2b509c1
Fixes type issue on headless mismatch
abtestingalpha Oct 28, 2024
4c84de7
Revert "removes widget changes"
abtestingalpha Oct 28, 2024
d1a73c9
adding eth to bnb chain
Defi-Moses Oct 28, 2024
7de1c18
moving dependencies around
Defi-Moses Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions packages/rest-api/src/constants/bridgeMap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,13 @@ export const BRIDGE_MAP = {
destination: ['JUMP'],
swappable: [],
},
'0x2170Ed0880ac9A755fd29B2688956BD959F933F8': {
decimals: 18,
symbol: 'ETH',
origin: ['RFQ.ETH'],
destination: [],
swappable: [],
},
'0x23b891e5C62E0955ae2bD185990103928Ab817b3': {
decimals: 18,
symbol: 'nUSD',
Expand Down
2 changes: 2 additions & 0 deletions packages/rest-api/src/constants/bridgeable.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,7 @@ export const ETH: BridgeableToken = {
[CHAINS.SCROLL.id]: NativeTokenAddress,
[CHAINS.LINEA.id]: NativeTokenAddress,
[CHAINS.WORLDCHAIN.id]: NativeTokenAddress,
[CHAINS.BNBCHAIN.id]: '0x2170Ed0880ac9A755fd29B2688956BD959F933F8',
},
decimals: {
[CHAINS.ETHEREUM.id]: 18,
Expand All @@ -821,6 +822,7 @@ export const ETH: BridgeableToken = {
[CHAINS.SCROLL.id]: 18,
[CHAINS.LINEA.id]: 18,
[CHAINS.WORLDCHAIN.id]: 18,
[CHAINS.BNBCHAIN.id]: 18,
},
symbol: 'ETH',
name: 'Ethereum',
Expand Down
4 changes: 2 additions & 2 deletions packages/synapse-constants/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@
"rollup": "^4.22.4",
"rollup-plugin-typescript2": "^0.36.0",
"typescript": "^5.3.3",
"viem": "^2.21.6"
"viem": "^2.21.6",
"@codecov/rollup-plugin": "^1.2.0"
Comment on lines +48 to +49
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Move @codecov/rollup-plugin to devDependencies

The @codecov/rollup-plugin is a development tool for code coverage reporting and should be in devDependencies rather than dependencies. This change will prevent the package from being installed in production environments where it's not needed.

Apply this diff to fix the dependency location:

  "dependencies": {
    "viem": "^2.21.6",
-   "@codecov/rollup-plugin": "^1.2.0"
  },
  "devDependencies": {
+   "@codecov/rollup-plugin": "^1.2.0"
  }
📝 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.

Suggested change
"viem": "^2.21.6",
"@codecov/rollup-plugin": "^1.2.0"
"viem": "^2.21.6"

💡 Codebase verification

Code coverage plugin addition appears unrelated to current changes

The addition of @codecov/rollup-plugin to synapse-constants package appears unnecessary as:

  • There are no test files in the synapse-constants package
  • No changes were detected in the rollup configuration to utilize this plugin
  • The package's purpose is to provide constants, which typically don't require code coverage

Consider:

  • Removing the @codecov/rollup-plugin dependency as it's not being used
  • If code coverage is needed, create a separate PR with proper test setup and configuration
🔗 Analysis chain

Verify the relevance of code coverage changes

The addition of the code coverage plugin appears unrelated to the PR's objective of adding ETH on BNB Chain. Could you clarify why these changes are included in this PR? Consider moving them to a separate PR focused on testing infrastructure improvements.

Let me gather more specific information about code coverage related changes.

Let me check the specific rollup configuration changes in the synapse-constants package.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if there are any test-related changes in this PR that would justify adding code coverage
rg -l "test|spec|coverage" --type ts --type js

Length of output: 6964

Script:

#!/bin/bash
# Check for any changes to test configuration files or coverage settings
git diff HEAD~1 -G"coverage|jest|test" -- "*.json" "*.js" "*.ts"

# Check if codecov plugin is used in any rollup configs
rg -l "codecov|rollup" --type js --type ts

Length of output: 363

Script:

#!/bin/bash
# Check the rollup config changes in synapse-constants
git diff HEAD~1 packages/synapse-constants/rollup.config.js

# Also check if there are any test files in synapse-constants
fd -t f "test|spec" packages/synapse-constants/

Length of output: 111

},
"devDependencies": {
"@codecov/rollup-plugin": "^1.2.0"
}
}
7 changes: 7 additions & 0 deletions packages/synapse-constants/src/constants/bridgeMap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,13 @@ export const BRIDGE_MAP = {
destination: ['JUMP'],
swappable: [],
},
'0x2170Ed0880ac9A755fd29B2688956BD959F933F8': {
decimals: 18,
symbol: 'ETH',
origin: ['RFQ.ETH'],
destination: [],
swappable: [],
},
Defi-Moses marked this conversation as resolved.
Show resolved Hide resolved
'0x23b891e5C62E0955ae2bD185990103928Ab817b3': {
decimals: 18,
symbol: 'nUSD',
Expand Down
1 change: 1 addition & 0 deletions packages/synapse-constants/src/constants/tokens/bridgeable.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -750,6 +750,7 @@ export const ETH = new Token({
[CHAINS.SCROLL.id]: nullAddress,
[CHAINS.LINEA.id]: nullAddress,
[CHAINS.WORLDCHAIN.id]: nullAddress,
[CHAINS.BNB.id]: '0x2170Ed0880ac9A755fd29B2688956BD959F933F8',
Defi-Moses marked this conversation as resolved.
Show resolved Hide resolved
},
decimals: 18,
symbol: 'ETH',
Expand Down
17 changes: 17 additions & 0 deletions packages/synapse-interface/assets/icons/bnb.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion packages/synapse-interface/components/Activity/Transaction/TransactionOptions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ export function TransactionPopoverContainer({
}) {
return (
<Transition
as={Fragment}
as={Fragment as any}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid using as any type assertion.

The type assertion Fragment as any bypasses TypeScript's type checking system, which could mask potential type-related issues. This is generally considered an anti-pattern in TypeScript development.

Consider one of these type-safe alternatives:

- as={Fragment as any}
+ as={Fragment}

Or if you're facing type compatibility issues with @headlessui/react, properly type it using:

import { TransitionRootProps } from '@headlessui/react'

type AsProps = TransitionRootProps['as']
// Then use it as
as={Fragment as AsProps}

enter="transition ease-out duration-200"
enterFrom="opacity-0 translate-y-1"
enterTo="opacity-100 translate-y-0"
Expand Down
2 changes: 1 addition & 1 deletion packages/synapse-interface/components/Pools/PoolActionOptions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export function TransactionPopoverContainer({
}) {
return (
<Transition
as={Fragment}
as={Fragment as any}
enter="transition ease-out duration-200"
enterFrom="opacity-0 translate-y-1"
enterTo="opacity-100 translate-y-0"
Expand Down
4 changes: 2 additions & 2 deletions packages/synapse-interface/components/layouts/LandingPageWrapper/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export function LandingNav() {
</div>

<Transition
as={Fragment}
as={Fragment as any}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid using as any type assertions with @headlessui/react components.

The type assertions Fragment as any in both Transition components bypass TypeScript's type checking, which could hide potential runtime errors. This is generally considered an anti-pattern.

Consider these alternatives:

  1. Use proper types from @headlessui/react:
import { Transition } from '@headlessui/react'
import type { Fragment } from 'react'

// Use proper type
as={Fragment as React.ComponentType}
  1. If the type issue persists, check for version mismatches:
npm info @headlessui/react version
npm info @types/react version
  1. Update dependencies if needed:
npm update @headlessui/react @types/react

Also applies to: 164-164

enter="duration-100 ease-out"
enterFrom=" opacity-0"
enterTo=" opacity-100"
Expand Down Expand Up @@ -161,7 +161,7 @@ export function PopoverPanelContainer({
}) {
return (
<Transition
as={Fragment}
as={Fragment as any}
enter="transition ease-out duration-200"
enterFrom="opacity-0 translate-y-1"
enterTo="opacity-100 translate-y-0"
Expand Down
7 changes: 7 additions & 0 deletions packages/synapse-interface/constants/bridgeMap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,13 @@ export const BRIDGE_MAP = {
destination: ['JUMP'],
swappable: [],
},
'0x2170Ed0880ac9A755fd29B2688956BD959F933F8': {
decimals: 18,
symbol: 'ETH',
origin: ['RFQ.ETH'],
destination: [],
swappable: [],
},
'0x23b891e5C62E0955ae2bD185990103928Ab817b3': {
decimals: 18,
symbol: 'nUSD',
Expand Down
1 change: 1 addition & 0 deletions packages/synapse-interface/constants/tokens/bridgeable.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,7 @@ export const ETH = new Token({
[CHAINS.SCROLL.id]: zeroAddress,
[CHAINS.LINEA.id]: zeroAddress,
[CHAINS.WORLDCHAIN.id]: zeroAddress,
[CHAINS.BNB.id]: '0x2170Ed0880ac9A755fd29B2688956BD959F933F8',
},
decimals: 18,
symbol: 'ETH',
Expand Down
7 changes: 7 additions & 0 deletions packages/widget/src/constants/bridgeMap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,13 @@ export const BRIDGE_MAP = {
destination: ['JUMP'],
swappable: [],
},
'0x2170Ed0880ac9A755fd29B2688956BD959F933F8': {
decimals: 18,
symbol: 'ETH',
origin: ['RFQ.ETH'],
destination: [],
swappable: [],
},
'0x23b891e5C62E0955ae2bD185990103928Ab817b3': {
decimals: 18,
symbol: 'nUSD',
Expand Down
Loading