Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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. ✨ 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. Comment |
token-specific URLs are returning 404s
There was a problem hiding this comment.
Pull Request Overview
This PR fixes Etherscan URL generation issues by correcting the URL builder to use token contract addresses instead of swap contract addresses, and ensures mixed case contract addresses to avoid checksum validation errors.
Key changes:
- Updated Etherscan URL builder to use token contract address instead of swap contract address
- Modified
get_token_infoto handle mixed case contract addresses and prevent checksum failures - Enhanced custom asset handling with proper parent-child relationship parsing
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| etherscan_transaction_history_strategy.dart | Updated URL construction to use token contract address and added validation |
| custom_asset_history_storage.dart | Added knownAssets parameter for proper asset parsing with parent relationships |
| activation_manager.dart | Updated custom token activation flow and improved asset refresh notification timing |
| asset_manager.dart | Passed known assets to custom asset history retrieval |
| coin_config_repository.dart | Implemented two-pass asset parsing strategy and added AssetParser dependency |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
..._defi_sdk/lib/src/transaction_history/strategies/etherscan_transaction_history_strategy.dart
Outdated
Show resolved
Hide resolved
packages/komodo_defi_sdk/lib/src/activation/activation_manager.dart
Outdated
Show resolved
Hide resolved
| /// NOTE: Parent/child relationships are not rebuilt for single asset retrieval. | ||
| /// Use [getAssets] if you need proper parent relationships. |
There was a problem hiding this comment.
This limitation could lead to inconsistent behavior when retrieving individual assets vs collections. Consider adding a warning log or throwing an exception when parent/child relationships are expected but not available in single asset retrieval.
some of the conversion operations likely change the order of the original list and keys, which caused a segfault on linux
takenagain
left a comment
There was a problem hiding this comment.
LGTM. Tested on Web and Linux.
I made the following follow-up changes:
Closes GLEECBTC/gleec-wallet#3098
get_token_infoto ensure mixed case contract address and avoid checksum fail errorsTo test: