Conversation
| const { name } = (data && abiDecoder.decodeMethod(data)) || {} | ||
| let name | ||
| try { | ||
| name = data && hstInterface.parseTransaction({ data }).name |
There was a problem hiding this comment.
ethers throws errors when invalid ABIs are passed to it whereas abi-decoder simply returns undefined.
There was a problem hiding this comment.
I think that behavior is only relied on in transaction controller tests, but we don't need to investigate that further here.
ui/app/helpers/utils/token-util.js
Outdated
| const toAddressData = tokenParams.find((param) => param.name === '_to') | ||
| return toAddressData ? toAddressData.value : tokenParams[0].value | ||
| export function getTokenToAddress (tokenData = {}) { | ||
| return tokenData?.args?.['_to']?.toString().toLowerCase() |
There was a problem hiding this comment.
ethers uses checksummed addresses.
| if (test.tokenData) { | ||
| assert.equal(result.current.name, test.tokenData.name) | ||
| assert.equal(result.current.args[0].toLowerCase(), test.tokenData.args[0]) | ||
| assert.ok(test.tokenData.args[1].eq(result.current.args[1])) | ||
| } else { | ||
| assert.equal(result.current, test.tokenData) | ||
| } |
There was a problem hiding this comment.
The data returned by ethers includes a bunch of stuff that doesn't serialize well, so I decided to simply check some emblematic data that we use in practice. I'm pretty sure that the correctness of the data we check implies the correctness of everything else, should we ever want to use it.
| export function getTokenData (data) { | ||
| try { | ||
| return hstInterface.parseTransaction({ data }) | ||
| } catch (_) { | ||
| return undefined | ||
| } |
There was a problem hiding this comment.
Builds ready [d8c90d0]
Page Load Metrics (478 ± 68 ms)
|
Gudahtt
left a comment
There was a problem hiding this comment.
It looks like there are still a few required changes as a result of the confirmTransaction.tokenData state. e.g there is a selector in ui/app/selectors/confirm-transaction.js that still references tokenData.params
Builds ready [b19f2ea]
Page Load Metrics (476 ± 60 ms)
|
|
@Gudahtt I addressed the |
|
After reviewing the Human Standard Token ABI (and our quite loose usage of the token param value getters), I restored their internal logic and added docstrings in 6279f39. In short, |
Builds ready [6279f39]
Page Load Metrics (422 ± 59 ms)
|
Replaces
abi-decoderwith the equivalent functionality inethers.This required significant changes to our unit tests, as the APIs and interfaces are quite different.