From a133e27f98f3dd919594ddd2cce8db53edcbbef2 Mon Sep 17 00:00:00 2001 From: Ryan Ghods Date: Wed, 25 May 2022 16:53:19 -0700 Subject: [PATCH 1/2] client: small engine updates (#1902) * simplify txpool, fix runExecution, allow safe block to also be zeros during transition * fix beacon sync skeleton fill with new vmexecution.run loop param --- packages/client/lib/blockchain/chain.ts | 3 - packages/client/lib/execution/vmexecution.ts | 21 ++- packages/client/lib/rpc/modules/engine.ts | 50 ++++--- packages/client/lib/service/txpool.ts | 123 ++++++++---------- packages/client/lib/sync/beaconsync.ts | 20 +-- .../integration/fullethereumservice.spec.ts | 8 +- .../client/test/miner/pendingBlock.spec.ts | 3 +- .../rpc/engine/forkchoiceUpdatedV1.spec.ts | 4 +- .../test/rpc/eth/sendRawTransaction.spec.ts | 6 +- packages/client/test/rpc/helpers.ts | 4 + packages/client/test/sync/txpool.spec.ts | 61 ++++----- 11 files changed, 141 insertions(+), 162 deletions(-) diff --git a/packages/client/lib/blockchain/chain.ts b/packages/client/lib/blockchain/chain.ts index 6bfd5a1b133..8ecd2166c67 100644 --- a/packages/client/lib/blockchain/chain.ts +++ b/packages/client/lib/blockchain/chain.ts @@ -83,9 +83,6 @@ export class Chain { public chainDB: LevelUp public blockchain: Blockchain public opened: boolean - public terminalPoWBlock: Block | undefined - public transitionPoSBlock: Block | undefined - public lastFinalizedBlockHash: Buffer | undefined private _headers: ChainHeaders = { latest: null, diff --git a/packages/client/lib/execution/vmexecution.ts b/packages/client/lib/execution/vmexecution.ts index 3f20076fc71..9cc29391a88 100644 --- a/packages/client/lib/execution/vmexecution.ts +++ b/packages/client/lib/execution/vmexecution.ts @@ -115,6 +115,7 @@ export class VMExecution extends Execution { * @param blocks Array of blocks to save pending receipts and set the last block as the head */ async setHead(blocks: Block[]): Promise { + await this.chain.blockchain.setIteratorHead('vm', blocks[blocks.length - 1].hash()) await this.chain.putBlocks(blocks, true) for (const block of blocks) { const receipts = this.pendingReceipts?.get(block.hash().toString('hex')) @@ -123,19 +124,15 @@ export class VMExecution extends Execution { this.pendingReceipts?.delete(block.hash().toString('hex')) } } - const head = blocks[blocks.length - 1] - await this.chain.blockchain.setIteratorHead('vm', head.hash()) } /** * Runs the VM execution - * + * @param loop Whether to continue iterating until vm head equals chain head (default: true) * @returns number of blocks executed */ - async run(): Promise { - if (this.running) { - return 0 - } + async run(loop = true): Promise { + if (this.running) return 0 this.running = true let numExecuted: number | undefined @@ -148,7 +145,7 @@ export class VMExecution extends Execution { let errorBlock: Block | undefined while ( - (numExecuted === undefined || numExecuted === this.NUM_BLOCKS_PER_ITERATION) && + (numExecuted === undefined || (loop && numExecuted === this.NUM_BLOCKS_PER_ITERATION)) && !startHeadBlock.hash().equals(canonicalHead.hash()) ) { let txCounter = 0 @@ -159,11 +156,9 @@ export class VMExecution extends Execution { this.vmPromise = blockchain.iterator( 'vm', async (block: Block, reorg: boolean) => { - if (errorBlock) { - return - } + if (errorBlock) return // determine starting state for block run - // if we are just starting or if a chain re-org has happened + // if we are just starting or if a chain reorg has happened if (!headBlock || reorg) { const parentBlock = await blockchain.getBlock(block.header.parentHash) parentState = parentBlock.header.stateRoot @@ -278,7 +273,7 @@ export class VMExecution extends Execution { canonicalHead = await this.vm.blockchain.getLatestBlock() } this.running = false - return numExecuted as number + return numExecuted ?? 0 } /** diff --git a/packages/client/lib/rpc/modules/engine.ts b/packages/client/lib/rpc/modules/engine.ts index 5c863df51d5..f57e2fb3601 100644 --- a/packages/client/lib/rpc/modules/engine.ts +++ b/packages/client/lib/rpc/modules/engine.ts @@ -536,23 +536,6 @@ export class Engine { } } - if (safeBlockHash !== headBlockHash) { - try { - await this.chain.getBlock(toBuffer(safeBlockHash)) - } catch (error) { - const message = 'safe block hash not available' - this.connectionManager.lastForkchoiceUpdate({ - state: params[0], - response: undefined, - error: message, - }) - throw { - code: INVALID_PARAMS, - message, - } - } - } - const vmHeadHash = this.chain.headers.latest!.hash() if (!vmHeadHash.equals(headBlock.hash())) { let parentBlocks: Block[] = [] @@ -601,19 +584,34 @@ export class Engine { } /* - * Process finalized block - * All zeros means no finalized block yet which is okay + * Process safe and finalized block + * Allowed to have zero value while transition block is finalizing */ - const zeroHash = zeros(32) - const finalizedHash = toBuffer(finalizedBlockHash) - if (!finalizedHash.equals(zeroHash)) { + const zeroBlockHash = zeros(32) + const safe = toBuffer(safeBlockHash) + if (!safe.equals(headBlock.hash()) && !safe.equals(zeroBlockHash)) { + try { + await this.chain.getBlock(safe) + } catch (error) { + const message = 'safe block not available' + this.connectionManager.lastForkchoiceUpdate({ + state: params[0], + response: undefined, + error: message, + }) + throw { + code: INVALID_PARAMS, + message, + } + } + } + const finalized = toBuffer(finalizedBlockHash) + if (!finalized.equals(zeroBlockHash)) { try { - this.chain.lastFinalizedBlockHash = ( - await this.chain.getBlock(toBuffer(finalizedBlockHash)) - ).hash() + await this.chain.getBlock(finalized) } catch (error) { throw { - message: 'finalized block hash not available', + message: 'finalized block not available', code: INVALID_PARAMS, } } diff --git a/packages/client/lib/service/txpool.ts b/packages/client/lib/service/txpool.ts index 1ebcad24535..ab95a79e759 100644 --- a/packages/client/lib/service/txpool.ts +++ b/packages/client/lib/service/txpool.ts @@ -16,7 +16,7 @@ import type { Block } from '@ethereumjs/block' // Configuration constants const MIN_GAS_PRICE_BUMP_PERCENT = 10 -const MIN_GAS_PRICE = new BN(1000000000) // 1 GWei +const MIN_GAS_PRICE = new BN(100000000) // .1 GWei const TX_MAX_DATA_SIZE = 128 * 1024 // 128KB const MAX_POOL_SIZE = 5000 const MAX_TXS_PER_ACCOUNT = 100 @@ -72,6 +72,7 @@ export class TxPool { public running: boolean /* global NodeJS */ + private _cleanupInterval: NodeJS.Timeout | undefined private _logInterval: NodeJS.Timeout | undefined /** @@ -178,6 +179,10 @@ export class TxPool { if (this.running) { return false } + this._cleanupInterval = setInterval( + this.cleanup.bind(this), + this.POOLED_STORAGE_TIME_LIMIT * 1000 * 60 + ) this._logInterval = setInterval(this._logPoolStats.bind(this), this.LOG_STATISTICS_INTERVAL) this.running = true this.config.logger.info('TxPool started.') @@ -225,15 +230,11 @@ export class TxPool { } private validateTxGasBump(existingTx: TypedTransaction, addedTx: TypedTransaction) { - const existingTxGasPrice = this.txGasPrice(existingTx) - const newGasPrice = this.txGasPrice(addedTx) - const minTipCap = existingTxGasPrice.tip.add( - existingTxGasPrice.tip.muln(MIN_GAS_PRICE_BUMP_PERCENT).divn(100) - ) - const minFeeCap = existingTxGasPrice.maxFee.add( - existingTxGasPrice.maxFee.muln(MIN_GAS_PRICE_BUMP_PERCENT).divn(100) - ) - if (newGasPrice.tip.lt(minTipCap) || newGasPrice.maxFee.lt(minFeeCap)) { + const { maxFee: existingMaxFee, tip: existingTip } = this.txGasPrice(existingTx) + const { maxFee: newMaxFee, tip: newTip } = this.txGasPrice(addedTx) + const minTipCap = existingTip.add(existingTip.muln(MIN_GAS_PRICE_BUMP_PERCENT).divn(100)) + const minFeeCap = existingMaxFee.add(existingMaxFee.muln(MIN_GAS_PRICE_BUMP_PERCENT).divn(100)) + if (newTip.lt(minTipCap) || newMaxFee.lt(minFeeCap)) { throw new Error('replacement gas too low') } } @@ -251,17 +252,15 @@ export class TxPool { `Tx is too large (${tx.data.length} bytes) and exceeds the max data size of ${TX_MAX_DATA_SIZE} bytes` ) } - const currentGasPrice = this.txGasPrice(tx) + const { maxFee, tip } = this.txGasPrice(tx) // This is the tip which the miner receives: miner does not want // to mine underpriced txs where miner gets almost no fees - const currentTip = currentGasPrice.tip if (!isLocalTransaction) { - const txsInPool = this.txsInPool - if (txsInPool >= MAX_POOL_SIZE) { + if (this.txsInPool >= MAX_POOL_SIZE) { throw new Error('Cannot add tx: pool is full') } // Local txs are not checked against MIN_GAS_PRICE - if (currentTip.lt(MIN_GAS_PRICE)) { + if (tip.lt(MIN_GAS_PRICE)) { throw new Error(`Tx does not pay the minimum gas price of ${MIN_GAS_PRICE}`) } } @@ -283,27 +282,27 @@ export class TxPool { this.validateTxGasBump(existingTxn.tx, tx) } } - const block = await this.service.chain.getLatestHeader() - if (block.baseFeePerGas) { - if (currentGasPrice.maxFee.lt(block.baseFeePerGas)) { - throw new Error( - `Tx cannot pay basefee of ${block.baseFeePerGas}, have ${currentGasPrice.maxFee}.` - ) - } + const latest = this.service.chain.headers.latest! + if (latest.baseFeePerGas && maxFee.lt(latest.baseFeePerGas.divn(2)) && !isLocalTransaction) { + throw new Error( + `Tx max fee of ${maxFee} not within 50% range of current basefee ${latest.baseFeePerGas}` + ) } - if (tx.gasLimit.gt(block.gasLimit)) { - throw new Error(`Tx gaslimit of ${tx.gasLimit} exceeds block gas limit of ${block.gasLimit}`) + if (tx.gasLimit.gt(latest.gasLimit)) { + throw new Error( + `Tx gas limit of ${tx.gasLimit} exceeds last block gas limit of ${latest.gasLimit}` + ) } const account = await this.vm.stateManager.getAccount(senderAddress) if (account.nonce.gt(tx.nonce)) { throw new Error( - `0x${sender} tries to send a tx with nonce ${tx.nonce}, but account has nonce ${account.nonce} (tx nonce too low)` + `Tx nonce too low for 0x${sender}, account has nonce ${account.nonce} and tx has nonce ${tx.nonce}` ) } - const minimumBalance = tx.value.add(currentGasPrice.maxFee.mul(tx.gasLimit)) + const minimumBalance = tx.value.add(maxFee.mul(tx.gasLimit)) if (account.balance.lt(minimumBalance)) { throw new Error( - `0x${sender} does not have enough balance to cover transaction costs, need ${minimumBalance}, but have ${account.balance}` + `Insufficient balance to cover transaction costs for 0x${sender}, need ${minimumBalance}, but have ${account.balance}` ) } } @@ -312,12 +311,12 @@ export class TxPool { * Adds a tx to the pool. * * If there is a tx in the pool with the same address and - * nonce it will be replaced by the new tx, if it has a sufficent gas bump. + * nonce it will be replaced by the new tx, if it has a sufficient gas bump. * This also verifies certain constraints, if these are not met, tx will not be added to the pool. * @param tx Transaction - * @param isLocalTransaction Boolean which is true if this is a local transaction (this drops some constraint checks) + * @param isLocalTransaction if this is a local transaction (loosens some constraints) (default: false) */ - async add(tx: TypedTransaction, isLocalTransaction: boolean = false) { + async add(tx: TypedTransaction, isLocalTransaction = false) { await this.validate(tx, isLocalTransaction) const address: UnprefixedAddress = tx.getSenderAddress().toString().slice(2) let add: TxPoolObject[] = this.pool.get(address) ?? [] @@ -330,8 +329,8 @@ export class TxPool { const added = Date.now() add.push({ tx, added, hash }) this.pool.set(address, add) - this.txsInPool++ this.handled.set(hash, { address, added }) + this.txsInPool++ } /** @@ -344,9 +343,7 @@ export class TxPool { for (const txHash of txHashes) { const txHashStr = txHash.toString('hex') const handled = this.handled.get(txHashStr) - if (!handled) { - continue - } + if (!handled) continue const inPool = this.pool.get(handled.address)?.filter((poolObj) => poolObj.hash === txHashStr) if (inPool && inPool.length === 1) { found.push(inPool[0].tx) @@ -360,14 +357,12 @@ export class TxPool { * @param txHash Hash of the transaction */ removeByHash(txHash: UnprefixedHash) { - if (!this.handled.has(txHash)) { - return - } - const address = this.handled.get(txHash)!.address - if (!this.pool.has(address)) { - return - } - const newPoolObjects = this.pool.get(address)!.filter((poolObj) => poolObj.hash !== txHash) + const handled = this.handled.get(txHash) + if (!handled) return + const { address } = handled + const poolObjects = this.pool.get(address) + if (!poolObjects) return + const newPoolObjects = poolObjects.filter((poolObj) => poolObj.hash !== txHash) this.txsInPool-- if (newPoolObjects.length === 0) { // List of txs for address is now empty, can delete @@ -463,15 +458,12 @@ export class TxPool { * @param peerPool Reference to the {@link PeerPool} */ async handleAnnouncedTxs(txs: TypedTransaction[], peer: Peer, peerPool: PeerPool) { - if (!this.running || txs.length === 0) { - return - } + if (!this.running || txs.length === 0) return this.config.logger.debug(`TxPool: received new transactions number=${txs.length}`) this.addToKnownByPeer( txs.map((tx) => tx.hash()), peer ) - this.cleanup() const newTxHashes = [] for (const tx of txs) { @@ -501,7 +493,6 @@ export class TxPool { async handleAnnouncedTxHashes(txHashes: Buffer[], peer: Peer, peerPool: PeerPool) { if (!this.running || txHashes.length === 0) return this.addToKnownByPeer(txHashes, peer) - this.cleanup() const reqHashes = [] for (const txHash of txHashes) { @@ -550,9 +541,7 @@ export class TxPool { * Remove txs included in the latest blocks from the tx pool */ removeNewBlockTxs(newBlocks: Block[]) { - if (!this.running) { - return - } + if (!this.running) return for (const block of newBlocks) { for (const tx of block.transactions) { const txHash: UnprefixedHash = tx.hash().toString('hex') @@ -567,27 +556,28 @@ export class TxPool { cleanup() { // Remove txs older than POOLED_STORAGE_TIME_LIMIT from the pool // as well as the list of txs being known by a peer - let compDate = Date.now() - this.POOLED_STORAGE_TIME_LIMIT * 60 - for (const mapToClean of [this.pool, this.knownByPeer]) { - mapToClean.forEach((objects, key) => { + let compDate = Date.now() - this.POOLED_STORAGE_TIME_LIMIT * 1000 * 60 + for (const [i, mapToClean] of [this.pool, this.knownByPeer].entries()) { + for (const [key, objects] of mapToClean) { const updatedObjects = objects.filter((obj) => obj.added >= compDate) if (updatedObjects.length < objects.length) { + if (i === 0) this.txsInPool -= objects.length - updatedObjects.length if (updatedObjects.length === 0) { mapToClean.delete(key) } else { mapToClean.set(key, updatedObjects) } } - }) + } } // Cleanup handled txs - compDate = Date.now() - this.HANDLED_CLEANUP_TIME_LIMIT * 60 - this.handled.forEach((handleObj, address) => { + compDate = Date.now() - this.HANDLED_CLEANUP_TIME_LIMIT * 1000 * 60 + for (const [address, handleObj] of this.handled) { if (handleObj.added < compDate) { this.handled.delete(address) } - }) + } } /** @@ -645,11 +635,11 @@ export class TxPool { ) if (!txsSortedByNonce[0].nonce.eq(nonce)) { // Account nonce does not match the lowest known tx nonce, - // therefore no txs from this address are currently exectuable + // therefore no txs from this address are currently executable continue } if (baseFee) { - // If any tx has an insiffucient gasPrice, + // If any tx has an insufficient gasPrice, // remove all txs after that since they cannot be executed const found = txsSortedByNonce.findIndex((tx) => this.normalizedGasPrice(tx).lt(baseFee)) if (found > -1) { @@ -663,10 +653,10 @@ export class TxPool { comparBefore: (a: TypedTransaction, b: TypedTransaction) => this.normalizedGasPrice(b, baseFee).sub(this.normalizedGasPrice(a, baseFee)).ltn(0), }) - byNonce.forEach((txs, address) => { + for (const [address, txs] of byNonce) { byPrice.insert(txs[0]) byNonce.set(address, txs.slice(1)) - }) + } // Merge by replacing the best with the next from the same account while (byPrice.length > 0) { // Retrieve the next best transaction by price @@ -689,11 +679,9 @@ export class TxPool { * Stop pool execution */ stop(): boolean { - if (!this.running) { - return false - } + if (!this.running) return false + clearInterval(this._cleanupInterval as NodeJS.Timeout) clearInterval(this._logInterval as NodeJS.Timeout) - this.running = false this.config.logger.info('TxPool stopped.') return true @@ -704,13 +692,14 @@ export class TxPool { */ close() { this.pool.clear() + this.handled.clear() + this.txsInPool = 0 this.opened = false } _logPoolStats() { - const count = this.txsInPool this.config.logger.info( - `TxPool Statistics txs=${count} senders=${this.pool.size} peers=${this.service.pool.peers.length}` + `TxPool Statistics txs=${this.txsInPool} senders=${this.pool.size} peers=${this.service.pool.peers.length}` ) } } diff --git a/packages/client/lib/sync/beaconsync.ts b/packages/client/lib/sync/beaconsync.ts index eaa1039b11a..67a4d417f5b 100644 --- a/packages/client/lib/sync/beaconsync.ts +++ b/packages/client/lib/sync/beaconsync.ts @@ -52,6 +52,9 @@ export class BeaconSynchronizer extends Synchronizer { await this.pool.open() await this.skeleton.open() + this.config.events.on(Event.SYNC_FETCHED_BLOCKS, this.processSkeletonBlocks) + this.config.events.on(Event.CHAIN_UPDATED, this.runExecution) + const subchain = this.skeleton.bounds() if (subchain) { const { head, tail, next } = subchain @@ -105,9 +108,6 @@ export class BeaconSynchronizer extends Synchronizer { if (this.running) return this.running = true - this.config.events.on(Event.SYNC_FETCHED_BLOCKS, this.processSkeletonBlocks) - this.config.events.on(Event.CHAIN_UPDATED, this.runExecution) - if (block) { await this.skeleton.initSync(block) } @@ -245,11 +245,13 @@ export class BeaconSynchronizer extends Synchronizer { * Runs vm execution on {@link Event.CHAIN_UPDATED} */ async runExecution(): Promise { - // Execute single block when executed head is near chain head, + // Execute single block when within 50 blocks of head, // otherwise run execution in batch of 50 blocks when filling canonical chain. - const vmHead = (await this.chain.blockchain.getIteratorHead('vm')).header.number - if (this.chain.blocks.height.lt(vmHead.addn(50)) || this.chain.blocks.height.modrn(50) === 0) { - void this.execution.run() + if ( + this.chain.blocks.height.gt(this.skeleton.bounds().head.subn(50)) || + this.chain.blocks.height.modrn(50) === 0 + ) { + void this.execution.run(false) } } @@ -257,8 +259,6 @@ export class BeaconSynchronizer extends Synchronizer { * Stop synchronization. Returns a promise that resolves once its stopped. */ async stop(): Promise { - this.config.events.removeListener(Event.SYNC_FETCHED_BLOCKS, this.processSkeletonBlocks) - this.config.events.removeListener(Event.CHAIN_UPDATED, this.runExecution) return super.stop() } @@ -267,6 +267,8 @@ export class BeaconSynchronizer extends Synchronizer { */ async close() { if (!this.opened) return + this.config.events.removeListener(Event.SYNC_FETCHED_BLOCKS, this.processSkeletonBlocks) + this.config.events.removeListener(Event.CHAIN_UPDATED, this.runExecution) await super.close() } } diff --git a/packages/client/test/integration/fullethereumservice.spec.ts b/packages/client/test/integration/fullethereumservice.spec.ts index 1539b394248..db4d85b8417 100644 --- a/packages/client/test/integration/fullethereumservice.spec.ts +++ b/packages/client/test/integration/fullethereumservice.spec.ts @@ -2,7 +2,7 @@ import tape from 'tape' import Blockchain from '@ethereumjs/blockchain' import { FeeMarketEIP1559Transaction } from '@ethereumjs/tx' import { Block } from '@ethereumjs/block' -import { BN, toBuffer } from 'ethereumjs-util' +import { Account, BN, toBuffer } from 'ethereumjs-util' import { Config } from '../../lib/config' import { FullEthereumService } from '../../lib/service' import { Event } from '../../lib/types' @@ -69,8 +69,12 @@ tape('[Integration:FullEthereumService]', async (t) => { peer.eth!.send('NewBlock', [block, new BN(1)]) const txData = - '0x02f90108018001018402625a0094cccccccccccccccccccccccccccccccccccccccc830186a0b8441a8451e600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000f85bf859940000000000000000000000000000000000000101f842a00000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000060a701a0afb6e247b1c490e284053c87ab5f6b59e219d51f743f7a4d83e400782bc7e4b9a0479a268e0e0acd4de3f1e28e4fac2a6b32a4195e8dfa9d19147abe8807aa6f64' + '0x02f901100180843b9aca00843b9aca008402625a0094cccccccccccccccccccccccccccccccccccccccc830186a0b8441a8451e600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000f85bf859940000000000000000000000000000000000000101f842a00000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000060a701a0afb6e247b1c490e284053c87ab5f6b59e219d51f743f7a4d83e400782bc7e4b9a0479a268e0e0acd4de3f1e28e4fac2a6b32a4195e8dfa9d19147abe8807aa6f64' const tx = FeeMarketEIP1559Transaction.fromSerializedTx(toBuffer(txData)) + await service.execution.vm.stateManager.putAccount( + tx.getSenderAddress(), + new Account(new BN(0), new BN('40000000000100000')) + ) await service.txPool.add(tx) const [_, txs] = await peer.eth!.getPooledTransactions({ hashes: [tx.hash()] }) t.equal( diff --git a/packages/client/test/miner/pendingBlock.spec.ts b/packages/client/test/miner/pendingBlock.spec.ts index ecfed453761..069e6906e2c 100644 --- a/packages/client/test/miner/pendingBlock.spec.ts +++ b/packages/client/test/miner/pendingBlock.spec.ts @@ -39,8 +39,7 @@ const config = new Config({ transports: [], common }) const setup = () => { const service: any = { chain: { - headers: { height: new BN(0) }, - getLatestHeader: () => BlockHeader.fromHeaderData({}), + headers: { height: new BN(0), latest: BlockHeader.fromHeaderData({}, { common }) }, }, execution: { vm: { diff --git a/packages/client/test/rpc/engine/forkchoiceUpdatedV1.spec.ts b/packages/client/test/rpc/engine/forkchoiceUpdatedV1.spec.ts index 707d2f9cbd0..ebf0bc7093e 100644 --- a/packages/client/test/rpc/engine/forkchoiceUpdatedV1.spec.ts +++ b/packages/client/test/rpc/engine/forkchoiceUpdatedV1.spec.ts @@ -211,7 +211,7 @@ tape(`${method}: unknown finalized block hash`, async (t) => { finalizedBlockHash: '0x3b8fb240d288781d4aac94d3fd16809ee413bc99294a085798a589dae51ddd4b', }, ]) - const expectRes = checkError(t, INVALID_PARAMS, 'finalized block hash not available') + const expectRes = checkError(t, INVALID_PARAMS, 'finalized block not available') await baseRequest(t, server, req, 200, expectRes) }) @@ -223,7 +223,7 @@ tape(`${method}: invalid safe block hash`, async (t) => { safeBlockHash: '0x3b8fb240d288781d4aac94d3fd16809ee413bc99294a085798a589dae51ddd4b', }, ]) - const expectRes = checkError(t, INVALID_PARAMS, 'safe block hash not available') + const expectRes = checkError(t, INVALID_PARAMS, 'safe block not available') await baseRequest(t, server, req, 200, expectRes) }) diff --git a/packages/client/test/rpc/eth/sendRawTransaction.spec.ts b/packages/client/test/rpc/eth/sendRawTransaction.spec.ts index 901efb2f7c4..bdf376a2f67 100644 --- a/packages/client/test/rpc/eth/sendRawTransaction.spec.ts +++ b/packages/client/test/rpc/eth/sendRawTransaction.spec.ts @@ -70,11 +70,7 @@ tape(`${method}: call with invalid arguments: not enough balance`, async (t) => '0x02f90108018001018402625a0094cccccccccccccccccccccccccccccccccccccccc830186a0b8441a8451e600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000f85bf859940000000000000000000000000000000000000101f842a00000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000060a701a0afb6e247b1c490e284053c87ab5f6b59e219d51f743f7a4d83e400782bc7e4b9a0479a268e0e0acd4de3f1e28e4fac2a6b32a4195e8dfa9d19147abe8807aa6f64' const req = params(method, [txData]) - const expectRes = checkError( - t, - INVALID_PARAMS, - 'does not have enough balance to cover transaction costs' - ) + const expectRes = checkError(t, INVALID_PARAMS, 'Insufficient balance to cover transaction costs') await baseRequest(t, server, req, 200, expectRes) }) diff --git a/packages/client/test/rpc/helpers.ts b/packages/client/test/rpc/helpers.ts index e7415b09fe3..600fc713816 100644 --- a/packages/client/test/rpc/helpers.ts +++ b/packages/client/test/rpc/helpers.ts @@ -1,5 +1,6 @@ import tape from 'tape' import { Server as RPCServer, HttpServer } from 'jayson/promise' +import { BlockHeader } from '@ethereumjs/block' import Blockchain from '@ethereumjs/blockchain' import Common, { Chain as ChainEnum } from '@ethereumjs/common' import { Address, BN } from 'ethereumjs-util' @@ -70,6 +71,9 @@ export function createClient(clientOpts: any = {}) { const clientConfig = { ...defaultClientConfig, ...clientOpts } chain.getTd = async (_hash: Buffer, _num: BN) => new BN(1000) + if (chain._headers) { + chain._headers.latest = BlockHeader.fromHeaderData({}, { common }) + } config.synchronized = true config.lastSyncDate = Date.now() diff --git a/packages/client/test/sync/txpool.spec.ts b/packages/client/test/sync/txpool.spec.ts index 53eb3b17521..b12dfd07758 100644 --- a/packages/client/test/sync/txpool.spec.ts +++ b/packages/client/test/sync/txpool.spec.ts @@ -11,8 +11,7 @@ const setup = () => { const config = new Config({ transports: [] }) const service: any = { chain: { - headers: { height: new BN(0) }, - getLatestHeader: () => BlockHeader.fromHeaderData({}), + headers: { height: new BN(0), latest: BlockHeader.fromHeaderData({}) }, }, execution: { vm: { @@ -297,7 +296,7 @@ tape('[TxPool]', async (t) => { } catch (e: any) { t.ok( e.message.includes('replacement gas too low'), - 'succesfully failed adding underpriced txn' + 'successfully failed adding underpriced txn' ) } t.equal(pool.pool.size, 1, 'pool size 1') @@ -361,7 +360,7 @@ tape('[TxPool]', async (t) => { break } } - t.notOk(await handleTxs(txs, 'pool is full'), 'succesfully rejected too many txs') + t.notOk(await handleTxs(txs, 'pool is full'), 'successfully rejected too many txs') }) t.test('announcedTxHashes() -> reject if account tries to send more than 100 txs', async (t) => { @@ -375,7 +374,7 @@ tape('[TxPool]', async (t) => { t.notOk( await handleTxs(txs, 'already have max amount of txs for this account'), - 'succesfully rejected too many txs from same account' + 'successfully rejected too many txs from same account' ) }) @@ -391,7 +390,7 @@ tape('[TxPool]', async (t) => { t.notOk( await handleTxs(txs, 'Attempting to add tx to txpool which is not signed'), - 'succesfully rejected unsigned tx' + 'successfully rejected unsigned tx' ) }) @@ -407,10 +406,10 @@ tape('[TxPool]', async (t) => { ) t.notOk( - await handleTxs(txs, 'tx nonce too low', { + await handleTxs(txs, 'Tx nonce too low', { getAccount: () => new Account(new BN(1), new BN('50000000000000000000')), }), - 'succesfully rejected tx with invalid nonce' + 'successfully rejected tx with invalid nonce' ) }) @@ -430,7 +429,7 @@ tape('[TxPool]', async (t) => { await handleTxs(txs, 'exceeds the max data size', { getAccount: () => new Account(new BN(0), new BN('50000000000000000000000')), }), - 'succesfully rejected tx with too much data' + 'successfully rejected tx with too much data' ) }) @@ -447,10 +446,10 @@ tape('[TxPool]', async (t) => { ) t.notOk( - await handleTxs(txs, 'does not have enough balance to cover transaction costs', { + await handleTxs(txs, 'Insufficient balance', { getAccount: () => new Account(new BN(0), new BN('0')), }), - 'succesfully rejected account with too low balance' + 'successfully rejected account with too low balance' ) }) @@ -467,15 +466,13 @@ tape('[TxPool]', async (t) => { const { pool } = setup() - ;(pool).service.chain.getLatestHeader = async () => { - return { - baseFeePerGas: new BN(1000000000 + 1), - } + ;(pool).service.chain.headers.latest = { + baseFeePerGas: new BN(3000000000), } t.notOk( - await handleTxs(txs, 'cannot pay basefee', undefined, pool), - 'succesfully rejected tx with too low gas price' + await handleTxs(txs, 'not within 50% range of current basefee', undefined, pool), + 'successfully rejected tx with too low gas price' ) }) @@ -495,15 +492,13 @@ tape('[TxPool]', async (t) => { const { pool } = setup() - ;(pool).service.chain.getLatestHeader = async () => { - return { - gasLimit: new BN(5000), - } + ;(pool).service.chain.headers.latest = { + gasLimit: new BN(5000), } t.notOk( - await handleTxs(txs, 'exceeds block gas limit', undefined, pool), - 'succesfully rejected tx which has gas limit higher than block gas limit' + await handleTxs(txs, 'exceeds last block gas limit', undefined, pool), + 'successfully rejected tx which has gas limit higher than block gas limit' ) } ) @@ -524,7 +519,7 @@ tape('[TxPool]', async (t) => { t.notOk( await handleTxs(txs, 'this transaction is already in the TxPool', undefined, pool), - 'succesfully rejected tx which is already in pool' + 'successfully rejected tx which is already in pool' ) }) @@ -533,15 +528,15 @@ tape('[TxPool]', async (t) => { txs.push( FeeMarketEIP1559Transaction.fromTxData({ - maxFeePerGas: 1000000000 - 1, - maxPriorityFeePerGas: 1000000000 - 1, + maxFeePerGas: 10000000, + maxPriorityFeePerGas: 10000000, nonce: 0, }).sign(A.privateKey) ) t.notOk( await handleTxs(txs, 'does not pay the minimum gas price of'), - 'succesfully rejected tx with too low gas price' + 'successfully rejected tx with too low gas price' ) }) @@ -552,14 +547,14 @@ tape('[TxPool]', async (t) => { txs.push( AccessListEIP2930Transaction.fromTxData({ - gasPrice: 1000000000 - 1, + gasPrice: 10000000, nonce: 0, }).sign(A.privateKey) ) t.notOk( await handleTxs(txs, 'does not pay the minimum gas price of'), - 'succesfully rejected tx with too low gas price' + 'successfully rejected tx with too low gas price' ) } ) @@ -583,7 +578,7 @@ tape('[TxPool]', async (t) => { txs.push(tx) - t.notOk(await handleTxs(txs, ''), 'succesfully rejected tx with invalid tx type') + t.notOk(await handleTxs(txs, ''), 'successfully rejected tx with invalid tx type') } ) @@ -712,17 +707,17 @@ tape('[TxPool]', async (t) => { const address = txB01.getSenderAddress().toString().slice(2) const poolObj = pool.pool.get(address)![0] - poolObj.added = Date.now() - pool.POOLED_STORAGE_TIME_LIMIT * 60 - 1 + poolObj.added = Date.now() - pool.POOLED_STORAGE_TIME_LIMIT * 1000 * 60 - 1 pool.pool.set(address, [poolObj]) const knownByPeerObj1 = (pool as any).knownByPeer.get(peer.id)[0] const knownByPeerObj2 = (pool as any).knownByPeer.get(peer.id)[1] - knownByPeerObj1.added = Date.now() - pool.POOLED_STORAGE_TIME_LIMIT * 60 - 1 + knownByPeerObj1.added = Date.now() - pool.POOLED_STORAGE_TIME_LIMIT * 1000 * 60 - 1 ;(pool as any).knownByPeer.set(peer.id, [knownByPeerObj1, knownByPeerObj2]) const hash = txB01.hash().toString('hex') const handledObj = (pool as any).handled.get(hash) - handledObj.added = Date.now() - pool.HANDLED_CLEANUP_TIME_LIMIT * 60 - 1 + handledObj.added = Date.now() - pool.HANDLED_CLEANUP_TIME_LIMIT * 1000 * 60 - 1 ;(pool as any).handled.set(hash, handledObj) pool.cleanup() From 2b993aa8a8b47ffe4266fac8932322f516d6b8e4 Mon Sep 17 00:00:00 2001 From: Jochem Brouwer Date: Thu, 26 May 2022 20:07:53 +0200 Subject: [PATCH 2/2] VM/tests: ensure verifyPostConditions works (#1900) * vm/tests: ensure verifyPostConditions works * vm/tests/util: update output * vm/tests/util: make storage comments more clear --- .../tester/runners/BlockchainTestsRunner.ts | 34 +++++---- packages/vm/tests/util.ts | 75 +++++++++++-------- 2 files changed, 61 insertions(+), 48 deletions(-) diff --git a/packages/vm/tests/tester/runners/BlockchainTestsRunner.ts b/packages/vm/tests/tester/runners/BlockchainTestsRunner.ts index e73af390eb0..fe6c6cace05 100644 --- a/packages/vm/tests/tester/runners/BlockchainTestsRunner.ts +++ b/packages/vm/tests/tester/runners/BlockchainTestsRunner.ts @@ -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) { diff --git a/packages/vm/tests/util.ts b/packages/vm/tests/util.ts index 7dede49a5c8..005b208c55f 100644 --- a/packages/vm/tests/util.ts +++ b/packages/vm/tests/util.ts @@ -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) } @@ -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: ' + key) + t.comment('invalid account in the trie: ' + key) } }) 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() }) @@ -174,12 +174,19 @@ export function verifyAccountPostConditions( ) { return new Promise((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) { @@ -188,38 +195,40 @@ export function verifyAccountPostConditions( ] = acctData.storage[key] } - if (storageKeys.length > 0) { - 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 ${ + 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() - } + }) }) }