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

Make sure prewarming and pruning trie store is running on ethereum tests #8076

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

asdacap
Copy link
Contributor

@asdacap asdacap commented Jan 20, 2025

  • Double check that ethereum tests passed with prewarmer and pruning trie store.
  • Aside from needing to explicitly clear cache, seems to work fine.

Types of changes

What types of changes does your code introduce?

  • New feature (a non-breaking change that adds functionality)

Testing

Requires testing

  • No?

If yes, did you write tests?

  • Yes

Base automatically changed from feature/end-to-end-synctest to master January 20, 2025 01:44
@asdacap asdacap force-pushed the feature/ensure-prewarming-is-running-en-ethereum-tests branch from bfb5d8d to ed58365 Compare January 20, 2025 01:46
@@ -104,6 +104,8 @@ public interface IWorldState : IJournal<Snapshot>, IReadOnlyStateProvider

void DecrementNonce(Address address) => DecrementNonce(address, UInt256.One);

void SetNonce(Address address, in UInt256 nonce);
Copy link
Member

Choose a reason for hiding this comment

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

This is not a normal use-case method when interacting with WorldState. Might be useful on some setups, but not sure if should be just public on interface.

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'd say set it public. rbuilder need it and it'll make it easier to test paprika.

Comment on lines +24 to +26
IWorldState WorldState,
IBlockProcessor BlockProcessor,
ITransactionProcessor TransactionProcessor,
Copy link
Member

Choose a reason for hiding this comment

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

Added parameters are not used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its used as a property.

@@ -247,6 +204,13 @@ protected async Task<EthereumTestResult> RunTest(BlockchainTest test, Stopwatch?
await blockchainProcessor.StopAsync(true);
stopwatch?.Stop();

IBlockCachePreWarmer? preWarmer = container.Resolve<MainBlockProcessingContext>().LifetimeScope.ResolveOptional<IBlockCachePreWarmer>();
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if, but does this create any issue with parallel test invocation? If not can you be more specific about it? I think maybe LifetimeScope solves this but it isn't totally clear for me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does not? Each invocation creates a completely different instance.

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.

2 participants