Skip to content

feat: update token for upgradable transactions where implementation i…#348

Open
kiriyaga-txfusion wants to merge 2 commits intomainfrom
kiriyaga-txfusion-proxy-transfers
Open

feat: update token for upgradable transactions where implementation i…#348
kiriyaga-txfusion wants to merge 2 commits intomainfrom
kiriyaga-txfusion-proxy-transfers

Conversation

@kiriyaga-txfusion
Copy link
Copy Markdown
Contributor

@kiriyaga-txfusion kiriyaga-txfusion commented Dec 4, 2024

…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

  • [+] PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • [+] Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 4, 2024

API E2E Test Results

207 tests   207 ✅  19s ⏱️
 14 suites    0 💤
  1 files      0 ❌

Results for commit a7e1e8a.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 4, 2024

Unit Test Results

    4 files    260 suites   12m 19s ⏱️
2 100 tests 2 099 ✅ 1 💤 0 ❌
2 314 runs  2 313 ✅ 1 💤 0 ❌

Results for commit a7e1e8a.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 4, 2024

Visit the preview URL for this PR:
https://staging-scan-v2--pr-348-rffzf1bu.web.app

"OwnershipTransferred(address,address)",
"AdminChanged(address,address)",
"OwnershipTransferred(address,address)",
];
Copy link
Copy Markdown
Collaborator

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 is previousOwner and the second one is newOwner, why the previous owner (topics[1]) is used as the implementation address?

transactionHash: txReceipt.hash,
creatorAddress: txReceipt.from,
logIndex: log.index,
implementationAddress: address,
Copy link
Copy Markdown
Collaborator

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",
Copy link
Copy Markdown
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet