From d6674768b66403341a73c0b1e8b20fd3614d3921 Mon Sep 17 00:00:00 2001 From: ejMina226 <118474890+ejMina226@users.noreply.github.com> Date: Tue, 11 Mar 2025 11:40:41 +0000 Subject: [PATCH 01/22] Add in removeTransactionWhen for Account State Hook. --- packages/protocol/src/hooks/AccountStateHook.ts | 15 +++++++++++++++ .../src/protocol/ProvableTransactionHook.ts | 6 ++++++ .../src/mempool/private/PrivateMempool.ts | 15 +++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/packages/protocol/src/hooks/AccountStateHook.ts b/packages/protocol/src/hooks/AccountStateHook.ts index 3369e6683..67eadeb63 100644 --- a/packages/protocol/src/hooks/AccountStateHook.ts +++ b/packages/protocol/src/hooks/AccountStateHook.ts @@ -54,4 +54,19 @@ export class AccountStateHook extends ProvableTransactionHook { public async afterTransaction() { noop(); } + + // Under these conditions we want the tx removed from the mempool. + public async removeTransactionWhen({ + transaction, + }: BeforeTransactionHookArguments) { + const sender = transaction.sender.value; + + const aso = await this.accountState.get(sender); + + const accountState = aso.orElse(new AccountState({ nonce: UInt64.zero })); + + const currentNonce = accountState.nonce; + + return transaction.nonce.value.lessThan(currentNonce).toBoolean(); + } } diff --git a/packages/protocol/src/protocol/ProvableTransactionHook.ts b/packages/protocol/src/protocol/ProvableTransactionHook.ts index 247517b44..32aec366f 100644 --- a/packages/protocol/src/protocol/ProvableTransactionHook.ts +++ b/packages/protocol/src/protocol/ProvableTransactionHook.ts @@ -74,4 +74,10 @@ export abstract class ProvableTransactionHook< public abstract afterTransaction( execution: AfterTransactionHookArguments ): Promise; + + public async removeTransactionWhen( + execution: BeforeTransactionHookArguments + ): Promise { + return false; + } } diff --git a/packages/sequencer/src/mempool/private/PrivateMempool.ts b/packages/sequencer/src/mempool/private/PrivateMempool.ts index f1c56f5af..c7090d270 100644 --- a/packages/sequencer/src/mempool/private/PrivateMempool.ts +++ b/packages/sequencer/src/mempool/private/PrivateMempool.ts @@ -175,6 +175,21 @@ export class PrivateMempool executionContext.setup(contextInputs); const signedTransaction = tx.toProtocolTransaction(); + + // eslint-disable-next-line no-await-in-loop + const removeTxWhen = await this.accountStateHook.removeTransactionWhen({ + networkState: networkState, + transaction: signedTransaction.transaction, + signature: signedTransaction.signature, + prover: proverState, + }); + if (removeTxWhen) { + log.trace( + `Deleting tx ${tx.hash().toString()} from mempool because removeTransactionWhen condition is satisfied` + ); + // eslint-disable-next-line no-continue + continue; + } // eslint-disable-next-line no-await-in-loop await this.accountStateHook.beforeTransaction({ networkState: networkState, From 96f483dd2acd847c79638f061fa765dfb11a815d Mon Sep 17 00:00:00 2001 From: ejMina226 <118474890+ejMina226@users.noreply.github.com> Date: Tue, 11 Mar 2025 15:08:28 +0000 Subject: [PATCH 02/22] Add in test --- .../test/integration/MempoolTxRemoved.test.ts | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 packages/sequencer/test/integration/MempoolTxRemoved.test.ts diff --git a/packages/sequencer/test/integration/MempoolTxRemoved.test.ts b/packages/sequencer/test/integration/MempoolTxRemoved.test.ts new file mode 100644 index 000000000..63a96dfc4 --- /dev/null +++ b/packages/sequencer/test/integration/MempoolTxRemoved.test.ts @@ -0,0 +1,74 @@ +import { Balances } from "@proto-kit/library"; +import { Runtime } from "@proto-kit/module"; +import { TestingAppChain } from "@proto-kit/sdk"; +import { Bool, PrivateKey, UInt64 } from "o1js"; +import "reflect-metadata"; + +import { PrivateMempool, Sequencer } from "../../src"; + +import { createTransaction } from "./utils"; +import { Balance } from "./mocks/Balance"; + +describe("block production", () => { + const senderKey = PrivateKey.random(); + const appChain = TestingAppChain.fromRuntime({ Balance }); + let mempool: PrivateMempool; + let runtime: Runtime<{ Balances: typeof Balances; Balance: typeof Balance }>; + let sequencer: Sequencer; + + beforeAll(async () => { + appChain.configurePartial({ + Runtime: { + Balances: {}, + Balance: {}, + }, + Protocol: { + ...appChain.config.Protocol!, + }, + }); + + await appChain.start(); + runtime = appChain.runtime; + sequencer = appChain.sequencer; + + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + mempool = sequencer.resolve("Mempool"); + }); + + it("check tx is removed", async () => { + await mempool.add( + createTransaction({ + runtime, + method: ["Balance", "setBalanceIf"], + privateKey: senderKey, + args: [senderKey.toPublicKey(), UInt64.from(100), Bool(true)], + nonce: 0, + }) + ); + + await mempool.add( + createTransaction({ + runtime, + method: ["Balance", "setBalanceIf"], + privateKey: senderKey, + args: [senderKey.toPublicKey(), UInt64.from(100), Bool(true)], + nonce: 1, + }) + ); + + await appChain.produceBlock(); + + await mempool.add( + createTransaction({ + runtime, + method: ["Balance", "setBalanceIf"], + privateKey: senderKey, + args: [senderKey.toPublicKey(), UInt64.from(100), Bool(true)], + nonce: 0, + }) + ); + + const txs = await mempool.getTxs(); + expect(txs.length).toBe(0); + }, 60_000); +}); From 734d4ca793bdd716d53423021fb6398b69bc8653 Mon Sep 17 00:00:00 2001 From: ejMina226 <118474890+ejMina226@users.noreply.github.com> Date: Tue, 11 Mar 2025 15:12:25 +0000 Subject: [PATCH 03/22] Update test --- .../test/integration/MempoolTxRemoved.test.ts | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/packages/sequencer/test/integration/MempoolTxRemoved.test.ts b/packages/sequencer/test/integration/MempoolTxRemoved.test.ts index 63a96dfc4..8202f3086 100644 --- a/packages/sequencer/test/integration/MempoolTxRemoved.test.ts +++ b/packages/sequencer/test/integration/MempoolTxRemoved.test.ts @@ -56,8 +56,21 @@ describe("block production", () => { }) ); + const txs = await mempool.getTxs(); + expect(txs.length).toBe(2); + await appChain.produceBlock(); + await mempool.add( + createTransaction({ + runtime, + method: ["Balance", "setBalanceIf"], + privateKey: senderKey, + args: [senderKey.toPublicKey(), UInt64.from(100), Bool(true)], + nonce: 2, + }) + ); + await mempool.add( createTransaction({ runtime, @@ -68,7 +81,7 @@ describe("block production", () => { }) ); - const txs = await mempool.getTxs(); - expect(txs.length).toBe(0); + const txs2 = await mempool.getTxs(); + expect(txs2.length).toBe(1); }, 60_000); }); From 632b11e809dc7180047fb9a35e90a7d6ed3add07 Mon Sep 17 00:00:00 2001 From: ejMina226 <118474890+ejMina226@users.noreply.github.com> Date: Wed, 12 Mar 2025 13:31:15 +0000 Subject: [PATCH 04/22] Change code to remove tx from list --- .../src/services/prisma/PrismaTransactionStorage.ts | 11 +++++++++++ .../sequencer/src/mempool/private/PrivateMempool.ts | 2 ++ .../storage/inmemory/InMemoryTransactionStorage.ts | 4 ++++ .../src/storage/repositories/TransactionStorage.ts | 4 ++++ 4 files changed, 21 insertions(+) diff --git a/packages/persistance/src/services/prisma/PrismaTransactionStorage.ts b/packages/persistance/src/services/prisma/PrismaTransactionStorage.ts index 8592ea71e..1353dad43 100644 --- a/packages/persistance/src/services/prisma/PrismaTransactionStorage.ts +++ b/packages/persistance/src/services/prisma/PrismaTransactionStorage.ts @@ -1,5 +1,6 @@ import { inject, injectable } from "tsyringe"; import { PendingTransaction, TransactionStorage } from "@proto-kit/sequencer"; +import { Field } from "o1js"; import type { PrismaConnection } from "../../PrismaDatabaseConnection"; @@ -28,6 +29,16 @@ export class PrismaTransactionStorage implements TransactionStorage { return txs.map((tx) => this.transactionMapper.mapIn(tx)); } + public async removeTx(txHash: Field) { + const { prismaClient } = this.connection; + + await prismaClient.transaction.deleteMany({ + where: { + hash: txHash.toString(), + }, + }); + } + public async pushUserTransaction(tx: PendingTransaction): Promise { const { prismaClient } = this.connection; diff --git a/packages/sequencer/src/mempool/private/PrivateMempool.ts b/packages/sequencer/src/mempool/private/PrivateMempool.ts index c7090d270..708457a1a 100644 --- a/packages/sequencer/src/mempool/private/PrivateMempool.ts +++ b/packages/sequencer/src/mempool/private/PrivateMempool.ts @@ -184,6 +184,8 @@ export class PrivateMempool prover: proverState, }); if (removeTxWhen) { + // eslint-disable-next-line no-await-in-loop + await this.transactionStorage.removeTx(tx.hash()); log.trace( `Deleting tx ${tx.hash().toString()} from mempool because removeTransactionWhen condition is satisfied` ); diff --git a/packages/sequencer/src/storage/inmemory/InMemoryTransactionStorage.ts b/packages/sequencer/src/storage/inmemory/InMemoryTransactionStorage.ts index c50b06b1b..9ae355aa4 100644 --- a/packages/sequencer/src/storage/inmemory/InMemoryTransactionStorage.ts +++ b/packages/sequencer/src/storage/inmemory/InMemoryTransactionStorage.ts @@ -22,6 +22,10 @@ export class InMemoryTransactionStorage implements TransactionStorage { @inject("BatchStorage") private readonly batchStorage: InMemoryBatchStorage ) {} + public async removeTx(txHash: Field) { + this.queue = this.queue.filter((tx) => tx.hash() !== txHash); + } + public async getPendingUserTransactions(): Promise { const nextHeight = await this.blockStorage.getCurrentBlockHeight(); for ( diff --git a/packages/sequencer/src/storage/repositories/TransactionStorage.ts b/packages/sequencer/src/storage/repositories/TransactionStorage.ts index c739f68b4..204194d65 100644 --- a/packages/sequencer/src/storage/repositories/TransactionStorage.ts +++ b/packages/sequencer/src/storage/repositories/TransactionStorage.ts @@ -1,3 +1,5 @@ +import { Field } from "o1js"; + import { PendingTransaction } from "../../mempool/PendingTransaction"; export interface TransactionStorage { @@ -5,6 +7,8 @@ export interface TransactionStorage { getPendingUserTransactions: () => Promise; + removeTx: (txHash: Field) => Promise; + /** * Finds a transaction by its hash. * It returns both pending transaction and already included transactions From 288165dc455dd1b7c4cd362e77d86fa9d1fcd2c9 Mon Sep 17 00:00:00 2001 From: ejMina226 <118474890+ejMina226@users.noreply.github.com> Date: Thu, 13 Mar 2025 11:13:42 +0000 Subject: [PATCH 05/22] Increase timeout --- packages/sequencer/test/integration/MempoolTxRemoved.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/sequencer/test/integration/MempoolTxRemoved.test.ts b/packages/sequencer/test/integration/MempoolTxRemoved.test.ts index 8202f3086..8db728c25 100644 --- a/packages/sequencer/test/integration/MempoolTxRemoved.test.ts +++ b/packages/sequencer/test/integration/MempoolTxRemoved.test.ts @@ -19,7 +19,6 @@ describe("block production", () => { beforeAll(async () => { appChain.configurePartial({ Runtime: { - Balances: {}, Balance: {}, }, Protocol: { @@ -83,5 +82,5 @@ describe("block production", () => { const txs2 = await mempool.getTxs(); expect(txs2.length).toBe(1); - }, 60_000); + }, 300_000); }); From 087ca11144d76fa813030cd63c1445a0e3ce9a00 Mon Sep 17 00:00:00 2001 From: ejMina226 <118474890+ejMina226@users.noreply.github.com> Date: Thu, 13 Mar 2025 11:15:50 +0000 Subject: [PATCH 06/22] Deletemany to delete --- .../persistance/src/services/prisma/PrismaTransactionStorage.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/persistance/src/services/prisma/PrismaTransactionStorage.ts b/packages/persistance/src/services/prisma/PrismaTransactionStorage.ts index 1353dad43..3d0f08d3c 100644 --- a/packages/persistance/src/services/prisma/PrismaTransactionStorage.ts +++ b/packages/persistance/src/services/prisma/PrismaTransactionStorage.ts @@ -32,7 +32,7 @@ export class PrismaTransactionStorage implements TransactionStorage { public async removeTx(txHash: Field) { const { prismaClient } = this.connection; - await prismaClient.transaction.deleteMany({ + await prismaClient.transaction.delete({ where: { hash: txHash.toString(), }, From c38b83f5830a7f35522459ec7715613c82661b44 Mon Sep 17 00:00:00 2001 From: Raphael Panic Date: Tue, 4 Nov 2025 13:50:55 +0100 Subject: [PATCH 07/22] Added general tx dropping to block production --- packages/persistance/prisma/schema.prisma | 1 + .../src/services/prisma/PrismaBlockStorage.ts | 1 + .../prisma/PrismaTransactionStorage.ts | 20 +++--- .../prisma/mappers/TransactionMapper.ts | 2 + packages/sequencer/src/mempool/Mempool.ts | 2 + .../src/mempool/private/PrivateMempool.ts | 10 ++- .../sequencing/BlockProducerModule.ts | 5 ++ .../sequencing/BlockProductionService.ts | 24 ++++--- .../sequencing/TransactionExecutionService.ts | 62 +++++++++++++------ .../inmemory/InMemoryTransactionStorage.ts | 7 ++- packages/sequencer/src/storage/model/Block.ts | 1 + .../repositories/TransactionStorage.ts | 2 +- 12 files changed, 100 insertions(+), 37 deletions(-) diff --git a/packages/persistance/prisma/schema.prisma b/packages/persistance/prisma/schema.prisma index 17aa71d5f..d0ed1472e 100644 --- a/packages/persistance/prisma/schema.prisma +++ b/packages/persistance/prisma/schema.prisma @@ -62,6 +62,7 @@ model TransactionExecutionResult { status Boolean statusMessage String? events Json @db.Json + hooksStatus Boolean tx Transaction @relation(fields: [txHash], references: [hash]) txHash String @id diff --git a/packages/persistance/src/services/prisma/PrismaBlockStorage.ts b/packages/persistance/src/services/prisma/PrismaBlockStorage.ts index cf457a1cf..f3146642e 100644 --- a/packages/persistance/src/services/prisma/PrismaBlockStorage.ts +++ b/packages/persistance/src/services/prisma/PrismaBlockStorage.ts @@ -122,6 +122,7 @@ export class PrismaBlockStorage implements BlockQueue, BlockStorage { data: transactions.map((tx) => { return { status: tx.status, + hooksStatus: tx.hooksStatus, statusMessage: tx.statusMessage, txHash: tx.txHash, diff --git a/packages/persistance/src/services/prisma/PrismaTransactionStorage.ts b/packages/persistance/src/services/prisma/PrismaTransactionStorage.ts index a857eb8b2..6a9c8b6c8 100644 --- a/packages/persistance/src/services/prisma/PrismaTransactionStorage.ts +++ b/packages/persistance/src/services/prisma/PrismaTransactionStorage.ts @@ -36,14 +36,20 @@ export class PrismaTransactionStorage implements TransactionStorage { return txs.map((tx) => this.transactionMapper.mapIn(tx)); } - public async removeTx(txHash: Field) { - const { prismaClient } = this.connection; + public async removeTx(hashes: string[], type: "included" | "dropped") { + // In our schema, included txs are simply just linked with blocks, so we only + // need to delete if we drop a tx + if (type === "dropped") { + const { prismaClient } = this.connection; - await prismaClient.transaction.delete({ - where: { - hash: txHash.toString(), - }, - }); + await prismaClient.transaction.deleteMany({ + where: { + hash: { + in: hashes, + }, + }, + }); + } } public async pushUserTransaction(tx: PendingTransaction): Promise { diff --git a/packages/persistance/src/services/prisma/mappers/TransactionMapper.ts b/packages/persistance/src/services/prisma/mappers/TransactionMapper.ts index 1e242dace..e999e9bc2 100644 --- a/packages/persistance/src/services/prisma/mappers/TransactionMapper.ts +++ b/packages/persistance/src/services/prisma/mappers/TransactionMapper.ts @@ -66,6 +66,7 @@ export class TransactionExecutionResultMapper return { tx: this.transactionMapper.mapIn(input[1]), status: Bool(executionResult.status), + hooksStatus: Bool(executionResult.hooksStatus), statusMessage: executionResult.statusMessage ?? undefined, stateTransitions: this.stBatchMapper.mapIn( executionResult.stateTransitions @@ -80,6 +81,7 @@ export class TransactionExecutionResultMapper const tx = this.transactionMapper.mapOut(input.tx); const executionResult = { status: input.status.toBoolean(), + hooksStatus: input.hooksStatus.toBoolean(), statusMessage: input.statusMessage ?? null, stateTransitions: this.stBatchMapper.mapOut(input.stateTransitions), events: this.eventArrayMapper.mapOut(input.events), diff --git a/packages/sequencer/src/mempool/Mempool.ts b/packages/sequencer/src/mempool/Mempool.ts index be2ee1bfc..97401a973 100644 --- a/packages/sequencer/src/mempool/Mempool.ts +++ b/packages/sequencer/src/mempool/Mempool.ts @@ -18,4 +18,6 @@ export interface Mempool * Retrieve all transactions that are currently in the mempool */ getTxs: (limit?: number) => Promise; + + removeTxs: (included: string[], dropped: string[]) => Promise; } diff --git a/packages/sequencer/src/mempool/private/PrivateMempool.ts b/packages/sequencer/src/mempool/private/PrivateMempool.ts index 14655a742..a87a7d5c2 100644 --- a/packages/sequencer/src/mempool/private/PrivateMempool.ts +++ b/packages/sequencer/src/mempool/private/PrivateMempool.ts @@ -110,6 +110,11 @@ export class PrivateMempool return result?.result.afterNetworkState; } + public async removeTxs(included: string[], dropped: string[]) { + await this.transactionStorage.removeTx(included, "included"); + await this.transactionStorage.removeTx(dropped, "dropped"); + } + @trace("mempool.get_txs") public async getTxs(limit?: number): Promise { const txs = await this.transactionStorage.getPendingUserTransactions(); @@ -198,7 +203,10 @@ export class PrivateMempool }); if (removeTxWhen) { // eslint-disable-next-line no-await-in-loop - await this.transactionStorage.removeTx(tx.hash()); + await this.transactionStorage.removeTx( + [tx.hash().toString()], + "dropped" + ); log.trace( `Deleting tx ${tx.hash().toString()} from mempool because removeTransactionWhen condition is satisfied` ); diff --git a/packages/sequencer/src/protocol/production/sequencing/BlockProducerModule.ts b/packages/sequencer/src/protocol/production/sequencing/BlockProducerModule.ts index 85427d22a..6aed2255b 100644 --- a/packages/sequencer/src/protocol/production/sequencing/BlockProducerModule.ts +++ b/packages/sequencer/src/protocol/production/sequencing/BlockProducerModule.ts @@ -251,6 +251,11 @@ export class BlockProducerModule extends SequencerModule { height: block.height.toString(), } ); + + await this.mempool.removeTxs( + blockResult.included, + blockResult.skippedTxs + ); } this.productionInProgress = false; diff --git a/packages/sequencer/src/protocol/production/sequencing/BlockProductionService.ts b/packages/sequencer/src/protocol/production/sequencing/BlockProductionService.ts index c4a7e9c44..6a17b9210 100644 --- a/packages/sequencer/src/protocol/production/sequencing/BlockProductionService.ts +++ b/packages/sequencer/src/protocol/production/sequencing/BlockProductionService.ts @@ -93,6 +93,8 @@ export class BlockProductionService { | { block: Block; stateChanges: CachedStateService; + included: string[]; + skippedTxs: string[]; } | undefined > { @@ -124,13 +126,16 @@ export class BlockProductionService { UntypedStateTransition.fromStateTransition(transition) ); - const [newBlockState, executionResults] = - await this.transactionExecutionService.createExecutionTraces( - stateService, - transactions, - networkState, - blockState - ); + const { + blockState: newBlockState, + executionResults, + skipped, + } = await this.transactionExecutionService.createExecutionTraces( + stateService, + transactions, + networkState, + blockState + ); const previousBlockHash = lastResult.blockHash === 0n ? undefined : Field(lastResult.blockHash); @@ -165,12 +170,17 @@ export class BlockProductionService { const hash = Block.hash(block); + const included = block.transactions.map((tx) => tx.tx.hash().toString()); + const skippedTxs = skipped.map((tx) => tx.tx.hash().toString()); + return { block: { ...block, hash, }, stateChanges: stateService, + included, + skippedTxs, }; } } diff --git a/packages/sequencer/src/protocol/production/sequencing/TransactionExecutionService.ts b/packages/sequencer/src/protocol/production/sequencing/TransactionExecutionService.ts index 0e12cd7a1..58fd88a51 100644 --- a/packages/sequencer/src/protocol/production/sequencing/TransactionExecutionService.ts +++ b/packages/sequencer/src/protocol/production/sequencing/TransactionExecutionService.ts @@ -269,14 +269,14 @@ export class TransactionExecutionService { runSimulated ); - if (!result.status.toBoolean()) { - const error = new Error( - `Protocol hooks not executable: ${result.statusMessage ?? "unknown"}` - ); - log.debug("Protocol hook error stack trace:", result.stackTrace); - // Propagate stack trace from the assertion - throw error; - } + // if (!result.status.toBoolean()) { + // const error = new Error( + // `Protocol hooks not executable: ${result.statusMessage ?? "unknown"}` + // ); + // log.debug("Protocol hook error stack trace:", result.stackTrace); + // // Propagate stack trace from the assertion + // throw error; + // } traceLogSTs(`${hookName} STs:`, result.stateTransitions); @@ -285,9 +285,13 @@ export class TransactionExecutionService { private buildSTBatches( transitions: StateTransition[][], - runtimeStatus: Bool + { + runtime: runtimeStatus, + hooks: hooksStatus, + }: { runtime: boolean; hooks: boolean } ): StateTransitionBatch[] { - const statuses = [true, runtimeStatus.toBoolean(), false]; + // TODO Why is the last one false by default? + const statuses = [hooksStatus, runtimeStatus && hooksStatus, false]; const reducedTransitions = transitions.map((batch) => reduceStateTransitions(batch).map((transition) => UntypedStateTransition.fromStateTransition(transition) @@ -322,15 +326,20 @@ export class TransactionExecutionService { transactions: PendingTransaction[], networkState: NetworkState, state: BlockTrackers - ): Promise<[BlockTrackers, TransactionExecutionResult[]]> { + ): Promise<{ + blockState: BlockTrackers; + executionResults: TransactionExecutionResult[]; + skipped: TransactionExecutionResult[]; + }> { let blockState = state; const executionResults: TransactionExecutionResult[] = []; + const skipped: TransactionExecutionResult[] = []; const networkStateHash = networkState.hash(); for (const tx of transactions) { try { - const newState = this.addTransactionToBlockProverState(state, tx); + const newState = this.addTransactionToBlockProverState(blockState, tx); // Create execution trace const executionTrace = @@ -343,10 +352,19 @@ export class TransactionExecutionService { newState ); - blockState = newState; - - // Push result to results and transaction onto bundle-hash - executionResults.push(executionTrace); + // If the hooks fail AND the tx is not a message (in which case we + // have to still execute it), we skip this tx and don't add it to the block + if ( + !executionTrace.hooksStatus.toBoolean() && + !executionTrace.tx.isMessage + ) { + skipped.push(executionTrace); + } else { + blockState = newState; + + // Push result to results and transaction onto bundle-hash + executionResults.push(executionTrace); + } } catch (error) { if (error instanceof Error) { log.error("Error in inclusion of tx, skipping", error); @@ -354,7 +372,7 @@ export class TransactionExecutionService { } } - return [blockState, executionResults]; + return { blockState, executionResults, skipped }; } @trace("block.transaction", ([, tx, { networkState }]) => ({ @@ -463,7 +481,12 @@ export class TransactionExecutionService { afterTxHookResult.stateTransitions ); - await recordingStateService.mergeIntoParent(); + const txHooksValid = + beforeTxHookResult.status.toBoolean() && + afterTxHookResult.status.toBoolean(); + if (txHooksValid) { + await recordingStateService.mergeIntoParent(); + } // Reset global stateservice this.stateServiceProvider.popCurrentStateService(); @@ -479,11 +502,12 @@ export class TransactionExecutionService { runtimeResult.stateTransitions, afterTxHookResult.stateTransitions, ], - runtimeResult.status + { runtime: runtimeResult.status.toBoolean(), hooks: txHooksValid } ); return { tx, + hooksStatus: Bool(txHooksValid), status: runtimeResult.status, statusMessage: runtimeResult.statusMessage, diff --git a/packages/sequencer/src/storage/inmemory/InMemoryTransactionStorage.ts b/packages/sequencer/src/storage/inmemory/InMemoryTransactionStorage.ts index 394d2067a..a7591302f 100644 --- a/packages/sequencer/src/storage/inmemory/InMemoryTransactionStorage.ts +++ b/packages/sequencer/src/storage/inmemory/InMemoryTransactionStorage.ts @@ -19,8 +19,11 @@ export class InMemoryTransactionStorage implements TransactionStorage { @inject("BatchStorage") private readonly batchStorage: InMemoryBatchStorage ) {} - public async removeTx(txHash: Field) { - this.queue = this.queue.filter((tx) => tx.hash() !== txHash); + public async removeTx(hashes: string[]) { + this.queue = this.queue.filter((tx) => { + const hash = tx.hash().toString(); + return !hashes.includes(hash); + }); } public async getPendingUserTransactions(): Promise { diff --git a/packages/sequencer/src/storage/model/Block.ts b/packages/sequencer/src/storage/model/Block.ts index b16e8d748..da2e44802 100644 --- a/packages/sequencer/src/storage/model/Block.ts +++ b/packages/sequencer/src/storage/model/Block.ts @@ -19,6 +19,7 @@ export interface TransactionExecutionResult { tx: PendingTransaction; stateTransitions: StateTransitionBatch[]; status: Bool; + hooksStatus: Bool; statusMessage?: string; events: { eventName: string; diff --git a/packages/sequencer/src/storage/repositories/TransactionStorage.ts b/packages/sequencer/src/storage/repositories/TransactionStorage.ts index 204194d65..05961bebb 100644 --- a/packages/sequencer/src/storage/repositories/TransactionStorage.ts +++ b/packages/sequencer/src/storage/repositories/TransactionStorage.ts @@ -7,7 +7,7 @@ export interface TransactionStorage { getPendingUserTransactions: () => Promise; - removeTx: (txHash: Field) => Promise; + removeTx: (txHashes: string[], type: "included" | "dropped") => Promise; /** * Finds a transaction by its hash. From 11ac9fc61e189794b86a2e83eb3eb5f7cc83428d Mon Sep 17 00:00:00 2001 From: Raphael Panic Date: Tue, 4 Nov 2025 23:59:55 +0100 Subject: [PATCH 08/22] Added consideration for shouldRemove hook when dropping txs --- .../prisma/PrismaTransactionStorage.ts | 1 - .../protocol/src/hooks/AccountStateHook.ts | 2 +- .../sequencing/BlockProducerModule.ts | 9 ++- .../sequencing/BlockProductionService.ts | 34 +++++----- .../sequencing/TransactionExecutionService.ts | 68 +++++++++++++++---- .../repositories/TransactionStorage.ts | 2 - 6 files changed, 78 insertions(+), 38 deletions(-) diff --git a/packages/persistance/src/services/prisma/PrismaTransactionStorage.ts b/packages/persistance/src/services/prisma/PrismaTransactionStorage.ts index 6a9c8b6c8..d4544a7c4 100644 --- a/packages/persistance/src/services/prisma/PrismaTransactionStorage.ts +++ b/packages/persistance/src/services/prisma/PrismaTransactionStorage.ts @@ -5,7 +5,6 @@ import { Tracer, TransactionStorage, } from "@proto-kit/sequencer"; -import { Field } from "o1js"; import type { PrismaConnection } from "../../PrismaDatabaseConnection"; diff --git a/packages/protocol/src/hooks/AccountStateHook.ts b/packages/protocol/src/hooks/AccountStateHook.ts index 67eadeb63..6e05f4762 100644 --- a/packages/protocol/src/hooks/AccountStateHook.ts +++ b/packages/protocol/src/hooks/AccountStateHook.ts @@ -58,7 +58,7 @@ export class AccountStateHook extends ProvableTransactionHook { // Under these conditions we want the tx removed from the mempool. public async removeTransactionWhen({ transaction, - }: BeforeTransactionHookArguments) { + }: BeforeTransactionHookArguments): Promise { const sender = transaction.sender.value; const aso = await this.accountState.get(sender); diff --git a/packages/sequencer/src/protocol/production/sequencing/BlockProducerModule.ts b/packages/sequencer/src/protocol/production/sequencing/BlockProducerModule.ts index 6aed2255b..8f035c338 100644 --- a/packages/sequencer/src/protocol/production/sequencing/BlockProducerModule.ts +++ b/packages/sequencer/src/protocol/production/sequencing/BlockProducerModule.ts @@ -252,9 +252,14 @@ export class BlockProducerModule extends SequencerModule { } ); + // Remove included or dropped txs, leave skipped ones alone await this.mempool.removeTxs( - blockResult.included, - blockResult.skippedTxs + blockResult.includedTxs + .filter((x) => x.type === "included") + .map((x) => x.hash), + blockResult.includedTxs + .filter((x) => x.type === "shouldRemove") + .map((x) => x.hash) ); } diff --git a/packages/sequencer/src/protocol/production/sequencing/BlockProductionService.ts b/packages/sequencer/src/protocol/production/sequencing/BlockProductionService.ts index 6a17b9210..ddafd630b 100644 --- a/packages/sequencer/src/protocol/production/sequencing/BlockProductionService.ts +++ b/packages/sequencer/src/protocol/production/sequencing/BlockProductionService.ts @@ -93,8 +93,10 @@ export class BlockProductionService { | { block: Block; stateChanges: CachedStateService; - included: string[]; - skippedTxs: string[]; + includedTxs: { + hash: string; + type: "included" | "skipped" | "shouldRemove"; + }[]; } | undefined > { @@ -126,16 +128,13 @@ export class BlockProductionService { UntypedStateTransition.fromStateTransition(transition) ); - const { - blockState: newBlockState, - executionResults, - skipped, - } = await this.transactionExecutionService.createExecutionTraces( - stateService, - transactions, - networkState, - blockState - ); + const { blockState: newBlockState, executionResults } = + await this.transactionExecutionService.createExecutionTraces( + stateService, + transactions, + networkState, + blockState + ); const previousBlockHash = lastResult.blockHash === 0n ? undefined : Field(lastResult.blockHash); @@ -148,7 +147,7 @@ export class BlockProductionService { } const block: Omit = { - transactions: executionResults, + transactions: executionResults.map((x) => x.result), transactionsHash: newBlockState.transactionList.commitment, fromEternalTransactionsHash: lastBlock.toEternalTransactionsHash, toEternalTransactionsHash: @@ -170,8 +169,10 @@ export class BlockProductionService { const hash = Block.hash(block); - const included = block.transactions.map((tx) => tx.tx.hash().toString()); - const skippedTxs = skipped.map((tx) => tx.tx.hash().toString()); + const includedTxs = executionResults.map((x) => ({ + hash: x.result.tx.hash().toString(), + type: x.status, + })); return { block: { @@ -179,8 +180,7 @@ export class BlockProductionService { hash, }, stateChanges: stateService, - included, - skippedTxs, + includedTxs, }; } } diff --git a/packages/sequencer/src/protocol/production/sequencing/TransactionExecutionService.ts b/packages/sequencer/src/protocol/production/sequencing/TransactionExecutionService.ts index 58fd88a51..792a6a6f7 100644 --- a/packages/sequencer/src/protocol/production/sequencing/TransactionExecutionService.ts +++ b/packages/sequencer/src/protocol/production/sequencing/TransactionExecutionService.ts @@ -204,6 +204,8 @@ export class TransactionExecutionService { private readonly blockProver: BlockProverProgrammable; + private readonly txHooks: ProvableTransactionHook[]; + public constructor( @inject("Runtime") private readonly runtime: Runtime, @inject("Protocol") @@ -219,6 +221,11 @@ export class TransactionExecutionService { ); // eslint-disable-next-line @typescript-eslint/consistent-type-assertions this.blockProver = (protocol.blockProver as BlockProver).zkProgrammable; + + this.txHooks = + protocol.dependencyContainer.resolveAll( + "ProvableTransactionHook" + ); } private async executeRuntimeMethod( @@ -328,12 +335,16 @@ export class TransactionExecutionService { state: BlockTrackers ): Promise<{ blockState: BlockTrackers; - executionResults: TransactionExecutionResult[]; - skipped: TransactionExecutionResult[]; + executionResults: { + result: TransactionExecutionResult; + status: "included" | "skipped" | "shouldRemove"; + }[]; }> { let blockState = state; - const executionResults: TransactionExecutionResult[] = []; - const skipped: TransactionExecutionResult[] = []; + const executionResults: { + result: TransactionExecutionResult; + status: "included" | "skipped" | "shouldRemove"; + }[] = []; const networkStateHash = networkState.hash(); @@ -342,7 +353,7 @@ export class TransactionExecutionService { const newState = this.addTransactionToBlockProverState(blockState, tx); // Create execution trace - const executionTrace = + const { result: executionTrace, shouldRemove } = // eslint-disable-next-line no-await-in-loop await this.createExecutionTrace( asyncStateService, @@ -358,12 +369,15 @@ export class TransactionExecutionService { !executionTrace.hooksStatus.toBoolean() && !executionTrace.tx.isMessage ) { - skipped.push(executionTrace); + executionResults.push({ + result: executionTrace, + status: shouldRemove ? "shouldRemove" : "skipped", + }); } else { blockState = newState; // Push result to results and transaction onto bundle-hash - executionResults.push(executionTrace); + executionResults.push({ result: executionTrace, status: "included" }); } } catch (error) { if (error instanceof Error) { @@ -372,7 +386,21 @@ export class TransactionExecutionService { } } - return { blockState, executionResults, skipped }; + return { blockState, executionResults }; + } + + private async shouldRemove( + state: CachedStateService, + args: BeforeTransactionHookArguments + ) { + this.stateServiceProvider.setCurrentStateService(state); + + const returnValues = await mapSequential(this.transactionHooks, (hook) => + hook.removeTransactionWhen(args) + ); + + this.stateServiceProvider.popCurrentStateService(); + return returnValues.some((x) => x); } @trace("block.transaction", ([, tx, { networkState }]) => ({ @@ -389,7 +417,7 @@ export class TransactionExecutionService { }: { networkState: NetworkState; hash: Field }, state: BlockTrackers, newState: BlockTrackers - ): Promise { + ): Promise<{ result: TransactionExecutionResult; shouldRemove: boolean }> { // TODO Use RecordingStateService -> async asProver needed const recordingStateService = new CachedStateService(asyncStateService); @@ -484,8 +512,15 @@ export class TransactionExecutionService { const txHooksValid = beforeTxHookResult.status.toBoolean() && afterTxHookResult.status.toBoolean(); + let shouldRemove = false; if (txHooksValid) { await recordingStateService.mergeIntoParent(); + } else { + // Execute removeWhen to determine whether it should be dropped + shouldRemove = await this.shouldRemove( + asyncStateService, + beforeTxArguments + ); } // Reset global stateservice @@ -506,13 +541,16 @@ export class TransactionExecutionService { ); return { - tx, - hooksStatus: Bool(txHooksValid), - status: runtimeResult.status, - statusMessage: runtimeResult.statusMessage, + result: { + tx, + hooksStatus: Bool(txHooksValid), + status: runtimeResult.status, + statusMessage: runtimeResult.statusMessage, - stateTransitions, - events: beforeHookEvents.concat(runtimeResultEvents, afterHookEvents), + stateTransitions, + events: beforeHookEvents.concat(runtimeResultEvents, afterHookEvents), + }, + shouldRemove, }; } } diff --git a/packages/sequencer/src/storage/repositories/TransactionStorage.ts b/packages/sequencer/src/storage/repositories/TransactionStorage.ts index 05961bebb..ef353ad08 100644 --- a/packages/sequencer/src/storage/repositories/TransactionStorage.ts +++ b/packages/sequencer/src/storage/repositories/TransactionStorage.ts @@ -1,5 +1,3 @@ -import { Field } from "o1js"; - import { PendingTransaction } from "../../mempool/PendingTransaction"; export interface TransactionStorage { From 80f0df4ada6b9a6f7a4f4ece7ffa19dbf8cefc5d Mon Sep 17 00:00:00 2001 From: Raphael Panic Date: Mon, 3 Nov 2025 16:38:42 +0100 Subject: [PATCH 09/22] Added BlockProver handling of message + tx hooks --- .../protocol/src/prover/block/BlockProver.ts | 47 ++++++++++++++----- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/packages/protocol/src/prover/block/BlockProver.ts b/packages/protocol/src/prover/block/BlockProver.ts index 6a7e89cb3..0e7b2a362 100644 --- a/packages/protocol/src/prover/block/BlockProver.ts +++ b/packages/protocol/src/prover/block/BlockProver.ts @@ -171,7 +171,8 @@ export class BlockProverProgrammable extends ZkProgrammable< // Apply beforeTransaction hook state transitions const beforeBatch = await this.executeTransactionHooks( async (module, args) => await module.beforeTransaction(args), - beforeTxHookArguments + beforeTxHookArguments, + isMessage ); state = this.addTransactionToBundle( @@ -200,7 +201,8 @@ export class BlockProverProgrammable extends ZkProgrammable< const afterBatch = await this.executeTransactionHooks( async (module, args) => await module.afterTransaction(args), - afterTxHookArguments + afterTxHookArguments, + isMessage ); state.pendingSTBatches.push(afterBatch); @@ -266,20 +268,31 @@ export class BlockProverProgrammable extends ZkProgrammable< T extends BeforeTransactionHookArguments | AfterTransactionHookArguments, >( hook: (module: ProvableTransactionHook, args: T) => Promise, - hookArguments: T + hookArguments: T, + isMessage: Bool ) { - const { batch } = await this.executeHooks(hookArguments, async () => { - for (const module of this.transactionHooks) { - // eslint-disable-next-line no-await-in-loop - await hook(module, hookArguments); - } - }); + const { batch, rawStatus } = await this.executeHooks( + hookArguments, + async () => { + for (const module of this.transactionHooks) { + // eslint-disable-next-line no-await-in-loop + await hook(module, hookArguments); + } + }, + isMessage + ); + + // This is going to set applied to false in case the hook fails + // (that's only possible for messages though as others are hard-asserted) + batch.applied = rawStatus; + return batch; } private async executeHooks( contextArguments: RuntimeMethodExecutionData, - method: () => Promise + method: () => Promise, + isMessage: Bool | undefined = undefined ) { const executionContext = container.resolve(RuntimeMethodExecutionContext); executionContext.clear(); @@ -297,11 +310,23 @@ export class BlockProverProgrammable extends ZkProgrammable< const { stateTransitions, status, statusMessage } = executionContext.current().result; - status.assertTrue(`Transaction hook call failed: ${statusMessage ?? "-"}`); + // See https://github.com/proto-kit/framework/issues/321 for why we do this here + if (isMessage !== undefined) { + // isMessage is defined for all tx hooks + status + .or(isMessage) + .assertTrue( + `Transaction hook call failed for non-message tx: ${statusMessage ?? "-"}` + ); + } else { + // isMessage is undefined for all block hooks + status.assertTrue(`Block hook call failed: ${statusMessage ?? "-"}`); + } return { batch: this.constructBatch(stateTransitions, Bool(true)), result, + rawStatus: status, }; } From 1df5f56f54e29217d689c3c5a710bb9e69446902 Mon Sep 17 00:00:00 2001 From: Raphael Panic Date: Wed, 5 Nov 2025 15:17:40 +0100 Subject: [PATCH 10/22] Fixed test --- packages/sequencer/test/integration/MempoolTxRemoved.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/sequencer/test/integration/MempoolTxRemoved.test.ts b/packages/sequencer/test/integration/MempoolTxRemoved.test.ts index 8db728c25..c13c423f9 100644 --- a/packages/sequencer/test/integration/MempoolTxRemoved.test.ts +++ b/packages/sequencer/test/integration/MempoolTxRemoved.test.ts @@ -20,6 +20,7 @@ describe("block production", () => { appChain.configurePartial({ Runtime: { Balance: {}, + Balances: {}, }, Protocol: { ...appChain.config.Protocol!, From 5fff36de4ef5e9bfae99fd6ba156a234a397da19 Mon Sep 17 00:00:00 2001 From: Raphael Panic Date: Wed, 5 Nov 2025 15:18:11 +0100 Subject: [PATCH 11/22] Fixed wrong handling of tx removal for mempool simulation mode --- .../src/mempool/private/PrivateMempool.ts | 39 ++++++++++--------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/packages/sequencer/src/mempool/private/PrivateMempool.ts b/packages/sequencer/src/mempool/private/PrivateMempool.ts index a87a7d5c2..693c5831b 100644 --- a/packages/sequencer/src/mempool/private/PrivateMempool.ts +++ b/packages/sequencer/src/mempool/private/PrivateMempool.ts @@ -194,25 +194,6 @@ export class PrivateMempool const signedTransaction = tx.toProtocolTransaction(); - // eslint-disable-next-line no-await-in-loop - const removeTxWhen = await this.accountStateHook.removeTransactionWhen({ - networkState: networkState, - transaction: signedTransaction.transaction, - signature: signedTransaction.signature, - prover: proverState, - }); - if (removeTxWhen) { - // eslint-disable-next-line no-await-in-loop - await this.transactionStorage.removeTx( - [tx.hash().toString()], - "dropped" - ); - log.trace( - `Deleting tx ${tx.hash().toString()} from mempool because removeTransactionWhen condition is satisfied` - ); - // eslint-disable-next-line no-continue - continue; - } // eslint-disable-next-line no-await-in-loop await this.accountStateHook.beforeTransaction({ networkState: networkState, @@ -243,6 +224,26 @@ export class PrivateMempool queue = queue.filter(distinctByPredicate((a, b) => a === b)); } } else { + // eslint-disable-next-line no-await-in-loop + const removeTxWhen = await this.accountStateHook.removeTransactionWhen({ + networkState: networkState, + transaction: signedTransaction.transaction, + signature: signedTransaction.signature, + prover: proverState, + }); + if (removeTxWhen) { + // eslint-disable-next-line no-await-in-loop + await this.transactionStorage.removeTx( + [tx.hash().toString()], + "dropped" + ); + log.trace( + `Deleting tx ${tx.hash().toString()} from mempool because removeTransactionWhen condition is satisfied` + ); + // eslint-disable-next-line no-continue + continue; + } + log.trace( `Skipped tx ${tx.hash().toString()} because ${statusMessage}` ); From adb0bd38d646dac54e139d7ed25c9a9a2d38da42 Mon Sep 17 00:00:00 2001 From: Raphael Panic Date: Wed, 5 Nov 2025 15:33:56 +0100 Subject: [PATCH 12/22] Increased test timeout for mempoolremovetx test --- packages/sequencer/test/integration/MempoolTxRemoved.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sequencer/test/integration/MempoolTxRemoved.test.ts b/packages/sequencer/test/integration/MempoolTxRemoved.test.ts index c13c423f9..14d14bec9 100644 --- a/packages/sequencer/test/integration/MempoolTxRemoved.test.ts +++ b/packages/sequencer/test/integration/MempoolTxRemoved.test.ts @@ -33,7 +33,7 @@ describe("block production", () => { // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment mempool = sequencer.resolve("Mempool"); - }); + }, 60_000); it("check tx is removed", async () => { await mempool.add( From 0c832746f33e20907a6b3ced3515492e912abaf0 Mon Sep 17 00:00:00 2001 From: Raphael Panic Date: Wed, 5 Nov 2025 20:05:13 +0100 Subject: [PATCH 13/22] Fixed status message propagation --- packages/protocol/src/state/assert/assert.ts | 2 +- packages/sdk/test/fees-failures.test.ts | 7 +++++-- .../sequencing/TransactionExecutionService.ts | 21 ++++++++++--------- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/packages/protocol/src/state/assert/assert.ts b/packages/protocol/src/state/assert/assert.ts index f15b554d3..1af3e494f 100644 --- a/packages/protocol/src/state/assert/assert.ts +++ b/packages/protocol/src/state/assert/assert.ts @@ -19,7 +19,7 @@ export function assert(condition: Bool, message?: string | (() => string)) { const status = condition.and(previousStatus); Provable.asProver(() => { - if (!condition.toBoolean()) { + if (!condition.toBoolean() && previousStatus.toBoolean()) { const messageString = message !== undefined && typeof message === "function" ? message() diff --git a/packages/sdk/test/fees-failures.test.ts b/packages/sdk/test/fees-failures.test.ts index 1e3d46b3d..a6df758d8 100644 --- a/packages/sdk/test/fees-failures.test.ts +++ b/packages/sdk/test/fees-failures.test.ts @@ -60,6 +60,10 @@ describe("fee errors due to limited funds in sender accounts", () => { appChain.setSigner(senderKey); }); + afterAll(async () => { + await appChain.close(); + }); + it("should allow a free faucet transaction", async () => { expect.assertions(2); @@ -105,8 +109,7 @@ describe("fee errors due to limited funds in sender accounts", () => { await appChain.produceBlock(); expect(logSpy).toHaveBeenCalledWith( - "Error in inclusion of tx, skipping", - Error("Protocol hooks not executable: From balance is insufficient") + "Error in inclusion of tx, skipping: Protocol hooks not executable: From balance is insufficient" ); const balance = await appChain.query.runtime.Balances.balances.get( diff --git a/packages/sequencer/src/protocol/production/sequencing/TransactionExecutionService.ts b/packages/sequencer/src/protocol/production/sequencing/TransactionExecutionService.ts index 792a6a6f7..33b9e9f03 100644 --- a/packages/sequencer/src/protocol/production/sequencing/TransactionExecutionService.ts +++ b/packages/sequencer/src/protocol/production/sequencing/TransactionExecutionService.ts @@ -276,15 +276,6 @@ export class TransactionExecutionService { runSimulated ); - // if (!result.status.toBoolean()) { - // const error = new Error( - // `Protocol hooks not executable: ${result.statusMessage ?? "unknown"}` - // ); - // log.debug("Protocol hook error stack trace:", result.stackTrace); - // // Propagate stack trace from the assertion - // throw error; - // } - traceLogSTs(`${hookName} STs:`, result.stateTransitions); return result; @@ -369,6 +360,12 @@ export class TransactionExecutionService { !executionTrace.hooksStatus.toBoolean() && !executionTrace.tx.isMessage ) { + const actionMessage = shouldRemove + ? "removing as to removeWhen hooks" + : "skipping"; + log.error( + `Error in inclusion of tx, ${actionMessage}: Protocol hooks not executable: ${executionTrace.statusMessage ?? "unknown reason"}` + ); executionResults.push({ result: executionTrace, status: shouldRemove ? "shouldRemove" : "skipped", @@ -380,6 +377,7 @@ export class TransactionExecutionService { executionResults.push({ result: executionTrace, status: "included" }); } } catch (error) { + console.log("error", error); if (error instanceof Error) { log.error("Error in inclusion of tx, skipping", error); } @@ -545,7 +543,10 @@ export class TransactionExecutionService { tx, hooksStatus: Bool(txHooksValid), status: runtimeResult.status, - statusMessage: runtimeResult.statusMessage, + statusMessage: + beforeTxHookResult.statusMessage ?? + afterTxHookResult.statusMessage ?? + runtimeResult.statusMessage, stateTransitions, events: beforeHookEvents.concat(runtimeResultEvents, afterHookEvents), From 0419fcc3f860e49975c9576237f2c17541bfa5ea Mon Sep 17 00:00:00 2001 From: Raphael Panic Date: Wed, 5 Nov 2025 20:37:22 +0100 Subject: [PATCH 14/22] Fixed linting, refactored typing --- .../sequencing/BlockProductionService.ts | 35 +++++++++++++++---- .../sequencing/TransactionExecutionService.ts | 25 +++++++------ 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/packages/sequencer/src/protocol/production/sequencing/BlockProductionService.ts b/packages/sequencer/src/protocol/production/sequencing/BlockProductionService.ts index ddafd630b..18010fa28 100644 --- a/packages/sequencer/src/protocol/production/sequencing/BlockProductionService.ts +++ b/packages/sequencer/src/protocol/production/sequencing/BlockProductionService.ts @@ -15,8 +15,13 @@ import { } from "@proto-kit/protocol"; import { Field } from "o1js"; import { log } from "@proto-kit/common"; +import { match } from "ts-pattern"; -import { Block, BlockWithResult } from "../../../storage/model/Block"; +import { + Block, + BlockWithResult, + TransactionExecutionResult, +} from "../../../storage/model/Block"; import { CachedStateService } from "../../../state/state/CachedStateService"; import { PendingTransaction } from "../../../mempool/PendingTransaction"; import { AsyncStateService } from "../../../state/async/AsyncStateService"; @@ -27,9 +32,16 @@ import { trace } from "../../../logging/trace"; import { BlockTrackers, executeWithExecutionContext, + TransactionExecutionResultStatus, TransactionExecutionService, } from "./TransactionExecutionService"; +function isIncludedTxs( + x: TransactionExecutionResultStatus +): x is { status: "included"; result: TransactionExecutionResult } { + return x.status === "included"; +} + @injectable() @scoped(Lifecycle.ContainerScoped) export class BlockProductionService { @@ -146,8 +158,12 @@ export class BlockProductionService { return undefined; } + const includedTransactions = executionResults + .filter(isIncludedTxs) + .map((x) => x.result); + const block: Omit = { - transactions: executionResults.map((x) => x.result), + transactions: includedTransactions, transactionsHash: newBlockState.transactionList.commitment, fromEternalTransactionsHash: lastBlock.toEternalTransactionsHash, toEternalTransactionsHash: @@ -169,10 +185,17 @@ export class BlockProductionService { const hash = Block.hash(block); - const includedTxs = executionResults.map((x) => ({ - hash: x.result.tx.hash().toString(), - type: x.status, - })); + const includedTxs = executionResults.map((x) => { + const txHash = match(x) + .with({ status: "included" }, ({ result }) => result.tx) + .otherwise(({ tx }) => tx) + .hash() + .toString(); + return { + hash: txHash, + type: x.status, + }; + }); return { block: { diff --git a/packages/sequencer/src/protocol/production/sequencing/TransactionExecutionService.ts b/packages/sequencer/src/protocol/production/sequencing/TransactionExecutionService.ts index 33b9e9f03..753cfbb8f 100644 --- a/packages/sequencer/src/protocol/production/sequencing/TransactionExecutionService.ts +++ b/packages/sequencer/src/protocol/production/sequencing/TransactionExecutionService.ts @@ -197,6 +197,14 @@ function traceLogSTs(msg: string, stateTransitions: StateTransition[]) { ); } +export type TransactionExecutionResultStatus = + | { + result: TransactionExecutionResult; + status: "included"; + } + | { tx: PendingTransaction; status: "skipped" } + | { tx: PendingTransaction; status: "shouldRemove" }; + @injectable() @scoped(Lifecycle.ContainerScoped) export class TransactionExecutionService { @@ -319,6 +327,7 @@ export class TransactionExecutionService { ); } + // eslint-disable-next-line sonarjs/cognitive-complexity public async createExecutionTraces( asyncStateService: CachedStateService, transactions: PendingTransaction[], @@ -326,16 +335,10 @@ export class TransactionExecutionService { state: BlockTrackers ): Promise<{ blockState: BlockTrackers; - executionResults: { - result: TransactionExecutionResult; - status: "included" | "skipped" | "shouldRemove"; - }[]; + executionResults: TransactionExecutionResultStatus[]; }> { let blockState = state; - const executionResults: { - result: TransactionExecutionResult; - status: "included" | "skipped" | "shouldRemove"; - }[] = []; + const executionResults: TransactionExecutionResultStatus[] = []; const networkStateHash = networkState.hash(); @@ -367,7 +370,7 @@ export class TransactionExecutionService { `Error in inclusion of tx, ${actionMessage}: Protocol hooks not executable: ${executionTrace.statusMessage ?? "unknown reason"}` ); executionResults.push({ - result: executionTrace, + tx, status: shouldRemove ? "shouldRemove" : "skipped", }); } else { @@ -377,9 +380,9 @@ export class TransactionExecutionService { executionResults.push({ result: executionTrace, status: "included" }); } } catch (error) { - console.log("error", error); if (error instanceof Error) { - log.error("Error in inclusion of tx, skipping", error); + log.error("Error in inclusion of tx, dropping", error); + executionResults.push({ tx, status: "shouldRemove" }); } } } From 6ad940147063f05eb687544eec933a80c84fdc3c Mon Sep 17 00:00:00 2001 From: Raphael Panic Date: Wed, 5 Nov 2025 21:47:59 +0100 Subject: [PATCH 15/22] Added prisma column --- .../migrations/20251105204729_hooksstatus/migration.sql | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 packages/persistance/prisma/migrations/20251105204729_hooksstatus/migration.sql diff --git a/packages/persistance/prisma/migrations/20251105204729_hooksstatus/migration.sql b/packages/persistance/prisma/migrations/20251105204729_hooksstatus/migration.sql new file mode 100644 index 000000000..9837ff667 --- /dev/null +++ b/packages/persistance/prisma/migrations/20251105204729_hooksstatus/migration.sql @@ -0,0 +1,8 @@ +/* + Warnings: + + - Added the required column `hooksStatus` to the `TransactionExecutionResult` table without a default value. This is not possible if the table is not empty. + +*/ +-- AlterTable +ALTER TABLE "TransactionExecutionResult" ADD COLUMN "hooksStatus" BOOLEAN NOT NULL; From b29ac0ce92970c3b953100ab60dc4392356a4940 Mon Sep 17 00:00:00 2001 From: Raphael Panic Date: Wed, 5 Nov 2025 21:55:17 +0100 Subject: [PATCH 16/22] Added test for block pipeline mempool removal --- .../src/mempool/private/PrivateMempool.ts | 2 +- .../test/integration/MempoolTxRemoved.test.ts | 201 +++++++++++++----- 2 files changed, 148 insertions(+), 55 deletions(-) diff --git a/packages/sequencer/src/mempool/private/PrivateMempool.ts b/packages/sequencer/src/mempool/private/PrivateMempool.ts index 693c5831b..9e363d818 100644 --- a/packages/sequencer/src/mempool/private/PrivateMempool.ts +++ b/packages/sequencer/src/mempool/private/PrivateMempool.ts @@ -124,7 +124,7 @@ export class PrivateMempool const networkState = (await this.getStagedNetworkState()) ?? NetworkState.empty(); - const validationEnabled = this.config.validationEnabled ?? true; + const validationEnabled = this.config.validationEnabled ?? false; const sortedTxs = validationEnabled ? await this.checkTxValid( txs, diff --git a/packages/sequencer/test/integration/MempoolTxRemoved.test.ts b/packages/sequencer/test/integration/MempoolTxRemoved.test.ts index 14d14bec9..de598cde9 100644 --- a/packages/sequencer/test/integration/MempoolTxRemoved.test.ts +++ b/packages/sequencer/test/integration/MempoolTxRemoved.test.ts @@ -3,20 +3,25 @@ import { Runtime } from "@proto-kit/module"; import { TestingAppChain } from "@proto-kit/sdk"; import { Bool, PrivateKey, UInt64 } from "o1js"; import "reflect-metadata"; +import { expectDefined, log } from "@proto-kit/common"; +import { afterEach, beforeEach, describe, expect } from "@jest/globals"; import { PrivateMempool, Sequencer } from "../../src"; import { createTransaction } from "./utils"; import { Balance } from "./mocks/Balance"; -describe("block production", () => { +describe("mempool removal mechanism", () => { const senderKey = PrivateKey.random(); - const appChain = TestingAppChain.fromRuntime({ Balance }); + let appChain: Awaited>; let mempool: PrivateMempool; let runtime: Runtime<{ Balances: typeof Balances; Balance: typeof Balance }>; let sequencer: Sequencer; - beforeAll(async () => { + const createAppChain = async (validationEnabled: boolean) => { + // eslint-disable-next-line @typescript-eslint/no-shadow + const appChain = TestingAppChain.fromRuntime({ Balance }); + appChain.configurePartial({ Runtime: { Balance: {}, @@ -25,6 +30,10 @@ describe("block production", () => { Protocol: { ...appChain.config.Protocol!, }, + Sequencer: { + ...appChain.config.Sequencer, + Mempool: { validationEnabled }, + }, }); await appChain.start(); @@ -33,55 +42,139 @@ describe("block production", () => { // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment mempool = sequencer.resolve("Mempool"); - }, 60_000); - - it("check tx is removed", async () => { - await mempool.add( - createTransaction({ - runtime, - method: ["Balance", "setBalanceIf"], - privateKey: senderKey, - args: [senderKey.toPublicKey(), UInt64.from(100), Bool(true)], - nonce: 0, - }) - ); - - await mempool.add( - createTransaction({ - runtime, - method: ["Balance", "setBalanceIf"], - privateKey: senderKey, - args: [senderKey.toPublicKey(), UInt64.from(100), Bool(true)], - nonce: 1, - }) - ); - - const txs = await mempool.getTxs(); - expect(txs.length).toBe(2); - - await appChain.produceBlock(); - - await mempool.add( - createTransaction({ - runtime, - method: ["Balance", "setBalanceIf"], - privateKey: senderKey, - args: [senderKey.toPublicKey(), UInt64.from(100), Bool(true)], - nonce: 2, - }) - ); - - await mempool.add( - createTransaction({ - runtime, - method: ["Balance", "setBalanceIf"], - privateKey: senderKey, - args: [senderKey.toPublicKey(), UInt64.from(100), Bool(true)], - nonce: 0, - }) - ); - - const txs2 = await mempool.getTxs(); - expect(txs2.length).toBe(1); - }, 300_000); + + return appChain; + }; + + afterEach(async () => { + await appChain.close(); + }); + + describe("block pipeline reaction", () => { + beforeEach(async () => { + appChain = await createAppChain(false); + }, 60_000); + + it("check only one is included, other is skipped", async () => { + log.setLevel("trace"); + + await mempool.add( + createTransaction({ + runtime, + method: ["Balance", "setBalanceIf"], + privateKey: senderKey, + args: [senderKey.toPublicKey(), UInt64.from(100), Bool(true)], + nonce: 0, + }) + ); + + await mempool.add( + createTransaction({ + runtime, + method: ["Balance", "setBalanceIf"], + privateKey: senderKey, + args: [senderKey.toPublicKey(), UInt64.from(100), Bool(true)], + nonce: 2, + }) + ); + + const txs2 = await mempool.getTxs(); + expect(txs2.length).toBe(2); + + const block = await appChain.produceBlock(); + + expectDefined(block); + expect(block.transactions).toHaveLength(1); + + await expect(mempool.getTxs()).resolves.toHaveLength(1); + }); + + it("check only one is included, other is removed", async () => { + await mempool.add( + createTransaction({ + runtime, + method: ["Balance", "setBalanceIf"], + privateKey: senderKey, + args: [senderKey.toPublicKey(), UInt64.from(100), Bool(true)], + nonce: 0, + }) + ); + + await mempool.add( + createTransaction({ + runtime, + method: ["Balance", "setBalanceIf"], + privateKey: senderKey, + args: [senderKey.toPublicKey(), UInt64.from(102), Bool(true)], + nonce: 0, + }) + ); + + const txs2 = await mempool.getTxs(); + expect(txs2.length).toBe(2); + + const block = await appChain.produceBlock(); + + expectDefined(block); + expect(block.transactions).toHaveLength(1); + + await expect(mempool.getTxs()).resolves.toHaveLength(0); + }); + }); + + describe("mempool simulation", () => { + beforeEach(async () => { + appChain = await createAppChain(true); + }, 60_000); + + it("check tx is removed", async () => { + await mempool.add( + createTransaction({ + runtime, + method: ["Balance", "setBalanceIf"], + privateKey: senderKey, + args: [senderKey.toPublicKey(), UInt64.from(100), Bool(true)], + nonce: 0, + }) + ); + + await mempool.add( + createTransaction({ + runtime, + method: ["Balance", "setBalanceIf"], + privateKey: senderKey, + args: [senderKey.toPublicKey(), UInt64.from(100), Bool(true)], + nonce: 1, + }) + ); + + const txs = await mempool.getTxs(); + expect(txs.length).toBe(2); + + await appChain!.produceBlock(); + + await mempool.add( + createTransaction({ + runtime, + method: ["Balance", "setBalanceIf"], + privateKey: senderKey, + args: [senderKey.toPublicKey(), UInt64.from(100), Bool(true)], + nonce: 2, + }) + ); + + await mempool.add( + createTransaction({ + runtime, + method: ["Balance", "setBalanceIf"], + privateKey: senderKey, + args: [senderKey.toPublicKey(), UInt64.from(100), Bool(true)], + nonce: 0, + }) + ); + + const txs2 = await mempool.getTxs(); + expect(txs2.length).toBe(1); + }, 300_000); + }); }); From 2fe29f093d93b516a7618fe9bef2efd847693e6b Mon Sep 17 00:00:00 2001 From: Raphael Panic Date: Fri, 7 Nov 2025 12:46:38 +0100 Subject: [PATCH 17/22] Applied limit to non-simulated mempool mode --- .../sequencer/src/mempool/private/PrivateMempool.ts | 3 ++- .../test/integration/BlockProductionSize.test.ts | 11 ++++++++++- packages/sequencer/test/integration/Mempool.test.ts | 9 ++++++++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/packages/sequencer/src/mempool/private/PrivateMempool.ts b/packages/sequencer/src/mempool/private/PrivateMempool.ts index 9e363d818..e750060ea 100644 --- a/packages/sequencer/src/mempool/private/PrivateMempool.ts +++ b/packages/sequencer/src/mempool/private/PrivateMempool.ts @@ -117,6 +117,7 @@ export class PrivateMempool @trace("mempool.get_txs") public async getTxs(limit?: number): Promise { + // TODO Add limit to the storage (or do something smarter entirely) const txs = await this.transactionStorage.getPendingUserTransactions(); const baseCachedStateService = new CachedStateService(this.stateService); @@ -133,7 +134,7 @@ export class PrivateMempool networkState, limit ) - : txs; + : txs.slice(0, limit); this.protocol.stateServiceProvider.popCurrentStateService(); return sortedTxs; diff --git a/packages/sequencer/test/integration/BlockProductionSize.test.ts b/packages/sequencer/test/integration/BlockProductionSize.test.ts index f47279974..b6453743a 100644 --- a/packages/sequencer/test/integration/BlockProductionSize.test.ts +++ b/packages/sequencer/test/integration/BlockProductionSize.test.ts @@ -5,6 +5,7 @@ import { Protocol } from "@proto-kit/protocol"; import { Bool, PrivateKey, Struct, UInt64 } from "o1js"; import "reflect-metadata"; import { container } from "tsyringe"; +import { afterEach } from "@jest/globals"; import { ManualBlockTrigger, @@ -37,6 +38,7 @@ describe("block limit", () => { NoopRuntime: typeof NoopRuntime; }>; let sequencer: Sequencer; + let appchain: AppChain; let blockTrigger: ManualBlockTrigger; let mempool: PrivateMempool; @@ -68,7 +70,9 @@ describe("block limit", () => { Sequencer: { Database: {}, BlockTrigger: {}, - Mempool: {}, + Mempool: { + validationEnabled: true, + }, BatchProducerModule: {}, BlockProducerModule: { maximumBlockSize: maxBlockSize, @@ -97,6 +101,7 @@ describe("block limit", () => { await app.start(false, container.createChildContainer()); ({ runtime, sequencer } = app); + appchain = app; mempool = sequencer.resolve("Mempool"); @@ -114,6 +119,10 @@ describe("block limit", () => { } } + afterEach(async () => { + await appchain.close(); + }); + it.each([ [5, 5], [10, 10], diff --git a/packages/sequencer/test/integration/Mempool.test.ts b/packages/sequencer/test/integration/Mempool.test.ts index 1eb68606a..2609ef78f 100644 --- a/packages/sequencer/test/integration/Mempool.test.ts +++ b/packages/sequencer/test/integration/Mempool.test.ts @@ -5,6 +5,7 @@ import { Protocol } from "@proto-kit/protocol"; import { Bool, PrivateKey, UInt64 } from "o1js"; import "reflect-metadata"; import { container } from "tsyringe"; +import { afterEach } from "@jest/globals"; import { InMemoryDatabase, @@ -96,7 +97,9 @@ describe.each([["InMemory", InMemoryDatabase]])( Sequencer: { Database: {}, BlockTrigger: {}, - Mempool: {}, + Mempool: { + validationEnabled: true, + }, FeeStrategy: {}, BatchProducerModule: {}, BlockProducerModule: {}, @@ -123,6 +126,10 @@ describe.each([["InMemory", InMemoryDatabase]])( mempool = sequencer.resolve("Mempool"); }); + afterEach(async () => { + await appChain.close(); + }); + it("transactions are returned in right order - simple", async () => { expect.assertions(13); From 02973714a909bd693eced8200a4ae869d79d6a2d Mon Sep 17 00:00:00 2001 From: Raphael Panic Date: Fri, 7 Nov 2025 14:54:38 +0100 Subject: [PATCH 18/22] Reenabled validation for fee test --- packages/sdk/test/fees-multi-zkprograms.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/sdk/test/fees-multi-zkprograms.test.ts b/packages/sdk/test/fees-multi-zkprograms.test.ts index b12469307..0328260a0 100644 --- a/packages/sdk/test/fees-multi-zkprograms.test.ts +++ b/packages/sdk/test/fees-multi-zkprograms.test.ts @@ -184,12 +184,21 @@ describe("check fee analyzer", () => { }, }, }, + Sequencer: { + Mempool: { + validationEnabled: true, + }, + }, }); await appChain.start(); appChain.setSigner(senderKey); }); + afterAll(async () => { + await appChain.close(); + }); + it("with multiple zk programs", async () => { expect.assertions(12); const testModule1 = appChain.runtime.resolve("TestModule1"); From a4ac17b724219c782aeba6638926b2776b017c1e Mon Sep 17 00:00:00 2001 From: Raphael Panic Date: Fri, 14 Nov 2025 14:21:41 +0100 Subject: [PATCH 19/22] Added removeTransactionWhen to TransactionHook --- .../library/src/hooks/TransactionFeeHook.ts | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/packages/library/src/hooks/TransactionFeeHook.ts b/packages/library/src/hooks/TransactionFeeHook.ts index 20d9b0adc..bd6570aa4 100644 --- a/packages/library/src/hooks/TransactionFeeHook.ts +++ b/packages/library/src/hooks/TransactionFeeHook.ts @@ -9,12 +9,13 @@ import { BeforeTransactionHookArguments, ProvableTransactionHook, PublicKeyOption, + StateMap, } from "@proto-kit/protocol"; import { Field, Provable, PublicKey } from "o1js"; import { noop } from "@proto-kit/common"; import { UInt64 } from "../math/UInt64"; -import { Balance, TokenId } from "../runtime/Balances"; +import { Balance, BalancesKey, TokenId } from "../runtime/Balances"; import { MethodFeeConfigData, @@ -29,6 +30,8 @@ interface Balances { to: PublicKey, amount: Balance ) => Promise; + + balances: StateMap; } export interface TransactionFeeHookConfig @@ -159,4 +162,27 @@ export class TransactionFeeHook extends ProvableTransactionHook { noop(); } + + public async removeTransactionWhen( + args: BeforeTransactionHookArguments + ): Promise { + const feeConfig = this.feeAnalyzer.getFeeConfig( + args.transaction.methodId.toBigInt() + ); + + const fee = this.getFee(feeConfig); + + const tokenId = new TokenId(this.config.tokenId); + const feeRecipient = PublicKey.fromBase58(this.config.feeRecipient); + + const balanceAvailable = await this.balances.balances.get({ + tokenId, + address: feeRecipient, + }); + + return balanceAvailable + .orElse(Balance.from(0)) + .greaterThanOrEqual(fee) + .toBoolean(); + } } From 4037f658a7e3b3939a5cebb90abe3e9d24fa81b0 Mon Sep 17 00:00:00 2001 From: Raphael Panic Date: Fri, 14 Nov 2025 14:24:39 +0100 Subject: [PATCH 20/22] Implemented removeTx as a set to increase lookup performance --- .../src/storage/inmemory/InMemoryTransactionStorage.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/sequencer/src/storage/inmemory/InMemoryTransactionStorage.ts b/packages/sequencer/src/storage/inmemory/InMemoryTransactionStorage.ts index a7591302f..df0fe7fd7 100644 --- a/packages/sequencer/src/storage/inmemory/InMemoryTransactionStorage.ts +++ b/packages/sequencer/src/storage/inmemory/InMemoryTransactionStorage.ts @@ -20,9 +20,10 @@ export class InMemoryTransactionStorage implements TransactionStorage { ) {} public async removeTx(hashes: string[]) { + const hashSet = new Set(hashes); this.queue = this.queue.filter((tx) => { const hash = tx.hash().toString(); - return !hashes.includes(hash); + return !hashSet.has(hash); }); } From 2221445e72543378f92fd930c1b1ab929d786211 Mon Sep 17 00:00:00 2001 From: Raphael Panic Date: Fri, 14 Nov 2025 14:53:24 +0100 Subject: [PATCH 21/22] Fixed implementation of TransactionFeeHook removeWhen --- packages/library/src/hooks/TransactionFeeHook.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/library/src/hooks/TransactionFeeHook.ts b/packages/library/src/hooks/TransactionFeeHook.ts index bd6570aa4..974c113f1 100644 --- a/packages/library/src/hooks/TransactionFeeHook.ts +++ b/packages/library/src/hooks/TransactionFeeHook.ts @@ -180,9 +180,6 @@ export class TransactionFeeHook extends ProvableTransactionHook Date: Fri, 14 Nov 2025 16:12:59 +0100 Subject: [PATCH 22/22] Fix fees failure error message --- packages/sdk/test/fees-failures.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sdk/test/fees-failures.test.ts b/packages/sdk/test/fees-failures.test.ts index a6df758d8..e8ebd35c3 100644 --- a/packages/sdk/test/fees-failures.test.ts +++ b/packages/sdk/test/fees-failures.test.ts @@ -109,7 +109,7 @@ describe("fee errors due to limited funds in sender accounts", () => { await appChain.produceBlock(); expect(logSpy).toHaveBeenCalledWith( - "Error in inclusion of tx, skipping: Protocol hooks not executable: From balance is insufficient" + "Error in inclusion of tx, removing as to removeWhen hooks: Protocol hooks not executable: From balance is insufficient" ); const balance = await appChain.query.runtime.Balances.balances.get(