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

Support longer token symbols via wallet_watchAsset #433

Merged
merged 3 commits into from
Mar 26, 2021

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Mar 24, 2021

This increases the maximum token length that can be suggested via wallet_watchAsset from 6 to 11, which matches the maximum symbol length for custom tokens added on the MetaMask extension.

This increases the maximum token length that can be suggested via
`watchAsset` from 6 to 11, which matches the maximum symbol length for
custom tokens added on the MetaMask extension.
@Gudahtt Gudahtt requested a review from a team as a code owner March 24, 2021 17:54
@Gudahtt Gudahtt changed the title Support longer token symbols via watchAsset Support longer token symbols via wallet_watchAsset Mar 24, 2021
The minimum and maximum token symbol lengths are now tested, as is the
failure case where the token symbol is an empty string.
@rekmarks rekmarks self-assigned this Mar 24, 2021
@rekmarks rekmarks requested a review from ryanml March 24, 2021 20:06
Copy link
Member

@rekmarks rekmarks left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@ryanml ryanml left a comment

Choose a reason for hiding this comment

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

++ thanks for the changes!

@Gudahtt
Copy link
Member Author

Gudahtt commented Mar 24, 2021

Thanks for the reviews! Marking this as "DO-NOT-MERGE" for now until the mobile team has signed off on this.

@Gudahtt
Copy link
Member Author

Gudahtt commented Mar 26, 2021

Removing the DO-NOT-MERGE label, as it seems everyone on the mobile team has signed off on this.

@Gudahtt Gudahtt merged commit ef974b5 into develop Mar 26, 2021
@Gudahtt Gudahtt deleted the support-longer-watch-asset-symbol-lengths branch March 26, 2021 13:11
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
* Support longer token symbols via `watchAsset`

This increases the maximum token length that can be suggested via
`watchAsset` from 6 to 11, which matches the maximum symbol length for
custom tokens added on the MetaMask extension.

* Add additional tests for symbol length boundary conditions

The minimum and maximum token symbol lengths are now tested, as is the
failure case where the token symbol is an empty string.
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
* Support longer token symbols via `watchAsset`

This increases the maximum token length that can be suggested via
`watchAsset` from 6 to 11, which matches the maximum symbol length for
custom tokens added on the MetaMask extension.

* Add additional tests for symbol length boundary conditions

The minimum and maximum token symbol lengths are now tested, as is the
failure case where the token symbol is an empty string.
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.

3 participants