diff --git a/contracts/BaseACPHook.sol b/contracts/BaseACPHook.sol index 893c8d0..76ebf9d 100644 --- a/contracts/BaseACPHook.sol +++ b/contracts/BaseACPHook.sol @@ -17,7 +17,7 @@ import "@openzeppelin/contracts/utils/introspection/ERC165.sol"; * AgenticCommerce supports operators, so the actual caller matters. * * Data encoding per selector (as produced by AgenticCommerce): - * setBudget : abi.encode(caller, amount, optParams) + * setBudget : abi.encode(caller, token, amount, optParams) * fund : abi.encode(caller, optParams) * submit : abi.encode(caller, deliverable, optParams) * complete : abi.encode(caller, reason, optParams) @@ -56,7 +56,7 @@ abstract contract BaseACPHook is ERC165, IACPHook { // --- Selector constants (avoid repeated keccak at runtime) ---------------- // These match AgenticCommerce function selectors. bytes4 private constant SEL_SET_BUDGET = - bytes4(keccak256("setBudget(uint256,uint256,bytes)")); + bytes4(keccak256("setBudget(uint256,address,uint256,bytes)")); bytes4 private constant SEL_FUND = bytes4(keccak256("fund(uint256,uint256,bytes)")); bytes4 private constant SEL_SUBMIT = @@ -74,9 +74,9 @@ abstract contract BaseACPHook is ERC165, IACPHook { bytes calldata data ) external override onlyACP { if (selector == SEL_SET_BUDGET) { - (address caller, uint256 amount, bytes memory optParams) = abi - .decode(data, (address, uint256, bytes)); - _preSetBudget(jobId, caller, amount, optParams); + (address caller, address token, uint256 amount, bytes memory optParams) = abi + .decode(data, (address, address, uint256, bytes)); + _preSetBudget(jobId, caller, token, amount, optParams); } else if (selector == SEL_FUND) { (address caller, bytes memory optParams) = abi.decode( data, @@ -104,9 +104,9 @@ abstract contract BaseACPHook is ERC165, IACPHook { bytes calldata data ) external override onlyACP { if (selector == SEL_SET_BUDGET) { - (address caller, uint256 amount, bytes memory optParams) = abi - .decode(data, (address, uint256, bytes)); - _postSetBudget(jobId, caller, amount, optParams); + (address caller, address token, uint256 amount, bytes memory optParams) = abi + .decode(data, (address, address, uint256, bytes)); + _postSetBudget(jobId, caller, token, amount, optParams); } else if (selector == SEL_FUND) { (address caller, bytes memory optParams) = abi.decode( data, @@ -133,12 +133,14 @@ abstract contract BaseACPHook is ERC165, IACPHook { function _preSetBudget( uint256 jobId, address caller, + address token, uint256 amount, bytes memory optParams ) internal virtual {} function _postSetBudget( uint256 jobId, address caller, + address token, uint256 amount, bytes memory optParams ) internal virtual {} diff --git a/contracts/acp b/contracts/acp index d35d995..9e4fcd9 160000 --- a/contracts/acp +++ b/contracts/acp @@ -1 +1 @@ -Subproject commit d35d995094dbb095f09aef16ccdc1152a94f95d3 +Subproject commit 9e4fcd9663f5af84ab96be1bd74d398634908ea0 diff --git a/contracts/hooks/BiddingHook.sol b/contracts/hooks/BiddingHook.sol index 6f82a73..3044ce9 100644 --- a/contracts/hooks/BiddingHook.sol +++ b/contracts/hooks/BiddingHook.sol @@ -73,7 +73,7 @@ contract BiddingHook is BaseACPHook { /// 2. deadline > 0 && committedAmount == 0: bid verification, decode /// signature from optParams, verify against provider. /// 3. committedAmount > 0: enforce budget == committedAmount. - function _preSetBudget(uint256 jobId, address, uint256 amount, bytes memory optParams) internal override { + function _preSetBudget(uint256 jobId, address, address, uint256 amount, bytes memory optParams) internal override { Bidding storage b = biddings[jobId]; // Mode 3: enforce budget matches the winning bid diff --git a/contracts/hooks/FundTransferHook.sol b/contracts/hooks/FundTransferHook.sol index 6189a02..db832b2 100644 --- a/contracts/hooks/FundTransferHook.sol +++ b/contracts/hooks/FundTransferHook.sol @@ -91,7 +91,7 @@ contract FundTransferHook is BaseACPHook { // ------------------------------------------------------------------------- /// @dev Store transfer commitment from setBudget optParams. - function _preSetBudget(uint256 jobId, address, uint256, bytes memory optParams) internal override { + function _preSetBudget(uint256 jobId, address, address, uint256, bytes memory optParams) internal override { if (optParams.length == 0) return; (address buyer, uint256 transferAmount) = abi.decode(optParams, (address, uint256)); if (buyer == address(0)) revert ZeroAddress(); diff --git a/contracts/hooks/MultiHookRouter.sol b/contracts/hooks/MultiHookRouter.sol new file mode 100644 index 0000000..e5a123f --- /dev/null +++ b/contracts/hooks/MultiHookRouter.sol @@ -0,0 +1,264 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import "@acp/IACPHook.sol"; +import "@acp/AgenticCommerce.sol"; +import "@openzeppelin/contracts/access/Ownable.sol"; +import "@openzeppelin/contracts/utils/introspection/ERC165.sol"; +import "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol"; +import "@openzeppelin/contracts/utils/ReentrancyGuardTransient.sol"; + +/// @title MultiHookRouter +/// @notice Routes hook callbacks to an ordered list of sub-hooks per job. +/// @dev Implements IACPHook so the core contract sees it as a single hook. +/// Non-upgradeable by design — hooks should be immutable once deployed. +/// Sub-hooks must be whitelisted on the core contract to be used. +/// Exposes passthrough view functions so sub-hooks deployed with +/// acpContract = routerAddress can call _core().getJob() etc. +contract MultiHookRouter is ERC165, IACPHook, ReentrancyGuardTransient, Ownable { + // ──────────────────── Immutables ──────────────────── + + /// @notice The ACP core contract + address public immutable acpContract; + + // ──────────────────── Storage ──────────────────── + + /// @notice Maximum sub-hooks per job (admin-configurable gas safety cap) + uint256 public maxHooksPerJob; + + /// @notice Per-job ordered list of sub-hooks + mapping(uint256 jobId => address[] hooks) private _jobHooks; + + // ──────────────────── Errors ──────────────────── + + error OnlyACPContract(); + error OnlyJobClient(); + error HooksLocked(); + error TooManyHooks(); + error InvalidHook(); + error DuplicateHook(); + error HookNotFound(); + error ZeroAddress(); + error EmptyArray(); + error HookSetMismatch(); + error SubHookNotWhitelisted(); + + // ──────────────────── Events ──────────────────── + + event HooksConfigured(uint256 indexed jobId, address[] hooks); + event HookAdded(uint256 indexed jobId, address indexed hook, uint256 position); + event HookRemoved(uint256 indexed jobId, address indexed hook); + event HooksReordered(uint256 indexed jobId, address[] hooks); + event MaxHooksPerJobUpdated(uint256 oldMax, uint256 newMax); + + // ──────────────────── Modifiers ──────────────────── + + modifier onlyACP() { + if (msg.sender != acpContract) revert OnlyACPContract(); + _; + } + + modifier onlyJobClient(uint256 jobId) { + AgenticCommerce.Job memory job = AgenticCommerce(acpContract).getJob(jobId); + if (msg.sender != job.client) revert OnlyJobClient(); + _; + } + + modifier hooksNotLocked(uint256 jobId) { + AgenticCommerce.Job memory job = AgenticCommerce(acpContract).getJob(jobId); + if (job.status != AgenticCommerce.JobStatus.Open) revert HooksLocked(); + _; + } + + // ──────────────────── Constructor ──────────────────── + + constructor(address acpContract_, address owner_, uint256 maxHooksPerJob_) Ownable(owner_) { + if (acpContract_ == address(0)) revert ZeroAddress(); + acpContract = acpContract_; + maxHooksPerJob = maxHooksPerJob_; + } + + // ──────────────────── Admin ──────────────────── + + /// @notice Update the maximum sub-hooks allowed per job + /// @param newMax New maximum + function setMaxHooksPerJob(uint256 newMax) external onlyOwner { + uint256 oldMax = maxHooksPerJob; + maxHooksPerJob = newMax; + emit MaxHooksPerJobUpdated(oldMax, newMax); + } + + // ──────────────────── Configuration ──────────────────── + + /// @notice Replace the entire hook list for a job + /// @param jobId The job ID + /// @param hooks Ordered array of sub-hook addresses + function configureHooks( + uint256 jobId, + address[] calldata hooks + ) external onlyJobClient(jobId) hooksNotLocked(jobId) { + if (hooks.length > maxHooksPerJob) revert TooManyHooks(); + + for (uint256 i; i < hooks.length; ) { + _validateSubHook(hooks[i]); + // O(n) duplicate check — acceptable for max ~10 hooks + for (uint256 j; j < i; ) { + if (hooks[j] == hooks[i]) revert DuplicateHook(); + unchecked { ++j; } + } + unchecked { ++i; } + } + + _jobHooks[jobId] = hooks; + emit HooksConfigured(jobId, hooks); + } + + /// @notice Append a hook to the end of the list + /// @param jobId The job ID + /// @param hook The sub-hook address to add + function addHook( + uint256 jobId, + address hook + ) external onlyJobClient(jobId) hooksNotLocked(jobId) { + _validateSubHook(hook); + + address[] storage hooks = _jobHooks[jobId]; + if (hooks.length >= maxHooksPerJob) revert TooManyHooks(); + + for (uint256 i; i < hooks.length; ) { + if (hooks[i] == hook) revert DuplicateHook(); + unchecked { ++i; } + } + + hooks.push(hook); + emit HookAdded(jobId, hook, hooks.length - 1); + } + + /// @notice Remove a hook from the list + /// @param jobId The job ID + /// @param hook The sub-hook address to remove + function removeHook( + uint256 jobId, + address hook + ) external onlyJobClient(jobId) hooksNotLocked(jobId) { + address[] storage hooks = _jobHooks[jobId]; + uint256 len = hooks.length; + + for (uint256 i; i < len; ) { + if (hooks[i] == hook) { + hooks[i] = hooks[len - 1]; + hooks.pop(); + emit HookRemoved(jobId, hook); + return; + } + unchecked { ++i; } + } + + revert HookNotFound(); + } + + /// @notice Replace the hook list with a reordered version (must be a permutation) + /// @param jobId The job ID + /// @param hooks New ordering (must contain the same hooks) + function reorderHooks( + uint256 jobId, + address[] calldata hooks + ) external onlyJobClient(jobId) hooksNotLocked(jobId) { + address[] storage current = _jobHooks[jobId]; + if (hooks.length != current.length) revert HookSetMismatch(); + if (hooks.length == 0) revert EmptyArray(); + + // Verify permutation: every new entry exists in current, no duplicates + for (uint256 i; i < hooks.length; ) { + // Check no duplicates in new array + for (uint256 k; k < i; ) { + if (hooks[k] == hooks[i]) revert DuplicateHook(); + unchecked { ++k; } + } + // Check exists in current array + bool found; + for (uint256 j; j < current.length; ) { + if (hooks[i] == current[j]) { + found = true; + break; + } + unchecked { ++j; } + } + if (!found) revert HookNotFound(); + unchecked { ++i; } + } + + _jobHooks[jobId] = hooks; + emit HooksReordered(jobId, hooks); + } + + // ──────────────────── IACPHook Implementation ──────────────────── + + /// @inheritdoc IACPHook + function beforeAction( + uint256 jobId, + bytes4 selector, + bytes calldata data + ) external override onlyACP nonReentrant { + address[] storage hooks = _jobHooks[jobId]; + uint256 len = hooks.length; + for (uint256 i; i < len; ) { + IACPHook(hooks[i]).beforeAction(jobId, selector, data); + unchecked { ++i; } + } + } + + /// @inheritdoc IACPHook + function afterAction( + uint256 jobId, + bytes4 selector, + bytes calldata data + ) external override onlyACP nonReentrant { + address[] storage hooks = _jobHooks[jobId]; + uint256 len = hooks.length; + for (uint256 i; i < len; ) { + IACPHook(hooks[i]).afterAction(jobId, selector, data); + unchecked { ++i; } + } + } + + // ──────────────────── Passthrough Views ──────────────────── + + /// @notice Passthrough to core getJob — allows sub-hooks to call _core().getJob() + function getJob(uint256 jobId) external view returns (AgenticCommerce.Job memory) { + return AgenticCommerce(acpContract).getJob(jobId); + } + + // ──────────────────── Views ──────────────────── + + /// @notice Get the ordered hook list for a job + function getHooks(uint256 jobId) external view returns (address[] memory) { + return _jobHooks[jobId]; + } + + /// @notice Get the number of hooks configured for a job + function hookCount(uint256 jobId) external view returns (uint256) { + return _jobHooks[jobId].length; + } + + // ──────────────────── ERC165 ──────────────────── + + function supportsInterface( + bytes4 interfaceId + ) public view override(ERC165, IERC165) returns (bool) { + return + interfaceId == type(IACPHook).interfaceId || + super.supportsInterface(interfaceId); + } + + // ──────────────────── Internal ──────────────────── + + /// @dev Validate a sub-hook: non-zero, whitelisted on core, supports IACPHook + function _validateSubHook(address hook) private view { + if (hook == address(0)) revert ZeroAddress(); + if (!AgenticCommerce(acpContract).whitelistedHooks(hook)) + revert SubHookNotWhitelisted(); + if (!ERC165Checker.supportsInterface(hook, type(IACPHook).interfaceId)) + revert InvalidHook(); + } +} diff --git a/docs/multi-hook-router.md b/docs/multi-hook-router.md new file mode 100644 index 0000000..11db28a --- /dev/null +++ b/docs/multi-hook-router.md @@ -0,0 +1,164 @@ +# Multi-Hook Router + +## The Problem + +Today, each ERC-8183 job can only attach **one hook**. If a job needs escrow handling, privacy verification, and reputation tracking, all that logic must be crammed into a single contract. + +This creates **monolithic hooks** — hard to build, hard to audit, and impossible to reuse across different job types. + +## The Solution + +A **Multi-Hook Router** sits between the core contract and individual hooks, forwarding callbacks to an ordered list of small, focused hooks. + +``` + CURRENT PROPOSED + ┌────────────────┐ ┌────────────────┐ + │ ERC-8183 │ │ ERC-8183 │ + └───────┬────────┘ └───────┬────────┘ + │ │ + ▼ ▼ + ┌──────────────────┐ ┌───────────────────┐ + │ Single Hook │ │ MultiHookRouter │ + │ (does everything│ └──┬──────┬──────┬──┘ + │ or nothing) │ │ │ │ + └──────────────────┘ ▼ ▼ ▼ + Hook1 Hook2 Hook3 + (escrow)(privacy)(reputation) +``` + +## How It Works + +1. Job creator sets the **MultiHookRouter** as the job's hook +2. Client configures which sub-hooks to use and in what order +3. On every state transition (fund, submit, complete, etc.): + - Router calls each sub-hook's `beforeAction` in order — any can block the transition + - Core executes the state change + - Router calls each sub-hook's `afterAction` in order — for bookkeeping + +## Hook Flow + +### Without MultiHookRouter (Single Hook) + +``` +Client ERC-8183 Single Hook + | | | + |-- fund() -------------->| | + | |-- beforeAction() -->| + | |<--------------------| + | | | + | | [state change] | + | | | + | |-- afterAction() --->| + | |<--------------------| + |<-------------------------| | +``` + +One hook = 2 external calls per transition. Always. + +### With MultiHookRouter — 5 Hooks + +``` +Client ERC-8183 Router H1 H2 H3 H4 H5 + | | | + |-- fund() ---->| | + | |-- beforeAction()-->| + | | |-- before() -->| + | | |<--------------| + | | |-- before() -------->| + | | |<--------------------| + | | |-- before() -------------->| + | | |<--------------------------| + | | |-- before() ---------------------->| + | | |<---------------------------------| + | | |-- before() ------------------------------>| + | | |<-----------------------------------------| + | |<----------------| + | | | + | | [state change] | + | | | + | |-- afterAction()-->| + | | |-- after() --->| + | | |<--------------| + | | |-- after() ---------->| + | | |<--------------------| + | | |-- after() --------------->| + | | |<--------------------------| + | | |-- after() ----------------------->| + | | |<---------------------------------| + | | |-- after() -------------------------------->| + | | |<------------------------------------------| + | |<----------------| + |<---------------| + +External calls: 2 (core -> router) + 10 (router -> hooks) = 12 total +``` + +### Gas Overhead + +| Hooks | External Calls Per Transition | Estimated Router Overhead | +|-------|-------------------------------|---------------------------| +| 0 (no router) | 2 (core -> hook) | -- | +| 1 (via router) | 4 (core -> router -> hook x2) | ~3,000 gas | +| 5 | 12 | ~15,000 gas | +| 10 | 22 | ~30,000 gas | +| 20 | 42 | ~60,000 gas | + +The formula is `2 + (N x 2)` external calls per transition. Each call from the router to a sub-hook is sequential — if any `beforeAction` reverts, the remaining hooks are never called and the entire transition is blocked. + +## Comparison + +| | Current (Single Hook) | Multi-Hook Router | +|---|---|---| +| **Hooks per job** | 1 | Configurable (admin-set cap) | +| **Composability** | Must build one contract that does everything | Mix and match small, focused hooks | +| **Reusability** | Low — each hook is custom-built per use case | High — same privacy hook works across job types | +| **Audit surface** | One large contract | Multiple small contracts (easier to review individually) | +| **Core changes needed** | — | None | +| **Gas overhead** | Baseline | ~3,000 gas per additional hook | +| **Flexibility** | Add a new concern = rewrite the hook | Add a new concern = plug in another hook | + +## Example: Job With Three Concerns + +**Current approach** — build a single `EscrowPrivacyReputationHook`: +- 1 contract, ~500+ lines +- Tightly coupled — changing escrow logic risks breaking privacy logic +- Cannot reuse the privacy piece for a different job type + +**With Multi-Hook Router** — configure 3 independent hooks: +- `FundTransferHook` — handles token escrow (already built) +- `PrivacyHook` — verifies ZK proofs for confidential deliverables +- `ReputationHook` — tracks provider completion rates + +Each is independently developed, tested, and audited. A job that only needs escrow + reputation simply drops the privacy hook from the list. + +## Industry Precedent + +This is not a novel pattern. Major protocols use the same approach in production: + +| Protocol | How They Do It | +|---|---| +| **Uniswap v4** | Hook middleware contracts chain multiple hooks per pool | +| **ERC-6900** | Modular smart accounts with composable validation, execution, and hook modules | +| **Safe (Gnosis)** | Multiple modules + guards enabled simultaneously on a single wallet | + +## What Changes for Users + +**Nothing.** Users interact with ERC-8183 the same way they do today. The router is transparent — it looks like a single hook to the core. + +The only difference is during job setup: instead of deploying a custom hook, the client configures which existing hooks to attach. + +## Risk and Tradeoffs + +| Consideration | Detail | +|---|---| +| **Gas cost** | Each additional hook adds ~3,000 gas per transition. 5 hooks = ~15,000 extra gas. Manageable on L2s, noticeable on L1. | +| **Ordering matters** | Hook execution order affects behavior. Access control hooks should run before payment hooks. | +| **Hook list is locked after funding** | Once money is escrowed, the hook list cannot change. This prevents manipulation mid-job. | +| **Sub-hook compatibility** | Existing hooks need minor adaptation to work inside the router (trust chain adjustment). New hooks built for the router work out of the box. | + +## Impact + +- No changes to the core `AgenticCommerce` contract +- The router is a new standalone contract (~250 lines) +- Existing `FundTransferHook` continues to work as-is for single-hook jobs +- Multi-hook support is additive — it does not break or replace anything