Conversation
CharlVS
changed the title
|
Visit the preview URL for this PR (updated for commit a93a934): https://komodo-playground--pr316-codex-tron-support-mrwh0zpn.web.app (expires Wed, 25 Mar 2026 15:06:08 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 2bfedd77fdea45b25ba7c784416e81f177aa5c47 |
CharlVS
left a comment
CharlVS
left a comment
There was a problem hiding this comment.
I did a pass on this against the existing SDK/types conventions and the main thing I would fix before merge is the legacy wallet-balance fallback.
-
In
packages/komodo_defi_rpc_methods/lib/src/rpc_methods/eth/enable_eth_with_tokens.dart,WalletBalance.fromLegacyAddressInfos()/WalletAddress.fromLegacyJson()only reconstruct balances frombalances. The repo fixtures inplayground/assets/komodo_defi.postman_collection.jsonalso contain legacyerc20_addresses_infosentries that have onlytickersand nobalances(for example the BNB/MATIC responses). With the new fallback path, those enabled zero-balance tokens would disappear from the synthesizedwallet_balanceinstead of being preserved as activated assets. Since this PR explicitly aims to preserve the docs-style legacy response shape, I think this should synthesize zero balances fromtickerstoo, and add a regression test for that case. -
The new
protocol.typeprecedence looks right for TRON, but it now changes parsing semantics across shared codepaths:AssetId.parse,ProtocolClass.fromJson, and the protocol constructors. The current regression coverage is TRX/TRC20-specific. I would add a couple non-TRON cases here as well so we know we did not accidentally change parsing behavior for existing configs while fixing the TRON shape. -
In
packages/komodo_defi_sdk/lib/src/activation/protocol_strategies/eth_task_activation_strategy.dart, activation progress still reports node count viaprotocol.requiredServers.electrum?.length ?? 0. For TRX/EVM-style configs that means the progress metadata/UI will report0nodes even whennodesis populated. Not merge-blocking, but worth cleaning up while this path is open.
Aside from those points, the overall layering and strategy reuse are aligned with the existing repo structure.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad37ab2fef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a93a93452c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| required ActivationParams activationParams, | ||
| required String platform, | ||
| required String contractAddress, | ||
| String protocolType = 'ERC20', |
There was a problem hiding this comment.
For callers who use client.rpc.erc20.enableCustomErc20Token(...) directly to add a custom TRC20 asset, this helper still defaults protocolType to ERC20. Unless they discover and override the extra string manually, the request serializes "protocol": {"type": "ERC20"} even when the rest of the payload is TRON-specific, so the new TRC20 support only works through CustomErc20ActivationStrategy and not through the public RPC namespace.
Useful? React with 👍 / 👎.
| JsonMap toRpcParams() => super.toRpcParams().deepMerge({ | ||
| 'nodes': nodes.map((e) => e.toJson()).toList(), | ||
| 'priv_key_policy': privKeyPolicy?.toJson(), |
There was a problem hiding this comment.
The new custom-TRC20 flow reaches Trc20ActivationParams.fromJsonConfig(...) without ever setting privKeyPolicy, and this serializer then overwrites the base default with "priv_key_policy": null. That means custom TRC20 activation requests can carry an invalid tagged-union value instead of the documented ContextPrivKey object, which is likely to make the headline custom-token path fail against stricter KDF builds.
Useful? React with 👍 / 👎.
* feat(sdk): add TRON and TRC20 support * chore(sdk): remove generated whitespace * fix(sdk): align config parsing and KDF compatibility
1 participant
Summary
TRXandTRC20protocol support inkomodo_defi_typesDetails
TrxProtocolandTrc20Protocolclasses and protocol-type resolution that prefersprotocol.typeover the legacy top-leveltypeCoinSubClass.trxandCoinSubClass.trc20, update parent-child handling, and keep TRON out of EVM-specific capability bucketsTrxWithTokensActivationParamsandTrc20ActivationParams, generalize ERC20 request surfaces to accept TRC20 protocol payloads, and supportwithdraw.expiration_secondsFeeInfoTronplus UI display support and preserve native TRX balances when parsing the docs-style legacyeth_addresses_infos/erc20_addresses_infosresponse shapeValidation
flutter testinpackages/komodo_defi_typesflutter testinpackages/komodo_defi_rpc_methodsflutter test test/activation/tron_activation_strategy_test.dartinpackages/komodo_defi_sdkflutter build webin/Users/charl/Code/UTXO/gleec-wallet-dev/sdk/packages/komodo_defi_sdk/exampleflutter build webin/Users/charl/Code/UTXO/gleec-wallet-dev