fix(transaction-pool): transaction expiry and rebroadcast

packages/configuration-generator/source/generators/milestones.test.ts

@@ -43,7 +43,7 @@ describe<{
 					epoch: date.toISOString().slice(0, 11) + "00:00:00.000Z",
 					evmSpec: Contracts.Evm.SpecId.SHANGHAI,
 					gas: {
-						maximumGasLimit: 2_000_000,
+						maximumGasLimit: 5_000_000,
 						maximumGasPrice: 10_000 * 1e9,
 						minimumGasLimit: 21_000,
 						minimumGasPrice: 5 * 1e9,

packages/configuration-generator/source/generators/milestones.ts

@@ -18,7 +18,7 @@ export class MilestonesGenerator {
 				epoch: options.epoch.toISOString().slice(0, 11) + "00:00:00.000Z",
 				evmSpec: Contracts.Evm.SpecId.SHANGHAI,
 				gas: {
-					maximumGasLimit: 2_000_000,
+					maximumGasLimit: 5_000_000,
 					maximumGasPrice: 10_000 * 1e9,
 					minimumGasLimit: 21_000,
 					minimumGasPrice: 5 * 1e9,

packages/contracts/source/contracts/transaction-pool/sender-mempool.ts

@@ -8,7 +8,7 @@ export interface SenderMempool {
 	getFromLatest(): Iterable<Transaction>;
 
 	addTransaction(transaction: Transaction): Promise<void>;
-	removeTransaction(id: string): Transaction[];
+	removeTransaction(hash: string): Transaction[];
 	reAddTransactions(): Promise<Transaction[]>;
 }
 

packages/contracts/source/contracts/transaction-pool/storage.ts

@@ -1,15 +1,15 @@
 export type StoredTransaction = {
-	height: number;
-	id: string;
+	blockNumber: number;
+	hash: string;
 	senderPublicKey: string;
 	serialized: Buffer;
 };
 
 export interface Storage {
 	addTransaction(storedTransaction: StoredTransaction): void;
-	hasTransaction(id: string): boolean;
+	hasTransaction(hash: string): boolean;
 	getAllTransactions(): Iterable<StoredTransaction>;
-	getOldTransactions(height: number): Iterable<StoredTransaction>;
-	removeTransaction(id: string): void;
+	getOldTransactions(blockNumber: number): Iterable<StoredTransaction>;
+	removeTransaction(hash: string): void;
 	flush(): void;
 }

packages/crypto-transaction/source/validation/keywords.test.ts

@@ -137,7 +137,7 @@ describe<{
 		assert.undefined(context.validator.validate("test", "0x").error);
 		assert.undefined(context.validator.validate("test", "00").error);
 
-		const maxBytecodeLength = cryptoJson.milestones[0].gas!.maximumGasLimit / 2;
+		const maxBytecodeLength = cryptoJson.milestones[0].gas!.maximumGasLimit / 16;
 		const maxPayload = "0x" + "a".repeat(maxBytecodeLength);
 		assert.undefined(context.validator.validate("test", maxPayload).error);
 

packages/crypto-transaction/source/validation/keywords.ts

@@ -138,7 +138,8 @@ export const makeKeywords = (configuration: Contracts.Crypto.Configuration) => {
 				}
 
 				// The allowed bytecode length is relative to the maximum transaction gas limit
-				const maxBytecodeLength = maximumGasLimit / 2;
+				// and cost of non-zero calldata per gas (16 byte).
+				const maxBytecodeLength = maximumGasLimit / 16;
 				const minBytecodeLength = 0;
 
 				const regex = new RegExp(`^(0x)?[0-9a-fA-F]{${minBytecodeLength},${maxBytecodeLength}}$`);

packages/transaction-pool-broadcaster/source/peer-communicator.ts

@@ -13,6 +13,9 @@
 	@tagged("plugin", "transaction-pool-broadcaster")
 	protected readonly configuration!: Providers.PluginConfiguration;
 
+	@inject(Identifiers.Services.Log.Service)
+	private readonly logger!: Contracts.Kernel.Logger;
+
 	public async postTransactions(
 		peer: Contracts.TransactionPool.Peer,
 		transactions: Contracts.Crypto.Transaction[],
@@ -30,6 +33,8 @@
 	}
 
 	private handleSocketError(peer: Contracts.TransactionPool.Peer, error: Error): void {
+		this.logger.debug(`socket error ${peer.ip}: ${error.message}`);
+
 		if (peer.errorCount++ > this.configuration.getRequired<number>("maxSequentialErrors")) {
 			this.repository.forgetPeer(peer.ip);
 			Ipc.emit("peer.removed", peer.ip);

packages/transaction-pool-service/source/defaults.ts

@@ -11,8 +11,8 @@ export const defaults = {
 	// then it will be removed.
 	maxTransactionAge: 2700,
 
-	// Based on a multipayment transaction with 256 recipients (base58), vendor field, 16 signatures plus some leeway
-	maxTransactionBytes: 9000,
+	// Based on limit used by other Ethereum clients such as Geth (128kb).
+	maxTransactionBytes: 128 * 1000,
 
 	// When the pool contains that many transactions, then a new transaction is
 	// only accepted if its fee is higher than the transaction with the lowest

packages/transaction-pool-service/source/mempool.ts

@@ -48,13 +48,13 @@
 		}
 	}
 
-	public async removeTransaction(address: string, id: string): Promise<Contracts.Crypto.Transaction[]> {
+	public async removeTransaction(address: string, hash: string): Promise<Contracts.Crypto.Transaction[]> {
 		const senderMempool = this.#senderMempools.get(address);
 		if (!senderMempool) {
 			return [];
 		}
 
-		const transactions = senderMempool.removeTransaction(id);
+		const transactions = senderMempool.removeTransaction(hash);
 		this.#removeDisposableMempool(address);
 
 		return transactions;

packages/transaction-pool-service/source/sender-mempool.ts

@@ -64,8 +64,8 @@
 		}
 	}
 
-	public removeTransaction(id: string): Contracts.Crypto.Transaction[] {
-		const index = this.#transactions.findIndex((t) => t.hash === id);
+	public removeTransaction(hash: string): Contracts.Crypto.Transaction[] {
+		const index = this.#transactions.findIndex((t) => t.hash === hash);
 		if (index === -1) {
 			return [];
 		}

packages/transaction-pool-service/source/service.ts

@@ -84,8 +84,8 @@
 			}
 
 			this.storage.addTransaction({
-				height: this.stateStore.getHeight(),
-				id: transaction.hash,
+				blockNumber: this.stateStore.getHeight(),
+				hash: transaction.hash,
 				senderPublicKey: transaction.data.senderPublicKey,
 				serialized: transaction.serialized,
 			});
@@ -121,11 +121,11 @@
 			let previouslyStoredFailures = 0;
 
 			const maxTransactionAge: number = this.pluginConfiguration.getRequired<number>("maxTransactionAge");
-			const lastHeight: number = this.stateStore.getHeight();
-			const expiredHeight: number = lastHeight - maxTransactionAge;
+			const lastBlockNumber: number = this.stateStore.getHeight();
+			const expiredBlockNumber: number = lastBlockNumber - maxTransactionAge;
 
-			for (const { height, id, serialized } of this.storage.getAllTransactions()) {
-				if (height > expiredHeight) {
+			for (const { blockNumber, hash, serialized } of this.storage.getAllTransactions()) {
+				if (blockNumber > expiredBlockNumber) {
 					try {
 						const previouslyStoredTransaction = await this.transactionFactory.fromBytes(serialized);
 						await this.#addTransactionToMempool(previouslyStoredTransaction);
@@ -137,14 +137,14 @@
 
 						previouslyStoredSuccesses++;
 					} catch (error) {
-						this.storage.removeTransaction(id);
-						this.logger.debug(`Failed to re-add previously stored tx ${id}: ${error.message}`);
+						this.storage.removeTransaction(hash);
+						this.logger.debug(`Failed to re-add previously stored tx ${hash}: ${error.message}`);
 
 						previouslyStoredFailures++;
 					}
 				} else {
-					this.storage.removeTransaction(id);
-					this.logger.debug(`Not re-adding previously stored expired tx ${id}`);
+					this.storage.removeTransaction(hash);
+					this.logger.debug(`Not re-adding previously stored expired tx ${hash}`);
 					previouslyStoredExpirations++;
 				}
 			}
@@ -179,20 +179,23 @@
 
 	async #removeOldTransactions(): Promise<void> {
 		const maxTransactionAge: number = this.pluginConfiguration.getRequired<number>("maxTransactionAge");
-		const lastHeight: number = this.stateStore.getHeight();
-		const expiredHeight: number = lastHeight - maxTransactionAge;
+		const lastBlockNumber: number = this.stateStore.getHeight();
+		const expiredBlockNumber: number = lastBlockNumber - maxTransactionAge;
 
-		for (const { senderPublicKey, id } of this.storage.getOldTransactions(expiredHeight)) {
+		for (const { senderPublicKey, hash } of this.storage.getOldTransactions(expiredBlockNumber)) {
 			const removedTransactions = await this.mempool.removeTransaction(
 				await this.addressFactory.fromPublicKey(senderPublicKey),
-				id,
+				hash,
 			);
 
-			for (const removedTransaction of removedTransactions) {
-				this.storage.removeTransaction(removedTransaction.hash);
-				this.logger.debug(`Removed old tx ${removedTransaction.hash}`);
+			const removedTransactionHashes = new Set(removedTransactions.map(({ hash }) => hash));
+			removedTransactionHashes.add(hash);
 
-				void this.events.dispatch(Events.TransactionEvent.Expired, removedTransaction.data);
+			for (const removedTransactionHash of removedTransactionHashes) {
+				this.storage.removeTransaction(removedTransactionHash);
+				this.logger.debug(`Removed old tx ${removedTransactionHash}`);
+
+				void this.events.dispatch(Events.TransactionEvent.Expired, removedTransactionHash);
 			}
 		}
 	}

packages/transaction-pool-service/source/storage.test.ts

@@ -67,13 +67,13 @@ describe<{
 
 		try {
 			storage.addTransaction({
-				height: 100,
-				id: "first-tx-id",
+				blockNumber: 100,
+				hash: "first-tx-hash",
 				senderPublicKey: "some-public-key",
 				serialized: Buffer.from("test"),
 			});
 
-			const has = storage.hasTransaction("first-tx-id");
+			const has = storage.hasTransaction("first-tx-hash");
 			assert.true(has);
 		} finally {
 			storage.dispose();
@@ -87,8 +87,8 @@ describe<{
 
 		try {
 			storage.addTransaction({
-				height: 100,
-				id: "first-tx-id",
+				blockNumber: 100,
+				hash: "first-tx-hash",
 				senderPublicKey: "some-public-key",
 				serialized: Buffer.from("test"),
 			});
@@ -107,15 +107,15 @@ describe<{
 
 		try {
 			const storedTransaction1 = {
-				height: 100,
-				id: "first-tx-id",
+				blockNumber: 100,
+				hash: "first-tx-hash",
 				senderPublicKey: "some-public-key",
 				serialized: Buffer.from("test"),
 			};
 
 			const storedTransaction2 = {
-				height: 100,
-				id: "second-tx-id",
+				blockNumber: 200,
+				hash: "second-tx-hash",
 				senderPublicKey: "second-public-key",
 				serialized: Buffer.from("second-serialized"),
 			};
@@ -137,15 +137,15 @@ describe<{
 
 		try {
 			const storedTransaction1 = {
-				height: 100,
-				id: "first-tx-id",
+				blockNumber: 100,
+				hash: "first-tx-hash",
 				senderPublicKey: "some-public-key",
 				serialized: Buffer.from("test"),
 			};
 
 			const storedTransaction2 = {
-				height: 200,
-				id: "second-tx-id",
+				blockNumber: 200,
+				hash: "second-tx-hash",
 				senderPublicKey: "second-public-key",
 				serialized: Buffer.from("second-serialized"),
 			};
@@ -167,15 +167,15 @@ describe<{
 
 		try {
 			const storedTransaction1 = {
-				height: 100,
-				id: "first-tx-id",
+				blockNumber: 100,
+				hash: "first-tx-hash",
 				senderPublicKey: "some-public-key",
 				serialized: Buffer.from("test"),
 			};
 
 			const storedTransaction2 = {
-				height: 200,
-				id: "second-tx-id",
+				blockNumber: 200,
+				hash: "second-tx-hash",
 				senderPublicKey: "second-public-key",
 				serialized: Buffer.from("second-serialized"),
 			};
@@ -197,13 +197,13 @@ describe<{
 
 		try {
 			storage.addTransaction({
-				height: 100,
-				id: "first-tx-id",
+				blockNumber: 100,
+				hash: "first-tx-hash",
 				senderPublicKey: "some-public-key",
 				serialized: Buffer.from("test"),
 			});
 
-			const has = storage.hasTransaction("first-tx-id");
+			const has = storage.hasTransaction("first-tx-hash");
 			assert.true(has);
 		} finally {
 			storage.dispose();
@@ -217,16 +217,16 @@ describe<{
 
 		try {
 			storage.addTransaction({
-				height: 100,
-				id: "first-tx-id",
+				blockNumber: 100,
+				hash: "first-tx-hash",
 				senderPublicKey: "some-public-key",
 				serialized: Buffer.from("test"),
 			});
 
 			assert.throws(() => {
 				storage.addTransaction({
-					height: 100,
-					id: "first-tx-id",
+					blockNumber: 100,
+					hash: "first-tx-hash",
 					senderPublicKey: "some-public-key",
 					serialized: Buffer.from("test"),
 				});
@@ -243,15 +243,15 @@ describe<{
 
 		try {
 			storage.addTransaction({
-				height: 100,
-				id: "first-tx-id",
+				blockNumber: 100,
+				hash: "first-tx-hash",
 				senderPublicKey: "some-public-key",
 				serialized: Buffer.from("test"),
 			});
 
-			storage.removeTransaction("first-tx-id");
+			storage.removeTransaction("first-tx-hash");
 
-			const has = storage.hasTransaction("first-tx-id");
+			const has = storage.hasTransaction("first-tx-hash");
 			assert.false(has);
 		} finally {
 			storage.dispose();
@@ -265,15 +265,15 @@ describe<{
 
 		try {
 			storage.addTransaction({
-				height: 100,
-				id: "first-tx-id",
+				blockNumber: 100,
+				hash: "first-tx-hash",
 				senderPublicKey: "dummy-sender-key-1",
 				serialized: Buffer.from("dummy-serialized-1"),
 			});
 
 			storage.addTransaction({
-				height: 100,
-				id: "second-tx-id",
+				blockNumber: 100,
+				hash: "second-tx-hash",
 				senderPublicKey: "dummy-sender-key-2",
 				serialized: Buffer.from("dummy-serialized-2"),
 			});