From 45a18a13e2568213840c4ba64d963371dd2734f9 Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Tue, 30 Apr 2019 16:34:26 +0200 Subject: [PATCH] wrapper: handle intent baskets (#286) --- .../aragon-wrapper/src/core/aragonOS/index.js | 14 - .../src/core/aragonOS/index.test.js | 33 +- .../src/core/aragonOS/kernel.js | 56 +++ .../src/core/aragonOS/kernel.test.js | 105 ++++++ packages/aragon-wrapper/src/index.js | 333 ++++++++---------- packages/aragon-wrapper/src/index.test.js | 4 +- packages/aragon-wrapper/src/radspec/index.js | 88 +++++ .../aragon-wrapper/src/templates/index.js | 2 +- packages/aragon-wrapper/src/utils/apps.js | 26 ++ packages/aragon-wrapper/src/utils/index.js | 20 -- packages/aragon-wrapper/src/utils/intents.js | 37 ++ .../aragon-wrapper/src/utils/transactions.js | 110 ++++++ 12 files changed, 588 insertions(+), 240 deletions(-) create mode 100644 packages/aragon-wrapper/src/core/aragonOS/kernel.js create mode 100644 packages/aragon-wrapper/src/core/aragonOS/kernel.test.js create mode 100644 packages/aragon-wrapper/src/radspec/index.js create mode 100644 packages/aragon-wrapper/src/utils/intents.js create mode 100644 packages/aragon-wrapper/src/utils/transactions.js diff --git a/packages/aragon-wrapper/src/core/aragonOS/index.js b/packages/aragon-wrapper/src/core/aragonOS/index.js index bc4a00b3..8b68efa4 100644 --- a/packages/aragon-wrapper/src/core/aragonOS/index.js +++ b/packages/aragon-wrapper/src/core/aragonOS/index.js @@ -1,12 +1,5 @@ -import { soliditySha3 } from 'web3-utils' import { getAppInfo, hasAppInfo } from '../../interfaces' -const KERNEL_NAMESPACES = new Map([ - [soliditySha3('core'), 'Core'], - [soliditySha3('app'), 'Default apps'], - [soliditySha3('base'), 'App code'] -]) - function getAragonOsInternalAppInfo (appId) { const appInfo = getAppInfo(appId, 'aragon') @@ -16,18 +9,11 @@ function getAragonOsInternalAppInfo (appId) { } } -function getKernelNamespace (hash) { - return KERNEL_NAMESPACES.has(hash) - ? { name: KERNEL_NAMESPACES.get(hash), hash } - : null -} - function isAragonOsInternalApp (appId) { return hasAppInfo(appId, 'aragon') } export { getAragonOsInternalAppInfo, - getKernelNamespace, isAragonOsInternalApp } diff --git a/packages/aragon-wrapper/src/core/aragonOS/index.test.js b/packages/aragon-wrapper/src/core/aragonOS/index.test.js index cfbf837a..9dbba5db 100644 --- a/packages/aragon-wrapper/src/core/aragonOS/index.test.js +++ b/packages/aragon-wrapper/src/core/aragonOS/index.test.js @@ -6,21 +6,23 @@ test.afterEach.always(() => { sinon.restore() }) -test('aragonOS: getKernelNamespace', async (t) => { - t.plan(3) +test('aragonOS: getAragonOsInternalAppInfo', async (t) => { + t.plan(4) // arrange - // soliditySha3('core') - const coreNamespaceHash = '0xc681a85306374a5ab27f0bbc385296a54bcd314a1948b6cf61c4ea1bc44bb9f8' + // namehash('acl.aragonpm.eth') + const aclNamehash = '0xe3262375f45a6e2026b7e7b18c2b807434f2508fe1a2a3dfb493c7df8f4aad6a' // act - const result = aragonOS.getKernelNamespace(coreNamespaceHash) - const emptyResult = aragonOS.getKernelNamespace() + const result = aragonOS.getAragonOsInternalAppInfo(aclNamehash) + const emptyResult = aragonOS.getAragonOsInternalAppInfo() // assert - t.is(result.hash, coreNamespaceHash) - t.is(result.name, 'Core') + t.is(result.name, 'ACL') + t.true(Array.isArray(result.abi)) + t.is(result.isAragonOsInternalApp, true) t.is(emptyResult, null) }) test('aragonOS: isAragonOsInternalApp', async (t) => { + t.plan(2) // arrange // namehash('acl.aragonpm.eth') const aclNamehash = '0xe3262375f45a6e2026b7e7b18c2b807434f2508fe1a2a3dfb493c7df8f4aad6a' @@ -31,18 +33,3 @@ test('aragonOS: isAragonOsInternalApp', async (t) => { t.true(result) t.false(emptyResult) }) - -test('aragonOS: getAragonOsInternalAppInfo', async (t) => { - t.plan(4) - // arrange - // namehash('acl.aragonpm.eth') - const aclNamehash = '0xe3262375f45a6e2026b7e7b18c2b807434f2508fe1a2a3dfb493c7df8f4aad6a' - // act - const result = aragonOS.getAragonOsInternalAppInfo(aclNamehash) - const emptyResult = aragonOS.getAragonOsInternalAppInfo() - // assert - t.is(result.name, 'ACL') - t.true(Array.isArray(result.abi)) - t.is(result.isAragonOsInternalApp, true) - t.is(emptyResult, null) -}) diff --git a/packages/aragon-wrapper/src/core/aragonOS/kernel.js b/packages/aragon-wrapper/src/core/aragonOS/kernel.js new file mode 100644 index 00000000..5519d060 --- /dev/null +++ b/packages/aragon-wrapper/src/core/aragonOS/kernel.js @@ -0,0 +1,56 @@ +import abi from 'web3-eth-abi' +import { soliditySha3 } from 'web3-utils' + +import { addressesEqual } from '../../utils' +import { findAppMethodFromData } from '../../utils/apps' + +const SET_APP_ABI = [ + { name: 'namespace', type: 'bytes32' }, + { name: 'appId', type: 'bytes32' }, + { name: 'appAddress', type: 'address' } +] + +const CORE_NAMESPACE = soliditySha3('core') +const APP_ADDR_NAMESPACE = soliditySha3('app') +const APP_BASES_NAMESPACE = soliditySha3('base') +const KERNEL_NAMESPACES_NAMES = new Map([ + [CORE_NAMESPACE, 'Core'], + [APP_ADDR_NAMESPACE, 'Default apps'], + [APP_BASES_NAMESPACE, 'App code'] +]) + +/** + * Decode `Kernel.setApp()` parameters based on transaction data. + * + * @param {Object} data Transaction data + * @return {Object} Decoded parameters for `setApp()` (namespace, appId, appAddress) + */ +export function decodeKernelSetAppParameters (data) { + // Strip 0x prefix + bytes4 sig to get parameter data + const paramData = data.substring(10) + return abi.decodeParameters(SET_APP_ABI, paramData) +} + +export function getKernelNamespace (hash) { + return KERNEL_NAMESPACES_NAMES.has(hash) + ? { name: KERNEL_NAMESPACES_NAMES.get(hash), hash } + : null +} + +export function isKernelAppCodeNamespace (namespaceHash) { + return namespaceHash === APP_BASES_NAMESPACE +} + +/** + * Is the transaction intent for `Kernel.setApp()`? + * + * @param {Object} kernelApp App artifact for Kernel + * @param {Object} intent Transaction intent + * @return {Boolean} Whether the intent is `Kernel.setApp()` + */ +export function isKernelSetAppIntent (kernelApp, intent) { + if (!addressesEqual(kernelApp.proxyAddress, intent.to)) return false + + const method = findAppMethodFromData(kernelApp, intent.data) + return !!method && method.sig === 'setApp(bytes32,bytes32,address)' +} diff --git a/packages/aragon-wrapper/src/core/aragonOS/kernel.test.js b/packages/aragon-wrapper/src/core/aragonOS/kernel.test.js new file mode 100644 index 00000000..3bbcaeeb --- /dev/null +++ b/packages/aragon-wrapper/src/core/aragonOS/kernel.test.js @@ -0,0 +1,105 @@ +import test from 'ava' +import sinon from 'sinon' +import { soliditySha3 } from 'web3-utils' +import * as kernel from './kernel' + +test.afterEach.always(() => { + sinon.restore() +}) + +// Kernel.setApp(APP_BASES_NAMESPACE, namehash('voting.aragonpm.eth'), '0x7FB5C052A391953De579b0ed7dC87aD59a9A5473') +const SET_APP_TX_DATA = '0xae5b2540f1f3eb40f5bc1ad1344716ced8b8a0431d840b5783aea1fd01786bc26f35ac0f9fa3927f639745e587912d4b0fea7ef9013bf93fb907d29faeab57417ba6e1d40000000000000000000000007fb5c052a391953de579b0ed7dc87ad59a9a5473' +// Kernel.initialize('0xd95677b5b3bc3c89c4c2c3ab702b0aa5d5cb28af', '0x0000000000000000000000000000000000000000') +const INITIALIZE_TX_DATA = '0x485cc955000000000000000000000000d95677b5b3bc3c89c4c2c3ab702b0aa5d5cb28af0000000000000000000000000000000000000000000000000000000000000000' + +test('aragonOS/kernel: decodeKernelSetAppParameters', async (t) => { + t.plan(4) + // arrange + const setAppData = SET_APP_TX_DATA + const notSetAppData = INITIALIZE_TX_DATA + // act + const decodedData = kernel.decodeKernelSetAppParameters(setAppData) + // assert + t.is(decodedData.namespace, '0xf1f3eb40f5bc1ad1344716ced8b8a0431d840b5783aea1fd01786bc26f35ac0f') + t.is(decodedData.appId, '0x9fa3927f639745e587912d4b0fea7ef9013bf93fb907d29faeab57417ba6e1d4') + t.is(decodedData.appAddress, '0x7FB5C052A391953De579b0ed7dC87aD59a9A5473') + + t.throws( + () => kernel.decodeKernelSetAppParameters(notSetAppData), + { + instanceOf: Error + } + ) +}) + +test('aragonOS/kernel: getKernelNamespace', async (t) => { + t.plan(3) + // arrange + // soliditySha3('core') + const coreNamespaceHash = '0xc681a85306374a5ab27f0bbc385296a54bcd314a1948b6cf61c4ea1bc44bb9f8' + // act + const result = kernel.getKernelNamespace(coreNamespaceHash) + const emptyResult = kernel.getKernelNamespace() + // assert + t.is(result.hash, coreNamespaceHash) + t.is(result.name, 'Core') + t.is(emptyResult, null) +}) + +test('aragonOS/kernel: isKernelAppCodeNamespace', async (t) => { + t.plan(2) + // arrange + const appCodeNamespace = soliditySha3('base') + // act + const result = kernel.isKernelAppCodeNamespace(appCodeNamespace) + const emptyResult = kernel.isKernelAppCodeNamespace() + // assert + t.true(result) + t.false(emptyResult) +}) + +test('aragonOS/kernel: isKernelSetAppIntent', async (t) => { + t.plan(3) + + // arrange + const kernelApp = { + proxyAddress: '0x123', + functions: [ + { + sig: 'initialize(address,address)', + roles: [], + notice: 'Initializes a kernel instance along with its ACL and sets `_permissionsCreator` as the entity that can create other permissions' + }, + { + sig: 'setApp(bytes32,bytes32,address)', + roles: [ + 'APP_MANAGER_ROLE' + ], + notice: 'Set the resolving address of `_appId` in namespace `_namespace` to `_app`' + } + ] + } + const setAppIntent = { + to: '0x123', + data: SET_APP_TX_DATA + } + const initializeIntent = { + to: '0x123', + data: INITIALIZE_TX_DATA + } + const otherAppIntent = { + to: '0x456', + // vote(0, true) + data: '0xc9d27afe00000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000001' + } + + // act + const setAppResult = kernel.isKernelSetAppIntent(kernelApp, setAppIntent) + const initializeResult = kernel.isKernelSetAppIntent(kernelApp, initializeIntent) + const otherAppResult = kernel.isKernelSetAppIntent(kernelApp, otherAppIntent) + + // assert + t.true(setAppResult) + t.false(initializeResult) + t.false(otherAppResult) +}) diff --git a/packages/aragon-wrapper/src/index.js b/packages/aragon-wrapper/src/index.js index 6357a755..606668e7 100644 --- a/packages/aragon-wrapper/src/index.js +++ b/packages/aragon-wrapper/src/index.js @@ -19,13 +19,10 @@ import { } from 'rxjs/operators' import uuidv4 from 'uuid/v4' import Web3 from 'web3' -import abi from 'web3-eth-abi' -import { isAddress, toBN } from 'web3-utils' +import { isAddress } from 'web3-utils' import dotprop from 'dot-prop' -import * as radspec from 'radspec' // APM -import { keccak256 } from 'js-sha3' import apm from '@aragon/apm' // RPC @@ -37,20 +34,30 @@ import { getApmAppInfo } from './core/apm' import { makeRepoProxy, getAllRepoVersions, getRepoVersionById } from './core/apm/repo' import { getAragonOsInternalAppInfo, - getKernelNamespace, isAragonOsInternalApp } from './core/aragonOS' +import { getKernelNamespace, isKernelAppCodeNamespace } from './core/aragonOS/kernel' import { CALLSCRIPT_ID, encodeCallScript } from './evmscript' +import { + tryDescribingUpdateAppIntent, + tryDescribingUpgradeOrganizationBasket, + tryEvaluatingRadspec +} from './radspec' import { addressesEqual, includesAddress, makeAddressMapProxy, makeProxy, makeProxyFromABI, - getRecommendedGasLimit, AsyncRequestCache, ANY_ENTITY } from './utils' +import { doIntentPathsMatch } from './utils/intents' +import { + createDirectTransaction, + createForwarderTransactionBuilder, + getRecommendedGasLimit +} from './utils/transactions' // Templates import Templates from './templates' @@ -413,10 +420,7 @@ export default class Aragon { .events('SetApp', {}) .pipe( // Only care about changes if they're in the APP_BASE namespace - filter(({ returnValues }) => { - const namespace = getKernelNamespace(returnValues.namespace) - return namespace && namespace.name === 'App code' - }), + filter(({ returnValues }) => isKernelAppCodeNamespace(returnValues.namespace)), // Merge with latest value of installedApps$ so we can return the full list of apps withLatestFrom( @@ -1241,13 +1245,116 @@ export default class Aragon { ) if (path.length > 0) { - return this.describeTransactionPath(path) + try { + return this.describeTransactionPath(path) + } catch (_) { } } } return [] } + /** + * Calculate the transaction path for a basket of intents. + * Expects the `intentBasket` to be an array of tuples holding the following: + * {string} destination: destination address + * {string} methodName: method to invoke on destination + * {Array<*>} params: method params + * These are the same parameters as the ones used for `getTransactionPath()` + * + * Allows user to specify how many of the intents should be checked to ensure their paths are + * compatible. `checkMode` supports: + * 'all': All intents will be checked to make sure they use the same forwarding path. + * 'single': assumes all intents can use the path found from the first intent + * + * @param {Array>>} intentBasket Intents + * @param {Object} [options] + * @param {string} [options.checkMode] Path checking mode + * @return {Promise} An object containing: + * - `direct`: whether the current account can directly invoke this basket + * (requiring separate transactions) + * - `transactions`: array of Ethereum transactions that describe each step in the path, with + * the last step being an array of transactions that describe each intent in the basket + */ + async getTransactionPathForIntentBasket (intentBasket, { checkMode = 'all' } = {}) { + // Get transaction paths for entire basket + const intentsToCheck = + checkMode === 'all' + ? intentBasket // all -- use all intents + : checkMode === 'single' + ? [intentBasket[0]] // single -- only use first intent + : [] + const intentPaths = await Promise.all( + intentsToCheck.map( + ([destination, methodName, params]) => + this.getTransactionPath(destination, methodName, params)) + ) + + // If the paths don't match, we can't send the transactions in this intent basket together + const pathsMatch = doIntentPathsMatch(intentPaths) + if (pathsMatch) { + // Create direct transactions for each intent in the intentBasket + const sender = (await this.getAccounts())[0] // TODO: don't assume it's the first account + const directTransactions = await Promise.all( + intentBasket.map( + async ([destination, methodName, params]) => + createDirectTransaction(sender, await this.getApp(destination), methodName, params, this.web3) + ) + ) + + if (intentPaths[0].length === 1) { + // Sender has direct access + try { + const decoratedTransactions = await this.describeTransactionPath( + await Promise.all( + directTransactions.map(transaction => this.applyTransactionGas(transaction)) + ) + ) + + return { + direct: true, + transactions: decoratedTransactions + } + } catch (_) { } + } else { + // Need to encode calls scripts for each forwarder transaction in the path + const createForwarderTransaction = createForwarderTransactionBuilder(sender, {}, this.web3) + const forwarderPath = intentPaths[0] + // Ignore the last part of the path, which was the original intent + .slice(0, -1) + // Start from the "last" forwarder and move backwards to the sender + .reverse() + // Just use the forwarders' addresses + .map(({ to }) => to) + .reduce( + (path, nextForwarder) => { + const lastStep = path[0] + const encodedLastStep = encodeCallScript(Array.isArray(lastStep) ? lastStep : [lastStep]) + return [createForwarderTransaction(nextForwarder, encodedLastStep), ...path] + }, + // Start the recursive calls script encoding with the direct transactions for the + // intent basket + [directTransactions] + ) + + try { + // Put the finishing touches: apply gas, and add radspec descriptions + forwarderPath[0] = await this.applyTransactionGas(forwarderPath[0], true) + return { + direct: false, + path: await this.describeTransactionPath(forwarderPath) + } + } catch (_) { } + } + } + + // Failed to find a path + return { + direct: false, + transactions: [] + } + } + /** * Get the permission manager for an `app`'s and `role`. * @@ -1360,109 +1467,50 @@ export default class Aragon { * Use radspec to create a human-readable description for each transaction in the given `path` * * @param {Array} path - * @return {Promise>} The given `path`, with descriptions included at each step + * @return {Promise>} The given `path`, with decorated with descriptions at each step */ describeTransactionPath (path) { return Promise.all(path.map(async (step) => { - const app = await this.getApp(step.to) - - // No app artifact - if (!app) return step + let decoratedStep - // Missing methods in artifact - if (!app.functions) return step - - // Find the method - const methodId = step.data.substring(2, 10) - const method = app.functions.find( - (method) => keccak256(method.sig).substring(0, 8) === methodId - ) - - // Method does not exist in artifact - if (!method) return step - - const expression = method.notice + if (Array.isArray(step)) { + // Intent basket with multiple transactions in a single callscript + // First see if the step can be handled with a specialized descriptor + try { + decoratedStep = tryDescribingUpgradeOrganizationBasket(step, this) + } catch (err) { } - // No expression - if (!expression) return step + // If the step wasn't handled, just individually describe each of the transactions + // TODO: annotate this description + return decoratedStep || Promise.all(step.map(this.describeTransactionPath)) + } - let description - let annotatedDescription + // Single transaction step + // First see if the step can be handled with a specialized descriptor + try { + decoratedStep = await tryDescribingUpdateAppIntent(step, this) + } catch (err) { } - // Detect if we should try to handle the description for this step ourselves - if (this.isKernelAddress(step.to) && method.sig === 'setApp(bytes32,bytes32,address)') { - // setApp() call; attempt to make a more friendly description + // Finally, if the step wasn't handled yet, evaluate via radspec normally + if (!decoratedStep) { try { - description = await this.tryDescribingUpdateAppIntent(step) + decoratedStep = await tryEvaluatingRadspec(step, this) } catch (err) { } } - if (!description) { - // Use radspec normally + // Annotate the description, if one was found + if (decoratedStep.description) { try { - description = await radspec.evaluate( - expression, - { - abi: app.abi, - transaction: step - }, - { ethNode: this.web3.currentProvider } - ) + const processed = await this.postprocessRadspecDescription(decoratedStep.description) + decoratedStep.description = processed.description + decoratedStep.annotatedDescription = processed.annotatedDescription } catch (err) { } } - if (description) { - const processed = await this.postprocessRadspecDescription(description) - description = processed.description - annotatedDescription = processed.annotatedDescription - } - - return Object.assign(step, { - description, - annotatedDescription, - name: app.name, - identifier: app.identifier - }) + return decoratedStep })) } - /** - * Attempt to describe a setApp() intent. Only describes the APP_BASE namespace. - * - * @param {string} description - * @return {Promise} Description, if one could be made. - */ - async tryDescribingUpdateAppIntent (intent) { - // Strip 0x prefix + bytes4 sig to get parameter data - const txData = intent.data.substring(10) - const types = [ - { - type: 'bytes32', - name: 'namespace' - }, { - type: 'bytes32', - name: 'appId' - }, { - type: 'address', - name: 'appAddress' - } - ] - const { appId, appAddress, namespace } = abi.decodeParameters(types, txData) - - const kernelNamespace = getKernelNamespace(namespace) - if (!kernelNamespace || kernelNamespace.name !== 'App code') { - return - } - - // Fetch aragonPM information - const repo = await makeRepoProxy(appId, this.apm, this.web3) - const latestVersion = (await repo.call('getLatestForContractAddress', appAddress)) - .semanticVersion - .join('.') - - return `Upgrade ${appId} app instances to v${latestVersion}.` - } - /** * Look for known addresses and roles in a radspec description and substitute them with a human string * @@ -1636,73 +1684,7 @@ export default class Aragon { const permissions = await this.permissions.pipe(first()).toPromise() const app = await this.getApp(destination) - - if (!app) { - throw new Error(`No artifact found for ${destination}`) - } - - const jsonInterface = app.abi - if (!jsonInterface) { - throw new Error(`No ABI specified in artifact for ${destination}`) - } - - const methodABI = app.abi.find( - (method) => method.name === methodName - ) - if (!methodABI) { - throw new Error(`${methodName} not found on ABI for ${destination}`) - } - - let transactionOptions = {} - - // If an extra parameter has been provided, it is the transaction options if it is an object - if (methodABI.inputs.length + 1 === params.length && typeof params[params.length - 1] === 'object') { - const options = params.pop() - transactionOptions = { ...transactionOptions, ...options } - } - - // The direct transaction we eventually want to perform - const directTransaction = { - ...transactionOptions, // Options are overwriten by the values below - from: sender, - to: destination, - data: this.web3.eth.abi.encodeFunctionCall(methodABI, params) - } - - if (transactionOptions.token) { - const { address: tokenAddress, value: tokenValue } = transactionOptions.token - - const erc20ABI = getAbi('standard/ERC20') - const tokenContract = new this.web3.eth.Contract(erc20ABI, tokenAddress) - const balance = await tokenContract.methods.balanceOf(sender).call() - - const tokenValueBN = toBN(tokenValue) - - if (toBN(balance).lt(tokenValueBN)) { - throw new Error(`Balance too low. ${sender} balance of ${tokenAddress} token is ${balance} (attempting to send ${tokenValue})`) - } - - const allowance = await tokenContract.methods.allowance(sender, destination).call() - const allowanceBN = toBN(allowance) - - // If allowance is already greater than or equal to amount, there is no need to do an approve transaction - if (allowanceBN.lt(tokenValueBN)) { - if (allowanceBN.gt(toBN(0))) { - // TODO: Actually handle existing approvals (some tokens fail when the current allowance is not 0) - console.warn(`${sender} already approved ${destination}. In some tokens, approval will fail unless the allowance is reset to 0 before re-approving again.`) - } - - const tokenApproveTransaction = { - // TODO: should we include transaction options? - from: sender, - to: tokenAddress, - data: tokenContract.methods.approve(destination, tokenValue).encodeABI() - } - - directTransaction.pretransaction = tokenApproveTransaction - delete transactionOptions.token - } - } + const directTransaction = await createDirectTransaction(sender, app, methodName, params, this.web3) let appsWithPermissionForMethod = [] @@ -1726,7 +1708,13 @@ export default class Aragon { // If the method has no ACL requirements, we assume we // can perform the action directly if (method.roles.length === 0) { - return [directTransaction] + try { + // `applyTransactionGas` can throw if the transaction will fail + // If that happens, we give up as we should've been able to perform the action directly + return [await this.applyTransactionGas(directTransaction)] + } catch (_) { + return [] + } } const roleSig = app.roles.find( @@ -1747,7 +1735,7 @@ export default class Aragon { try { // `applyTransactionGas` can throw if the transaction will fail - // if that happens, we will try to find a transaction path through a forwarder + // If that happens, we will try to find a transaction path through a forwarder return [await this.applyTransactionGas(directTransaction)] } catch (_) { } } @@ -1797,20 +1785,7 @@ export default class Aragon { const pretransaction = directTransaction.pretransaction delete directTransaction.pretransaction - // TODO: No need for contract? - // A helper method to create a transaction that calls `forward` on a forwarder with `script` - const forwardMethod = new this.web3.eth.Contract( - getAbi('aragon/Forwarder') - ).methods['forward'] - - const createForwarderTransaction = (forwarderAddress, script) => ( - { - ...directTransaction, // Options are overwriten by the values below - from: sender, - to: forwarderAddress, - data: forwardMethod(script).encodeABI() - } - ) + const createForwarderTransaction = createForwarderTransactionBuilder(sender, directTransaction, this.web3) // Check if one of the forwarders that has permission to perform an action // with `sig` on `address` can forward for us directly diff --git a/packages/aragon-wrapper/src/index.test.js b/packages/aragon-wrapper/src/index.test.js index e08a2d02..d0002430 100644 --- a/packages/aragon-wrapper/src/index.test.js +++ b/packages/aragon-wrapper/src/index.test.js @@ -3,7 +3,6 @@ import sinon from 'sinon' import proxyquire from 'proxyquire' import { Subject, empty, of, from } from 'rxjs' import { first } from 'rxjs/operators' -import { getKernelNamespace } from './core/aragonOS' import { encodeCallScript } from './evmscript' import AsyncRequestCache from './utils/AsyncRequestCache' @@ -15,8 +14,7 @@ const CORE_NAMESPACE_HASH = '0xc681a85306374a5ab27f0bbc385296a54bcd314a1948b6cf6 test.beforeEach(t => { const apmStub = sinon.stub() const aragonOSCoreStub = { - getAragonOsInternalAppInfo: sinon.stub(), - getKernelNamespace + getAragonOsInternalAppInfo: sinon.stub() } const apmCoreStub = { getApmAppInfo: sinon.stub() diff --git a/packages/aragon-wrapper/src/radspec/index.js b/packages/aragon-wrapper/src/radspec/index.js new file mode 100644 index 00000000..17a0daea --- /dev/null +++ b/packages/aragon-wrapper/src/radspec/index.js @@ -0,0 +1,88 @@ +import * as radspec from 'radspec' +import { makeRepoProxy } from '../core/apm/repo' +import { findAppMethodFromData, knownAppIds } from '../utils/apps' +import { filterAndDecodeAppUpgradeIntents } from '../utils/intents' + +/** + * Attempt to describe intent via radspec. + * + * @param {Object} intent transaction intent + * @param {Object} wrapper + * @return {Promise} Decorated intent with description, if one could be made + */ +export async function tryEvaluatingRadspec (intent, wrapper) { + const app = await wrapper.getApp(intent.to) + const method = findAppMethodFromData(app, intent.data) + + let evaluatedNotice + if (method.notice) { + try { + evaluatedNotice = await radspec.evaluate( + method.notice, + { + abi: app.abi, + transaction: intent + }, + { ethNode: wrapper.web3.currentProvider } + ) + } catch (err) { } + } + + return { + ...intent, + description: evaluatedNotice + } +} + +/** + * Attempt to describe a setApp() intent. Only describes the APP_BASE namespace. + * + * @param {Object} intent transaction intent + * @param {Object} wrapper + * @return {Promise} Decorated intent with description, if one could be made + */ +export async function tryDescribingUpdateAppIntent (intent, wrapper) { + const upgradeIntentParams = (await filterAndDecodeAppUpgradeIntents([intent], wrapper))[0] + if (!upgradeIntentParams) return + + const { appId, appAddress } = upgradeIntentParams + // Fetch aragonPM information + const repo = await makeRepoProxy(appId, wrapper.apm, wrapper.web3) + const latestVersion = (await repo.call('getLatestForContractAddress', appAddress)) + .semanticVersion + .join('.') + + return { + ...intent, + description: `Upgrade ${appId} app instances to v${latestVersion}` + } +} + +/** + * Attempt to parse a complete organization upgrade intent + * + * @param {Array} intents intent basket + * @param {Object} wrapper + * @return {Promise} Decorated intent with description, if one could be made + */ +export async function tryDescribingUpgradeOrganizationBasket (intents, wrapper) { + const upgradedKnownAppIds = (await filterAndDecodeAppUpgradeIntents(intents, wrapper)) + .map(({ appId }) => appId) + // Take intersection with knownAppIds + .filter( + appId => knownAppIds.includes(appId) + ) + + if ( + // All intents are for upgrading known apps + intents.length === upgradedKnownAppIds.length && + // All known apps are being upgraded + knownAppIds.length === upgradedKnownAppIds.length + ) { + return { + description: 'Upgrade organization to Aragon 0.7 Bella', + from: intents[0].from, + to: intents[0].to + } + } +} diff --git a/packages/aragon-wrapper/src/templates/index.js b/packages/aragon-wrapper/src/templates/index.js index 34c0393d..a127f2b5 100644 --- a/packages/aragon-wrapper/src/templates/index.js +++ b/packages/aragon-wrapper/src/templates/index.js @@ -1,5 +1,5 @@ import { resolve as ensResolve } from '../ens' -import { getRecommendedGasLimit } from '../utils' +import { getRecommendedGasLimit } from '../utils/transactions' const zeroAddress = '0x0000000000000000000000000000000000000000' diff --git a/packages/aragon-wrapper/src/utils/apps.js b/packages/aragon-wrapper/src/utils/apps.js index c6d8ccdc..ceb9f502 100644 --- a/packages/aragon-wrapper/src/utils/apps.js +++ b/packages/aragon-wrapper/src/utils/apps.js @@ -1,3 +1,29 @@ import { hash as namehash } from 'eth-ens-namehash' +import { keccak256 } from 'js-sha3' export const apmAppId = appName => namehash(`${appName}.aragonpm.eth`) + +/** + * Find the method descriptor corresponding to the data component of a + * transaction sent to `app`. + * + * @param {Object} app App artifact + * @param {Object} data Data component of a transaction to app + * @return {Object} Method with radspec notice and function signature + */ +export function findAppMethodFromData (app, data) { + if (app && app.functions) { + // Find the method + const methodId = data.substring(2, 10) + return app.functions.find( + (method) => keccak256(method.sig).substring(0, 8) === methodId + ) + } +} + +export const knownAppIds = [ + apmAppId('finance'), + apmAppId('token-manager'), + apmAppId('vault'), + apmAppId('voting') +] diff --git a/packages/aragon-wrapper/src/utils/index.js b/packages/aragon-wrapper/src/utils/index.js index e971ecf5..73604c48 100644 --- a/packages/aragon-wrapper/src/utils/index.js +++ b/packages/aragon-wrapper/src/utils/index.js @@ -2,9 +2,6 @@ import { isAddress } from 'web3-utils' import ContractProxy from '../core/proxy' import { getAbi } from '../interfaces' -const DEFAULT_GAS_FUZZ_FACTOR = 1.5 -const PREVIOUS_BLOCK_GAS_LIMIT_FACTOR = 0.95 - export const ANY_ENTITY = '0xFFfFfFffFFfffFFfFFfFFFFFffFFFffffFfFFFfF' // Check address equality without checksums @@ -51,21 +48,4 @@ export function makeProxyFromABI (address, abi, web3, initializationBlock) { return new ContractProxy(address, abi, web3, initializationBlock) } -export async function getRecommendedGasLimit (web3, estimatedGasLimit, { gasFuzzFactor = DEFAULT_GAS_FUZZ_FACTOR } = {}) { - const latestBlock = await web3.eth.getBlock('latest') - const latestBlockGasLimit = latestBlock.gasLimit - - const upperGasLimit = Math.round(latestBlockGasLimit * PREVIOUS_BLOCK_GAS_LIMIT_FACTOR) - const bufferedGasLimit = Math.round(estimatedGasLimit * gasFuzzFactor) - - if (estimatedGasLimit > upperGasLimit) { - // TODO: Consider whether we should throw an error rather than returning with a high gas limit - return estimatedGasLimit - } else if (bufferedGasLimit < upperGasLimit) { - return bufferedGasLimit - } else { - return upperGasLimit - } -} - export { default as AsyncRequestCache } from './AsyncRequestCache' diff --git a/packages/aragon-wrapper/src/utils/intents.js b/packages/aragon-wrapper/src/utils/intents.js new file mode 100644 index 00000000..d1f0dab2 --- /dev/null +++ b/packages/aragon-wrapper/src/utils/intents.js @@ -0,0 +1,37 @@ +import { + decodeKernelSetAppParameters, + isKernelAppCodeNamespace, + isKernelSetAppIntent +} from '../core/aragonOS/kernel' + +export function doIntentPathsMatch (intentPaths) { + const individualPaths = intentPaths + // Map each path to just be an array of destination addresses + .map(path => + path.map(({ to }) => to) + ) + // Take each array of destination addresses and create a single string + .map(path => path.join('.')) + + // Check if they all match by seeing if a unique set of the individual path + // strings is a single path + return (new Set(individualPaths)).size === 1 +} + +export async function filterAndDecodeAppUpgradeIntents (intents, wrapper) { + const kernelApp = await wrapper.getApp(wrapper.kernelProxy.address) + + return intents + // Filter for setApp() calls to the kernel + .filter((intent) => isKernelSetAppIntent(kernelApp, intent)) + // Try to decode setApp() params + .map((intent) => { + try { + return decodeKernelSetAppParameters(intent.data) + } catch (_) {} + + return {} + }) + // Filter for changes to APP_BASES_NAMESPACE + .filter(({ namespace }) => isKernelAppCodeNamespace(namespace)) +} diff --git a/packages/aragon-wrapper/src/utils/transactions.js b/packages/aragon-wrapper/src/utils/transactions.js new file mode 100644 index 00000000..64aec57f --- /dev/null +++ b/packages/aragon-wrapper/src/utils/transactions.js @@ -0,0 +1,110 @@ +import { toBN } from 'web3-utils' +import { getAbi } from '../interfaces' + +const DEFAULT_GAS_FUZZ_FACTOR = 1.5 +const PREVIOUS_BLOCK_GAS_LIMIT_FACTOR = 0.95 + +export async function createDirectTransaction (sender, app, methodName, params, web3) { + if (!app) { + throw new Error(`Could not create transaction due to missing app artifact`) + } + + const { proxyAddress: destination } = app + + const jsonInterface = app.abi + if (!jsonInterface) { + throw new Error(`No ABI specified in artifact for ${destination}`) + } + + const methodABI = app.abi.find( + (method) => method.name === methodName + ) + if (!methodABI) { + throw new Error(`${methodName} not found on ABI for ${destination}`) + } + + let transactionOptions = {} + + // If an extra parameter has been provided, it is the transaction options if it is an object + if (methodABI.inputs.length + 1 === params.length && typeof params[params.length - 1] === 'object') { + const options = params.pop() + transactionOptions = { ...transactionOptions, ...options } + } + + // The direct transaction we eventually want to perform + const directTransaction = { + ...transactionOptions, // Options are overwriten by the values below + from: sender, + to: destination, + data: web3.eth.abi.encodeFunctionCall(methodABI, params) + } + + if (transactionOptions.token) { + const { address: tokenAddress, value: tokenValue } = transactionOptions.token + + const erc20ABI = getAbi('standard/ERC20') + const tokenContract = new web3.eth.Contract(erc20ABI, tokenAddress) + const balance = await tokenContract.methods.balanceOf(sender).call() + + const tokenValueBN = toBN(tokenValue) + + if (toBN(balance).lt(tokenValueBN)) { + throw new Error(`Balance too low. ${sender} balance of ${tokenAddress} token is ${balance} (attempting to send ${tokenValue})`) + } + + const allowance = await tokenContract.methods.allowance(sender, destination).call() + const allowanceBN = toBN(allowance) + + // If allowance is already greater than or equal to amount, there is no need to do an approve transaction + if (allowanceBN.lt(tokenValueBN)) { + if (allowanceBN.gt(toBN(0))) { + // TODO: Actually handle existing approvals (some tokens fail when the current allowance is not 0) + console.warn(`${sender} already approved ${destination}. In some tokens, approval will fail unless the allowance is reset to 0 before re-approving again.`) + } + + const tokenApproveTransaction = { + // TODO: should we include transaction options? + from: sender, + to: tokenAddress, + data: tokenContract.methods.approve(destination, tokenValue).encodeABI() + } + + directTransaction.pretransaction = tokenApproveTransaction + delete transactionOptions.token + } + } + + return directTransaction +} + +export function createForwarderTransactionBuilder (sender, directTransaction, web3) { + const forwardMethod = new web3.eth.Contract( + getAbi('aragon/Forwarder') + ).methods['forward'] + + return (forwarderAddress, script) => ( + { + ...directTransaction, // Options are overwriten by the values below + from: sender, + to: forwarderAddress, + data: forwardMethod(script).encodeABI() + } + ) +} + +export async function getRecommendedGasLimit (web3, estimatedGasLimit, { gasFuzzFactor = DEFAULT_GAS_FUZZ_FACTOR } = {}) { + const latestBlock = await web3.eth.getBlock('latest') + const latestBlockGasLimit = latestBlock.gasLimit + + const upperGasLimit = Math.round(latestBlockGasLimit * PREVIOUS_BLOCK_GAS_LIMIT_FACTOR) + const bufferedGasLimit = Math.round(estimatedGasLimit * gasFuzzFactor) + + if (estimatedGasLimit > upperGasLimit) { + // TODO: Consider whether we should throw an error rather than returning with a high gas limit + return estimatedGasLimit + } else if (bufferedGasLimit < upperGasLimit) { + return bufferedGasLimit + } else { + return upperGasLimit + } +}