From 9b2cc18c450590a01dd8a0e7fbb83743a7341eae Mon Sep 17 00:00:00 2001 From: Jan-Felix Date: Thu, 20 Apr 2023 14:45:43 +0200 Subject: [PATCH] streamline l2: get rid of special inputsMatch entrypoint --- packages/sdk/README.md | 17 ++++---- .../sdk/src/presets/authoring/batching.ts | 6 ++- .../presets/authoring/conditions/matches.ts | 4 +- packages/sdk/src/presets/authoring/erc20.ts | 8 ++-- packages/sdk/src/presets/authoring/index.ts | 1 - .../sdk/src/presets/authoring/inputsMatch.ts | 21 ---------- packages/sdk/src/presets/fillPreset.ts | 31 +++++++------- .../sdk/src/presets/mergeFunctionEntries.ts | 42 ++++++++++--------- packages/sdk/src/presets/types.ts | 13 +++++- packages/sdk/src/presets/utils.ts | 42 +++++++++++++++---- .../presets/deFiManageENS1Untyped.ts | 39 +++++++++-------- 11 files changed, 122 insertions(+), 102 deletions(-) delete mode 100644 packages/sdk/src/presets/authoring/inputsMatch.ts diff --git a/packages/sdk/README.md b/packages/sdk/README.md index 41748b5a4..63365cf05 100644 --- a/packages/sdk/README.md +++ b/packages/sdk/README.md @@ -37,15 +37,13 @@ It also offers various sanity checks and normalizations for conditions. A permission preset is a parametrized set of permissions. It can be applied to different roles, filling in the specific parameter values for the placeholders in conditions. -On this layer, the sdk offers the `inputsMatch()` helper function for defining conditions, relying on user-provided ABIs for contract function parameters. - **Example:** ```javascript { targetAddress: '0x182B723a58739a9c974cFDB385ceaDb237453c28', signature: "claim_rewards(address)", - condition: inputsMatch([AVATAR], ["address"]), + condition: c.matchesAbi([AVATAR], ["address"]), } ``` @@ -87,7 +85,7 @@ Matching patterns can also be supplied as arrays using the `matches([ 1, undefin - `eq` - `lt` - `gt` -- `bitmask` +- `bitmask` _not yet implemented_ #### Array conditions @@ -99,7 +97,6 @@ Matching patterns can also be supplied as arrays using the `matches([ 1, undefin - `and` - `or` -- `xor` - `nor` #### Tuple matching @@ -109,6 +106,10 @@ Matching patterns can also be supplied as arrays using the `matches([ 1, undefin #### Allowance conditions -- `withinAllowance` -- `etherWithinAllowance` -- `callWithinAllowance` +- `withinAllowance` _not yet implemented_ +- `etherWithinAllowance` _not yet implemented_ +- `callWithinAllowance` _not yet implemented_ + +#### Custom conditions + +- `custom` _not yet implemented_ diff --git a/packages/sdk/src/presets/authoring/batching.ts b/packages/sdk/src/presets/authoring/batching.ts index 93a68531f..e690cdd81 100644 --- a/packages/sdk/src/presets/authoring/batching.ts +++ b/packages/sdk/src/presets/authoring/batching.ts @@ -1,8 +1,12 @@ +import { BytesLike } from "ethers" + import { ExecutionFlags, PresetAllowEntry, PresetCondition } from "../types" +import { ConditionFunction } from "./conditions/types" + type PartialPresetFullyClearedTarget = ExecutionFlags type PartialPresetFunction = ({ sighash: string } | { signature: string }) & { - condition?: PresetCondition + condition?: PresetCondition | ConditionFunction } & ExecutionFlags export const forAll = ( diff --git a/packages/sdk/src/presets/authoring/conditions/matches.ts b/packages/sdk/src/presets/authoring/conditions/matches.ts index 230ed5012..ae0a5912d 100644 --- a/packages/sdk/src/presets/authoring/conditions/matches.ts +++ b/packages/sdk/src/presets/authoring/conditions/matches.ts @@ -97,9 +97,9 @@ export const matchesAbi = const paramTypes = abiTypes.map((abiType) => ParamType.from(abiType)) // only supported at the top level or for bytes type params - if (abiType && abiType.name !== "bytes") { + if (abiType && abiType.type !== "bytes") { throw new Error( - `Can only use \`matchesAbi\` on bytes types params, got: ${abiType.type}` + `Can only use \`matchesAbi\` on bytes type params, got: ${abiType.type}` ) } diff --git a/packages/sdk/src/presets/authoring/erc20.ts b/packages/sdk/src/presets/authoring/erc20.ts index 860046870..b166fb90e 100644 --- a/packages/sdk/src/presets/authoring/erc20.ts +++ b/packages/sdk/src/presets/authoring/erc20.ts @@ -1,8 +1,8 @@ import { Placeholder } from "../types" import { forAll } from "./batching" +import { matchesAbi } from "./conditions" import { or } from "./conditions/branching" -import { inputsMatch } from "./inputsMatch" export const allowErc20Approve = ( tokens: string[], @@ -12,7 +12,7 @@ export const allowErc20Approve = ( return forAll(tokens, { signature: "approve(address,uint256)", - condition: inputsMatch( + condition: matchesAbi( [ spenders.length === 1 ? spenders[0] @@ -33,7 +33,7 @@ export const allowErc20Revoke = ( signature: "approve(address,uint256)", condition: spenders && - inputsMatch( + matchesAbi( [ spenders.length === 1 ? spenders[0] @@ -50,7 +50,7 @@ export const allowErc20Transfer = (tokens: string[], recipients: string[]) => { return forAll(tokens, { signature: "transfer(address,uint256)", - condition: inputsMatch( + condition: matchesAbi( [ recipients.length === 1 ? recipients[0] diff --git a/packages/sdk/src/presets/authoring/index.ts b/packages/sdk/src/presets/authoring/index.ts index cbbb854e1..58aa0429a 100644 --- a/packages/sdk/src/presets/authoring/index.ts +++ b/packages/sdk/src/presets/authoring/index.ts @@ -1,2 +1 @@ -export { inputsMatch } from "./inputsMatch" export { allow, EVERYTHING, ethSdk } from "./kit" diff --git a/packages/sdk/src/presets/authoring/inputsMatch.ts b/packages/sdk/src/presets/authoring/inputsMatch.ts deleted file mode 100644 index 533f702d8..000000000 --- a/packages/sdk/src/presets/authoring/inputsMatch.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { ParamType } from "ethers/lib/utils" - -import { AbiType } from "../types" - -import { matchesAbi } from "./conditions" -import { TupleScopings } from "./conditions/types" - -/** - * Entry point function for defining conditions when not using a typed allow kit - * @param scoping The conditions structure over the function inputs - * @param abiTypes The ABI types of the function inputs - **/ -export const inputsMatch = ( - scopings: TupleScopings, - abiTypes: AbiType[] -) => { - return matchesAbi( - scopings, - abiTypes.map((abiType) => ParamType.from(abiType)) - )() -} diff --git a/packages/sdk/src/presets/fillPreset.ts b/packages/sdk/src/presets/fillPreset.ts index 42876eafc..dd3efe0a1 100644 --- a/packages/sdk/src/presets/fillPreset.ts +++ b/packages/sdk/src/presets/fillPreset.ts @@ -19,8 +19,9 @@ import { Placeholder, ComparisonValue, PresetCondition, + PresetFunctionCoerced, } from "./types" -import { functionId, isScoped, sighash } from "./utils" +import { allowEntryId, isScoped } from "./utils" /** * Processes a RolePreset, filling in the placeholders and returning the final permissions @@ -32,15 +33,17 @@ export const fillPreset =

( preset: P, placeholderValues: PlaceholderValues

): Target[] => { - preset = mergeFunctionEntries(preset) as P - sanityCheck(preset) + const mergedPreset = mergeFunctionEntries(preset) + sanityCheck(mergedPreset) const placeholderLookupMap = makePlaceholderLookupMap( - preset, + mergedPreset, placeholderValues ) - const fullyClearedTargets = preset.allow + const { allow } = mergedPreset + + const fullyClearedTargets = allow .filter((entry) => !isScoped(entry)) .map((entry) => ({ address: entry.targetAddress.toLowerCase(), @@ -49,24 +52,20 @@ export const fillPreset =

( functions: [], })) + const allowFunctionEntries = allow.filter( + (entry) => "selector" in entry + ) as PresetFunctionCoerced[] + const functionScopedTargets = Object.entries( - groupBy(preset.allow.filter(isScoped), (entry) => - entry.targetAddress.toLowerCase() - ) + groupBy(allowFunctionEntries, (entry) => entry.targetAddress) ).map(([targetAddress, allowFunctions]) => ({ address: targetAddress.toLowerCase(), clearance: Clearance.Function, executionOptions: ExecutionOptions.None, functions: allowFunctions.map((allowFunction) => { - let selector = "selector" in allowFunction && allowFunction.selector - if (!selector) { - selector = - "signature" in allowFunction && sighash(allowFunction.signature) - } - if (!selector) throw new Error("invariant violation") const { condition } = allowFunction return { - selector, + selector: allowFunction.selector, executionOptions: execOptions(allowFunction), wildcarded: !condition, condition: @@ -168,7 +167,7 @@ const assertNoWildcardScopedIntersection = (preset: PermissionPreset) => { } const assertNoDuplicateAllowFunction = (preset: PermissionPreset) => { - const allowFunctions = preset.allow.filter(isScoped).map(functionId) + const allowFunctions = preset.allow.filter(isScoped).map(allowEntryId) const counts = allowFunctions.reduce( (result, item) => ({ ...result, [item]: (result[item] || 0) + 1 }), diff --git a/packages/sdk/src/presets/mergeFunctionEntries.ts b/packages/sdk/src/presets/mergeFunctionEntries.ts index f0886120f..4e3fe235a 100644 --- a/packages/sdk/src/presets/mergeFunctionEntries.ts +++ b/packages/sdk/src/presets/mergeFunctionEntries.ts @@ -1,12 +1,12 @@ import { Operator, ParameterType } from "../types" import { - PresetAllowEntry, PermissionPreset, - PresetFunction, PresetCondition, + PresetAllowEntryCoerced, + PresetFunctionCoerced, } from "./types" -import { functionId, isScoped } from "./utils" +import { coercePresetFunction, allowEntryId, isScoped } from "./utils" /** * Processes the allow entries of a preset and merges entries addressing the same function into a single entry. @@ -14,21 +14,26 @@ import { functionId, isScoped } from "./utils" * @param preset The preset to process * @returns The updated preset */ -export const mergeFunctionEntries = ( - preset: PermissionPreset -): PermissionPreset => ({ +export const mergeFunctionEntries = (preset: PermissionPreset) => ({ ...preset, allow: preset.allow.reduce((result, entry) => { if (!isScoped(entry)) { - result.push(entry) + result.push({ + ...entry, + targetAddress: entry.targetAddress.toLowerCase(), + }) return result } - const matchingEntry = result - .filter(isScoped) - .find((existingEntry) => functionId(existingEntry) === functionId(entry)) + const coercedEntry = coercePresetFunction(entry) + + const matchingEntry = result.find( + (existingEntry) => + allowEntryId(existingEntry) === allowEntryId(coercedEntry) + ) as PresetFunctionCoerced | undefined + if (!matchingEntry) { - result.push(entry) + result.push(coercedEntry) return result } @@ -37,14 +42,14 @@ export const mergeFunctionEntries = ( !!matchingEntry.delegatecall !== !!entry.delegatecall ) { // we don't merge if execution options are different - result.push(entry) + result.push(coercedEntry) return result } // merge conditions into the entry we already have - matchingEntry.condition = mergeConditions(matchingEntry, entry) + matchingEntry.condition = mergeConditions(matchingEntry, coercedEntry) return result - }, [] as PresetAllowEntry[]), + }, [] as PresetAllowEntryCoerced[]), }) /** @@ -52,14 +57,13 @@ export const mergeFunctionEntries = ( * These will be pruned later in sanitizeCondition(). */ const mergeConditions = ( - a: PresetFunction, - b: PresetFunction + a: PresetFunctionCoerced, + b: PresetFunctionCoerced ): PresetCondition | undefined => { if (!!a.condition !== !!b.condition) { + const targetId = allowEntryId(a) console.warn( - `Target ${functionId( - a - )} is allowed with and without conditions. It will be allowed without conditions.` + `Target ${targetId} is allowed with and without conditions. It will be allowed without conditions.` ) return undefined } diff --git a/packages/sdk/src/presets/types.ts b/packages/sdk/src/presets/types.ts index 6c5e4b200..e3f9d5117 100644 --- a/packages/sdk/src/presets/types.ts +++ b/packages/sdk/src/presets/types.ts @@ -1,7 +1,9 @@ -import { ParamType } from "ethers/lib/utils" +import { BytesLike, ParamType } from "ethers/lib/utils" import { Operator, ParameterType } from "../types" +import { ConditionFunction } from "./authoring/conditions/types" + export type AbiType = string | ParamType export interface ExecutionFlags { @@ -60,11 +62,20 @@ export type PresetFullyClearedTarget = { // allows calls to specific functions, optionally with parameter scoping export type PresetFunction = ({ selector: string } | { signature: string }) & { + targetAddress: string + condition?: PresetCondition | ConditionFunction // condition entrypoint can be a condition function that will be invoked with `bytes` abiType (undecoded calldata) +} & ExecutionFlags + +export type PresetFunctionCoerced = { + selector: string targetAddress: string condition?: PresetCondition } & ExecutionFlags export type PresetAllowEntry = PresetFullyClearedTarget | PresetFunction +export type PresetAllowEntryCoerced = + | PresetFullyClearedTarget + | PresetFunctionCoerced export type ComparisonValue = string | Placeholder diff --git a/packages/sdk/src/presets/utils.ts b/packages/sdk/src/presets/utils.ts index 2a166aa92..1ea17cff0 100644 --- a/packages/sdk/src/presets/utils.ts +++ b/packages/sdk/src/presets/utils.ts @@ -1,14 +1,38 @@ -import { keccak256, toUtf8Bytes } from "ethers/lib/utils" +import { keccak256, ParamType, toUtf8Bytes } from "ethers/lib/utils" -import { PresetAllowEntry, PresetFunction } from "./types" +import { + PresetAllowEntry, + PresetAllowEntryCoerced, + PresetFunction, + PresetFunctionCoerced, +} from "./types" -export const sighash = (signature: string): string => +const sighash = (signature: string): string => keccak256(toUtf8Bytes(signature)).substring(0, 10) -export const isScoped = (entry: PresetAllowEntry): entry is PresetFunction => - "selector" in entry || "signature" in entry +export const coercePresetFunction = ( + entry: PresetFunction +): PresetFunctionCoerced => { + return { + targetAddress: entry.targetAddress.toLowerCase(), + selector: + "selector" in entry + ? entry.selector.toLowerCase() + : sighash(entry.signature), + condition: + typeof entry.condition === "function" + ? entry.condition(ParamType.from("bytes")) + : entry.condition, + send: entry.send, + delegatecall: entry.delegatecall, + } +} -export const functionId = (entry: PresetFunction) => - `${entry.targetAddress.toLowerCase()}.${ - "selector" in entry ? entry.selector : sighash(entry.signature) - }` +export const isScoped = (entry: PresetAllowEntry): entry is PresetFunction => { + return "selector" in entry || "signature" in entry +} + +export const allowEntryId = (entry: PresetAllowEntryCoerced) => + "selector" in entry + ? `${entry.targetAddress.toLowerCase()}.${entry.selector}` + : entry.targetAddress.toLowerCase() diff --git a/packages/sdk/test/karpatkey/presets/deFiManageENS1Untyped.ts b/packages/sdk/test/karpatkey/presets/deFiManageENS1Untyped.ts index a751a118d..f11fd0536 100644 --- a/packages/sdk/test/karpatkey/presets/deFiManageENS1Untyped.ts +++ b/packages/sdk/test/karpatkey/presets/deFiManageENS1Untyped.ts @@ -1,4 +1,3 @@ -import { inputsMatch } from "../../../src/presets/authoring" import * as c from "../../../src/presets/authoring/conditions" import { AVATAR } from "../../../src/presets/placeholders" import { PermissionPreset } from "../../../src/presets/types" @@ -66,7 +65,7 @@ const preset = { { targetAddress: stETH, signature: "submit(address)", - condition: inputsMatch([ZERO_ADDRESS], ["address"]), + condition: c.matchesAbi([ZERO_ADDRESS], ["address"]), send: true, }, { targetAddress: wstETH, signature: "wrap(uint256)" }, @@ -124,7 +123,7 @@ const preset = { { targetAddress: COMPTROLLER, signature: "claimComp(address,address[])", - condition: inputsMatch( + condition: c.matchesAbi( [AVATAR, c.subset([cDAI, cUSDC])], ["address", "address[]"] ), @@ -145,7 +144,7 @@ const preset = { { targetAddress: STAKEWISE_MERKLE_DIS, signature: "claim(uint256,address,address[],uint256[],bytes32[])", - condition: inputsMatch( + condition: c.matchesAbi( [undefined, AVATAR, [rETH2, SWISE]], ["uint256", "address", "address[]", "uint256[]", "bytes32[]"] ), @@ -165,7 +164,7 @@ const preset = { targetAddress: UV3_NFT_POSITIONS, signature: "mint((address,address,uint24,int24,int24,uint256,uint256,uint256,uint256,address,uint256))", - condition: inputsMatch( + condition: c.matchesAbi( [ { token0: WETH, @@ -188,7 +187,7 @@ const preset = { targetAddress: UV3_NFT_POSITIONS, signature: "increaseLiquidity((uint256,uint256,uint256,uint256,uint256,uint256))", - condition: inputsMatch( + condition: c.matchesAbi( [{ tokenId: 424810 }], [ "(uint256 tokenId, uint256 amount0Desired, uint256 amount1Desired, uint256 amount0Min, uint256 amount1Min, uint256 deadline)", @@ -213,7 +212,7 @@ const preset = { { targetAddress: UV3_NFT_POSITIONS, signature: "collect((uint256,address,uint128,uint128))", - condition: inputsMatch( + condition: c.matchesAbi( [c.matches([undefined, AVATAR])], ["(uint256,address,uint128,uint128)"] ), @@ -266,14 +265,14 @@ const preset = { { targetAddress: CURVE_stETH_ETH_GAUGE, signature: "claim_rewards(address)", // IMPORTANT!: CHANGE FOR "claim_rewards()" - condition: inputsMatch([AVATAR], ["address"]), + condition: c.matchesAbi([AVATAR], ["address"]), }, //Claiming CRV rewards { targetAddress: CRV_MINTER, signature: "mint(address)", - condition: inputsMatch([CURVE_stETH_ETH_GAUGE], ["address"]), + condition: c.matchesAbi([CURVE_stETH_ETH_GAUGE], ["address"]), }, //--------------------------------------------------------------------------------------------------------------------------------- @@ -286,7 +285,7 @@ const preset = { targetAddress: AURA_REWARD_POOL_DEPOSIT_WRAPPER, signature: "depositSingle(address,address,uint256,bytes32,(address[],uint256[],bytes,bool))", - condition: inputsMatch( + condition: c.matchesAbi( [ AURA_BALANCER_stETH_VAULT, // rewardPoolAddress WETH, // inputToken @@ -332,7 +331,7 @@ const preset = { targetAddress: BALANCER_VAULT, signature: "exitPool(bytes32,address,address,(address[],uint256[],bytes,bool))", - condition: inputsMatch( + condition: c.matchesAbi( [ "0x32296969ef14eb0c6d29669c550d4a0449130230000200000000000000000080", // poolId AVATAR, // sender @@ -367,7 +366,7 @@ const preset = { { targetAddress: UV3_ROUTER_2, signature: "swapExactTokensForTokens(uint256,uint256,address[],address)", - condition: inputsMatch( + condition: c.matchesAbi( [ undefined, // amountIn undefined, // amountOutMin @@ -411,7 +410,7 @@ const preset = { targetAddress: UV3_ROUTER_2, signature: "exactInputSingle((address,address,uint24,address,uint256,uint256,uint160))", - condition: inputsMatch( + condition: c.matchesAbi( [ { tokenIn: c.or( @@ -465,7 +464,7 @@ const preset = { targetAddress: BALANCER_VAULT, signature: "swap((bytes32,uint8,address,address,uint256,bytes),(address,bool,address,bool),uint256,uint256)", - condition: inputsMatch( + condition: c.matchesAbi( [ { poolId: @@ -495,7 +494,7 @@ const preset = { targetAddress: BALANCER_VAULT, signature: "swap((bytes32,uint8,address,address,uint256,bytes),(address,bool,address,bool),uint256,uint256)", - condition: inputsMatch( + condition: c.matchesAbi( [ { poolId: @@ -525,7 +524,7 @@ const preset = { targetAddress: BALANCER_VAULT, signature: "swap((bytes32,uint8,address,address,uint256,bytes),(address,bool,address,bool),uint256,uint256)", - condition: inputsMatch( + condition: c.matchesAbi( [ { poolId: @@ -555,7 +554,7 @@ const preset = { targetAddress: BALANCER_VAULT, signature: "swap((bytes32,uint8,address,address,uint256,bytes),(address,bool,address,bool),uint256,uint256)", - condition: inputsMatch( + condition: c.matchesAbi( [ { poolId: @@ -585,7 +584,7 @@ const preset = { targetAddress: BALANCER_VAULT, signature: "swap((bytes32,uint8,address,address,uint256,bytes),(address,bool,address,bool),uint256,uint256)", - condition: inputsMatch( + condition: c.matchesAbi( [ { poolId: @@ -615,7 +614,7 @@ const preset = { targetAddress: BALANCER_VAULT, signature: "swap((bytes32,uint8,address,address,uint256,bytes),(address,bool,address,bool),uint256,uint256)", - condition: inputsMatch( + condition: c.matchesAbi( [ c.or( { @@ -660,7 +659,7 @@ const preset = { targetAddress: SUSHISWAP_ROUTER, signature: "swapExactTokensForTokens(uint256,uint256,address[],address,uint256)", - condition: inputsMatch( + condition: c.matchesAbi( [ undefined, undefined,