meta-txs: add kernel validation

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() {

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
   }

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/)
+      }))
     })
   })
 })