From 2405da8d5c9ace9601f0312c48b7be997a9a8c29 Mon Sep 17 00:00:00 2001 From: Scorbajio Date: Thu, 12 Oct 2023 02:10:03 -0700 Subject: [PATCH] Code cache should be able to save prestate without reading from statemanager DB (#3080) * Await accountEval when using it's return value in asserts * Fix account cache revert and _saveCachePreState * Do not use value from db for putting code into cache * Fix prestate save and revert logic * Update packages/statemanager/src/cache/code.ts * Check if key exists in map before checking the element to not overwrite undefined values * Change code comment to be more clear * Simplify save prestate check --------- Co-authored-by: acolytec3 <17355484+acolytec3@users.noreply.github.com> Co-authored-by: Holger Drewes --- packages/statemanager/src/cache/account.ts | 6 +- packages/statemanager/src/cache/code.ts | 28 ++------ packages/statemanager/src/stateManager.ts | 9 +-- .../test/checkpointing.account.spec.ts | 64 +++++++++---------- 4 files changed, 42 insertions(+), 65 deletions(-) diff --git a/packages/statemanager/src/cache/account.ts b/packages/statemanager/src/cache/account.ts index a485e8db33..4424c49e08 100644 --- a/packages/statemanager/src/cache/account.ts +++ b/packages/statemanager/src/cache/account.ts @@ -49,15 +49,15 @@ export class AccountCache extends Cache { } _saveCachePreState(cacheKeyHex: string) { - const it = this._diffCache[this._checkpoints].get(cacheKeyHex) - if (it === undefined) { + const diffMap = this._diffCache[this._checkpoints] + if (!diffMap.has(cacheKeyHex)) { let oldElem: AccountCacheElement | undefined if (this._lruCache) { oldElem = this._lruCache!.get(cacheKeyHex) } else { oldElem = this._orderedMapCache!.getElementByKey(cacheKeyHex) } - this._diffCache[this._checkpoints].set(cacheKeyHex, oldElem) + diffMap.set(cacheKeyHex, oldElem) } } diff --git a/packages/statemanager/src/cache/code.ts b/packages/statemanager/src/cache/code.ts index 33f9255b7b..f28f607ce0 100644 --- a/packages/statemanager/src/cache/code.ts +++ b/packages/statemanager/src/cache/code.ts @@ -52,32 +52,17 @@ export class CodeCache extends Cache { * Saves the state of the code cache before making changes to it. * * @param cacheKeyHex Account key for which code is being modified. - * @param currentPutCode New value of code that is being saved for the account. - * @param codeExists If account codeHash is null or not. Needed in the case of `putContractCode` for checking if the account has a non-null value. */ - _saveCachePreState( - cacheKeyHex: string, - currentPutCode?: Uint8Array | undefined, - codeExists?: boolean | undefined - ) { - const it = this._diffCache[this._checkpoints].get(cacheKeyHex) - if (it === undefined) { + _saveCachePreState(cacheKeyHex: string) { + const diffMap = this._diffCache[this._checkpoints] + if (!diffMap.has(cacheKeyHex)) { let oldElem: CodeCacheElement | undefined if (this._lruCache) { oldElem = this._lruCache.get(cacheKeyHex) } else { oldElem = this._orderedMapCache!.getElementByKey(cacheKeyHex) } - - // if the account has no code before this modification, save diff value as undefined so that in case - // of a revert, the code will be deleted and removed from the account - let val - if (codeExists !== undefined && codeExists === false) { - val = undefined - } else { - val = currentPutCode - } - this._diffCache[this._checkpoints].set(cacheKeyHex, oldElem ?? { code: val }) + diffMap.set(cacheKeyHex, oldElem) } } @@ -86,11 +71,10 @@ export class CodeCache extends Cache { * * @param address - Address of account code is being modified for. * @param code - Bytecode or undefined if code doesn't exist. - * @param codeExists - If account codeHash is null or not. Needed in the case of `putContractCode` for checking if the account has a non-null value. */ - put(address: Address, code: Uint8Array | undefined, codeExists?: boolean | undefined): void { + put(address: Address, code: Uint8Array | undefined): void { const addressHex = bytesToUnprefixedHex(address.bytes) - this._saveCachePreState(addressHex, code, codeExists) + this._saveCachePreState(addressHex) const elem = { code, } diff --git a/packages/statemanager/src/stateManager.ts b/packages/statemanager/src/stateManager.ts index c2d09467a6..383c55acf7 100644 --- a/packages/statemanager/src/stateManager.ts +++ b/packages/statemanager/src/stateManager.ts @@ -354,18 +354,11 @@ export class DefaultStateManager implements EVMStateManagerInterface { * @param value - The value of the `code` */ async putContractCode(address: Address, value: Uint8Array): Promise { + this._codeCache?.put(address, value) const codeHash = keccak256(value) - if (!this._codeCacheSettings.deactivate) { - const codeExists = !equalsBytes(await this.getContractCode(address), new Uint8Array()) - this._codeCache?.put(address, value, codeExists) - } if (equalsBytes(codeHash, KECCAK256_NULL)) { return } - if (this._codeCacheSettings.deactivate) { - const key = this._prefixCodeHashes ? concatBytes(CODEHASH_PREFIX, codeHash) : codeHash - await this._getCodeDB().put(key, value) - } if (this.DEBUG) { this._debug(`Update codeHash (-> ${short(codeHash)}) for account ${address}`) diff --git a/packages/statemanager/test/checkpointing.account.spec.ts b/packages/statemanager/test/checkpointing.account.spec.ts index b08df8f8dc..29e3a48204 100644 --- a/packages/statemanager/test/checkpointing.account.spec.ts +++ b/packages/statemanager/test/checkpointing.account.spec.ts @@ -118,10 +118,10 @@ describe('StateManager -> Account Checkpointing', () => { await sm.putAccount(address, as.a1[0]) await sm.flush() - assert.ok(accountEval(sm, address, as.a1[1])) + assert.ok(await accountEval(sm, address, as.a1[1])) sm.clearCaches() - assert.ok(accountEval(sm, address, as.a1[1])) + assert.ok(await accountEval(sm, address, as.a1[1])) }) it(`CP -> A1.1 -> Commit -> Flush() (-> A1.1)`, async () => { @@ -131,10 +131,10 @@ describe('StateManager -> Account Checkpointing', () => { await sm.putAccount(address, as.a1[0]) await sm.commit() await sm.flush() - assert.ok(accountEval(sm, address, as.a1[1])) + assert.ok(await accountEval(sm, address, as.a1[1])) sm.clearCaches() - assert.ok(accountEval(sm, address, as.a1[1])) + assert.ok(await accountEval(sm, address, as.a1[1])) }) it(`CP -> A1.1 -> Revert -> Flush() (-> Undefined)`, async () => { @@ -144,10 +144,10 @@ describe('StateManager -> Account Checkpointing', () => { await sm.putAccount(address, as.a1[0]) await sm.revert() await sm.flush() - assert.ok(accountEval(sm, address, undefined)) + assert.ok(await accountEval(sm, address, undefined)) sm.clearCaches() - assert.ok(accountEval(sm, address, undefined)) + assert.ok(await accountEval(sm, address, undefined)) }) it(`A1.1 -> CP -> Commit -> Flush() (-> A1.1)`, async () => { @@ -157,10 +157,10 @@ describe('StateManager -> Account Checkpointing', () => { await sm.checkpoint() await sm.commit() await sm.flush() - assert.ok(accountEval(sm, address, as.a1[1])) + assert.ok(await accountEval(sm, address, as.a1[1])) sm.clearCaches() - assert.ok(accountEval(sm, address, as.a1[1])) + assert.ok(await accountEval(sm, address, as.a1[1])) }) it(`A1.1 -> CP -> Revert -> Flush() (-> A1.1)`, async () => { @@ -170,10 +170,10 @@ describe('StateManager -> Account Checkpointing', () => { await sm.checkpoint() await sm.revert() await sm.flush() - assert.ok(accountEval(sm, address, as.a1[1])) + assert.ok(await accountEval(sm, address, as.a1[1])) sm.clearCaches() - assert.ok(accountEval(sm, address, as.a1[1])) + assert.ok(await accountEval(sm, address, as.a1[1])) }) it(`A1.1 -> CP -> A1.2 -> Commit -> Flush() (-> A1.2)`, async () => { @@ -184,10 +184,10 @@ describe('StateManager -> Account Checkpointing', () => { await sm.putAccount(address, as.a2[0]) await sm.commit() await sm.flush() - assert.ok(accountEval(sm, address, as.a2[1])) + assert.ok(await accountEval(sm, address, as.a2[1])) sm.clearCaches() - assert.ok(accountEval(sm, address, as.a2[1])) + assert.ok(await accountEval(sm, address, as.a2[1])) }) it(`A1.1 -> CP -> A1.2 -> Commit -> A1.3 -> Flush() (-> A1.3)`, async () => { @@ -199,10 +199,10 @@ describe('StateManager -> Account Checkpointing', () => { await sm.commit() await sm.putAccount(address, as.a3[0]) await sm.flush() - assert.ok(accountEval(sm, address, as.a3[1])) + assert.ok(await accountEval(sm, address, as.a3[1])) sm.clearCaches() - assert.ok(accountEval(sm, address, as.a3[1])) + assert.ok(await accountEval(sm, address, as.a3[1])) }) it(`A1.1 -> CP -> A1.2 -> A1.3 -> Commit -> Flush() (-> A1.3)`, async () => { @@ -214,10 +214,10 @@ describe('StateManager -> Account Checkpointing', () => { await sm.putAccount(address, as.a3[0]) await sm.commit() await sm.flush() - assert.ok(accountEval(sm, address, as.a3[1])) + assert.ok(await accountEval(sm, address, as.a3[1])) sm.clearCaches() - assert.ok(accountEval(sm, address, as.a3[1])) + assert.ok(await accountEval(sm, address, as.a3[1])) }) it(`CP -> A1.1 -> A1.2 -> Commit -> Flush() (-> A1.2)`, async () => { @@ -228,10 +228,10 @@ describe('StateManager -> Account Checkpointing', () => { await sm.putAccount(address, as.a2[0]) await sm.commit() await sm.flush() - assert.ok(accountEval(sm, address, as.a2[1])) + assert.ok(await accountEval(sm, address, as.a2[1])) sm.clearCaches() - assert.ok(accountEval(sm, address, as.a2[1])) + assert.ok(await accountEval(sm, address, as.a2[1])) }) it(`CP -> A1.1 -> A1.2 -> Revert -> Flush() (-> Undefined)`, async () => { @@ -243,10 +243,10 @@ describe('StateManager -> Account Checkpointing', () => { await sm.putAccount(address, as.a2[0]) await sm.revert() await sm.flush() - assert.ok(accountEval(sm, address, undefined)) + assert.ok(await accountEval(sm, address, undefined)) sm.clearCaches() - assert.ok(accountEval(sm, address, undefined)) + assert.ok(await accountEval(sm, address, undefined)) }) it(`A1.1 -> CP -> A1.2 -> Revert -> Flush() (-> A1.1)`, async () => { @@ -257,10 +257,10 @@ describe('StateManager -> Account Checkpointing', () => { await sm.putAccount(address, as.a2[0]) await sm.revert() await sm.flush() - assert.ok(accountEval(sm, address, as.a1[1])) + assert.ok(await accountEval(sm, address, as.a1[1])) sm.clearCaches() - assert.ok(accountEval(sm, address, as.a1[1])) + assert.ok(await accountEval(sm, address, as.a1[1])) }) it('A1.1 -> CP -> A1.2 -> CP -> A1.3 -> Commit -> Commit -> Flush() (-> A1.3)', async () => { @@ -274,10 +274,10 @@ describe('StateManager -> Account Checkpointing', () => { await sm.commit() await sm.commit() await sm.flush() - assert.ok(accountEval(sm, address, as.a3[1])) + assert.ok(await accountEval(sm, address, as.a3[1])) sm.clearCaches() - assert.ok(accountEval(sm, address, as.a3[1])) + assert.ok(await accountEval(sm, address, as.a3[1])) }) it('A1.1 -> CP -> A1.2 -> CP -> A1.3 -> Commit -> Revert -> Flush() (-> A1.1)', async () => { @@ -291,10 +291,10 @@ describe('StateManager -> Account Checkpointing', () => { await sm.commit() await sm.revert() await sm.flush() - assert.ok(accountEval(sm, address, as.a1[1])) + assert.ok(await accountEval(sm, address, as.a1[1])) sm.clearCaches() - assert.ok(accountEval(sm, address, as.a1[1])) + assert.ok(await accountEval(sm, address, as.a1[1])) }) it('A1.1 -> CP -> A1.2 -> CP -> A1.3 -> Revert -> Commit -> Flush() (-> A1.2)', async () => { @@ -308,10 +308,10 @@ describe('StateManager -> Account Checkpointing', () => { await sm.revert() await sm.commit() await sm.flush() - assert.ok(accountEval(sm, address, as.a2[1])) + assert.ok(await accountEval(sm, address, as.a2[1])) sm.clearCaches() - assert.ok(accountEval(sm, address, as.a2[1])) + assert.ok(await accountEval(sm, address, as.a2[1])) }) it('A1.1 -> CP -> A1.2 -> CP -> A1.3 -> Revert -> A1.4 -> Commit -> Flush() (-> A1.4)', async () => { @@ -326,10 +326,10 @@ describe('StateManager -> Account Checkpointing', () => { await sm.putAccount(address, as.a4[0]) await sm.commit() await sm.flush() - assert.ok(accountEval(sm, address, as.a4[1])) + assert.ok(await accountEval(sm, address, as.a4[1])) sm.clearCaches() - assert.ok(accountEval(sm, address, as.a4[1])) + assert.ok(await accountEval(sm, address, as.a4[1])) }) it('A1.1 -> CP -> A1.2 -> CP -> A1.3 -> Revert -> A1.4 -> CP -> A1.5 -> Commit -> Commit -> Flush() (-> A1.5)', async () => { @@ -347,10 +347,10 @@ describe('StateManager -> Account Checkpointing', () => { await sm.commit() await sm.commit() await sm.flush() - assert.ok(accountEval(sm, address, as.a5[1])) + assert.ok(await accountEval(sm, address, as.a5[1])) sm.clearCaches() - assert.ok(accountEval(sm, address, as.a5[1])) + assert.ok(await accountEval(sm, address, as.a5[1])) }) } })