Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

OG-615 Server db file fix #696

Merged
merged 4 commits into from
Oct 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions packages/contracts/src/Penalizer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion packages/contracts/src/interfaces/IPenalizer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ interface IPenalizer {

struct Transaction {
uint256 nonce;
uint256 gasPrice;
uint256 gasLimit;
address to;
uint256 value;
Expand Down
22 changes: 16 additions & 6 deletions packages/contracts/src/utils/RLPReader.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

/*
Expand Down
52 changes: 44 additions & 8 deletions packages/dev/test/RelayHubPenalizations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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 =
{
Expand Down Expand Up @@ -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 () {
Expand All @@ -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')}`
Expand All @@ -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')}`
Expand All @@ -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]
Expand Down Expand Up @@ -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())
Expand Down
4 changes: 3 additions & 1 deletion packages/dev/test/RelayServer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
})
})

Expand Down
4 changes: 2 additions & 2 deletions packages/dev/test/ServerTestEnvironment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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,
Expand Down
41 changes: 40 additions & 1 deletion packages/dev/test/TxStoreManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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()
Expand Down Expand Up @@ -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)
})
3 changes: 3 additions & 0 deletions packages/relay/src/ServerConfigParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export interface ServerConfigParams {
refreshStateTimeoutBlocks: number
pendingTransactionTimeoutBlocks: number
confirmationsNeeded: number
dbAutoCompactionInterval: number
retryGasPriceFactor: number
maxGasPrice: string
defaultGasLimit: number
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -190,6 +192,7 @@ const ConfigParamsTypes = {
initialReputation: 'number',

requiredVersionRange: 'string',
dbAutoCompactionInterval: 'number',
retryGasPriceFactor: 'number',
runPaymasterReputations: 'boolean',
refreshStateTimeoutBlocks: 'number',
Expand Down
5 changes: 4 additions & 1 deletion packages/relay/src/TxStoreManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,16 @@ export class TxStoreManager {
private readonly txstore: AsyncNedb<any>
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 })

Expand Down
Loading