-
Notifications
You must be signed in to change notification settings - Fork 1
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.
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
feat: custom token import #22
Conversation
add boolean flag to classes rather than creating a new asset manager type that makes the assets interface more cumbersome to use
Significant changes include: - Updated the commit reference for bundled coins in the build configuration - Added a new class, CustomAssetHistoryStorage, to manage custom token history - Enhanced AssetManager to handle custom tokens by adding them to wallet history and activating them - Extended the Asset class with a method to create an instance from JSON data
fixes runtime error in KW where wallet page fails to load due to ticker index not being initialized
9489a18
to
75b7d5c
Compare
the custom token erc20 activation strategy was placed in the factory which had it's reference removed in the latest changes or merge from dev
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work! I'm happy with the architectural changes. Everything meets the standard I'm striving for and is consistent with the current architecture.
Thank you for taking the time to fully understand the architecture and the broader context of your changes. The system has many components and is mostly undocumented. When we have some breathing room and a stable release, we'll work together to document everything (mainly me) and on test coverage (mainly you)
}; | ||
} | ||
|
||
/// [AddressFormat] format field options. | ||
enum AddressFormatFormat { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This enum is confusing/redundant. Can you rename it to something clearer? I suggest AddressFormatType
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed in f91cf60
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spicy. Some of your best contributions to date.
Changes