Skip to content
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

Cardano token definitions fix #13189

Merged
merged 6 commits into from
Jul 5, 2024

Conversation

tomasklim
Copy link
Member

@tomasklim tomasklim commented Jul 4, 2024

Description

  • revert using policy id as contract address as it is not really needed and it was breaking send form because policy id is not unique (aha) and this solution is better (no changes to critical methods)
  • fixes coins without token definitions have all tokens "legit" by default
  • fiats are now fetched first for policy id, then for unit
    • coingecko mixes them together (token definitions are modified so that they use only policy ids)
  • policy id is now used for token definitions (policy id is part of unit which is contract address so it works!)
  • show also unit to a user

Related Issue

Resolve #13169

Screenshots:

if you compare it with #13093 we also have fiat for banker coin! because that one uses unit and not policy id on coingecko 🙃
Screenshot 2024-07-04 at 13 19 40

(token-definitions turned on just for this screenshot)
Screenshot 2024-07-04 at 13 19 48
Screenshot 2024-07-04 at 13 25 51

@tomasklim tomasklim marked this pull request as draft July 4, 2024 11:11
@tomasklim tomasklim force-pushed the feat/cardano-without-token-definitions-4ever branch from 761ba00 to 46ac130 Compare July 4, 2024 11:20
@tomasklim tomasklim changed the title Feat/cardano without token definitions 4ever Cardano token definitions fix Jul 4, 2024
@tomasklim tomasklim marked this pull request as ready for review July 4, 2024 11:25
@tomasklim tomasklim added release Will be included in the upcoming release. Needs to be backported to the release branch. build-desktop This will trigger the build of desktop apps for your PR labels Jul 4, 2024
@tomasklim
Copy link
Member Author

tomasklim commented Jul 4, 2024

@trezor/qa

  • check that cardano tokens can be send (also the ones with same policy id)
  • check migration again please and that it works after the migration
  • check that fiat rates and historic rates work for coins using and not blockbook
  • coins without token definitions (e.g. testnet ada should have tokens by default legit)

@tomasklim tomasklim force-pushed the feat/cardano-without-token-definitions-4ever branch from 46ac130 to 006302a Compare July 4, 2024 11:37
@bosomt
Copy link
Contributor

bosomt commented Jul 4, 2024

work in progress

Upgrade from
2.6.0 8ca0111 to
24.8.0 13658c4

  • send TADA
  • explorer link ok
  • send TADA token
  • explorer link ok
  • hide token ok
  • show token ok
  • view token in explorer ok
  • fingerprint copy ok
  • policy id ok

@tomasklim
Copy link
Member Author

/rebase

Copy link

github-actions bot commented Jul 4, 2024

@trezor-ci trezor-ci force-pushed the feat/cardano-without-token-definitions-4ever branch from 35861a2 to ab06bcf Compare July 4, 2024 14:23
@tomasklim
Copy link
Member Author

do not merge yet please

@tomasklim tomasklim force-pushed the feat/cardano-without-token-definitions-4ever branch from ab06bcf to 67111eb Compare July 4, 2024 19:58
@tomasklim tomasklim merged commit b938640 into develop Jul 5, 2024
77 of 86 checks passed
@tomasklim tomasklim deleted the feat/cardano-without-token-definitions-4ever branch July 5, 2024 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-desktop This will trigger the build of desktop apps for your PR release Will be included in the upcoming release. Needs to be backported to the release branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Carano tokens with same policy id are hidden at once
3 participants