Skip to content

Conversation

@AurelienRichez
Copy link
Contributor

Description

We accidentally removed the storage key preimage cache in test mode during our refactoring, which broke lots of ETS tests.

This PR fixes that to make those ETS test work again

} yield execResult.copy(worldState = worldPersisted)
}

protected def buildInitialWorld(block: Block, parentHeader: BlockHeader): InMemoryWorldStateProxy = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok this is weird. apparently this simple change causes this error :

[error] /.../mantis/src/main/scala/io/iohk/ethereum/ledger/BlockExecution.scala:146:3: Could not find a
ny member to link for "BlockExecutionError".
[error]   /** Executes and validates a list of blocks, storing the results in the blockchain.
[error]   ^
[error] one error found
[error] Scaladoc generation failed

The class is mentioned in the scaladoc, and replacing it with a full name with the namespace solves the problem. But I don't really understand how this change causes a problem.

private var currentConfig: BlockchainConfig = initialConfig
private var blockTimestamp: Long = 0
private var sealEngine: SealEngineType = SealEngineType.NoReward
private val preimageCache: collection.concurrent.Map[ByteString, UInt256] =
Copy link
Contributor

@lukasz-golebiewski lukasz-golebiewski Jun 30, 2021

Choose a reason for hiding this comment

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

Looks like this preimageCache is used to store only storage data. If that's the case it would make sense to make this obvious from the type definition by using AnyVal wrappers for ByteString and UInt256 like say StorageHash, StorageKey

@AurelienRichez AurelienRichez merged commit cf30683 into develop Jun 30, 2021
@AurelienRichez AurelienRichez deleted the fix/ETCM-964-fix-vmArithmeticTest branch June 30, 2021 12:30
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.

4 participants