Skip to content

Speed up ID precompile #8529

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

lu-pinto
Copy link
Member

@lu-pinto lu-pinto commented Apr 8, 2025

PR description

If one looks at Bytes it is supposed to be an immutable structure so there wouldn't be any need for copying it in IDPrecompileContract. However, looking closely at where this data comes from is required to know if the backing byte[] can also be mutated from other sources.
Currently inputData that is passed to the ID precompile can only come from 3 sources:

  1. Transaction data
  2. From memory during a CALL
  3. From memory during a CREATE

To me CALL seems to be the only place that is passing a Bytes instance that can be mutated later, so by making a copy of it before returning we ensure that modifying memory does not change the returned Bytes instance.
I'm also cutting off a bit of fat in MessageFrame.readMemory because we are actually copying the same data twice.

This needs to go through performance benchmarks before gets pulled in and a full block import with mainnet to be safe.

Fixed Issue(s)

Thanks for sending a pull request! Have you done the following?

  • Checked out our contribution guidelines?
  • Considered documentation and added the doc-change-required label to this PR if updates are required.
  • Considered the changelog and included an update if required.
  • For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

  • spotless: ./gradlew spotlessApply
  • unit tests: ./gradlew build
  • acceptance tests: ./gradlew acceptanceTest
  • integration tests: ./gradlew integrationTest
  • reference tests: ./gradlew ethereum:referenceTests:referenceTests

@lu-pinto lu-pinto added performance DO NOT MERGE label to not merge PR yet - PR has some pending checks before merging labels Apr 8, 2025
@lu-pinto lu-pinto marked this pull request as draft April 8, 2025 15:22
@lu-pinto lu-pinto force-pushed the speedup-id-precompile branch 4 times, most recently from 35abb34 to af78fff Compare April 16, 2025 09:29
@lu-pinto lu-pinto force-pushed the speedup-id-precompile branch from af78fff to dbb0450 Compare April 16, 2025 20:18
Copy link

This pr is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the Stale label May 17, 2025
@lu-pinto lu-pinto removed the Stale label May 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO NOT MERGE label to not merge PR yet - PR has some pending checks before merging performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant