diff --git a/packages/library/src/hooks/TransactionFeeHook.ts b/packages/library/src/hooks/TransactionFeeHook.ts index 20d9b0adc..974c113f1 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,24 @@ 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)).lessThan(fee).toBoolean(); + } } 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; 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 e2ccaecd9..d4544a7c4 100644 --- a/packages/persistance/src/services/prisma/PrismaTransactionStorage.ts +++ b/packages/persistance/src/services/prisma/PrismaTransactionStorage.ts @@ -35,6 +35,22 @@ export class PrismaTransactionStorage implements TransactionStorage { return txs.map((tx) => this.transactionMapper.mapIn(tx)); } + 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.deleteMany({ + where: { + hash: { + in: hashes, + }, + }, + }); + } + } + public async pushUserTransaction(tx: PendingTransaction): Promise { const { prismaClient } = this.connection; 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/protocol/src/hooks/AccountStateHook.ts b/packages/protocol/src/hooks/AccountStateHook.ts index 3369e6683..6e05f4762 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): Promise { + 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/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, }; } 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..e8ebd35c3 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, removing as to removeWhen hooks: Protocol hooks not executable: From balance is insufficient" ); const balance = await appChain.query.runtime.Balances.balances.get( 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"); 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 aa9d3f3ca..e750060ea 100644 --- a/packages/sequencer/src/mempool/private/PrivateMempool.ts +++ b/packages/sequencer/src/mempool/private/PrivateMempool.ts @@ -110,8 +110,14 @@ 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 { + // TODO Add limit to the storage (or do something smarter entirely) const txs = await this.transactionStorage.getPendingUserTransactions(); const baseCachedStateService = new CachedStateService(this.stateService); @@ -119,7 +125,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, @@ -128,7 +134,7 @@ export class PrivateMempool networkState, limit ) - : txs; + : txs.slice(0, limit); this.protocol.stateServiceProvider.popCurrentStateService(); return sortedTxs; @@ -188,6 +194,7 @@ export class PrivateMempool executionContext.setup(contextInputs); const signedTransaction = tx.toProtocolTransaction(); + // eslint-disable-next-line no-await-in-loop await this.accountStateHook.beforeTransaction({ networkState: networkState, @@ -218,6 +225,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}` ); diff --git a/packages/sequencer/src/protocol/production/sequencing/BlockProducerModule.ts b/packages/sequencer/src/protocol/production/sequencing/BlockProducerModule.ts index 85427d22a..8f035c338 100644 --- a/packages/sequencer/src/protocol/production/sequencing/BlockProducerModule.ts +++ b/packages/sequencer/src/protocol/production/sequencing/BlockProducerModule.ts @@ -251,6 +251,16 @@ export class BlockProducerModule extends SequencerModule { height: block.height.toString(), } ); + + // Remove included or dropped txs, leave skipped ones alone + await this.mempool.removeTxs( + blockResult.includedTxs + .filter((x) => x.type === "included") + .map((x) => x.hash), + blockResult.includedTxs + .filter((x) => x.type === "shouldRemove") + .map((x) => x.hash) + ); } 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..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 { @@ -93,6 +105,10 @@ export class BlockProductionService { | { block: Block; stateChanges: CachedStateService; + includedTxs: { + hash: string; + type: "included" | "skipped" | "shouldRemove"; + }[]; } | undefined > { @@ -124,7 +140,7 @@ export class BlockProductionService { UntypedStateTransition.fromStateTransition(transition) ); - const [newBlockState, executionResults] = + const { blockState: newBlockState, executionResults } = await this.transactionExecutionService.createExecutionTraces( stateService, transactions, @@ -142,8 +158,12 @@ export class BlockProductionService { return undefined; } + const includedTransactions = executionResults + .filter(isIncludedTxs) + .map((x) => x.result); + const block: Omit = { - transactions: executionResults, + transactions: includedTransactions, transactionsHash: newBlockState.transactionList.commitment, fromEternalTransactionsHash: lastBlock.toEternalTransactionsHash, toEternalTransactionsHash: @@ -165,12 +185,25 @@ export class BlockProductionService { const hash = Block.hash(block); + 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: { ...block, hash, }, stateChanges: stateService, + includedTxs, }; } } diff --git a/packages/sequencer/src/protocol/production/sequencing/TransactionExecutionService.ts b/packages/sequencer/src/protocol/production/sequencing/TransactionExecutionService.ts index 0e12cd7a1..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 { @@ -204,6 +212,8 @@ export class TransactionExecutionService { private readonly blockProver: BlockProverProgrammable; + private readonly txHooks: ProvableTransactionHook[]; + public constructor( @inject("Runtime") private readonly runtime: Runtime, @inject("Protocol") @@ -219,6 +229,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( @@ -269,15 +284,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; @@ -285,9 +291,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) @@ -317,23 +327,27 @@ export class TransactionExecutionService { ); } + // eslint-disable-next-line sonarjs/cognitive-complexity public async createExecutionTraces( asyncStateService: CachedStateService, transactions: PendingTransaction[], networkState: NetworkState, state: BlockTrackers - ): Promise<[BlockTrackers, TransactionExecutionResult[]]> { + ): Promise<{ + blockState: BlockTrackers; + executionResults: TransactionExecutionResultStatus[]; + }> { let blockState = state; - const executionResults: TransactionExecutionResult[] = []; + const executionResults: TransactionExecutionResultStatus[] = []; 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 = + const { result: executionTrace, shouldRemove } = // eslint-disable-next-line no-await-in-loop await this.createExecutionTrace( asyncStateService, @@ -343,18 +357,51 @@ export class TransactionExecutionService { newState ); - blockState = newState; + // 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 + ) { + 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({ + tx, + status: shouldRemove ? "shouldRemove" : "skipped", + }); + } else { + blockState = newState; - // Push result to results and transaction onto bundle-hash - executionResults.push(executionTrace); + // Push result to results and transaction onto bundle-hash + executionResults.push({ result: executionTrace, status: "included" }); + } } catch (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" }); } } } - return [blockState, executionResults]; + 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 }]) => ({ @@ -371,7 +418,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); @@ -463,7 +510,19 @@ export class TransactionExecutionService { afterTxHookResult.stateTransitions ); - await recordingStateService.mergeIntoParent(); + 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 this.stateServiceProvider.popCurrentStateService(); @@ -479,16 +538,23 @@ export class TransactionExecutionService { runtimeResult.stateTransitions, afterTxHookResult.stateTransitions, ], - runtimeResult.status + { runtime: runtimeResult.status.toBoolean(), hooks: txHooksValid } ); return { - tx, - status: runtimeResult.status, - statusMessage: runtimeResult.statusMessage, + result: { + tx, + hooksStatus: Bool(txHooksValid), + status: runtimeResult.status, + statusMessage: + beforeTxHookResult.statusMessage ?? + afterTxHookResult.statusMessage ?? + runtimeResult.statusMessage, - stateTransitions, - events: beforeHookEvents.concat(runtimeResultEvents, afterHookEvents), + stateTransitions, + events: beforeHookEvents.concat(runtimeResultEvents, afterHookEvents), + }, + shouldRemove, }; } } diff --git a/packages/sequencer/src/storage/inmemory/InMemoryTransactionStorage.ts b/packages/sequencer/src/storage/inmemory/InMemoryTransactionStorage.ts index 011e7d26d..df0fe7fd7 100644 --- a/packages/sequencer/src/storage/inmemory/InMemoryTransactionStorage.ts +++ b/packages/sequencer/src/storage/inmemory/InMemoryTransactionStorage.ts @@ -19,6 +19,14 @@ export class InMemoryTransactionStorage implements TransactionStorage { @inject("BatchStorage") private readonly batchStorage: InMemoryBatchStorage ) {} + public async removeTx(hashes: string[]) { + const hashSet = new Set(hashes); + this.queue = this.queue.filter((tx) => { + const hash = tx.hash().toString(); + return !hashSet.has(hash); + }); + } + public async getPendingUserTransactions(): Promise { const nextHeight = await this.blockStorage.getCurrentBlockHeight(); for ( 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 c739f68b4..ef353ad08 100644 --- a/packages/sequencer/src/storage/repositories/TransactionStorage.ts +++ b/packages/sequencer/src/storage/repositories/TransactionStorage.ts @@ -5,6 +5,8 @@ export interface TransactionStorage { getPendingUserTransactions: () => Promise; + removeTx: (txHashes: string[], type: "included" | "dropped") => Promise; + /** * Finds a transaction by its hash. * It returns both pending transaction and already included transactions 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); diff --git a/packages/sequencer/test/integration/MempoolTxRemoved.test.ts b/packages/sequencer/test/integration/MempoolTxRemoved.test.ts new file mode 100644 index 000000000..de598cde9 --- /dev/null +++ b/packages/sequencer/test/integration/MempoolTxRemoved.test.ts @@ -0,0 +1,180 @@ +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 { 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("mempool removal mechanism", () => { + const senderKey = PrivateKey.random(); + let appChain: Awaited>; + let mempool: PrivateMempool; + let runtime: Runtime<{ Balances: typeof Balances; Balance: typeof Balance }>; + let sequencer: Sequencer; + + const createAppChain = async (validationEnabled: boolean) => { + // eslint-disable-next-line @typescript-eslint/no-shadow + const appChain = TestingAppChain.fromRuntime({ Balance }); + + appChain.configurePartial({ + Runtime: { + Balance: {}, + Balances: {}, + }, + Protocol: { + ...appChain.config.Protocol!, + }, + Sequencer: { + ...appChain.config.Sequencer, + Mempool: { validationEnabled }, + }, + }); + + await appChain.start(); + runtime = appChain.runtime; + sequencer = appChain.sequencer; + + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + mempool = sequencer.resolve("Mempool"); + + 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); + }); +});