Skip to content

Feature/preimage store rebased#10208

Draft
garyschulte wants to merge 2 commits intobesu-eth:mainfrom
garyschulte:feature/preimage-store-rebased
Draft

Feature/preimage store rebased#10208
garyschulte wants to merge 2 commits intobesu-eth:mainfrom
garyschulte:feature/preimage-store-rebased

Conversation

@garyschulte
Copy link
Copy Markdown
Contributor

@garyschulte garyschulte commented Apr 9, 2026

PR description

fixes #7796

rebased/refactored preimage store PRs, see #7800 , #9553

What is still outstanding to come out of draft / be mergeable to main?

  • address the additional rocksdb transaction commit for preimage store, ideally work it into the trielog transaction for not only durability/correctness of preimage state in the case of an abnormal shutdown, as well as write performance
  • measure block execution and io performance, verify there is no significant regression when preimage feature is default disabled
  • verify forest in-memory preimage cache is not broken by this refactor (at least until forest is removed)

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
  • hive tests: Engine or other RPCs modified?

Signed-off-by: garyschulte <garyschulte@gmail.com>
…ill is introducing an additional commit of preimagestorage. subsequent commits should add preimage storage to the trielog transaction commit, eliminating the additional transaction commit.

Signed-off-by: garyschulte <garyschulte@gmail.com>
BlockchainSetupUtil::mainnetProtocolContextProvider,
new EthScheduler(1, 1, 1, 1, new NoOpMetricsSystem()),
serviceManager);
new EthScheduler(1, 1, 1, 1, new NoOpMetricsSystem()));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is unwanted refactor. we should keep the non-null service manager constructor

final WorldStateArchive worldArchive =
storageFormat == DataStorageFormat.BONSAI
? createBonsaiInMemoryWorldStateArchive(blockchain, serviceManager)
? createBonsaiInMemoryWorldStateArchive(blockchain, null)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

same

.filter(val -> val.size() == Address.SIZE)
.map(val -> Address.wrap(Bytes.wrap(val)));
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

if getRawPreimage() just a test consideration, we should remove it. The interface should not expose implementation details like shared hash preimage pool across trie keys and accounts

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feature: Implement hash preimage storage persistence

1 participant