Skip to content
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

fix: fixed eth_getCode to now use blockNumber when address is an HTS token #3433

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

simzzz
Copy link
Contributor

@simzzz simzzz commented Jan 28, 2025

Description:
If we pass an HTS token address to getCode, we do not take into account the block number, resulting in a bug where it would return the smart contract code even if the block number provided is earlier than the one where the token was created.

Related issue(s):

Fixes #3155

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@simzzz simzzz requested review from a team as code owners January 28, 2025 12:12
@simzzz simzzz requested a review from bubo January 28, 2025 12:12
Copy link

github-actions bot commented Jan 28, 2025

Test Results

 22 files  ± 0  278 suites  +7   44m 6s ⏱️ - 21m 37s
621 tests +12  607 ✅ +15  4 💤 +1  10 ❌  - 4 
847 runs  + 1  829 ✅ + 5  6 💤 +2  12 ❌  - 6 

For more details on these failures, see this check.

Results for commit 7a9e00b. ± Comparison against base commit 373f1dc.

This pull request removes 3 and adds 15 tests. Note that renamed tests count towards both.
"after all" hook in "RPC Server Acceptance Tests" ‑ RPC Server Acceptance Tests "after all" hook in "RPC Server Acceptance Tests"
"before all" hook for "should execute "eth_getCode" for hts token" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests eth_getCode "before all" hook for "should execute "eth_getCode" for hts token"
"before each" hook for "from/to Addresses when transferring HTS tokens to the tokenAddress are in evm and long-zero format" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests "before each" hook for "from/to Addresses when transferring HTS tokens to the tokenAddress are in evm and long-zero format"
"before all" hook in "Debug API Test Suite" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests Debug API Test Suite "before all" hook in "Debug API Test Suite"
"before each" hook for "should throw error for invalid block number" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests "before each" hook for "should throw error for invalid block number"
@release should execute "eth_getCode" for contract evm_address ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests eth_getCode @release should execute "eth_getCode" for contract evm_address
@release should execute "eth_getCode" for contract with id converted to evm_address ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests eth_getCode @release should execute "eth_getCode" for contract with id converted to evm_address
should execute "eth_getBlockByNumber", hydrated transactions = true for a block that contains a call with CONTRACT_NEGATIVE_VALUE status ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-1 RPC Server Acceptance Tests RPC Server Acceptance Tests Block related RPC calls should execute "eth_getBlockByNumber", hydrated transactions = true for a block that contains a call with CONTRACT_NEGATIVE_VALUE status
should execute "eth_getCode" for hts token ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests eth_getCode should execute "eth_getCode" for hts token
should not return contract bytecode after sefldestruct ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests eth_getCode should not return contract bytecode after sefldestruct
should return 0x0 for account alias on eth_getCode ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests eth_getCode should return 0x0 for account alias on eth_getCode
should return 0x0 for account evm_address on eth_getCode ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests eth_getCode should return 0x0 for account evm_address on eth_getCode
should return 0x0 for non-existing contract on eth_getCode ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests eth_getCode should return 0x0 for non-existing contract on eth_getCode
…

♻️ This comment has been updated with latest results.

@simzzz simzzz added the bug Something isn't working label Jan 28, 2025
@simzzz simzzz added this to the 0.64.3 milestone Jan 28, 2025
Signed-off-by: Simeon Nakov <[email protected]>
Signed-off-by: Simeon Nakov <[email protected]>
Signed-off-by: Simeon Nakov <[email protected]>
@quiet-node quiet-node modified the milestones: 0.64.3, 0.66.0 Jan 29, 2025
@simzzz simzzz requested a review from Nana-EC as a code owner January 31, 2025 14:23
Signed-off-by: Simeon Nakov <[email protected]>
Signed-off-by: Simeon Nakov <[email protected]>
@quiet-node
Copy link
Member

@acuarica please kindly take a look when you have a chance to see if it meet yoru original requirements. Thanks much!

packages/relay/src/lib/eth.ts Show resolved Hide resolved
throw predefined.UNKNOWN_BLOCK(`Block number ${blockNumber} does not exist`);
}

const tokenId = Utils.addressToTokenId(address);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assumes address is a long zero address, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct

packages/relay/src/lib/eth.ts Outdated Show resolved Hide resolved
Copy link

sonarqubecloud bot commented Feb 4, 2025

Copy link

codecov bot commented Feb 4, 2025

Codecov Report

Attention: Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.

Project coverage is 84.45%. Comparing base (fe6c0cf) to head (7a9e00b).
Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
packages/relay/src/lib/eth.ts 81.81% 1 Missing and 1 partial ⚠️
packages/relay/src/lib/clients/mirrorNodeClient.ts 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3433      +/-   ##
==========================================
- Coverage   85.20%   84.45%   -0.76%     
==========================================
  Files          69       69              
  Lines        4711     4772      +61     
  Branches     1048     1064      +16     
==========================================
+ Hits         4014     4030      +16     
- Misses        397      428      +31     
- Partials      300      314      +14     
Flag Coverage Δ
config-service 98.14% <ø> (ø)
relay 79.43% <83.33%> (+0.21%) ⬆️
server 83.30% <ø> (ø)
ws-server 36.66% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/relay/src/utils.ts 97.87% <100.00%> (+0.19%) ⬆️
packages/relay/src/lib/clients/mirrorNodeClient.ts 88.57% <50.00%> (+0.25%) ⬆️
packages/relay/src/lib/eth.ts 81.70% <81.81%> (-4.42%) ⬇️

... and 6 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

eth_getCode does not use blockNumber when address corresponds to an HTS token
3 participants