-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Turned PostMergeContext to be a regular class instead of a singleton.… #8248
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -89,6 +89,7 @@ | |
| import org.hyperledger.besu.config.GenesisConfig; | ||
| import org.hyperledger.besu.config.GenesisConfigOptions; | ||
| import org.hyperledger.besu.config.MergeConfiguration; | ||
| import org.hyperledger.besu.consensus.merge.PostMergeContext; | ||
| import org.hyperledger.besu.controller.BesuController; | ||
| import org.hyperledger.besu.controller.BesuControllerBuilder; | ||
| import org.hyperledger.besu.crypto.Blake2bfMessageDigest; | ||
|
|
@@ -1836,6 +1837,7 @@ public BesuControllerBuilder setupControllerBuilder() { | |
| .cacheLastBlocks(numberOfblocksToCache) | ||
| .genesisStateHashCacheEnabled(genesisStateHashCacheEnabled) | ||
| .apiConfiguration(apiConfigurationSupplier.get()) | ||
| .postMergeContext(new PostMergeContext()) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this can be simpler. There is no need to expose the |
||
| .besuComponent(besuComponent); | ||
| if (DataStorageFormat.BONSAI.equals(getDataStorageConfiguration().getDataStorageFormat())) { | ||
| final DiffBasedSubStorageConfiguration subStorageConfiguration = | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -147,7 +147,8 @@ protected MiningCoordinator createMiningCoordinator( | |
| transitionMiningConfiguration, | ||
| syncState, | ||
| transitionBackwardsSyncContext, | ||
| ethProtocolManager.ethContext().getScheduler())); | ||
| ethProtocolManager.ethContext().getScheduler()), | ||
| preMergeBesuControllerBuilder.postMergeContext); | ||
| initTransitionWatcher(protocolContext, composedCoordinator); | ||
| return composedCoordinator; | ||
| } | ||
|
|
@@ -185,7 +186,7 @@ protected ProtocolSchedule createProtocolSchedule() { | |
| new TransitionProtocolSchedule( | ||
| preMergeBesuControllerBuilder.createProtocolSchedule(), | ||
| mergeBesuControllerBuilder.createProtocolSchedule(), | ||
| PostMergeContext.get()); | ||
| postMergeContext); | ||
| return transitionProtocolSchedule; | ||
| } | ||
|
|
||
|
|
@@ -205,6 +206,8 @@ protected ConsensusContext createConsensusContext( | |
| final Blockchain blockchain, | ||
| final WorldStateArchive worldStateArchive, | ||
| final ProtocolSchedule protocolSchedule) { | ||
| preMergeBesuControllerBuilder.postMergeContext(postMergeContext); | ||
| mergeBesuControllerBuilder.postMergeContext(postMergeContext); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please note this change. I couldn't find a better way to get around this and I wonder if there's a better way |
||
| return new TransitionContext( | ||
| preMergeBesuControllerBuilder.createConsensusContext( | ||
| blockchain, worldStateArchive, protocolSchedule), | ||
|
|
@@ -290,7 +293,7 @@ public BesuControllerBuilder storageProvider(final StorageProvider storageProvid | |
| @Override | ||
| public BesuController build() { | ||
| final BesuController controller = super.build(); | ||
| PostMergeContext.get().setSyncState(controller.getSyncState()); | ||
| super.postMergeContext.setSyncState(controller.getSyncState()); | ||
| return controller; | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need a tip on this part. ThreadBesuNodeRunner can only create a Besu instance that doesn't have persistence, meaning it's losing all the state after restart. In-memory KeyValueStorageProvider is provided by BesuControllerModule::provideKeyValueStorageProvider called by dagger. I wonder if there's a non invasive way to invert this dependency and provide a different key value storage, namely I'd like to set it to a persistent version for my testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jflo thoughts on dependency nuances here?