diff --git a/packages/contracts/src/Penalizer.sol b/packages/contracts/src/Penalizer.sol index 27ff7e602..113c2cb9e 100644 --- a/packages/contracts/src/Penalizer.sol +++ b/packages/contracts/src/Penalizer.sol @@ -26,26 +26,38 @@ contract Penalizer is IPenalizer { penalizeBlockExpiration = _penalizeBlockExpiration; } - function isTransactionType1(bytes calldata rawTransaction) public pure returns (bool) { + function isLegacyTransaction(bytes calldata rawTransaction) internal pure returns (bool) { + uint8 transactionTypeByte = uint8(rawTransaction[0]); + return (transactionTypeByte >= 0xc0 && transactionTypeByte <= 0xfe); + } + + function isTransactionType1(bytes calldata rawTransaction) internal pure returns (bool) { return (uint8(rawTransaction[0]) == 1); } + function isTransactionType2(bytes calldata rawTransaction) internal pure returns (bool) { + return (uint8(rawTransaction[0]) == 2); + } + function isTransactionTypeValid(bytes calldata rawTransaction) public pure returns(bool) { - uint8 transactionTypeByte = uint8(rawTransaction[0]); - return (transactionTypeByte >= 0xc0 && transactionTypeByte <= 0xfe); + return isLegacyTransaction(rawTransaction) || isTransactionType1(rawTransaction) || isTransactionType2(rawTransaction); } function decodeTransaction(bytes calldata rawTransaction) public pure returns (Transaction memory transaction) { if (isTransactionType1(rawTransaction)) { (transaction.nonce, - transaction.gasPrice, transaction.gasLimit, transaction.to, transaction.value, transaction.data) = RLPReader.decodeTransactionType1(rawTransaction); + } else if (isTransactionType2(rawTransaction)) { + (transaction.nonce, + transaction.gasLimit, + transaction.to, + transaction.value, + transaction.data) = RLPReader.decodeTransactionType2(rawTransaction); } else { (transaction.nonce, - transaction.gasPrice, transaction.gasLimit, transaction.to, transaction.value, diff --git a/packages/contracts/src/interfaces/IPenalizer.sol b/packages/contracts/src/interfaces/IPenalizer.sol index 44688784e..2ab0980b1 100644 --- a/packages/contracts/src/interfaces/IPenalizer.sol +++ b/packages/contracts/src/interfaces/IPenalizer.sol @@ -9,7 +9,6 @@ interface IPenalizer { struct Transaction { uint256 nonce; - uint256 gasPrice; uint256 gasLimit; address to; uint256 value; diff --git a/packages/contracts/src/utils/RLPReader.sol b/packages/contracts/src/utils/RLPReader.sol index ca1d96368..28de4677e 100644 --- a/packages/contracts/src/utils/RLPReader.sol +++ b/packages/contracts/src/utils/RLPReader.sol @@ -25,22 +25,32 @@ library RLPReader { // helper function to decode rlp encoded legacy ethereum transaction /* * @param rawTransaction RLP encoded legacy ethereum transaction rlp([nonce, gasPrice, gasLimit, to, value, data])) - * @return tuple (nonce,gasPrice,gasLimit,to,value,data) + * @return tuple (nonce,gasLimit,to,value,data) */ - function decodeLegacyTransaction(bytes calldata rawTransaction) internal pure returns (uint, uint, uint, address, uint, bytes memory){ + function decodeLegacyTransaction(bytes calldata rawTransaction) internal pure returns (uint, uint, address, uint, bytes memory){ RLPReader.RLPItem[] memory values = rawTransaction.toRlpItem().toList(); // must convert to an rlpItem first! - return (values[0].toUint(), values[1].toUint(), values[2].toUint(), values[3].toAddress(), values[4].toUint(), values[5].toBytes()); + return (values[0].toUint(), values[2].toUint(), values[3].toAddress(), values[4].toUint(), values[5].toBytes()); } /* * @param rawTransaction format: 0x01 || rlp([chainId, nonce, gasPrice, gasLimit, to, value, data, access_list])) - * @return tuple (nonce,gasPrice,gasLimit,to,value,data) + * @return tuple (nonce,gasLimit,to,value,data) */ - function decodeTransactionType1(bytes calldata rawTransaction) internal pure returns (uint, uint, uint, address, uint, bytes memory){ + function decodeTransactionType1(bytes calldata rawTransaction) internal pure returns (uint, uint, address, uint, bytes memory){ bytes memory payload = rawTransaction[1:rawTransaction.length]; RLPReader.RLPItem[] memory values = payload.toRlpItem().toList(); // must convert to an rlpItem first! - return (values[1].toUint(), values[2].toUint(), values[3].toUint(), values[4].toAddress(), values[5].toUint(), values[6].toBytes()); + return (values[1].toUint(), values[3].toUint(), values[4].toAddress(), values[5].toUint(), values[6].toBytes()); + } + + /* + * @param rawTransaction format: 0x02 || rlp([chain_id, nonce, max_priority_fee_per_gas, max_fee_per_gas, gas_limit, destination, amount, data, access_list])) + * @return tuple (nonce,gasLimit,to,value,data) + */ + function decodeTransactionType2(bytes calldata rawTransaction) internal pure returns (uint, uint, address, uint, bytes memory){ + bytes memory payload = rawTransaction[1:rawTransaction.length]; + RLPReader.RLPItem[] memory values = payload.toRlpItem().toList(); // must convert to an rlpItem first! + return (values[1].toUint(), values[4].toUint(), values[5].toAddress(), values[6].toUint(), values[7].toBytes()); } /* diff --git a/packages/dev/test/RelayHubPenalizations.test.ts b/packages/dev/test/RelayHubPenalizations.test.ts index 99f228761..a455a2bfd 100644 --- a/packages/dev/test/RelayHubPenalizations.test.ts +++ b/packages/dev/test/RelayHubPenalizations.test.ts @@ -3,7 +3,7 @@ import { balance, ether, expectEvent, expectRevert } from '@openzeppelin/test-helpers' import BN from 'bn.js' -import { Transaction, AccessListEIP2930Transaction } from '@ethereumjs/tx' +import { Transaction, AccessListEIP2930Transaction, FeeMarketEIP1559Transaction } from '@ethereumjs/tx' import Common from '@ethereumjs/common' import { TxOptions } from '@ethereumjs/tx/dist/types' import { encode } from 'rlp' @@ -162,15 +162,16 @@ contract('RelayHub Penalizations', function ([_, relayOwner, committer, nonCommi describe('penalizations', function () { const stake = ether('1') - describe('TransactionType1 penalization', function () { + describe('TransactionType penalization', function () { let relayRequest: RelayRequest let encodedCall: string let common: Common let legacyTx: Transaction let eip2930Transaction: AccessListEIP2930Transaction + let eip1559Transaction: FeeMarketEIP1559Transaction let describeSnapshotId: string before(async function () { - common = new Common({ chain: 'mainnet', hardfork: 'berlin' }) + common = new Common({ chain: 'mainnet', hardfork: 'london' }) // TODO: 'encodedCallArgs' is no longer needed. just keep the RelayRequest in test relayRequest = { @@ -206,6 +207,13 @@ contract('RelayHub Penalizations', function ([_, relayOwner, committer, nonCommi }, { common }) eip2930Transaction = AccessListEIP2930Transaction.fromTxData(legacyTx, { common }) + eip1559Transaction = FeeMarketEIP1559Transaction.fromTxData({ + nonce: relayCallArgs.nonce, + gasLimit: relayCallArgs.gasLimit, + to: relayHub.address, + value: relayCallArgs.value, + data: encodedCall + }, { common }) }) beforeEach(async function () { @@ -216,13 +224,20 @@ contract('RelayHub Penalizations', function ([_, relayOwner, committer, nonCommi }) describe('#decodeTransaction', function () { - it('should decode new TransactionType1 tx', async function () { + it('should decode TransactionType1 tx', async function () { const input = [bnToRlp(eip2930Transaction.chainId), bnToRlp(eip2930Transaction.nonce), bnToRlp(eip2930Transaction.gasPrice), bnToRlp(eip2930Transaction.gasLimit), eip2930Transaction.to!.toBuffer(), bnToRlp(eip2930Transaction.value), eip2930Transaction.data, eip2930Transaction.accessList] const penalizableTxData = `0x01${encode(input).toString('hex')}` const decodedTx = await penalizer.decodeTransaction(penalizableTxData) // @ts-ignore validateDecodedTx(decodedTx, eip2930Transaction) }) + it('should decode new TransactionType2 tx', async function () { + const input = [bnToRlp(eip1559Transaction.chainId), bnToRlp(eip1559Transaction.nonce), bnToRlp(eip1559Transaction.maxPriorityFeePerGas), bnToRlp(eip1559Transaction.maxFeePerGas), bnToRlp(eip1559Transaction.gasLimit), eip1559Transaction.to!.toBuffer(), bnToRlp(eip1559Transaction.value), eip1559Transaction.data, eip1559Transaction.accessList] + const penalizableTxData = `0x02${encode(input).toString('hex')}` + const decodedTx = await penalizer.decodeTransaction(penalizableTxData) + // @ts-ignore + validateDecodedTx(decodedTx, eip1559Transaction) + }) it('should decode legacy tx', async function () { const input = [bnToRlp(legacyTx.nonce), bnToRlp(legacyTx.gasPrice), bnToRlp(legacyTx.gasLimit), legacyTx.to!.toBuffer(), bnToRlp(legacyTx.value), legacyTx.data] const penalizableTxData = `0x${encode(input).toString('hex')}` @@ -232,7 +247,7 @@ contract('RelayHub Penalizations', function ([_, relayOwner, committer, nonCommi }) }) - it('should penalize new TransactionType1 tx', async function () { + it('should not penalize TransactionType1 tx', async function () { const signedTx = eip2930Transaction.sign(relayCallArgs.privateKey) const input = [bnToRlp(eip2930Transaction.chainId), bnToRlp(eip2930Transaction.nonce), bnToRlp(eip2930Transaction.gasPrice), bnToRlp(eip2930Transaction.gasLimit), eip2930Transaction.to!.toBuffer(), bnToRlp(eip2930Transaction.value), eip2930Transaction.data, eip2930Transaction.accessList] const penalizableTxData = `0x01${encode(input).toString('hex')}` @@ -246,8 +261,30 @@ contract('RelayHub Penalizations', function ([_, relayOwner, committer, nonCommi const commitHash = web3.utils.keccak256(web3.utils.keccak256(request) + committer.slice(2)) await penalizer.commit(commitHash, { from: committer }) await evmMineMany(10) - const res = await penalizer.penalizeIllegalTransaction(penalizableTxData, penalizableTxSignature, relayHub.address, '0x', { from: committer }) - expectEvent(res, 'StakePenalized', { relayManager: relayManager, beneficiary: committer, reward: stake.divn(2) }) + await expectRevert( + penalizer.penalizeIllegalTransaction(penalizableTxData, penalizableTxSignature, relayHub.address, '0x', { from: committer }), + 'Legal relay transaction' + ) + }) + + it('should not penalize TransactionType2 tx', async function () { + const signedTx = eip1559Transaction.sign(relayCallArgs.privateKey) + const input = [bnToRlp(eip1559Transaction.chainId), bnToRlp(eip1559Transaction.nonce), bnToRlp(eip1559Transaction.maxPriorityFeePerGas), bnToRlp(eip1559Transaction.maxFeePerGas), bnToRlp(eip1559Transaction.gasLimit), eip1559Transaction.to!.toBuffer(), bnToRlp(eip1559Transaction.value), eip1559Transaction.data, eip1559Transaction.accessList] + const penalizableTxData = `0x02${encode(input).toString('hex')}` + + const newV = (signedTx.v!.toNumber() + 27) + const penalizableTxSignature = signatureRSV2Hex(signedTx.r!, signedTx.s!, newV) + + const request = penalizer.contract.methods.penalizeIllegalTransaction(penalizableTxData, penalizableTxSignature, relayHub.address, '0x').encodeABI() + + // eslint-disable-next-line + const commitHash = web3.utils.keccak256(web3.utils.keccak256(request) + committer.slice(2)) + await penalizer.commit(commitHash, { from: committer }) + await evmMineMany(10) + await expectRevert( + penalizer.penalizeIllegalTransaction(penalizableTxData, penalizableTxSignature, relayHub.address, '0x', { from: committer }), + 'Legal relay transaction' + ) }); // legacy tx first byte is in [0xc0, 0xfe] @@ -774,7 +811,6 @@ contract('RelayHub Penalizations', function ([_, relayOwner, committer, nonCommi function validateDecodedTx (decodedTx: { nonce: string, gasPrice: string, gasLimit: string, to: string, value: string, data: string}, originalTx: AccessListEIP2930Transaction | Transaction): void { assert.equal(decodedTx.nonce, originalTx.nonce.toString()) - assert.equal(decodedTx.gasPrice, originalTx.gasPrice.toString()) assert.equal(decodedTx.gasLimit, originalTx.gasLimit.toString()) assert.equal(removeHexPrefix(decodedTx.data), originalTx.data.toString('hex')) assert.equal(decodedTx.to.toLowerCase(), originalTx.to!.toString().toLowerCase()) diff --git a/packages/dev/test/RelayServer.test.ts b/packages/dev/test/RelayServer.test.ts index 2bd7a5a66..16aed0d97 100644 --- a/packages/dev/test/RelayServer.test.ts +++ b/packages/dev/test/RelayServer.test.ts @@ -57,7 +57,7 @@ contract('RelayServer', function (accounts: Truffle.Accounts) { }) describe('#init()', function () { - it('should initialize relay params (chainId, networkId, gasPrice)', async function () { + it('should initialize relay params', async function () { const env = new ServerTestEnvironment(web3.currentProvider as HttpProvider, accounts) await env.init({}) env.newServerInstanceNoFunding() @@ -71,6 +71,8 @@ contract('RelayServer', function (accounts: Truffle.Accounts) { assert.equal(relayServerToInit.isReady(), false, 'relay should not be ready yet') assert.equal(relayServerToInit.chainId, chainId) assert.equal(relayServerToInit.networkId, networkId) + // @ts-ignore + expect(relayServerToInit.txStoreManager.txstore.persistence.autocompactionIntervalId).to.exist }) }) diff --git a/packages/dev/test/ServerTestEnvironment.ts b/packages/dev/test/ServerTestEnvironment.ts index a3446ed19..dfbae3e1f 100644 --- a/packages/dev/test/ServerTestEnvironment.ts +++ b/packages/dev/test/ServerTestEnvironment.ts @@ -25,7 +25,7 @@ import { RelayClient } from '@opengsn/provider/dist/RelayClient' import { RelayInfo } from '@opengsn/common/dist/types/RelayInfo' import { RelayRegisteredEventInfo } from '@opengsn/common/dist/types/GSNContractsDataTypes' import { RelayServer } from '@opengsn/relay/dist/RelayServer' -import { configureServer, ServerConfigParams } from '@opengsn/relay/dist/ServerConfigParams' +import { configureServer, ServerConfigParams, serverDefaultConfiguration } from '@opengsn/relay/dist/ServerConfigParams' import { TxStoreManager } from '@opengsn/relay/dist/TxStoreManager' import { GSNConfig } from '@opengsn/provider/dist/GSNConfigurator' import { constants } from '@opengsn/common/dist/Constants' @@ -201,7 +201,7 @@ export class ServerTestEnvironment { const logger = createServerLogger('error', '', '') const managerKeyManager = this._createKeyManager(serverWorkdirs?.managerWorkdir) const workersKeyManager = this._createKeyManager(serverWorkdirs?.workersWorkdir) - const txStoreManager = new TxStoreManager({ workdir: serverWorkdirs?.workdir ?? getTemporaryWorkdirs().workdir }, logger) + const txStoreManager = new TxStoreManager({ workdir: serverWorkdirs?.workdir ?? getTemporaryWorkdirs().workdir, autoCompactionInterval: serverDefaultConfiguration.dbAutoCompactionInterval }, logger) const gasPriceFetcher = new GasPriceFetcher('', '', this.contractInteractor, logger) const serverDependencies = { contractInteractor: this.contractInteractor, diff --git a/packages/dev/test/TxStoreManager.test.ts b/packages/dev/test/TxStoreManager.test.ts index 4d642fbd4..5c45acb41 100644 --- a/packages/dev/test/TxStoreManager.test.ts +++ b/packages/dev/test/TxStoreManager.test.ts @@ -4,6 +4,15 @@ import { ServerAction, StoredTransaction } from '@opengsn/relay/dist/StoredTrans import { TXSTORE_FILENAME, TxStoreManager } from '@opengsn/relay/dist/TxStoreManager' import { createServerLogger } from '@opengsn/relay/dist/ServerWinstonLogger' import { toHex } from 'web3-utils' +import { Logger } from 'winston' +import sinon from 'sinon' +import { serverDefaultConfiguration } from '@opengsn/relay/dist/ServerConfigParams' +import chai from 'chai' +import chaiAsPromised from 'chai-as-promised' +import sinonChai from 'sinon-chai' +import { sleep } from '@opengsn/common/dist' + +const { expect, assert } = chai.use(chaiAsPromised).use(sinonChai) // NOTICE: this dir is removed in 'after', do not use this in any other test const workdir = '/tmp/gsn/test/txstore_manager' @@ -26,9 +35,10 @@ contract('TxStoreManager', function (accounts) { let tx: StoredTransaction let tx2: StoredTransaction let tx3: StoredTransaction + let logger: Logger before('create txstore', async function () { - const logger = createServerLogger('error', '', '') + logger = createServerLogger('error', '', '') cleanFolder() txmanager = new TxStoreManager({ workdir }, logger) await txmanager.clearAll() @@ -136,5 +146,34 @@ contract('TxStoreManager', function (accounts) { assert.deepEqual(1, (await txmanager.getAll()).length) }) + it('should compact txstore file', async function () { + const txStoreStringFile = '{"$$indexCreated":{"fieldName":"txId","unique":true,"sparse":false}}\n' + + '{"$$indexCreated":{"fieldName":"nonceSigner","unique":true,"sparse":false}}\n' + + '{"from":"","to":"","gas":0,"gasPrice":0,"data":"","nonce":111,"value":"0xde0b6b3a7640000","txId":"123456","serverAction":3,"creationBlockNumber":0,"minedBlockNumber":0,"attempts":1,"nonceSigner":{"nonce":111,"signer":""},"_id":"jY4JcN9yBl9iQnTd","createdAt":{"$$date":1634512480542},"updatedAt":{"$$date":1634512480542}}\n' + + '{"from":"","to":"","gas":0,"gasPrice":0,"data":"","nonce":112,"value":"0xde0b6b3a7640000","txId":"1234567","serverAction":3,"creationBlockNumber":0,"minedBlockNumber":0,"attempts":1,"nonceSigner":{"nonce":112,"signer":""},"_id":"OUgaFNbNrbpQdefG","createdAt":{"$$date":1634512480543},"updatedAt":{"$$date":1634512480543}}\n' + + '{"from":"","to":"","gas":0,"gasPrice":0,"data":"","nonce":113,"value":"0xde0b6b3a7640000","txId":"12345678","serverAction":3,"creationBlockNumber":0,"minedBlockNumber":0,"attempts":1,"nonceSigner":{"nonce":113,"signer":""},"_id":"93HrwOrI27LxKMO4","createdAt":{"$$date":1634512480543},"updatedAt":{"$$date":1634512480543}}\n' + + '{"$$deleted":true,"_id":"93HrwOrI27LxKMO4"}\n' + + '{"$$deleted":true,"_id":"OUgaFNbNrbpQdefG"}\n' + + '{"$$deleted":true,"_id":"jY4JcN9yBl9iQnTd"}\n' + fs.writeFileSync(txStoreFilePath, txStoreStringFile) + let linesCount = (fs.readFileSync(txStoreFilePath, 'utf8')).split('\n').length + assert.equal(9, linesCount) + const clock = sinon.useFakeTimers(Date.now()) + try { + txmanager = new TxStoreManager({ workdir, autoCompactionInterval: serverDefaultConfiguration.dbAutoCompactionInterval }, logger) + // @ts-ignore + sinon.spy(txmanager.txstore.persistence, 'compactDatafile') + await clock.tickAsync(serverDefaultConfiguration.dbAutoCompactionInterval) + // @ts-ignore + expect(txmanager.txstore.persistence.compactDatafile).to.have.been.calledOnce + clock.restore() + await sleep(500) + linesCount = (fs.readFileSync(txStoreFilePath, 'utf8')).split('\n').length + assert.equal(3, linesCount) + } finally { + clock.restore() + } + }) + after('remove txstore', cleanFolder) }) diff --git a/packages/relay/src/ServerConfigParams.ts b/packages/relay/src/ServerConfigParams.ts index 0f0037558..4501e29ea 100644 --- a/packages/relay/src/ServerConfigParams.ts +++ b/packages/relay/src/ServerConfigParams.ts @@ -58,6 +58,7 @@ export interface ServerConfigParams { refreshStateTimeoutBlocks: number pendingTransactionTimeoutBlocks: number confirmationsNeeded: number + dbAutoCompactionInterval: number retryGasPriceFactor: number maxGasPrice: string defaultGasLimit: number @@ -126,6 +127,7 @@ export const serverDefaultConfiguration: ServerConfigParams = { refreshStateTimeoutBlocks: 5, pendingTransactionTimeoutBlocks: 30, // around 5 minutes with 10 seconds block times confirmationsNeeded: 12, + dbAutoCompactionInterval: 604800000, // Week in ms: 1000*60*60*24*7 retryGasPriceFactor: 1.2, defaultGasLimit: 500000, maxGasPrice: 100e9.toString(), @@ -190,6 +192,7 @@ const ConfigParamsTypes = { initialReputation: 'number', requiredVersionRange: 'string', + dbAutoCompactionInterval: 'number', retryGasPriceFactor: 'number', runPaymasterReputations: 'boolean', refreshStateTimeoutBlocks: 'number', diff --git a/packages/relay/src/TxStoreManager.ts b/packages/relay/src/TxStoreManager.ts index 4a73cc650..39dd206a9 100644 --- a/packages/relay/src/TxStoreManager.ts +++ b/packages/relay/src/TxStoreManager.ts @@ -14,13 +14,16 @@ export class TxStoreManager { private readonly txstore: AsyncNedb private readonly logger: LoggerInterface - constructor ({ workdir = '/tmp/test/', inMemory = false }, logger: LoggerInterface) { + constructor ({ workdir = '/tmp/test/', inMemory = false, autoCompactionInterval = 0 }, logger: LoggerInterface) { this.logger = logger this.txstore = new AsyncNedb({ filename: inMemory ? undefined : `${workdir}/${TXSTORE_FILENAME}`, autoload: true, timestampData: true }) + if (autoCompactionInterval !== 0) { + this.txstore.persistence.setAutocompactionInterval(autoCompactionInterval) + } this.txstore.ensureIndex({ fieldName: 'txId', unique: true }) this.txstore.ensureIndex({ fieldName: 'nonceSigner', unique: true }) diff --git a/packages/relay/src/runServer.ts b/packages/relay/src/runServer.ts index 277e2ca31..0e15e7ab0 100755 --- a/packages/relay/src/runServer.ts +++ b/packages/relay/src/runServer.ts @@ -94,7 +94,7 @@ async function run (): Promise { console.log('Creating managers...\n') const managerKeyManager = new KeyManager(1, workdir + '/manager') const workersKeyManager = new KeyManager(1, workdir + '/workers') - const txStoreManager = new TxStoreManager({ workdir }, logger) + const txStoreManager = new TxStoreManager({ workdir, autoCompactionInterval: config.dbAutoCompactionInterval }, logger) console.log('Creating interactor...\n') const contractInteractor = new ContractInteractor({ provider: web3provider,