Skip to content

feat: improves eth_getCode caching #3698

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

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

Conversation

simzzz
Copy link
Contributor

@simzzz simzzz commented Apr 16, 2025

Description:
This PR improves the caching for eth_getCode by caching even with prohibited opcodes, and removing caching for blockNumber === latest

Related issue(s):

Fixes #3634

Checklist

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

@simzzz simzzz requested review from a team as code owners April 16, 2025 13:05
@simzzz simzzz requested a review from Ferparishuertas April 16, 2025 13:05
@simzzz simzzz self-assigned this Apr 16, 2025
@simzzz simzzz added the enhancement New feature or request label Apr 16, 2025
@simzzz simzzz added this to the 0.68.0 milestone Apr 16, 2025
Copy link

github-actions bot commented Apr 16, 2025

Test Results

 23 files   -   1  271 suites   - 59   37m 7s ⏱️ - 10m 15s
634 tests ±  0  620 ✅ +  8  4 💤 ±0  10 ❌  - 8 
718 runs   - 286  703 ✅  - 277  4 💤 ±0  11 ❌  - 9 

For more details on these failures, see this check.

Results for commit 2a05e8f. ± Comparison against base commit 4fa65dc.

This pull request removes 4 and adds 4 tests. Note that renamed tests count towards both.
"before all" hook for "@release should execute "eth_getTransactionCount" primary" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests eth_getTransactionCount "before all" hook for "@release should execute "eth_getTransactionCount" primary"
"before all" hook for "Function calling HederaTokenService.isToken(token)" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests eth_call with contract that calls precompiles "before all" hook for "Function calling HederaTokenService.isToken(token)"
"before all" hook: beforeFunc for "001 Should call pureMultiply" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests eth_call Caller contract With evm address "before all" hook: beforeFunc for "001 Should call pureMultiply"
"before all" hook: beforeFunc for "001 Should call pureMultiply" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests eth_call Caller contract With long-zero address "before all" hook: beforeFunc for "001 Should call pureMultiply"
"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 all" hook in "HBAR Rate Limit Tests" ‑ RPC Server Acceptance Tests Acceptance tests @hbarlimiter HBAR Limiter Acceptance Tests HBAR Rate Limit Tests "before all" hook in "HBAR Rate Limit Tests"
"before all" hook in "RPC Server Acceptance Tests" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-1 RPC Server Acceptance Tests RPC Server Acceptance Tests "before all" hook in "RPC Server Acceptance Tests"
"before each" hook for "should execute "eth_getStorageAt" request to get current state changes" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests "before each" hook for "should execute "eth_getStorageAt" request to get current state changes"

♻️ This comment has been updated with latest results.

Signed-off-by: Simeon Nakov <[email protected]>
// First call with 'latest' should make network calls
await ethImpl.getCode(CONTRACT_ADDRESS_1, 'latest', requestDetails);
const afterFirstGetCount = restMock.history.get.length;
expect(afterFirstGetCount).to.be.greaterThan(initialGetCount);
Copy link
Contributor

Choose a reason for hiding this comment

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

@simzzz Can we make this test slightly more specific. e.g instead of checking if its greater than we may expect an exact count, or maybe just check if the cache service is not called at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made an improvement that I think makes it more clear now. You can take a look @konstantinabl

Signed-off-by: Simeon Nakov <[email protected]>
Copy link
Contributor

@quiet-node quiet-node left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve eth_getCode caching strategy when bytecode contains "prohibited opcode"
3 participants