-
Notifications
You must be signed in to change notification settings - Fork 5k
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
refactor: memoize fetchErc20Decimals, relocate to utils/token, and apply to other instances #27088
Merged
digiwand
merged 21 commits into
develop
from
refactor-token-decimals-fetch-logic-and-memoize
Sep 19, 2024
Merged
Changes from 17 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
b8fec4a
refactor: mv fetchErc20Decimals to utils/token
digiwand d9913ab
fix: lint needs EOL
digiwand 171c4a1
refactor: use fetchErc20Decimals in permit simulation and dataTree
digiwand eff0525
refactor: memoize fetchErc20Decimals
digiwand a83c8af
fix: lint add EOF line
digiwand 557d8a4
test: memoized fetchErc20Decimals util
digiwand fb228be
test:fix: fetchErc20Decimals test phrasing
digiwand 282a8a6
fix:test: uncomment needed afterEach code
digiwand 6f9b3c3
fix: yarn lint:fix
digiwand 68fa1c5
fix:test: useBalanceChanges memoized fetchErc20Decimals
digiwand 7c4afaf
fix:test: mv useBalanceChanges memoized clear cache
digiwand d07cc85
Merge branch 'develop' into refactor-token-decimals-fetch-logic-and-m…
digiwand 595975e
fix: allow string with hex token addresses
digiwand 6ab6f3b
fix:test: memoize fetchErc20Decimals for confirm test
digiwand 09a9fee
fix: lint utils/token comment
digiwand dba0ba3
Merge branch 'develop' into refactor-token-decimals-fetch-logic-and-m…
digiwand 5602fd9
Merge branch 'develop' into refactor-token-decimals-fetch-logic-and-m…
digiwand 913a79a
refactor: rm new utils/token.ts
digiwand a7c315d
Merge branch 'develop' into refactor-token-decimals-fetch-logic-and-m…
digiwand 4f8e366
fix: cherry-pick PermitSimulationValueDisplay code
digiwand 99cfb20
Revert "fix: cherry-pick PermitSimulationValueDisplay code"
digiwand File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export const ERC20_DEFAULT_DECIMALS = 18; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
import { getTokenStandardAndDetails } from '../../../store/actions'; | ||
import { ERC20_DEFAULT_DECIMALS } from '../constants/token'; | ||
import { fetchErc20Decimals } from './token'; | ||
|
||
const MOCK_ADDRESS = '0x514910771af9ca656af840dff83e8264ecf986ca'; | ||
const MOCK_DECIMALS = 36; | ||
|
||
jest.mock('../../../store/actions', () => ({ | ||
getTokenStandardAndDetails: jest.fn(), | ||
})); | ||
|
||
describe('fetchErc20Decimals', () => { | ||
afterEach(() => { | ||
jest.clearAllMocks(); | ||
|
||
/** Reset memoized function using getTokenStandardAndDetails for each test */ | ||
fetchErc20Decimals?.cache?.clear?.(); | ||
}); | ||
|
||
it(`should return the default number, ${ERC20_DEFAULT_DECIMALS}, if no decimals were found from details`, async () => { | ||
(getTokenStandardAndDetails as jest.Mock).mockResolvedValue({}); | ||
const decimals = await fetchErc20Decimals(MOCK_ADDRESS); | ||
|
||
expect(decimals).toBe(ERC20_DEFAULT_DECIMALS); | ||
}); | ||
|
||
it('should return the decimals for a given token address', async () => { | ||
(getTokenStandardAndDetails as jest.Mock).mockResolvedValue({ | ||
decimals: MOCK_DECIMALS, | ||
}); | ||
const decimals = await fetchErc20Decimals(MOCK_ADDRESS); | ||
|
||
expect(decimals).toBe(MOCK_DECIMALS); | ||
}); | ||
|
||
it('should memoize the result for the same token addresses', async () => { | ||
(getTokenStandardAndDetails as jest.Mock).mockResolvedValue({ | ||
decimals: MOCK_DECIMALS, | ||
}); | ||
|
||
const firstCallResult = await fetchErc20Decimals(MOCK_ADDRESS); | ||
const secondCallResult = await fetchErc20Decimals(MOCK_ADDRESS); | ||
|
||
expect(firstCallResult).toBe(secondCallResult); | ||
expect(getTokenStandardAndDetails).toHaveBeenCalledTimes(1); | ||
|
||
await fetchErc20Decimals('0xDifferentAddress'); | ||
expect(getTokenStandardAndDetails).toHaveBeenCalledTimes(2); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
import { memoize } from 'lodash'; | ||
import { Hex } from '@metamask/utils'; | ||
import { getTokenStandardAndDetails } from '../../../store/actions'; | ||
import { ERC20_DEFAULT_DECIMALS } from '../constants/token'; | ||
|
||
/** | ||
* Fetches the decimals for the given token address. | ||
* | ||
* @param {Hex | string} address - The ethereum token contract address. It is expected to be in hex format. | ||
* We currently accept strings since we have a patch that accepts a custom string | ||
* {@see .yarn/patches/@metamask-eth-json-rpc-middleware-npm-14.0.1-b6c2ccbe8c.patch} | ||
*/ | ||
export const fetchErc20Decimals = memoize( | ||
async (address: Hex | string): Promise<number> => { | ||
try { | ||
const { decimals: decStr } = await getTokenStandardAndDetails(address); | ||
if (!decStr) { | ||
return ERC20_DEFAULT_DECIMALS; | ||
} | ||
for (const radix of [10, 16]) { | ||
const parsedDec = parseInt(decStr, radix); | ||
if (isFinite(parsedDec)) { | ||
return parsedDec; | ||
} | ||
} | ||
return ERC20_DEFAULT_DECIMALS; | ||
} catch { | ||
return ERC20_DEFAULT_DECIMALS; | ||
} | ||
}, | ||
); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Minor, given there is very little in
index.ts
currently, do we warrant this level of modularity?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.
questionable, so updated this to share
ui/pages/confirmations/utils/token.ts