From bd3cf67df5c3b151ae10904f7edc86e3d2ded647 Mon Sep 17 00:00:00 2001 From: Facundo Spagnuolo Date: Fri, 17 May 2019 12:52:02 -0300 Subject: [PATCH 1/4] chore: downgrade ganache-cli to 6.2 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index f6a3218ed..6990c38e0 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,7 @@ "eth-ens-namehash": "^2.0.8", "eth-gas-reporter": "^0.1.1", "ethereumjs-abi": "^0.6.5", - "ganache-cli": "^6.4.2", + "ganache-cli": "~6.2.0", "mocha-lcov-reporter": "^1.3.0", "solidity-coverage": "0.5.8", "solium": "^1.2.3", From eac00710f77d32fe78e7401c217cf989106ff75c Mon Sep 17 00:00:00 2001 From: Facundo Spagnuolo Date: Fri, 17 May 2019 12:54:57 -0300 Subject: [PATCH 2/4] meta-txs: provide js libs to implement off-chain service --- contracts/test/mocks/relayer/RelayerMock.sol | 15 +- lib/relayer/GasPriceOracle.js | 20 +++ lib/relayer/RelayTransactionSigner.js | 118 +++++++++++++++ lib/relayer/RelayerService.js | 77 ++++++++++ lib/signTypedData.js | 35 ----- test/contracts/relayer/relayer.js | 83 +++++------ test/lib/relayer/RelayerService.js | 145 +++++++++++++++++++ 7 files changed, 413 insertions(+), 80 deletions(-) create mode 100644 lib/relayer/GasPriceOracle.js create mode 100644 lib/relayer/RelayTransactionSigner.js create mode 100644 lib/relayer/RelayerService.js delete mode 100644 lib/signTypedData.js create mode 100644 test/lib/relayer/RelayerService.js diff --git a/contracts/test/mocks/relayer/RelayerMock.sol b/contracts/test/mocks/relayer/RelayerMock.sol index 946747733..d5f45f5a8 100644 --- a/contracts/test/mocks/relayer/RelayerMock.sol +++ b/contracts/test/mocks/relayer/RelayerMock.sol @@ -5,11 +5,22 @@ import "../../../test/mocks/common/TimeHelpersMock.sol"; contract RelayerMock is Relayer, TimeHelpersMock { + uint256 public chainId; + + function initializeWithChainId(uint256 _monthlyRefundQuota, uint256 _chainId) external onlyInit { + initialized(); + startDate = getTimestamp(); + monthlyRefundQuota = _monthlyRefundQuota; + setDepositable(true); + + chainId = _chainId; + } + function messageHash(address to, uint256 nonce, bytes data, uint256 gasRefund, uint256 gasPrice) public view returns (bytes32) { return _messageHash(to, nonce, data, gasRefund, gasPrice); } - function domainSeparator() public view returns (bytes32) { - return _domainSeparator(); + function _domainChainId() internal view returns (uint256) { + return chainId == 0 ? 1 : chainId; } } diff --git a/lib/relayer/GasPriceOracle.js b/lib/relayer/GasPriceOracle.js new file mode 100644 index 000000000..ea9303753 --- /dev/null +++ b/lib/relayer/GasPriceOracle.js @@ -0,0 +1,20 @@ +const GAS_STATION_API_URL = 'https://ethgasstation.info/json/ethgasAPI.json' + +const DEFAULT_GAS_PRICE_VALUE = 1e6 + +module.exports = { + async fetch(networkId) { + if (networkId === 1) return this._fetchMainnetGasPrice() + else return DEFAULT_GAS_PRICE_VALUE + }, + + async _fetchMainnetGasPrice() { + try { + const axios = require('axios') + const { data: responseData } = await axios.get(GAS_STATION_API_URL) + return (responseData.average / 10) * 1e9 + } catch (error) { + throw new Error(`Could not fetch gas price from ETH gas station: ${error}`) + } + } +} diff --git a/lib/relayer/RelayTransactionSigner.js b/lib/relayer/RelayTransactionSigner.js new file mode 100644 index 000000000..2818815a9 --- /dev/null +++ b/lib/relayer/RelayTransactionSigner.js @@ -0,0 +1,118 @@ +const GasPriceOracle = require('./GasPriceOracle') + +const DEFAULT_DOMAIN_NAME = 'Aragon Relayer' +const DEFAULT_DOMAIN_VERSION = '1' + +const GAS_MULTIPLIER = 1.18 + +const FIRST_TX_GAS_OVERLOAD = 83500 +const NORMAL_TX_GAS_OVERLOAD = 53500 + +const DATA_TYPES = { + EIP712Domain: [ + { name: 'name', type: 'string' }, + { name: 'version', type: 'string' }, + { name: 'chainId', type: 'uint256' }, + { name: 'verifyingContract', type: 'address' }, + ], + Transaction: [ + { name: 'to', type: 'address' }, + { name: 'nonce', type: 'uint256' }, + { name: 'data', type: 'bytes' }, + { name: 'gasRefund', type: 'uint256' }, + { name: 'gasPrice', type: 'uint256' } + ], +} + +module.exports = web3 => class RelayTransactionSigner { + constructor(relayer, domainName = DEFAULT_DOMAIN_NAME, domainVersion = DEFAULT_DOMAIN_VERSION) { + this.relayer = relayer + this.domainName = domainName + this.domainVersion = domainVersion + } + + async signTransaction({ from, to, data, nonce = undefined, gasRefund = undefined, gasPrice = undefined }, service = undefined) { + if (!nonce) nonce = await this._fetchNextNonce(from) + if (!gasPrice) gasPrice = await this._estimateGasPrice() + if (!gasRefund) gasRefund = service + ? await this._estimateRelayGasWithService(service, { from, to, data, nonce, gasPrice }) + : await this._estimateRelayGasConservatively({ from, to, data, nonce, gasPrice }) + + const message = { from, to, nonce, data, gasRefund, gasPrice } + const signature = await this.signMessage(message) + return { ...message, signature } + } + + async signMessage({ from, to, nonce, data, gasRefund, gasPrice }) { + const message = { to, nonce, data, gasRefund, gasPrice } + const params = { method: 'eth_signTypedData', params: [from, this._typeData(message)], from } + return new Promise((resolve, reject) => { + web3.currentProvider.sendAsync(params, (error, tx) => { + return error ? reject(error) : resolve(tx.result) + }) + }) + } + + async _fetchNextNonce(sender) { + const lastUsedNonce = (await this.relayer.getSender(sender))[1] + return parseInt(lastUsedNonce.toString()) + 1 + } + + async _estimateRelayGasWithService(service, { from, to, data, nonce, gasPrice }) { + // simulate signature using gas limit as worst case scenario + const gasLimit = await this._gasLimit() + const signature = await this.signMessage({ from, to, nonce, data, gasRefund: gasLimit, gasPrice }) + const estimatedGas = await service.estimateGas({ from, to, nonce, data, gasRefund: gasLimit, gasPrice, signature }) + return Math.ceil(estimatedGas * GAS_MULTIPLIER) + } + + async _estimateRelayGasConservatively({ from, to, data, nonce }) { + const estimatedGas = await this._estimateCallGas({ from, to, data }) + const relayOverload = nonce === 1 ? FIRST_TX_GAS_OVERLOAD : NORMAL_TX_GAS_OVERLOAD + return Math.ceil((estimatedGas + relayOverload) * GAS_MULTIPLIER) + } + + async _estimateCallGas({ from, to, data }) { + const call = { from, to, data } + return new Promise((resolve, reject) => { + web3.eth.estimateGas(call, (error, response) => { + return error ? reject(error) : resolve(response) + }) + }) + } + + async _estimateGasPrice() { + return GasPriceOracle.fetch(this._networkId()) + } + + async _gasLimit() { + const block = await this._latestBlock() + return block.gasLimit + } + + async _latestBlock() { + return new Promise((resolve, reject) => { + web3.eth.getBlock('latest', (error, response) => { + return error ? reject(error) : resolve(response) + }) + }) + } + + _networkId() { + return parseInt(this.relayer.constructor.network_id) + } + + _typeData(message) { + return { + types: DATA_TYPES, + primaryType: 'Transaction', + domain: { + name: this.domainName, + version: this.domainVersion, + chainId: this._networkId(), + verifyingContract: this.relayer.address + }, + message: message + } + } +} diff --git a/lib/relayer/RelayerService.js b/lib/relayer/RelayerService.js new file mode 100644 index 000000000..1844889c9 --- /dev/null +++ b/lib/relayer/RelayerService.js @@ -0,0 +1,77 @@ +const GasPriceOracle = require('./GasPriceOracle') + +module.exports = web3 => class RelayerService { + constructor(wallet, relayer) { + this.wallet = wallet + this.relayer = relayer + } + + async relay(transaction) { + await this._assertTransactionWontRevert(transaction) + await this._assertTransactionGasCostIsCovered(transaction) + await this._assertTransactionReasonableGasPrice(transaction) + + return this._relay(transaction) + } + + async estimateGas(transaction) { + return this._estimateGas(transaction) + } + + async _relay(transaction) { + const { from, to, nonce, data, gasRefund, gasPrice, signature } = transaction + const txParams = { from: this.wallet, gas: gasRefund, gasPrice } + + // console.log(`\nRelaying transaction ${JSON.stringify(transaction)} with params ${JSON.stringify(txParams)}`) + return this.relayer.relay(from, to, nonce, data, gasRefund, gasPrice, signature, txParams) + } + + async _assertTransactionWontRevert(transaction) { + const error = await this._transactionWillFail(transaction) + if (!error) return + + throw Error(error.message.search(/(revert|invalid opcode|invalid jump)/) > -1 + ? `Will not relay failing transaction: ${error.message}` + : `Could not estimate gas: ${error.message}`) + } + + async _assertTransactionGasCostIsCovered(transaction) { + const { gasRefund } = transaction + const estimatedGas = await this._estimateGas(transaction) + if (gasRefund < estimatedGas) throw Error(`Given gas refund amount ${gasRefund} does not cover transaction gas cost ${estimatedGas}`) + } + + async _assertTransactionReasonableGasPrice(transaction) { + if (!this._isMainnet()) return; + const averageGasPrice = await GasPriceOracle.fetch(this._networkId()) + if (transaction.gasPrice < averageGasPrice) throw Error(`Given gas price is below the average ${averageGasPrice}`) + } + + async _transactionWillFail(transaction) { + try { + await this._estimateGas(transaction) + return false + } catch (error) { + return error + } + } + + async _estimateGas({ from, to, nonce, data, gasRefund, gasPrice, signature }) { + const calldata = this.relayer.contract.relay.getData(from, to, nonce, data, gasRefund, gasPrice, signature) + const call = { from: this.wallet, to: this.relayer.address, data: calldata } + + return new Promise((resolve, reject) => { + web3.eth.estimateGas(call, (error, response) => { + return error ? reject(error) : resolve(response) + }) + }) + } + + _isMainnet() { + return this._networkId() === 1 + } + + _networkId() { + return this.relayer.constructor.network_id + } +} diff --git a/lib/signTypedData.js b/lib/signTypedData.js deleted file mode 100644 index a4b0af064..000000000 --- a/lib/signTypedData.js +++ /dev/null @@ -1,35 +0,0 @@ -const TYPED_DATA = (relayer, message) => ({ - types: { - EIP712Domain: [ - { name: 'name', type: 'string' }, - { name: 'version', type: 'string' }, - { name: 'chainId', type: 'uint256' }, - { name: 'verifyingContract', type: 'address' }, - ], - Transaction: [ - { name: 'to', type: 'address' }, - { name: 'nonce', type: 'uint256' }, - { name: 'data', type: 'bytes' }, - { name: 'gasRefund', type: 'uint256' }, - { name: 'gasPrice', type: 'uint256' } - ], - }, - primaryType: "Transaction", - domain: { - name: 'Aragon Relayer', - version: '1', - chainId: 1, - verifyingContract: relayer.address - }, - message: message -}) - - -module.exports = (web3) => async (relayer, sender, message) => { - const params = { method: 'eth_signTypedData', params: [sender, TYPED_DATA(relayer, message)], from: sender } - return new Promise((resolve, reject) => { - web3.currentProvider.sendAsync(params, (error, tx) => { - return error ? reject(error) : resolve(tx.result) - }) - }) -} diff --git a/test/contracts/relayer/relayer.js b/test/contracts/relayer/relayer.js index b9f4f5c9f..0c91b9edf 100644 --- a/test/contracts/relayer/relayer.js +++ b/test/contracts/relayer/relayer.js @@ -1,4 +1,5 @@ -const signTypedData = require('../../../lib/signTypedData')(web3) +const RelayTransactionSigner = require('../../../lib/relayer/RelayTransactionSigner')(web3) + const { assertRevert } = require('../../helpers/assertThrow') const { skipCoverage } = require('../../helpers/coverage') const { getEventArgument, getNewProxyAddress } = require('../../helpers/events') @@ -15,7 +16,7 @@ const ONE_MONTH = 60 * 60 * 24 * 30 const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000' contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) => { - let daoFactory, dao, acl, app, relayer + let daoFactory, dao, acl, app, relayer, signer let kernelBase, aclBase, sampleAppBase, relayerBase let WRITING_ROLE, APP_MANAGER_ROLE, RELAYER_APP_ID let SET_MONTHLY_REFUND_QUOTA_ROLE, ALLOW_SENDER_ROLE, DISALLOW_SENDER_ROLE, ALLOW_OFF_CHAIN_SERVICE_ROLE, DISALLOW_OFF_CHAIN_SERVICE_ROLE @@ -26,11 +27,6 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) const SEND_ETH_GAS = 31000 // 21k base tx cost + 10k limit on depositable proxies - const signRelayedTx = async ({ from, to, nonce, calldata, gasRefund, gasPrice = GAS_PRICE }) => { - const message = { to, nonce, data: calldata, gasRefund, gasPrice } - return signTypedData(relayer, from, message) - } - before('deploy base implementations', async () => { aclBase = await ACL.new() kernelBase = await Kernel.new(true) // petrify immediately @@ -68,6 +64,7 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) beforeEach('create relayer instance', async () => { const receipt = await dao.newAppInstance(RELAYER_APP_ID, relayerBase.address, '0x', true, { from: root }) relayer = Relayer.at(getNewProxyAddress(receipt)) + signer = new RelayTransactionSigner(relayer) await relayer.mockSetTimestamp(NOW) @@ -218,7 +215,7 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) describe('getSender', () => { context('when the app is initialized', () => { - beforeEach('initialize relayer app', async () => await relayer.initialize(MONTHLY_REFUND_QUOTA)) + beforeEach('initialize relayer app', async () => await relayer.initializeWithChainId(MONTHLY_REFUND_QUOTA, Relayer.network_id)) context('when the given sender did not send transactions yet', () => { it('returns empty data', async () => { @@ -236,13 +233,13 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) beforeEach('relay a transaction', async () => { const nonce = 2 - const calldata = '0x11111111' - const signature = await signRelayedTx({ from: member, to: someone, nonce, calldata, gasRefund }) + const data = '0x11111111' + const signature = await signer.signMessage({ from: member, to: someone, nonce, data, gasRefund, gasPrice: GAS_PRICE }) await web3.eth.sendTransaction({ from: vault, to: relayer.address, value: 1e18, gas: SEND_ETH_GAS }) await relayer.allowService(offChainRelayerService, { from: root }) await relayer.allowSender(member, { from: root }) - await relayer.relay(member, someone, nonce, calldata, gasRefund, GAS_PRICE, signature, { from: offChainRelayerService }) + await relayer.relay(member, someone, nonce, data, gasRefund, GAS_PRICE, signature, { from: offChainRelayerService }) }) it('returns the corresponding information', async () => { @@ -337,7 +334,7 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) describe('canUseNonce', () => { context('when the app is initialized', () => { - beforeEach('initialize relayer app', async () => await relayer.initialize(MONTHLY_REFUND_QUOTA)) + beforeEach('initialize relayer app', async () => await relayer.initializeWithChainId(MONTHLY_REFUND_QUOTA, Relayer.network_id)) context('when the given sender did not send transactions yet', () => { context('when the requested nonce is zero', () => { @@ -361,14 +358,14 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) const usedNonce = 2 beforeEach('relay a transaction', async () => { - const calldata = '0x11111111' + const data = '0x11111111' const gasRefund = 50000 - const signature = await signRelayedTx({ from: member, to: someone, nonce: usedNonce, calldata, gasRefund }) + const signature = await signer.signMessage({ from: member, to: someone, nonce: usedNonce, data, gasRefund, gasPrice: GAS_PRICE }) await web3.eth.sendTransaction({ from: vault, to: relayer.address, value: 1e18, gas: SEND_ETH_GAS }) await relayer.allowService(offChainRelayerService, { from: root }) await relayer.allowSender(member, { from: root }) - await relayer.relay(member, someone, usedNonce, calldata, gasRefund, GAS_PRICE, signature, { from: offChainRelayerService }) + await relayer.relay(member, someone, usedNonce, data, gasRefund, GAS_PRICE, signature, { from: offChainRelayerService }) }) context('when the requested nonce is zero', () => { @@ -463,7 +460,7 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) describe('canRefund', () => { context('when the app is initialized', () => { beforeEach('initialize relayer app', async () => { - await relayer.initialize(MONTHLY_REFUND_QUOTA) + await relayer.initializeWithChainId(MONTHLY_REFUND_QUOTA, Relayer.network_id) }) context('when the given sender did not send transactions yet', () => { @@ -499,13 +496,13 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) beforeEach('relay a transaction', async () => { const nonce = 1 - const calldata = '0x11111111' - const signature = await signRelayedTx({ from: member, to: someone, nonce, calldata, gasRefund }) + const data = '0x11111111' + const signature = await signer.signMessage({ from: member, to: someone, nonce, data, gasRefund, gasPrice: GAS_PRICE }) await web3.eth.sendTransaction({ from: vault, to: relayer.address, value: 1e18, gas: SEND_ETH_GAS }) await relayer.allowService(offChainRelayerService, { from: root }) await relayer.allowSender(member, { from: root }) - await relayer.relay(member, someone, nonce, calldata, gasRefund, GAS_PRICE, signature, { from: offChainRelayerService }) + await relayer.relay(member, someone, nonce, data, gasRefund, GAS_PRICE, signature, { from: offChainRelayerService }) }) context('when the requested amount does not exceed the remaining monthly quota', () => { @@ -543,9 +540,9 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) describe('relay', () => { context('when the app is initialized', () => { - let signature, calldata, gasRefund = 50000, nonce = 10 + let signature, data, gasRefund = 50000, nonce = 10 - beforeEach('initialize relayer app', async () => await relayer.initialize(MONTHLY_REFUND_QUOTA)) + beforeEach('initialize relayer app', async () => await relayer.initializeWithChainId(MONTHLY_REFUND_QUOTA, Relayer.network_id)) context('when the service is allowed', () => { const from = offChainRelayerService @@ -558,8 +555,8 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) context('when the signature valid', () => { beforeEach('sign relayed call', async () => { - calldata = app.contract.write.getData(10) - signature = await signRelayedTx({ from: member, to: app.address, nonce, calldata, gasRefund }) + data = app.contract.write.getData(10) + signature = await signer.signMessage({ from: member, to: app.address, nonce, data, gasRefund, gasPrice: GAS_PRICE }) }) context('when the nonce is not used', () => { @@ -570,7 +567,7 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) }) it('relays transactions to app', async () => { - await relayer.relay(member, app.address, nonce, calldata, gasRefund, GAS_PRICE, signature, { from }) + await relayer.relay(member, app.address, nonce, data, gasRefund, GAS_PRICE, signature, { from }) assert.equal((await app.read()).toString(), 10, 'app value does not match') }) @@ -578,7 +575,7 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) const previousRelayerBalance = await web3.eth.getBalance(relayer.address) const previousServiceBalance = await web3.eth.getBalance(offChainRelayerService) - const { tx, receipt: { gasUsed } } = await relayer.relay(member, app.address, nonce, calldata, gasRefund, GAS_PRICE, signature, { from }) + const { tx, receipt: { gasUsed } } = await relayer.relay(member, app.address, nonce, data, gasRefund, GAS_PRICE, signature, { from }) const { gasPrice: gasPriceUsed } = await web3.eth.getTransaction(tx) const txRefund = gasRefund * GAS_PRICE @@ -592,7 +589,7 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) }) it('updates the last nonce used of the sender', async () => { - await relayer.relay(member, app.address, nonce, calldata, gasRefund, GAS_PRICE, signature, { from }) + await relayer.relay(member, app.address, nonce, data, gasRefund, GAS_PRICE, signature, { from }) const [_, lastUsedNonce] = await relayer.getSender(member) @@ -603,7 +600,7 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) it('updates the monthly refunds of the sender', async () => { const previousMonthlyRefunds = (await relayer.getSender(member))[3] - await relayer.relay(member, app.address, nonce, calldata, gasRefund, GAS_PRICE, signature, { from }) + await relayer.relay(member, app.address, nonce, data, gasRefund, GAS_PRICE, signature, { from }) const txRefund = gasRefund * GAS_PRICE const currentMonthlyRefunds = (await relayer.getSender(member))[3] @@ -611,19 +608,19 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) }) it('emits an event', async () => { - const receipt = await relayer.relay(member, app.address, nonce, calldata, gasRefund, GAS_PRICE, signature, { from }) + const receipt = await relayer.relay(member, app.address, nonce, data, gasRefund, GAS_PRICE, signature, { from }) assertAmountOfEvents(receipt, 'TransactionRelayed') - assertEvent(receipt, 'TransactionRelayed', { from: member, to: app.address, nonce, data: calldata }) + assertEvent(receipt, 'TransactionRelayed', { from: member, to: app.address, nonce, data }) }) it('overloads the first relayed transaction with ~83k and the followings with ~53k of gas', skipCoverage(async () => { const { receipt: { cumulativeGasUsed: nonRelayerGasUsed } } = await app.write(10, { from: member }) - const { receipt: { cumulativeGasUsed: firstRelayedGasUsed } } = await relayer.relay(member, app.address, nonce, calldata, gasRefund, GAS_PRICE, signature, { from }) + const { receipt: { cumulativeGasUsed: firstRelayedGasUsed } } = await relayer.relay(member, app.address, nonce, data, gasRefund, GAS_PRICE, signature, { from }) - const secondSignature = await signRelayedTx({ from: member, to: app.address, nonce: nonce + 1, calldata, gasRefund }) - const { receipt: { cumulativeGasUsed: secondRelayedGasUsed } } = await relayer.relay(member, app.address, nonce + 1, calldata, gasRefund, GAS_PRICE, secondSignature, { from }) + const secondSignature = await signer.signMessage({ from: member, to: app.address, nonce: nonce + 1, data, gasRefund, gasPrice: GAS_PRICE }) + const { receipt: { cumulativeGasUsed: secondRelayedGasUsed } } = await relayer.relay(member, app.address, nonce + 1, data, gasRefund, GAS_PRICE, secondSignature, { from }) const firstGasOverload = firstRelayedGasUsed - nonRelayerGasUsed const secondGasOverload = secondRelayedGasUsed - nonRelayerGasUsed @@ -638,7 +635,7 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) context('when the relayer does not have funds', () => { it('reverts', async () => { - await assertRevert(relayer.relay(member, app.address, nonce, calldata, gasRefund, GAS_PRICE, signature, { from }), 'RELAYER_GAS_REFUND_FAIL') + await assertRevert(relayer.relay(member, app.address, nonce, data, gasRefund, GAS_PRICE, signature, { from }), 'RELAYER_GAS_REFUND_FAIL') }) }) }) @@ -649,7 +646,7 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) }) it('reverts', async () => { - await assertRevert(relayer.relay(member, app.address, nonce, calldata, gasRefund, GAS_PRICE, signature, { from }), 'RELAYER_GAS_QUOTA_EXCEEDED') + await assertRevert(relayer.relay(member, app.address, nonce, data, gasRefund, GAS_PRICE, signature, { from }), 'RELAYER_GAS_QUOTA_EXCEEDED') }) }) }) @@ -657,11 +654,11 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) context('when the nonce is already used', () => { beforeEach('relay tx', async () => { await web3.eth.sendTransaction({ from: vault, to: relayer.address, value: 1e18, gas: SEND_ETH_GAS }) - await relayer.relay(member, app.address, nonce, calldata, gasRefund, GAS_PRICE, signature, { from }) + await relayer.relay(member, app.address, nonce, data, gasRefund, GAS_PRICE, signature, { from }) }) it('reverts', async () => { - await assertRevert(relayer.relay(member, app.address, nonce, calldata, gasRefund, GAS_PRICE, signature, { from }), 'RELAYER_NONCE_ALREADY_USED') + await assertRevert(relayer.relay(member, app.address, nonce, data, gasRefund, GAS_PRICE, signature, { from }), 'RELAYER_NONCE_ALREADY_USED') }) }) }) @@ -670,7 +667,7 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) it('reverts', async () => { const signature = web3.eth.sign(member, 'bla') - await assertRevert(relayer.relay(member, app.address, nonce, calldata, gasRefund, GAS_PRICE, signature, { from }), 'RELAYER_INVALID_SENDER_SIGNATURE') + await assertRevert(relayer.relay(member, app.address, nonce, data, gasRefund, GAS_PRICE, signature, { from }), 'RELAYER_INVALID_SENDER_SIGNATURE') }) }) }) @@ -680,10 +677,10 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) context('when the signature is valid', () => { it('forwards the revert reason', async () => { - calldata = app.contract.write.getData(10) - signature = await signRelayedTx({ from: someone, to: app.address, calldata, nonce, gasRefund }) + data = app.contract.write.getData(10) + signature = await signer.signMessage({ from: someone, to: app.address, nonce, data, gasRefund, gasPrice: GAS_PRICE }) - await assertRevert(relayer.relay(someone, app.address, nonce, calldata, gasRefund, GAS_PRICE, signature, { from }), 'APP_AUTH_FAILED') + await assertRevert(relayer.relay(someone, app.address, nonce, data, gasRefund, GAS_PRICE, signature, { from }), 'APP_AUTH_FAILED') }) }) @@ -691,7 +688,7 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) it('reverts', async () => { const signature = web3.eth.sign(someone, 'bla') - await assertRevert(relayer.relay(someone, app.address, nonce, calldata, gasRefund, GAS_PRICE, signature, { from }), 'RELAYER_INVALID_SENDER_SIGNATURE') + await assertRevert(relayer.relay(someone, app.address, nonce, data, gasRefund, GAS_PRICE, signature, { from }), 'RELAYER_INVALID_SENDER_SIGNATURE') }) }) }) @@ -699,7 +696,7 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) context('when the sender is not allowed', () => { it('reverts', async () => { - await assertRevert(relayer.relay(member, someone, nonce, calldata, gasRefund, GAS_PRICE, signature, { from }), 'RELAYER_SENDER_NOT_ALLOWED') + await assertRevert(relayer.relay(member, someone, nonce, data, gasRefund, GAS_PRICE, signature, { from }), 'RELAYER_SENDER_NOT_ALLOWED') }) }) }) @@ -708,7 +705,7 @@ contract('Relayer', ([_, root, member, someone, vault, offChainRelayerService]) const from = someone it('reverts', async () => { - await assertRevert(relayer.relay(member, someone, nonce, calldata, gasRefund, GAS_PRICE, signature, { from }), 'RELAYER_SERVICE_NOT_ALLOWED') + await assertRevert(relayer.relay(member, someone, nonce, data, gasRefund, GAS_PRICE, signature, { from }), 'RELAYER_SERVICE_NOT_ALLOWED') }) }) }) diff --git a/test/lib/relayer/RelayerService.js b/test/lib/relayer/RelayerService.js new file mode 100644 index 000000000..81595fff0 --- /dev/null +++ b/test/lib/relayer/RelayerService.js @@ -0,0 +1,145 @@ +const { getEventArgument, getNewProxyAddress } = require('../../helpers/events') + +const RelayerService = require('../../../lib/relayer/RelayerService')(web3) +const RelayTransactionSigner = require('../../../lib/relayer/RelayTransactionSigner')(web3) + +const ACL = artifacts.require('ACL') +const Kernel = artifacts.require('Kernel') +const Relayer = artifacts.require('RelayerMock') +const DAOFactory = artifacts.require('DAOFactory') +const SampleApp = artifacts.require('RelayedAppMock') + +const NOW = 1557945653 + +contract('RelayerService', ([_, root, member, someone, vault, offChainRelayerService]) => { + let daoFactory, dao, acl, app, relayer, service, signer + let kernelBase, aclBase, sampleAppBase, relayerBase + let WRITING_ROLE, APP_MANAGER_ROLE, RELAYER_APP_ID + let SET_MONTHLY_REFUND_QUOTA_ROLE, ALLOW_SENDER_ROLE, DISALLOW_SENDER_ROLE, ALLOW_OFF_CHAIN_SERVICE_ROLE, DISALLOW_OFF_CHAIN_SERVICE_ROLE + + const GAS_PRICE = 10e9 + const MONTHLY_REFUND_GAS = 1e6 * 5 + const MONTHLY_REFUND_QUOTA = MONTHLY_REFUND_GAS * GAS_PRICE + + const SEND_ETH_GAS = 31000 // 21k base tx cost + 10k limit on depositable proxies + + before('deploy base implementations', async () => { + aclBase = await ACL.new() + kernelBase = await Kernel.new(true) // petrify immediately + relayerBase = await Relayer.new() + sampleAppBase = await SampleApp.new() + daoFactory = await DAOFactory.new(kernelBase.address, aclBase.address, '0x0') + }) + + before('load constants', async () => { + RELAYER_APP_ID = await kernelBase.DEFAULT_RELAYER_APP_ID() + WRITING_ROLE = await sampleAppBase.WRITING_ROLE() + APP_MANAGER_ROLE = await kernelBase.APP_MANAGER_ROLE() + SET_MONTHLY_REFUND_QUOTA_ROLE = await relayerBase.SET_MONTHLY_REFUND_QUOTA_ROLE() + ALLOW_SENDER_ROLE = await relayerBase.ALLOW_SENDER_ROLE() + DISALLOW_SENDER_ROLE = await relayerBase.DISALLOW_SENDER_ROLE() + ALLOW_OFF_CHAIN_SERVICE_ROLE = await relayerBase.ALLOW_OFF_CHAIN_SERVICE_ROLE() + DISALLOW_OFF_CHAIN_SERVICE_ROLE = await relayerBase.DISALLOW_OFF_CHAIN_SERVICE_ROLE() + }) + + before('deploy DAO', async () => { + const receipt = await daoFactory.newDAO(root) + dao = Kernel.at(getEventArgument(receipt, 'DeployDAO', 'dao')) + acl = ACL.at(await dao.acl()) + + await acl.createPermission(root, dao.address, APP_MANAGER_ROLE, root, { from: root }) + }) + + before('create sample app instance', async () => { + const receipt = await dao.newAppInstance('0x22222', sampleAppBase.address, '0x', false, { from: root }) + app = SampleApp.at(getNewProxyAddress(receipt)) + await app.initialize() + await acl.createPermission(member, app.address, WRITING_ROLE, root, { from: root }) + }) + + beforeEach('create and initialize relayer instance', async () => { + const receipt = await dao.newAppInstance(RELAYER_APP_ID, relayerBase.address, '0x', true, { from: root }) + relayer = Relayer.at(getNewProxyAddress(receipt)) + + await relayer.mockSetTimestamp(NOW) + await relayer.initializeWithChainId(MONTHLY_REFUND_QUOTA, Relayer.network_id) + + await acl.createPermission(root, relayer.address, ALLOW_SENDER_ROLE, root, { from: root }) + await acl.createPermission(root, relayer.address, DISALLOW_SENDER_ROLE, root, { from: root }) + await acl.createPermission(root, relayer.address, SET_MONTHLY_REFUND_QUOTA_ROLE, root, { from: root }) + await acl.createPermission(root, relayer.address, ALLOW_OFF_CHAIN_SERVICE_ROLE, root, { from: root }) + await acl.createPermission(root, relayer.address, DISALLOW_OFF_CHAIN_SERVICE_ROLE, root, { from: root }) + }) + + beforeEach('create and initialize relayer service', async () => { + signer = new RelayTransactionSigner(relayer) + service = new RelayerService(offChainRelayerService, relayer) + await relayer.allowService(offChainRelayerService, { from: root }) + }) + + beforeEach('allow sender and fund relayer', async () => { + await relayer.allowSender(member, { from: root }) + await web3.eth.sendTransaction({ from: vault, to: relayer.address, value: MONTHLY_REFUND_QUOTA, gas: SEND_ETH_GAS }) + }) + + describe('relay', () => { + let data + + beforeEach('build transaction data', async () => { + data = app.contract.write.getData(10) + }) + + context('when the relayed call does not revert', () => { + context('when the given gas amount does cover the transaction cost', () => { + it('relays transactions to app', async () => { + const transaction = await signer.signTransaction({ from: member, to: app.address, data }) + await service.relay(transaction) + + assert.equal((await app.read()).toString(), 10, 'app value does not match') + }) + }) + + context('when the given gas amount does not cover the transaction cost', () => { + const gasRefund = 5000 + + it('relays transactions to app', async () => { + const transaction = await signer.signTransaction({ from: member, to: app.address, data, gasRefund }) + + await assertRejects(service.relay(transaction), /Given gas refund amount \d* does not cover transaction gas cost \d*/) + }) + }) + }) + + context('when the relayed call reverts', () => { + context('when the given gas amount does cover the transaction cost', () => { + it('throws an error', async () => { + const transaction = await signer.signTransaction({ from: member, to: app.address, data }) + transaction.from = someone + + await assertRejects(service.relay(transaction), /Will not relay failing transaction.*RELAYER_SENDER_NOT_ALLOWED/) + }) + }) + + context('when the given gas amount does not cover the transaction cost', () => { + const gasRefund = 5000 + + it('throws an error', async () => { + const transaction = await signer.signTransaction({ from: someone, to: app.address, data, gasRefund }) + + await assertRejects(service.relay(transaction), /Will not relay failing transaction.*RELAYER_SENDER_NOT_ALLOWED/) + }) + }) + }) + }) +}) + +async function assertRejects(promise, regExp) { + let f + try { + await promise + } catch (e) { + f = () => { throw e } + } finally { + assert.throws(f, regExp) + } +} From 5ca36f7b175184d3419ea28c418a618a655e2236 Mon Sep 17 00:00:00 2001 From: Facundo Spagnuolo Date: Fri, 17 May 2019 14:10:23 -0300 Subject: [PATCH 3/4] meta-txs: ignore relayer service tests for coverage --- test/lib/relayer/RelayerService.js | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/test/lib/relayer/RelayerService.js b/test/lib/relayer/RelayerService.js index 81595fff0..de7938ac9 100644 --- a/test/lib/relayer/RelayerService.js +++ b/test/lib/relayer/RelayerService.js @@ -1,3 +1,4 @@ +const { skipCoverage } = require('../../helpers/coverage') const { getEventArgument, getNewProxyAddress } = require('../../helpers/events') const RelayerService = require('../../../lib/relayer/RelayerService')(web3) @@ -91,43 +92,43 @@ contract('RelayerService', ([_, root, member, someone, vault, offChainRelayerSer context('when the relayed call does not revert', () => { context('when the given gas amount does cover the transaction cost', () => { - it('relays transactions to app', async () => { + it('relays transactions to app', skipCoverage(async () => { const transaction = await signer.signTransaction({ from: member, to: app.address, data }) await service.relay(transaction) assert.equal((await app.read()).toString(), 10, 'app value does not match') - }) + })) }) context('when the given gas amount does not cover the transaction cost', () => { const gasRefund = 5000 - it('relays transactions to app', async () => { + it('relays transactions to app', skipCoverage(async () => { const transaction = await signer.signTransaction({ from: member, to: app.address, data, gasRefund }) await assertRejects(service.relay(transaction), /Given gas refund amount \d* does not cover transaction gas cost \d*/) - }) + })) }) }) context('when the relayed call reverts', () => { context('when the given gas amount does cover the transaction cost', () => { - it('throws an error', async () => { + it('throws an error', skipCoverage(async () => { const transaction = await signer.signTransaction({ from: member, to: app.address, data }) transaction.from = someone await assertRejects(service.relay(transaction), /Will not relay failing transaction.*RELAYER_SENDER_NOT_ALLOWED/) - }) + })) }) context('when the given gas amount does not cover the transaction cost', () => { const gasRefund = 5000 - it('throws an error', async () => { + it('throws an error', skipCoverage(async () => { const transaction = await signer.signTransaction({ from: someone, to: app.address, data, gasRefund }) await assertRejects(service.relay(transaction), /Will not relay failing transaction.*RELAYER_SENDER_NOT_ALLOWED/) - }) + })) }) }) }) From 4344738f143197db69b4e51cabeac8abe18fc20f Mon Sep 17 00:00:00 2001 From: Facundo Spagnuolo Date: Fri, 17 May 2019 15:30:57 -0300 Subject: [PATCH 4/4] meta-txs: add kernel validation --- lib/relayer/GasPriceOracle.js | 11 +++- lib/relayer/RelayerService.js | 20 ++++-- test/lib/relayer/RelayerService.js | 100 +++++++++++++++++++---------- 3 files changed, 89 insertions(+), 42 deletions(-) diff --git a/lib/relayer/GasPriceOracle.js b/lib/relayer/GasPriceOracle.js index ea9303753..90648831f 100644 --- a/lib/relayer/GasPriceOracle.js +++ b/lib/relayer/GasPriceOracle.js @@ -1,11 +1,16 @@ const GAS_STATION_API_URL = 'https://ethgasstation.info/json/ethgasAPI.json' -const DEFAULT_GAS_PRICE_VALUE = 1e6 +const DEFAULT_DEVNET_GAS_PRICE = 1e5 +const DEFAULT_TESTNET_GAS_PRICE = 1e6 + +const MAINNET_ID = 1 +const TESTNET_IDS = [2, 3, 42] // ropsten, rinkeby and kovan module.exports = { async fetch(networkId) { - if (networkId === 1) return this._fetchMainnetGasPrice() - else return DEFAULT_GAS_PRICE_VALUE + if (MAINNET_ID === networkId) return this._fetchMainnetGasPrice() + if (TESTNET_IDS.includes(networkId)) return DEFAULT_TESTNET_GAS_PRICE + return DEFAULT_DEVNET_GAS_PRICE }, async _fetchMainnetGasPrice() { diff --git a/lib/relayer/RelayerService.js b/lib/relayer/RelayerService.js index 1844889c9..69605d2f7 100644 --- a/lib/relayer/RelayerService.js +++ b/lib/relayer/RelayerService.js @@ -1,12 +1,13 @@ const GasPriceOracle = require('./GasPriceOracle') -module.exports = web3 => class RelayerService { +module.exports = (artifacts, web3) => class RelayerService { constructor(wallet, relayer) { this.wallet = wallet this.relayer = relayer } async relay(transaction) { + await this._assertTargetIsAragonApp(transaction) await this._assertTransactionWontRevert(transaction) await this._assertTransactionGasCostIsCovered(transaction) await this._assertTransactionReasonableGasPrice(transaction) @@ -26,6 +27,18 @@ module.exports = web3 => class RelayerService { return this.relayer.relay(from, to, nonce, data, gasRefund, gasPrice, signature, txParams) } + async _assertTargetIsAragonApp({ to }) { + let relayerKernel, aragonAppKernel + try { + relayerKernel = await this.relayer.kernel() + aragonAppKernel = await artifacts.require('AragonApp').at(to).kernel() + } catch (error) { + throw Error(`Could not ensure target address is actually an AragonApp from the same Kernel: ${error}`) + } + if (relayerKernel === aragonAppKernel) return; + throw Error(`The Kernel of the target app ${aragonAppKernel} does not match with the Kernel of the current realyer ${relayerKernel}`) + } + async _assertTransactionWontRevert(transaction) { const error = await this._transactionWillFail(transaction) if (!error) return @@ -42,7 +55,6 @@ module.exports = web3 => class RelayerService { } async _assertTransactionReasonableGasPrice(transaction) { - if (!this._isMainnet()) return; const averageGasPrice = await GasPriceOracle.fetch(this._networkId()) if (transaction.gasPrice < averageGasPrice) throw Error(`Given gas price is below the average ${averageGasPrice}`) } @@ -67,10 +79,6 @@ module.exports = web3 => class RelayerService { }) } - _isMainnet() { - return this._networkId() === 1 - } - _networkId() { return this.relayer.constructor.network_id } diff --git a/test/lib/relayer/RelayerService.js b/test/lib/relayer/RelayerService.js index de7938ac9..e02de0976 100644 --- a/test/lib/relayer/RelayerService.js +++ b/test/lib/relayer/RelayerService.js @@ -1,7 +1,7 @@ const { skipCoverage } = require('../../helpers/coverage') const { getEventArgument, getNewProxyAddress } = require('../../helpers/events') -const RelayerService = require('../../../lib/relayer/RelayerService')(web3) +const RelayerService = require('../../../lib/relayer/RelayerService')(artifacts, web3) const RelayTransactionSigner = require('../../../lib/relayer/RelayTransactionSigner')(web3) const ACL = artifacts.require('ACL') @@ -86,50 +86,84 @@ contract('RelayerService', ([_, root, member, someone, vault, offChainRelayerSer describe('relay', () => { let data - beforeEach('build transaction data', async () => { - data = app.contract.write.getData(10) - }) + beforeEach('build transaction data', () => data = app.contract.write.getData(10)) context('when the relayed call does not revert', () => { - context('when the given gas amount does cover the transaction cost', () => { - it('relays transactions to app', skipCoverage(async () => { - const transaction = await signer.signTransaction({ from: member, to: app.address, data }) - await service.relay(transaction) - - assert.equal((await app.read()).toString(), 10, 'app value does not match') - })) + context('when the target address is an aragon app', () => { + context('when the target aragon app belongs to the same DAO', () => { + context('when the given gas amount does cover the transaction cost', () => { + context('when the given gas price is above the average', () => { + it('relays transactions to app', skipCoverage(async () => { + const transaction = await signer.signTransaction({ from: member, to: app.address, data }) + await service.relay(transaction) + + assert.equal((await app.read()).toString(), 10, 'app value does not match') + })) + }) + + context('when the given gas price is below the average', () => { + const gasPrice = 1 + + it('throws an error', skipCoverage(async () => { + const transaction = await signer.signTransaction({ from: member, to: app.address, data, gasPrice }) + + await assertRejects(service.relay(transaction), /Given gas price is below the average \d*/) + })) + }) + }) + + context('when the given gas amount does not cover the transaction cost', () => { + const gasRefund = 5000 + + it('throws an error', skipCoverage(async () => { + const transaction = await signer.signTransaction({ from: member, to: app.address, data, gasRefund }) + + await assertRejects(service.relay(transaction), /Given gas refund amount \d* does not cover transaction gas cost \d*/) + })) + }) + }) + + context('when the target aragon app belongs to another DAO', () => { + let foreignDAO, foreignApp + + beforeEach('deploy app from another DAO', async () => { + const receiptForeignDAO = await daoFactory.newDAO(root) + foreignDAO = Kernel.at(getEventArgument(receiptForeignDAO, 'DeployDAO', 'dao')) + const foreignACL = ACL.at(await foreignDAO.acl()) + await foreignACL.createPermission(root, foreignDAO.address, APP_MANAGER_ROLE, root, { from: root }) + + const receiptForeignApp = await foreignDAO.newAppInstance('0x22222', sampleAppBase.address, '0x', false, { from: root }) + foreignApp = SampleApp.at(getNewProxyAddress(receiptForeignApp)) + foreignApp.initialize() + await foreignACL.createPermission(someone, foreignApp.address, WRITING_ROLE, root, { from: root }) + }) + + it('throws an error', skipCoverage(async () => { + const transaction = await signer.signTransaction({ from: someone, to: foreignApp.address, data }) + + await assertRejects(service.relay(transaction), `The Kernel of the target app ${foreignDAO.address} does not match with the Kernel of the current realyer ${dao.address}`) + })) + }) }) - context('when the given gas amount does not cover the transaction cost', () => { - const gasRefund = 5000 - - it('relays transactions to app', skipCoverage(async () => { - const transaction = await signer.signTransaction({ from: member, to: app.address, data, gasRefund }) + context('when the target address is not an aragon app', () => { + it('throws an error', skipCoverage(async () => { + const transaction = await signer.signTransaction({ from: member, to: someone, data }) - await assertRejects(service.relay(transaction), /Given gas refund amount \d* does not cover transaction gas cost \d*/) + await assertRejects(service.relay(transaction), `The Kernel of the target app 0x does not match with the Kernel of the current realyer ${dao.address}`) })) }) }) context('when the relayed call reverts', () => { - context('when the given gas amount does cover the transaction cost', () => { - it('throws an error', skipCoverage(async () => { - const transaction = await signer.signTransaction({ from: member, to: app.address, data }) - transaction.from = someone + it('throws an error', skipCoverage(async () => { + const transaction = await signer.signTransaction({ from: member, to: app.address, data }) - await assertRejects(service.relay(transaction), /Will not relay failing transaction.*RELAYER_SENDER_NOT_ALLOWED/) - })) - }) - - context('when the given gas amount does not cover the transaction cost', () => { - const gasRefund = 5000 - - it('throws an error', skipCoverage(async () => { - const transaction = await signer.signTransaction({ from: someone, to: app.address, data, gasRefund }) + // change the transaction sender + transaction.from = someone - await assertRejects(service.relay(transaction), /Will not relay failing transaction.*RELAYER_SENDER_NOT_ALLOWED/) - })) - }) + await assertRejects(service.relay(transaction), /Will not relay failing transaction.*RELAYER_SENDER_NOT_ALLOWED/) + })) }) }) })