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

(WIP) Coins deprecation #15509

Closed
wants to merge 10 commits into from
Closed

Conversation

MiroslavProchazka
Copy link
Contributor

@MiroslavProchazka MiroslavProchazka commented Nov 21, 2024

Closed in favor of #16131

As of February 2025, Trezor Suite will discontinue support for Dash, Bitcoin Gold, DigiByte, Namecoin, and Vertcoin. Users are advised to migrate their funds to alternative wallets to ensure continued access and security. The coins will still be supported on the FW level, so users can still access them using third party wallets.

Proposed Timeline:

  • February Suite Release: This version of Suite will no longer support the affected coins. However, users can still use older versions of Suite to access their coins.
  • July 2025: The BEs will be turned off. From this point forward, third-party apps will be the only way to access the coins.

I will divide the commits by coin and by environment (desktop/native) to make code review easier.

Please read the following before conducting the code review:

  • Since the coins will still be supported by the firmware and third-party apps, we should maintain support at the Connect level. Similarly, we will continue support in blockchain-link and utxo-lib.
  • For now, we have decided to keep these coins in Trading (users will need to use external addresses for this), so we need to retain the Address Validator as well.
  • We need to create a migration for users with remembered wallets.

Given the points above, I have prepared only basic deprecation that mainly affects NetworksConfig. Everything else (tests, icons, etc.) will remain unchanged until we decide to disable trading options and/or turn off the backends.

Todo:

  • In case that the coins will be still available for trading (to external wallets), can we delete their icons, names in messages.ts and other mentions in general files?
  • Should I also delete all tests mentioning those coins?
  • As some of the coins files needs to stay in the codespace (address validator, blockchain-link, connect etc.), it would be wise to determinate minimal scope for just Suite deprecation - will this be just NetworksConfig?
  • Create migration for users with remembered wallet
  • Adjust coins.ts
  • Adjust /trezor-suite/suite-common/suite-constants/src/protocol.ts

This pull request involves the removal of several cryptocurrency networks from the codebase. The changes span multiple files and aim to simplify the configuration and support for the remaining networks.

Removal of cryptocurrency networks:

Copy link

github-actions bot commented Nov 21, 2024

🚀 Expo preview is ready!

  • Project → trezor-suite-preview
  • Platforms → android, ios
  • Scheme → trezorsuitelite
  • Runtime Version → 11
  • More info

Learn more about 𝝠 Expo Github Action

@martykan
Copy link
Member

What will happen if user has remembered accounts for these coins? Maybe a migration to delete them?

@@ -269,33 +269,6 @@ export default [
subscribe: '',
},
},
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be deleted @mroz22?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather keep support in the under-suite layers. so no.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solved

@@ -9,7 +9,6 @@ export const COINS: Record<NetworkSymbol | LegacyNetworkSymbol, string> = {
bnb: require('../../images/coins/bnb.svg'),
btc: require('../../images/coins/btc.svg'),
btg: require('../../images/coins/btg.svg'),
dash: require('../../images/coins/dash.svg'),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should stay here, right? @martykan

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Connect Explorer doesn't need it

Copy link
Contributor

@mroz22 mroz22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please lets not do changes in blockchain-link and utxo-lib. Support can peacefully live there for some time.

@@ -269,33 +269,6 @@ export default [
subscribe: '',
},
},
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather keep support in the under-suite layers. so no.

@MiroslavProchazka
Copy link
Contributor Author

please lets not do changes in blockchain-link and utxo-lib. Support can peacefully live there for some time.

Solved

@MiroslavProchazka
Copy link
Contributor Author

Closed in favor of #16131

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants