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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✅ Test: Add 100% test coverage to tevm/blockchain package #1319

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// Bun Snapshot v1, https://goo.gl/fbAQLP

exports[`createChain should throw an error for invalid block header validation 1`] = `"header.isGenesis is not a function. (In 'header.isGenesis()', 'header.isGenesis' is undefined)"`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Bun Snapshot v1, https://goo.gl/fbAQLP

exports[`validateHeader should throw an error for invalid block number 1`] = `[TypeError: header.isGenesis is not a function. (In 'header.isGenesis()', 'header.isGenesis' is undefined)]`;

exports[`validateHeader should throw an error for invalid timestamp 1`] = `[TypeError: header.isGenesis is not a function. (In 'header.isGenesis()', 'header.isGenesis' is undefined)]`;

exports[`validateHeader should throw an error for unsupported consensus type 1`] = `"header.isGenesis is not a function. (In 'header.isGenesis()', 'header.isGenesis' is undefined)"`;

exports[`validateHeader should throw an error for invalid timestamp diff (clique) 1`] = `"header.isGenesis is not a function. (In 'header.isGenesis()', 'header.isGenesis' is undefined)"`;
34 changes: 34 additions & 0 deletions packages/blockchain/src/actions/deepCopy.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { describe, expect, it } from 'bun:test'
import { deepCopy } from './deepCopy.js'
import { createBaseChain } from '../createBaseChain.js'
import { optimism } from '@tevm/common'
import { putBlock } from './putBlock.js'
import { bytesToHex } from '@tevm/utils'
import { getBlock } from './getBlock.js'
import { getMockBlocks } from '../test/getBlocks.js'

describe(deepCopy.name, async () => {
it('should deepCopy the chain', async () => {
const blocks = await getMockBlocks()
const chain = createBaseChain({
common: optimism.copy(),
})
await putBlock(chain)(blocks[0])
await putBlock(chain)(blocks[1])
await putBlock(chain)(blocks[2])
await putBlock(chain)(blocks[3])
const copy = await deepCopy(chain)()
expect(await getBlock(copy)(blocks[0].header.number)).toEqual(blocks[0])
expect(await getBlock(copy)(blocks[1].header.number)).toEqual(blocks[1])
expect(await getBlock(copy)(blocks[3].header.number)).toEqual(blocks[3])
expect(chain.blocksByTag.get('latest')).toEqual(blocks[3])
expect(chain.blocks.get(bytesToHex(blocks[0].hash()))).toEqual(blocks[0])
expect(chain.blocks.get(bytesToHex(blocks[1].hash()))).toEqual(blocks[1])
expect(chain.blocks.get(bytesToHex(blocks[2].hash()))).toEqual(blocks[2])
expect(chain.blocks.get(bytesToHex(blocks[3].hash()))).toEqual(blocks[3])
expect(chain.blocksByNumber.get(blocks[0].header.number)).toEqual(blocks[0])
expect(chain.blocksByNumber.get(blocks[1].header.number)).toEqual(blocks[1])
expect(chain.blocksByNumber.get(blocks[2].header.number)).toEqual(blocks[2])
expect(chain.blocksByNumber.get(blocks[3].header.number)).toEqual(blocks[3])
})
})
23 changes: 20 additions & 3 deletions packages/blockchain/src/actions/delBlock.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,28 @@
import { bytesToHex } from 'viem'
import { getCanonicalHeadBlock } from './getCanonicalHeadBlock.js'
import { getBlock } from './getBlock.js'
import { InvalidBlockError } from '@tevm/errors'

/**
* Deletes a block from the blockchain
* @param {import('../BaseChain.js').BaseChain} baseChain
* @returns {import('../Chain.js').Chain['delBlock']}
* @throws {InvalidBlockError} If the block is the `forked` block
*/
export const delBlock = (baseChain) => (blockHash) => {
baseChain.blocks.delete(bytesToHex(blockHash))
export const delBlock = (baseChain) => async (blockHash) => {
const block = await getBlock(baseChain)(blockHash)
const hexHash = bytesToHex(blockHash)

const latest = await getCanonicalHeadBlock(baseChain)()
const forkedBlock = baseChain.blocksByTag.get('forked')

if (forkedBlock && hexHash === bytesToHex(forkedBlock.hash())) {
throw new InvalidBlockError('Cannot delete the forked block!')
}
if (hexHash === bytesToHex(latest.hash())) {
baseChain.blocksByTag.set('latest', await getBlock(baseChain)(latest.header.parentHash))
}
baseChain.blocksByNumber.delete(block.header.number)
baseChain.blocks.delete(hexHash)
baseChain.logger.debug({ blockHash }, 'deleted block')
return Promise.resolve()
}
56 changes: 56 additions & 0 deletions packages/blockchain/src/actions/delBlock.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { describe, expect, it } from 'bun:test'
import { createBaseChain } from '../createBaseChain.js'
import { optimism } from '@tevm/common'
import { putBlock } from './putBlock.js'
import { delBlock } from './delBlock.js'
import { getMockBlocks } from '../test/getBlocks.js'
import { bytesToHex } from 'viem'
import { getCanonicalHeadBlock } from './getCanonicalHeadBlock.js'
import { InvalidBlockError } from '@tevm/errors'

describe(delBlock.name, async () => {
it('should delete block', async () => {
const chain = createBaseChain({
common: optimism.copy(),
})
const blocks = await getMockBlocks()
await putBlock(chain)(blocks[0])
await putBlock(chain)(blocks[1])
await putBlock(chain)(blocks[2])
await putBlock(chain)(blocks[3])

await delBlock(chain)(blocks[2].hash())

expect(chain.blocks.get(bytesToHex(blocks[2].hash()))).toBeUndefined()
expect(chain.blocksByNumber.get(blocks[2].header.number)).toBeUndefined()
})

it('should set latest to parent if we delete latest', async () => {
const chain = createBaseChain({
common: optimism.copy(),
})
const blocks = await getMockBlocks()
await putBlock(chain)(blocks[0])
await putBlock(chain)(blocks[1])
await putBlock(chain)(blocks[2])
await putBlock(chain)(blocks[3])

await delBlock(chain)(blocks[3].hash())

expect(await getCanonicalHeadBlock(chain)()).toBe(blocks[2])
})

it('should throw an InvalidBlockError if we attempt to delete the fork block', async () => {
const chain = createBaseChain({
common: optimism.copy(),
})
const blocks = await getMockBlocks()
await putBlock(chain)(blocks[0])
chain.blocksByTag.set('forked', blocks[0])

const error = await delBlock(chain)(blocks[0].hash()).then((e) => e)

expect(error).toBeInstanceOf(InvalidBlockError)
expect(error).toMatchSnapshot()
})
})
15 changes: 5 additions & 10 deletions packages/blockchain/src/actions/getBlock.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { bytesToHex } from '@tevm/utils'
import { getBlockFromRpc } from '../utils/getBlockFromRpc.js'
import { putBlock } from './putBlock.js'
import { UnknownBlockError } from '@tevm/errors'
import { getCanonicalHeadBlock } from './getCanonicalHeadBlock.js'

/**
* @param {import('../BaseChain.js').BaseChain} baseChain
Expand Down Expand Up @@ -29,7 +31,7 @@ export const getBlock = (baseChain) => async (blockId) => {
}

if (!baseChain.options.fork?.transport) {
throw new Error(
throw new UnknownBlockError(
blockId instanceof Uint8Array
? `Block with hash ${bytesToHex(blockId)} does not exist`
: `Block number ${blockId} does not exist`,
Expand All @@ -48,15 +50,8 @@ export const getBlock = (baseChain) => async (blockId) => {

baseChain.logger.debug(fetchedBlock.header.toJSON(), 'Saving forked block to blockchain')

const forkedBlock = baseChain.blocksByTag.get('forked')
const latestBlock = baseChain.blocksByTag.get('latest')
if (!forkedBlock || !latestBlock) {
throw new Error('TevmInternalError: Expected forked and latest blocktags to exist in tevm blockchain')
}
if (fetchedBlock.header.number > latestBlock.header.number) {
throw new Error(`Current blockheight is ${latestBlock.header.number} and the fork block is ${forkedBlock.header.number}. The block requested has height of ${fetchedBlock.header.number}.
Fetching blocks from future of the current block is not allowed`)
}
const forkedBlock = /** @type {import('@tevm/block').Block}*/ (baseChain.blocksByTag.get('forked'))
const latestBlock = await getCanonicalHeadBlock(baseChain)()
if (fetchedBlock.header.number > forkedBlock.header.number) {
throw new Error(`The fetched block ${fetchedBlock.header.number} has a higher block height than the forked block ${forkedBlock.header.number} but less than the latest block ${latestBlock.header.number}
This could indicate a bug in tevm as it implies a block is missing if the internal chain tried fetching it from rpc
Expand Down
81 changes: 81 additions & 0 deletions packages/blockchain/src/actions/getBlock.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import { describe, expect, it } from 'bun:test'
import { getBlock } from './getBlock.js'
import { createBaseChain } from '../createBaseChain.js'
import { mainnet } from '@tevm/common'
import { putBlock } from './putBlock.js'
import { getMockBlocks } from '../test/getBlocks.js'
import { UnknownBlockError } from '@tevm/errors'
import { transports } from '@tevm/test-utils'

describe(getBlock.name, async () => {
const chain = createBaseChain({
common: mainnet.copy(),
})

const blocks = await getMockBlocks()

await putBlock(chain)(blocks[0])
await putBlock(chain)(blocks[1])
await putBlock(chain)(blocks[2])

it('can get a block by hash', async () => {
expect(await getBlock(chain)(blocks[0].hash())).toBe(blocks[0])
})

it('can get a block by number', async () => {
expect(await getBlock(chain)(blocks[0].header.number)).toBe(blocks[0])
})

it('can get a block by number that is not a big int', async () => {
expect(await getBlock(chain)(Number(blocks[0].header.number))).toBe(blocks[0])
})

it('should throw an error if the block does not exist', async () => {
let error = await getBlock(chain)(69).then((e) => e)
expect(error).toBeInstanceOf(UnknownBlockError)
expect(error).toMatchSnapshot()
error = await getBlock(chain)(blocks[3].hash()).catch((e) => e)
expect(error).toBeInstanceOf(UnknownBlockError)
expect(error).toMatchSnapshot()
})

it('should fetch and cache the block from rpc if it does not exist', async () => {
const chain = createBaseChain({
common: mainnet.copy(),
fork: {
transport: transports.optimism,
blockTag: blocks[0].header.number,
},
})
await chain.ready()
expect(await getBlock(chain)(blocks[1].hash())).toEqual(blocks[1])
expect(chain.blocksByNumber.get(blocks[1].header.number)).toEqual(blocks[1])
})

it('should fetch and cache the block by number from rpc if it does not exist', async () => {
const chain = createBaseChain({
common: mainnet.copy(),
fork: {
transport: transports.optimism,
blockTag: blocks[0].header.number,
},
})
await chain.ready()
expect(await getBlock(chain)(blocks[1].header.number)).toEqual(blocks[1])
expect(chain.blocksByNumber.get(blocks[1].header.number)).toEqual(blocks[1])
})

it('should throw an error if attempting to fetch a block newer than the forked block', async () => {
const chain = createBaseChain({
common: mainnet.copy(),
fork: {
transport: transports.optimism,
blockTag: blocks[0].header.number,
},
})
await chain.ready()
const error = await getBlock(chain)(blocks[1].header.number).catch((e) => e)
expect(error).toBeInstanceOf(UnknownBlockError)
expect(error).toMatchSnapshot()
})
})
62 changes: 62 additions & 0 deletions packages/blockchain/src/actions/getBlockByTag.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { describe, expect, it } from 'bun:test'
import { getBlockByTag } from './getBlockByTag.js'
import { createBaseChain } from '../createBaseChain.js'
import { mainnet } from '@tevm/common'
import { getMockBlocks } from '../test/getBlocks.js'
import { putBlock } from './putBlock.js'
import { UnknownBlockError } from '@tevm/errors'
import { transports } from '@tevm/test-utils'

describe(getBlockByTag.name, async () => {
const chain = createBaseChain({
common: mainnet.copy(),
})

const blocks = await getMockBlocks()

await putBlock(chain)(blocks[0])
await putBlock(chain)(blocks[1])
await putBlock(chain)(blocks[2])

it('can get a block by hash', async () => {
expect(await getBlockByTag(chain)(blocks[0].hash())).toBe(blocks[0])
})

it('can get a block by number', async () => {
expect(await getBlockByTag(chain)(blocks[0].header.number)).toBe(blocks[0])
})

it('can get a block by number that is not a big int', async () => {
expect(await getBlockByTag(chain)(Number(blocks[0].header.number))).toBe(blocks[0])
})

it('should throw an error if the block does not exist', async () => {
let error = await getBlockByTag(chain)(69).then((e) => e)
expect(error).toBeInstanceOf(UnknownBlockError)
expect(error).toMatchSnapshot()
error = await getBlockByTag(chain)(blocks[3].hash()).catch((e) => e)
expect(error).toBeInstanceOf(UnknownBlockError)
expect(error).toMatchSnapshot()
})

it('should fetch and cache the block from rpc if it does not exist', async () => {
const chain = createBaseChain({
common: mainnet.copy(),
fork: {
transport: transports.optimism,
blockTag: blocks[0].header.number,
},
})
await chain.ready()
expect(await getBlockByTag(chain)('earliest')).toMatchSnapshot()
})

it('should throw UnknownBlockError if tag doesn not exist', async () => {
const chain = createBaseChain({
common: mainnet.copy(),
})
const error = await getBlockByTag(chain)('safe').catch((e) => e)
expect(error).toBeInstanceOf(UnknownBlockError)
expect(error).toMatchSnapshot()
})
})
4 changes: 3 additions & 1 deletion packages/blockchain/src/actions/getCanonicalHeadBlock.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { InternalError } from '@tevm/errors'
Copy link
Contributor

Choose a reason for hiding this comment

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

Use of InternalError enhances error specificity.

The introduction of InternalError for non-existent canonical head blocks is a good practice, enhancing the clarity and manageability of errors. However, there's a small typo in the error message.

- throw new InternalError('No cannonical head exists on blockchain')
+ throw new InternalError('No canonical head exists on blockchain')

Also applies to: 10-10


/**
* @param {import('../BaseChain.js').BaseChain} baseChain
* @returns {import('../Chain.js').Chain['getCanonicalHeadBlock']}
*/
export const getCanonicalHeadBlock = (baseChain) => () => {
const block = baseChain.blocksByTag.get('latest')
if (!block) {
throw new Error('No cannonical head exists on blockchain')
throw new InternalError('No cannonical head exists on blockchain')
}
return Promise.resolve(block)
}
23 changes: 23 additions & 0 deletions packages/blockchain/src/actions/getCanonicalHeadBlock.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { describe, expect, it } from 'bun:test'
import { getCanonicalHeadBlock } from './getCanonicalHeadBlock.js'
import { createBaseChain } from '../createBaseChain.js'
import { putBlock } from './putBlock.js'
import { getMockBlocks } from '../test/getBlocks.js'
import { optimism } from '@tevm/common'
import { InternalError } from '@tevm/errors'

describe(getCanonicalHeadBlock.name, async () => {
const blocks = await getMockBlocks()
it('should get the canonical head block', async () => {
const chain = createBaseChain({ common: optimism.copy() })
await putBlock(chain)(blocks[0])
expect(await getCanonicalHeadBlock(chain)()).toBe(blocks[0])
})
it('should throw an error if not cannonical block', async () => {
const chain = createBaseChain({ common: optimism.copy() })
chain.blocksByTag.set('latest', undefined)
let error = await getCanonicalHeadBlock(chain)().catch((e) => e)
expect(error).toBeInstanceOf(InternalError)
expect(error).toMatchSnapshot()
})
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Comprehensive tests for getCanonicalHeadBlock, minor improvement suggested.

The test suite comprehensively covers the functionality of getCanonicalHeadBlock, including both successful retrieval and error handling scenarios. The use of snapshot testing for error messages is a good practice to ensure consistency. However, there's a minor improvement suggested by static analysis tools that should be implemented.

- let error = await getCanonicalHeadBlock(chain)().catch((e) => e)
+ const error = await getCanonicalHeadBlock(chain)().catch((e) => e)

This change ensures adherence to best practices in variable declaration.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { describe, expect, it } from 'bun:test'
import { getCanonicalHeadBlock } from './getCanonicalHeadBlock.js'
import { createBaseChain } from '../createBaseChain.js'
import { putBlock } from './putBlock.js'
import { getMockBlocks } from '../test/getBlocks.js'
import { optimism } from '@tevm/common'
import { InternalError } from '@tevm/errors'
describe(getCanonicalHeadBlock.name, async () => {
const blocks = await getMockBlocks()
it('should get the canonical head block', async () => {
const chain = createBaseChain({ common: optimism.copy() })
await putBlock(chain)(blocks[0])
expect(await getCanonicalHeadBlock(chain)()).toBe(blocks[0])
})
it('should throw an error if not cannonical block', async () => {
const chain = createBaseChain({ common: optimism.copy() })
chain.blocksByTag.set('latest', undefined)
let error = await getCanonicalHeadBlock(chain)().catch((e) => e)
expect(error).toBeInstanceOf(InternalError)
expect(error).toMatchSnapshot()
})
})
import { describe, expect, it } from 'bun:test'
import { getCanonicalHeadBlock } from './getCanonicalHeadBlock.js'
import { createBaseChain } from '../createBaseChain.js'
import { putBlock } from './putBlock.js'
import { getMockBlocks } from '../test/getBlocks.js'
import { optimism } from '@tevm/common'
import { InternalError } from '@tevm/errors'
describe(getCanonicalHeadBlock.name, async () => {
const blocks = await getMockBlocks()
it('should get the canonical head block', async () => {
const chain = createBaseChain({ common: optimism.copy() })
await putBlock(chain)(blocks[0])
expect(await getCanonicalHeadBlock(chain)()).toBe(blocks[0])
})
it('should throw an error if not cannonical block', async () => {
const chain = createBaseChain({ common: optimism.copy() })
chain.blocksByTag.set('latest', undefined)
const error = await getCanonicalHeadBlock(chain)().catch((e) => e)
expect(error).toBeInstanceOf(InternalError)
expect(error).toMatchSnapshot()
})
})
Tools
Biome

[error] 19-19: This let declares a variable that is only assigned once.

'error' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

39 changes: 39 additions & 0 deletions packages/blockchain/src/actions/getIteratorHead.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { describe, expect, it } from 'bun:test'
import { getIteratorHead } from './getIteratorHead.js'
import { createBaseChain } from '../createBaseChain.js'
import { putBlock } from './putBlock.js'
import { getMockBlocks } from '../test/getBlocks.js'
import { optimism } from '@tevm/common'

describe(getIteratorHead.name, async () => {
const blocks = await getMockBlocks()

it('should get the iterator head block for default tag', async () => {
const chain = createBaseChain({ common: optimism.copy() })
await putBlock(chain)(blocks[0])
chain.blocksByTag.set('vm' as any, blocks[0]) // Set the iterator head
expect(await getIteratorHead(chain)()).toBe(blocks[0])
})

it('should get the iterator head block for a specific tag', async () => {
const chain = createBaseChain({ common: optimism.copy() })
await putBlock(chain)(blocks[1])
chain.blocksByTag.set('myTag' as any, blocks[1]) // Set the iterator head
expect(await getIteratorHead(chain)('myTag')).toBe(blocks[1])
})

it('should throw an error if the iterator head block does not exist', async () => {
const chain = createBaseChain({ common: optimism.copy() })
let error = await getIteratorHead(chain)('nonExistentTag').catch((e) => e)
Copy link
Contributor

Choose a reason for hiding this comment

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

Use const for variables that are not reassigned.

The variables error are never reassigned and should be declared with const instead of let to reflect their immutable nature after assignment.

- let error = await getIteratorHead(chain)('nonExistentTag').catch((e) => e)
+ const error = await getIteratorHead(chain)('nonExistentTag').catch((e) => e)

Also applies to: 35-35

Tools
Biome

[error] 27-27: This let declares a variable that is only assigned once.

'error' is never reassigned.

Safe fix: Use const instead.

(lint/style/useConst)

expect(error).toBeInstanceOf(Error)
expect(error.message).toBe('No block with tag nonExistentTag exists. Current tags include ')
})

it('should include current tags in error message if the iterator head block does not exist', async () => {
const chain = createBaseChain({ common: optimism.copy() })
chain.blocksByTag.set('someTag' as any, blocks[0])
let error = await getIteratorHead(chain)('nonExistentTag').catch((e) => e)
expect(error).toBeInstanceOf(Error)
expect(error.message).toBe(`No block with tag nonExistentTag exists. Current tags include someTag`)
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid unnecessary template literals.

The error message does not require a template literal as it does not perform any interpolation.

- expect(error.message).toBe(`No block with tag nonExistentTag exists. Current tags include someTag`)
+ expect(error.message).toBe('No block with tag nonExistentTag exists. Current tags include someTag')
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
expect(error.message).toBe(`No block with tag nonExistentTag exists. Current tags include someTag`)
expect(error.message).toBe('No block with tag nonExistentTag exists. Current tags include someTag')
Tools
Biome

[error] 37-37: Do not use template literals if interpolation and special-character handling are not needed.

Unsafe fix: Replace with string literal

(lint/style/noUnusedTemplateLiteral)

})
})
Loading
Loading