Make ethersStateManager more awesome
#2720
Conversation
acolytec3
added
dependencies
Codecov Report
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. |
| @@ -52,7 +55,7 @@ export class EthersStateManager implements EVMStateManagerInterface { | |||
| }) | |||
| ;(newState as any).contractCache = new Map(this.contractCache) | |||
| ;(newState as any).storageCache = new StorageCache({ size: 10000, type: CacheType.LRU }) | |||
| ;(newState as any)._accountCache = this._accountCache | |||
| ;(newState as any)._accountCache = new AccountCache({ size: 10000, type: CacheType.LRU }) | |||
There was a problem hiding this comment.
We shouldn't use an LRU cache for EthersStateManager since this is such an ad-hoc usage, also just a general reminder that something like 10000 for LRU is still "unsafe" since too low, so this would e.g. break on mainnet genesis since genesis state accounts are > 10.000 (admittedly an obscure use case for ESM, but just as an example). Maybe we still need to point this out more clearly in the docs adding some "real" number advices ("only use LRU with size >= 50000"). Or maybe also an option: we just plainly throw below a certain threshold, might be too harsh though. 🤔
Anyhow: generally this looks great, I will directly push and change to ORDERED_MAP cache and fix another thing, then we can merge.
I would generally be a fan if we can also harmonize the options over time (so: before the releases), it would definitely make sense and be useful if people have the option here as well to customize the caches, also from a general design PoV it would be good to have this harmonized.
Just wouldn't want to do now because this might interfere too much with the work from @jochem-brouwer on EEI/StateManager, so likely something for a PR beyond that (or Jochem does this directly when integrating these changes here).
Some side remarks on this task: so we should definitely also create a normal types.ts file for SM and put the options there, just realized that we haven't yet. Also wonder if this prefixCodeHashes option is still necessary? Rather a question for Jochem though.
…move console.log() from tests
ethersdependency to v6mockProviderto reflectethersv6 provider API#operator syntax thatethersuses to specify private class membersethersStateManagerlockfile-lintCI run to allow for newetherssubdependency that installs from github