-
Notifications
You must be signed in to change notification settings - Fork 23
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
Feature/chains improvement #1955
base: master
Are you sure you want to change the base?
Conversation
@@ -149,7 +150,7 @@ class TokenModel extends Equatable { | |||
} | |||
|
|||
const seedsToken = TokenModel( | |||
chainName: "Telos", | |||
chainName: SeedsChains.telos.value, |
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.
Seems we can't use extensions in constant expressions. Ref explanation here: dart-lang/language#663
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.
Maybe you need to update your flutter version?
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.
Oh you're right - there's something weird going on with this PR.
@@ -0,0 +1,26 @@ | |||
const String _chainTelos = 'telos'; |
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.
Lower case "telos" makes sense but we have a conflict with legacy code in seeds.tokensmaster.cpp
and the existing tmastr.seeds
table entries which use "Telos".
Suggested fix would be to patch a conditional expression here which specifically converts "Telos" to "telos".
chainName: parsedJson["chain"]!, |
e.g.
chainName: parsedJson["chain"]! == "Telos" ? "telos" : parsedJson["chain"]!, // fix legacy capitalization
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.
Is this an actual bug or ... ?
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.
We need to choose whether "telos" or "Telos" is the string we use in TokenModel.chainName.
- If "Telos" (capitalized), change
const String _chainTelos = 'telos'; - If "telos" (lowercase), make the patch so that legacy metadata containing "Telos" is normalized.
So yes, it's a bug that will appear as soon as we start using this PR code.
@n13 This looks like the beginning of an evolution to multichain capability as we will be able to show token cards from Telos and EOS (and other Antelope chains as well). IMHO this is great and I love it!. Is there a bigger plan for how this UX should work? update Currency cards are filtered by the active chain. Active chain is set when you switch to a new account; the account selector tab should have a UI indication of chain. Token metadata for all Antelope chains can be managed thru the Is it true that only Telos retains the original eosio resource payment model? In any case, SEEDS payforcpu doesn't make sense for non-SEEDS accounts on other chains, and we may have to figure out how to integrate PowerUp etc. |
Merged master into this - probably need to do a new PR as it is it makes little sense |
@@ -95,7 +97,11 @@ class GetTokenModelsUseCase extends InputUseCase<List<TokenModel>, TokenModelSel | |||
print("supply: $supply"); | |||
} | |||
}, | |||
).catchError((dynamic error) => _statRepository.mapHttpError(error)); | |||
).catchError((dynamic error) { | |||
// This entire code here is really funky - Nik |
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.
I resemble this remark! ;)
see #1960
No description provided.