-
Notifications
You must be signed in to change notification settings - Fork 115
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
feat: update token for upgradable transactions where implementation i… #348
base: main
Are you sure you want to change the base?
Conversation
API E2E Test Results207 tests 207 ✅ 19s ⏱️ Results for commit a7e1e8a. ♻️ This comment has been updated with latest results. |
Unit Test Results 4 files 260 suites 12m 19s ⏱️ Results for commit a7e1e8a. ♻️ This comment has been updated with latest results. |
Visit the preview URL for this PR: |
"OwnershipTransferred(address,address)", | ||
"AdminChanged(address,address)", | ||
"OwnershipTransferred(address,address)", | ||
]; |
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.
OwnershipTransferred(address,address)
is listed twice.AdminChanged(address,address)
doesn't contain indexed arguments, does it? If so,log.topics[1]
will be always null for this event.OwnershipTransferred(address,address)
- where the first address ispreviousOwner
and the second one isnewOwner
, why the previous owner (topics[1]) is used as the implementation address?
transactionHash: txReceipt.hash, | ||
creatorAddress: txReceipt.from, | ||
logIndex: log.index, | ||
implementationAddress: address, |
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.
After the latest changes were merged from main, the TS compiler complains that the isEvmLike
field is missing for the ProxyAddress
type. The exact error can be checked locally or in the failed GitHub Action for this PR.
transactionHash: "0x5e018d2a81dbd1ef80ff45171dd241cb10670dcb091e324401ff8f52293841b0", | ||
address: "0x1BEB2aBb1678D8a25431d9728A425455f29d12B7", | ||
topics: [ | ||
"0xbc7cd75a20ee27fd9adebab32041f755214dbc6bffa90cc0225b39da2e5c2d3b", |
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.
Let's add test examples that test all event types that the handler processes. Currently only 0xbc7cd75a20ee27fd9adebab32041f755214dbc6bffa90cc0225b39da2e5c2d3b
is covered. Ideally, I'd use real log examples.
…s token
What ❔
This PR updates the token reference in the database for Transparent and UUPS proxies linked to ERC20 token implementations. The upgradable transaction handler detects and replaces outdated tokens with the correct updated version.
Why ❔
Users can modify the symbol, name, and decimals of tokens deployed as proxies, so these details must be updated in the database to ensure they are displayed correctly in the UI. Note that Etherscan does not support this feature, and certain proxy patterns, such as Beacon proxies, are not affected in our case. (since beacon is upgraded but beacon proxy points to him).
fixes #326
Checklist