Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

VM/tests: ensure verifyPostConditions works #1900

Merged
merged 3 commits into from May 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 19 additions & 15 deletions packages/vm/tests/tester/runners/BlockchainTestsRunner.ts
Expand Up @@ -154,23 +154,27 @@ export default async function runBlockchainTest(options: any, testData: any, t:
// blockchain tests come with their own `pre` world state.
// TODO: Add option to `runBlockchain` not to generate genesis state.
vm._common.genesis().stateRoot = vm.stateManager._trie.root
await vm.runBlockchain()
const headBlock = await vm.blockchain.getHead()

// if the test fails, then block.header is the prev because
// vm.runBlock has a check that prevents the actual postState from being
// imported if it is not equal to the expected postState. it is useful
// for debugging to skip this, so that verifyPostConditions will compare
// testData.postState to the actual postState, rather than to the preState.
if (!options.debug) {
// make sure the state is set before checking post conditions
vm.stateManager._trie.root = headBlock.header.stateRoot
}
try {
await vm.runBlockchain()
} catch (e: any) {
const headBlock = await vm.blockchain.getHead()

// if the test fails, then block.header is the prev because
// vm.runBlock has a check that prevents the actual postState from being
// imported if it is not equal to the expected postState. it is useful
// for debugging to skip this, so that verifyPostConditions will compare
// testData.postState to the actual postState, rather than to the preState.
if (!options.debug) {
// make sure the state is set before checking post conditions
vm.stateManager._trie.root = headBlock.header.stateRoot
}

if (options.debug) {
await verifyPostConditions(state, testData.postState, t)
}
if (options.debug) {
await verifyPostConditions(state, testData.postState, t)
}

throw e
}
await cacheDB.close()

if (expectException) {
Expand Down
75 changes: 42 additions & 33 deletions packages/vm/tests/util.ts
Expand Up @@ -70,7 +70,7 @@ export function dumpState(state: any, cb: Function) {
})
}

export function format(a: any, toZero: boolean = false, isHex: boolean = false) {
export function format(a: any, toZero: boolean = false, isHex: boolean = false): Buffer {
if (a === '') {
return Buffer.alloc(0)
}
Expand Down Expand Up @@ -144,14 +144,14 @@ export async function verifyPostConditions(state: any, testData: any, t: tape.Te
const promise = verifyAccountPostConditions(state, address, account, testData, t)
queue.push(promise)
} else {
t.fail('invalid account in the trie: ' + <string>key)
t.comment('invalid account in the trie: ' + <string>key)
Copy link
Member Author

Choose a reason for hiding this comment

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

Converted all tape commands to comment. The reason is that if you have failing tests, then this adds to the number of tests counter. Let's say we accidentally do not run some tests, then --verify-amount-alltests flag could still pass while this is actually not the case.

Also note that if any of these comments get triggered, then the state root of the block is wrong, so the test fails anyways.

}
})

stream.on('end', async function () {
await Promise.all(queue)
for (const [_key, address] of Object.entries(keyMap)) {
t.fail(`Missing account!: ${address}`)
t.comment(`Missing account!: ${address}`)
}
resolve()
})
Expand All @@ -174,12 +174,19 @@ export function verifyAccountPostConditions(
) {
return new Promise<void>((resolve) => {
t.comment('Account: ' + address)
t.ok(format(account.balance, true).equals(format(acctData.balance, true)), 'correct balance')
t.ok(format(account.nonce, true).equals(format(acctData.nonce, true)), 'correct nonce')
if (!format(account.balance, true).equals(format(acctData.balance, true))) {
t.comment(
`Expected balance of ${new BN(format(acctData.balance, true))}, but got ${account.balance}`
)
}
if (!format(account.nonce, true).equals(format(acctData.nonce, true))) {
t.comment(
`Expected nonce of ${new BN(format(acctData.nonce, true))}, but got ${account.nonce}`
)
}

// validate storage
const origRoot = state.root
const storageKeys = Object.keys(acctData.storage)

const hashedStorage: any = {}
for (const key in acctData.storage) {
Expand All @@ -188,38 +195,40 @@ export function verifyAccountPostConditions(
] = acctData.storage[key]
}

if (storageKeys.length > 0) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed this if statement: if there is no storage in the postState but we somehow still store something, then we should detect that anyways.

state.root = account.stateRoot
const rs = state.createReadStream()
rs.on('data', function (data: any) {
let key = data.key.toString('hex')
const val = '0x' + rlp.decode(data.value).toString('hex')

if (key === '0x') {
key = '0x00'
acctData.storage['0x00'] = acctData.storage['0x00']
? acctData.storage['0x00']
: acctData.storage['0x']
delete acctData.storage['0x']
}
state.root = account.stateRoot
const rs = state.createReadStream()
rs.on('data', function (data: any) {
let key = data.key.toString('hex')
const val = '0x' + rlp.decode(data.value).toString('hex')

if (key === '0x') {
key = '0x00'
acctData.storage['0x00'] = acctData.storage['0x00']
? acctData.storage['0x00']
: acctData.storage['0x']
delete acctData.storage['0x']
}

t.equal(val, hashedStorage[key], 'correct storage value')
delete hashedStorage[key]
})
if (val !== hashedStorage[key]) {
t.comment(
`Expected storage key 0x${data.key.toString('hex')} at address ${address} to have value ${
Copy link
Member Author

Choose a reason for hiding this comment

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

Have to add address here since it is not clear when these promises are fulfilled.

hashedStorage[key] ?? '0x'
}, but got ${val}}`
)
}
delete hashedStorage[key]
})

rs.on('end', function () {
for (const key in hashedStorage) {
if (hashedStorage[key] !== '0x00') {
t.fail('key: ' + key + ' not found in storage')
}
rs.on('end', function () {
for (const key in hashedStorage) {
if (hashedStorage[key] !== '0x00') {
t.comment(`key: ${key} not found in storage at address ${address}`)
}
}

state.root = origRoot
resolve()
})
} else {
state.root = origRoot
resolve()
}
})
})
}

Expand Down