Skip to content

Commit

Permalink
Code cache should be able to save prestate without reading from state…
Browse files Browse the repository at this point in the history
…manager 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 <[email protected]>
Co-authored-by: Holger Drewes <[email protected]>
  • Loading branch information
3 people authored Oct 12, 2023
1 parent 02a7abb commit 2405da8
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 65 deletions.
6 changes: 3 additions & 3 deletions packages/statemanager/src/cache/account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
28 changes: 6 additions & 22 deletions packages/statemanager/src/cache/code.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand All @@ -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,
}
Expand Down
9 changes: 1 addition & 8 deletions packages/statemanager/src/stateManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -354,18 +354,11 @@ export class DefaultStateManager implements EVMStateManagerInterface {
* @param value - The value of the `code`
*/
async putContractCode(address: Address, value: Uint8Array): Promise<void> {
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}`)
Expand Down
64 changes: 32 additions & 32 deletions packages/statemanager/test/checkpointing.account.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -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]))
})
}
})

0 comments on commit 2405da8

Please sign in to comment.