From 8bcf23ad908f51c6779c4bc301ee6cfaebc3c089 Mon Sep 17 00:00:00 2001 From: HFazelinia Date: Sat, 30 Nov 2024 09:20:44 +0000 Subject: [PATCH 1/5] refactor EVM fee --- .changeset/dull-hotels-trade.md | 6 + .changeset/wet-terms-decide.md | 5 + packages/chains/evm/lib/EvmChain.ts | 137 +++++++++++++----- .../evm/lib/network/AbstractEvmNetwork.ts | 19 +-- packages/chains/evm/tests/EvmChain.spec.ts | 109 ++++++-------- packages/chains/evm/tests/TestUtils.ts | 15 +- .../evm/tests/network/TestEvmNetwork.ts | 8 +- .../networks/evm-rpc/lib/EvmRpcNetwork.ts | 23 +++ .../evm-rpc/tests/EvmRpcNetwork.spec.ts | 32 +--- 9 files changed, 200 insertions(+), 154 deletions(-) create mode 100644 .changeset/dull-hotels-trade.md create mode 100644 .changeset/wet-terms-decide.md diff --git a/.changeset/dull-hotels-trade.md b/.changeset/dull-hotels-trade.md new file mode 100644 index 0000000..6b7e71e --- /dev/null +++ b/.changeset/dull-hotels-trade.md @@ -0,0 +1,6 @@ +--- +'@rosen-chains/evm': major +--- + +Replace getMaxFeePerGas and getMaxPriorityFeePerGas functions with getFeeData function in AbstractEvmNetwork +Use gas price instead of getMaxFee variables in tx generation and verification only if maxFee variables are not defined in the network diff --git a/.changeset/wet-terms-decide.md b/.changeset/wet-terms-decide.md new file mode 100644 index 0000000..946dcd8 --- /dev/null +++ b/.changeset/wet-terms-decide.md @@ -0,0 +1,5 @@ +--- +'@rosen-chains/evm-rpc': major +--- + +Replace getMaxFeePerGas and getMaxPriorityFeePerGas functions with getFeeData function in AbstractEvmNetwork diff --git a/packages/chains/evm/lib/EvmChain.ts b/packages/chains/evm/lib/EvmChain.ts index 4cf9fdf..8e1363d 100644 --- a/packages/chains/evm/lib/EvmChain.ts +++ b/packages/chains/evm/lib/EvmChain.ts @@ -128,7 +128,29 @@ abstract class EvmChain extends AbstractChain { nextNonce++; } - const gasPrice = await this.network.getMaxFeePerGas(); + const feeData = await this.network.getFeeData(); + let requiredEth = 0n; + let txFeeData; + if (feeData.maxFeePerGas !== null) { + requiredEth = + this.configs.gasLimitCap * BigInt(orders.length) * feeData.maxFeePerGas; + txFeeData = { + maxFeePerGas: feeData.maxFeePerGas, + maxPriorityFeePerGas: feeData.maxPriorityFeePerGas, + }; + } else if (feeData.gasPrice !== null) { + requiredEth = + this.configs.gasLimitCap * BigInt(orders.length) * feeData.gasPrice; + txFeeData = { + gasPrice: feeData.gasPrice, + }; + } else { + throw new ImpossibleBehavior( + `Both 'maxFeePerGas' and 'gasPrice' values are falsy: ${JsonBigInt.stringify( + feeData + )}` + ); + } // check the balance in the lock address const requiredAssets: AssetBalance = ChainUtils.sumAssetBalance( @@ -136,7 +158,7 @@ abstract class EvmChain extends AbstractChain { { nativeToken: this.tokenMap.wrapAmount( this.NATIVE_TOKEN_ID, - this.configs.gasLimitCap * BigInt(orders.length) * gasPrice, + requiredEth, this.CHAIN ).amount, tokens: [], @@ -156,7 +178,6 @@ abstract class EvmChain extends AbstractChain { // try to generate transactions let totalGas = 0n; - const maxPriorityFeePerGas = await this.network.getMaxPriorityFeePerGas(); const evmTrxs: Array = []; for (const singleOrder of orders) { let trx; @@ -170,11 +191,10 @@ abstract class EvmChain extends AbstractChain { type: 2, to: singleOrder.address, nonce: nextNonce, - maxPriorityFeePerGas: maxPriorityFeePerGas, - maxFeePerGas: gasPrice, data: '0x' + eventId, value: value, chainId: this.CHAIN_ID, + ...txFeeData, }); } else { const token = singleOrder.assets.tokens[0]; @@ -193,11 +213,10 @@ abstract class EvmChain extends AbstractChain { type: 2, to: token.id, nonce: nextNonce, - maxPriorityFeePerGas: maxPriorityFeePerGas, - maxFeePerGas: gasPrice, data: data + eventId, value: 0n, chainId: this.CHAIN_ID, + ...txFeeData, }); } let estimatedRequiredGas = await this.network.getGasRequired(trx); @@ -367,6 +386,7 @@ abstract class EvmChain extends AbstractChain { transaction: PaymentTransaction ): Promise => { let tx: Transaction; + const baseError = `Tx [${transaction.txId}] is not verified: `; try { tx = Serializer.deserialize(transaction.txBytes); } catch (error) { @@ -420,40 +440,91 @@ abstract class EvmChain extends AbstractChain { if (gasDifference > gasLimitSlippage) { this.logger.warn( - `Tx [${transaction.txId}] is not verified: Transaction gas limit [${tx.gasLimit}] is too far from calculated gas limit [${gasRequired}]` + baseError + + `Transaction gas limit [${tx.gasLimit}] is too far from calculated gas limit [${gasRequired}]` ); return false; } // check fees - const networkMaxFee = await this.network.getMaxFeePerGas(); - const maxFeeSlippage = - (networkMaxFee * this.configs.gasPriceSlippage) / 100n; - const maxFeeDifference = - tx.maxFeePerGas >= networkMaxFee - ? tx.maxFeePerGas - networkMaxFee - : networkMaxFee - tx.maxFeePerGas; - - if (maxFeeDifference > maxFeeSlippage) { - this.logger.warn( - `Tx [${transaction.txId}] is not verified: Transaction max fee [${tx.maxFeePerGas}] is too far from network's max fee [${networkMaxFee}]` - ); - return false; - } + const feeData = await this.network.getFeeData(); + if ( + feeData.maxFeePerGas !== null && + feeData.maxPriorityFeePerGas !== null + ) { + const networkMaxFee = feeData.maxFeePerGas; + const maxFeeSlippage = + (networkMaxFee * this.configs.gasPriceSlippage) / 100n; + const maxFeeDifference = + tx.maxFeePerGas >= networkMaxFee + ? tx.maxFeePerGas - networkMaxFee + : networkMaxFee - tx.maxFeePerGas; + + if (maxFeeDifference > maxFeeSlippage) { + this.logger.warn( + baseError + + `Transaction max fee [${tx.maxFeePerGas}] is too far from network's max fee [${networkMaxFee}]` + ); + return false; + } - const networkMaxPriorityFee = await this.network.getMaxPriorityFeePerGas(); - const priorityFeeSlippage = - (networkMaxPriorityFee * this.configs.gasPriceSlippage) / 100n; - const maxPriorityFeeDifference = - tx.maxPriorityFeePerGas >= networkMaxPriorityFee - ? tx.maxPriorityFeePerGas - networkMaxPriorityFee - : networkMaxPriorityFee - tx.maxPriorityFeePerGas; + const networkMaxPriorityFee = feeData.maxPriorityFeePerGas; + const priorityFeeSlippage = + (networkMaxPriorityFee * this.configs.gasPriceSlippage) / 100n; + const maxPriorityFeeDifference = + tx.maxPriorityFeePerGas >= networkMaxPriorityFee + ? tx.maxPriorityFeePerGas - networkMaxPriorityFee + : networkMaxPriorityFee - tx.maxPriorityFeePerGas; - if (maxPriorityFeeDifference > priorityFeeSlippage) { - this.logger.warn( - `Tx [${transaction.txId}] is not verified: Transaction max priority fee [${tx.maxPriorityFeePerGas}] is too far from network's max priority fee [${networkMaxPriorityFee}]` + if (maxPriorityFeeDifference > priorityFeeSlippage) { + this.logger.warn( + baseError + + `Transaction max priority fee [${tx.maxPriorityFeePerGas}] is too far from network's max priority fee [${networkMaxPriorityFee}]` + ); + return false; + } + + if (tx.gasPrice) { + this.logger.warn( + baseError + + `Transaction has gas price [${ + tx.gasPrice + }] while network support's max fee is defined [${JsonBigInt.stringify( + feeData + )}]` + ); + return false; + } + } else if (feeData.gasPrice) { + if (!tx.gasPrice) { + this.logger.warn( + baseError + + `Expected transaction with gas price but found [${tx.gasPrice}]` + ); + return false; + } + + const networkGasPrice = feeData.gasPrice; + const gasPriceSlippage = + (networkGasPrice * this.configs.gasPriceSlippage) / 100n; + const gasPriceDifference = + tx.gasPrice >= networkGasPrice + ? tx.gasPrice - networkGasPrice + : networkGasPrice - tx.gasPrice; + + if (gasPriceDifference > gasPriceSlippage) { + this.logger.warn( + baseError + + `Transaction gas price [${tx.gasPrice}] is too far from network's gas price [${networkGasPrice}]` + ); + return false; + } + } else { + throw new ImpossibleBehavior( + `Both 'maxFeePerGas' and 'gasPrice' values are falsy: ${JsonBigInt.stringify( + feeData + )}` ); - return false; } return true; }; diff --git a/packages/chains/evm/lib/network/AbstractEvmNetwork.ts b/packages/chains/evm/lib/network/AbstractEvmNetwork.ts index 64139fc..b1bbc06 100644 --- a/packages/chains/evm/lib/network/AbstractEvmNetwork.ts +++ b/packages/chains/evm/lib/network/AbstractEvmNetwork.ts @@ -1,5 +1,5 @@ import { AbstractChainNetwork } from '@rosen-chains/abstract-chain'; -import { Transaction, TransactionResponse } from 'ethers'; +import { FeeData, Transaction } from 'ethers'; import { EvmTxStatus, TransactionHashes } from '../types'; abstract class AbstractEvmNetwork extends AbstractChainNetwork { @@ -36,19 +36,16 @@ abstract class AbstractEvmNetwork extends AbstractChainNetwork { * @returns gas required in bigint */ abstract getGasRequired: (transaction: Transaction) => Promise; - /** - * gets the maximum wei we would pay to the miner according - * to the network's current condition - * @returns gas price as a bigint - */ - abstract getMaxPriorityFeePerGas: () => Promise; /** - * gets the maximum wei we would pay (miner + base fee) according - * to the network's current condition - * @returns gas price as a bigint + * gets fee-related values associated with the network + * - the legacy gas price + * - the maximum fee to pay per gas + * - the additional amount to pay per gas to miner + * it may or may not include all the values, depends on the instance implementation + * @returns fee-related values as bigint or null */ - abstract getMaxFeePerGas: () => Promise; + abstract getFeeData: () => Promise; /** * gets all transactions in mempool (returns empty list if the chain has no mempool) diff --git a/packages/chains/evm/tests/EvmChain.spec.ts b/packages/chains/evm/tests/EvmChain.spec.ts index 8eedafe..fe18b25 100644 --- a/packages/chains/evm/tests/EvmChain.spec.ts +++ b/packages/chains/evm/tests/EvmChain.spec.ts @@ -12,7 +12,7 @@ import TestEvmNetwork from './network/TestEvmNetwork'; import * as TestData from './testData'; import * as testUtils from './TestUtils'; import Serializer from '../lib/Serializer'; -import { Transaction, TransactionLike } from 'ethers'; +import { FeeData, Transaction, TransactionLike } from 'ethers'; import { mockGetAddressBalanceForNativeToken } from './TestUtils'; import { EvmTxStatus } from '../lib'; import TestChain from './TestChain'; @@ -50,9 +50,8 @@ describe('EvmChain', () => { * successfully for multiple orders * @dependencies * @scenario - * - mock hasLockAddressEnoughAssets, getMaxFeePerGas + * - mock hasLockAddressEnoughAssets, getFeeData * - mock getGasRequired, getAddressNextNonce - * - mock getMaxPriorityFeePerGas * - call the function * - check returned value * @expected @@ -86,14 +85,13 @@ describe('EvmChain', () => { Buffer.from(Serializer.signedSerialize(elem)).toString('hex') ); - // mock hasLockAddressEnoughAssets, getMaxFeePerGas, - // getGasRequired, getAddressNextNonce, getMaxPriorityFeePerGas + // mock hasLockAddressEnoughAssets, getFeeData, + // getGasRequired, getAddressNextNonce const requiredGas = 100000n; testUtils.mockHasLockAddressEnoughAssets(evmChain, true); - testUtils.mockGetMaxFeePerGas(network, 10n); + testUtils.mockGetFeeData(network, new FeeData(1n, 10n, 10n)); testUtils.mockGetGasRequired(network, requiredGas); testUtils.mockGetAddressNextAvailableNonce(network, nonce); - testUtils.mockGetMaxPriorityFeePerGas(network, 10n); // run test const evmTxs = await evmChain.generateMultipleTransactions( @@ -146,9 +144,8 @@ describe('EvmChain', () => { * transaction successfully for single order * @dependencies * @scenario - * - mock hasLockAddressEnoughAssets, getMaxFeePerGas + * - mock hasLockAddressEnoughAssets, getFeeData * - mock getGasRequired, getAddressNextNonce - * - mock getMaxPriorityFeePerGas * - call the function * - check returned value * @expected @@ -168,14 +165,13 @@ describe('EvmChain', () => { const txType = TransactionType.payment; const nonce = 54; - // mock hasLockAddressEnoughAssets, getMaxFeePerGas, - // getGasRequired, getAddressNextNonce, getMaxPriorityFeePerGas + // mock hasLockAddressEnoughAssets, getFeeData, + // getGasRequired, getAddressNextNonce const requiredGas = 21000n; testUtils.mockHasLockAddressEnoughAssets(evmChain, true); - testUtils.mockGetMaxFeePerGas(network, 10n); + testUtils.mockGetFeeData(network, new FeeData(1n, 10n, 10n)); testUtils.mockGetGasRequired(network, requiredGas); testUtils.mockGetAddressNextAvailableNonce(network, nonce); - testUtils.mockGetMaxPriorityFeePerGas(network, 10n); // run test const evmTx = await evmChain.generateMultipleTransactions( @@ -221,9 +217,8 @@ describe('EvmChain', () => { * transaction with capped gas limit when required is too high * @dependencies * @scenario - * - mock hasLockAddressEnoughAssets, getMaxFeePerGas + * - mock hasLockAddressEnoughAssets, getFeeData * - mock getGasRequired, getAddressNextNonce - * - mock getMaxPriorityFeePerGas * - call the function * - check returned value * @expected @@ -243,13 +238,12 @@ describe('EvmChain', () => { const txType = TransactionType.payment; const nonce = 54; - // mock hasLockAddressEnoughAssets, getMaxFeePerGas, - // getGasRequired, getAddressNextNonce, getMaxPriorityFeePerGas + // mock hasLockAddressEnoughAssets, getFeeData, + // getGasRequired, getAddressNextNonce testUtils.mockHasLockAddressEnoughAssets(evmChain, true); - testUtils.mockGetMaxFeePerGas(network, 10n); + testUtils.mockGetFeeData(network, new FeeData(1n, 10n, 10n)); testUtils.mockGetGasRequired(network, 200000n); testUtils.mockGetAddressNextAvailableNonce(network, nonce); - testUtils.mockGetMaxPriorityFeePerGas(network, 10n); // run test const evmTx = await evmChain.generateMultipleTransactions( @@ -388,14 +382,13 @@ describe('EvmChain', () => { ); const nonce = 53; - // mock hasLockAddressEnoughAssets, getMaxFeePerGas, - // getGasRequired, getAddressNextNonce, getMaxPriorityFeePerGas + // mock hasLockAddressEnoughAssets, getFeeData, + // getGasRequired, getAddressNextNonce const requiredGas = 21000n; testUtils.mockHasLockAddressEnoughAssets(evmChain, true); - testUtils.mockGetMaxFeePerGas(network, 10n); + testUtils.mockGetFeeData(network, new FeeData(1n, 10n, 10n)); testUtils.mockGetGasRequired(network, requiredGas); testUtils.mockGetAddressNextAvailableNonce(network, nonce); - testUtils.mockGetMaxPriorityFeePerGas(network, 10n); // run test const evmTx = await evmChain.generateMultipleTransactions( @@ -488,9 +481,8 @@ describe('EvmChain', () => { * transaction successfully for wrapped order * @dependencies * @scenario - * - mock hasLockAddressEnoughAssets, getMaxFeePerGas + * - mock hasLockAddressEnoughAssets, getFeeData * - mock getGasRequired, getAddressNextNonce - * - mock getMaxPriorityFeePerGas * - call the function * - check returned value * @expected @@ -515,10 +507,9 @@ describe('EvmChain', () => { // mock hasLockAddressEnoughAssets, getMaxFeePerGas, // getGasRequired, getAddressNextNonce, getMaxPriorityFeePerGas testUtils.mockHasLockAddressEnoughAssets(evmChain, true); - testUtils.mockGetMaxFeePerGas(network, 10n); + testUtils.mockGetFeeData(network, new FeeData(1n, 10n, 10n)); testUtils.mockGetGasRequired(network, 21000n); testUtils.mockGetAddressNextAvailableNonce(network, nonce); - testUtils.mockGetMaxPriorityFeePerGas(network, 10n); // run test const evmTx = await evmChain.generateMultipleTransactions( @@ -782,7 +773,7 @@ describe('EvmChain', () => { * @dependencies * @scenario * - mock mockGetGasRequired - * - mock mockGetMaxFeePerGas, mockGetMaxPriorityFeePerGas + * - mock getFeeData * - mock PaymentTransaction * - check returned value * @expected @@ -791,8 +782,7 @@ describe('EvmChain', () => { it('should return true when both fees and gasLimit are set properly', async () => { // mock a config that has almost the same fee as the mocked transaction testUtils.mockGetGasRequired(network, 100000n); - testUtils.mockGetMaxFeePerGas(network, 20n); - testUtils.mockGetMaxPriorityFeePerGas(network, 7n); + testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); // mock PaymentTransaction const tx = Transaction.from(TestData.transaction1Json); @@ -823,7 +813,7 @@ describe('EvmChain', () => { * @dependencies * @scenario * - mock mockGetGasRequired - * - mock mockGetMaxFeePerGas, mockGetMaxPriorityFeePerGas + * - mock getFeeData * - mock PaymentTransaction * @expected * - return false @@ -831,8 +821,7 @@ describe('EvmChain', () => { it('should return false when `to` is null', async () => { // mock a config that has almost the same fee as the transaction fee testUtils.mockGetGasRequired(network, 100000n); - testUtils.mockGetMaxFeePerGas(network, 20n); - testUtils.mockGetMaxPriorityFeePerGas(network, 7n); + testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); // mock PaymentTransaction const tx = Transaction.from(TestData.transaction1Json); @@ -864,7 +853,7 @@ describe('EvmChain', () => { * @dependencies * @scenario * - mock mockGetGasRequired - * - mock mockGetMaxFeePerGas, mockGetMaxPriorityFeePerGas + * - mock getFeeData * - mock PaymentTransaction * @expected * - return false @@ -872,8 +861,7 @@ describe('EvmChain', () => { it('should return false when transaction is not of type 2', async () => { // mock a config that has almost the same fee as the transaction fee testUtils.mockGetGasRequired(network, 55000n); - testUtils.mockGetMaxFeePerGas(network, 20n); - testUtils.mockGetMaxPriorityFeePerGas(network, 7n); + testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); // mock PaymentTransaction const tx = Transaction.from(TestData.transaction1Json); @@ -906,7 +894,7 @@ describe('EvmChain', () => { * @dependencies * @scenario * - mock mockGetGasRequired - * - mock mockGetMaxFeePerGas, mockGetMaxPriorityFeePerGas + * - mock getFeeData * - mock PaymentTransaction * - check returned value * @expected @@ -916,8 +904,7 @@ describe('EvmChain', () => { // mock a config that has more fee and wrong required gas // comparing to the mocked transaction testUtils.mockGetGasRequired(network, 90000n); - testUtils.mockGetMaxFeePerGas(network, 20n); - testUtils.mockGetMaxPriorityFeePerGas(network, 7n); + testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); // mock PaymentTransaction const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; @@ -949,7 +936,7 @@ describe('EvmChain', () => { * @dependencies * @scenario * - mock mockGetGasRequired - * - mock mockGetMaxFeePerGas, mockGetMaxPriorityFeePerGas + * - mock getFeeData * - mock PaymentTransaction * - check returned value * @expected @@ -959,8 +946,7 @@ describe('EvmChain', () => { // mock a config that has more fee and wrong required gas // comparing to the mocked transaction testUtils.mockGetGasRequired(network, 90000n); - testUtils.mockGetMaxFeePerGas(network, 20n); - testUtils.mockGetMaxPriorityFeePerGas(network, 7n); + testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); // mock PaymentTransaction const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; @@ -992,7 +978,7 @@ describe('EvmChain', () => { * @dependencies * @scenario * - mock mockGetGasRequired - * - mock mockGetMaxFeePerGas, mockGetMaxPriorityFeePerGas + * - mock getFeeData * - mock PaymentTransaction * - check returned value * @expected @@ -1002,8 +988,7 @@ describe('EvmChain', () => { // mock a config that has more fee and wrong required gas // comparing to the mocked transaction testUtils.mockGetGasRequired(network, 20000n); - testUtils.mockGetMaxFeePerGas(network, 20n); - testUtils.mockGetMaxPriorityFeePerGas(network, 7n); + testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); // mock PaymentTransaction const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; @@ -1036,7 +1021,7 @@ describe('EvmChain', () => { * @dependencies * @scenario * - mock mockGetGasRequired - * - mock mockGetMaxFeePerGas, mockGetMaxPriorityFeePerGas + * - mock getFeeData * - mock PaymentTransaction * - check returned value * @expected @@ -1045,8 +1030,7 @@ describe('EvmChain', () => { it('should return false when maxFeePerGas is too much bigger than expected', async () => { // mock a config that has too much bigger max fee comparing to the mocked transaction testUtils.mockGetGasRequired(network, 76000n); - testUtils.mockGetMaxFeePerGas(network, 20n); - testUtils.mockGetMaxPriorityFeePerGas(network, 7n); + testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); // mock PaymentTransaction const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; @@ -1078,7 +1062,7 @@ describe('EvmChain', () => { * @dependencies * @scenario * - mock mockGetGasRequired - * - mock mockGetMaxFeePerGas, mockGetMaxPriorityFeePerGas + * - mock getFeeData * - mock PaymentTransaction * - check returned value * @expected @@ -1087,8 +1071,7 @@ describe('EvmChain', () => { it('should return false when maxFeePerGas is too much smaller than exptected', async () => { // mock a config that has too much smaller max fee comparing to the mocked transaction testUtils.mockGetGasRequired(network, 76000n); - testUtils.mockGetMaxFeePerGas(network, 20n); - testUtils.mockGetMaxPriorityFeePerGas(network, 7n); + testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); // mock PaymentTransaction const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; @@ -1120,7 +1103,7 @@ describe('EvmChain', () => { * @dependencies * @scenario * - mock mockGetGasRequired - * - mock mockGetMaxFeePerGas, mockGetMaxPriorityFeePerGas + * - mock getFeeData * - mock PaymentTransaction * - check returned value * @expected @@ -1129,8 +1112,7 @@ describe('EvmChain', () => { it('should return false when maxPriorityFeePerGas is too much bigger than expected', async () => { // mock a config that has too much bigger max fee comparing to the mocked transaction testUtils.mockGetGasRequired(network, 76000n); - testUtils.mockGetMaxFeePerGas(network, 20n); - testUtils.mockGetMaxPriorityFeePerGas(network, 7n); + testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); // mock PaymentTransaction const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; @@ -1162,7 +1144,7 @@ describe('EvmChain', () => { * @dependencies * @scenario * - mock mockGetGasRequired - * - mock mockGetMaxFeePerGas, mockGetMaxPriorityFeePerGas + * - mock getFeeData * - mock PaymentTransaction * - check returned value * @expected @@ -1171,8 +1153,7 @@ describe('EvmChain', () => { it('should return false when maxPriorityFeePerGas is too much smaller than expected', async () => { // mock a config that has too much bigger max fee comparing to the mocked transaction testUtils.mockGetGasRequired(network, 76000n); - testUtils.mockGetMaxFeePerGas(network, 20n); - testUtils.mockGetMaxPriorityFeePerGas(network, 7n); + testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); // mock PaymentTransaction const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; @@ -1460,7 +1441,7 @@ describe('EvmChain', () => { * @dependencies * @scenario * - mock valid PaymentTransaction - * - mock getMaxFeePerGas, getMaxPriorityFeePerGas + * - mock getFeeData * - mock hasLockAddressEnoughAssets * - run test * - check function is called @@ -1468,9 +1449,8 @@ describe('EvmChain', () => { * - it should call the function */ it('should submit the transaction when transaction is of type 2, fees are set properly and lock address has enough assets', async () => { - // mock getMaxFeePerGas, getMaxPriorityFeePerGas, and hasLockAddressEnoughAssets - testUtils.mockGetMaxFeePerGas(network, 20n); - testUtils.mockGetMaxPriorityFeePerGas(network, 7n); + // mock getFeeData and hasLockAddressEnoughAssets + testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); testUtils.mockHasLockAddressEnoughAssets(evmChain, true); // mock PaymentTransaction @@ -1572,7 +1552,7 @@ describe('EvmChain', () => { * @dependencies * @scenario * - mock invalid PaymentTransaction - * - mock getMaxFeePerGas, getMaxPriorityFeePerGas + * - mock getFeeData * - mock hasLockAddressEnoughAssets * - run test * - check the function is not called @@ -1580,9 +1560,8 @@ describe('EvmChain', () => { * - it should not call the function */ it('should not submit the transaction when lock address does not have enough asset', async () => { - // mock getMaxFeePerGas, getMaxPriorityFeePerGas, and hasLockAddressEnoughAssets - testUtils.mockGetMaxFeePerGas(network, 20n); - testUtils.mockGetMaxPriorityFeePerGas(network, 7n); + // mock getFeeData and hasLockAddressEnoughAssets + testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); testUtils.mockHasLockAddressEnoughAssets(evmChain, false); // mock PaymentTransaction diff --git a/packages/chains/evm/tests/TestUtils.ts b/packages/chains/evm/tests/TestUtils.ts index 067c37e..a7b848a 100644 --- a/packages/chains/evm/tests/TestUtils.ts +++ b/packages/chains/evm/tests/TestUtils.ts @@ -10,6 +10,7 @@ import { vi } from 'vitest'; import { AbstractEvmNetwork } from '../lib'; import TestEvmNetwork from './network/TestEvmNetwork'; import TestChain from './TestChain'; +import { FeeData } from 'ethers'; const spyOn = vi.spyOn; const observationTxConfirmation = 5; @@ -62,11 +63,8 @@ export const mockGetAddressBalanceForNativeToken = ( spyOn(network, 'getAddressBalanceForNativeToken').mockResolvedValue(value); }; -export const mockGetMaxFeePerGas = ( - network: AbstractEvmNetwork, - value: bigint -) => { - spyOn(network, 'getMaxFeePerGas').mockResolvedValue(value); +export const mockGetFeeData = (network: AbstractEvmNetwork, value: FeeData) => { + spyOn(network, 'getFeeData').mockResolvedValue(value); }; export const mockGetGasRequired = ( @@ -90,13 +88,6 @@ export const mockGetTransactionByNonce = ( spyOn(network, 'getTransactionByNonce').mockResolvedValue(value); }; -export const mockGetMaxPriorityFeePerGas = ( - network: AbstractEvmNetwork, - value: bigint -) => { - spyOn(network, 'getMaxPriorityFeePerGas').mockResolvedValue(value); -}; - export const generateChainObject = ( network: TestEvmNetwork, signFn: TssSignFunction = mockedSignFn diff --git a/packages/chains/evm/tests/network/TestEvmNetwork.ts b/packages/chains/evm/tests/network/TestEvmNetwork.ts index 763dd3d..54d2fed 100644 --- a/packages/chains/evm/tests/network/TestEvmNetwork.ts +++ b/packages/chains/evm/tests/network/TestEvmNetwork.ts @@ -1,4 +1,4 @@ -import { Transaction } from 'ethers'; +import { FeeData, Transaction } from 'ethers'; import { AbstractEvmNetwork, EvmTxStatus, TransactionHashes } from '../../lib'; import { BlockInfo, @@ -66,11 +66,7 @@ class TestEvmNetwork extends AbstractEvmNetwork { throw Error('Not mocked'); }; - getMaxPriorityFeePerGas = (): Promise => { - throw Error('Not mocked'); - }; - - getMaxFeePerGas = (): Promise => { + getFeeData = (): Promise => { throw Error('Not mocked'); }; diff --git a/packages/networks/evm-rpc/lib/EvmRpcNetwork.ts b/packages/networks/evm-rpc/lib/EvmRpcNetwork.ts index 5a94517..3347141 100644 --- a/packages/networks/evm-rpc/lib/EvmRpcNetwork.ts +++ b/packages/networks/evm-rpc/lib/EvmRpcNetwork.ts @@ -370,6 +370,29 @@ class EvmRpcNetwork extends AbstractEvmNetwork { return feeData.maxFeePerGas; }; + /** + * gets fee-related values associated with the network + * - the legacy gas price + * - the maximum fee to pay per gas + * - the additional amount to pay per gas to miner + * it includes all the values + * @returns fee-related values as bigint or null + */ + getFeeData = async (): Promise => { + const baseError = `Failed to get fee data from ${this.chain} RPC: `; + try { + const feeData = await this.provider.getFeeData(); + this.logger.debug( + `requested 'getFeeData' of ${ + this.chain + } RPC. res: ${JsonBigInt.stringify(feeData)}` + ); + return feeData; + } catch (e: unknown) { + throw new UnexpectedApiError(baseError + `${e}`); + } + }; + /** * gets the transaction status (mempool, succeed, failed) * Note: this function considers the hash as unsigned hash diff --git a/packages/networks/evm-rpc/tests/EvmRpcNetwork.spec.ts b/packages/networks/evm-rpc/tests/EvmRpcNetwork.spec.ts index a3175d7..dcc5e32 100644 --- a/packages/networks/evm-rpc/tests/EvmRpcNetwork.spec.ts +++ b/packages/networks/evm-rpc/tests/EvmRpcNetwork.spec.ts @@ -457,9 +457,9 @@ describe('EvmRpcNetwork', () => { }); }); - describe('getMaxPriorityFeePerGas', () => { + describe('getFeeData', () => { /** - * @target `EvmRpcNetwork.getMaxPriorityFeePerGas` should return max priority fee per gas successfully + * @target `EvmRpcNetwork.getFeeData` should return fee data successfully * @dependencies * @scenario * - mock provider.`getFeeData` to return address tx count @@ -468,36 +468,14 @@ describe('EvmRpcNetwork', () => { * @expected * - it should be mocked nonce */ - it('should return max priority fee per gas successfully', async () => { + it('should return fee data successfully', async () => { vi.spyOn(network.getProvider(), 'getFeeData').mockResolvedValue( testData.feeDataResponse ); - const result = await network.getMaxPriorityFeePerGas(); + const result = await network.getFeeData(); - expect(result).toEqual(testData.maxPriorityFeePerGas); - }); - }); - - describe('getMaxFeePerGas', () => { - /** - * @target `EvmRpcNetwork.getMaxFeePerGas` should return max fee per gas successfully - * @dependencies - * @scenario - * - mock provider.`getFeeData` to return address tx count - * - run test - * - check returned value - * @expected - * - it should be mocked nonce - */ - it('should return max fee per gas successfully', async () => { - vi.spyOn(network.getProvider(), 'getFeeData').mockResolvedValue( - testData.feeDataResponse - ); - - const result = await network.getMaxFeePerGas(); - - expect(result).toEqual(testData.maxFeePerGas); + expect(result).toEqual(testData.feeDataResponse); }); }); From 0611c2cb78b8fa6572adf5ca115d1ee85fece533 Mon Sep 17 00:00:00 2001 From: HFazelinia Date: Mon, 2 Dec 2024 11:41:25 +0000 Subject: [PATCH 2/5] refactor EVM transaction type and fix EvmChain tests --- packages/chains/binance/lib/BinanceChain.ts | 1 + packages/chains/ethereum/lib/EthereumChain.ts | 1 + packages/chains/evm/lib/EvmChain.ts | 165 ++++----- packages/chains/evm/tests/EvmChain.spec.ts | 345 ++++++++++++------ packages/chains/evm/tests/TestChain.ts | 6 +- packages/chains/evm/tests/TestUtils.ts | 12 +- packages/chains/evm/tests/testData.ts | 12 + 7 files changed, 332 insertions(+), 210 deletions(-) diff --git a/packages/chains/binance/lib/BinanceChain.ts b/packages/chains/binance/lib/BinanceChain.ts index 6b9e6e5..1318889 100644 --- a/packages/chains/binance/lib/BinanceChain.ts +++ b/packages/chains/binance/lib/BinanceChain.ts @@ -29,6 +29,7 @@ class BinanceChain extends EvmChain { signFunction, BINANCE_CHAIN, BNB, + 0, logger ); } diff --git a/packages/chains/ethereum/lib/EthereumChain.ts b/packages/chains/ethereum/lib/EthereumChain.ts index 30b5967..7c63b49 100644 --- a/packages/chains/ethereum/lib/EthereumChain.ts +++ b/packages/chains/ethereum/lib/EthereumChain.ts @@ -29,6 +29,7 @@ class EthereumChain extends EvmChain { signFunction, ETHEREUM_CHAIN, ETH, + 2, logger ); } diff --git a/packages/chains/evm/lib/EvmChain.ts b/packages/chains/evm/lib/EvmChain.ts index 8e1363d..f3ac319 100644 --- a/packages/chains/evm/lib/EvmChain.ts +++ b/packages/chains/evm/lib/EvmChain.ts @@ -31,6 +31,7 @@ abstract class EvmChain extends AbstractChain { declare network: AbstractEvmNetwork; declare configs: EvmConfigs; abstract CHAIN_ID: bigint; + evmTxType: number; extractor: EvmRosenExtractor | undefined; supportedTokens: Array; @@ -44,9 +45,11 @@ abstract class EvmChain extends AbstractChain { signFunction: TssSignFunction, CHAIN: string, NATIVE_TOKEN_ID: string, + evmTxType = 0, logger?: AbstractLogger ) { super(network, configs, tokens, logger); + this.evmTxType = evmTxType; this.supportedTokens = supportedTokens; this.signFunction = signFunction; this.extractor = new EvmRosenExtractor( @@ -131,24 +134,34 @@ abstract class EvmChain extends AbstractChain { const feeData = await this.network.getFeeData(); let requiredEth = 0n; let txFeeData; - if (feeData.maxFeePerGas !== null) { + if (this.evmTxType === 2) { + if ( + feeData.maxFeePerGas === null || + feeData.maxPriorityFeePerGas === null + ) { + throw new ImpossibleBehavior( + `Tx type is 2 but max fee variables are null` + ); + } requiredEth = this.configs.gasLimitCap * BigInt(orders.length) * feeData.maxFeePerGas; txFeeData = { maxFeePerGas: feeData.maxFeePerGas, maxPriorityFeePerGas: feeData.maxPriorityFeePerGas, }; - } else if (feeData.gasPrice !== null) { - requiredEth = - this.configs.gasLimitCap * BigInt(orders.length) * feeData.gasPrice; - txFeeData = { - gasPrice: feeData.gasPrice, - }; + } else if (this.evmTxType === 0) { + if (feeData.gasPrice === null) { + throw new ImpossibleBehavior(`Tx type is 0 but gas price is null`); + } else { + requiredEth = + this.configs.gasLimitCap * BigInt(orders.length) * feeData.gasPrice; + txFeeData = { + gasPrice: feeData.gasPrice, + }; + } } else { throw new ImpossibleBehavior( - `Both 'maxFeePerGas' and 'gasPrice' values are falsy: ${JsonBigInt.stringify( - feeData - )}` + `Type ${this.evmTxType} transaction is not supported in EvmChain` ); } @@ -188,7 +201,7 @@ abstract class EvmChain extends AbstractChain { this.CHAIN ).amount; trx = Transaction.from({ - type: 2, + type: this.evmTxType, to: singleOrder.address, nonce: nextNonce, data: '0x' + eventId, @@ -210,7 +223,7 @@ abstract class EvmChain extends AbstractChain { ); trx = Transaction.from({ - type: 2, + type: this.evmTxType, to: token.id, nonce: nextNonce, data: data + eventId, @@ -267,24 +280,29 @@ abstract class EvmChain extends AbstractChain { ); } - if (tx.type !== 2) { - throw new TransactionFormatError( - `Transaction [${transaction.txId}] is not of type 2` - ); - } - - if (tx.maxFeePerGas === null) { - throw new ImpossibleBehavior( - 'Type 2 transaction can not have null maxFeePerGas' - ); - } - const assets: AssetBalance = { nativeToken: 0n, tokens: [], }; - const networkFee = tx.maxFeePerGas * tx.gasLimit; + let networkFee = tx.gasLimit; + if (this.evmTxType === 2) { + if (tx.maxFeePerGas === null) + throw new ImpossibleBehavior( + "Type 2 transaction doesn't have null maxFeePerGas or maxPriorityFeePerGas" + ); + networkFee *= tx.maxFeePerGas; + } else if (this.evmTxType === 0) { + if (tx.gasPrice === null) + throw new ImpossibleBehavior( + "Type 0 transaction doesn't have null gasPrice" + ); + networkFee *= tx.gasPrice; + } else { + throw new ImpossibleBehavior( + `Type ${this.evmTxType} transaction is not supported in EvmChain` + ); + } assets.nativeToken = tx.value + networkFee; if (EvmUtils.isTransfer(tx.to, tx.data)) { @@ -375,10 +393,13 @@ abstract class EvmChain extends AbstractChain { /** * verifies transaction fee for a PaymentTransaction * - `to` shouldn't be null - * - transaction must of of type 2 + * - transaction type must be as expected * - gasLimit must be as expected - * - maxFeePerGas shouldn't be different than current network condition by more than slippage - * - maxPriorityFeePerGas shouldn't be different than current network condition by more than slippage + * - for type 2 transactions + * - maxFeePerGas shouldn't be different than current network condition by more than slippage + * - maxPriorityFeePerGas shouldn't be different than current network condition by more than slippage + * - for type 0 transactions + * - gasPrice shouldn't be different than current network condition by more than slippage * @param transaction the PaymentTransaction * @returns true if the transaction fee is verified */ @@ -403,25 +424,13 @@ abstract class EvmChain extends AbstractChain { return false; } - if (tx.type !== 2) { + if (tx.type !== this.evmTxType) { this.logger.warn( - `Tx [${transaction.txId}] is not verified: is not of type 2` + `Tx [${transaction.txId}] is not verified: expected type [${this.evmTxType}] found [${tx.type}]` ); return false; } - if (tx.maxFeePerGas === null) { - throw new ImpossibleBehavior( - "Type 2 transaction can't have null maxFeePerGas" - ); - } - - if (tx.maxPriorityFeePerGas === null) { - throw new ImpossibleBehavior( - "Type 2 transaction can't have null maxPriorityFeePerGas" - ); - } - // check gas limit let estimatedRequiredGas = await this.network.getGasRequired(tx); if (estimatedRequiredGas > this.configs.gasLimitCap) { @@ -448,10 +457,19 @@ abstract class EvmChain extends AbstractChain { // check fees const feeData = await this.network.getFeeData(); - if ( - feeData.maxFeePerGas !== null && - feeData.maxPriorityFeePerGas !== null - ) { + if (this.evmTxType === 2) { + if (tx.maxFeePerGas === null || tx.maxPriorityFeePerGas === null) + throw new ImpossibleBehavior( + "Type 2 transaction can't have null maxFeePerGas or maxPriorityFeePerGas" + ); + if ( + feeData.maxFeePerGas === null || + feeData.maxPriorityFeePerGas === null + ) + throw new ImpossibleBehavior( + 'Chain is using type 2 transactions but network is replying with null maxFeePerGas or maxPriorityFeePerGas' + ); + const networkMaxFee = feeData.maxFeePerGas; const maxFeeSlippage = (networkMaxFee * this.configs.gasPriceSlippage) / 100n; @@ -483,26 +501,15 @@ abstract class EvmChain extends AbstractChain { ); return false; } - - if (tx.gasPrice) { - this.logger.warn( - baseError + - `Transaction has gas price [${ - tx.gasPrice - }] while network support's max fee is defined [${JsonBigInt.stringify( - feeData - )}]` + } else if (this.evmTxType === 0) { + if (tx.gasPrice === null) + throw new ImpossibleBehavior( + "Type 0 transaction can't have null gasPrice" ); - return false; - } - } else if (feeData.gasPrice) { - if (!tx.gasPrice) { - this.logger.warn( - baseError + - `Expected transaction with gas price but found [${tx.gasPrice}]` + if (feeData.gasPrice === null) + throw new ImpossibleBehavior( + 'Chain is using type 0 transactions but network is replying with null gasPrice' ); - return false; - } const networkGasPrice = feeData.gasPrice; const gasPriceSlippage = @@ -521,9 +528,7 @@ abstract class EvmChain extends AbstractChain { } } else { throw new ImpossibleBehavior( - `Both 'maxFeePerGas' and 'gasPrice' values are falsy: ${JsonBigInt.stringify( - feeData - )}` + `Type ${this.evmTxType} transaction is not supported in EvmChain` ); } return true; @@ -647,7 +652,6 @@ abstract class EvmChain extends AbstractChain { /** * submits a transaction to the blockchain * checks the following conditions before: - * - transaction must of of type 2 * - fees are set appropriately according to the current network's condition * - lock address still have enough funds * @param transaction the transaction @@ -667,19 +671,11 @@ abstract class EvmChain extends AbstractChain { } try { - // check type - if (tx.type !== 2) { - this.logger.warn( - `Cannot submit transaction [${transaction.txId}]: Transaction is not of type 2` - ); - return; - } - // check fees const gasRequired = await this.network.getGasRequired(tx); if (gasRequired > tx.gasLimit) { this.logger.warn( - `Cannot submit transaction [${transaction.txId}]: Transaction gas limit [${tx.maxFeePerGas}] is less than the required gas [${gasRequired}]` + `Cannot submit transaction [${transaction.txId}]: Transaction gas limit [${tx.gasLimit}] is less than the required gas [${gasRequired}]` ); return; } @@ -756,7 +752,7 @@ abstract class EvmChain extends AbstractChain { /** * generates PaymentTransaction object from raw tx json string - * checks the transaction is of type 2 + * also checks the transaction type * @param rawTxJsonString * @returns PaymentTransaction object */ @@ -764,9 +760,9 @@ abstract class EvmChain extends AbstractChain { rawTxJsonString: string ): Promise => { const trx = Transaction.from(JSON.parse(rawTxJsonString)); - if (trx.type !== 2) { + if (trx.type !== this.evmTxType) { throw new TransactionFormatError( - `Only transaction of type 2 is supported while parsing raw transaction` + `Expected type [${this.evmTxType}] transaction while parsing raw transaction, found type [${trx.type}]` ); } const evmTx = new PaymentTransaction( @@ -787,7 +783,6 @@ abstract class EvmChain extends AbstractChain { * verifies additional conditions for a PaymentTransaction * - `to` shouldn't be null * - `data` shouldn't be null - * - transaction must be of type 2 * - `data` length must be either: * native-token transfer: 2 (0x) + eventId.length * erc-20 transfer: 2 (0x) + 136 (`transfer` data) + eventId.length @@ -825,14 +820,6 @@ abstract class EvmChain extends AbstractChain { const eidlen = transaction.eventId.length; - // only type 2 transactions are allowed - if (tx.type !== 2) { - this.logger.warn( - `Tx [${transaction.txId}] is not verified. It is not of type 2` - ); - return false; - } - // tx data must have correct length if (![eidlen + 2, eidlen + 2 + 136].includes(tx.data.length)) { this.logger.warn( diff --git a/packages/chains/evm/tests/EvmChain.spec.ts b/packages/chains/evm/tests/EvmChain.spec.ts index fe18b25..c149da2 100644 --- a/packages/chains/evm/tests/EvmChain.spec.ts +++ b/packages/chains/evm/tests/EvmChain.spec.ts @@ -1,7 +1,6 @@ import { expect, vi } from 'vitest'; import { AssetNotSupportedError, - MaxParallelTxError, NotEnoughAssetsError, PaymentTransaction, SigningStatus, @@ -18,10 +17,6 @@ import { EvmTxStatus } from '../lib'; import TestChain from './TestChain'; describe('EvmChain', () => { - const network = new TestEvmNetwork(); - - const evmChain = testUtils.generateChainObject(network); - describe(`constructor`, () => { /** * @target EvmChain.constructor should initialize rosen-extractor successfully @@ -33,12 +28,14 @@ describe('EvmChain', () => { * - CHAIN variable should be the same as the one defined EvmChain */ it('should initialize rosen-extractor successfully', async () => { + const network = new TestEvmNetwork(); const chain = new TestChain( network, testUtils.configs, TestData.testTokenMap, TestData.supportedTokens, - testUtils.mockedSignFn + testUtils.mockedSignFn, + 2 ); expect(chain.extractor?.chain).toEqual(chain.CHAIN); }); @@ -87,9 +84,11 @@ describe('EvmChain', () => { // mock hasLockAddressEnoughAssets, getFeeData, // getGasRequired, getAddressNextNonce + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network); const requiredGas = 100000n; testUtils.mockHasLockAddressEnoughAssets(evmChain, true); - testUtils.mockGetFeeData(network, new FeeData(1n, 10n, 10n)); + testUtils.mockGetFeeData(network, new FeeData(10n, 10n, 10n)); testUtils.mockGetGasRequired(network, requiredGas); testUtils.mockGetAddressNextAvailableNonce(network, nonce); @@ -158,6 +157,7 @@ describe('EvmChain', () => { * - transaction must be of type 2 and has no blobs * - nonce must be the same as the next available nonce * - gas limit should be as expected + * - maxFeePerGas and maxPriorityFeePerGas should be as expected */ it('should generate payment transaction successfully for single order', async () => { const order = TestData.nativePaymentOrder; @@ -167,9 +167,11 @@ describe('EvmChain', () => { // mock hasLockAddressEnoughAssets, getFeeData, // getGasRequired, getAddressNextNonce + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network); const requiredGas = 21000n; testUtils.mockHasLockAddressEnoughAssets(evmChain, true); - testUtils.mockGetFeeData(network, new FeeData(1n, 10n, 10n)); + testUtils.mockGetFeeData(network, new FeeData(10n, 10n, 10n)); testUtils.mockGetGasRequired(network, requiredGas); testUtils.mockGetAddressNextAvailableNonce(network, nonce); @@ -210,6 +212,10 @@ describe('EvmChain', () => { // check gas limit expect(tx.gasLimit).toEqual(requiredGas * 3n); // requiredGas * gasLimitMultiplier + + // check maxFeePerGas and maxPriorityFeePerGas + expect(tx.maxFeePerGas).toEqual(10n); + expect(tx.maxPriorityFeePerGas).toEqual(10n); }); /** @@ -240,8 +246,10 @@ describe('EvmChain', () => { // mock hasLockAddressEnoughAssets, getFeeData, // getGasRequired, getAddressNextNonce + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network); testUtils.mockHasLockAddressEnoughAssets(evmChain, true); - testUtils.mockGetFeeData(network, new FeeData(1n, 10n, 10n)); + testUtils.mockGetFeeData(network, new FeeData(10n, 10n, 10n)); testUtils.mockGetGasRequired(network, 200000n); testUtils.mockGetAddressNextAvailableNonce(network, nonce); @@ -301,6 +309,8 @@ describe('EvmChain', () => { const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; const txType = TransactionType.payment; // run test and expect error + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network); await expect(async () => { await evmChain.generateMultipleTransactions( eventId, @@ -317,7 +327,7 @@ describe('EvmChain', () => { * when lock address does not have enough assets * @dependencies * @scenario - * - mock hasLockAddressEnoughAssets + * - mock hasLockAddressEnoughAssets and getAddressNextNonce * - call the function * @expected * - throw NotEnoughAssetsError @@ -326,9 +336,14 @@ describe('EvmChain', () => { const order = TestData.multipleOrders; const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; const txType = TransactionType.payment; + const nonce = 54; - // mock hasLockAddressEnoughAssets + // mock hasLockAddressEnoughAssets and getAddressNextNonce + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network); testUtils.mockHasLockAddressEnoughAssets(evmChain, false); + testUtils.mockGetFeeData(network, new FeeData(10n, 10n, 10n)); + testUtils.mockGetAddressNextAvailableNonce(network, nonce); // run test and expect error await expect(async () => { @@ -384,9 +399,11 @@ describe('EvmChain', () => { // mock hasLockAddressEnoughAssets, getFeeData, // getGasRequired, getAddressNextNonce + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network); const requiredGas = 21000n; testUtils.mockHasLockAddressEnoughAssets(evmChain, true); - testUtils.mockGetFeeData(network, new FeeData(1n, 10n, 10n)); + testUtils.mockGetFeeData(network, new FeeData(10n, 10n, 10n)); testUtils.mockGetGasRequired(network, requiredGas); testUtils.mockGetAddressNextAvailableNonce(network, nonce); @@ -460,7 +477,11 @@ describe('EvmChain', () => { ); // mock hasLockAddressEnoughAssets, getAddressNextNonce, + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network); testUtils.mockHasLockAddressEnoughAssets(evmChain, true); + testUtils.mockGetFeeData(network, new FeeData(10n, 10n, 10n)); + testUtils.mockGetGasRequired(network, 200000n); testUtils.mockGetAddressNextAvailableNonce(network, 53); // run test and expect no error @@ -496,6 +517,7 @@ describe('EvmChain', () => { * - nonce must be the same as the next available nonce */ it('should generate payment transaction successfully for wrapped order', async () => { + const network = new TestEvmNetwork(); const evmChain = testUtils.generateChainObjectWithMultiDecimalTokenMap(network); @@ -507,7 +529,7 @@ describe('EvmChain', () => { // mock hasLockAddressEnoughAssets, getMaxFeePerGas, // getGasRequired, getAddressNextNonce, getMaxPriorityFeePerGas testUtils.mockHasLockAddressEnoughAssets(evmChain, true); - testUtils.mockGetFeeData(network, new FeeData(1n, 10n, 10n)); + testUtils.mockGetFeeData(network, new FeeData(10n, 10n, 10n)); testUtils.mockGetGasRequired(network, 21000n); testUtils.mockGetAddressNextAvailableNonce(network, nonce); @@ -546,9 +568,91 @@ describe('EvmChain', () => { // check nonce expect(tx.nonce).toEqual(nonce); }); + + /** + * @target EvmChain.generateMultipleTransactions should generate + * type 0 transaction successfully for single order + * @dependencies + * @scenario + * - mock hasLockAddressEnoughAssets, getFeeData + * - mock getGasRequired, getAddressNextNonce + * - call the function + * - check returned value + * @expected + * - PaymentTransaction txType, eventId and network should be as + * expected + * - extracted order of generated transaction should be the same as input + * order + * - eventId should be properly in the transaction data + * - no extra data should be found in the transaction data + * - transaction must be of type 2 and has no blobs + * - nonce must be the same as the next available nonce + * - gas limit should be as expected + * - gas price should be as expected + */ + it('should generate type 0 transaction successfully for single order', async () => { + const order = TestData.nativePaymentOrder; + const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; + const txType = TransactionType.payment; + const nonce = 54; + + // mock hasLockAddressEnoughAssets, getFeeData, + // getGasRequired, getAddressNextNonce + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network, undefined, 0); + const requiredGas = 21000n; + testUtils.mockHasLockAddressEnoughAssets(evmChain, true); + testUtils.mockGetFeeData(network, new FeeData(10n, null, null)); + testUtils.mockGetGasRequired(network, requiredGas); + testUtils.mockGetAddressNextAvailableNonce(network, nonce); + + // run test + const evmTx = await evmChain.generateMultipleTransactions( + eventId, + txType, + order, + [], + [] + ); + + // check returned value + expect(evmTx[0].txType).toEqual(txType); + expect(evmTx[0].eventId).toEqual(eventId); + expect(evmTx[0].network).toEqual(evmChain.CHAIN); + + // extracted order of generated transaction should be the same as input order + const extractedOrder = evmChain.extractTransactionOrder(evmTx[0]); + expect(extractedOrder).toEqual(order); + + const tx = Serializer.deserialize(evmTx[0].txBytes); + + // check eventId encoded at the end of the data + expect(tx.data.substring(2, 34)).toEqual(eventId); + + // check there is no more data + expect(tx.data.length).toEqual(34); + + // check transaction type + expect(tx.type).toEqual(0); + + // check blobs zero + expect(tx.maxFeePerBlobGas).toEqual(null); + + // check nonce + expect(tx.nonce).toEqual(nonce); + + // check gas limit + expect(tx.gasLimit).toEqual(requiredGas * 3n); // requiredGas * gasLimitMultiplier + + // check gas price + expect(tx.gasPrice).toEqual(10n); + }); }); describe('rawTxToPaymentTransaction', () => { + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network); + /** * @target EvmChain.rawTxToPaymentTransaction should construct transaction successfully * @dependencies @@ -574,7 +678,7 @@ describe('EvmChain', () => { /** * @target EvmChain.rawTxToPaymentTransaction should throw error when transaction - * is not of type 2 + * type is unexpected * @dependencies * @scenario * - mock invalid transaction @@ -582,7 +686,8 @@ describe('EvmChain', () => { * @expected * - throw TransactionFormatError */ - it('should throw error when transaction is not of type 2', async () => { + it('should throw error when transaction type is unexpected', async () => { + const evmChain = testUtils.generateChainObject(network, undefined, 0); expect(async () => { await evmChain.rawTxToPaymentTransaction( TestData.transaction1JsonString @@ -592,6 +697,9 @@ describe('EvmChain', () => { }); describe('getTransactionAssets', () => { + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network); + /** * @target EvmChain.getTransactionAssets should get transaction assets * successfully when there is ERC-20 token transfer. @@ -694,24 +802,26 @@ describe('EvmChain', () => { }); /** - * @target EvmChain.getTransactionAssets should throw error when transaction is not of type 2 + * @target EvmChain.getTransactionAssets should get transaction assets + * successfully for type 0 transactions * @dependencies * @scenario * - mock PaymentTransaction * - call the function + * - check returned value * @expected - * - throw error + * - it should return mocked transaction assets (both input and output assets) */ - it('should throw error when transaction is not of type 2', async () => { + it('should get transaction assets successfully for type 0 transactions', async () => { + const evmChain = testUtils.generateChainObject(network, undefined, 0); const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; const txType = TransactionType.payment; // mock PaymentTransaction - const trx = { ...TestData.transaction1Json }; + const trx = { ...TestData.transaction1WithType0Json }; trx.data = '0x'; const tx = Transaction.from(trx); const assets = { ...TestData.transaction1Assets }; assets.tokens = []; - tx.type = 3; const paymentTx = new PaymentTransaction( evmChain.CHAIN, tx.unsignedHash, @@ -720,10 +830,12 @@ describe('EvmChain', () => { txType ); - // run test function and expect error - expect(async () => { - await evmChain.getTransactionAssets(paymentTx); - }).rejects.toThrowError(TransactionFormatError); + // check returned value + const result = await evmChain.getTransactionAssets(paymentTx); + + // check returned value + expect(result.inputAssets).toEqual(assets); + expect(result.outputAssets).toEqual(assets); }); /** @@ -738,6 +850,7 @@ describe('EvmChain', () => { * - it should return mocked transaction assets (both input and output assets) */ it('should wrap transaction assets successfully', async () => { + const network = new TestEvmNetwork(); const evmChain = testUtils.generateChainObjectWithMultiDecimalTokenMap(network); @@ -781,10 +894,12 @@ describe('EvmChain', () => { */ it('should return true when both fees and gasLimit are set properly', async () => { // mock a config that has almost the same fee as the mocked transaction + const network = new TestEvmNetwork(); testUtils.mockGetGasRequired(network, 100000n); - testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); + testUtils.mockGetFeeData(network, new FeeData(20n, 20n, 7n)); // mock PaymentTransaction + const evmChain = testUtils.generateChainObject(network); const tx = Transaction.from(TestData.transaction1Json); tx.gasLimit = 85000n * evmChain.configs.gasLimitMultiplier; tx.maxFeePerGas = 22n; @@ -820,10 +935,12 @@ describe('EvmChain', () => { */ it('should return false when `to` is null', async () => { // mock a config that has almost the same fee as the transaction fee + const network = new TestEvmNetwork(); testUtils.mockGetGasRequired(network, 100000n); - testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); + testUtils.mockGetFeeData(network, new FeeData(20n, 20n, 7n)); // mock PaymentTransaction + const evmChain = testUtils.generateChainObject(network); const tx = Transaction.from(TestData.transaction1Json); tx.gasLimit = 100000n * evmChain.configs.gasLimitMultiplier; tx.maxFeePerGas = 22n; @@ -849,7 +966,7 @@ describe('EvmChain', () => { }); /** - * @target EvmChain.verifyTransactionFee should return false when transaction is not of type 2 + * @target EvmChain.verifyTransactionFee should return false when transaction type is unexpected * @dependencies * @scenario * - mock mockGetGasRequired @@ -858,12 +975,14 @@ describe('EvmChain', () => { * @expected * - return false */ - it('should return false when transaction is not of type 2', async () => { + it('should return false when transaction type is unexpected', async () => { // mock a config that has almost the same fee as the transaction fee + const network = new TestEvmNetwork(); testUtils.mockGetGasRequired(network, 55000n); - testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); + testUtils.mockGetFeeData(network, new FeeData(20n, 20n, 7n)); // mock PaymentTransaction + const evmChain = testUtils.generateChainObject(network, undefined, 0); const tx = Transaction.from(TestData.transaction1Json); tx.gasLimit = 55000n * evmChain.configs.gasLimitMultiplier; tx.maxFeePerGas = 22n; @@ -903,10 +1022,12 @@ describe('EvmChain', () => { it('should return false when gasLimit is wrong for erc-20 transfer', async () => { // mock a config that has more fee and wrong required gas // comparing to the mocked transaction + const network = new TestEvmNetwork(); testUtils.mockGetGasRequired(network, 90000n); - testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); + testUtils.mockGetFeeData(network, new FeeData(20n, 20n, 7n)); // mock PaymentTransaction + const evmChain = testUtils.generateChainObject(network); const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; const txType = TransactionType.payment; const tx = Transaction.from(TestData.transaction1Json); @@ -945,10 +1066,12 @@ describe('EvmChain', () => { it('should return false when gasLimit is over the cap for erc-20 transfer', async () => { // mock a config that has more fee and wrong required gas // comparing to the mocked transaction + const network = new TestEvmNetwork(); testUtils.mockGetGasRequired(network, 90000n); - testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); + testUtils.mockGetFeeData(network, new FeeData(20n, 20n, 7n)); // mock PaymentTransaction + const evmChain = testUtils.generateChainObject(network); const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; const txType = TransactionType.payment; const tx = Transaction.from(TestData.transaction1Json); @@ -987,10 +1110,12 @@ describe('EvmChain', () => { it('should return false when gasLimit is wrong for native-token transfer', async () => { // mock a config that has more fee and wrong required gas // comparing to the mocked transaction + const network = new TestEvmNetwork(); testUtils.mockGetGasRequired(network, 20000n); - testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); + testUtils.mockGetFeeData(network, new FeeData(20n, 20n, 7n)); // mock PaymentTransaction + const evmChain = testUtils.generateChainObject(network); const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; const txType = TransactionType.payment; const tx = Transaction.from(TestData.transaction1Json); @@ -1029,10 +1154,12 @@ describe('EvmChain', () => { */ it('should return false when maxFeePerGas is too much bigger than expected', async () => { // mock a config that has too much bigger max fee comparing to the mocked transaction + const network = new TestEvmNetwork(); testUtils.mockGetGasRequired(network, 76000n); - testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); + testUtils.mockGetFeeData(network, new FeeData(20n, 20n, 7n)); // mock PaymentTransaction + const evmChain = testUtils.generateChainObject(network); const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; const txType = TransactionType.payment; const tx = Transaction.from(TestData.transaction1Json); @@ -1070,10 +1197,12 @@ describe('EvmChain', () => { */ it('should return false when maxFeePerGas is too much smaller than exptected', async () => { // mock a config that has too much smaller max fee comparing to the mocked transaction + const network = new TestEvmNetwork(); testUtils.mockGetGasRequired(network, 76000n); - testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); + testUtils.mockGetFeeData(network, new FeeData(20n, 20n, 7n)); // mock PaymentTransaction + const evmChain = testUtils.generateChainObject(network); const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; const txType = TransactionType.payment; const tx = Transaction.from(TestData.transaction1Json); @@ -1111,10 +1240,12 @@ describe('EvmChain', () => { */ it('should return false when maxPriorityFeePerGas is too much bigger than expected', async () => { // mock a config that has too much bigger max fee comparing to the mocked transaction + const network = new TestEvmNetwork(); testUtils.mockGetGasRequired(network, 76000n); - testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); + testUtils.mockGetFeeData(network, new FeeData(20n, 20n, 7n)); // mock PaymentTransaction + const evmChain = testUtils.generateChainObject(network); const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; const txType = TransactionType.payment; const tx = Transaction.from(TestData.transaction1Json); @@ -1152,10 +1283,12 @@ describe('EvmChain', () => { */ it('should return false when maxPriorityFeePerGas is too much smaller than expected', async () => { // mock a config that has too much bigger max fee comparing to the mocked transaction + const network = new TestEvmNetwork(); testUtils.mockGetGasRequired(network, 76000n); - testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); + testUtils.mockGetFeeData(network, new FeeData(20n, 20n, 7n)); // mock PaymentTransaction + const evmChain = testUtils.generateChainObject(network); const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; const txType = TransactionType.payment; const tx = Transaction.from(TestData.transaction1Json); @@ -1181,6 +1314,9 @@ describe('EvmChain', () => { }); describe('extractTransactionOrder', () => { + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network); + /** * @target EvmChain.extractTransactionOrder should extract transaction * order successfully for ERC-20 token transfer only @@ -1440,18 +1576,23 @@ describe('EvmChain', () => { * when transaction is of type 2, fees are set properly and lock address has enough assets * @dependencies * @scenario + * - mock network functions + * - getFeeData + * - hasLockAddressEnoughAssets + * - getGasRequired * - mock valid PaymentTransaction - * - mock getFeeData - * - mock hasLockAddressEnoughAssets * - run test * - check function is called * @expected * - it should call the function */ it('should submit the transaction when transaction is of type 2, fees are set properly and lock address has enough assets', async () => { - // mock getFeeData and hasLockAddressEnoughAssets - testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); + // mock network functions + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network); + testUtils.mockGetFeeData(network, new FeeData(20n, 20n, 7n)); testUtils.mockHasLockAddressEnoughAssets(evmChain, true); + testUtils.mockGetGasRequired(network, 70000n); // mock PaymentTransaction const tx = Transaction.from(TestData.transaction1Json); @@ -1475,53 +1616,27 @@ describe('EvmChain', () => { expect(submitTransactionSpy).toHaveBeenCalled(); }); - /** - * @target EvmChain.submitTransaction should not submit the transaction - * when transaction is not of type 2 - * @dependencies - * @scenario - * - mock invalid PaymentTransaction - * - run test - * - check function is not called - * @expected - * - it should not call the function - */ - it('should submit the transaction when transaction is not of type 2', async () => { - // mock PaymentTransaction - const tx = Transaction.from(TestData.transaction1Json); - tx.gasLimit = 55000n + 21000n; - tx.type = 3; - - const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; - const txType = TransactionType.payment; - const paymentTx = new PaymentTransaction( - evmChain.CHAIN, - tx.unsignedHash, - eventId, - Serializer.serialize(tx), - txType - ); - const submitTransactionSpy = vi.spyOn(network, 'submitTransaction'); - submitTransactionSpy.mockImplementation(async () => undefined); - await evmChain.submitTransaction(paymentTx); - expect(submitTransactionSpy).not.toHaveBeenCalled(); - }); - /** * @target EvmChain.submitTransaction should not submit the transaction * when gasLimit is wrong * @dependencies * @scenario + * - mock network functions + * - getFeeData + * - hasLockAddressEnoughAssets + * - getGasRequired * - mock invalid PaymentTransaction - * - mock getGasRequired - * - mock hasLockAddressEnoughAssets * - run test * - check function is not called * @expected * - it should not call the function */ it('should not submit the transaction when gasLimit is wrong', async () => { - // mock getGasRequired, and hasLockAddressEnoughAssets + // mock network functions + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network); + testUtils.mockGetFeeData(network, new FeeData(20n, 20n, 7n)); + testUtils.mockHasLockAddressEnoughAssets(evmChain, true); testUtils.mockGetGasRequired(network, 77000n); // mock PaymentTransaction @@ -1551,18 +1666,23 @@ describe('EvmChain', () => { * when lock address does not have enough asset * @dependencies * @scenario + * - mock network functions + * - getFeeData + * - hasLockAddressEnoughAssets + * - getGasRequired * - mock invalid PaymentTransaction - * - mock getFeeData - * - mock hasLockAddressEnoughAssets * - run test * - check the function is not called * @expected * - it should not call the function */ it('should not submit the transaction when lock address does not have enough asset', async () => { - // mock getFeeData and hasLockAddressEnoughAssets - testUtils.mockGetFeeData(network, new FeeData(1n, 20n, 7n)); + // mock network functions + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network); + testUtils.mockGetFeeData(network, new FeeData(20n, 20n, 7n)); testUtils.mockHasLockAddressEnoughAssets(evmChain, false); + testUtils.mockGetGasRequired(network, 70000n); // mock PaymentTransaction const tx = Transaction.from(TestData.transaction1Json); @@ -1591,15 +1711,22 @@ describe('EvmChain', () => { * when checking failed due to error * @dependencies * @scenario + * - mock network functions + * - getFeeData + * - hasLockAddressEnoughAssets + * - getGasRequired to throw error * - mock invalid PaymentTransaction - * - mock getGasRequired to throw error * - run test * - check function is not called * @expected * - it should not call the function */ it('should not submit the transaction when checking failed due to error', async () => { - // mock getGasRequired + // mock network functions + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network); + testUtils.mockGetFeeData(network, new FeeData(20n, 20n, 7n)); + testUtils.mockHasLockAddressEnoughAssets(evmChain, false); vi.spyOn(network, 'getGasRequired').mockImplementation(() => { throw Error(`test Error`); }); @@ -1628,6 +1755,9 @@ describe('EvmChain', () => { }); describe('verifyTransactionExtraConditions', () => { + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network); + /** * @target EvmChain.verifyTransactionExtraConditions should return true * for erc-20 transfer when extra conditions are met and eventId is not empty @@ -1659,39 +1789,6 @@ describe('EvmChain', () => { expect(result).toEqual(true); }); - /** - * @target EvmChain.verifyTransactionExtraConditions should return false - * when transaction is not of type 2 - * @dependencies - * @scenario - * - mock valid PaymentTransaction - * - run test - * - check returned value - * @expected - * - it should return false - */ - it('should return false when transaction is not of type 2', async () => { - // mock PaymentTransaction - const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; - const txType = TransactionType.payment; - const tx = Transaction.from(TestData.erc20transaction as TransactionLike); - tx.type = 3; - - const paymentTx = new PaymentTransaction( - evmChain.CHAIN, - tx.unsignedHash, - eventId, - Serializer.serialize(tx), - txType - ); - - // run test - const result = evmChain.verifyTransactionExtraConditions(paymentTx); - - // check returned value - expect(result).toEqual(false); - }); - /** * @target EvmChain.verifyTransactionExtraConditions should return true * for erc-20 transfer when extra conditions are met and eventId is empty @@ -2049,6 +2146,8 @@ describe('EvmChain', () => { */ it('should return true when tx is not found and nonce is not used', async () => { // mock PaymentTransaction + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network); const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; const txType = TransactionType.payment; const tx = Transaction.from(TestData.erc20transaction as TransactionLike); @@ -2093,6 +2192,8 @@ describe('EvmChain', () => { */ it('should return false when nonce is already used by another transaction', async () => { // mock PaymentTransaction + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network); const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; const txType = TransactionType.payment; const tx = Transaction.from(TestData.erc20transaction as TransactionLike); @@ -2147,6 +2248,8 @@ describe('EvmChain', () => { */ it('should return true when nonce is used by current transaction', async () => { // mock PaymentTransaction + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network); const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; const txType = TransactionType.payment; const tx = Transaction.from(TestData.erc20transaction as TransactionLike); @@ -2196,6 +2299,8 @@ describe('EvmChain', () => { */ it('should return false when tx is failed', async () => { // mock PaymentTransaction + const network = new TestEvmNetwork(); + const evmChain = testUtils.generateChainObject(network); const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; const txType = TransactionType.payment; const tx = Transaction.from(TestData.erc20transaction as TransactionLike); @@ -2229,6 +2334,8 @@ describe('EvmChain', () => { }); describe('signTransaction', () => { + const network = new TestEvmNetwork(); + /** * @target EvmChain.signTransaction should return PaymentTransaction of the * signed transaction @@ -2339,7 +2446,8 @@ describe('EvmChain', () => { * - it should return mocked address assets (both input and output assets) */ it('should get address assets successfully', async () => { - mockGetAddressBalanceForNativeToken(evmChain.network, 1000n); + const network = new TestEvmNetwork(); + mockGetAddressBalanceForNativeToken(network, 1000n); vi.spyOn(network, 'getAddressBalanceForERC20Asset').mockImplementation( async (address, tokenId) => { if (tokenId === '0xedee4752e5a2f595151c94762fb38e5730357785') @@ -2355,6 +2463,7 @@ describe('EvmChain', () => { ); // run test + const evmChain = testUtils.generateChainObject(network); const result = await evmChain.getAddressAssets(TestData.lockAddress); // check returned value @@ -2380,6 +2489,7 @@ describe('EvmChain', () => { */ it('should return 0 balance with no token if address is empty', async () => { // run test + const evmChain = testUtils.generateChainObject(new TestEvmNetwork()); const result = await evmChain.getAddressAssets(''); // check returned value @@ -2398,6 +2508,7 @@ describe('EvmChain', () => { * - it should return mocked address assets (both input and output assets) */ it('should wrap address assets successfully', async () => { + const network = new TestEvmNetwork(); const evmChain = testUtils.generateChainObjectWithMultiDecimalTokenMap(network); @@ -2450,9 +2561,11 @@ describe('EvmChain', () => { const tx = Transaction.from(TestData.erc20transaction as TransactionLike); // mock getTransactionStatus + const network = new TestEvmNetwork(); testUtils.mockGetTransactionStatus(network, EvmTxStatus.succeed); // run test + const evmChain = testUtils.generateChainObject(network); const result = await evmChain.verifyLockTransactionExtraConditions( tx, {} as any @@ -2479,9 +2592,11 @@ describe('EvmChain', () => { const tx = Transaction.from(TestData.erc20transaction as TransactionLike); // mock getTransactionStatus + const network = new TestEvmNetwork(); testUtils.mockGetTransactionStatus(network, EvmTxStatus.failed); // run test + const evmChain = testUtils.generateChainObject(network); const result = await evmChain.verifyLockTransactionExtraConditions( tx, {} as any diff --git a/packages/chains/evm/tests/TestChain.ts b/packages/chains/evm/tests/TestChain.ts index 414d368..14d8b32 100644 --- a/packages/chains/evm/tests/TestChain.ts +++ b/packages/chains/evm/tests/TestChain.ts @@ -11,7 +11,8 @@ class TestChain extends EvmChain { configs: any, tokens: RosenTokens, supportedTokens: Array, - signFunction: TssSignFunction + signFunction: TssSignFunction, + evmTxType: number ) { super( network, @@ -20,7 +21,8 @@ class TestChain extends EvmChain { supportedTokens, signFunction, 'test', - 'test-native-token' + 'test-native-token', + evmTxType ); } } diff --git a/packages/chains/evm/tests/TestUtils.ts b/packages/chains/evm/tests/TestUtils.ts index a7b848a..8d81534 100644 --- a/packages/chains/evm/tests/TestUtils.ts +++ b/packages/chains/evm/tests/TestUtils.ts @@ -90,27 +90,31 @@ export const mockGetTransactionByNonce = ( export const generateChainObject = ( network: TestEvmNetwork, - signFn: TssSignFunction = mockedSignFn + signFn: TssSignFunction = mockedSignFn, + evmTxType = 2 ) => { return new TestChain( network, configs, testData.testTokenMap, testData.supportedTokens, - signFn + signFn, + evmTxType ); }; export const generateChainObjectWithMultiDecimalTokenMap = ( network: TestEvmNetwork, - signFn: TssSignFunction = mockedSignFn + signFn: TssSignFunction = mockedSignFn, + evmTxType = 2 ) => { return new TestChain( network, configs, testData.multiDecimalTokenMap, testData.supportedTokens, - signFn + signFn, + evmTxType ); }; diff --git a/packages/chains/evm/tests/testData.ts b/packages/chains/evm/tests/testData.ts index c6d3d47..af7a3bd 100644 --- a/packages/chains/evm/tests/testData.ts +++ b/packages/chains/evm/tests/testData.ts @@ -92,6 +92,18 @@ export const transaction1Json = { accessList: [], }; +export const transaction1WithType0Json = { + type: 0, + to: '0xeDee4752e5a2F595151c94762fB38e5730357785', + data: '0xa9059cbb0000000000000000000000004f0d2dde80b45e24ad4019a5aabd6c23aff2842b00000000000000000000000000000000000000000000000000000000e319aa30bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb', + nonce: 10, + gasLimit: '21000', + gasPrice: '48978500000', + value: 0, + chainId: 1, + sig: null, +}; + export const erc20transaction = { type: 2, to: '0xeDee4752e5a2F595151c94762fB38e5730357785', From b56f7eafb64621dbeb9636ccc403039d1c3d4fa0 Mon Sep 17 00:00:00 2001 From: HFazelinia Date: Mon, 2 Dec 2024 12:02:02 +0000 Subject: [PATCH 3/5] update changelog and add more tests --- .changeset/dull-hotels-trade.md | 2 +- packages/chains/evm/lib/EvmChain.ts | 8 +- packages/chains/evm/tests/EvmChain.spec.ts | 127 ++++++++++++++++++++- 3 files changed, 131 insertions(+), 6 deletions(-) diff --git a/.changeset/dull-hotels-trade.md b/.changeset/dull-hotels-trade.md index 6b7e71e..aad1174 100644 --- a/.changeset/dull-hotels-trade.md +++ b/.changeset/dull-hotels-trade.md @@ -3,4 +3,4 @@ --- Replace getMaxFeePerGas and getMaxPriorityFeePerGas functions with getFeeData function in AbstractEvmNetwork -Use gas price instead of getMaxFee variables in tx generation and verification only if maxFee variables are not defined in the network +Support both type 0 and type 2 transactions diff --git a/packages/chains/evm/lib/EvmChain.ts b/packages/chains/evm/lib/EvmChain.ts index f3ac319..1187808 100644 --- a/packages/chains/evm/lib/EvmChain.ts +++ b/packages/chains/evm/lib/EvmChain.ts @@ -286,13 +286,13 @@ abstract class EvmChain extends AbstractChain { }; let networkFee = tx.gasLimit; - if (this.evmTxType === 2) { + if (tx.type === 2) { if (tx.maxFeePerGas === null) throw new ImpossibleBehavior( "Type 2 transaction doesn't have null maxFeePerGas or maxPriorityFeePerGas" ); networkFee *= tx.maxFeePerGas; - } else if (this.evmTxType === 0) { + } else if (tx.type === 0) { if (tx.gasPrice === null) throw new ImpossibleBehavior( "Type 0 transaction doesn't have null gasPrice" @@ -457,7 +457,7 @@ abstract class EvmChain extends AbstractChain { // check fees const feeData = await this.network.getFeeData(); - if (this.evmTxType === 2) { + if (tx.type === 2) { if (tx.maxFeePerGas === null || tx.maxPriorityFeePerGas === null) throw new ImpossibleBehavior( "Type 2 transaction can't have null maxFeePerGas or maxPriorityFeePerGas" @@ -501,7 +501,7 @@ abstract class EvmChain extends AbstractChain { ); return false; } - } else if (this.evmTxType === 0) { + } else if (tx.type === 0) { if (tx.gasPrice === null) throw new ImpossibleBehavior( "Type 0 transaction can't have null gasPrice" diff --git a/packages/chains/evm/tests/EvmChain.spec.ts b/packages/chains/evm/tests/EvmChain.spec.ts index c149da2..ed43654 100644 --- a/packages/chains/evm/tests/EvmChain.spec.ts +++ b/packages/chains/evm/tests/EvmChain.spec.ts @@ -1195,7 +1195,7 @@ describe('EvmChain', () => { * @expected * - it should return false */ - it('should return false when maxFeePerGas is too much smaller than exptected', async () => { + it('should return false when maxFeePerGas is too much smaller than expected', async () => { // mock a config that has too much smaller max fee comparing to the mocked transaction const network = new TestEvmNetwork(); testUtils.mockGetGasRequired(network, 76000n); @@ -1308,6 +1308,131 @@ describe('EvmChain', () => { // run test const result = await evmChain.verifyTransactionFee(paymentTx); + // check returned value + expect(result).toEqual(false); + }); + /** + * @target EvmChain.verifyTransactionFee should return true when + * gasPrice and gasLimit are set properly for a type 0 transaction + * @dependencies + * @scenario + * - mock mockGetGasRequired + * - mock getFeeData + * - mock PaymentTransaction + * - check returned value + * @expected + * - it should return true + */ + it('should return true when gasPrice and gasLimit are set properly for a type 0 transaction', async () => { + // mock a config that has almost the same fee as the mocked transaction + const network = new TestEvmNetwork(); + testUtils.mockGetGasRequired(network, 100000n); + testUtils.mockGetFeeData(network, new FeeData(20n, null, null)); + + // mock PaymentTransaction + const evmChain = testUtils.generateChainObject(network, undefined, 0); + const tx = Transaction.from(TestData.transaction1WithType0Json); + tx.gasLimit = 85000n * evmChain.configs.gasLimitMultiplier; + tx.gasPrice = 22n; + tx.value = 2n; + + const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; + const txType = TransactionType.payment; + const paymentTx = new PaymentTransaction( + evmChain.CHAIN, + tx.unsignedHash, + eventId, + Serializer.serialize(tx), + txType + ); + + // run test + const result = await evmChain.verifyTransactionFee(paymentTx); + + // check returned value + expect(result).toEqual(true); + }); + + /** + * @target EvmChain.verifyTransactionFee should return false when gasPrice + * is too much bigger than expected + * @dependencies + * @scenario + * - mock mockGetGasRequired + * - mock getFeeData + * - mock PaymentTransaction + * - check returned value + * @expected + * - it should return false + */ + it('should return false when gasPrice is too much bigger than expected', async () => { + // mock a config that has too much bigger max fee comparing to the mocked transaction + const network = new TestEvmNetwork(); + testUtils.mockGetGasRequired(network, 76000n); + testUtils.mockGetFeeData(network, new FeeData(20n, null, null)); + + // mock PaymentTransaction + const evmChain = testUtils.generateChainObject(network, undefined, 0); + const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; + const txType = TransactionType.payment; + const tx = Transaction.from(TestData.transaction1Json); + tx.gasLimit = 76000n * evmChain.configs.gasLimitMultiplier; + tx.gasPrice = 25n; + tx.value = 10n; + + const paymentTx = new PaymentTransaction( + evmChain.CHAIN, + tx.unsignedHash, + eventId, + Serializer.serialize(tx), + txType + ); + + // run test + const result = await evmChain.verifyTransactionFee(paymentTx); + + // check returned value + expect(result).toEqual(false); + }); + + /** + * @target EvmChain.verifyTransactionFee should return false when gasPrice + * is too much smaller than expected + * @dependencies + * @scenario + * - mock mockGetGasRequired + * - mock getFeeData + * - mock PaymentTransaction + * - check returned value + * @expected + * - it should return false + */ + it('should return false when gasPrice is too much smaller than expected', async () => { + // mock a config that has too much smaller max fee comparing to the mocked transaction + const network = new TestEvmNetwork(); + testUtils.mockGetGasRequired(network, 76000n); + testUtils.mockGetFeeData(network, new FeeData(20n, null, null)); + + // mock PaymentTransaction + const evmChain = testUtils.generateChainObject(network, undefined, 0); + const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; + const txType = TransactionType.payment; + const tx = Transaction.from(TestData.transaction1Json); + tx.gasLimit = 76000n * evmChain.configs.gasLimitMultiplier; + tx.gasPrice = 16n; + tx.value = 10n; + + const paymentTx = new PaymentTransaction( + evmChain.CHAIN, + tx.unsignedHash, + eventId, + Serializer.serialize(tx), + txType + ); + + // run test + const result = await evmChain.verifyTransactionFee(paymentTx); + // check returned value expect(result).toEqual(false); }); From a7425185bfc893163e26dce9069f2bbcc358ac54 Mon Sep 17 00:00:00 2001 From: HFazelinia Date: Mon, 2 Dec 2024 12:14:10 +0000 Subject: [PATCH 4/5] fix EvmChain.verifyTransactionFee tests --- packages/chains/evm/tests/EvmChain.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/chains/evm/tests/EvmChain.spec.ts b/packages/chains/evm/tests/EvmChain.spec.ts index ed43654..52167c0 100644 --- a/packages/chains/evm/tests/EvmChain.spec.ts +++ b/packages/chains/evm/tests/EvmChain.spec.ts @@ -1375,7 +1375,7 @@ describe('EvmChain', () => { const evmChain = testUtils.generateChainObject(network, undefined, 0); const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; const txType = TransactionType.payment; - const tx = Transaction.from(TestData.transaction1Json); + const tx = Transaction.from(TestData.transaction1WithType0Json); tx.gasLimit = 76000n * evmChain.configs.gasLimitMultiplier; tx.gasPrice = 25n; tx.value = 10n; @@ -1417,7 +1417,7 @@ describe('EvmChain', () => { const evmChain = testUtils.generateChainObject(network, undefined, 0); const eventId = 'bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'; const txType = TransactionType.payment; - const tx = Transaction.from(TestData.transaction1Json); + const tx = Transaction.from(TestData.transaction1WithType0Json); tx.gasLimit = 76000n * evmChain.configs.gasLimitMultiplier; tx.gasPrice = 16n; tx.value = 10n; From 8a09d20a9291c06cf9ea874fa378e8af720dfeda Mon Sep 17 00:00:00 2001 From: HFazelinia Date: Tue, 3 Dec 2024 12:28:10 +0000 Subject: [PATCH 5/5] minor improvements --- packages/chains/evm/lib/EvmChain.ts | 8 ++++---- packages/chains/evm/tests/EvmChain.spec.ts | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/chains/evm/lib/EvmChain.ts b/packages/chains/evm/lib/EvmChain.ts index 1187808..da354ad 100644 --- a/packages/chains/evm/lib/EvmChain.ts +++ b/packages/chains/evm/lib/EvmChain.ts @@ -132,7 +132,7 @@ abstract class EvmChain extends AbstractChain { } const feeData = await this.network.getFeeData(); - let requiredEth = 0n; + let requiredNativeToken = 0n; let txFeeData; if (this.evmTxType === 2) { if ( @@ -143,7 +143,7 @@ abstract class EvmChain extends AbstractChain { `Tx type is 2 but max fee variables are null` ); } - requiredEth = + requiredNativeToken = this.configs.gasLimitCap * BigInt(orders.length) * feeData.maxFeePerGas; txFeeData = { maxFeePerGas: feeData.maxFeePerGas, @@ -153,7 +153,7 @@ abstract class EvmChain extends AbstractChain { if (feeData.gasPrice === null) { throw new ImpossibleBehavior(`Tx type is 0 but gas price is null`); } else { - requiredEth = + requiredNativeToken = this.configs.gasLimitCap * BigInt(orders.length) * feeData.gasPrice; txFeeData = { gasPrice: feeData.gasPrice, @@ -171,7 +171,7 @@ abstract class EvmChain extends AbstractChain { { nativeToken: this.tokenMap.wrapAmount( this.NATIVE_TOKEN_ID, - requiredEth, + requiredNativeToken, this.CHAIN ).amount, tokens: [], diff --git a/packages/chains/evm/tests/EvmChain.spec.ts b/packages/chains/evm/tests/EvmChain.spec.ts index 52167c0..a919661 100644 --- a/packages/chains/evm/tests/EvmChain.spec.ts +++ b/packages/chains/evm/tests/EvmChain.spec.ts @@ -585,7 +585,7 @@ describe('EvmChain', () => { * order * - eventId should be properly in the transaction data * - no extra data should be found in the transaction data - * - transaction must be of type 2 and has no blobs + * - transaction must be of type 0 and has no blobs * - nonce must be the same as the next available nonce * - gas limit should be as expected * - gas price should be as expected @@ -1311,6 +1311,7 @@ describe('EvmChain', () => { // check returned value expect(result).toEqual(false); }); + /** * @target EvmChain.verifyTransactionFee should return true when * gasPrice and gasLimit are set properly for a type 0 transaction