-
Notifications
You must be signed in to change notification settings - Fork 785
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
VM: Diff-based Touched Accounts Checkpointing #2581
Merged
jochem-brouwer
merged 10 commits into
master
from
vm-diff-based-touched-account-checkpointing
Mar 17, 2023
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
499eaef
VM: Switched to a more efficient diff-based way of touched account ch…
holgerd77 097ba6a
VM: move accessed storage inefficient checkpointing problem to berlin…
holgerd77 3dc802e
EVM: avoid memory copy in MLOAD opcode function
holgerd77 1a234ed
Remove console.log() in EVM
holgerd77 7b422ad
vmState: ensure touched accounts delete stack gets properly updated o…
jochem-brouwer 464749b
vm/eei: save touched height
jochem-brouwer a0bfbc0
vm/vmState: new possible fix for touched accounts
jochem-brouwer a7f3df9
vm/vmState: another attempt to fix touched accounts journaling
jochem-brouwer a5fb9be
vm: add journaling
jochem-brouwer a9bab81
Check correct journal height on revert
acolytec3 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
export class Journaling<T> { | ||
public journal: Set<T> | ||
protected journalStack: { [key: number]: Set<T> } | ||
protected journalHeight: Map<T, number> | ||
protected height: number | ||
|
||
constructor() { | ||
this.journal = new Set() | ||
this.journalStack = {} | ||
this.journalHeight = new Map() | ||
this.height = 0 | ||
} | ||
|
||
clear() { | ||
this.journal.clear() | ||
this.journalStack = {} | ||
this.journalHeight = new Map() | ||
this.height = 0 | ||
} | ||
|
||
checkpoint() { | ||
this.height++ | ||
} | ||
|
||
revert(ignoreItem?: T) { | ||
const height = this.height | ||
if (height in this.journalStack) { | ||
for (const key of this.journalStack[height]) { | ||
// Exceptional case due to consensus issue in Geth and Parity. | ||
// See [EIP issue #716](https://github.com/ethereum/EIPs/issues/716) for context. | ||
// The RIPEMD precompile has to remain *touched* even when the call reverts, | ||
// and be considered for deletion. | ||
if (key === ignoreItem) { | ||
continue | ||
} | ||
|
||
if (this.journal.has(key) && this.journalHeight.get(key)! >= height) { | ||
this.journal.delete(key) | ||
this.journalHeight.delete(key) | ||
} | ||
} | ||
delete this.journalStack[height] | ||
} | ||
this.height-- | ||
} | ||
|
||
commit() { | ||
const height = this.height | ||
if (height in this.journalStack) { | ||
// Copy the items-to-delete in case of a revert into one level higher | ||
if (height > 0) { | ||
if (this.journalStack[height - 1] === undefined) { | ||
this.journalStack[height - 1] = new Set() | ||
} | ||
for (const address of this.journalStack[height]) { | ||
this.journalStack[height - 1].add(address) | ||
if (this.journalHeight.get(address) === height) { | ||
this.journalHeight.set(address, height - 1) | ||
} | ||
} | ||
} else { | ||
this.journal = new Set() | ||
this.journalHeight = new Map() | ||
} | ||
delete this.journalStack[height] | ||
} | ||
this.height-- | ||
} | ||
|
||
addJournalItem(input: T) { | ||
const height = this.height | ||
if (!(height in this.journalStack)) { | ||
this.journalStack[height] = new Set() | ||
} | ||
this.journalStack[height].add(input) | ||
|
||
this.journal.add(input) | ||
if (this.journalHeight.get(input) === undefined) { | ||
this.journalHeight.set(input, height) | ||
} | ||
} | ||
} | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,8 @@ import { ripemdPrecompileAddress } from '@ethereumjs/evm/dist/precompiles' | |
import { Account, Address, toBuffer } from '@ethereumjs/util' | ||
import { debug as createDebugLogger } from 'debug' | ||
|
||
import { Journaling } from './journaling' | ||
|
||
import type { EVMStateAccess } from '@ethereumjs/evm/dist/types' | ||
import type { AccountFields, StateManager } from '@ethereumjs/statemanager' | ||
import type { AccessList, AccessListItem } from '@ethereumjs/tx' | ||
|
@@ -16,8 +18,6 @@ export class VmState implements EVMStateAccess { | |
|
||
protected _checkpointCount: number | ||
protected _stateManager: StateManager | ||
protected _touched: Set<AddressHex> | ||
protected _touchedStack: Set<AddressHex>[] | ||
|
||
// EIP-2929 address/storage trackers. | ||
// This maps both the accessed accounts and the accessed storage slots. | ||
|
@@ -27,6 +27,10 @@ export class VmState implements EVMStateAccess { | |
// Each call level tracks their access themselves. | ||
// In case of a commit, copy everything if the value does not exist, to the level above | ||
// In case of a revert, discard any warm slots. | ||
// | ||
// TODO: Switch to diff based version similar to _touchedStack | ||
// (_accessStorage representing the actual state, separate _accessedStorageStack dictionary | ||
// tracking the access diffs per commit) | ||
protected _accessedStorage: Map<string, Set<string>>[] | ||
|
||
// Backup structure for address/storage tracker frames on reverts | ||
|
@@ -35,18 +39,20 @@ export class VmState implements EVMStateAccess { | |
|
||
protected _originalStorageCache: Map<AddressHex, Map<AddressHex, Buffer>> | ||
|
||
protected readonly touchedJournal: Journaling<AddressHex> | ||
|
||
protected readonly DEBUG: boolean = false | ||
|
||
constructor({ common, stateManager }: { common?: Common; stateManager: StateManager }) { | ||
this._checkpointCount = 0 | ||
this._stateManager = stateManager | ||
this._common = common ?? new Common({ chain: Chain.Mainnet, hardfork: Hardfork.Petersburg }) | ||
this._touched = new Set() | ||
this._touchedStack = [] | ||
this._originalStorageCache = new Map() | ||
this._accessedStorage = [new Map()] | ||
this._accessedStorageReverted = [new Map()] | ||
|
||
this.touchedJournal = new Journaling<AddressHex>() | ||
|
||
// Skip DEBUG calls unless 'ethjs' included in environmental DEBUG variables | ||
this.DEBUG = process?.env?.DEBUG?.includes('ethjs') ?? false | ||
|
||
|
@@ -61,10 +67,12 @@ export class VmState implements EVMStateAccess { | |
* Partial implementation, called from the subclass. | ||
*/ | ||
async checkpoint(): Promise<void> { | ||
this._touchedStack.push(new Set(Array.from(this._touched))) | ||
this._accessedStorage.push(new Map()) | ||
if (this._common.gteHardfork(Hardfork.Berlin)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it is cleaner if we check for EIP 2929 active here |
||
this._accessedStorage.push(new Map()) | ||
} | ||
await this._stateManager.checkpoint() | ||
this._checkpointCount++ | ||
this.touchedJournal.checkpoint() | ||
|
||
if (this.DEBUG) { | ||
this._debug('-'.repeat(100)) | ||
|
@@ -73,14 +81,15 @@ export class VmState implements EVMStateAccess { | |
} | ||
|
||
async commit(): Promise<void> { | ||
// setup cache checkpointing | ||
this._touchedStack.pop() | ||
// Copy the contents of the map of the current level to a map higher. | ||
const storageMap = this._accessedStorage.pop() | ||
if (storageMap) { | ||
this._accessedStorageMerge(this._accessedStorage, storageMap) | ||
if (this._common.gteHardfork(Hardfork.Berlin)) { | ||
// Copy the contents of the map of the current level to a map higher. | ||
const storageMap = this._accessedStorage.pop() | ||
if (storageMap) { | ||
this._accessedStorageMerge(this._accessedStorage, storageMap) | ||
} | ||
} | ||
await this._stateManager.commit() | ||
this.touchedJournal.commit() | ||
this._checkpointCount-- | ||
|
||
if (this._checkpointCount === 0) { | ||
|
@@ -100,24 +109,16 @@ export class VmState implements EVMStateAccess { | |
* Partial implementation , called from the subclass. | ||
*/ | ||
async revert(): Promise<void> { | ||
// setup cache checkpointing | ||
const lastItem = this._accessedStorage.pop() | ||
if (lastItem) { | ||
this._accessedStorageReverted.push(lastItem) | ||
} | ||
const touched = this._touchedStack.pop() | ||
if (!touched) { | ||
throw new Error('Reverting to invalid state checkpoint failed') | ||
} | ||
// Exceptional case due to consensus issue in Geth and Parity. | ||
// See [EIP issue #716](https://github.com/ethereum/EIPs/issues/716) for context. | ||
// The RIPEMD precompile has to remain *touched* even when the call reverts, | ||
// and be considered for deletion. | ||
if (this._touched.has(ripemdPrecompileAddress)) { | ||
touched.add(ripemdPrecompileAddress) | ||
if (this._common.gteHardfork(Hardfork.Berlin)) { | ||
// setup cache checkpointing | ||
const lastItem = this._accessedStorage.pop() | ||
if (lastItem) { | ||
this._accessedStorageReverted.push(lastItem) | ||
} | ||
} | ||
this._touched = touched | ||
|
||
await this._stateManager.revert() | ||
this.touchedJournal.revert(ripemdPrecompileAddress) | ||
|
||
this._checkpointCount-- | ||
|
||
|
@@ -202,7 +203,7 @@ export class VmState implements EVMStateAccess { | |
* at the end of the tx. | ||
*/ | ||
touchAccount(address: Address): void { | ||
this._touched.add(address.buf.toString('hex')) | ||
this.touchedJournal.addJournalItem(address.buf.toString('hex')) | ||
} | ||
|
||
/** | ||
|
@@ -273,7 +274,7 @@ export class VmState implements EVMStateAccess { | |
*/ | ||
async cleanupTouchedAccounts(): Promise<void> { | ||
if (this._common.gteHardfork(Hardfork.SpuriousDragon) === true) { | ||
const touchedArray = Array.from(this._touched) | ||
const touchedArray = Array.from(this.touchedJournal.journal) | ||
for (const addressHex of touchedArray) { | ||
const address = new Address(Buffer.from(addressHex, 'hex')) | ||
const empty = await this.accountIsEmpty(address) | ||
|
@@ -285,7 +286,7 @@ export class VmState implements EVMStateAccess { | |
} | ||
} | ||
} | ||
this._touched.clear() | ||
this.touchedJournal.clear() | ||
} | ||
|
||
/** | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, this looks super-clean. Great stuff! 🙂 👍