feat(crypto transaction): enforce mininum gas price (#830)

* add `transactionGasPrice` schema * update tests * update fixtures * add comment * style: resolve style guide violations * fix failing tests * take genesis block transactions into account * always convert tx_env.gas_price to wei
ArkEcosystem · Jan 21, 2025 · 8123c96 · 8123c96
1 parent 12fbdd0
commit 8123c96
Show file tree

Hide file tree

Showing 21 changed files with 166 additions and 27 deletions.
diff --git a/packages/api-http/test/fixtures/node_configuration.json b/packages/api-http/test/fixtures/node_configuration.json
@@ -9,7 +9,8 @@
 		"epoch": "2024-05-31T00:00:00.000Z",
 		"evmSpec": "Shanghai",
 		"gas": {
-			"minimumGasFee": 5,
+			"minimumGasPrice": 5,
+			"maximumGasPrice": 10000,
 			"minimumGasLimit": 21000,
 			"maximumGasLimit": 2000000
 		},

diff --git a/packages/configuration-generator/source/generators/genesis-block.test.ts b/packages/configuration-generator/source/generators/genesis-block.test.ts
@@ -36,7 +36,8 @@ describe<{
 					// @ts-ignore
 					gas: {
 						maximumGasLimit: 2000000,
-						minimumGasFee: 5,
+						minimumGasPrice: 5,
+						maximumGasPrice: 10000,
 						minimumGasLimit: 21000,
 					},
 				},

diff --git a/packages/configuration-generator/source/generators/milestones.test.ts b/packages/configuration-generator/source/generators/milestones.test.ts
@@ -48,7 +48,8 @@ describe<{
 					epoch: date.toISOString().slice(0, 11) + "00:00:00.000Z",
 					evmSpec: Contracts.Evm.SpecId.SHANGHAI,
 					gas: {
-						minimumGasFee: 5,
+						minimumGasPrice: 5,
+						maximumGasPrice: 10_000,
 						minimumGasLimit: 21_000,
 						maximumGasLimit: 2_000_000,
 					},

diff --git a/packages/configuration-generator/source/generators/milestones.ts b/packages/configuration-generator/source/generators/milestones.ts
@@ -19,8 +19,9 @@ export class MilestonesGenerator {
 				evmSpec: Contracts.Evm.SpecId.SHANGHAI,
 				gas: {
 					maximumGasLimit: 2_000_000,
-					minimumGasFee: 5,
+					maximumGasPrice: 10_000,
 					minimumGasLimit: 21_000,
+					minimumGasPrice: 5,
 				},
 				height: 0,
 				reward: "0",

diff --git a/packages/contracts/source/contracts/crypto/networks.ts b/packages/contracts/source/contracts/crypto/networks.ts
@@ -48,7 +48,8 @@ export type MilestoneTimeouts = {
 export type MilestoneGas = {
 	minimumGasLimit: number;
 	maximumGasLimit: number;
-	minimumGasFee: number;
+	minimumGasPrice: number;
+	maximumGasPrice: number;
 };
 
 export type MilestoneSnapshot = {

diff --git a/packages/core/bin/config/testnet/core/crypto.json b/packages/core/bin/config/testnet/core/crypto.json
@@ -2583,7 +2583,8 @@
 			"evmSpec": "Shanghai",
 			"gas": {
 				"maximumGasLimit": 2000000,
-				"minimumGasFee": 5,
+				"minimumGasPrice": 5,
+				"maximumGasPrice": 10000,
 				"minimumGasLimit": 21000
 			},
 			"height": 0,

diff --git a/packages/crypto-transaction-evm-call/source/versions/1.test.ts b/packages/crypto-transaction-evm-call/source/versions/1.test.ts
@@ -91,10 +91,13 @@ describe<{
 		}
 	});
 
-	it("#getSchema - gasPrice should be integer, min 0, max 1000", ({ validator }) => {
+	it("#getSchema - gasPrice should be integer, min 5, max 1000", ({ sandbox, validator }) => {
 		validator.addSchema(EvmCallTransaction.getSchema());
 
-		const validValues = [0, 5, 6, 1000];
+		const configuration = sandbox.app.get<Configuration>(Identifiers.Cryptography.Configuration);
+		configuration.setHeight(1);
+
+		const validValues = [5, 6, 1000];
 		for (const value of validValues) {
 			const transaction = {
 				...transactionOriginal,
@@ -104,7 +107,7 @@ describe<{
 			assert.undefined(validator.validate("evmCall", transaction).error);
 		}
 
-		const invalidValues = [-1, 1.1, "test", null, undefined, {}];
+		const invalidValues = [0, -1, 1.1, "test", null, undefined, {}];
 		for (const value of invalidValues) {
 			const transaction = {
 				...transactionOriginal,

diff --git a/packages/crypto-transaction/source/validation/keywords.test.ts b/packages/crypto-transaction/source/validation/keywords.test.ts
@@ -18,10 +18,14 @@ describe<{
 		context.sandbox.app.bind(Identifiers.Cryptography.Configuration).to(Configuration).inSingletonScope();
 		context.sandbox.app.get<Configuration>(Identifiers.Cryptography.Configuration).setConfig(cryptoJson);
 
-		const keywords = makeKeywords(context.sandbox.app.get<Configuration>(Identifiers.Cryptography.Configuration));
+		const configuration = context.sandbox.app.get<Configuration>(Identifiers.Cryptography.Configuration);
+		configuration.setHeight(0);
+
+		const keywords = makeKeywords(configuration);
 		context.validator.addKeyword(keywords.transactionType);
 		context.validator.addKeyword(keywords.network);
 		context.validator.addKeyword(keywords.transactionGasLimit);
+		context.validator.addKeyword(keywords.transactionGasPrice);
 		context.validator.addKeyword(keywords.bytecode);
 	});
 
@@ -81,6 +85,28 @@ describe<{
 		assert.undefined(context.validator.validate("test", "a").error);
 	});
 
+	it("keyword transactionGasPrice should be ok", (context) => {
+		const schema = {
+			$id: "test",
+			transactionGasPrice: {},
+		};
+		context.validator.addSchema(schema);
+
+		// Accept 0 gasFee for genesis block
+		const configuration = context.sandbox.app.get<Configuration>(Identifiers.Cryptography.Configuration);
+		configuration.setHeight(1); // simulate non-genesis block
+
+		assert.undefined(context.validator.validate("test", cryptoJson.milestones[0].gas!.minimumGasPrice).error);
+		assert.undefined(context.validator.validate("test", "5").error);
+		assert.undefined(context.validator.validate("test", 10000).error);
+
+		assert.defined(context.validator.validate("test", 1).error);
+		assert.defined(context.validator.validate("test", 0).error);
+		assert.defined(context.validator.validate("test", -1).error);
+		assert.defined(context.validator.validate("test", 10001).error);
+		assert.defined(context.validator.validate("test", Number.MAX_SAFE_INTEGER).error);
+	});
+
 	it("keyword transactionGasLimit should be ok", (context) => {
 		const schema = {
 			$id: "test",

diff --git a/packages/crypto-transaction/source/validation/keywords.ts b/packages/crypto-transaction/source/validation/keywords.ts
@@ -33,6 +33,66 @@ export const makeKeywords = (configuration: Contracts.Crypto.Configuration) => {
 		},
 	};
 
+	const transactionGasPrice: FuncKeywordDefinition = {
+		// @ts-ignore
+		compile(schema) {
+			// Used as lazy cache
+			const genesisTransactionsLookup: Set<string> = new Set();
+
+			return (data, parentSchema: AnySchemaObject) => {
+				const {
+					gas: { minimumGasPrice, maximumGasPrice },
+				} = configuration.getMilestone();
+
+				try {
+					const bignum = BigNumber.make(data);
+					if (bignum.isLessThan(minimumGasPrice)) {
+						// Accept 0 gasFee when processing genesis block only
+						if (!bignum.isZero()) {
+							return false;
+						}
+
+						// The height check is needed for when e.g. the genesis block itself is being built.
+						const height = configuration.getHeight();
+						let valid = height === 0;
+
+						// Otherwise lookup by transaction id
+						if (!valid && parentSchema && parentSchema.parentData && parentSchema.parentData.id) {
+							if (genesisTransactionsLookup.size === 0) {
+								const genesisBlock = configuration.get<Contracts.Crypto.BlockData | undefined>(
+									"genesisBlock.block",
+								);
+								for (const transaction of genesisBlock?.transactions || []) {
+									genesisTransactionsLookup.add(transaction.id);
+								}
+							}
+
+							valid = genesisTransactionsLookup.has(parentSchema.parentData.id);
+						}
+
+						return valid;
+					}
+
+					// The upper limit technically isn't needed and solely acts as a safeguard
+					// as there's no legit reason to go beyond it.
+					if (bignum.isGreaterThan(maximumGasPrice)) {
+						return false;
+					}
+				} catch {
+					return false;
+				}
+
+				return true;
+			};
+		},
+		errors: false,
+		keyword: "transactionGasPrice",
+		metaSchema: {
+			properties: {},
+			type: "object",
+		},
+	};
+
 	const transactionGasLimit: FuncKeywordDefinition = {
 		// @ts-ignore
 		compile(schema) {
@@ -108,6 +168,7 @@ export const makeKeywords = (configuration: Contracts.Crypto.Configuration) => {
 		bytecode,
 		network,
 		transactionGasLimit,
+		transactionGasPrice,
 		transactionType,
 	};
 };
diff --git a/packages/crypto-transaction/source/validation/schemas.test.ts b/packages/crypto-transaction/source/validation/schemas.test.ts
@@ -1,4 +1,4 @@
-import { Identifiers } from "@mainsail/contracts";
+import { Contracts, Identifiers } from "@mainsail/contracts";
 import { schemas as addressSchemas } from "@mainsail/crypto-address-keccak256";
 import { Configuration } from "@mainsail/crypto-config";
 import { schemas as keyPairSchemas } from "@mainsail/crypto-key-pair-ecdsa";
@@ -86,7 +86,7 @@ describe<{
 
 	const transactionOriginal = {
 		gasLimit: 21_000,
-		gasPrice: 1,
+		gasPrice: 5,
 		id: "1".repeat(64),
 		network: 30,
 		nonce: 1,
@@ -165,10 +165,11 @@ describe<{
 		}
 	});
 
-	it("transactionBaseSchema - gasPrice should be number min 0", ({ validator }) => {
+	it("transactionBaseSchema - gasPrice should be number min 5", ({ sandbox, validator }) => {
+		sandbox.app.get<Configuration>(Identifiers.Cryptography.Configuration).setHeight(1);
 		validator.addSchema(schema);
 
-		const validValues = [0, 1, 100];
+		const validValues = [5, 10, 100];
 		for (const value of validValues) {
 			const transaction = {
 				...transactionOriginal,
@@ -178,7 +179,7 @@ describe<{
 			assert.undefined(validator.validate("transaction", transaction).error);
 		}
 
-		const invalidValues = [-1, "-1", 1.1, BigNumber.make(-1), -1, null, undefined, {}, "test"];
+		const invalidValues = [0, -1, "-1", 1.1, BigNumber.make(-1), -1, null, undefined, {}, "test"];
 
 		for (const value of invalidValues) {
 			const transaction = {
@@ -190,6 +191,32 @@ describe<{
 		}
 	});
 
+	it("transactionBaseSchema - gasPrice should accept 0 for genesis block", ({ sandbox, validator }) => {
+		const configuration = sandbox.app.get<Configuration>(Identifiers.Cryptography.Configuration);
+		configuration.setHeight(1);
+
+		const genesisBlock: Contracts.Crypto.BlockData = configuration.get("genesisBlock.block");
+
+		validator.addSchema(schema);
+
+		const transaction = {
+			...transactionOriginal,
+			gasPrice: 0,
+		};
+
+		genesisBlock.transactions.push(transaction as unknown as Contracts.Crypto.TransactionData);
+
+		assert.undefined(validator.validate("transaction", transaction).error);
+
+		// Fails for non-genesis tx
+		transaction.id = "2".repeat(64);
+		assert.true(validator.validate("transaction", transaction).error.includes("gasPrice"));
+
+		// But works on height 0
+		configuration.setHeight(0);
+		assert.undefined(validator.validate("transaction", transaction).error);
+	});
+
 	it("transactionBaseSchema - id should be transactionId", ({ validator }) => {
 		validator.addSchema(schema);
 

diff --git a/packages/crypto-transaction/source/validation/schemas.ts b/packages/crypto-transaction/source/validation/schemas.ts
@@ -19,7 +19,7 @@ export const schemas = {
 export const transactionBaseSchema: SchemaObject = {
 	properties: {
 		gasLimit: { transactionGasLimit: {} },
-		gasPrice: { bignumber: { minimum: 0 } },
+		gasPrice: { transactionGasPrice: {} },
 		id: { anyOf: [{ $ref: "transactionId" }, { type: "null" }] },
 		// Legacy
 		legacySecondSignature: {

diff --git a/packages/evm/bindings/src/lib.rs b/packages/evm/bindings/src/lib.rs
@@ -35,6 +35,8 @@ mod logger;
 mod result;
 mod utils;
 
+const ONE_GWEI: u64 = 1_000_000_000;
+
 // A complex struct which cannot be exposed to JavaScript directly.
 pub struct EvmInner {
     persistent_db: PersistentDB,
@@ -432,7 +434,10 @@ impl EvmInner {
             })
             .modify_tx_env(|tx_env| {
                 tx_env.gas_limit = ctx.gas_limit;
-                tx_env.gas_price = ctx.gas_price.unwrap_or_else(|| U256::ZERO);
+                tx_env.gas_price = ctx
+                    .gas_price
+                    .unwrap_or_else(|| U256::ZERO)
+                    .saturating_mul(U256::from(ONE_GWEI));
                 tx_env.caller = ctx.caller;
                 tx_env.value = ctx.value;
                 tx_env.nonce = Some(ctx.nonce);
@@ -687,7 +692,10 @@ impl EvmInner {
             })
             .modify_tx_env(|tx_env| {
                 tx_env.gas_limit = ctx.gas_limit.unwrap_or_else(|| 15_000_000);
-                tx_env.gas_price = ctx.gas_price.unwrap_or_else(|| U256::ZERO);
+                tx_env.gas_price = ctx
+                    .gas_price
+                    .unwrap_or_else(|| U256::ZERO)
+                    .saturating_mul(U256::from(ONE_GWEI));
                 tx_env.caller = ctx.caller;
                 tx_env.value = ctx.value;
                 tx_env.nonce = ctx.nonce;

diff --git a/packages/test-framework/source/factories/factories/block.ts b/packages/test-framework/source/factories/factories/block.ts
@@ -7,6 +7,7 @@ import secrets from "../../internal/passphrases.json";
 import { Signer } from "../../internal/signer.js";
 import { FactoryBuilder } from "../factory-builder.js";
 import { generateApp } from "./generate-app.js";
+import { GAS_PRICE } from "./transaction.js";
 
 export const registerBlockFactory = async (
 	factory: FactoryBuilder,
@@ -40,7 +41,7 @@ export const registerBlockFactory = async (
 				transactions.push(
 					await signer.makeTransfer({
 						amount: ((options.amount || 2) + index).toString(),
-						gasPrice: options.fee || 1,
+						gasPrice: options.fee || GAS_PRICE,
 						passphrase: secrets[0],
 						recipientId: genesisAddresses[Math.floor(Math.random() * genesisAddresses.length)],
 					}),

diff --git a/packages/test-framework/source/factories/factories/transaction.ts b/packages/test-framework/source/factories/factories/transaction.ts
@@ -9,7 +9,7 @@ import { EvmCallOptions, TransactionOptions, TransferOptions } from "../types.js
 import { generateApp } from "./generate-app.js";
 
 const AMOUNT = 1;
-const GAS_PRICE = 1;
+export const GAS_PRICE = 5;
 
 interface EntityOptions<T extends TransactionBuilder<T>> {
 	entity: TransactionBuilder<T>;

diff --git a/tests/e2e/consensus/nodes/node0/core/crypto.json b/tests/e2e/consensus/nodes/node0/core/crypto.json
@@ -279,7 +279,8 @@
 			"evmSpec": "Shanghai",
 			"gas": {
 				"maximumGasLimit": 2000000,
-				"minimumGasFee": 5,
+				"minimumGasPrice": 5,
+				"maximumGasPrice": 10000,
 				"minimumGasLimit": 21000
 			},
 			"height": 0,

diff --git a/tests/e2e/consensus/nodes/node1/core/crypto.json b/tests/e2e/consensus/nodes/node1/core/crypto.json
@@ -279,7 +279,8 @@
 			"evmSpec": "Shanghai",
 			"gas": {
 				"maximumGasLimit": 2000000,
-				"minimumGasFee": 5,
+				"minimumGasPrice": 5,
+				"maximumGasPrice": 10000,
 				"minimumGasLimit": 21000
 			},
 			"height": 0,

diff --git a/tests/e2e/consensus/nodes/node2/core/crypto.json b/tests/e2e/consensus/nodes/node2/core/crypto.json
@@ -279,7 +279,8 @@
 			"evmSpec": "Shanghai",
 			"gas": {
 				"maximumGasLimit": 2000000,
-				"minimumGasFee": 5,
+				"minimumGasPrice": 5,
+				"maximumGasPrice": 10000,
 				"minimumGasLimit": 21000
 			},
 			"height": 0,

diff --git a/tests/e2e/consensus/nodes/node3/core/crypto.json b/tests/e2e/consensus/nodes/node3/core/crypto.json
@@ -279,7 +279,8 @@
 			"evmSpec": "Shanghai",
 			"gas": {
 				"maximumGasLimit": 2000000,
-				"minimumGasFee": 5,
+				"minimumGasPrice": 5,
+				"maximumGasPrice": 10000,
 				"minimumGasLimit": 21000
 			},
 			"height": 0,

diff --git a/tests/e2e/consensus/nodes/node4/core/crypto.json b/tests/e2e/consensus/nodes/node4/core/crypto.json
@@ -279,7 +279,8 @@
 			"evmSpec": "Shanghai",
 			"gas": {
 				"maximumGasLimit": 2000000,
-				"minimumGasFee": 5,
+				"minimumGasPrice": 5,
+				"maximumGasPrice": 10000,
 				"minimumGasLimit": 21000
 			},
 			"height": 0,