Support Terra Clasic #7983
Support Terra Clasic #7983
Conversation
niodamian
requested review from
ezailWang,
huhuanming,
originalix,
revan-zhang and
sidmorizon
as code owners
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis update adds support for the Terra Classic network ("columbus-5") across multiple modules. It introduces Terra in network configuration, vault settings, migration presets, and query mappings. Terra is registered with Cosmwasm query, while its MintScan registration is commented out. No existing logic or error handling is changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Also couldnt find where i could add CW20 support token for the network. Wanted to add Juris protocol logo and ca with gecko id for price injection * Terra Classic - https://github.com/cosmostation/chainlist/tree/main/chain/terra-classic Terra classsic main 2 native coins are LUNC and USTC [ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
packages/shared/types/earn/earnProvider.constants.ts (2)
60-76: Add “LUNC” to the staking whitelist.
isSupportStaking()still omits LUNC, so Terra Classic users cannot stake even after you add the network.'POL', 'USDC', 'USDT', 'DAI', 'WETH', 'CBBTC', 'WBTC', 'USDF', + 'LUNC', ].includes(symbol.toUpperCase());
91-109: Normalize function misses “lunc”.
normalizeToEarnSymbol()lacks a'lunc'key. Calls with lowercase symbols will fail to resolve.'usde': 'USDe', + 'lunc': 'LUNC',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
packages/core/src/chains/cosmos/sdkCosmos/query/IQuery.ts(1 hunks)packages/kit-bg/src/migrations/v4ToV5Migration/v4CoinTypeToNetworkId.ts(2 hunks)packages/shared/types/earn/earnProvider.constants.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/shared/types/earn/earnProvider.constants.ts (1)
packages/shared/src/config/networkIds.ts (1)
getNetworkIdsMap(72-94)
packages/core/src/chains/cosmos/sdkCosmos/query/IQuery.ts (2)
packages/shared/src/config/networkIds.ts (1)
getNetworkIdsMap(72-94)packages/core/src/chains/cosmos/sdkCosmos/query/MintScanQuery.ts (1)
MintScanQuery(33-162)
🔇 Additional comments (3)
packages/kit-bg/src/migrations/v4ToV5Migration/v4CoinTypeToNetworkId.ts (2)
12-13: Good addition – constant imported.The extra
COINTYPE_TERRAimport is correct and keeps the mapping list tidy.
58-59: Mapping looks right.
COINTYPE_TERRA → networkIdsMap.terralines up with the new preset network.packages/shared/types/earn/earnProvider.constants.ts (1)
190-194: Symbol map but no staking support.Good to map
'LUNC'to Terra. Make sure the two earlier fixes land, otherwise the earn module will still refuse LUNC.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
ok i did the changes bot said * how can i import logo of juris ? cw20 token |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
packages/core/src/chains/cosmos/sdkCosmos/query/IQuery.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/core/src/chains/cosmos/sdkCosmos/query/IQuery.ts (2)
packages/shared/src/config/networkIds.ts (1)
getNetworkIdsMap(72-94)packages/core/src/chains/cosmos/sdkCosmos/query/MintScanQuery.ts (1)
MintScanQuery(33-162)
🪛 ESLint
packages/core/src/chains/cosmos/sdkCosmos/query/IQuery.ts
[error] 55-55: Expected exception block, space or tab after '//' in comment.
(spaced-comment)
🔇 Additional comments (1)
packages/core/src/chains/cosmos/sdkCosmos/query/IQuery.ts (1)
62-62: Terra Classic MintScan registration looks good.The switch from
CosmwasmQuerytoMintScanQueryfor Terra Classic enables CW20 token catalog support, which aligns with the PR objective to add Terra Classic token support.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
packages/core/src/chains/cosmos/sdkCosmos/query/IQuery.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
originalix
left a comment
originalix
left a comment
There was a problem hiding this comment.
Hi thanks for this pr. Supporting Terra Classic also requires server-side handling for token metadata, fee estimation, and other related features. Simply adding the network config on the client side won’t be sufficient for it to work properly.
So the review will be put on hold for now until server-side support is in place.
| [COINTYPE_BCH]: networkIdsMap.bch, | ||
| [COINTYPE_XRP]: networkIdsMap.xrp, | ||
| [COINTYPE_COSMOS]: networkIdsMap.cosmoshub, | ||
| [COINTYPE_TERRA]: networkIdsMap.terra, |
There was a problem hiding this comment.
Is COINTYPE_TERRA different from COINTYPE_COSMOS ?
There was a problem hiding this comment.
removed COINTYPE_TERRA since i added here : https://github.com/niodamian/app-monorepo/blob/x/packages/kit-bg/src/vaults/impls/cosmos/settings.ts#L97
There was a problem hiding this comment.
Terra uses more native coins LUNC / USTC / KRW should i add them or keep it only on uluna (LUNC ) ?
There was a problem hiding this comment.
We currently only support one native token.
I believe I added the fees of terra ...keplr wallet https://github.com/chainapsis/keplr-chain-registry/blob/main/cosmos/columbus.json |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
packages/kit-bg/src/migrations/v4ToV5Migration/v4CoinTypeToNetworkId.ts(2 hunks)packages/shared/src/config/presetNetworks.ts(1 hunks)
| COINTYPE_COSMOS, | ||
| COINTYPE_DNX, |
There was a problem hiding this comment.
Missing Terra coin-type mapping – migration will drop user accounts.
Terra uses BIP-44 coin-type 330 (COINTYPE_TERRA). It is not imported or mapped, so any v4 wallet holding LUNC/USTC addresses will lose its link during migration.
@@
COINTYPE_COSMOS,
+ COINTYPE_TERRA,
@@
[COINTYPE_COSMOS]: networkIdsMap.cosmoshub,
+ [COINTYPE_TERRA]: networkIdsMap.terra,networkIdsMap.terra being defined – ensure the previous file registers terra correctly.
Also applies to: 56-56
🤖 Prompt for AI Agents
In packages/kit-bg/src/migrations/v4ToV5Migration/v4CoinTypeToNetworkId.ts
around lines 11 to 12 and line 56, the Terra coin-type (BIP-44 coin-type 330,
COINTYPE_TERRA) is missing from the coin-type to network ID mapping, which will
cause user accounts with Terra addresses to be dropped during migration. Add the
import for COINTYPE_TERRA and include it in the mapping to networkIdsMap.terra.
Also verify that networkIdsMap.terra is properly defined in the related file to
ensure the migration handles Terra wallets correctly.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
changed mintscan to cosmwasmQuery - currently mintscan doesnt support the chain or it supports the chain its not on the website cause its facing performance issues currently |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
packages/core/src/chains/cosmos/sdkCosmos/query/IQuery.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/core/src/chains/cosmos/sdkCosmos/query/IQuery.ts (2)
packages/shared/src/config/networkIds.ts (1)
getNetworkIdsMap(72-94)packages/core/src/chains/cosmos/sdkCosmos/query/MintScanQuery.ts (1)
MintScanQuery(33-162)
🪛 ESLint
packages/core/src/chains/cosmos/sdkCosmos/query/IQuery.ts
[error] 55-55: Delete ·
(prettier/prettier)
🔇 Additional comments (1)
packages/core/src/chains/cosmos/sdkCosmos/query/IQuery.ts (1)
62-62: Verify Terra CW20 metadata display
I ran a search for any Terra-specific CW20 calls but found no references in the codebase. Since you’ve removed the MintScan registry line and rely solely on CosmWasmQuery, we need to ensure Terra Classic CW20 token metadata (name, logo, decimals) still shows up in the UI.Please manually verify that:
- Terra Classic CW20 tokens load correctly in the token list
- Their name, logo, and decimals appear as expected
|
removed terra 2.0 logo and added terra 1.0 logo - |
Terra Classic uses all the natives coins but since we closed all the stablecoins we currently using USTC / LUNC We got also a tax of 0.5% which applies on LUNC and USTC only CW20 tokens dont have a tax |
|
originalix any update ? I already did some PRs on the other OneKey repos |
4 participants
Summary by CodeRabbit