From e3fb7896cf3c88094bf5f736b5f23cd53da14cc2 Mon Sep 17 00:00:00 2001 From: 0xGabi Date: Thu, 20 Aug 2020 17:51:09 -0300 Subject: [PATCH 1/5] Refactor: update imports and definitions for Transction entity --- .../src/connections/ConnectorJson.ts | 1 + .../src/connections/IOrganizationConnector.ts | 1 + .../connect-core/src/entities/Organization.ts | 1 + packages/connect-core/src/index.ts | 1 + .../src/transactions/TransactionIntent.ts | 4 ++-- .../src/transactions/TransactionPath.ts | 6 +++--- .../src/transactions/TransactionRequest.ts | 19 ------------------- packages/connect-core/src/types.ts | 15 ++++++--------- packages/connect-core/src/utils/app.ts | 5 +++-- packages/connect-core/src/utils/callScript.ts | 2 +- .../connect-core/src/utils/descriptions.ts | 10 +++++----- packages/connect-core/src/utils/network.ts | 1 + .../src/utils/path/calculatePath.ts | 2 +- .../connect-core/src/utils/path/decodePath.ts | 7 +++---- .../connect-core/src/utils/radspec/index.ts | 5 +++-- 15 files changed, 32 insertions(+), 48 deletions(-) delete mode 100644 packages/connect-core/src/transactions/TransactionRequest.ts diff --git a/packages/connect-core/src/connections/ConnectorJson.ts b/packages/connect-core/src/connections/ConnectorJson.ts index 5043c482..b7c28310 100644 --- a/packages/connect-core/src/connections/ConnectorJson.ts +++ b/packages/connect-core/src/connections/ConnectorJson.ts @@ -4,6 +4,7 @@ /* eslint-disable @typescript-eslint/explicit-function-return-type */ import { AppFilters, Network, SubscriptionHandler } from '@aragon/connect-types' + import { ConnectionContext } from '../types' import IOrganizationConnector from './IOrganizationConnector' import App from '../entities/App' diff --git a/packages/connect-core/src/connections/IOrganizationConnector.ts b/packages/connect-core/src/connections/IOrganizationConnector.ts index ec2deda5..5dfc127f 100644 --- a/packages/connect-core/src/connections/IOrganizationConnector.ts +++ b/packages/connect-core/src/connections/IOrganizationConnector.ts @@ -1,4 +1,5 @@ import { AppFilters, Network, SubscriptionHandler } from '@aragon/connect-types' + import { ConnectionContext } from '../types' import App from '../entities/App' import Organization from '../entities/Organization' diff --git a/packages/connect-core/src/entities/Organization.ts b/packages/connect-core/src/entities/Organization.ts index 3de1ab0e..a47c74e7 100644 --- a/packages/connect-core/src/entities/Organization.ts +++ b/packages/connect-core/src/entities/Organization.ts @@ -4,6 +4,7 @@ import { AppFiltersParam, SubscriptionHandler, } from '@aragon/connect-types' + import { ConnectionContext } from '../types' import TransactionIntent from '../transactions/TransactionIntent' import { toArrayEntry } from '../utils/misc' diff --git a/packages/connect-core/src/index.ts b/packages/connect-core/src/index.ts index e98fce60..2487a5be 100644 --- a/packages/connect-core/src/index.ts +++ b/packages/connect-core/src/index.ts @@ -6,6 +6,7 @@ export type { Networkish, SubscriptionHandler, } from '@aragon/connect-types' + export { default as IOrganizationConnector } from './connections/IOrganizationConnector' export { default as ConnectorJson, diff --git a/packages/connect-core/src/transactions/TransactionIntent.ts b/packages/connect-core/src/transactions/TransactionIntent.ts index 6d3710ad..0c2bfb22 100644 --- a/packages/connect-core/src/transactions/TransactionIntent.ts +++ b/packages/connect-core/src/transactions/TransactionIntent.ts @@ -1,7 +1,7 @@ import { ethers } from 'ethers' import TransactionPath from './TransactionPath' -import TransactionRequest from './TransactionRequest' +import Transaction from '../entities/Transaction' import Organization from '../entities/Organization' import { TransactionIntentData } from '../types' import { calculateTransactionPath } from '../utils/path/calculatePath' @@ -68,7 +68,7 @@ export default class TransactionIntent { async transactions( account: string, options?: { as: string; path?: string[] } - ): Promise { + ): Promise { return (await this.paths(account, options)).transactions } } diff --git a/packages/connect-core/src/transactions/TransactionPath.ts b/packages/connect-core/src/transactions/TransactionPath.ts index 03f8ea60..43499cb1 100644 --- a/packages/connect-core/src/transactions/TransactionPath.ts +++ b/packages/connect-core/src/transactions/TransactionPath.ts @@ -1,12 +1,12 @@ -import TransactionRequest from './TransactionRequest' import App from '../entities/App' +import Transaction from '../entities/Transaction' import { TransactionPathData } from '../types' export default class TransactionPath { readonly apps!: App[] readonly destination!: App - readonly forwardingFeePretransaction?: TransactionRequest - readonly transactions!: TransactionRequest[] + readonly forwardingFeePretransaction?: Transaction + readonly transactions!: Transaction[] constructor(data: TransactionPathData) { this.apps = data.apps diff --git a/packages/connect-core/src/transactions/TransactionRequest.ts b/packages/connect-core/src/transactions/TransactionRequest.ts deleted file mode 100644 index abaaba46..00000000 --- a/packages/connect-core/src/transactions/TransactionRequest.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { Annotation, TransactionRequestData } from '../types' - -export default class TransactionRequest { - readonly children?: TransactionRequest[] - readonly description?: string - readonly descriptionAnnotated?: Annotation[] - readonly data!: string - readonly from?: string - readonly to!: string - - constructor(data: TransactionRequestData) { - this.children = data.children - this.description = data.description - this.descriptionAnnotated = data.descriptionAnnotated - this.data = data.data - this.from = data.from - this.to = data.to - } -} diff --git a/packages/connect-core/src/types.ts b/packages/connect-core/src/types.ts index 2f4fb24b..2e20b0b2 100644 --- a/packages/connect-core/src/types.ts +++ b/packages/connect-core/src/types.ts @@ -3,7 +3,7 @@ import { Address, Network } from '@aragon/connect-types' import IOrganizationConnector from './connections/IOrganizationConnector' import App from './entities/App' -import TransactionRequest from './transactions/TransactionRequest' +import Transaction from './entities/Transaction' export type Abi = (ethers.utils.EventFragment | ethers.utils.FunctionFragment)[] @@ -100,8 +100,8 @@ export interface RoleData { export interface TransactionPathData { apps: App[] destination: App - forwardingFeePretransaction?: TransactionRequest - transactions: TransactionRequest[] + forwardingFeePretransaction?: Transaction + transactions: Transaction[] } export interface TransactionIntentData { @@ -110,13 +110,10 @@ export interface TransactionIntentData { functionArgs: any[] } -export interface TransactionRequestData { - children?: TransactionRequest[] - description?: string - descriptionAnnotated?: Annotation[] +export interface TransactionData { data: string - from?: string - to: string + from: Address + to: Address } ////// METADATA ////// diff --git a/packages/connect-core/src/utils/app.ts b/packages/connect-core/src/utils/app.ts index 1d64594d..dfaadf96 100644 --- a/packages/connect-core/src/utils/app.ts +++ b/packages/connect-core/src/utils/app.ts @@ -1,7 +1,8 @@ import { ethers } from 'ethers' -import { AppIntent, TransactionRequestData } from '../types' +import { AppIntent } from '../types' import App from '../entities/App' +import Transaction from '../entities/Transaction' export const apmAppId = (appName: string): string => ethers.utils.namehash(`${appName}.aragonpm.eth`) @@ -49,7 +50,7 @@ export function validateMethod( */ export function findAppMethodFromIntent( app: App, - transaction: TransactionRequestData + transaction: Transaction ): AppIntent | undefined { const methodId = transaction.data.substring(0, 10) diff --git a/packages/connect-core/src/utils/callScript.ts b/packages/connect-core/src/utils/callScript.ts index d265d716..11120ec3 100644 --- a/packages/connect-core/src/utils/callScript.ts +++ b/packages/connect-core/src/utils/callScript.ts @@ -2,7 +2,7 @@ import { ethers } from 'ethers' export const CALLSCRIPT_ID = '0x00000001' -interface CallScriptAction { +export interface CallScriptAction { to: string data: string } diff --git a/packages/connect-core/src/utils/descriptions.ts b/packages/connect-core/src/utils/descriptions.ts index 0a88c23e..31e374f7 100644 --- a/packages/connect-core/src/utils/descriptions.ts +++ b/packages/connect-core/src/utils/descriptions.ts @@ -1,7 +1,7 @@ import { ethers } from 'ethers' import App from '../entities/App' -import TransactionRequest from '../transactions/TransactionRequest' +import Transaction from '../entities/Transaction' import { decodeTransactionPath, TransactionWithChildren, @@ -15,7 +15,7 @@ export async function describeTransaction( transaction: TransactionWithChildren, apps: App[], provider?: ethers.providers.Provider -): Promise { +): Promise { if (!transaction.to) { throw new Error(`Could not describe transaction: missing 'to'`) } @@ -44,7 +44,7 @@ export async function describeTransaction( throw new Error(`Could not describe transaction: ${err}`) } - return new TransactionRequest({ + return new Transaction({ ...transaction, description, descriptionAnnotated, @@ -59,7 +59,7 @@ export async function describeTransactionPath( path: TransactionWithChildren[], apps: App[], provider?: ethers.providers.Provider -): Promise { +): Promise { return Promise.all( path.map((step) => describeTransaction(step, apps, provider)) ) @@ -69,7 +69,7 @@ export async function describeScript( script: string, apps: App[], provider?: ethers.providers.Provider -): Promise { +): Promise { const path = decodeTransactionPath(script) return describeTransactionPath(path, apps, provider) diff --git a/packages/connect-core/src/utils/network.ts b/packages/connect-core/src/utils/network.ts index 4a7a3a99..4ac68997 100644 --- a/packages/connect-core/src/utils/network.ts +++ b/packages/connect-core/src/utils/network.ts @@ -1,4 +1,5 @@ import { Address, Network, Networkish } from '@aragon/connect-types' + import { NETWORKS } from '../params' export function networkFromChainId(chainId: number): Network | null { diff --git a/packages/connect-core/src/utils/path/calculatePath.ts b/packages/connect-core/src/utils/path/calculatePath.ts index d5ad871e..81d273e7 100644 --- a/packages/connect-core/src/utils/path/calculatePath.ts +++ b/packages/connect-core/src/utils/path/calculatePath.ts @@ -2,12 +2,12 @@ import { ethers } from 'ethers' import { AppIntent } from '../../types' import App from '../../entities/App' +import Transaction from '../../entities/Transaction' import { addressesEqual, includesAddress, ANY_ENTITY } from '../address' import { isFullMethodSignature } from '../app' import { encodeCallScript } from '../callScript' import { canForward } from '../forwarding' import { - Transaction, createDirectTransactionForApp, createForwarderTransactionBuilder, buildForwardingFeePretransaction, diff --git a/packages/connect-core/src/utils/path/decodePath.ts b/packages/connect-core/src/utils/path/decodePath.ts index 61137381..872f3054 100644 --- a/packages/connect-core/src/utils/path/decodePath.ts +++ b/packages/connect-core/src/utils/path/decodePath.ts @@ -1,8 +1,7 @@ -import { isCallScript, decodeCallScript } from '../callScript' +import { isCallScript, decodeCallScript, CallScriptAction } from '../callScript' import { isValidForwardCall, parseForwardCall } from '../forwarding' -import { Transaction } from '../transactions' -export interface TransactionWithChildren extends Transaction { +export interface TransactionWithChildren extends CallScriptAction { children?: TransactionWithChildren[] } @@ -25,7 +24,7 @@ export function decodeTransactionPath( return path.reduce((decodeSegments, segment) => { const { data } = segment - let children + let children: TransactionWithChildren[] = [] if (isValidForwardCall(data)) { const forwardedEvmScript = parseForwardCall(data) diff --git a/packages/connect-core/src/utils/radspec/index.ts b/packages/connect-core/src/utils/radspec/index.ts index 94261f77..ba9093cb 100644 --- a/packages/connect-core/src/utils/radspec/index.ts +++ b/packages/connect-core/src/utils/radspec/index.ts @@ -4,7 +4,8 @@ import * as radspec from 'radspec' import { addressesEqual } from '../address' import { findAppMethodFromIntent } from '../app' import App from '../../entities/App' -import { Abi, AppIntent, TransactionRequestData } from '../../types' +import Transaction from '../../entities/Transaction' +import { Abi, AppIntent } from '../../types' interface FoundMethod { method?: AppIntent @@ -15,7 +16,7 @@ interface FoundMethod { * Attempt to describe intent via radspec. */ export async function tryEvaluatingRadspec( - intent: TransactionRequestData, + intent: Transaction, apps: App[], provider?: ethers.providers.Provider // Decorated intent with description, if one could be made ): Promise { From 453b4ef382850ea440ac0986f04a9eff90c49c65 Mon Sep 17 00:00:00 2001 From: 0xGabi Date: Thu, 20 Aug 2020 17:51:57 -0300 Subject: [PATCH 2/5] Refactor: Create Transaction entity & update how handle transactions --- packages/connect-core/src/entities/App.ts | 11 ++ .../connect-core/src/entities/Transaction.ts | 15 ++ .../connect-core/src/utils/transactions.ts | 132 +++++------------- 3 files changed, 58 insertions(+), 100 deletions(-) create mode 100644 packages/connect-core/src/entities/Transaction.ts diff --git a/packages/connect-core/src/entities/App.ts b/packages/connect-core/src/entities/App.ts index 1440ef53..b1eeb97d 100644 --- a/packages/connect-core/src/entities/App.ts +++ b/packages/connect-core/src/entities/App.ts @@ -1,3 +1,5 @@ +import { ethers } from 'ethers' + import Organization from './Organization' import Repo from './Repo' import Role from './Role' @@ -59,6 +61,15 @@ export default class App { return this.organization.connection.orgConnector } + interface(): ethers.utils.Interface { + if (!this.abi) { + throw new Error( + `No ABI specified in app for ${this.address}. Make sure the metada for the app is available` + ) + } + return new ethers.utils.Interface(this.abi) + } + async repo(): Promise { return this.orgConnector().repoForApp(this.organization, this.address) } diff --git a/packages/connect-core/src/entities/Transaction.ts b/packages/connect-core/src/entities/Transaction.ts new file mode 100644 index 00000000..e67bf7ac --- /dev/null +++ b/packages/connect-core/src/entities/Transaction.ts @@ -0,0 +1,15 @@ +import { Address } from '@aragon/connect-types' + +import { TransactionData } from '../types' + +export default class Transaction { + readonly data: string + readonly from: Address + readonly to: Address + + constructor(data: TransactionData) { + this.data = data.data + this.from = data.from + this.to = data.to + } +} diff --git a/packages/connect-core/src/utils/transactions.ts b/packages/connect-core/src/utils/transactions.ts index 1444c933..710fa052 100644 --- a/packages/connect-core/src/utils/transactions.ts +++ b/packages/connect-core/src/utils/transactions.ts @@ -1,52 +1,13 @@ import { ethers } from 'ethers' import { erc20ABI, forwarderAbi, forwarderFeeAbi } from './abis' -import { isFullMethodSignature } from './app' import App from '../entities/App' +import Transaction from '../entities/Transaction' -export interface Transaction { - data: string - from?: string - to: string -} - -export interface TransactionWithTokenData extends Transaction { - token: { - address: string - value: string - spender: string - } -} - -export async function createDirectTransaction( - sender: string, - destination: string, - methodAbiFragment: ethers.utils.FunctionFragment, - params: any[] -): Promise { - let transactionOptions = {} - - // If an extra parameter has been provided, it is the transaction options if it is an object - if ( - methodAbiFragment.inputs.length + 1 === params.length && - typeof params[params.length - 1] === 'object' - ) { - const options = params.pop() - transactionOptions = { ...transactionOptions, ...options } - } - - const ethersInterface = new ethers.utils.Interface([methodAbiFragment]) - - // The direct transaction we eventually want to perform - return { - ...transactionOptions, // Options are overwriten by the values below - from: sender, - to: destination, - data: ethersInterface.encodeFunctionData( - ethers.utils.FunctionFragment.from(methodAbiFragment), - params - ), - } +interface TokenData { + address: string + value: string + spender: string } export async function createDirectTransactionForApp( @@ -55,42 +16,17 @@ export async function createDirectTransactionForApp( methodSignature: string, params: any[] ): Promise { - if (!app) { - throw new Error(`Could not create transaction due to missing app artifact`) - } + const appInterface = app.interface() + const functionFragment = appInterface.getFunction(methodSignature) - const destination = app.address - - if (!app.abi) { - throw new Error(`No ABI specified in artifact for ${destination}`) - } - - const methodAbiFragment = app.abi.find((method) => { - // If the full signature isn't given, just find the first overload declared - if (!isFullMethodSignature(methodSignature)) { - return method.name === methodSignature - } - - // Fallback functions don't have inputs in the ABI - const currentParameterTypes = Array.isArray(method.inputs) - ? method.inputs.map(({ type }) => type) - : [] - const currentMethodSignature = `${method.name}(${currentParameterTypes.join( - ',' - )})` - return currentMethodSignature === methodSignature + return new Transaction({ + from: sender, + to: app.address, + data: appInterface.encodeFunctionData( + ethers.utils.FunctionFragment.from(functionFragment), + params + ), }) - - if (!methodAbiFragment) { - throw new Error(`${methodSignature} not found on ABI for ${destination}`) - } - - return createDirectTransaction( - sender, - destination, - methodAbiFragment as ethers.utils.FunctionFragment, - params - ) } export function createForwarderTransactionBuilder( @@ -99,24 +35,23 @@ export function createForwarderTransactionBuilder( ): Function { const forwarder = new ethers.utils.Interface(forwarderAbi) - return (forwarderAddress: string, script: string): Transaction => ({ - ...directTransaction, // Options are overwriten by the values below - from: sender, - to: forwarderAddress, - data: forwarder.encodeFunctionData('forward', [script]), - }) + return (forwarderAddress: string, script: string): Transaction => + new Transaction({ + ...directTransaction, // Options are overwriten by the values below + from: sender, + to: forwarderAddress, + data: forwarder.encodeFunctionData('forward', [script]), + }) } export async function buildPretransaction( - transaction: TransactionWithTokenData, + transaction: Transaction, + tokenData: TokenData, provider: ethers.providers.Provider ): Promise { // Token allowance pretransactionn - const { - from, - to, - token: { address: tokenAddress, value: tokenValue, spender }, - } = transaction + const { from, to } = transaction + const { address: tokenAddress, value: tokenValue, spender } = tokenData // Approve the transaction destination unless an spender is passed to approve a different contract const approveSpender = spender || to @@ -144,11 +79,11 @@ export async function buildPretransaction( const erc20 = new ethers.utils.Interface(erc20ABI) - return { + return new Transaction({ from, to: tokenAddress, data: erc20.encodeFunctionData('approve', [approveSpender, tokenValue]), - } + }) } return undefined @@ -181,16 +116,13 @@ export async function buildForwardingFeePretransaction( if (feeDetails.tokenAddress && feeDetails.amount > BigInt(0)) { // Needs a token approval pretransaction - const forwardingTxWithTokenData: TransactionWithTokenData = { - ...forwardingTransaction, - token: { - address: feeDetails.tokenAddress, - spender: forwarderAddress, // since it's a forwarding transaction, always show the real spender - value: feeDetails.amount.toString(), - }, + const tokenData: TokenData = { + address: feeDetails.tokenAddress, + spender: forwarderAddress, // since it's a forwarding transaction, always show the real spender + value: feeDetails.amount.toString(), } - return buildPretransaction(forwardingTxWithTokenData, provider) + return buildPretransaction(forwardingTransaction, tokenData, provider) } return undefined } From 163166aaa5d42f73316714cc2e06fb113363b6df Mon Sep 17 00:00:00 2001 From: 0xGabi Date: Thu, 20 Aug 2020 18:07:33 -0300 Subject: [PATCH 3/5] Comment logic related to forwarding path description --- .../src/transactions/TransactionIntent.ts | 9 +- .../connect-core/src/utils/descriptions.ts | 136 +++++++++--------- .../connect-core/src/utils/path/decodePath.ts | 64 ++++----- 3 files changed, 101 insertions(+), 108 deletions(-) diff --git a/packages/connect-core/src/transactions/TransactionIntent.ts b/packages/connect-core/src/transactions/TransactionIntent.ts index 0c2bfb22..c67ead5b 100644 --- a/packages/connect-core/src/transactions/TransactionIntent.ts +++ b/packages/connect-core/src/transactions/TransactionIntent.ts @@ -5,7 +5,6 @@ import Transaction from '../entities/Transaction' import Organization from '../entities/Organization' import { TransactionIntentData } from '../types' import { calculateTransactionPath } from '../utils/path/calculatePath' -import { describeTransactionPath } from '../utils/descriptions' export default class TransactionIntent { readonly contractAddress!: string @@ -47,12 +46,6 @@ export default class TransactionIntent { this.#provider ) - const describedPath = await describeTransactionPath( - path, - apps, - this.#provider - ) - return new TransactionPath({ apps: apps.filter((app) => path @@ -61,7 +54,7 @@ export default class TransactionIntent { ), destination: apps.find((app) => app.address == this.contractAddress)!, forwardingFeePretransaction, - transactions: describedPath, + transactions: path, }) } diff --git a/packages/connect-core/src/utils/descriptions.ts b/packages/connect-core/src/utils/descriptions.ts index 31e374f7..99afe46a 100644 --- a/packages/connect-core/src/utils/descriptions.ts +++ b/packages/connect-core/src/utils/descriptions.ts @@ -1,76 +1,76 @@ -import { ethers } from 'ethers' +// import { ethers } from 'ethers' -import App from '../entities/App' -import Transaction from '../entities/Transaction' -import { - decodeTransactionPath, - TransactionWithChildren, -} from './path/decodePath' -import { - tryEvaluatingRadspec, - postprocessRadspecDescription, -} from './radspec/index' +// import App from '../entities/App' +// import Transaction from '../entities/Transaction' +// import { +// decodeTransactionPath, +// TransactionWithChildren, +// } from './path/decodePath' +// import { +// tryEvaluatingRadspec, +// postprocessRadspecDescription, +// } from './radspec/index' -export async function describeTransaction( - transaction: TransactionWithChildren, - apps: App[], - provider?: ethers.providers.Provider -): Promise { - if (!transaction.to) { - throw new Error(`Could not describe transaction: missing 'to'`) - } - if (!transaction.data) { - throw new Error(`Could not describe transaction: missing 'data'`) - } - let description, descriptionAnnotated - try { - description = await tryEvaluatingRadspec(transaction, apps, provider) +// export async function describeTransaction( +// transaction: Transaction, +// apps: App[], +// provider?: ethers.providers.Provider +// ): Promise { +// if (!transaction.to) { +// throw new Error(`Could not describe transaction: missing 'to'`) +// } +// if (!transaction.data) { +// throw new Error(`Could not describe transaction: missing 'data'`) +// } +// let description, descriptionAnnotated +// try { +// description = await tryEvaluatingRadspec(transaction, apps, provider) - if (description) { - const processed = await postprocessRadspecDescription(description, apps) - descriptionAnnotated = processed.annotatedDescription - description = processed.description - } +// if (description) { +// const processed = await postprocessRadspecDescription(description, apps) +// descriptionAnnotated = processed.annotatedDescription +// description = processed.description +// } - if (transaction.children) { - // eslint-disable-next-line @typescript-eslint/no-use-before-define - transaction.children = await describeTransactionPath( - transaction.children, - apps, - provider - ) - } - } catch (err) { - throw new Error(`Could not describe transaction: ${err}`) - } +// // if (transaction.children) { +// // // eslint-disable-next-line @typescript-eslint/no-use-before-define +// // transaction.children = await describeTransactionPath( +// // transaction.children, +// // apps, +// // provider +// // ) +// // } +// } catch (err) { +// throw new Error(`Could not describe transaction: ${err}`) +// } - return new Transaction({ - ...transaction, - description, - descriptionAnnotated, - }) -} +// return new Transaction({ +// ...transaction, +// description, +// descriptionAnnotated, +// }) +// } -/** - * Use radspec to create a human-readable description for each transaction in the given `path` - * - */ -export async function describeTransactionPath( - path: TransactionWithChildren[], - apps: App[], - provider?: ethers.providers.Provider -): Promise { - return Promise.all( - path.map((step) => describeTransaction(step, apps, provider)) - ) -} +// /** +// * Use radspec to create a human-readable description for each transaction in the given `path` +// * +// */ +// export async function describeTransactionPath( +// path: Transaction[], +// apps: App[], +// provider?: ethers.providers.Provider +// ): Promise { +// return Promise.all( +// path.map((step) => describeTransaction(step, apps, provider)) +// ) +// } -export async function describeScript( - script: string, - apps: App[], - provider?: ethers.providers.Provider -): Promise { - const path = decodeTransactionPath(script) +// export async function describeScript( +// script: string, +// apps: App[], +// provider?: ethers.providers.Provider +// ): Promise { +// const path = decodeTransactionPath(script) - return describeTransactionPath(path, apps, provider) -} +// return describeTransactionPath(path, apps, provider) +// } diff --git a/packages/connect-core/src/utils/path/decodePath.ts b/packages/connect-core/src/utils/path/decodePath.ts index 872f3054..f8aa38a2 100644 --- a/packages/connect-core/src/utils/path/decodePath.ts +++ b/packages/connect-core/src/utils/path/decodePath.ts @@ -1,39 +1,39 @@ -import { isCallScript, decodeCallScript, CallScriptAction } from '../callScript' -import { isValidForwardCall, parseForwardCall } from '../forwarding' +// import { isCallScript, decodeCallScript, CallScriptAction } from '../callScript' +// import { isValidForwardCall, parseForwardCall } from '../forwarding' -export interface TransactionWithChildren extends CallScriptAction { - children?: TransactionWithChildren[] -} +// export interface TransactionWithChildren extends CallScriptAction { +// children?: TransactionWithChildren[] +// } -/** - * Decodes an EVM callscript and returns the transaction path it describes. - * - * @return An array of Ethereum transactions that describe each step in the path - */ -export function decodeTransactionPath( - script: string -): TransactionWithChildren[] { - // In the future we may support more EVMScripts, but for now let's just assume we're only - // dealing with call scripts - if (!isCallScript(script)) { - throw new Error(`Script could not be decoded: ${script}`) - } +// /** +// * Decodes an EVM callscript and returns the transaction path it describes. +// * +// * @return An array of Ethereum transactions that describe each step in the path +// */ +// export function decodeTransactionPath( +// script: string +// ): TransactionWithChildren[] { +// // In the future we may support more EVMScripts, but for now let's just assume we're only +// // dealing with call scripts +// if (!isCallScript(script)) { +// throw new Error(`Script could not be decoded: ${script}`) +// } - const path = decodeCallScript(script) +// const path = decodeCallScript(script) - return path.reduce((decodeSegments, segment) => { - const { data } = segment +// return path.reduce((decodeSegments, segment) => { +// const { data } = segment - let children: TransactionWithChildren[] = [] - if (isValidForwardCall(data)) { - const forwardedEvmScript = parseForwardCall(data) +// let children: TransactionWithChildren[] = [] +// if (isValidForwardCall(data)) { +// const forwardedEvmScript = parseForwardCall(data) - try { - children = decodeTransactionPath(forwardedEvmScript) - // eslint-disable-next-line no-empty - } catch (err) {} - } +// try { +// children = decodeTransactionPath(forwardedEvmScript) +// // eslint-disable-next-line no-empty +// } catch (err) {} +// } - return decodeSegments.concat({ ...segment, children }) - }, [] as TransactionWithChildren[]) -} +// return decodeSegments.concat({ ...segment, children }) +// }, [] as TransactionWithChildren[]) +// } From 4c0890b761e041eb40563fa2d142fd47a8719911 Mon Sep 17 00:00:00 2001 From: 0xGabi Date: Thu, 20 Aug 2020 18:11:48 -0300 Subject: [PATCH 4/5] Comment logic related to forwarding path description --- .../src/transactions/TransactionIntent.ts | 9 +- .../connect-core/src/utils/descriptions.ts | 136 +++++++++--------- packages/connect-core/src/utils/index.ts | 2 +- .../connect-core/src/utils/path/decodePath.ts | 64 ++++----- 4 files changed, 102 insertions(+), 109 deletions(-) diff --git a/packages/connect-core/src/transactions/TransactionIntent.ts b/packages/connect-core/src/transactions/TransactionIntent.ts index 0c2bfb22..c67ead5b 100644 --- a/packages/connect-core/src/transactions/TransactionIntent.ts +++ b/packages/connect-core/src/transactions/TransactionIntent.ts @@ -5,7 +5,6 @@ import Transaction from '../entities/Transaction' import Organization from '../entities/Organization' import { TransactionIntentData } from '../types' import { calculateTransactionPath } from '../utils/path/calculatePath' -import { describeTransactionPath } from '../utils/descriptions' export default class TransactionIntent { readonly contractAddress!: string @@ -47,12 +46,6 @@ export default class TransactionIntent { this.#provider ) - const describedPath = await describeTransactionPath( - path, - apps, - this.#provider - ) - return new TransactionPath({ apps: apps.filter((app) => path @@ -61,7 +54,7 @@ export default class TransactionIntent { ), destination: apps.find((app) => app.address == this.contractAddress)!, forwardingFeePretransaction, - transactions: describedPath, + transactions: path, }) } diff --git a/packages/connect-core/src/utils/descriptions.ts b/packages/connect-core/src/utils/descriptions.ts index 31e374f7..99afe46a 100644 --- a/packages/connect-core/src/utils/descriptions.ts +++ b/packages/connect-core/src/utils/descriptions.ts @@ -1,76 +1,76 @@ -import { ethers } from 'ethers' +// import { ethers } from 'ethers' -import App from '../entities/App' -import Transaction from '../entities/Transaction' -import { - decodeTransactionPath, - TransactionWithChildren, -} from './path/decodePath' -import { - tryEvaluatingRadspec, - postprocessRadspecDescription, -} from './radspec/index' +// import App from '../entities/App' +// import Transaction from '../entities/Transaction' +// import { +// decodeTransactionPath, +// TransactionWithChildren, +// } from './path/decodePath' +// import { +// tryEvaluatingRadspec, +// postprocessRadspecDescription, +// } from './radspec/index' -export async function describeTransaction( - transaction: TransactionWithChildren, - apps: App[], - provider?: ethers.providers.Provider -): Promise { - if (!transaction.to) { - throw new Error(`Could not describe transaction: missing 'to'`) - } - if (!transaction.data) { - throw new Error(`Could not describe transaction: missing 'data'`) - } - let description, descriptionAnnotated - try { - description = await tryEvaluatingRadspec(transaction, apps, provider) +// export async function describeTransaction( +// transaction: Transaction, +// apps: App[], +// provider?: ethers.providers.Provider +// ): Promise { +// if (!transaction.to) { +// throw new Error(`Could not describe transaction: missing 'to'`) +// } +// if (!transaction.data) { +// throw new Error(`Could not describe transaction: missing 'data'`) +// } +// let description, descriptionAnnotated +// try { +// description = await tryEvaluatingRadspec(transaction, apps, provider) - if (description) { - const processed = await postprocessRadspecDescription(description, apps) - descriptionAnnotated = processed.annotatedDescription - description = processed.description - } +// if (description) { +// const processed = await postprocessRadspecDescription(description, apps) +// descriptionAnnotated = processed.annotatedDescription +// description = processed.description +// } - if (transaction.children) { - // eslint-disable-next-line @typescript-eslint/no-use-before-define - transaction.children = await describeTransactionPath( - transaction.children, - apps, - provider - ) - } - } catch (err) { - throw new Error(`Could not describe transaction: ${err}`) - } +// // if (transaction.children) { +// // // eslint-disable-next-line @typescript-eslint/no-use-before-define +// // transaction.children = await describeTransactionPath( +// // transaction.children, +// // apps, +// // provider +// // ) +// // } +// } catch (err) { +// throw new Error(`Could not describe transaction: ${err}`) +// } - return new Transaction({ - ...transaction, - description, - descriptionAnnotated, - }) -} +// return new Transaction({ +// ...transaction, +// description, +// descriptionAnnotated, +// }) +// } -/** - * Use radspec to create a human-readable description for each transaction in the given `path` - * - */ -export async function describeTransactionPath( - path: TransactionWithChildren[], - apps: App[], - provider?: ethers.providers.Provider -): Promise { - return Promise.all( - path.map((step) => describeTransaction(step, apps, provider)) - ) -} +// /** +// * Use radspec to create a human-readable description for each transaction in the given `path` +// * +// */ +// export async function describeTransactionPath( +// path: Transaction[], +// apps: App[], +// provider?: ethers.providers.Provider +// ): Promise { +// return Promise.all( +// path.map((step) => describeTransaction(step, apps, provider)) +// ) +// } -export async function describeScript( - script: string, - apps: App[], - provider?: ethers.providers.Provider -): Promise { - const path = decodeTransactionPath(script) +// export async function describeScript( +// script: string, +// apps: App[], +// provider?: ethers.providers.Provider +// ): Promise { +// const path = decodeTransactionPath(script) - return describeTransactionPath(path, apps, provider) -} +// return describeTransactionPath(path, apps, provider) +// } diff --git a/packages/connect-core/src/utils/index.ts b/packages/connect-core/src/utils/index.ts index 07249352..e53e1e33 100644 --- a/packages/connect-core/src/utils/index.ts +++ b/packages/connect-core/src/utils/index.ts @@ -1,4 +1,4 @@ -export * from './descriptions' +// export * from './descriptions' export * from './misc' export * from './network' export * from './app-connectors' diff --git a/packages/connect-core/src/utils/path/decodePath.ts b/packages/connect-core/src/utils/path/decodePath.ts index 872f3054..f8aa38a2 100644 --- a/packages/connect-core/src/utils/path/decodePath.ts +++ b/packages/connect-core/src/utils/path/decodePath.ts @@ -1,39 +1,39 @@ -import { isCallScript, decodeCallScript, CallScriptAction } from '../callScript' -import { isValidForwardCall, parseForwardCall } from '../forwarding' +// import { isCallScript, decodeCallScript, CallScriptAction } from '../callScript' +// import { isValidForwardCall, parseForwardCall } from '../forwarding' -export interface TransactionWithChildren extends CallScriptAction { - children?: TransactionWithChildren[] -} +// export interface TransactionWithChildren extends CallScriptAction { +// children?: TransactionWithChildren[] +// } -/** - * Decodes an EVM callscript and returns the transaction path it describes. - * - * @return An array of Ethereum transactions that describe each step in the path - */ -export function decodeTransactionPath( - script: string -): TransactionWithChildren[] { - // In the future we may support more EVMScripts, but for now let's just assume we're only - // dealing with call scripts - if (!isCallScript(script)) { - throw new Error(`Script could not be decoded: ${script}`) - } +// /** +// * Decodes an EVM callscript and returns the transaction path it describes. +// * +// * @return An array of Ethereum transactions that describe each step in the path +// */ +// export function decodeTransactionPath( +// script: string +// ): TransactionWithChildren[] { +// // In the future we may support more EVMScripts, but for now let's just assume we're only +// // dealing with call scripts +// if (!isCallScript(script)) { +// throw new Error(`Script could not be decoded: ${script}`) +// } - const path = decodeCallScript(script) +// const path = decodeCallScript(script) - return path.reduce((decodeSegments, segment) => { - const { data } = segment +// return path.reduce((decodeSegments, segment) => { +// const { data } = segment - let children: TransactionWithChildren[] = [] - if (isValidForwardCall(data)) { - const forwardedEvmScript = parseForwardCall(data) +// let children: TransactionWithChildren[] = [] +// if (isValidForwardCall(data)) { +// const forwardedEvmScript = parseForwardCall(data) - try { - children = decodeTransactionPath(forwardedEvmScript) - // eslint-disable-next-line no-empty - } catch (err) {} - } +// try { +// children = decodeTransactionPath(forwardedEvmScript) +// // eslint-disable-next-line no-empty +// } catch (err) {} +// } - return decodeSegments.concat({ ...segment, children }) - }, [] as TransactionWithChildren[]) -} +// return decodeSegments.concat({ ...segment, children }) +// }, [] as TransactionWithChildren[]) +// } From 068a952206688c54e1a6172911d1c215e58a5c24 Mon Sep 17 00:00:00 2001 From: Gabriel Garcia Date: Mon, 31 Aug 2020 18:06:37 -0300 Subject: [PATCH 5/5] Refactor: Add Intent entity + refactor calculate path logic (#225) --- packages/connect-core/src/entities/App.ts | 19 ++- .../src/entities/ForwardingPath.ts | 77 ++++++++++ packages/connect-core/src/entities/Intent.ts | 81 ++++++++++ .../connect-core/src/entities/Organization.ts | 16 +- .../src/transactions/TransactionIntent.ts | 67 -------- packages/connect-core/src/types.ts | 29 ++-- packages/connect-core/src/utils/app.ts | 34 +++-- .../connect-core/src/utils/description.ts | 94 ++++++++++++ .../connect-core/src/utils/descriptions.ts | 76 ---------- packages/connect-core/src/utils/index.ts | 2 +- .../src/utils/path/calculatePath.ts | 143 +++++++----------- .../connect-core/src/utils/path/decodePath.ts | 39 ----- .../connect-core/src/utils/radspec/index.ts | 9 +- .../src/utils/radspec/postprocess.ts | 7 +- .../src/__test__/apps.test.ts | 6 +- 15 files changed, 380 insertions(+), 319 deletions(-) create mode 100644 packages/connect-core/src/entities/ForwardingPath.ts create mode 100644 packages/connect-core/src/entities/Intent.ts delete mode 100644 packages/connect-core/src/transactions/TransactionIntent.ts create mode 100644 packages/connect-core/src/utils/description.ts delete mode 100644 packages/connect-core/src/utils/descriptions.ts delete mode 100644 packages/connect-core/src/utils/path/decodePath.ts diff --git a/packages/connect-core/src/entities/App.ts b/packages/connect-core/src/entities/App.ts index b1eeb97d..b297e5f7 100644 --- a/packages/connect-core/src/entities/App.ts +++ b/packages/connect-core/src/entities/App.ts @@ -5,7 +5,7 @@ import Repo from './Repo' import Role from './Role' import { Abi, - AppIntent, + AppMethod, AragonArtifact, AragonManifest, Metadata, @@ -61,6 +61,19 @@ export default class App { return this.organization.connection.orgConnector } + contract(): ethers.Contract { + if (!this.abi) { + throw new Error( + `No ABI specified in app for ${this.address}. Make sure the metada for the app is available` + ) + } + return new ethers.Contract( + this.address, + this.abi, + this.organization.connection.ethersProvider + ) + } + interface(): ethers.utils.Interface { if (!this.abi) { throw new Error( @@ -90,11 +103,11 @@ export default class App { return this.artifact.abi } - get intents(): AppIntent[] { + get methods(): AppMethod[] { return this.artifact.functions } - get deprecatedIntents(): { [version: string]: AppIntent[] } { + get deprecatedMethods(): { [version: string]: AppMethod[] } { return this.artifact.deprecatedFunctions } diff --git a/packages/connect-core/src/entities/ForwardingPath.ts b/packages/connect-core/src/entities/ForwardingPath.ts new file mode 100644 index 00000000..79afe6d1 --- /dev/null +++ b/packages/connect-core/src/entities/ForwardingPath.ts @@ -0,0 +1,77 @@ +import { ethers } from 'ethers' + +import App from './App' +import Transaction from './Transaction' +import { + AppOrAddress, + ForwardingPathData, + ForwardingPathDescriptionData, + PostProcessDescription, +} from '../types' +import { describeForwardingPath } from '../utils/description' + +export default class ForwardingPath { + #provider: ethers.providers.Provider + readonly apps: App[] + readonly destination: App + readonly transactions: Transaction[] + + constructor(data: ForwardingPathData, provider: ethers.providers.Provider) { + this.#provider = provider + this.apps = data.apps + this.destination = data.destination + this.transactions = data.transactions + } + + // Lets consumers pass a callback to sign any number of transactions. + // This is similar to calling transactions() and using a loop, but shorter. + // It returns the value returned by the library, usually a transaction receipt. + async sign( + callback: (tx: Transaction) => Promise + ): Promise { + return Promise.all(this.transactions.map(async (tx) => await callback(tx))) + } + + // Return a description of the forwarding path, to be rendered. + async describe(): Promise { + // TODO: Make sure we are safe to only provide the apps on the path here + return describeForwardingPath(this.transactions, this.apps, this.#provider) + } + + // Return a description of the forwarding path, as text. + // Shorthand for .describe().toString() + toString(): string { + return this.describe().toString() + } +} + +type ForwardingPathDescriptionTreeEntry = + | AppOrAddress + | [AppOrAddress, ForwardingPathDescriptionTreeEntry[]] + +export type ForwardingPathDescriptionTree = ForwardingPathDescriptionTreeEntry[] + +export class ForwardingPathDescription { + readonly apps: App[] + readonly describeSteps: PostProcessDescription[] + + constructor(data: ForwardingPathDescriptionData) { + this.apps = data.apps + this.describeSteps = data.describeSteps + } + + // Return a tree that can get used to render the path. + tree(): ForwardingPathDescriptionTree { + // TODO: + return [] + } + + // Renders the forwarding path description as text + toString(): string { + return this.tree().toString() + } + + // TBD: a utility that makes it easy to render the tree, + // e.g. as a nested list in HTML or React. + reduce(callback: Function): any {} +} diff --git a/packages/connect-core/src/entities/Intent.ts b/packages/connect-core/src/entities/Intent.ts new file mode 100644 index 00000000..63e532c6 --- /dev/null +++ b/packages/connect-core/src/entities/Intent.ts @@ -0,0 +1,81 @@ +import { ethers } from 'ethers' + +import Organization from './Organization' +import Transaction from './Transaction' +import TransactionPath from '../transactions/TransactionPath' +import { PathOptions, IntentData } from '../types' +import { calculateTransactionPath } from '../utils/path/calculatePath' + +export default class Intent { + #org: Organization + #provider: ethers.providers.Provider + readonly appAddress: string + readonly functionName: string + readonly functionArgs: any[] + + constructor( + data: IntentData, + org: Organization, + provider: ethers.providers.Provider + ) { + this.#org = org + this.#provider = provider + this.appAddress = data.appAddress + this.functionName = data.functionName + this.functionArgs = data.functionArgs + } + + // Retrieve a single forwarding path. Defaults to the shortest one. + async path({ actAs, path }: PathOptions): Promise { + const apps = await this.#org.apps() + + // Get the destination app + const destination = apps.find((app) => app.address == this.appAddress) + if (!destination) { + throw new Error( + `Destination (${this.appAddress}) is not an installed app` + ) + } + + const transactions = await calculateTransactionPath( + actAs, + destination, + this.functionName, + this.functionArgs, + apps, + this.#provider + ) + + return new TransactionPath({ + apps: apps.filter((app) => + transactions + .map((tx) => tx.to) + .some((address) => address === app.address) + ), + destination, + transactions, + }) + } + + // Retrieve the different possible forwarding paths. + async paths({ actAs, path }: PathOptions): Promise { + // TODO: support logic to calculate multiple Forwarding paths + return [await this.path({ actAs, path })] + } + + // A list of transaction requests ready to get signed. + async transactions(options: PathOptions): Promise { + return (await this.path(options)).transactions + } + + // Lets consumers pass a callback to sign any number of transactions. + // This is similar to calling transactions() and using a loop, but shorter. + // It returns the value returned by the library, usually a transaction receipt. + async sign( + callback: (tx: Transaction) => Promise, + options: PathOptions + ): Promise { + const txs = await this.transactions(options) + return Promise.all(txs.map(async (tx) => await callback(tx))) + } +} diff --git a/packages/connect-core/src/entities/Organization.ts b/packages/connect-core/src/entities/Organization.ts index a47c74e7..d9feb461 100644 --- a/packages/connect-core/src/entities/Organization.ts +++ b/packages/connect-core/src/entities/Organization.ts @@ -6,9 +6,11 @@ import { } from '@aragon/connect-types' import { ConnectionContext } from '../types' -import TransactionIntent from '../transactions/TransactionIntent' +import { decodeForwardingPath } from '../utils/description' import { toArrayEntry } from '../utils/misc' import App from './App' +import Intent from './Intent' +import { ForwardingPathDescription } from './ForwardingPath' import Permission from './Permission' // TODO @@ -127,11 +129,17 @@ export default class Organization { appAddress: Address, functionName: string, functionArgs: any[] - ): TransactionIntent { - return new TransactionIntent( - { contractAddress: appAddress, functionName, functionArgs }, + ): Intent { + return new Intent( + { appAddress, functionName, functionArgs }, this, this.connection.ethersProvider ) } + + //////// DESCRIPTIONS ///////// + + async describeScript(script: string): Promise { + return decodeForwardingPath(script, this.apps(), this.connection) + } } diff --git a/packages/connect-core/src/transactions/TransactionIntent.ts b/packages/connect-core/src/transactions/TransactionIntent.ts deleted file mode 100644 index c67ead5b..00000000 --- a/packages/connect-core/src/transactions/TransactionIntent.ts +++ /dev/null @@ -1,67 +0,0 @@ -import { ethers } from 'ethers' - -import TransactionPath from './TransactionPath' -import Transaction from '../entities/Transaction' -import Organization from '../entities/Organization' -import { TransactionIntentData } from '../types' -import { calculateTransactionPath } from '../utils/path/calculatePath' - -export default class TransactionIntent { - readonly contractAddress!: string - readonly functionName!: string - readonly functionArgs!: any[] - - #org: Organization - #provider: ethers.providers.Provider - - constructor( - data: TransactionIntentData, - org: Organization, - provider: ethers.providers.Provider - ) { - this.#org = org - this.#provider = provider - - this.contractAddress = data.contractAddress - this.functionArgs = data.functionArgs - this.functionName = data.functionName - } - - async paths( - account: string, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - options?: { as?: string; path?: string[] } - ): Promise { - const apps = await this.#org.apps() - - const { - forwardingFeePretransaction, - path, - } = await calculateTransactionPath( - account, - this.contractAddress, - this.functionName, - this.functionArgs, - apps, - this.#provider - ) - - return new TransactionPath({ - apps: apps.filter((app) => - path - .map((transaction) => transaction.to) - .some((address) => address === app.address) - ), - destination: apps.find((app) => app.address == this.contractAddress)!, - forwardingFeePretransaction, - transactions: path, - }) - } - - async transactions( - account: string, - options?: { as: string; path?: string[] } - ): Promise { - return (await this.paths(account, options)).transactions - } -} diff --git a/packages/connect-core/src/types.ts b/packages/connect-core/src/types.ts index 2e20b0b2..4f620476 100644 --- a/packages/connect-core/src/types.ts +++ b/packages/connect-core/src/types.ts @@ -63,6 +63,23 @@ export interface AppData { version?: string } +export interface ForwardingPathData { + apps: App[] + destination: App + transactions: Transaction[] +} + +export interface ForwardingPathDescriptionData { + apps: App[] + describeSteps: PostProcessDescription[] +} + +export interface IntentData { + appAddress: string + functionName: string + functionArgs: any[] +} + export interface ParamData { argumentId: number operationType: number @@ -104,12 +121,6 @@ export interface TransactionPathData { transactions: Transaction[] } -export interface TransactionIntentData { - contractAddress: string - functionName: string - functionArgs: any[] -} - export interface TransactionData { data: string from: Address @@ -120,7 +131,7 @@ export interface TransactionData { export type Metadata = (AragonArtifact | AragonManifest)[] -export interface AppIntent { +export interface AppMethod { roles: string[] sig: string /** @@ -171,12 +182,12 @@ export interface AragonArtifact extends AragonAppJson { * Includes metadata needed for radspec and transaction pathing * initialize() function should also be included for completeness */ - functions: AppIntent[] + functions: AppMethod[] /** * Functions that are no longer available at `version` */ deprecatedFunctions: { - [version: string]: AppIntent[] + [version: string]: AppMethod[] } /** * The flaten source code of the contracts must be included in diff --git a/packages/connect-core/src/utils/app.ts b/packages/connect-core/src/utils/app.ts index dfaadf96..4643e1d5 100644 --- a/packages/connect-core/src/utils/app.ts +++ b/packages/connect-core/src/utils/app.ts @@ -1,6 +1,6 @@ import { ethers } from 'ethers' -import { AppIntent } from '../types' +import { AppMethod } from '../types' import App from '../entities/App' import Transaction from '../entities/Transaction' @@ -16,14 +16,15 @@ export const isFullMethodSignature = (methodSignature: string): boolean => { ) } -export function validateMethod( - destination: string, - methodSignature: string, - destinationApp: App -): AppIntent { - const methods = destinationApp.intents +export function getAppMethod( + destinationApp: App, + methodSignature: string +): AppMethod { + const methods = destinationApp.methods if (!methods) { - throw new Error(`No functions specified in artifact for ${destination}`) + throw new Error( + `No methods specified in the app for ${destinationApp.address}. Make sure the metada for the app is available` + ) } // Find the relevant method information @@ -33,8 +34,11 @@ export function validateMethod( : // If the full signature isn't given, just select the first overload declared method.sig.split('(')[0] === methodSignature ) + if (!method) { - throw new Error(`No method named ${methodSignature} on ${destination}`) + throw new Error( + `No method named ${methodSignature} on ${destinationApp.address}` + ) } return method @@ -51,7 +55,7 @@ export function validateMethod( export function findAppMethodFromIntent( app: App, transaction: Transaction -): AppIntent | undefined { +): AppMethod | undefined { const methodId = transaction.data.substring(0, 10) const checkMethodSignature = (siganture: string): boolean => { @@ -60,22 +64,22 @@ export function findAppMethodFromIntent( return sigHash === methodId } - const { deprecatedIntents, intents } = app || {} + const { deprecatedMethods, methods } = app || {} let method // First try to find the method in the current functions - if (Array.isArray(intents)) { - method = intents.find((method) => checkMethodSignature(method.sig)) + if (Array.isArray(methods)) { + method = methods.find((method) => checkMethodSignature(method.sig)) } if (!method) { // The current functions didn't have it; try with each deprecated version's functions const deprecatedFunctionsFromVersions = Object.values( - deprecatedIntents || {} + deprecatedMethods || {} ) if (deprecatedFunctionsFromVersions.every(Array.isArray)) { // Flatten all the deprecated functions - const allDeprecatedFunctions = ([] as AppIntent[]).concat( + const allDeprecatedFunctions = ([] as AppMethod[]).concat( ...deprecatedFunctionsFromVersions ) method = allDeprecatedFunctions.find((method) => diff --git a/packages/connect-core/src/utils/description.ts b/packages/connect-core/src/utils/description.ts new file mode 100644 index 00000000..42770e2e --- /dev/null +++ b/packages/connect-core/src/utils/description.ts @@ -0,0 +1,94 @@ +import { ethers } from 'ethers' + +import { isCallScript, decodeCallScript } from './callScript' +import { isValidForwardCall, parseForwardCall } from './forwarding' +import App from '../entities/App' +import Transaction from '../entities/Transaction' +import { + ForwardingPathDescription, + ForwardingPathDescriptionTree, +} from '../entities/ForwardingPath' +import { + tryEvaluatingRadspec, + postprocessRadspecDescription, +} from './radspec/index' +import { PostProcessDescription } from '../types' + +export async function describeStep( + transaction: Transaction, + apps: App[], + provider: ethers.providers.Provider +): Promise { + if (!transaction.to) { + throw new Error(`Could not describe transaction: missing 'to'`) + } + if (!transaction.data) { + throw new Error(`Could not describe transaction: missing 'data'`) + } + + let description, annotatedDescription + try { + description = await tryEvaluatingRadspec(transaction, apps, provider) + + if (description) { + const processed = await postprocessRadspecDescription(description, apps) + annotatedDescription = processed.annotatedDescription + description = processed.description + } + } catch (err) { + throw new Error(`Could not describe transaction: ${err}`) + } + + return { + description, + annotatedDescription, + } +} + +/** + * Use radspec to create a human-readable description for each step in the given `path` + * + */ +export async function describeForwardingPath( + path: Transaction[], + apps: App[], + provider: ethers.providers.Provider +): Promise { + const describeSteps = await Promise.all( + path.map((step) => describeStep(step, apps, provider)) + ) + return new ForwardingPathDescription({ describeSteps, apps }) +} + +/** + * Decodes an EVM callscript and returns the forwarding path it description. + * + * @return An array of Ethereum transactions that describe each step in the path + */ +export function decodeForwardingPath( + script: string +): ForwardingPathDescriptionTree { + // In the future we may support more EVMScripts, but for now let's just assume we're only + // dealing with call scripts + if (!isCallScript(script)) { + throw new Error(`Script could not be decoded: ${script}`) + } + + const path = decodeCallScript(script) + + return path.reduce((decodeSegments, segment) => { + const { data } = segment + + let children + if (isValidForwardCall(data)) { + const forwardedEvmScript = parseForwardCall(data) + + try { + children = decodeForwardingPath(forwardedEvmScript) + // eslint-disable-next-line no-empty + } catch (err) {} + } + + return decodeSegments.concat({ ...segment, children }) + }, [] as ForwardingPathDescriptionTree) +} diff --git a/packages/connect-core/src/utils/descriptions.ts b/packages/connect-core/src/utils/descriptions.ts deleted file mode 100644 index 99afe46a..00000000 --- a/packages/connect-core/src/utils/descriptions.ts +++ /dev/null @@ -1,76 +0,0 @@ -// import { ethers } from 'ethers' - -// import App from '../entities/App' -// import Transaction from '../entities/Transaction' -// import { -// decodeTransactionPath, -// TransactionWithChildren, -// } from './path/decodePath' -// import { -// tryEvaluatingRadspec, -// postprocessRadspecDescription, -// } from './radspec/index' - -// export async function describeTransaction( -// transaction: Transaction, -// apps: App[], -// provider?: ethers.providers.Provider -// ): Promise { -// if (!transaction.to) { -// throw new Error(`Could not describe transaction: missing 'to'`) -// } -// if (!transaction.data) { -// throw new Error(`Could not describe transaction: missing 'data'`) -// } -// let description, descriptionAnnotated -// try { -// description = await tryEvaluatingRadspec(transaction, apps, provider) - -// if (description) { -// const processed = await postprocessRadspecDescription(description, apps) -// descriptionAnnotated = processed.annotatedDescription -// description = processed.description -// } - -// // if (transaction.children) { -// // // eslint-disable-next-line @typescript-eslint/no-use-before-define -// // transaction.children = await describeTransactionPath( -// // transaction.children, -// // apps, -// // provider -// // ) -// // } -// } catch (err) { -// throw new Error(`Could not describe transaction: ${err}`) -// } - -// return new Transaction({ -// ...transaction, -// description, -// descriptionAnnotated, -// }) -// } - -// /** -// * Use radspec to create a human-readable description for each transaction in the given `path` -// * -// */ -// export async function describeTransactionPath( -// path: Transaction[], -// apps: App[], -// provider?: ethers.providers.Provider -// ): Promise { -// return Promise.all( -// path.map((step) => describeTransaction(step, apps, provider)) -// ) -// } - -// export async function describeScript( -// script: string, -// apps: App[], -// provider?: ethers.providers.Provider -// ): Promise { -// const path = decodeTransactionPath(script) - -// return describeTransactionPath(path, apps, provider) -// } diff --git a/packages/connect-core/src/utils/index.ts b/packages/connect-core/src/utils/index.ts index e53e1e33..8c540157 100644 --- a/packages/connect-core/src/utils/index.ts +++ b/packages/connect-core/src/utils/index.ts @@ -1,4 +1,4 @@ -// export * from './descriptions' +export * from './description' export * from './misc' export * from './network' export * from './app-connectors' diff --git a/packages/connect-core/src/utils/path/calculatePath.ts b/packages/connect-core/src/utils/path/calculatePath.ts index 81d273e7..d349daae 100644 --- a/packages/connect-core/src/utils/path/calculatePath.ts +++ b/packages/connect-core/src/utils/path/calculatePath.ts @@ -1,10 +1,9 @@ import { ethers } from 'ethers' -import { AppIntent } from '../../types' import App from '../../entities/App' import Transaction from '../../entities/Transaction' import { addressesEqual, includesAddress, ANY_ENTITY } from '../address' -import { isFullMethodSignature } from '../app' +import { getAppMethod } from '../app' import { encodeCallScript } from '../callScript' import { canForward } from '../forwarding' import { @@ -13,35 +12,6 @@ import { buildForwardingFeePretransaction, } from '../transactions' -interface PathData { - forwardingFeePretransaction?: Transaction - path: Transaction[] -} - -function validateMethod( - destination: string, - methodSignature: string, - destinationApp: App -): AppIntent { - const methods = destinationApp.intents - if (!methods) { - throw new Error(`No functions specified in artifact for ${destination}`) - } - - // Find the relevant method information - const method = methods.find((method) => - isFullMethodSignature(methodSignature) - ? method.sig === methodSignature - : // If the full signature isn't given, just select the first overload declared - method.sig.split('(')[0] === methodSignature - ) - if (!method) { - throw new Error(`No method named ${methodSignature} on ${destination}`) - } - - return method -} - /** * Calculate the forwarding path for a transaction to `destination` * that invokes `directTransaction`. @@ -53,10 +23,10 @@ async function calculateForwardingPath( forwardersWithPermission: string[], forwarders: string[], provider: ethers.providers.Provider -): Promise { +): Promise { // No forwarders can perform the requested action if (forwardersWithPermission.length === 0) { - return { path: [] } + return [] } const createForwarderTransaction = createForwarderTransactionBuilder( @@ -64,26 +34,42 @@ async function calculateForwardingPath( directTransaction ) + const buildForwardingPath = async ( + forwarder: string, + script: string, + path: Transaction[], + provider: ethers.providers.Provider + ): Promise => { + const transaction = createForwarderTransaction(forwarder, script) + + // Only apply pretransactions to the first transaction in the path + // as it's the only one that will be executed by the user + try { + const forwardingFeePretransaction = await buildForwardingFeePretransaction( + transaction, + provider + ) + // If that happens, we give up as we should've been able to perform the action with this + // forwarding path + return forwardingFeePretransaction + ? [forwardingFeePretransaction, transaction, ...path] + : [transaction, ...path] + } catch (err) { + return [] + } + } + // Check if one of the forwarders that has permission to perform an action // with `sig` on `address` can forward for us directly for (const forwarder of forwardersWithPermission) { const script = encodeCallScript([directTransaction]) if (await canForward(forwarder, sender, script, provider)) { - const transaction = createForwarderTransaction(forwarder, script) - try { - const forwardingFeePretransaction = await buildForwardingFeePretransaction( - transaction, - provider - ) - // If that happens, we give up as we should've been able to perform the action with this - // forwarder - return { - forwardingFeePretransaction, - path: [transaction, directTransaction], - } - } catch (err) { - return { path: [] } - } + return buildForwardingPath( + forwarder, + script, + [directTransaction], + provider + ) } } @@ -134,24 +120,7 @@ async function calculateForwardingPath( if (await canForward(forwarder, sender, script, provider)) { // The previous forwarder can forward a transaction for this forwarder, // and this forwarder can forward for our address, so we have found a path - const transaction = createForwarderTransaction(forwarder, script) - - // Only apply pretransactions to the first transaction in the path - // as it's the only one that will be executed by the user - try { - const forwardingFeePretransaction = await buildForwardingFeePretransaction( - transaction, - provider - ) - // If that happens, we give up as we should've been able to perform the action with this - // forwarding path - return { - forwardingFeePretransaction, - path: [transaction, ...path], - } - } catch (err) { - return { path: [] } - } + return buildForwardingPath(forwarder, script, path, provider) } else { // The previous forwarder can forward a transaction for this forwarder, // but this forwarder can not forward for our address, so we add it as a @@ -171,7 +140,7 @@ async function calculateForwardingPath( queue.push([path, nextQueue]) } while (queue.length) - return { path: [] } + return [] } /** @@ -181,34 +150,26 @@ async function calculateForwardingPath( */ export async function calculateTransactionPath( sender: string, - destination: string, + destinationApp: App, methodSignature: string, params: any[], apps: App[], provider: ethers.providers.Provider, finalForwarder?: string //Address of the final forwarder that can perfom the action. Needed for actions that aren't in the ACL but whose execution depends on other factors -): Promise { - // Get the destination app - const destinationApp = apps.find((app) => app.address == destination) - if (!destinationApp) { - throw new Error( - `Transaction path destination (${destination}) is not an installed app` - ) - } - - // Make sure the method signature is correct - const method = validateMethod(destination, methodSignature, destinationApp) - - const finalForwarderProvided = finalForwarder - ? ethers.utils.isAddress(finalForwarder) - : false +): Promise { + // The direct transaction we eventually want to perform const directTransaction = await createDirectTransactionForApp( sender, destinationApp, - method.sig, + methodSignature, params ) + const finalForwarderProvided = finalForwarder + ? ethers.utils.isAddress(finalForwarder) + : false + + const method = getAppMethod(destinationApp, methodSignature) // We can already assume the user is able to directly invoke the action if: // - The method has no ACL requirements and no final forwarder was given, or // - The final forwarder matches the sender @@ -217,11 +178,11 @@ export async function calculateTransactionPath( (finalForwarder && addressesEqual(finalForwarder, sender)) ) { try { - return { path: [directTransaction] } + return [directTransaction] } catch (_) { // If the direct transaction fails, we give up as we should have been able to // perform the action directly - return { path: [] } + return [] } } @@ -236,7 +197,7 @@ export async function calculateTransactionPath( if (!includesAddress(forwarders, finalForwarder)) { // Final forwarder was given, but did not match any available forwarders, so no path // could be found - return { path: [] } + return [] } // Only attempt to find path with declared final forwarder; assume the final forwarder @@ -249,11 +210,13 @@ export async function calculateTransactionPath( (role) => role.name === method.roles[0] ) const allowedEntities = - role?.permissions?.map((permission) => permission.granteeAddress) || [] + role?.permissions + ?.filter((permission) => permission.allowed) + .map((permission) => permission.granteeAddress) || [] // No one has access, so of course we don't as well if (allowedEntities.length === 0) { - return { path: [] } + return [] } // User may have permission; attempt direct transaction @@ -262,7 +225,7 @@ export async function calculateTransactionPath( includesAddress(allowedEntities, ANY_ENTITY) ) { try { - return { path: [directTransaction] } + return [directTransaction] } catch (_) { // Don't immediately fail as the permission could have parameters applied that // disallows the user from the current action and forces us to use the full diff --git a/packages/connect-core/src/utils/path/decodePath.ts b/packages/connect-core/src/utils/path/decodePath.ts deleted file mode 100644 index f8aa38a2..00000000 --- a/packages/connect-core/src/utils/path/decodePath.ts +++ /dev/null @@ -1,39 +0,0 @@ -// import { isCallScript, decodeCallScript, CallScriptAction } from '../callScript' -// import { isValidForwardCall, parseForwardCall } from '../forwarding' - -// export interface TransactionWithChildren extends CallScriptAction { -// children?: TransactionWithChildren[] -// } - -// /** -// * Decodes an EVM callscript and returns the transaction path it describes. -// * -// * @return An array of Ethereum transactions that describe each step in the path -// */ -// export function decodeTransactionPath( -// script: string -// ): TransactionWithChildren[] { -// // In the future we may support more EVMScripts, but for now let's just assume we're only -// // dealing with call scripts -// if (!isCallScript(script)) { -// throw new Error(`Script could not be decoded: ${script}`) -// } - -// const path = decodeCallScript(script) - -// return path.reduce((decodeSegments, segment) => { -// const { data } = segment - -// let children: TransactionWithChildren[] = [] -// if (isValidForwardCall(data)) { -// const forwardedEvmScript = parseForwardCall(data) - -// try { -// children = decodeTransactionPath(forwardedEvmScript) -// // eslint-disable-next-line no-empty -// } catch (err) {} -// } - -// return decodeSegments.concat({ ...segment, children }) -// }, [] as TransactionWithChildren[]) -// } diff --git a/packages/connect-core/src/utils/radspec/index.ts b/packages/connect-core/src/utils/radspec/index.ts index ba9093cb..fb862f28 100644 --- a/packages/connect-core/src/utils/radspec/index.ts +++ b/packages/connect-core/src/utils/radspec/index.ts @@ -5,10 +5,10 @@ import { addressesEqual } from '../address' import { findAppMethodFromIntent } from '../app' import App from '../../entities/App' import Transaction from '../../entities/Transaction' -import { Abi, AppIntent } from '../../types' +import { Abi, AppMethod } from '../../types' interface FoundMethod { - method?: AppIntent + method?: AppMethod abi?: Abi } @@ -69,7 +69,4 @@ export async function tryEvaluatingRadspec( return evaluatedNotice } -export { - postprocessRadspecDescription, - PostProcessDescription, -} from './postprocess' +export { postprocessRadspecDescription } from './postprocess' diff --git a/packages/connect-core/src/utils/radspec/postprocess.ts b/packages/connect-core/src/utils/radspec/postprocess.ts index c883c058..79e229d1 100644 --- a/packages/connect-core/src/utils/radspec/postprocess.ts +++ b/packages/connect-core/src/utils/radspec/postprocess.ts @@ -2,12 +2,7 @@ import { addressesEqual, ANY_ENTITY } from '../address' import { getKernelNamespace } from '../kernel' import App from '../../entities/App' import Role from '../../entities/Role' -import { Annotation } from '../../types' - -export interface PostProcessDescription { - description: string - annotatedDescription?: Annotation[] -} +import { Annotation, PostProcessDescription } from '../../types' interface CompiledTokens { description: string[] diff --git a/packages/connect-thegraph/src/__test__/apps.test.ts b/packages/connect-thegraph/src/__test__/apps.test.ts index a70f6874..ec2fc321 100644 --- a/packages/connect-thegraph/src/__test__/apps.test.ts +++ b/packages/connect-thegraph/src/__test__/apps.test.ts @@ -50,9 +50,9 @@ describe('when connecting to the mainnet subgraph', () => { expect(app.contentUri!.length).toBeGreaterThan(0) }) - test('should have valid intents', () => { - expect(app.intents).toBeDefined() - expect(app.intents!.length).toBeGreaterThan(0) + test('should have valid methods', () => { + expect(app.methods).toBeDefined() + expect(app.methods!.length).toBeGreaterThan(0) }) test('should have valid isForwarder', () => { expect(app.isForwarder).toBeDefined()