Stop using 4bytes for contract deployment #14598
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
|
Can you plz add some description to the PR and if possible test coverage. |
Builds ready [23b7571]Page Load Metrics (1773 ± 41 ms)
highlights: |
| title = | ||
| (methodData?.name && camelCaseToCapitalize(methodData.name)) || | ||
| transactionTypeTitle; |
There was a problem hiding this comment.
Do we ever add methodData to a transaction where the to address is null/undefined?
There was a problem hiding this comment.
We shouldn't. We should only use it for valid method calls to a contract.
There was a problem hiding this comment.
I feel like ideally we would guarantee that methodData won't be attached to a transaction where address is empty or zero address further upstream (maybe in the TransactionsController), but won't block on it here...
There was a problem hiding this comment.
in what scenario would we have a transaction without a 'to' address?
There was a problem hiding this comment.
I think when you deploy a contract no?
There was a problem hiding this comment.
🤦🏻 yeah. that makes sense...
There was a problem hiding this comment.
We don't attach methoData anywhere, its fetched at that top of the useTransactionDisplayData hook.
There was a problem hiding this comment.
Yeah, it's not affiliated with the transaction outside of this hook afaik @adonesky1
Added a description and test coverage! 👍 |
| title = | ||
| (methodData?.name && camelCaseToCapitalize(methodData.name)) || | ||
| transactionTypeTitle; |
There was a problem hiding this comment.
I feel like ideally we would guarantee that methodData won't be attached to a transaction where address is empty or zero address further upstream (maybe in the TransactionsController), but won't block on it here...
Builds ready [13d7fe4]Page Load Metrics (1717 ± 29 ms)
highlights: |
Explanation
This PR fixes a bug where we would try to show called method names for contract deployments. Since contract deployments have the bytecode as the data we can't look up method signatures using the first 4 bytes as you can with a normal contract interaction.
Screenshots/Screencaps
Before
After
Manual Testing Steps
Pre-Merge Checklist