chore(wallet): add GetEthTokenInfo and refactor #21191

onyb · 2023-12-01T18:38:49Z

Removes dependency on Etherscan Pro API (costs us $200/mo), and allows us to pre-fill fields like name, symbol, and decimals for any token on any EVM chain.

Resolves brave/brave-browser#33314
Resolves brave/brave-browser#31062

Submitter Checklist:

I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
There is a ticket for my issue
Used Github auto-closing keywords in the PR description above
Wrote a good PR/commit description
Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally:
- npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
- npm run lint, npm run presubmit wiki, npm run gn_check, npm run tslint
Ran git rebase master (if needed)

Reviewer Checklist:

A security review is not needed, or a link to one is included in the PR description
New files have MPL-2.0 license header
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

Adding ERC20 tokens and NFTs should work as expected.
Editing an existing NFT should work as expected.
(new) Adding an ERC20 token on any EVM chain should pre-fill the name, symbol, and decimals fields.

components/brave_wallet_ui/components/shared/add-custom-token-form/add-custom-token-form.tsx

josheleonard

Frontend ++

StephenHeaps

Awesome 🎉.
iOS ++

brave-builds · 2023-12-04T19:55:02Z

A Storybook has been deployed to preview UI for the latest push

supermassive · 2023-12-05T03:14:11Z

components/brave_wallet/browser/asset_ratio_response_parser.cc

-mojom::BlockchainTokenPtr ParseTokenInfo(const base::Value& json_value,
-                                         const std::string& chain_id,
-                                         mojom::CoinType coin) {
-  auto token_info = api::asset_ratio::TokenInfo::FromValue(json_value);


Let's also drop TokenInfo and TokenInfoResult from asset_ratio.idl

supermassive · 2023-12-05T03:29:54Z

components/brave_wallet/common/brave_wallet.mojom

@@ -1335,6 +1328,34 @@ interface JsonRpcService {
                      string chain_id) => (string decimals,
                                           ProviderError error,
                                           string error_message);
+
+  // Fetches name of an ERC20 or ERC721 token.
+  GetEthTokenName(string contract_address,


Is it possible not to expose GetEthTokenName and GetEthTokenDecimals + GetEthTokenSymbol above in mojom, but reuse GetEthTokenInfo for

brave-core/components/brave_wallet_ui/components/extension/swap/index.tsx

Lines 141 to 181 in c65fcba

const { data: sellTokenDecimals } = useGetEthTokenDecimalsQuery(

sellToken &&

sellToken.coingeckoId === UNKNOWN_TOKEN_COINGECKO_ID &&

!isNativeToken(sellToken)

? {

chainId: sellToken.chainId,

contractAddress: sellToken.contractAddress

}

: skipToken

)

const { data: sellTokenSymbol } = useGetEthTokenSymbolQuery(

sellToken &&

sellToken.coingeckoId === UNKNOWN_TOKEN_COINGECKO_ID &&

!isNativeToken(sellToken)

? {

chainId: sellToken.chainId,

contractAddress: sellToken.contractAddress

}

: skipToken

)

const { data: buyTokenDecimals } = useGetEthTokenDecimalsQuery(

buyToken &&

buyToken.coingeckoId === UNKNOWN_TOKEN_COINGECKO_ID &&

!isNativeToken(buyToken)

? {

chainId: buyToken.chainId,

contractAddress: buyToken.contractAddress

}

: skipToken

)

const { data: buyTokenSymbol } = useGetEthTokenSymbolQuery(

buyToken &&

buyToken.coingeckoId === UNKNOWN_TOKEN_COINGECKO_ID &&

!isNativeToken(buyToken)

? {

chainId: buyToken.chainId,

contractAddress: buyToken.contractAddress

}

: skipToken

)

Fixed here: 17a0e1f

Also cc: @StephenHeaps, since he may be using those removed methods.

supermassive · 2023-12-05T03:32:13Z

components/brave_wallet_ui/common/hooks/use-pending-transaction.ts

-    }
-  }, [transactionDetails?.recipient, transactionInfo?.txType])
+      ? {
+          contractOrMintAddress: transactionDetails.recipient,


Should it have Solanas term Mint in contractOrMintAddress if it is EVM only?

Fixed here: 9549c59

I do intend to add support for Solana as well, but will change back to contractOrMintAddress once it is done.

github-actions · 2023-12-05T08:57:44Z

[puLL-Merge] - brave/brave-core@21191

Description

The pull request is a diff from the Brave core repository, specifically for the brave-core component related to the Brave Wallet. It removes code related to obtaining token information via an interprocess communication (IPC) call to Etherscan and replaces it with a more direct and simplified approach.

Changes

Removed:

Token information retrieval code that relied on Etherscan API calls.
Code related to parsing token information from Etherscan responses.
Unnecessary imports and methods after the change.

Added:

New approach for fetching token info directly without relying on IPC and Etherscan.
Use of useGetTokenInfo hook to fetch token information in components.

Modified:

React state and component logic to accommodate new token information retrieval method.
Component props to remove unused callbacks after changes in data fetching.

Security Hotspots

components/brave_wallet_ui/components/shared/add-custom-token-form/add-custom-token-form.tsx:
- User-input data is being used to fetch token info, ensure that input sanitization/validation is performed to prevent potential injection attacks.
components/brave_wallet_ui/common/hooks/use-get-token-info.ts:
- Ensure that the data obtained from hooks is appropriately sanitized and validated.
components/brave_wallet_ui/common/slices/api.slice.ts:
- Network requests to get token info can contain sensitive data. Any errors should not expose internal service details to the client.

Overall, most changes seem to be refactoring to simplify the process of retrieving token information for the Brave Wallet, including removing reliance on Etherscan's IPC calls. However, it's crucial to audit these changes for any potential security issues related to network requests and user input handling.

brave-builds · 2023-12-05T09:38:19Z

A Storybook has been deployed to preview UI for the latest push

supermassive

wallet core lgtm

github-actions · 2023-12-12T16:36:21Z

[puLL-Merge] - brave/brave-core@21191

Description

The pull request removes the interaction with AssetRatioService for token information within AddAssetActivity.java for the Brave browser's crypto wallet. Instead of using AssetRatioService::GetTokenInfo, the PR switches to using JsonRpcService::GetEthTokenInfo. This is part of a broader change to use a chain-agnostic method for EVM token information querying.

Changes

Changes

Removed use of AssetRatioService in the AddAssetActivity.java.
Added comments indicating replacement with JsonRpcService::GetEthTokenInfo and commented out the old code instead of removing it.
Removed definitions related to TokenType, TokenInfoResult, and TokenInfo from asset_ratio.idl.
Removed ParseTokenInfo related code from asset_ratio_response_parser.cc and related unit tests.
Removed GetTokenInfo methods from asset_ratio_service.* files and corresponding unit tests.
Adjusted json_rpc_service.cc and json_rpc_service.h to reflect new method signatures and added GetEthTokenInfo logic.
Updated brave_wallet.mojom to include the new GetEthTokenInfo interface.
Updated wallet UI components to use the new hooks or methods for token information.

Security Hotspots

Security hotspots are not directly addressed in the changes; however, the switch to JsonRpcService::GetEthTokenInfo should be reviewed to ensure that it doesn't open up new security vulnerabilities, especially with regards to input validation and handling of external responses.

simoarpe

👍 AddAssetActivity will be completely removed soon, so part of the class could be safely commented.

brave-builds · 2023-12-12T18:31:30Z

A Storybook has been deployed to preview UI for the latest push

onyb self-assigned this Dec 1, 2023

onyb requested review from a team as code owners December 1, 2023 18:38

github-actions bot added CI/storybook-url Deploy storybook and provide a unique URL for each build feature/web3/wallet feature/web3/wallet/core labels Dec 1, 2023

onyb force-pushed the f/wallet/deprecate-etherscan branch from c3ec878 to 5a76ef8 Compare December 4, 2023 13:51

josheleonard reviewed Dec 4, 2023

View reviewed changes

components/brave_wallet_ui/components/shared/add-custom-token-form/add-custom-token-form.tsx Outdated Show resolved Hide resolved

josheleonard approved these changes Dec 4, 2023

View reviewed changes

StephenHeaps approved these changes Dec 4, 2023

View reviewed changes

supermassive reviewed Dec 5, 2023

View reviewed changes

onyb force-pushed the f/wallet/deprecate-etherscan branch from d0f58c7 to a5d245c Compare December 5, 2023 08:56

github-actions bot added the puLL-Merge label Dec 5, 2023

onyb requested a review from supermassive December 5, 2023 09:01

StephenHeaps mentioned this pull request Dec 7, 2023

Fix #8529: Update logic to support unknown tokens from non-mainnet transactions brave/brave-ios#8530

Merged

11 tasks

supermassive approved these changes Dec 12, 2023

View reviewed changes

onyb added 8 commits December 12, 2023 21:55

chore(wallet): add GetEthTokenInfo and refactor

3481915

Fix TS unit tests

94a7a06

fix presubmit errors

69774de

storybook fixes

9ae7ed2

review(supermassive): remove unused definitions from asset_ratio.idl

bb2af36

review(supermassive): remove GetEthToken{Decimals,Symbol,Name} methods

e23ece6

review(supermassive): rename contractOrMintAddress to contractAddress

b13ef5d

fix bug for unknown tokens

b8c99cc

onyb added 2 commits December 12, 2023 21:55

review(JL): remove unnecessary condition fallback

48b4993

Fix broken android build

70dda32

onyb force-pushed the f/wallet/deprecate-etherscan branch from c0b57fd to 70dda32 Compare December 12, 2023 16:35

onyb requested a review from simoarpe December 12, 2023 16:36

simoarpe approved these changes Dec 12, 2023

View reviewed changes

Apply code formatting

0022f26

onyb enabled auto-merge (squash) December 12, 2023 19:40

onyb merged commit a56352d into master Dec 12, 2023
17 checks passed

onyb deleted the f/wallet/deprecate-etherscan branch December 12, 2023 22:15

github-actions bot added this to the 1.63.x - Nightly milestone Dec 12, 2023

srirambv mentioned this pull request Feb 19, 2024

Autofill decimals, symbol, name when adding ERC20 tokens brave/brave-browser#33314

Closed

onyb mentioned this pull request Mar 6, 2024

get asset info for EVM assets when contract address is inputted brave/brave-browser#31062

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(wallet): add GetEthTokenInfo and refactor #21191

chore(wallet): add GetEthTokenInfo and refactor #21191

onyb commented Dec 1, 2023 •

edited

Loading

josheleonard left a comment

StephenHeaps left a comment

brave-builds commented Dec 4, 2023

supermassive Dec 5, 2023

onyb Dec 5, 2023

supermassive Dec 5, 2023

onyb Dec 5, 2023

supermassive Dec 5, 2023

onyb Dec 5, 2023

github-actions bot commented Dec 5, 2023 •

edited by thypon

Loading

Changes

brave-builds commented Dec 5, 2023

supermassive left a comment

github-actions bot commented Dec 12, 2023

Changes

Security Hotspots

simoarpe left a comment

brave-builds commented Dec 12, 2023

	const { data: sellTokenDecimals } = useGetEthTokenDecimalsQuery(
	sellToken &&
	sellToken.coingeckoId === UNKNOWN_TOKEN_COINGECKO_ID &&
	!isNativeToken(sellToken)
	? {
	chainId: sellToken.chainId,
	contractAddress: sellToken.contractAddress
	}
	: skipToken
	)
	const { data: sellTokenSymbol } = useGetEthTokenSymbolQuery(
	sellToken &&
	sellToken.coingeckoId === UNKNOWN_TOKEN_COINGECKO_ID &&
	!isNativeToken(sellToken)
	? {
	chainId: sellToken.chainId,
	contractAddress: sellToken.contractAddress
	}
	: skipToken
	)

	const { data: buyTokenDecimals } = useGetEthTokenDecimalsQuery(
	buyToken &&
	buyToken.coingeckoId === UNKNOWN_TOKEN_COINGECKO_ID &&
	!isNativeToken(buyToken)
	? {
	chainId: buyToken.chainId,
	contractAddress: buyToken.contractAddress
	}
	: skipToken
	)
	const { data: buyTokenSymbol } = useGetEthTokenSymbolQuery(
	buyToken &&
	buyToken.coingeckoId === UNKNOWN_TOKEN_COINGECKO_ID &&
	!isNativeToken(buyToken)
	? {
	chainId: buyToken.chainId,
	contractAddress: buyToken.contractAddress
	}
	: skipToken
	)

chore(wallet): add GetEthTokenInfo and refactor #21191

chore(wallet): add GetEthTokenInfo and refactor #21191

Conversation

onyb commented Dec 1, 2023 • edited Loading

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

josheleonard left a comment

Choose a reason for hiding this comment

StephenHeaps left a comment

Choose a reason for hiding this comment

brave-builds commented Dec 4, 2023

supermassive Dec 5, 2023

Choose a reason for hiding this comment

onyb Dec 5, 2023

Choose a reason for hiding this comment

supermassive Dec 5, 2023

Choose a reason for hiding this comment

onyb Dec 5, 2023

Choose a reason for hiding this comment

supermassive Dec 5, 2023

Choose a reason for hiding this comment

onyb Dec 5, 2023

Choose a reason for hiding this comment

github-actions bot commented Dec 5, 2023 • edited by thypon Loading

Description

Changes

Security Hotspots

brave-builds commented Dec 5, 2023

supermassive left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 12, 2023

Description

Changes

Security Hotspots

simoarpe left a comment

Choose a reason for hiding this comment

brave-builds commented Dec 12, 2023

onyb commented Dec 1, 2023 •

edited

Loading

github-actions bot commented Dec 5, 2023 •

edited by thypon

Loading