From 8aae21fdf7a673b340cd59a656dab995cbe13578 Mon Sep 17 00:00:00 2001 From: Shunji Zhan Date: Thu, 7 Dec 2023 10:41:36 +0800 Subject: [PATCH] throw `outOfGas` error when gaslimit too low (#899) * throw outOfGas err when gaslimit too low * fix * polish * polish --- packages/eth-providers/src/base-provider.ts | 64 +++++++----------- .../src/utils/transactionHelper.ts | 43 ++++-------- .../src/__tests__/e2e/errors.test.ts | 65 +++++++++++++++++-- 3 files changed, 97 insertions(+), 75 deletions(-) diff --git a/packages/eth-providers/src/base-provider.ts b/packages/eth-providers/src/base-provider.ts index 490b269a6..b7736dd82 100644 --- a/packages/eth-providers/src/base-provider.ts +++ b/packages/eth-providers/src/base-provider.ts @@ -26,7 +26,7 @@ import { FrameSystemAccountInfo } from '@polkadot/types/lookup'; import { Logger } from '@ethersproject/logger'; import { Network } from '@ethersproject/networks'; import { Observable, ReplaySubject, Subscription, firstValueFrom, throwError } from 'rxjs'; -import { Option, UInt, decorateStorage, unwrapStorageType } from '@polkadot/types'; +import { Option, decorateStorage, unwrapStorageType } from '@polkadot/types'; import { Storage } from '@polkadot/types/metadata/decorate/types'; import { SubmittableExtrinsic } from '@polkadot/api/types'; import { VersionedRegistry } from '@polkadot/api/base/types'; @@ -64,6 +64,7 @@ import { LogFilter, PROVIDER_ERRORS, SanitizedLogFilter, + TxRequestWithGas, calcEthereumTransactionParams, calcSubstrateTransactionParams, checkEvmExecutionError, @@ -183,6 +184,7 @@ export interface TXReceipt extends partialTX { export interface GasConsts { storageDepositPerByte: bigint; txFeePerGas: bigint; + storageByteDeposit: bigint; } export interface EventData { [index: string]: { @@ -789,32 +791,26 @@ export abstract class BaseProvider extends AbstractProvider { return code.toHex(); }; - // TODO: removable? call = async ( - _transaction: Deferrable, + transaction: Deferrable, _blockTag?: BlockTag | Promise | Eip1898BlockTag ): Promise => { const blockTag = await this._ensureSafeModeBlockTagFinalization(await parseBlockTag(_blockTag)); - const [txRequest, blockHash] = await Promise.all([ - getTransactionRequest(_transaction), + const [ethReq, blockHash] = await Promise.all([ + getTransactionRequest(transaction), this._getBlockHash(blockTag), ]); - const transaction = txRequest.gasLimit && txRequest.gasPrice - ? txRequest - : { ...txRequest, ...(await this._getEthGas()) }; + ethReq.gasPrice ??= await this.getGasPrice(); + ethReq.gasLimit ??= BigNumber.from(999999920); - const { storageLimit, gasLimit } = this._getSubstrateGasParams(transaction); + const substrateGas = this._getSubstrateGasParams(ethReq); const callRequest: SubstrateEvmCallRequest = { - from: transaction.from, - to: transaction.to, - gasLimit, - storageLimit, - value: transaction.value?.toBigInt(), - data: transaction.data, - accessList: transaction.accessList, + ...ethReq, + value: ethReq.value?.toBigInt(), + ...substrateGas, }; const res = await this._ethCall(callRequest, blockHash); @@ -826,7 +822,7 @@ export abstract class BaseProvider extends AbstractProvider { const api = at ? await this.api.at(at) : this.api; const { from, to, gasLimit, storageLimit, value, data, accessList } = callRequest; - const estimate = true; + const estimate = false; const res = to ? await api.call.evmRuntimeRPCApi.call(from, to, data, value, gasLimit, storageLimit, accessList, estimate) @@ -919,8 +915,9 @@ export abstract class BaseProvider extends AbstractProvider { }; _getGasConsts = (): GasConsts => ({ - storageDepositPerByte: (this.api.consts.evm.storageDepositPerByte as UInt).toBigInt(), - txFeePerGas: (this.api.consts.evm.txFeePerGas as UInt).toBigInt(), + storageDepositPerByte: this.api.consts.evm.storageDepositPerByte.toBigInt(), + txFeePerGas: this.api.consts.evm.txFeePerGas.toBigInt(), + storageByteDeposit: this.api.consts.evm.storageDepositPerByte.toBigInt(), }); /** @@ -1026,15 +1023,11 @@ export abstract class BaseProvider extends AbstractProvider { validUntil = blockNumber + 100; } - const storageByteDeposit = (this.api.consts.evm.storageDepositPerByte as UInt).toBigInt(); - const txFeePerGas = (this.api.consts.evm.txFeePerGas as UInt).toBigInt(); - const { txGasLimit, txGasPrice } = calcEthereumTransactionParams({ gasLimit, storageLimit, validUntil, - storageByteDeposit, - txFeePerGas, + ...this._getGasConsts(), }); return { @@ -1061,15 +1054,12 @@ export abstract class BaseProvider extends AbstractProvider { gasLimit: BigNumber; }> => { const validUntil = _validUntil || (await this.getBlockNumber()) + 150; // default 150 * 12 / 60 = 30min - const storageByteDeposit = (this.api.consts.evm.storageDepositPerByte as UInt).toBigInt(); - const txFeePerGas = (this.api.consts.evm.txFeePerGas as UInt).toBigInt(); const { txGasLimit, txGasPrice } = calcEthereumTransactionParams({ gasLimit, storageLimit, validUntil, - storageByteDeposit, - txFeePerGas, + ...this._getGasConsts(), }); return { @@ -1091,17 +1081,11 @@ export abstract class BaseProvider extends AbstractProvider { gasLimit: BigNumber; storageLimit: BigNumber; validUntil: BigNumber; - } => { - const storageByteDeposit = (this.api.consts.evm.storageDepositPerByte as UInt).toBigInt(); - const txFeePerGas = (this.api.consts.evm.txFeePerGas as UInt).toBigInt(); - - return calcSubstrateTransactionParams({ - txGasPrice: gasPrice, - txGasLimit: gasLimit, - storageByteDeposit, - txFeePerGas, - }); - }; + } => calcSubstrateTransactionParams({ + txGasPrice: gasPrice, + txGasLimit: gasLimit, + ...this._getGasConsts(), + }); /** * Estimate resources for a transaction. @@ -1124,7 +1108,7 @@ export abstract class BaseProvider extends AbstractProvider { const txRequest = { ..._txRequest, value: BigNumber.isBigNumber(_txRequest.value) ? _txRequest.value.toBigInt() : _txRequest.value, - gasLimit: _txRequest.gasLimit?.toBigInt() || MAX_GAS_LIMIT, + gasLimit: _txRequest.gasLimit?.toBigInt() ?? MAX_GAS_LIMIT, storageLimit: STORAGE_LIMIT, }; diff --git a/packages/eth-providers/src/utils/transactionHelper.ts b/packages/eth-providers/src/utils/transactionHelper.ts index 3c2055d82..844694b21 100644 --- a/packages/eth-providers/src/utils/transactionHelper.ts +++ b/packages/eth-providers/src/utils/transactionHelper.ts @@ -8,6 +8,8 @@ import { GAS_LIMIT_CHUNK, GAS_MASK, MAX_GAS_LIMIT_CC, ONE_HUNDRED_GWEI, STORAGE_ import { ethToNativeDecimal } from './utils'; import { formatter } from './receiptHelper'; +export type TxRequestWithGas = TransactionRequest & { gas?: BigNumberish }; + type TxConsts = { storageByteDeposit: BigNumberish; txFeePerGas: BigNumberish; @@ -78,45 +80,28 @@ export const calcSubstrateTransactionParams = ( }; export const getTransactionRequest = async ( - transaction: Deferrable + txRequest: Deferrable ): Promise> => { - const values: any = await transaction; + const req = await resolveProperties(txRequest); + const tx: Partial = {}; - const tx: any = {}; + if (!req.gasLimit && req.gas !== undefined) { + req.gasLimit = req.gas; + } - ['from', 'to'].forEach(key => { - if (values[key] === null || values[key] === undefined) { - return; - } - tx[key] = Promise.resolve(values[key]).then(v => (v ? v : null)); + ['from', 'to', 'type'].forEach(key => { + tx[key] = req[key]; }); ['gasLimit', 'gasPrice', 'maxFeePerGas', 'maxPriorityFeePerGas', 'value'].forEach(key => { - if (values[key] === null || values[key] === undefined) { - return; - } - tx[key] = Promise.resolve(values[key]).then(v => (v ? BigNumber.from(v) : null)); - }); - - ['type'].forEach(key => { - if (values[key] === null || values[key] === undefined) { - return; - } - tx[key] = Promise.resolve(values[key]).then(v => (v !== null || v !== undefined ? v : null)); + tx[key] = req[key] && BigNumber.from(req[key]); }); - if (values.accessList) { - tx.accessList = accessListify(values.accessList); - } + tx.accessList = req.accessList && accessListify(req.accessList); - ['data'].forEach(key => { - if (values[key] === null || values[key] === undefined) { - return; - } - tx[key] = Promise.resolve(values[key]).then(v => (v ? hexlify(v) : null)); - }); + tx.data = req.data && hexlify(req.data); - return formatter.transactionRequest(await resolveProperties(tx)); + return formatter.transactionRequest(tx); }; export const encodeGasLimit = ( diff --git a/packages/eth-rpc-adapter/src/__tests__/e2e/errors.test.ts b/packages/eth-rpc-adapter/src/__tests__/e2e/errors.test.ts index 254102064..396577d6c 100644 --- a/packages/eth-rpc-adapter/src/__tests__/e2e/errors.test.ts +++ b/packages/eth-rpc-adapter/src/__tests__/e2e/errors.test.ts @@ -1,13 +1,14 @@ +import { AcalaJsonRpcProvider } from '@acala-network/eth-providers'; +import { Contract, Wallet } from 'ethers'; import { JsonRpcProvider } from '@ethersproject/providers'; -import { RPC_URL, rpcGet } from './utils'; -import { Wallet } from 'ethers'; import { describe, expect, it } from 'vitest'; +import { parseEther } from 'ethers/lib/utils'; +import ADDRESS from '@acala-network/contracts/utils/MandalaAddress'; +import TokenABI from '@acala-network/contracts/build/contracts/Token.json'; import axios from 'axios'; -const eth_getEthGas = rpcGet('eth_getEthGas', RPC_URL); -const eth_sendRawTransaction = rpcGet('eth_sendRawTransaction', RPC_URL); -const eth_chainId = rpcGet('eth_chainId', RPC_URL); -const eth_call = rpcGet('eth_call', RPC_URL); +import { RPC_URL, eth_call, eth_chainId, eth_estimateGas, eth_getEthGas, eth_sendRawTransaction } from './utils'; +import { evmAccounts } from './consts'; describe('errors', () => { const POOR_ACCOUNT = '0xa872f6cbd25a0e04a08b1e21098017a9e6194d101d75e13111f71410c59cd570'; @@ -113,4 +114,56 @@ describe('errors', () => { message: 'execution reverted: ERC20: insufficient allowance', }); }); + + describe('throws outOfGas error when gaslimit too small', () => { + const provider = new AcalaJsonRpcProvider(RPC_URL); + const wallet = new Wallet(evmAccounts[0].privateKey, provider); + const aca = new Contract(ADDRESS.ACA, TokenABI.abi, wallet); + + it('when eth call', async () => { + const tx = await aca.populateTransaction.transfer(evmAccounts[1].evmAddress, parseEther('1.32')); + const { error } = (await eth_call([{ ...tx, gas: 0 }, 'latest'])).data; + + expect(error).toMatchInlineSnapshot(` + { + "code": -32603, + "message": "execution error: outOfGas", + } + `); + }); + + it('when estimateGas', async () => { + const tx = await aca.populateTransaction.transfer(evmAccounts[1].evmAddress, parseEther('1.32')); + const { error } = (await eth_estimateGas([{ ...tx, gas: 0 }, 'latest'])).data; + + expect(error).toMatchInlineSnapshot(` + { + "code": -32603, + "message": "execution error: outOfGas", + } + `); + }); + + it('when send raw transaction', async () => { + const tx = await aca.populateTransaction.transfer(evmAccounts[1].evmAddress, parseEther('1.32')); + const signedTx = await wallet.signTransaction({ ...tx, gasLimit: 0 }); + + const { error } = (await eth_sendRawTransaction([signedTx])).data; + expect(error).toMatchInlineSnapshot(` + { + "code": -32603, + "message": "execution error: outOfGas", + } + `); + }); + + it('when send transaction with ethers', async () => { + try { + await aca.transfer(evmAccounts[1].evmAddress, parseEther('1.32'), { gasLimit: 0 }); + expect.fail('did not throw an err'); + } catch (err) { + expect((err as any).error).toMatchInlineSnapshot('[Error: execution error: outOfGas]'); + } + }); + }); });