feat: config starknet chain #5228
Conversation
mshojaei-txfusion
requested review from
jmrossy,
yorhodes,
ltyu and
paulbalaji
as code owners
🦋 Changeset detectedLatest commit: 32b0a7f The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…g required for Starknet chains
| @@ -7,7 +7,7 @@ import { EthereumLogo } from './Ethereum.js'; | |||
| import { SolanaLogo } from './Solana.js'; | |||
|
|
|||
| export const PROTOCOL_TO_LOGO: Record< | |||
| ProtocolType, | |||
| Exclude<ProtocolType, 'starknet'>, | |||
There was a problem hiding this comment.
reuse the NonStarknetProtocol type defined elsewhere?
There was a problem hiding this comment.
wanted to do this but as this was temporary, what should I call it, where should I put this? on sdk?
| @@ -40,11 +40,14 @@ const logger = widgetLogger.child({ | |||
| module: 'walletIntegrations/multiProtocol', | |||
| }); | |||
|
|
|||
| type NonStarknetProtocol = Exclude<ProtocolType, 'starknet'>; | |||
| type ProtocolRecord<T> = Record<NonStarknetProtocol, T>; | |||
There was a problem hiding this comment.
ProtocolMap might be a better word given the existing ChainMap nomenclature we use
| @@ -5,6 +5,7 @@ export enum ProtocolType { | |||
| Ethereum = 'ethereum', | |||
| Sealevel = 'sealevel', | |||
| Cosmos = 'cosmos', | |||
| Starknet = 'starknet', | |||
There was a problem hiding this comment.
since we're adding a new protocol type - automatically i would class this PR as a major change and not minor or patch
| }; | ||
| } | ||
|
|
||
| function formatChainIdBasedOnProtocol(chainId: string, protocol: ProtocolType) { |
There was a problem hiding this comment.
nit: formatChainIdForProtocol
There was a problem hiding this comment.
and should that be SupportedProtocol as well?
There was a problem hiding this comment.
You are right better to be SupportedProtocol
| if (isStarknet) { | ||
| denom = await input({ | ||
| message: "Enter the native token's address:", | ||
| validate: (value) => isAddressStarknet(value), | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
is there any way to fetch this for the user without requiring input?
There was a problem hiding this comment.
On starknet mainnet there are two native tokens for fees - STRK and ETH, and AFAIK Paradex chain has an native token ETH address completely different from starknet mainnet @mshojaei-txfusion correct me if I am wrong
There was a problem hiding this comment.
We did not find any address on starknet.js to import, but we may be able to add a default native token address for starknet, and ask user to change it in case it was different.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5228 +/- ##
=======================================
Coverage 77.53% 77.53%
=======================================
Files 103 103
Lines 2110 2110
Branches 190 190
=======================================
Hits 1636 1636
Misses 453 453
Partials 21 21
|
Description
This PR adds Starknet protocol support to the chain configuration CLI, including:
Difference with last PR
Related issues
N/A
Backward compatibility
Yes - Only adds new functionality without modifying existing behavior
Testing
N/A