From 560e043a42971bb0716067651b6934d8627f181b Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Mon, 15 Jun 2026 15:24:20 +0800 Subject: [PATCH 01/32] feat: make setProvider hookable with optParams and actor in ProviderSet --- contracts/ERC8183.sol | 29 +++++++-- contracts/ERC8183WithAuthorization.sol | 16 ++++- test/ERC8183.t.sol | 83 ++++++++++++++++++++++++-- test/ERC8183WithAuthorization.t.sol | 36 +++++++++-- 4 files changed, 146 insertions(+), 18 deletions(-) diff --git a/contracts/ERC8183.sol b/contracts/ERC8183.sol index a9c118a..f2322ba 100644 --- a/contracts/ERC8183.sol +++ b/contracts/ERC8183.sol @@ -111,8 +111,9 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable ); /// @notice Emitted when a provider is assigned to a job event ProviderSet( - uint256 indexed jobId, - address indexed provider, + uint256 indexed jobId, + address indexed actor, + address indexed provider, uint256 agentId ); /// @notice Emitted when the payout receiver for a job is set or updated @@ -592,11 +593,21 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable /// @notice Assigns a provider to an Open job that has no provider yet. Client only. /// @param jobId The job to assign a provider to /// @param provider_ The provider address - function setProvider(uint256 jobId, address provider_, uint256 agentId) external whenNotPaused nonReentrant { - _setProvider(msg.sender, jobId, provider_, agentId); + function setProvider(uint256 jobId, address provider_, uint256 agentId, bytes calldata optParams) + external + whenNotPaused + nonReentrant + { + _setProvider(msg.sender, jobId, provider_, agentId, optParams); } - function _setProvider(address actor, uint256 jobId, address provider_, uint256 agentId) internal { + function _setProvider( + address actor, + uint256 jobId, + address provider_, + uint256 agentId, + bytes calldata optParams + ) internal { Job storage job = jobs[jobId]; if (jobId == 0 || jobId > jobCounter) revert InvalidJob(); if (job.status != JobStatus.Open) revert WrongStatus(); @@ -606,9 +617,15 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable if (provider_ == address(0)) revert ZeroAddress(); if (provider_ == job.client) revert ClientCannotBeProvider(); if (provider_ == job.evaluator) revert ProviderCannotBeEvaluator(); + + bytes memory data = abi.encode(actor, provider_, agentId, optParams); + _beforeHook(job.hook, jobId, this.setProvider.selector, data); + job.provider = provider_; job.providerAgentId = agentId; - emit ProviderSet(jobId, provider_, agentId); + emit ProviderSet(jobId, actor, provider_, agentId); + + _afterHook(job.hook, jobId, this.setProvider.selector, data); } /// @notice Provider sets or updates the job budget. Can be called multiple times while Open and not expired. diff --git a/contracts/ERC8183WithAuthorization.sol b/contracts/ERC8183WithAuthorization.sol index 22b6974..680d7ed 100644 --- a/contracts/ERC8183WithAuthorization.sol +++ b/contracts/ERC8183WithAuthorization.sol @@ -14,7 +14,7 @@ contract ERC8183WithAuthorization is ERC8183 { "SetPayoutReceiverAuthorization(address signer,uint256 jobId,address payoutReceiver,uint72 nonce,uint256 deadline)" ); bytes32 public constant SET_PROVIDER_AUTHORIZATION_TYPEHASH = keccak256( - "SetProviderAuthorization(address signer,uint256 jobId,address provider,uint256 agentId,uint72 nonce,uint256 deadline)" + "SetProviderAuthorization(address signer,uint256 jobId,address provider,uint256 agentId,bytes32 optParamsHash,uint72 nonce,uint256 deadline)" ); bytes32 public constant SET_BUDGET_AUTHORIZATION_TYPEHASH = keccak256( "SetBudgetAuthorization(address signer,uint256 jobId,address token,uint256 amount,bytes32 optParamsHash,uint72 nonce,uint256 deadline)" @@ -158,6 +158,7 @@ contract ERC8183WithAuthorization is ERC8183 { uint256 jobId, address provider_, uint256 agentId, + bytes calldata optParams, Authorization calldata auth ) external whenNotPaused nonReentrant { _verifyAuthorization( @@ -165,11 +166,20 @@ contract ERC8183WithAuthorization is ERC8183 { auth.nonce, auth.deadline, keccak256( - abi.encode(SET_PROVIDER_AUTHORIZATION_TYPEHASH, auth.signer, jobId, provider_, agentId, auth.nonce, auth.deadline) + abi.encode( + SET_PROVIDER_AUTHORIZATION_TYPEHASH, + auth.signer, + jobId, + provider_, + agentId, + keccak256(optParams), + auth.nonce, + auth.deadline + ) ), auth.sig ); - _setProvider(auth.signer, jobId, provider_, agentId); + _setProvider(auth.signer, jobId, provider_, agentId, optParams); } function setBudgetWithAuthorization( diff --git a/test/ERC8183.t.sol b/test/ERC8183.t.sol index a2ac18d..0b61203 100644 --- a/test/ERC8183.t.sol +++ b/test/ERC8183.t.sol @@ -46,6 +46,45 @@ contract PendingClaimObserverHook is IERC8183Hook { } } +/// @notice Records before/after hook invocations for assertions. +contract RecordingHook is IERC8183Hook { + bytes4 public lastSelector; + uint256 public beforeCount; + uint256 public afterCount; + bytes public lastData; + + function beforeAction(uint256, bytes4 selector, bytes calldata data) external override { + lastSelector = selector; + beforeCount++; + lastData = data; + } + + function afterAction(uint256, bytes4 selector, bytes calldata data) external override { + lastSelector = selector; + afterCount++; + lastData = data; + } + + function supportsInterface(bytes4 id) external pure override returns (bool) { + return id == type(IERC8183Hook).interfaceId || id == type(IERC165).interfaceId; + } +} + +/// @notice Reverts on every callback — used to prove hooks can gate transitions. +contract RevertingHook is IERC8183Hook { + function beforeAction(uint256, bytes4, bytes calldata) external pure override { + revert("blocked"); + } + + function afterAction(uint256, bytes4, bytes calldata) external pure override { + revert("blocked"); + } + + function supportsInterface(bytes4 id) external pure override returns (bool) { + return id == type(IERC8183Hook).interfaceId || id == type(IERC165).interfaceId; + } +} + /// @notice Image Generation — E2E flow (no hook, core-only payment). /// Mirrors the original Hardhat suite in test/ERC8183.test.js. contract ERC8183Test is Test { @@ -63,7 +102,7 @@ contract ERC8183Test is Test { address evaluator = makeAddr("evaluator"); // Events (must match ERC8183.sol exactly for vm.expectEmit) - event ProviderSet(uint256 indexed jobId, address indexed provider, uint256 agentId); + event ProviderSet(uint256 indexed jobId, address indexed actor, address indexed provider, uint256 agentId); event BudgetSet(uint256 indexed jobId, address indexed token, uint256 amount); event JobFunded(uint256 indexed jobId, address indexed client, uint256 amount); event JobSubmitted(uint256 indexed jobId, address indexed provider, bytes32 deliverable); @@ -243,9 +282,9 @@ contract ERC8183Test is Test { uint256 AGENT_ID_2 = 7; vm.expectEmit(true, true, true, true, address(core)); - emit ProviderSet(2, provider, AGENT_ID_2); + emit ProviderSet(2, client, provider, AGENT_ID_2); vm.prank(client); - core.setProvider(2, provider, AGENT_ID_2); + core.setProvider(2, provider, AGENT_ID_2, ""); assertEq(core.getJob(2).providerAgentId, AGENT_ID_2); @@ -264,7 +303,41 @@ contract ERC8183Test is Test { vm.prank(client); vm.expectRevert(ERC8183.ClientCannotBeProvider.selector); - core.setProvider(1, client, 0); + core.setProvider(1, client, 0, ""); + } + + // setProvider fires before + after hooks with the encoded payload + function test_setProvider_firesBeforeAndAfterHooks() public { + RecordingHook hook = new RecordingHook(); + vm.prank(deployer); + core.setHookWhitelist(address(hook), true); + + vm.prank(client); + core.createJob(address(0), evaluator, _futureExpiry(), "hooked", address(hook), 0); + + vm.prank(client); + core.setProvider(1, provider, 7, hex"beef"); + + assertEq(hook.lastSelector(), core.setProvider.selector); + assertEq(hook.beforeCount(), 1); + assertEq(hook.afterCount(), 1); + assertEq(hook.lastData(), abi.encode(client, provider, uint256(7), bytes(hex"beef"))); + } + + // a reverting before hook gates the transition + function test_setProvider_beforeHookRevertGates() public { + RevertingHook hook = new RevertingHook(); + vm.prank(deployer); + core.setHookWhitelist(address(hook), true); + + vm.prank(client); + core.createJob(address(0), evaluator, _futureExpiry(), "hooked", address(hook), 0); + + vm.prank(client); + vm.expectRevert(bytes("blocked")); + core.setProvider(1, provider, 0, ""); + + assertEq(core.getJob(1).provider, address(0)); } // ────────────────────────────────────────────────────────── @@ -828,7 +901,7 @@ contract ERC8183Test is Test { core.createJob(address(0), evaluator, _futureExpiry(), "late receiver provider", address(0), 0); vm.prank(client); - core.setProvider(1, provider, 0); + core.setProvider(1, provider, 0, ""); vm.expectEmit(true, true, true, true, address(core)); emit PayoutReceiverSet(1, payoutReceiver); diff --git a/test/ERC8183WithAuthorization.t.sol b/test/ERC8183WithAuthorization.t.sol index 2cc7601..ffad7c3 100644 --- a/test/ERC8183WithAuthorization.t.sol +++ b/test/ERC8183WithAuthorization.t.sol @@ -27,7 +27,7 @@ contract ERC8183WithAuthorizationTest is Test { "SetPayoutReceiverAuthorization(address signer,uint256 jobId,address payoutReceiver,uint72 nonce,uint256 deadline)" ); bytes32 constant SET_PROVIDER_AUTHORIZATION_TYPEHASH = keccak256( - "SetProviderAuthorization(address signer,uint256 jobId,address provider,uint256 agentId,uint72 nonce,uint256 deadline)" + "SetProviderAuthorization(address signer,uint256 jobId,address provider,uint256 agentId,bytes32 optParamsHash,uint72 nonce,uint256 deadline)" ); bytes32 constant SET_BUDGET_AUTHORIZATION_TYPEHASH = keccak256( "SetBudgetAuthorization(address signer,uint256 jobId,address token,uint256 amount,bytes32 optParamsHash,uint72 nonce,uint256 deadline)" @@ -243,13 +243,23 @@ contract ERC8183WithAuthorizationTest is Test { uint256 jobId, address provider_, uint256 agentId, + bytes memory optParams, uint72 nonce, uint256 deadline ) internal view returns (bytes memory) { return _sign( signerPk, keccak256( - abi.encode(SET_PROVIDER_AUTHORIZATION_TYPEHASH, signer, jobId, provider_, agentId, nonce, deadline) + abi.encode( + SET_PROVIDER_AUTHORIZATION_TYPEHASH, + signer, + jobId, + provider_, + agentId, + _hashBytes(optParams), + nonce, + deadline + ) ) ); } @@ -695,18 +705,36 @@ contract ERC8183WithAuthorizationTest is Test { uint256 jobId = core.createJob(address(0), evaluator, _futureExpiry(), "late provider", address(0), 0); uint256 deadline = _deadline(); uint256 agentId = 7; - bytes memory sig = _signSetProvider(clientPk, client, jobId, provider, agentId, 61, deadline); + bytes memory sig = _signSetProvider(clientPk, client, jobId, provider, agentId, "", 61, deadline); vm.expectEmit(true, true, true, true, address(core)); emit AuthorizationUsed(client, _packNonce(client, 61)); vm.prank(relayer); - core.setProviderWithAuthorization(jobId, provider, agentId, _auth(client, 61, deadline, sig)); + core.setProviderWithAuthorization(jobId, provider, agentId, "", _auth(client, 61, deadline, sig)); assertEq(core.getJob(jobId).provider, provider); assertEq(core.getJob(jobId).providerAgentId, agentId); assertTrue(core.authorizationNonceUsed(_packNonce(client, 61))); } + function test_setProviderWithAuthorization_bindsOptParams() public { + vm.prank(client); + uint256 jobId = core.createJob(address(0), evaluator, _futureExpiry(), "late provider", address(0), 0); + uint256 deadline = _deadline(); + uint256 agentId = 7; + + // Signature is over optParams = 0x01; relaying with 0x02 must fail. + bytes memory sig = _signSetProvider(clientPk, client, jobId, provider, agentId, hex"01", 62, deadline); + vm.prank(relayer); + vm.expectRevert(ERC8183WithAuthorization.InvalidAuthorizationSignature.selector); + core.setProviderWithAuthorization(jobId, provider, agentId, hex"02", _auth(client, 62, deadline, sig)); + + // Matching optParams succeeds and records the signer as actor. + vm.prank(relayer); + core.setProviderWithAuthorization(jobId, provider, agentId, hex"01", _auth(client, 62, deadline, sig)); + assertEq(core.getJob(jobId).provider, provider); + } + function test_relaysEvaluatorAuthorizedReject() public { uint256 jobId = _createFundedJob(); uint256 deadline = _deadline(); From 4e7c551e06bc529dde134d9fd48c5472327ed2f4 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Mon, 15 Jun 2026 15:28:55 +0800 Subject: [PATCH 02/32] feat: make setPayoutReceiver hookable with optParams and actor in PayoutReceiverSet --- contracts/ERC8183.sol | 24 +++++++-- contracts/ERC8183WithAuthorization.sol | 6 ++- test/ERC8183.t.sol | 73 +++++++++++++++++++------- test/ERC8183WithAuthorization.t.sol | 47 +++++++++++++---- 4 files changed, 115 insertions(+), 35 deletions(-) diff --git a/contracts/ERC8183.sol b/contracts/ERC8183.sol index f2322ba..59ef380 100644 --- a/contracts/ERC8183.sol +++ b/contracts/ERC8183.sol @@ -119,6 +119,7 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable /// @notice Emitted when the payout receiver for a job is set or updated event PayoutReceiverSet( uint256 indexed jobId, + address indexed actor, address indexed payoutReceiver ); /// @notice Emitted when onDisbursement is invoked for a receiver that advertises IDisburser @@ -575,19 +576,34 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable /// Locked once the job is funded. /// @param jobId The job to update /// @param payoutReceiver New payout receiver (address(0) = pay provider directly) - function setPayoutReceiver(uint256 jobId, address payoutReceiver) external whenNotPaused nonReentrant { - _setPayoutReceiver(msg.sender, jobId, payoutReceiver); + function setPayoutReceiver(uint256 jobId, address payoutReceiver, bytes calldata optParams) + external + whenNotPaused + nonReentrant + { + _setPayoutReceiver(msg.sender, jobId, payoutReceiver, optParams); } - function _setPayoutReceiver(address actor, uint256 jobId, address payoutReceiver) internal { + function _setPayoutReceiver( + address actor, + uint256 jobId, + address payoutReceiver, + bytes calldata optParams + ) internal { Job storage job = jobs[jobId]; if (jobId == 0 || jobId > jobCounter) revert InvalidJob(); if (job.status != JobStatus.Open) revert WrongStatus(); if (block.timestamp >= job.expiredAt) revert WrongStatus(); if (actor != job.provider) revert Unauthorized(); _validatePayoutReceiver(payoutReceiver, job.paymentToken); + + bytes memory data = abi.encode(actor, payoutReceiver, optParams); + _beforeHook(job.hook, jobId, this.setPayoutReceiver.selector, data); + job.payoutReceiver = payoutReceiver; - emit PayoutReceiverSet(jobId, payoutReceiver); + emit PayoutReceiverSet(jobId, actor, payoutReceiver); + + _afterHook(job.hook, jobId, this.setPayoutReceiver.selector, data); } /// @notice Assigns a provider to an Open job that has no provider yet. Client only. diff --git a/contracts/ERC8183WithAuthorization.sol b/contracts/ERC8183WithAuthorization.sol index 680d7ed..4841dd8 100644 --- a/contracts/ERC8183WithAuthorization.sol +++ b/contracts/ERC8183WithAuthorization.sol @@ -11,7 +11,7 @@ contract ERC8183WithAuthorization is ERC8183 { "CreateJobAuthorization(address signer,address provider,address evaluator,uint48 expiredAt,bytes32 descriptionHash,address hook,uint256 providerAgentId,uint72 nonce,uint256 deadline)" ); bytes32 public constant SET_PAYOUT_RECEIVER_AUTHORIZATION_TYPEHASH = keccak256( - "SetPayoutReceiverAuthorization(address signer,uint256 jobId,address payoutReceiver,uint72 nonce,uint256 deadline)" + "SetPayoutReceiverAuthorization(address signer,uint256 jobId,address payoutReceiver,bytes32 optParamsHash,uint72 nonce,uint256 deadline)" ); bytes32 public constant SET_PROVIDER_AUTHORIZATION_TYPEHASH = keccak256( "SetProviderAuthorization(address signer,uint256 jobId,address provider,uint256 agentId,bytes32 optParamsHash,uint72 nonce,uint256 deadline)" @@ -133,6 +133,7 @@ contract ERC8183WithAuthorization is ERC8183 { function setPayoutReceiverWithAuthorization( uint256 jobId, address payoutReceiver, + bytes calldata optParams, Authorization calldata auth ) external whenNotPaused nonReentrant { _verifyAuthorization( @@ -145,13 +146,14 @@ contract ERC8183WithAuthorization is ERC8183 { auth.signer, jobId, payoutReceiver, + keccak256(optParams), auth.nonce, auth.deadline ) ), auth.sig ); - _setPayoutReceiver(auth.signer, jobId, payoutReceiver); + _setPayoutReceiver(auth.signer, jobId, payoutReceiver, optParams); } function setProviderWithAuthorization( diff --git a/test/ERC8183.t.sol b/test/ERC8183.t.sol index 0b61203..976f313 100644 --- a/test/ERC8183.t.sol +++ b/test/ERC8183.t.sol @@ -108,7 +108,7 @@ contract ERC8183Test is Test { event JobSubmitted(uint256 indexed jobId, address indexed provider, bytes32 deliverable); event JobCompleted(uint256 indexed jobId, address indexed evaluator, bytes32 reason); event PaymentReleased(uint256 indexed jobId, address indexed recipient, uint256 amount); - event PayoutReceiverSet(uint256 indexed jobId, address indexed payoutReceiver); + event PayoutReceiverSet(uint256 indexed jobId, address indexed actor, address indexed payoutReceiver); event Disbursed(uint256 indexed jobId, address indexed receiver, bytes4 selector, uint256 amount); event PaymentTokenAllowlistUpdated(address indexed token, bool status); event Settled(uint256 indexed jobId, uint256 cumulativeAmount, uint256 delta); @@ -161,7 +161,7 @@ contract ERC8183Test is Test { jobId = core.createJob(provider, evaluator, _futureExpiry(), "claim job", address(0), 0); if (payoutReceiver != address(0)) { vm.prank(provider); - core.setPayoutReceiver(jobId, payoutReceiver); + core.setPayoutReceiver(jobId, payoutReceiver, ""); } vm.prank(provider); core.setBudget(jobId, address(usdc), amount, ""); @@ -794,7 +794,7 @@ contract ERC8183Test is Test { vm.expectRevert(ERC8183.InvalidReceiver.selector); vm.prank(provider); - core.setPayoutReceiver(1, address(core)); + core.setPayoutReceiver(1, address(core), ""); } function test_payoutReceiver_RevertsWhenReceiverIsPaymentTokenAfterBudget() public { @@ -806,7 +806,7 @@ contract ERC8183Test is Test { vm.expectRevert(ERC8183.InvalidReceiver.selector); vm.prank(provider); - core.setPayoutReceiver(jobId, address(usdc)); + core.setPayoutReceiver(jobId, address(usdc), ""); } function test_setBudget_RevertsWhenExistingReceiverIsPaymentToken() public { @@ -814,7 +814,7 @@ contract ERC8183Test is Test { uint256 jobId = core.createJob(provider, evaluator, _futureExpiry(), "budget token receiver", address(0), 0); vm.prank(provider); - core.setPayoutReceiver(jobId, address(usdc)); + core.setPayoutReceiver(jobId, address(usdc), ""); vm.expectRevert(ERC8183.InvalidReceiver.selector); vm.prank(provider); @@ -829,7 +829,7 @@ contract ERC8183Test is Test { uint256 jobId = core.createJob(provider, evaluator, expiry, "refund receiver", address(0), 0); vm.prank(provider); - core.setPayoutReceiver(jobId, payoutReceiver); + core.setPayoutReceiver(jobId, payoutReceiver, ""); vm.prank(provider); core.setBudget(jobId, address(usdc), TWENTY_USDC, ""); vm.prank(client); @@ -856,27 +856,27 @@ contract ERC8183Test is Test { vm.expectRevert(ERC8183.Unauthorized.selector); vm.prank(client); - core.setPayoutReceiver(jobId, address(disburser)); + core.setPayoutReceiver(jobId, address(disburser), ""); vm.expectEmit(true, true, true, true, address(core)); - emit PayoutReceiverSet(jobId, address(plainReceiver)); + emit PayoutReceiverSet(jobId, provider, address(plainReceiver)); vm.prank(provider); - core.setPayoutReceiver(jobId, address(plainReceiver)); + core.setPayoutReceiver(jobId, address(plainReceiver), ""); vm.expectEmit(true, true, true, true, address(core)); - emit PayoutReceiverSet(jobId, address(disburser)); + emit PayoutReceiverSet(jobId, provider, address(disburser)); vm.prank(provider); - core.setPayoutReceiver(jobId, address(disburser)); + core.setPayoutReceiver(jobId, address(disburser), ""); assertEq(core.getJob(jobId).payoutReceiver, address(disburser)); vm.expectEmit(true, true, true, true, address(core)); - emit PayoutReceiverSet(jobId, address(0)); + emit PayoutReceiverSet(jobId, provider, address(0)); vm.prank(provider); - core.setPayoutReceiver(jobId, address(0)); + core.setPayoutReceiver(jobId, address(0), ""); assertEq(core.getJob(jobId).payoutReceiver, address(0)); vm.prank(provider); - core.setPayoutReceiver(jobId, address(disburser)); + core.setPayoutReceiver(jobId, address(disburser), ""); vm.prank(provider); core.setBudget(jobId, address(usdc), TWENTY_USDC, ""); @@ -885,13 +885,13 @@ contract ERC8183Test is Test { vm.expectRevert(ERC8183.WrongStatus.selector); vm.prank(provider); - core.setPayoutReceiver(jobId, address(0)); + core.setPayoutReceiver(jobId, address(0), ""); } function test_setPayoutReceiver_RevertsForInvalidJob() public { vm.expectRevert(ERC8183.InvalidJob.selector); vm.prank(provider); - core.setPayoutReceiver(1, address(0)); + core.setPayoutReceiver(1, address(0), ""); } function test_setPayoutReceiver_AssignedProviderCanSetReceiver() public { @@ -904,9 +904,9 @@ contract ERC8183Test is Test { core.setProvider(1, provider, 0, ""); vm.expectEmit(true, true, true, true, address(core)); - emit PayoutReceiverSet(1, payoutReceiver); + emit PayoutReceiverSet(1, provider, payoutReceiver); vm.prank(provider); - core.setPayoutReceiver(1, payoutReceiver); + core.setPayoutReceiver(1, payoutReceiver, ""); assertEq(core.getJob(1).payoutReceiver, payoutReceiver); } @@ -920,7 +920,42 @@ contract ERC8183Test is Test { vm.warp(uint256(expiry)); vm.expectRevert(ERC8183.WrongStatus.selector); vm.prank(provider); - core.setPayoutReceiver(1, makeAddr("lateReceiver")); + core.setPayoutReceiver(1, makeAddr("lateReceiver"), ""); + } + + // setPayoutReceiver fires before + after hooks with the encoded payload + function test_setPayoutReceiver_firesBeforeAndAfterHooks() public { + RecordingHook hook = new RecordingHook(); + vm.prank(deployer); + core.setHookWhitelist(address(hook), true); + + vm.prank(client); + core.createJob(provider, evaluator, _futureExpiry(), "hooked", address(hook), 0); + + address receiver = makeAddr("hookReceiver"); + vm.prank(provider); + core.setPayoutReceiver(1, receiver, hex"cafe"); + + assertEq(hook.lastSelector(), core.setPayoutReceiver.selector); + assertEq(hook.beforeCount(), 1); + assertEq(hook.afterCount(), 1); + assertEq(hook.lastData(), abi.encode(provider, receiver, bytes(hex"cafe"))); + } + + // a reverting before hook gates the transition + function test_setPayoutReceiver_beforeHookRevertGates() public { + RevertingHook hook = new RevertingHook(); + vm.prank(deployer); + core.setHookWhitelist(address(hook), true); + + vm.prank(client); + core.createJob(provider, evaluator, _futureExpiry(), "hooked", address(hook), 0); + + vm.prank(provider); + vm.expectRevert(bytes("blocked")); + core.setPayoutReceiver(1, makeAddr("hookReceiver"), ""); + + assertEq(core.getJob(1).payoutReceiver, address(0)); } function test_complete_RevertsWhenReceiverDisburserRevertsStrictly() public { diff --git a/test/ERC8183WithAuthorization.t.sol b/test/ERC8183WithAuthorization.t.sol index ffad7c3..4ba9469 100644 --- a/test/ERC8183WithAuthorization.t.sol +++ b/test/ERC8183WithAuthorization.t.sol @@ -24,7 +24,7 @@ contract ERC8183WithAuthorizationTest is Test { "CreateJobAuthorization(address signer,address provider,address evaluator,uint48 expiredAt,bytes32 descriptionHash,address hook,uint256 providerAgentId,uint72 nonce,uint256 deadline)" ); bytes32 constant SET_PAYOUT_RECEIVER_AUTHORIZATION_TYPEHASH = keccak256( - "SetPayoutReceiverAuthorization(address signer,uint256 jobId,address payoutReceiver,uint72 nonce,uint256 deadline)" + "SetPayoutReceiverAuthorization(address signer,uint256 jobId,address payoutReceiver,bytes32 optParamsHash,uint72 nonce,uint256 deadline)" ); bytes32 constant SET_PROVIDER_AUTHORIZATION_TYPEHASH = keccak256( "SetProviderAuthorization(address signer,uint256 jobId,address provider,uint256 agentId,bytes32 optParamsHash,uint72 nonce,uint256 deadline)" @@ -71,7 +71,7 @@ contract ERC8183WithAuthorizationTest is Test { event AuthorizationUsed(address indexed signer, bytes32 indexed nonce); event AuthorizationCanceled(address indexed signer, bytes32 indexed nonce); - event PayoutReceiverSet(uint256 indexed jobId, address indexed payoutReceiver); + event PayoutReceiverSet(uint256 indexed jobId, address indexed actor, address indexed payoutReceiver); event ClaimSubmitted( uint256 indexed jobId, address indexed provider, @@ -226,13 +226,22 @@ contract ERC8183WithAuthorizationTest is Test { address signer, uint256 jobId, address payoutReceiver, + bytes memory optParams, uint72 nonce, uint256 deadline ) internal view returns (bytes memory) { return _sign( signerPk, keccak256( - abi.encode(SET_PAYOUT_RECEIVER_AUTHORIZATION_TYPEHASH, signer, jobId, payoutReceiver, nonce, deadline) + abi.encode( + SET_PAYOUT_RECEIVER_AUTHORIZATION_TYPEHASH, + signer, + jobId, + payoutReceiver, + _hashBytes(optParams), + nonce, + deadline + ) ) ); } @@ -676,28 +685,46 @@ contract ERC8183WithAuthorizationTest is Test { assertEq(core.SET_PAYOUT_RECEIVER_AUTHORIZATION_TYPEHASH(), SET_PAYOUT_RECEIVER_AUTHORIZATION_TYPEHASH); - bytes memory clientSig = _signSetPayoutReceiver(clientPk, client, jobId, plainReceiver, 2, deadline); + bytes memory clientSig = _signSetPayoutReceiver(clientPk, client, jobId, plainReceiver, "", 2, deadline); vm.expectRevert(ERC8183.Unauthorized.selector); vm.prank(relayer); - core.setPayoutReceiverWithAuthorization(jobId, plainReceiver, _auth(client, 2, deadline, clientSig)); + core.setPayoutReceiverWithAuthorization(jobId, plainReceiver, "", _auth(client, 2, deadline, clientSig)); - bytes memory providerSig = _signSetPayoutReceiver(providerPk, provider, jobId, plainReceiver, 3, deadline); + bytes memory providerSig = _signSetPayoutReceiver(providerPk, provider, jobId, plainReceiver, "", 3, deadline); vm.expectEmit(true, true, true, true, address(core)); emit AuthorizationUsed(provider, _packNonce(provider, 3)); vm.expectEmit(true, true, true, true, address(core)); - emit PayoutReceiverSet(jobId, plainReceiver); + emit PayoutReceiverSet(jobId, provider, plainReceiver); vm.prank(relayer); - core.setPayoutReceiverWithAuthorization(jobId, plainReceiver, _auth(provider, 3, deadline, providerSig)); + core.setPayoutReceiverWithAuthorization(jobId, plainReceiver, "", _auth(provider, 3, deadline, providerSig)); assertEq(core.getJob(jobId).payoutReceiver, plainReceiver); _relaySetBudget(jobId, 4, deadline); _relayFund(jobId, 5, deadline); - bytes memory lockedSig = _signSetPayoutReceiver(providerPk, provider, jobId, secondReceiver, 6, deadline); + bytes memory lockedSig = _signSetPayoutReceiver(providerPk, provider, jobId, secondReceiver, "", 6, deadline); vm.expectRevert(ERC8183.WrongStatus.selector); vm.prank(relayer); - core.setPayoutReceiverWithAuthorization(jobId, secondReceiver, _auth(provider, 6, deadline, lockedSig)); + core.setPayoutReceiverWithAuthorization(jobId, secondReceiver, "", _auth(provider, 6, deadline, lockedSig)); + } + + function test_setPayoutReceiverWithAuthorization_bindsOptParams() public { + uint48 expiry = _futureExpiry(); + uint256 deadline = _deadline(); + uint256 jobId = _relayCreateJob(client, clientPk, provider, evaluator, expiry, "receiver optparams", 1, deadline); + address receiver = makeAddr("optParamsReceiver"); + + // Signature over optParams = 0x01; relaying with 0x02 must fail. + bytes memory sig = _signSetPayoutReceiver(providerPk, provider, jobId, receiver, hex"01", 2, deadline); + vm.prank(relayer); + vm.expectRevert(ERC8183WithAuthorization.InvalidAuthorizationSignature.selector); + core.setPayoutReceiverWithAuthorization(jobId, receiver, hex"02", _auth(provider, 2, deadline, sig)); + + // Matching optParams succeeds. + vm.prank(relayer); + core.setPayoutReceiverWithAuthorization(jobId, receiver, hex"01", _auth(provider, 2, deadline, sig)); + assertEq(core.getJob(jobId).payoutReceiver, receiver); } function test_relaysClientAuthorizedSetProvider() public { From 0310b99d96eb393f9723a7508c6d75e1b5435561 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Mon, 15 Jun 2026 15:32:28 +0800 Subject: [PATCH 03/32] feat: make claimRefund hookable with optParams Fully hookable; before-hook can revert. Whitelist + ERC-165 + batchDetachHook mitigate the permissionless-refund fund-lock risk. Updated reentrancy mock to the new claimRefund signature. --- contracts/ERC8183.sol | 16 +++++- contracts/mocks/MockDisburser.sol | 4 +- test/ERC8183.t.sol | 89 +++++++++++++++++++++++++++---- 3 files changed, 96 insertions(+), 13 deletions(-) diff --git a/contracts/ERC8183.sol b/contracts/ERC8183.sol index 59ef380..710f1e3 100644 --- a/contracts/ERC8183.sol +++ b/contracts/ERC8183.sol @@ -889,9 +889,16 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable /// @notice Claims a refund for an expired job. Anyone can call. /// Transitions Open/Funded/Submitted -> Expired after expiry time. - /// Not hookable; pending claims must still be resolved before refund. + /// Pending claims must still be resolved before refund. + /// @dev Hookable: the attached hook's before/after callbacks fire around the + /// refund and MAY revert. Because this is the permissionless escrow + /// recovery path, a misbehaving hook that reverts can block the refund + /// and lock escrowed funds. Mitigations: hooks are admin-whitelisted and + /// ERC-165-checked at attach time, and an admin can sever a bad hook from + /// in-flight jobs via batchDetachHook (after which claimRefund succeeds). /// @param jobId The expired job to claim refund for - function claimRefund(uint256 jobId) external whenNotPaused nonReentrant { + /// @param optParams Hook-specific parameters (passed to before/after hooks) + function claimRefund(uint256 jobId, bytes calldata optParams) external whenNotPaused nonReentrant { Job storage job = jobs[jobId]; if (jobId == 0 || jobId > jobCounter) revert InvalidJob(); if (job.status != JobStatus.Open && job.status != JobStatus.Funded && job.status != JobStatus.Submitted) @@ -905,6 +912,9 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable if (block.timestamp < job.expiredAt) revert WrongStatus(); } + bytes memory data = abi.encode(msg.sender, optParams); + _beforeHook(job.hook, jobId, this.claimRefund.selector, data); + JobStatus prev = job.status; job.status = JobStatus.Expired; @@ -915,6 +925,8 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable } emit JobExpired(jobId); + + _afterHook(job.hook, jobId, this.claimRefund.selector, data); } function _distributeSettlement( diff --git a/contracts/mocks/MockDisburser.sol b/contracts/mocks/MockDisburser.sol index d9b22f0..e1782e4 100644 --- a/contracts/mocks/MockDisburser.sol +++ b/contracts/mocks/MockDisburser.sol @@ -8,7 +8,7 @@ interface IReentrantERC8183 { function settleClaim(uint256 jobId, uint256 cumulativeAmount, bytes32 deliverable, bytes calldata optParams) external; function complete(uint256 jobId, bytes32 reason, bytes calldata optParams) external; - function claimRefund(uint256 jobId) external; + function claimRefund(uint256 jobId, bytes calldata optParams) external; } contract MockDisburser is IDisburser, ERC165 { @@ -104,7 +104,7 @@ contract ReentrantDisburser is IDisburser, ERC165 { ); } if (action == Action.ClaimRefund) { - return address(core).call(abi.encodeCall(IReentrantERC8183.claimRefund, (targetJobId))); + return address(core).call(abi.encodeCall(IReentrantERC8183.claimRefund, (targetJobId, bytes("")))); } return (true, ""); } diff --git a/test/ERC8183.t.sol b/test/ERC8183.t.sol index 976f313..8aa5253 100644 --- a/test/ERC8183.t.sol +++ b/test/ERC8183.t.sol @@ -85,6 +85,19 @@ contract RevertingHook is IERC8183Hook { } } +/// @notice Reverts only on claimRefund — lets a job be funded but blocks the refund path. +contract ClaimRefundBlockingHook is IERC8183Hook { + function beforeAction(uint256, bytes4 selector, bytes calldata) external pure override { + if (selector == ERC8183.claimRefund.selector) revert("blocked"); + } + + function afterAction(uint256, bytes4, bytes calldata) external pure override {} + + function supportsInterface(bytes4 id) external pure override returns (bool) { + return id == type(IERC8183Hook).interfaceId || id == type(IERC165).interfaceId; + } +} + /// @notice Image Generation — E2E flow (no hook, core-only payment). /// Mirrors the original Hardhat suite in test/ERC8183.test.js. contract ERC8183Test is Test { @@ -430,7 +443,7 @@ contract ERC8183Test is Test { // Move past expiry but within grace period vm.warp(uint256(expiry) + 1); vm.expectRevert(ERC8183.GracePeriodActive.selector); - core.claimRefund(jobId); + core.claimRefund(jobId, ""); // Evaluator can still complete during grace period vm.prank(evaluator); @@ -458,7 +471,7 @@ contract ERC8183Test is Test { // Move past expiry + grace period (1 hour) vm.warp(uint256(expiry) + 3601); - core.claimRefund(jobId); + core.claimRefund(jobId, ""); assertEq(uint8(core.getJob(jobId).status), uint8(ERC8183.JobStatus.Expired)); assertEq(usdc.balanceOf(client), TWENTY_USDC); } @@ -585,7 +598,7 @@ contract ERC8183Test is Test { // NOT submitted - stays Funded vm.warp(uint256(expiry) + 1); - core.claimRefund(jobId); + core.claimRefund(jobId, ""); assertEq(uint8(core.getJob(jobId).status), uint8(ERC8183.JobStatus.Expired)); assertEq(usdc.balanceOf(client), TWENTY_USDC); } @@ -836,7 +849,7 @@ contract ERC8183Test is Test { core.fund(jobId, address(usdc), TWENTY_USDC, ""); vm.warp(uint256(expiry) + 1); - core.claimRefund(jobId); + core.claimRefund(jobId, ""); assertEq(uint8(core.getJob(jobId).status), uint8(ERC8183.JobStatus.Expired)); assertEq(usdc.balanceOf(client), TWENTY_USDC); @@ -1218,12 +1231,12 @@ contract ERC8183Test is Test { vm.warp(uint256(expiry) + 1); vm.expectRevert(ERC8183.PendingClaimExists.selector); - core.claimRefund(jobId); + core.claimRefund(jobId, ""); vm.prank(client); core.rejectClaim(jobId, TEN_USDC, deliverable, staleReason, ""); - core.claimRefund(jobId); + core.claimRefund(jobId, ""); assertEq(uint8(core.getJob(jobId).status), uint8(ERC8183.JobStatus.Expired)); assertEq(core.pendingClaimHash(jobId), bytes32(0)); @@ -1394,7 +1407,7 @@ contract ERC8183Test is Test { vm.warp(uint256(expiry) + 30 days); vm.expectRevert(ERC8183.PendingClaimExists.selector); - core.claimRefund(jobId); + core.claimRefund(jobId, ""); vm.prank(evaluator); core.approveClaim(jobId, TEN_USDC, deliverable, ""); @@ -1420,16 +1433,74 @@ contract ERC8183Test is Test { vm.warp(uint256(expiry) + 30 days); vm.expectRevert(ERC8183.PendingClaimExists.selector); - core.claimRefund(jobId); + core.claimRefund(jobId, ""); vm.prank(provider); core.rejectClaim(jobId, TEN_USDC, deliverable, bytes32("withdrawn"), ""); - core.claimRefund(jobId); + core.claimRefund(jobId, ""); assertEq(uint8(core.getJob(jobId).status), uint8(ERC8183.JobStatus.Expired)); assertEq(core.pendingClaimHash(jobId), bytes32(0)); assertEq(usdc.balanceOf(client), TWENTY_USDC); assertEq(usdc.balanceOf(address(core)), 0); } + + // claimRefund fires before + after hooks on the refund path + function test_claimRefund_firesBeforeAndAfterHooks() public { + RecordingHook hook = new RecordingHook(); + vm.prank(deployer); + core.setHookWhitelist(address(hook), true); + + vm.prank(client); + uint256 jobId = core.createJob(provider, evaluator, _futureExpiry(), "hooked", address(hook), 0); + vm.prank(provider); + core.setBudget(jobId, address(usdc), TEN_USDC, ""); + vm.prank(client); + core.fund(jobId, address(usdc), TEN_USDC, ""); + + vm.warp(block.timestamp + 3601); + // Hook already saw setBudget + fund; assert the claimRefund delta specifically. + uint256 beforeBefore = hook.beforeCount(); + uint256 afterBefore = hook.afterCount(); + core.claimRefund(jobId, hex"f00d"); + + assertEq(hook.lastSelector(), core.claimRefund.selector); + assertEq(hook.beforeCount(), beforeBefore + 1); + assertEq(hook.afterCount(), afterBefore + 1); + assertEq(hook.lastData(), abi.encode(address(this), bytes(hex"f00d"))); + assertEq(usdc.balanceOf(client), TWENTY_USDC); + } + + // a reverting hook blocks the refund (accepted fund-lock risk); admin detach recovers it + function test_claimRefund_revertingHookLocksFundsThenDetachRecovers() public { + ClaimRefundBlockingHook hook = new ClaimRefundBlockingHook(); + vm.prank(deployer); + core.setHookWhitelist(address(hook), true); + + vm.prank(client); + uint256 jobId = core.createJob(provider, evaluator, _futureExpiry(), "hooked", address(hook), 0); + vm.prank(provider); + core.setBudget(jobId, address(usdc), TEN_USDC, ""); + vm.prank(client); + core.fund(jobId, address(usdc), TEN_USDC, ""); + + vm.warp(block.timestamp + 3601); + + // Hook blocks the refund: funds are locked in escrow. + vm.expectRevert(bytes("blocked")); + core.claimRefund(jobId, ""); + assertEq(usdc.balanceOf(address(core)), TEN_USDC); + + // Admin severs the bad hook; refund now succeeds. + uint256[] memory ids = new uint256[](1); + ids[0] = jobId; + vm.prank(deployer); + core.batchDetachHook(ids); + + uint256 balBefore = usdc.balanceOf(client); + core.claimRefund(jobId, ""); + assertEq(usdc.balanceOf(client), balBefore + TEN_USDC); + assertEq(uint8(core.getJob(jobId).status), uint8(ERC8183.JobStatus.Expired)); + } } From 48b9cc3033a5f8bb6f5abbf1a186cdd8817ef4f0 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Mon, 15 Jun 2026 15:37:55 +0800 Subject: [PATCH 04/32] feat: fire afterAction hook on createJob with optParams createJob is afterAction-only (no beforeAction pre-attachment). Adds optParams to createJob + createJobWithAuthorization with optParamsHash in the EIP-712 typehash. Adds SelectiveRevertHook test helper now that createJob fires hooks. --- contracts/ERC8183.sol | 20 +++- contracts/ERC8183WithAuthorization.sol | 7 +- test/ERC8183.t.sol | 139 +++++++++++++++++-------- test/ERC8183WithAuthorization.t.sol | 66 +++++++++++- 4 files changed, 177 insertions(+), 55 deletions(-) diff --git a/contracts/ERC8183.sol b/contracts/ERC8183.sol index 710f1e3..fffe9f5 100644 --- a/contracts/ERC8183.sol +++ b/contracts/ERC8183.sol @@ -515,9 +515,10 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable uint48 expiredAt, string calldata description, address hook, - uint256 providerAgentId + uint256 providerAgentId, + bytes calldata optParams ) external whenNotPaused nonReentrant returns (uint256) { - return _createJob(msg.sender, provider, evaluator, expiredAt, description, hook, providerAgentId); + return _createJob(msg.sender, provider, evaluator, expiredAt, description, hook, providerAgentId, optParams); } function _createJob( @@ -527,7 +528,8 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable uint48 expiredAt, string calldata description, address hook, - uint256 providerAgentId + uint256 providerAgentId, + bytes calldata optParams ) internal returns (uint256) { if (client == address(0)) revert ZeroAddress(); if (expiredAt <= block.timestamp + 5 minutes) revert ExpiryTooShort(); @@ -569,6 +571,18 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable expiredAt, hook ); + + // afterAction only: the hook is now attached and the job exists, so it can + // initialize per-job bookkeeping or revert to reject a job it will not service. + // No beforeAction — there is no attached hook before creation, and attachment + // is already gated by the whitelist + ERC-165 check above. + _afterHook( + hook, + jobId, + this.createJob.selector, + abi.encode(client, provider, evaluator, expiredAt, hook, providerAgentId, optParams) + ); + return jobId; } diff --git a/contracts/ERC8183WithAuthorization.sol b/contracts/ERC8183WithAuthorization.sol index 4841dd8..cc2fca4 100644 --- a/contracts/ERC8183WithAuthorization.sol +++ b/contracts/ERC8183WithAuthorization.sol @@ -8,7 +8,7 @@ import "./ERC8183.sol"; /// @notice Adds EIP-712 signed authorization entrypoints to ERC8183. contract ERC8183WithAuthorization is ERC8183 { bytes32 public constant CREATE_JOB_AUTHORIZATION_TYPEHASH = keccak256( - "CreateJobAuthorization(address signer,address provider,address evaluator,uint48 expiredAt,bytes32 descriptionHash,address hook,uint256 providerAgentId,uint72 nonce,uint256 deadline)" + "CreateJobAuthorization(address signer,address provider,address evaluator,uint48 expiredAt,bytes32 descriptionHash,address hook,uint256 providerAgentId,bytes32 optParamsHash,uint72 nonce,uint256 deadline)" ); bytes32 public constant SET_PAYOUT_RECEIVER_AUTHORIZATION_TYPEHASH = keccak256( "SetPayoutReceiverAuthorization(address signer,uint256 jobId,address payoutReceiver,bytes32 optParamsHash,uint72 nonce,uint256 deadline)" @@ -61,6 +61,7 @@ contract ERC8183WithAuthorization is ERC8183 { string description; address hook; uint256 providerAgentId; + bytes optParams; } event AuthorizationUsed(address indexed signer, bytes32 indexed nonce); @@ -113,6 +114,7 @@ contract ERC8183WithAuthorization is ERC8183 { keccak256(bytes(params.description)), params.hook, params.providerAgentId, + keccak256(params.optParams), auth.nonce, auth.deadline ) @@ -126,7 +128,8 @@ contract ERC8183WithAuthorization is ERC8183 { params.expiredAt, params.description, params.hook, - params.providerAgentId + params.providerAgentId, + params.optParams ); } diff --git a/test/ERC8183.t.sol b/test/ERC8183.t.sol index 8aa5253..bba83e5 100644 --- a/test/ERC8183.t.sol +++ b/test/ERC8183.t.sol @@ -85,13 +85,22 @@ contract RevertingHook is IERC8183Hook { } } -/// @notice Reverts only on claimRefund — lets a job be funded but blocks the refund path. -contract ClaimRefundBlockingHook is IERC8183Hook { - function beforeAction(uint256, bytes4 selector, bytes calldata) external pure override { - if (selector == ERC8183.claimRefund.selector) revert("blocked"); +/// @notice Reverts only on a single configured selector — lets other lifecycle calls +/// (e.g. createJob/setBudget/fund) proceed while gating one specific action. +contract SelectiveRevertHook is IERC8183Hook { + bytes4 public immutable blockedSelector; + + constructor(bytes4 blockedSelector_) { + blockedSelector = blockedSelector_; } - function afterAction(uint256, bytes4, bytes calldata) external pure override {} + function beforeAction(uint256, bytes4 selector, bytes calldata) external view override { + if (selector == blockedSelector) revert("blocked"); + } + + function afterAction(uint256, bytes4 selector, bytes calldata) external view override { + if (selector == blockedSelector) revert("blocked"); + } function supportsInterface(bytes4 id) external pure override returns (bool) { return id == type(IERC8183Hook).interfaceId || id == type(IERC165).interfaceId; @@ -171,7 +180,7 @@ contract ERC8183Test is Test { function _createFundedJob(uint256 amount, address payoutReceiver) internal returns (uint256 jobId) { vm.prank(client); - jobId = core.createJob(provider, evaluator, _futureExpiry(), "claim job", address(0), 0); + jobId = core.createJob(provider, evaluator, _futureExpiry(), "claim job", address(0), 0, ""); if (payoutReceiver != address(0)) { vm.prank(provider); core.setPayoutReceiver(jobId, payoutReceiver, ""); @@ -236,7 +245,7 @@ contract ERC8183Test is Test { // Job 1: paid in USDC vm.prank(client); - core.createJob(provider, evaluator, expiry, "Job paid in USDC", address(0), 0); + core.createJob(provider, evaluator, expiry, "Job paid in USDC", address(0), 0, ""); uint256 jobId1 = 1; vm.prank(provider); core.setBudget(jobId1, address(usdc), TWENTY_USDC, ""); @@ -244,7 +253,7 @@ contract ERC8183Test is Test { // Job 2: paid in cbBTC vm.prank(client); - core.createJob(provider, evaluator, expiry, "Job paid in cbBTC", address(0), 0); + core.createJob(provider, evaluator, expiry, "Job paid in cbBTC", address(0), 0, ""); uint256 jobId2 = 2; vm.prank(provider); core.setBudget(jobId2, address(cbbtc), ONE_CBBTC, ""); @@ -285,12 +294,12 @@ contract ERC8183Test is Test { // createJob with agentId when provider is known vm.prank(client); - core.createJob(provider, evaluator, expiry, "Job with agentId", address(0), AGENT_ID); + core.createJob(provider, evaluator, expiry, "Job with agentId", address(0), AGENT_ID, ""); assertEq(core.getJob(1).providerAgentId, AGENT_ID); // createJob without provider: agentId should be 0 even if a non-zero value is passed vm.prank(client); - core.createJob(address(0), evaluator, expiry, "Job without provider", address(0), 99); + core.createJob(address(0), evaluator, expiry, "Job without provider", address(0), 99, ""); assertEq(core.getJob(2).providerAgentId, 0); uint256 AGENT_ID_2 = 7; @@ -303,7 +312,7 @@ contract ERC8183Test is Test { // agentId = 0 is valid vm.prank(client); - core.createJob(provider, evaluator, expiry, "No agentId", address(0), 0); + core.createJob(provider, evaluator, expiry, "No agentId", address(0), 0, ""); assertEq(core.getJob(3).providerAgentId, 0); } @@ -312,7 +321,7 @@ contract ERC8183Test is Test { // ────────────────────────────────────────────────────────── function test_setProvider_RevertsWhenProviderIsClient() public { vm.prank(client); - core.createJob(address(0), evaluator, _futureExpiry(), "Job without provider", address(0), 0); + core.createJob(address(0), evaluator, _futureExpiry(), "Job without provider", address(0), 0, ""); vm.prank(client); vm.expectRevert(ERC8183.ClientCannotBeProvider.selector); @@ -326,25 +335,28 @@ contract ERC8183Test is Test { core.setHookWhitelist(address(hook), true); vm.prank(client); - core.createJob(address(0), evaluator, _futureExpiry(), "hooked", address(hook), 0); + core.createJob(address(0), evaluator, _futureExpiry(), "hooked", address(hook), 0, ""); + // createJob already fired afterAction; assert the setProvider delta specifically. + uint256 beforeBefore = hook.beforeCount(); + uint256 afterBefore = hook.afterCount(); vm.prank(client); core.setProvider(1, provider, 7, hex"beef"); assertEq(hook.lastSelector(), core.setProvider.selector); - assertEq(hook.beforeCount(), 1); - assertEq(hook.afterCount(), 1); + assertEq(hook.beforeCount(), beforeBefore + 1); + assertEq(hook.afterCount(), afterBefore + 1); assertEq(hook.lastData(), abi.encode(client, provider, uint256(7), bytes(hex"beef"))); } // a reverting before hook gates the transition function test_setProvider_beforeHookRevertGates() public { - RevertingHook hook = new RevertingHook(); + SelectiveRevertHook hook = new SelectiveRevertHook(ERC8183.setProvider.selector); vm.prank(deployer); core.setHookWhitelist(address(hook), true); vm.prank(client); - core.createJob(address(0), evaluator, _futureExpiry(), "hooked", address(hook), 0); + core.createJob(address(0), evaluator, _futureExpiry(), "hooked", address(hook), 0, ""); vm.prank(client); vm.expectRevert(bytes("blocked")); @@ -361,7 +373,7 @@ contract ERC8183Test is Test { // Step 1: create job vm.prank(client); - core.createJob(provider, evaluator, expiry, "Generate a beautiful landscape wallpaper image", address(0), 0); + core.createJob(provider, evaluator, expiry, "Generate a beautiful landscape wallpaper image", address(0), 0, ""); uint256 jobId = 1; ERC8183.Job memory job = core.getJob(jobId); @@ -427,7 +439,7 @@ contract ERC8183Test is Test { uint48 expiry = _futureExpiry(); vm.prank(client); - core.createJob(provider, evaluator, expiry, "grace period test", address(0), 0); + core.createJob(provider, evaluator, expiry, "grace period test", address(0), 0, ""); uint256 jobId = 1; vm.prank(provider); @@ -459,7 +471,7 @@ contract ERC8183Test is Test { uint48 expiry = _futureExpiry(); vm.prank(client); - core.createJob(provider, evaluator, expiry, "grace expiry test", address(0), 0); + core.createJob(provider, evaluator, expiry, "grace expiry test", address(0), 0, ""); uint256 jobId = 1; vm.prank(provider); @@ -484,7 +496,7 @@ contract ERC8183Test is Test { uint48 expiry = _futureExpiry(); vm.prank(client); - core.createJob(provider, evaluator, expiry, "test", address(0), 0); + core.createJob(provider, evaluator, expiry, "test", address(0), 0, ""); uint256 jobId = 1; vm.expectRevert(ERC8183.PaymentTokenNotAllowed.selector); @@ -540,7 +552,7 @@ contract ERC8183Test is Test { uint48 expiry = _futureExpiry(); vm.prank(client); - core.createJob(provider, evaluator, expiry, "fot", address(0), 0); + core.createJob(provider, evaluator, expiry, "fot", address(0), 0, ""); uint256 jobId = 1; vm.prank(provider); @@ -565,7 +577,7 @@ contract ERC8183Test is Test { cbbtc.approve(address(core), TWENTY_USDC); vm.prank(client); - uint256 jobId = core.createJob(provider, evaluator, _futureExpiry(), "token swap", address(0), 0); + uint256 jobId = core.createJob(provider, evaluator, _futureExpiry(), "token swap", address(0), 0, ""); vm.prank(provider); core.setBudget(jobId, address(usdc), TWENTY_USDC, ""); @@ -588,7 +600,7 @@ contract ERC8183Test is Test { uint48 expiry = _futureExpiry(); vm.prank(client); - core.createJob(provider, evaluator, expiry, "no grace test", address(0), 0); + core.createJob(provider, evaluator, expiry, "no grace test", address(0), 0, ""); uint256 jobId = 1; vm.prank(provider); @@ -803,7 +815,7 @@ contract ERC8183Test is Test { function test_payoutReceiver_RevertsWhenReceiverIsEscrow() public { vm.prank(client); - core.createJob(provider, evaluator, _futureExpiry(), "setter self receiver", address(0), 0); + core.createJob(provider, evaluator, _futureExpiry(), "setter self receiver", address(0), 0, ""); vm.expectRevert(ERC8183.InvalidReceiver.selector); vm.prank(provider); @@ -812,7 +824,7 @@ contract ERC8183Test is Test { function test_payoutReceiver_RevertsWhenReceiverIsPaymentTokenAfterBudget() public { vm.prank(client); - uint256 jobId = core.createJob(provider, evaluator, _futureExpiry(), "receiver token", address(0), 0); + uint256 jobId = core.createJob(provider, evaluator, _futureExpiry(), "receiver token", address(0), 0, ""); vm.prank(provider); core.setBudget(jobId, address(usdc), TWENTY_USDC, ""); @@ -824,7 +836,7 @@ contract ERC8183Test is Test { function test_setBudget_RevertsWhenExistingReceiverIsPaymentToken() public { vm.prank(client); - uint256 jobId = core.createJob(provider, evaluator, _futureExpiry(), "budget token receiver", address(0), 0); + uint256 jobId = core.createJob(provider, evaluator, _futureExpiry(), "budget token receiver", address(0), 0, ""); vm.prank(provider); core.setPayoutReceiver(jobId, address(usdc), ""); @@ -839,7 +851,7 @@ contract ERC8183Test is Test { uint48 expiry = _futureExpiry(); vm.prank(client); - uint256 jobId = core.createJob(provider, evaluator, expiry, "refund receiver", address(0), 0); + uint256 jobId = core.createJob(provider, evaluator, expiry, "refund receiver", address(0), 0, ""); vm.prank(provider); core.setPayoutReceiver(jobId, payoutReceiver, ""); @@ -864,7 +876,7 @@ contract ERC8183Test is Test { uint48 expiry = _futureExpiry(); vm.prank(client); - core.createJob(provider, evaluator, expiry, "setter test", address(0), 0); + core.createJob(provider, evaluator, expiry, "setter test", address(0), 0, ""); uint256 jobId = 1; vm.expectRevert(ERC8183.Unauthorized.selector); @@ -911,7 +923,7 @@ contract ERC8183Test is Test { address payoutReceiver = makeAddr("assignedProviderReceiver"); vm.prank(client); - core.createJob(address(0), evaluator, _futureExpiry(), "late receiver provider", address(0), 0); + core.createJob(address(0), evaluator, _futureExpiry(), "late receiver provider", address(0), 0, ""); vm.prank(client); core.setProvider(1, provider, 0, ""); @@ -928,7 +940,7 @@ contract ERC8183Test is Test { uint48 expiry = _futureExpiry(); vm.prank(client); - core.createJob(provider, evaluator, expiry, "expired receiver", address(0), 0); + core.createJob(provider, evaluator, expiry, "expired receiver", address(0), 0, ""); vm.warp(uint256(expiry)); vm.expectRevert(ERC8183.WrongStatus.selector); @@ -943,26 +955,29 @@ contract ERC8183Test is Test { core.setHookWhitelist(address(hook), true); vm.prank(client); - core.createJob(provider, evaluator, _futureExpiry(), "hooked", address(hook), 0); + core.createJob(provider, evaluator, _futureExpiry(), "hooked", address(hook), 0, ""); address receiver = makeAddr("hookReceiver"); + // createJob already fired afterAction; assert the setPayoutReceiver delta specifically. + uint256 beforeBefore = hook.beforeCount(); + uint256 afterBefore = hook.afterCount(); vm.prank(provider); core.setPayoutReceiver(1, receiver, hex"cafe"); assertEq(hook.lastSelector(), core.setPayoutReceiver.selector); - assertEq(hook.beforeCount(), 1); - assertEq(hook.afterCount(), 1); + assertEq(hook.beforeCount(), beforeBefore + 1); + assertEq(hook.afterCount(), afterBefore + 1); assertEq(hook.lastData(), abi.encode(provider, receiver, bytes(hex"cafe"))); } // a reverting before hook gates the transition function test_setPayoutReceiver_beforeHookRevertGates() public { - RevertingHook hook = new RevertingHook(); + SelectiveRevertHook hook = new SelectiveRevertHook(ERC8183.setPayoutReceiver.selector); vm.prank(deployer); core.setHookWhitelist(address(hook), true); vm.prank(client); - core.createJob(provider, evaluator, _futureExpiry(), "hooked", address(hook), 0); + core.createJob(provider, evaluator, _futureExpiry(), "hooked", address(hook), 0, ""); vm.prank(provider); vm.expectRevert(bytes("blocked")); @@ -971,6 +986,40 @@ contract ERC8183Test is Test { assertEq(core.getJob(1).payoutReceiver, address(0)); } + // createJob fires afterAction only (no beforeAction) and the hook can read the new job + function test_createJob_firesAfterHookOnly() public { + RecordingHook hook = new RecordingHook(); + vm.prank(deployer); + core.setHookWhitelist(address(hook), true); + + uint48 expiry = _futureExpiry(); + vm.prank(client); + uint256 jobId = core.createJob(provider, evaluator, expiry, "hooked", address(hook), 0, hex"aa"); + + assertEq(hook.afterCount(), 1); + assertEq(hook.beforeCount(), 0); + assertEq(hook.lastSelector(), core.createJob.selector); + assertEq( + hook.lastData(), + abi.encode(client, provider, evaluator, expiry, address(hook), uint256(0), bytes(hex"aa")) + ); + assertGt(jobId, 0); + } + + // a reverting afterAction rejects job creation entirely + function test_createJob_afterHookRevertRejectsCreation() public { + RevertingHook hook = new RevertingHook(); + vm.prank(deployer); + core.setHookWhitelist(address(hook), true); + + vm.prank(client); + vm.expectRevert(bytes("blocked")); + core.createJob(provider, evaluator, _futureExpiry(), "hooked", address(hook), 0, ""); + + // No job was persisted. + assertEq(core.jobCounter(), 0); + } + function test_complete_RevertsWhenReceiverDisburserRevertsStrictly() public { MockDisburser disburser = new MockDisburser(); uint256 jobId = _createSubmittedJob(address(disburser)); @@ -1140,7 +1189,7 @@ contract ERC8183Test is Test { function test_claims_SubmitClaimRevertsAtExpiry() public { uint48 expiry = _futureExpiry(); vm.prank(client); - uint256 jobId = core.createJob(provider, evaluator, expiry, "expired claim", address(0), 0); + uint256 jobId = core.createJob(provider, evaluator, expiry, "expired claim", address(0), 0, ""); vm.prank(provider); core.setBudget(jobId, address(usdc), TWENTY_USDC, ""); vm.prank(client); @@ -1157,7 +1206,7 @@ contract ERC8183Test is Test { function test_claims_SettleClaimRevertsAtExpiry() public { uint48 expiry = _futureExpiry(); vm.prank(client); - uint256 jobId = core.createJob(provider, evaluator, expiry, "expired settlement", address(0), 0); + uint256 jobId = core.createJob(provider, evaluator, expiry, "expired settlement", address(0), 0, ""); vm.prank(provider); core.setBudget(jobId, address(usdc), TWENTY_USDC, ""); vm.prank(client); @@ -1210,7 +1259,7 @@ contract ERC8183Test is Test { function test_claims_DeadPendingClaimCanBeRejectedThenRefunded() public { uint48 expiry = _futureExpiry(); vm.prank(client); - uint256 jobId = core.createJob(provider, evaluator, expiry, "dead claim", address(0), 0); + uint256 jobId = core.createJob(provider, evaluator, expiry, "dead claim", address(0), 0, ""); vm.prank(provider); core.setBudget(jobId, address(usdc), TWENTY_USDC, ""); vm.prank(client); @@ -1295,7 +1344,7 @@ contract ERC8183Test is Test { core.setHookWhitelist(address(hook), true); vm.prank(client); - uint256 jobId = core.createJob(provider, evaluator, _futureExpiry(), "hooked claim job", address(hook), 0); + uint256 jobId = core.createJob(provider, evaluator, _futureExpiry(), "hooked claim job", address(hook), 0, ""); vm.prank(provider); core.setBudget(jobId, address(usdc), TWENTY_USDC, ""); vm.prank(client); @@ -1394,7 +1443,7 @@ contract ERC8183Test is Test { function test_claims_PendingClaimBlocksRefundUntilResolved() public { uint48 expiry = _futureExpiry(); vm.prank(client); - uint256 jobId = core.createJob(provider, evaluator, expiry, "pending claim block", address(0), 0); + uint256 jobId = core.createJob(provider, evaluator, expiry, "pending claim block", address(0), 0, ""); vm.prank(provider); core.setBudget(jobId, address(usdc), TWENTY_USDC, ""); vm.prank(client); @@ -1421,7 +1470,7 @@ contract ERC8183Test is Test { function test_claims_RejectPendingClaimAfterExpiryThenRefundsRemainingEscrow() public { uint48 expiry = _futureExpiry(); vm.prank(client); - uint256 jobId = core.createJob(provider, evaluator, expiry, "pending claim reject", address(0), 0); + uint256 jobId = core.createJob(provider, evaluator, expiry, "pending claim reject", address(0), 0, ""); vm.prank(provider); core.setBudget(jobId, address(usdc), TWENTY_USDC, ""); vm.prank(client); @@ -1453,7 +1502,7 @@ contract ERC8183Test is Test { core.setHookWhitelist(address(hook), true); vm.prank(client); - uint256 jobId = core.createJob(provider, evaluator, _futureExpiry(), "hooked", address(hook), 0); + uint256 jobId = core.createJob(provider, evaluator, _futureExpiry(), "hooked", address(hook), 0, ""); vm.prank(provider); core.setBudget(jobId, address(usdc), TEN_USDC, ""); vm.prank(client); @@ -1474,12 +1523,12 @@ contract ERC8183Test is Test { // a reverting hook blocks the refund (accepted fund-lock risk); admin detach recovers it function test_claimRefund_revertingHookLocksFundsThenDetachRecovers() public { - ClaimRefundBlockingHook hook = new ClaimRefundBlockingHook(); + SelectiveRevertHook hook = new SelectiveRevertHook(ERC8183.claimRefund.selector); vm.prank(deployer); core.setHookWhitelist(address(hook), true); vm.prank(client); - uint256 jobId = core.createJob(provider, evaluator, _futureExpiry(), "hooked", address(hook), 0); + uint256 jobId = core.createJob(provider, evaluator, _futureExpiry(), "hooked", address(hook), 0, ""); vm.prank(provider); core.setBudget(jobId, address(usdc), TEN_USDC, ""); vm.prank(client); diff --git a/test/ERC8183WithAuthorization.t.sol b/test/ERC8183WithAuthorization.t.sol index 4ba9469..38be027 100644 --- a/test/ERC8183WithAuthorization.t.sol +++ b/test/ERC8183WithAuthorization.t.sol @@ -21,7 +21,7 @@ contract ERC8183WithAuthorizationTest is Test { bytes32 constant EIP712_DOMAIN_TYPEHASH = keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); bytes32 constant CREATE_JOB_AUTHORIZATION_TYPEHASH = keccak256( - "CreateJobAuthorization(address signer,address provider,address evaluator,uint48 expiredAt,bytes32 descriptionHash,address hook,uint256 providerAgentId,uint72 nonce,uint256 deadline)" + "CreateJobAuthorization(address signer,address provider,address evaluator,uint48 expiredAt,bytes32 descriptionHash,address hook,uint256 providerAgentId,bytes32 optParamsHash,uint72 nonce,uint256 deadline)" ); bytes32 constant SET_PAYOUT_RECEIVER_AUTHORIZATION_TYPEHASH = keccak256( "SetPayoutReceiverAuthorization(address signer,uint256 jobId,address payoutReceiver,bytes32 optParamsHash,uint72 nonce,uint256 deadline)" @@ -186,7 +186,8 @@ contract ERC8183WithAuthorizationTest is Test { expiredAt: expiredAt, description: description, hook: hook, - providerAgentId: providerAgentId + providerAgentId: providerAgentId, + optParams: "" }); } @@ -214,6 +215,7 @@ contract ERC8183WithAuthorizationTest is Test { _hashString(description), hook, providerAgentId, + _hashBytes(""), nonce, deadline ) @@ -502,7 +504,7 @@ contract ERC8183WithAuthorizationTest is Test { function _createFundedJob() internal returns (uint256 jobId) { vm.prank(client); - jobId = core.createJob(provider, evaluator, _futureExpiry(), "claim auth job", address(0), 0); + jobId = core.createJob(provider, evaluator, _futureExpiry(), "claim auth job", address(0), 0, ""); vm.prank(provider); core.setBudget(jobId, address(usdc), TWENTY_USDC, ""); vm.prank(client); @@ -727,9 +729,63 @@ contract ERC8183WithAuthorizationTest is Test { assertEq(core.getJob(jobId).payoutReceiver, receiver); } + function test_createJobWithAuthorization_bindsOptParams() public { + uint48 expiry = _futureExpiry(); + uint256 deadline = _deadline(); + string memory description = "createjob optparams"; + + // Sign over optParams = 0x01. + bytes memory sig = _sign( + clientPk, + keccak256( + abi.encode( + CREATE_JOB_AUTHORIZATION_TYPEHASH, + client, + provider, + evaluator, + expiry, + _hashString(description), + address(0), + uint256(0), + _hashBytes(hex"01"), + uint72(70), + deadline + ) + ) + ); + + ERC8183WithAuthorization.CreateJobAuthorizationParams memory paramsWrong = ERC8183WithAuthorization + .CreateJobAuthorizationParams({ + provider: provider, + evaluator: evaluator, + expiredAt: expiry, + description: description, + hook: address(0), + providerAgentId: 0, + optParams: hex"02" + }); + vm.prank(relayer); + vm.expectRevert(ERC8183WithAuthorization.InvalidAuthorizationSignature.selector); + core.createJobWithAuthorization(paramsWrong, _auth(client, 70, deadline, sig)); + + ERC8183WithAuthorization.CreateJobAuthorizationParams memory paramsRight = ERC8183WithAuthorization + .CreateJobAuthorizationParams({ + provider: provider, + evaluator: evaluator, + expiredAt: expiry, + description: description, + hook: address(0), + providerAgentId: 0, + optParams: hex"01" + }); + vm.prank(relayer); + uint256 jobId = core.createJobWithAuthorization(paramsRight, _auth(client, 70, deadline, sig)); + assertEq(core.getJob(jobId).provider, provider); + } + function test_relaysClientAuthorizedSetProvider() public { vm.prank(client); - uint256 jobId = core.createJob(address(0), evaluator, _futureExpiry(), "late provider", address(0), 0); + uint256 jobId = core.createJob(address(0), evaluator, _futureExpiry(), "late provider", address(0), 0, ""); uint256 deadline = _deadline(); uint256 agentId = 7; bytes memory sig = _signSetProvider(clientPk, client, jobId, provider, agentId, "", 61, deadline); @@ -746,7 +802,7 @@ contract ERC8183WithAuthorizationTest is Test { function test_setProviderWithAuthorization_bindsOptParams() public { vm.prank(client); - uint256 jobId = core.createJob(address(0), evaluator, _futureExpiry(), "late provider", address(0), 0); + uint256 jobId = core.createJob(address(0), evaluator, _futureExpiry(), "late provider", address(0), 0, ""); uint256 deadline = _deadline(); uint256 agentId = 7; From d2d2a04a95724657245a06e52d466ed7d1777400 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Mon, 15 Jun 2026 15:38:56 +0800 Subject: [PATCH 05/32] feat: add indexed actor to BudgetSet for meta-tx attribution --- contracts/ERC8183.sol | 7 ++++--- test/ERC8183.t.sol | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/contracts/ERC8183.sol b/contracts/ERC8183.sol index fffe9f5..b1dc7ad 100644 --- a/contracts/ERC8183.sol +++ b/contracts/ERC8183.sol @@ -131,8 +131,9 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable ); /// @notice Emitted when the provider sets or updates the job budget event BudgetSet( - uint256 indexed jobId, - address indexed token, + uint256 indexed jobId, + address indexed actor, + address indexed token, uint256 amount ); /// @notice Emitted when the client funds the job escrow @@ -693,7 +694,7 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable job.paymentToken = token; job.budget = amount; - emit BudgetSet(jobId, token, amount); + emit BudgetSet(jobId, actor, token, amount); _afterHook(job.hook, jobId, this.setBudget.selector, data); } diff --git a/test/ERC8183.t.sol b/test/ERC8183.t.sol index bba83e5..3df04d8 100644 --- a/test/ERC8183.t.sol +++ b/test/ERC8183.t.sol @@ -125,7 +125,7 @@ contract ERC8183Test is Test { // Events (must match ERC8183.sol exactly for vm.expectEmit) event ProviderSet(uint256 indexed jobId, address indexed actor, address indexed provider, uint256 agentId); - event BudgetSet(uint256 indexed jobId, address indexed token, uint256 amount); + event BudgetSet(uint256 indexed jobId, address indexed actor, address indexed token, uint256 amount); event JobFunded(uint256 indexed jobId, address indexed client, uint256 amount); event JobSubmitted(uint256 indexed jobId, address indexed provider, bytes32 deliverable); event JobCompleted(uint256 indexed jobId, address indexed evaluator, bytes32 reason); @@ -384,7 +384,7 @@ contract ERC8183Test is Test { // Step 2: provider sets budget — expect BudgetSet event vm.expectEmit(true, true, true, true, address(core)); - emit BudgetSet(jobId, address(usdc), TWENTY_USDC); + emit BudgetSet(jobId, provider, address(usdc), TWENTY_USDC); vm.prank(provider); core.setBudget(jobId, address(usdc), TWENTY_USDC, ""); From 9deae83fe72e6f82631797c53e6a8d6855ba86a9 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Mon, 15 Jun 2026 15:41:03 +0800 Subject: [PATCH 06/32] docs: document hookable lifecycle additions and ordering guarantees Standardize hook-data terminology on actor; add createJob/setProvider/ setPayoutReceiver/claimRefund encoding rows; document afterAction-only createJob, the claimRefund fund-lock caveat, the before-hook supersede ordering for submit/reject, and the EIP-712 typehash changes. --- contracts/IERC8183Hook.sol | 7 ++++- docs/02-hook-system.md | 52 ++++++++++++++++++++++++++------------ 2 files changed, 42 insertions(+), 17 deletions(-) diff --git a/contracts/IERC8183Hook.sol b/contracts/IERC8183Hook.sol index 6b71d49..aecff94 100644 --- a/contracts/IERC8183Hook.sol +++ b/contracts/IERC8183Hook.sol @@ -21,7 +21,12 @@ import "@openzeppelin/contracts/utils/introspection/IERC165.sol"; * `data` according to the hook encoding documented for each selector. */ interface IERC8183Hook is IERC165 { - /// @dev Called before the core function executes. MAY revert to block the action. + /// @dev Called before the core function's primary state transition. MAY revert to block the action. + /// Ordering guarantee: no primary state change for this action has occurred yet when + /// beforeAction runs. Exception: for `submit` and `reject`, any pending milestone claim is + /// first superseded (its pendingClaimHash is cleared and a ClaimRejected event is emitted) + /// *before* beforeAction fires, so a hook observes the post-supersede claim state. + /// Note: `createJob` is afterAction-only and never invokes beforeAction. /// @param jobId The job ID. /// @param selector The function selector of the core function being called. /// @param data Encoded function-specific parameters (see protocol documentation for decoding). diff --git a/docs/02-hook-system.md b/docs/02-hook-system.md index a51723d..77c645b 100644 --- a/docs/02-hook-system.md +++ b/docs/02-hook-system.md @@ -25,9 +25,9 @@ interface IERC8183Hook is IERC165 { | Core function | Hooked? | Notes | |---------------|---------|-------| -| `createJob` | **No** | Hook is stored on the job, but no callbacks fire on creation. | -| `setPayoutReceiver` | **No** | Provider-side payout routing is set while Open; no hook callbacks. | -| `setProvider` | **No** | Client-only assignment of the provider. | +| `createJob` | **Yes (afterAction only)** | Hook is attached during creation, so only `afterAction` fires — the hook can initialize per-job bookkeeping or revert to reject the job. No `beforeAction`: no attached hook exists before creation, and attachment is already gated by the whitelist + ERC-165 check. | +| `setPayoutReceiver` | Yes | before + after | +| `setProvider` | Yes | before + after | | `setBudget` | Yes | before + after | | `fund` | Yes | before + after | | `submit` | Yes | before + after | @@ -37,7 +37,7 @@ interface IERC8183Hook is IERC165 { | `rejectClaim` | Yes | before + after | | `complete` | Yes | before + after | | `reject` | Yes | before + after | -| `claimRefund` | **No** | Permissionless safety mechanism — never hookable. | +| `claimRefund` | Yes | before + after. Permissionless refund path — see the fund-lock caveat below. | ## Data Encoding per Selector @@ -45,17 +45,37 @@ As produced by `ERC8183`: | Selector | `data` encoding | |-------------|------------------------------------------------------------------------------| -| `setBudget` | `abi.encode(address caller, address token, uint256 amount, bytes optParams)` | -| `fund` | `abi.encode(address caller, bytes optParams)` | -| `submit` | `abi.encode(address caller, bytes32 deliverable, bytes optParams)` | -| `submitClaim` | `abi.encode(address caller, uint256 cumulativeAmount, bytes32 deliverable, bytes optParams)` | -| `settleClaim` | `abi.encode(address caller, uint256 cumulativeAmount, bytes32 deliverable, bytes optParams)` | -| `approveClaim` | `abi.encode(address caller, uint256 cumulativeAmount, bytes32 deliverable, bytes optParams)` | -| `rejectClaim` | `abi.encode(address caller, uint256 cumulativeAmount, bytes32 deliverable, bytes32 reason, bytes optParams)` | -| `complete` | `abi.encode(address caller, bytes32 reason, bytes optParams)` | -| `reject` | `abi.encode(address caller, bytes32 reason, bytes optParams)` | - -All payloads include `address caller` so the hook knows who initiated the transition. +| `createJob` | `abi.encode(address actor, address provider, address evaluator, uint48 expiredAt, address hook, uint256 providerAgentId, bytes optParams)` | +| `setProvider` | `abi.encode(address actor, address provider, uint256 agentId, bytes optParams)` | +| `setPayoutReceiver` | `abi.encode(address actor, address payoutReceiver, bytes optParams)` | +| `setBudget` | `abi.encode(address actor, address token, uint256 amount, bytes optParams)` | +| `fund` | `abi.encode(address actor, bytes optParams)` | +| `submit` | `abi.encode(address actor, bytes32 deliverable, bytes optParams)` | +| `submitClaim` | `abi.encode(address actor, uint256 cumulativeAmount, bytes32 deliverable, bytes optParams)` | +| `settleClaim` | `abi.encode(address actor, uint256 cumulativeAmount, bytes32 deliverable, bytes optParams)` | +| `approveClaim` | `abi.encode(address actor, uint256 cumulativeAmount, bytes32 deliverable, bytes optParams)` | +| `rejectClaim` | `abi.encode(address actor, uint256 cumulativeAmount, bytes32 deliverable, bytes32 reason, bytes optParams)` | +| `complete` | `abi.encode(address actor, bytes32 reason, bytes optParams)` | +| `reject` | `abi.encode(address actor, bytes32 reason, bytes optParams)` | +| `claimRefund` | `abi.encode(address actor, bytes optParams)` | + +All payloads begin with `address actor` so the hook knows who authorized the transition. +For direct calls `actor` is `msg.sender`; for `*WithAuthorization` calls it is the EIP-712 +signer; for the permissionless `claimRefund` it is `msg.sender` (whoever triggered the refund). + +> **Fund-lock caveat for `claimRefund`.** `claimRefund` is the permissionless escrow +> recovery path, and it is fully hookable — a hook's `beforeAction`/`afterAction` MAY revert. +> A misbehaving hook that reverts can therefore block the refund and lock escrowed funds. +> Mitigations: hooks are admin-whitelisted and ERC-165-checked at attach time, and an admin +> can sever a bad hook from in-flight jobs via `batchDetachHook` (after which `claimRefund` +> succeeds). Only whitelist hooks you fully trust and have audited. + +### Signed-authorization note + +The `optParams` for `createJob`, `setProvider`, and `setPayoutReceiver` are bound into their +EIP-712 authorization typehashes via an `optParamsHash` field (matching `setBudget`/`fund`/etc.). +Adding `optParams` changed these three typehashes, so any authorization signatures produced +against the previous typehashes are no longer valid. ## How Hooks Attach to Jobs @@ -150,7 +170,7 @@ The core passes canonical base-function selectors such as `this.fund.selector`, - Hooks MUST NOT be able to change job state outside of defined transitions — they observe and gate, they do not write to `jobs[jobId]`. - `beforeAction` can revert to gate transitions — this is intentional and by design. - `afterAction` reverts roll back the whole transaction — hook state must stay consistent with core state. -- `claimRefund` is intentionally not hookable, but it still requires pending provider claims to be resolved first. +- `claimRefund` is hookable (before + after), but because it is the permissionless escrow recovery path, a reverting hook can block the refund and lock escrowed funds — mitigate via the whitelist/ERC-165 attach checks and `batchDetachHook` (see the fund-lock caveat above). It still requires pending provider claims to be resolved first. - `settleClaim` can run while a provider claim is pending; it updates cumulative settlement but does not close the pending claim lifecycle. - `approveClaim` and `rejectClaim` are hookable resolution actions; trusted hooks can gate them like other business transitions. - When `submit` supersedes a pending claim, the pending claim is cleared before submit hooks run so hooks observe the post-supersede state. From 9b5b527654b35878ac316069a30dada0947aa434 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Mon, 15 Jun 2026 16:21:34 +0800 Subject: [PATCH 07/32] fix: canonicalize createJob hook agentId payload and sync eip.md - createJob afterAction payload reads providerAgentId back from storage so a providerless job (canonicalized to 0) never hands hooks a disagreeing value; adds a regression test. - eip.md: update IERC8183 interface (optParams on createJob/setProvider/ setPayoutReceiver/claimRefund), event shapes (actor on ProviderSet/BudgetSet/ PayoutReceiverSet), hookability table (createJob afterAction-only; claimRefund hookable), and document the claimRefund refund-liveness change as breaking. --- contracts/ERC8183.sol | 5 +++- eip.md | 56 ++++++++++++++++++++++++------------------- test/ERC8183.t.sol | 20 ++++++++++++++++ 3 files changed, 55 insertions(+), 26 deletions(-) diff --git a/contracts/ERC8183.sol b/contracts/ERC8183.sol index b1dc7ad..7544825 100644 --- a/contracts/ERC8183.sol +++ b/contracts/ERC8183.sol @@ -558,6 +558,7 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable budget: 0, hook: hook, paymentToken: address(0), + // A providerless job has no provider to attribute, so its agent id is zero. providerAgentId: provider != address(0) ? providerAgentId : 0, description: description, settledAmount: 0, @@ -577,11 +578,13 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable // initialize per-job bookkeeping or revert to reject a job it will not service. // No beforeAction — there is no attached hook before creation, and attachment // is already gated by the whitelist + ERC-165 check above. + // The payload reads providerAgentId back from storage so the hook never sees an + // agent id that disagrees with jobs[jobId] (e.g. a providerless job canonicalizes to 0). _afterHook( hook, jobId, this.createJob.selector, - abi.encode(client, provider, evaluator, expiredAt, hook, providerAgentId, optParams) + abi.encode(client, provider, evaluator, expiredAt, hook, jobs[jobId].providerAgentId, optParams) ); return jobId; diff --git a/eip.md b/eip.md index 7d5e18a..f8f8de0 100644 --- a/eip.md +++ b/eip.md @@ -109,7 +109,7 @@ Called by evaluator only. SHALL revert if job is not Submitted or caller is not - **reject(jobId, reason, optParams?)** Called by **client or provider when job is Open**, or by **evaluator when job is Funded or Submitted**. SHALL revert if job is not Open, Funded, or Submitted, or if the caller is not authorised for the current status. SHALL set status to Rejected. If Funded or Submitted, SHALL refund the unsettled remainder (`budget - settledAmount`) to client. If a settlement claim is pending, SHALL clear it (see Claim interactions below). `reason` OPTIONAL. SHALL emit an event including `reason` and the caller (rejector) if provided. `optParams` forwarded to hook if set. - **claimRefund(jobId)** -Callable by anyone when status is Open, Funded, or Submitted. SHALL revert if status is Open or Funded and `block.timestamp < job.expiredAt`. SHALL revert if status is Submitted and `block.timestamp < job.expiredAt + EVALUATION_GRACE_PERIOD` (the grace period protects an evaluator who is mid-review from being censored by a third-party refund call; implementations MAY set the grace period length or omit it). SHALL revert if a settlement claim is pending on a non-Submitted job (claims can only arise while Funded) — pending claims MUST be resolved (approved, rejected, or withdrawn) before an expiry refund. SHALL set status to Expired. If the prior status was Funded or Submitted, SHALL transfer the unsettled remainder (`budget - settledAmount`), if non-zero, to the client. SHALL NOT be hookable. +Callable by anyone when status is Open, Funded, or Submitted. SHALL revert if status is Open or Funded and `block.timestamp < job.expiredAt`. SHALL revert if status is Submitted and `block.timestamp < job.expiredAt + EVALUATION_GRACE_PERIOD` (the grace period protects an evaluator who is mid-review from being censored by a third-party refund call; implementations MAY set the grace period length or omit it). SHALL revert if a settlement claim is pending on a non-Submitted job (claims can only arise while Funded) — pending claims MUST be resolved (approved, rejected, or withdrawn) before an expiry refund. SHALL set status to Expired. If the prior status was Funded or Submitted, SHALL transfer the unsettled remainder (`budget - settledAmount`), if non-zero, to the client. MAY be hookable; if hookable, the `beforeAction`/`afterAction` callbacks fire around the refund and a reverting hook can block it (see the liveness caveat under **Hook security**). Implementations requiring an unconditional post-expiry refund SHALL keep `claimRefund` non-hookable. ### Claim Settlement @@ -180,13 +180,13 @@ function beforeAction(uint256 jobId, bytes4 selector, bytes calldata data) exter } ``` -When a job has a hook set, the core contract SHALL call `hook.beforeAction(...)` and `hook.afterAction(...)` around each hookable function. `createJob` is intentionally not hookable in the reference implementation — the hook is stored on the job, but no callbacks fire on creation. Implementations MAY add an `afterAction`-only callback for `createJob` if they need post-creation bookkeeping; conformant non-hooked kernels simply ignore the field. +When a job has a hook set, the core contract SHALL call `hook.beforeAction(...)` and `hook.afterAction(...)` around each hookable function. `createJob` is `afterAction`-only: the hook is attached during creation, so no `beforeAction` can fire (there is no attached hook before the job exists), but `afterAction` fires once the job is created so the hook can initialize per-job bookkeeping or revert to reject the job. Conformant non-hooked kernels simply ignore the field. | Core function | Hookable | | -------------- | -------- | -| `createJob` | **No** — hook is stored on the job but no callback fires on creation in the reference implementation | -| `setPayoutReceiver` | **No** | -| `setProvider` | **No** | +| `createJob` | **Yes (`afterAction` only)** — hook is attached during creation; only the after-callback fires | +| `setPayoutReceiver` | Yes | +| `setProvider` | Yes | | `setBudget` | Yes | | `fund` | Yes | | `submit` | Yes | @@ -196,7 +196,7 @@ When a job has a hook set, the core contract SHALL call `hook.beforeAction(...)` | `settleClaim` | Yes | | `approveClaim` | Yes | | `rejectClaim` | Yes | -| `claimRefund` | **No** — permissionless safety mechanism, SHALL NOT be hookable | +| `claimRefund` | Yes — but see the liveness caveat under **Hook security** (a reverting hook can block the permissionless refund) | #### Data encoding @@ -204,15 +204,21 @@ The `data` parameter passed to hooks contains the core function's parameters enc | Core function | `data` encoding | | -------------- | ---------------------------------------------------- | -| `setBudget` | `abi.encode(address caller, address token, uint256 amount, bytes optParams)` | -| `fund` | `abi.encode(address caller, bytes optParams)` | -| `submit` | `abi.encode(address caller, bytes32 deliverable, bytes optParams)` | -| `complete` | `abi.encode(address caller, bytes32 reason, bytes optParams)` | -| `reject` | `abi.encode(address caller, bytes32 reason, bytes optParams)` | -| `submitClaim` | `abi.encode(address caller, uint256 cumulativeAmount, bytes32 deliverable, bytes optParams)` | -| `settleClaim` | `abi.encode(address caller, uint256 cumulativeAmount, bytes32 deliverable, bytes optParams)` | -| `approveClaim` | `abi.encode(address caller, uint256 cumulativeAmount, bytes32 deliverable, bytes optParams)` | -| `rejectClaim` | `abi.encode(address caller, uint256 cumulativeAmount, bytes32 deliverable, bytes32 reason, bytes optParams)` | +| `createJob` | `abi.encode(address actor, address provider, address evaluator, uint48 expiredAt, address hook, uint256 providerAgentId, bytes optParams)` | +| `setProvider` | `abi.encode(address actor, address provider, uint256 agentId, bytes optParams)` | +| `setPayoutReceiver` | `abi.encode(address actor, address payoutReceiver, bytes optParams)` | +| `setBudget` | `abi.encode(address actor, address token, uint256 amount, bytes optParams)` | +| `fund` | `abi.encode(address actor, bytes optParams)` | +| `submit` | `abi.encode(address actor, bytes32 deliverable, bytes optParams)` | +| `complete` | `abi.encode(address actor, bytes32 reason, bytes optParams)` | +| `reject` | `abi.encode(address actor, bytes32 reason, bytes optParams)` | +| `submitClaim` | `abi.encode(address actor, uint256 cumulativeAmount, bytes32 deliverable, bytes optParams)` | +| `settleClaim` | `abi.encode(address actor, uint256 cumulativeAmount, bytes32 deliverable, bytes optParams)` | +| `approveClaim` | `abi.encode(address actor, uint256 cumulativeAmount, bytes32 deliverable, bytes optParams)` | +| `rejectClaim` | `abi.encode(address actor, uint256 cumulativeAmount, bytes32 deliverable, bytes32 reason, bytes optParams)` | +| `claimRefund` | `abi.encode(address actor, bytes optParams)` | + +`actor` is the principal who authorized the action: `msg.sender` for direct calls, the EIP-712 signer for `*WithAuthorization` calls, and `msg.sender` (whoever triggered the refund) for the permissionless `claimRefund`. For `createJob`, `providerAgentId` is the canonicalized value stored on the job (zero for a providerless job), so it always agrees with `jobs[jobId]`. When `submit` or `reject` supersedes a pending claim, no claim-specific hook callback fires for the supersession itself; hooks observe the post-supersession claim state in the `submit`/`reject` callbacks. @@ -226,7 +232,7 @@ When `submit` or `reject` supersedes a pending claim, no claim-specific hook cal #### Hook security - Hooks are **trusted** contracts chosen by the client at job creation. A malicious or buggy hook can revert valid actions or execute arbitrary logic in callbacks. Clients SHOULD audit or use well-known hook implementations. -- **Liveness:** A reverting hook can block all hookable actions for that job until `expiredAt`. This is by design — the hook is part of the job's policy. The guaranteed recovery path is `claimRefund` after expiry, which is deliberately **not hookable** so that refunds cannot be blocked. +- **Liveness (BREAKING CHANGE):** A reverting hook can block all hookable actions for that job until `expiredAt`. This is by design — the hook is part of the job's policy. In earlier revisions of this specification `claimRefund` was deliberately **not** hookable, which gave an *unconditional* recovery guarantee: refunds after expiry could never be blocked. This revision makes `claimRefund` hookable (before + after). As a result, a buggy or malicious attached hook can revert in the `claimRefund` callbacks and **block the expired-escrow refund**, locking funds. The refund path is therefore no longer unconditionally live; recovery now depends on (a) only attaching hooks that do not revert on `claimRefund`, and (b) an admin severing a misbehaving hook from in-flight jobs (`batchDetachHook` in the reference implementation), after which `claimRefund` succeeds. Implementations that require an unconditional post-expiry refund guarantee SHOULD keep `claimRefund` non-hookable or provide a hook-bypassing refund path. - **Atomicity:** After-callbacks run after state changes but within the same transaction. If an after-callback reverts, the entire transaction (including the core state change) is rolled back. This is intentional — it enables atomic multi-step flows (e.g. escrow funding + side token transfer must both succeed or both revert). - `onlyERC8183` modifiers on hooks are RECOMMENDED so that hook functions cannot be called directly by external actors. - Hooks SHOULD NOT be upgradeable after a job is created, as this would allow the hook to change behaviour mid-job. @@ -284,7 +290,7 @@ Step 6 — complete Recovery: - reject: hook.afterAction returns escrowed tokens to provider (if deposited). - - expiry: claimRefund (not hookable) refunds serviceFee to client. + - expiry: claimRefund refunds serviceFee to client. Provider calls recoverTokens(jobId) on hook to recover deposited tokens. ``` @@ -340,11 +346,11 @@ Step 5 — job continues normally Implementations SHOULD emit at least: - **JobCreated**(jobId, client, provider, evaluator, expiredAt, hook) — includes the hook address (`address(0)` if no hook) -- **ProviderSet**(jobId, provider, agentId) — when provider is set on a job that was created without one; `agentId` is 0 if not specified -- **BudgetSet**(jobId, token, amount) — includes the payment token address +- **ProviderSet**(jobId, actor, provider, agentId) — when provider is set on a job that was created without one; `actor` is the caller (or authorizing signer); `agentId` is 0 if not specified +- **BudgetSet**(jobId, actor, token, amount) — `actor` is the caller (or authorizing signer); includes the payment token address - **JobFunded**(jobId, client, amount) - **JobSubmitted**(jobId, provider, deliverable) — when provider submits work for evaluation -- **PayoutReceiverSet**(jobId, payoutReceiver) — when a provider-side payout receiver is set or updated +- **PayoutReceiverSet**(jobId, actor, payoutReceiver) — when a provider-side payout receiver is set or updated; `actor` is the caller (or authorizing signer) - **JobCompleted**(jobId, evaluator, reason) - **JobRejected**(jobId, rejector, reason) - **JobExpired**(jobId) @@ -486,15 +492,15 @@ The interfaces below summarize the core job escrow with claim settlement, payout ```solidity interface IERC8183 { // ── Job lifecycle ── - function createJob(address provider, address evaluator, uint48 expiredAt, string calldata description, address hook, uint256 providerAgentId) external returns (uint256 jobId); - function setPayoutReceiver(uint256 jobId, address payoutReceiver) external; - function setProvider(uint256 jobId, address provider, uint256 agentId) external; + function createJob(address provider, address evaluator, uint48 expiredAt, string calldata description, address hook, uint256 providerAgentId, bytes calldata optParams) external returns (uint256 jobId); + function setPayoutReceiver(uint256 jobId, address payoutReceiver, bytes calldata optParams) external; + function setProvider(uint256 jobId, address provider, uint256 agentId, bytes calldata optParams) external; function setBudget(uint256 jobId, address token, uint256 amount, bytes calldata optParams) external; function fund(uint256 jobId, address expectedToken, uint256 expectedBudget, bytes calldata optParams) external; function submit(uint256 jobId, bytes32 deliverable, bytes calldata optParams) external; function complete(uint256 jobId, bytes32 reason, bytes calldata optParams) external; function reject(uint256 jobId, bytes32 reason, bytes calldata optParams) external; - function claimRefund(uint256 jobId) external; + function claimRefund(uint256 jobId, bytes calldata optParams) external; // ── Claim settlement ── function submitClaim(uint256 jobId, uint256 cumulativeAmount, bytes32 deliverable, bytes calldata optParams) external; @@ -535,7 +541,7 @@ function DOMAIN_SEPARATOR() external view returns (bytes32); - **Tokens:** Use SafeERC-20 or equivalent for [ERC-20](./eip-20.md). - **Evaluator:** MUST be set at creation; if "client completes", pass `evaluator = client`. - **Hook gas limits** (for hooked implementations): Implementations SHOULD impose a gas limit on hook calls (e.g. `call{gas: HOOK_GAS_LIMIT}(...)`) to bound execution cost and prevent hooks from consuming unbounded gas. The specific limit is left to the implementation as gas costs vary across chains. -- Hook contracts are client-supplied and trusted by the client; implementations MUST NOT allow hooks to modify core escrow state directly. `claimRefund` is deliberately not hookable so that refunds after expiry cannot be blocked by a malicious hook. +- Hook contracts are client-supplied and trusted by the client; implementations MUST NOT allow hooks to modify core escrow state directly. Where `claimRefund` is hookable (as in the reference implementation), a reverting hook can block refunds after expiry; the reference implementation mitigates this with hook whitelisting/ERC-165 checks at attach time and an admin `batchDetachHook` recovery path. Implementations requiring an unconditional post-expiry refund SHALL keep `claimRefund` non-hookable. - Jobs that use **advanced hooks** (e.g. two‑phase escrow / fund‑transfer hooks that custody additional tokens) are expected to have **more revert paths and tighter coupling** to external logic than plain, non‑hooked Agentic Commerce jobs. Such hooks SHOULD be reserved for agents and users who understand and accept this trade‑off; for most simple jobs, a non‑hooked or policy‑only hook is RECOMMENDED. ## Copyright diff --git a/test/ERC8183.t.sol b/test/ERC8183.t.sol index 3df04d8..7577038 100644 --- a/test/ERC8183.t.sol +++ b/test/ERC8183.t.sol @@ -1006,6 +1006,26 @@ contract ERC8183Test is Test { assertGt(jobId, 0); } + // for a providerless job, the hook payload's agentId is canonicalized to 0, matching storage + function test_createJob_providerlessJobCanonicalizesAgentIdInHookPayload() public { + RecordingHook hook = new RecordingHook(); + vm.prank(deployer); + core.setHookWhitelist(address(hook), true); + + uint48 expiry = _futureExpiry(); + // provider == address(0) but a nonzero agentId is supplied. + vm.prank(client); + uint256 jobId = core.createJob(address(0), evaluator, expiry, "hooked", address(hook), 999, ""); + + // Stored agent id is canonicalized to 0... + assertEq(core.getJob(jobId).providerAgentId, 0); + // ...and the hook payload must agree with storage (not the raw 999). + assertEq( + hook.lastData(), + abi.encode(client, address(0), evaluator, expiry, address(hook), uint256(0), bytes("")) + ); + } + // a reverting afterAction rejects job creation entirely function test_createJob_afterHookRevertRejectsCreation() public { RevertingHook hook = new RevertingHook(); From afb4093cea568e3195cd3c2490dbce651ed37802 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Mon, 15 Jun 2026 17:14:57 +0800 Subject: [PATCH 08/32] docs: fix residual hookability inconsistencies - NatSpec: add @param optParams to createJob/setProvider/setPayoutReceiver and @param agentId to setProvider. - README + docs/01-architecture: correct stale 'claimRefund/createJob not hookable' claims to match the new afterAction-only createJob and hookable claimRefund. - eip.md: claimRefund normative heading carries optParams. --- README.md | 2 +- contracts/ERC8183.sol | 4 ++++ docs/01-architecture.md | 2 +- eip.md | 2 +- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index d4ebbb1..10ddfd6 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,7 @@ contracts/ - **Pending claim resolution** — after expiry, a Funded job with a pending provider claim cannot be force-refunded until the claim is approved, rejected, or withdrawn; if all parties stay idle, escrow remains parked - **Claim replay guard** — rejected claim hashes stay consumed, so providers must vary `cumulativeAmount`, the deliverable, or `optParams` to refile - **Authorization extension** — `ERC8183WithAuthorization` uses the base `ERC8183` EIP-712 domain so relayed entrypoints extend the same protocol identity; signers can call `cancelAuthorization` directly to burn one of their own outstanding nonces -- **Hook safety** — `claimRefund` is intentionally not hookable; pending claims must be resolved before refund +- **Hook safety** — `claimRefund` is hookable, but as the permissionless refund path a reverting hook can block it; hook whitelisting + ERC-165 checks and admin `batchDetachHook` mitigate this. Pending claims must still be resolved before refund See [docs/01-architecture.md](docs/01-architecture.md) for state machine and sequence diagrams. diff --git a/contracts/ERC8183.sol b/contracts/ERC8183.sol index 7544825..a2953b4 100644 --- a/contracts/ERC8183.sol +++ b/contracts/ERC8183.sol @@ -509,6 +509,7 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable /// @param description Human-readable job description /// @param hook Hook contract address (address(0) = no hook, must be whitelisted) /// @param providerAgentId Optional ERC-8004 agent identity for provider + /// @param optParams Hook-specific parameters (passed to the afterAction hook) /// @return The new job ID function createJob( address provider, @@ -594,6 +595,7 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable /// Locked once the job is funded. /// @param jobId The job to update /// @param payoutReceiver New payout receiver (address(0) = pay provider directly) + /// @param optParams Hook-specific parameters (passed to before/after hooks) function setPayoutReceiver(uint256 jobId, address payoutReceiver, bytes calldata optParams) external whenNotPaused @@ -627,6 +629,8 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable /// @notice Assigns a provider to an Open job that has no provider yet. Client only. /// @param jobId The job to assign a provider to /// @param provider_ The provider address + /// @param agentId Optional ERC-8004 agent identity for the provider + /// @param optParams Hook-specific parameters (passed to before/after hooks) function setProvider(uint256 jobId, address provider_, uint256 agentId, bytes calldata optParams) external whenNotPaused diff --git a/docs/01-architecture.md b/docs/01-architecture.md index 7ffc4c6..0a4f8d9 100644 --- a/docs/01-architecture.md +++ b/docs/01-architecture.md @@ -91,7 +91,7 @@ sequenceDiagram ## Sequence — Job with Hook -`createJob` is not hookable in the reference implementation — the hook is stored on the job but no callbacks fire on creation. Hooks begin firing on `setBudget`. +`createJob` fires the hook's `afterAction` only (the hook is attached during creation, so no `beforeAction` can run); `beforeAction`/`afterAction` then fire on every subsequent lifecycle call (`setProvider`, `setPayoutReceiver`, `setBudget`, and onward). ```mermaid sequenceDiagram diff --git a/eip.md b/eip.md index f8f8de0..4687671 100644 --- a/eip.md +++ b/eip.md @@ -108,7 +108,7 @@ Called by provider only. SHALL revert if caller is not the job's provider, or if Called by evaluator only. SHALL revert if job is not Submitted or caller is not the job's evaluator. SHALL set status to Completed. SHALL transfer the unsettled remainder (`budget - settledAmount`) to the provider-side payout recipient, minus optional platform fee to a configurable treasury and optional evaluator fee paid to the evaluator address. `reason` MAY be `bytes32(0)` or an attestation hash (OPTIONAL). SHALL emit an event including `reason` if provided. `optParams` forwarded to hook if set. - **reject(jobId, reason, optParams?)** Called by **client or provider when job is Open**, or by **evaluator when job is Funded or Submitted**. SHALL revert if job is not Open, Funded, or Submitted, or if the caller is not authorised for the current status. SHALL set status to Rejected. If Funded or Submitted, SHALL refund the unsettled remainder (`budget - settledAmount`) to client. If a settlement claim is pending, SHALL clear it (see Claim interactions below). `reason` OPTIONAL. SHALL emit an event including `reason` and the caller (rejector) if provided. `optParams` forwarded to hook if set. -- **claimRefund(jobId)** +- **claimRefund(jobId, optParams)** Callable by anyone when status is Open, Funded, or Submitted. SHALL revert if status is Open or Funded and `block.timestamp < job.expiredAt`. SHALL revert if status is Submitted and `block.timestamp < job.expiredAt + EVALUATION_GRACE_PERIOD` (the grace period protects an evaluator who is mid-review from being censored by a third-party refund call; implementations MAY set the grace period length or omit it). SHALL revert if a settlement claim is pending on a non-Submitted job (claims can only arise while Funded) — pending claims MUST be resolved (approved, rejected, or withdrawn) before an expiry refund. SHALL set status to Expired. If the prior status was Funded or Submitted, SHALL transfer the unsettled remainder (`budget - settledAmount`), if non-zero, to the client. MAY be hookable; if hookable, the `beforeAction`/`afterAction` callbacks fire around the refund and a reverting hook can block it (see the liveness caveat under **Hook security**). Implementations requiring an unconditional post-expiry refund SHALL keep `claimRefund` non-hookable. ### Claim Settlement From fd2167dbf6c79be21acbb98db0bd537c07dbfa81 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Mon, 15 Jun 2026 18:05:07 +0800 Subject: [PATCH 09/32] docs: document claimRefund blocking-hook rationale; test atomic forward claimRefund's hook is intentionally blocking: job.client may be a contract that forwards the refund to the real beneficiary in afterAction (which runs after the refund transfer), so a failed forward must revert the whole refund rather than strand funds in the intermediary. Reframe NatSpec/eip.md/docs/README accordingly and add ForwardingClient tests for the atomic-forward happy path and the revert-rolls-back-everything path. --- README.md | 2 +- contracts/ERC8183.sol | 19 ++++++--- docs/02-hook-system.md | 18 +++++--- eip.md | 2 +- test/ERC8183.t.sol | 94 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 121 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 10ddfd6..2cd788b 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,7 @@ contracts/ - **Pending claim resolution** — after expiry, a Funded job with a pending provider claim cannot be force-refunded until the claim is approved, rejected, or withdrawn; if all parties stay idle, escrow remains parked - **Claim replay guard** — rejected claim hashes stay consumed, so providers must vary `cumulativeAmount`, the deliverable, or `optParams` to refile - **Authorization extension** — `ERC8183WithAuthorization` uses the base `ERC8183` EIP-712 domain so relayed entrypoints extend the same protocol identity; signers can call `cancelAuthorization` directly to burn one of their own outstanding nonces -- **Hook safety** — `claimRefund` is hookable, but as the permissionless refund path a reverting hook can block it; hook whitelisting + ERC-165 checks and admin `batchDetachHook` mitigate this. Pending claims must still be resolved before refund +- **Hook safety** — `claimRefund` is hookable and intentionally blocking: the refund is paid to `job.client` (which may be a contract) and the `afterAction` hook runs after the transfer so a contract-client can atomically forward funds to the real beneficiary. The trade-off is that a reverting hook can block the permissionless refund; hook whitelisting + ERC-165 checks and admin `batchDetachHook` mitigate this. Pending claims must still be resolved before refund See [docs/01-architecture.md](docs/01-architecture.md) for state machine and sequence diagrams. diff --git a/contracts/ERC8183.sol b/contracts/ERC8183.sol index a2953b4..c3a7dca 100644 --- a/contracts/ERC8183.sol +++ b/contracts/ERC8183.sol @@ -912,12 +912,19 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable /// @notice Claims a refund for an expired job. Anyone can call. /// Transitions Open/Funded/Submitted -> Expired after expiry time. /// Pending claims must still be resolved before refund. - /// @dev Hookable: the attached hook's before/after callbacks fire around the - /// refund and MAY revert. Because this is the permissionless escrow - /// recovery path, a misbehaving hook that reverts can block the refund - /// and lock escrowed funds. Mitigations: hooks are admin-whitelisted and - /// ERC-165-checked at attach time, and an admin can sever a bad hook from - /// in-flight jobs via batchDetachHook (after which claimRefund succeeds). + /// @dev Hookable, and intentionally BLOCKING. The refund is paid to `job.client`, + /// which may be a contract standing in for the real beneficiary (e.g. a + /// smart account or router). The `afterAction` callback runs after the refund + /// transfer (see ordering below) so that client contract can atomically + /// forward the funds to the rightful end-client. The callbacks therefore MAY + /// revert to roll the whole refund back — this is required: a failed forward + /// MUST NOT leave funds stranded in the intermediary client contract. + /// Trade-off: this removes the unconditional post-expiry refund guarantee — a + /// buggy/reverting hook can block the refund. Mitigations: hooks are + /// admin-whitelisted and ERC-165-checked at attach time, and an admin can sever + /// a hook from in-flight jobs via batchDetachHook (after which claimRefund + /// proceeds with no callbacks). Deployments that need an unconditional refund + /// guarantee should attach no hook (or a hook that never reverts on this path). /// @param jobId The expired job to claim refund for /// @param optParams Hook-specific parameters (passed to before/after hooks) function claimRefund(uint256 jobId, bytes calldata optParams) external whenNotPaused nonReentrant { diff --git a/docs/02-hook-system.md b/docs/02-hook-system.md index 77c645b..b91d397 100644 --- a/docs/02-hook-system.md +++ b/docs/02-hook-system.md @@ -63,12 +63,18 @@ All payloads begin with `address actor` so the hook knows who authorized the tra For direct calls `actor` is `msg.sender`; for `*WithAuthorization` calls it is the EIP-712 signer; for the permissionless `claimRefund` it is `msg.sender` (whoever triggered the refund). -> **Fund-lock caveat for `claimRefund`.** `claimRefund` is the permissionless escrow -> recovery path, and it is fully hookable — a hook's `beforeAction`/`afterAction` MAY revert. -> A misbehaving hook that reverts can therefore block the refund and lock escrowed funds. -> Mitigations: hooks are admin-whitelisted and ERC-165-checked at attach time, and an admin -> can sever a bad hook from in-flight jobs via `batchDetachHook` (after which `claimRefund` -> succeeds). Only whitelist hooks you fully trust and have audited. +> **`claimRefund` is intentionally a blocking hook.** The refund is paid to `job.client`, +> which may be a contract standing in for the real beneficiary (e.g. a smart account or +> router). `claimRefund`'s `afterAction` runs *after* the refund transfer, so that client +> contract can atomically forward the funds to the rightful end-client. The callback must be +> able to revert the whole refund — a failed forward must not strand funds in the intermediary. +> +> The accepted trade-off: as the permissionless recovery path, a buggy or fail-closed hook +> that reverts on `claimRefund` can block the refund and lock escrowed funds. Mitigations: +> hooks are admin-whitelisted and ERC-165-checked at attach time, and an admin can sever a +> bad hook from in-flight jobs via `batchDetachHook` (after which `claimRefund` succeeds with +> no callbacks). Only whitelist hooks you fully trust and have audited; deployments that don't +> need contract-client forwarding and want an unconditional refund should attach no hook. ### Signed-authorization note diff --git a/eip.md b/eip.md index 4687671..afb59ad 100644 --- a/eip.md +++ b/eip.md @@ -232,7 +232,7 @@ When `submit` or `reject` supersedes a pending claim, no claim-specific hook cal #### Hook security - Hooks are **trusted** contracts chosen by the client at job creation. A malicious or buggy hook can revert valid actions or execute arbitrary logic in callbacks. Clients SHOULD audit or use well-known hook implementations. -- **Liveness (BREAKING CHANGE):** A reverting hook can block all hookable actions for that job until `expiredAt`. This is by design — the hook is part of the job's policy. In earlier revisions of this specification `claimRefund` was deliberately **not** hookable, which gave an *unconditional* recovery guarantee: refunds after expiry could never be blocked. This revision makes `claimRefund` hookable (before + after). As a result, a buggy or malicious attached hook can revert in the `claimRefund` callbacks and **block the expired-escrow refund**, locking funds. The refund path is therefore no longer unconditionally live; recovery now depends on (a) only attaching hooks that do not revert on `claimRefund`, and (b) an admin severing a misbehaving hook from in-flight jobs (`batchDetachHook` in the reference implementation), after which `claimRefund` succeeds. Implementations that require an unconditional post-expiry refund guarantee SHOULD keep `claimRefund` non-hookable or provide a hook-bypassing refund path. +- **Liveness (BREAKING CHANGE):** A reverting hook can block all hookable actions for that job until `expiredAt`. This is by design — the hook is part of the job's policy. In earlier revisions of this specification `claimRefund` was deliberately **not** hookable, which gave an *unconditional* recovery guarantee: refunds after expiry could never be blocked. This revision makes `claimRefund` hookable (before + after) **intentionally and as a blocking callback**, to support a `job.client` that is a contract standing in for the real beneficiary (e.g. a smart account or router): the refund is paid to that contract and its `afterAction` callback (which runs *after* the refund transfer) atomically forwards the funds to the rightful end-client. The callback MUST be able to revert the whole refund, because a failed forward must not leave funds stranded in the intermediary client contract. The accepted trade-off is that the post-expiry refund is no longer unconditionally live: a buggy or fail-closed hook can revert and block it. Recovery then depends on (a) only attaching hooks that do not revert on `claimRefund`, and (b) an admin severing a misbehaving hook from in-flight jobs (`batchDetachHook` in the reference implementation), after which `claimRefund` succeeds with no callbacks. Deployments that do not need contract-client forwarding and require an unconditional post-expiry refund SHOULD attach no hook (or a non-reverting hook) on such jobs. - **Atomicity:** After-callbacks run after state changes but within the same transaction. If an after-callback reverts, the entire transaction (including the core state change) is rolled back. This is intentional — it enables atomic multi-step flows (e.g. escrow funding + side token transfer must both succeed or both revert). - `onlyERC8183` modifiers on hooks are RECOMMENDED so that hook functions cannot be called directly by external actors. - Hooks SHOULD NOT be upgradeable after a job is created, as this would allow the hook to change behaviour mid-job. diff --git a/test/ERC8183.t.sol b/test/ERC8183.t.sol index 7577038..b0c0a31 100644 --- a/test/ERC8183.t.sol +++ b/test/ERC8183.t.sol @@ -107,6 +107,50 @@ contract SelectiveRevertHook is IERC8183Hook { } } +/// @notice A contract client that also acts as its own hook: on claimRefund it atomically +/// forwards the refunded escrow to the real end-beneficiary. Models the intended +/// reason claimRefund hooks are blocking — a failed forward must roll back the +/// entire refund so funds are never stranded in this intermediary. +contract ForwardingClient is IERC8183Hook { + ERC8183 immutable core; + MockUSDC immutable token; + address public beneficiary; + bool public failForward; + + constructor(ERC8183 core_, MockUSDC token_, address beneficiary_) { + core = core_; + token = token_; + beneficiary = beneficiary_; + } + + function setFailForward(bool v) external { + failForward = v; + } + + function createJob(address provider, address evaluator, uint48 expiry) external returns (uint256) { + return core.createJob(provider, evaluator, expiry, "forwarded", address(this), 0, ""); + } + + function fund(uint256 jobId, uint256 amount) external { + token.approve(address(core), amount); + core.fund(jobId, address(token), amount, ""); + } + + function beforeAction(uint256, bytes4, bytes calldata) external override {} + + function afterAction(uint256, bytes4 selector, bytes calldata) external override { + if (selector == ERC8183.claimRefund.selector) { + require(!failForward, "forward failed"); + uint256 bal = token.balanceOf(address(this)); + if (bal > 0) require(token.transfer(beneficiary, bal), "transfer failed"); + } + } + + function supportsInterface(bytes4 id) external pure override returns (bool) { + return id == type(IERC8183Hook).interfaceId || id == type(IERC165).interfaceId; + } +} + /// @notice Image Generation — E2E flow (no hook, core-only payment). /// Mirrors the original Hardhat suite in test/ERC8183.test.js. contract ERC8183Test is Test { @@ -1572,4 +1616,54 @@ contract ERC8183Test is Test { assertEq(usdc.balanceOf(client), balBefore + TEN_USDC); assertEq(uint8(core.getJob(jobId).status), uint8(ERC8183.JobStatus.Expired)); } + + // A contract client forwards the refund to the real beneficiary in its afterAction hook. + // This is the intended reason the claimRefund hook is blocking. + function test_claimRefund_contractClientForwardsRefundAtomically() public { + address endClient = makeAddr("endClient"); + ForwardingClient fc = new ForwardingClient(core, usdc, endClient); + vm.prank(deployer); + core.setHookWhitelist(address(fc), true); + + // Fund the contract-client so it can fund the job (it is job.client and the hook). + usdc.mint(address(fc), TEN_USDC); + uint256 jobId = fc.createJob(provider, evaluator, _futureExpiry()); + vm.prank(provider); + core.setBudget(jobId, address(usdc), TEN_USDC, ""); + fc.fund(jobId, TEN_USDC); + + vm.warp(block.timestamp + 3601); + core.claimRefund(jobId, ""); + + // Refund landed at the contract-client and was atomically forwarded to the real client. + assertEq(usdc.balanceOf(endClient), TEN_USDC); + assertEq(usdc.balanceOf(address(fc)), 0); + assertEq(uint8(core.getJob(jobId).status), uint8(ERC8183.JobStatus.Expired)); + } + + // If the contract client's forward fails, the entire refund MUST roll back — funds are + // never stranded in the intermediary. This is why the hook is intentionally blocking. + function test_claimRefund_contractClientFailedForwardRevertsEntireRefund() public { + address endClient = makeAddr("endClient"); + ForwardingClient fc = new ForwardingClient(core, usdc, endClient); + vm.prank(deployer); + core.setHookWhitelist(address(fc), true); + + usdc.mint(address(fc), TEN_USDC); + uint256 jobId = fc.createJob(provider, evaluator, _futureExpiry()); + vm.prank(provider); + core.setBudget(jobId, address(usdc), TEN_USDC, ""); + fc.fund(jobId, TEN_USDC); + + fc.setFailForward(true); + vm.warp(block.timestamp + 3601); + + vm.expectRevert(bytes("forward failed")); + core.claimRefund(jobId, ""); + + // Whole tx rolled back: job still Funded, escrow intact, nothing forwarded. + assertEq(uint8(core.getJob(jobId).status), uint8(ERC8183.JobStatus.Funded)); + assertEq(usdc.balanceOf(address(core)), TEN_USDC); + assertEq(usdc.balanceOf(endClient), 0); + } } From b2523504753c5e2f0b0af75c61afa465188965b4 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Mon, 15 Jun 2026 21:05:34 +0800 Subject: [PATCH 10/32] docs: expand claimRefund contract-client forwarding rationale Add a dedicated design-rationale note in eip.md and a concrete forwarding-flow example in docs/02-hook-system.md explaining why claimRefund is hookable and blocking (refund paid to a contract client, afterAction forwards atomically to the real beneficiary; a failed forward must roll back the whole refund). --- docs/02-hook-system.md | 17 +++++++++++++++++ eip.md | 21 +++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/docs/02-hook-system.md b/docs/02-hook-system.md index b91d397..5327f1e 100644 --- a/docs/02-hook-system.md +++ b/docs/02-hook-system.md @@ -76,6 +76,23 @@ signer; for the permissionless `claimRefund` it is `msg.sender` (whoever trigger > no callbacks). Only whitelist hooks you fully trust and have audited; deployments that don't > need contract-client forwarding and want an unconditional refund should attach no hook. +The forwarding flow, concretely (the `ForwardingClient` in the test suite demonstrates it): + +``` +claimRefund(jobId) + ├─ status -> Expired + ├─ transfer (budget - settledAmount) to job.client // the client *contract* + └─ afterAction(jobId, claimRefund.selector, data) // runs AFTER the transfer + └─ job.client forwards the received funds to the real end-beneficiary + └─ if the forward reverts, the whole claimRefund reverts: + escrow stays put, nothing is stranded in job.client +``` + +Because the callback runs after the transfer and can revert the whole call, the refund and +the onward forward are atomic. A hook attached to such a job MUST handle the `claimRefund` +selector (forward, or no-op) and MUST NOT revert on it for reasons unrelated to a genuine +forward failure — otherwise it permanently blocks the only refund path until `batchDetachHook`. + ### Signed-authorization note The `optParams` for `createJob`, `setProvider`, and `setPayoutReceiver` are bound into their diff --git a/eip.md b/eip.md index afb59ad..02430d5 100644 --- a/eip.md +++ b/eip.md @@ -111,6 +111,27 @@ Called by **client or provider when job is Open**, or by **evaluator when job is - **claimRefund(jobId, optParams)** Callable by anyone when status is Open, Funded, or Submitted. SHALL revert if status is Open or Funded and `block.timestamp < job.expiredAt`. SHALL revert if status is Submitted and `block.timestamp < job.expiredAt + EVALUATION_GRACE_PERIOD` (the grace period protects an evaluator who is mid-review from being censored by a third-party refund call; implementations MAY set the grace period length or omit it). SHALL revert if a settlement claim is pending on a non-Submitted job (claims can only arise while Funded) — pending claims MUST be resolved (approved, rejected, or withdrawn) before an expiry refund. SHALL set status to Expired. If the prior status was Funded or Submitted, SHALL transfer the unsettled remainder (`budget - settledAmount`), if non-zero, to the client. MAY be hookable; if hookable, the `beforeAction`/`afterAction` callbacks fire around the refund and a reverting hook can block it (see the liveness caveat under **Hook security**). Implementations requiring an unconditional post-expiry refund SHALL keep `claimRefund` non-hookable. +> **Design rationale — refund forwarding for contract clients.** `claimRefund` is +> intentionally hookable *and* its callbacks are intentionally **blocking**, to support a +> `job.client` that is a contract standing in for the ultimate beneficiary (for example a +> smart account, custodial vault, or routing contract that fans funds out to an off-protocol +> recipient). The refund is paid to `job.client`, and `claimRefund`'s `afterAction` callback +> runs **after** that transfer (the ordering is normative: pay the client, then call +> `afterAction`). This lets the client contract observe the freshly received refund and +> **atomically forward it onward** to the rightful end-client within the same transaction. +> +> Because the forward and the refund must be all-or-nothing, the callback MUST be able to +> revert the entire `claimRefund`: a forward that fails (e.g. the downstream recipient +> reverts, or the client contract is misconfigured) MUST roll the refund back rather than +> leave funds stranded in the intermediary contract with no way to reach the beneficiary. +> A non-blocking (fail-open) callback would be unsafe here, since it would complete the +> refund into the intermediary while silently dropping the forward. +> +> The accepted cost is that the post-expiry refund is no longer *unconditionally* live: a +> buggy or fail-closed hook can revert and block it (see **Hook security**). Deployments that +> do not need contract-client forwarding SHOULD attach no hook (or a hook that never reverts +> on the `claimRefund` selector) so the refund stays unconditional. + ### Claim Settlement While a job is Funded, escrow MAY be released incrementally through **claim settlement**, ahead of (or instead of) terminal completion. Two settlement paths share one ledger: From 14ca4b7754afd51c8ecf4458bb615a7a7dc3c5df Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Tue, 16 Jun 2026 11:54:33 +0800 Subject: [PATCH 11/32] docs: adopt audit + admin emergency-withdraw trust model for claimRefund hook claimRefund stays hookable and blocking (for contract-client forwarding). The refund-liveness risk is handled by governance rather than removing the hook: whitelisted hooks are audited to never block lifecycle paths (and to ignore caller-supplied optParams on the permissionless claimRefund path), and admin pause() + emergencyWithdraw is the on-chain break-glass recovery. Document this in NatSpec/eip.md/docs/README and add a test proving emergencyWithdraw recovers escrow stuck behind a blocking hook. --- README.md | 2 +- contracts/ERC8183.sol | 14 +++++++++----- docs/02-hook-system.md | 18 +++++++++++++----- eip.md | 4 ++-- test/ERC8183.t.sol | 32 ++++++++++++++++++++++++++++++++ 5 files changed, 57 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 2cd788b..076262e 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,7 @@ contracts/ - **Pending claim resolution** — after expiry, a Funded job with a pending provider claim cannot be force-refunded until the claim is approved, rejected, or withdrawn; if all parties stay idle, escrow remains parked - **Claim replay guard** — rejected claim hashes stay consumed, so providers must vary `cumulativeAmount`, the deliverable, or `optParams` to refile - **Authorization extension** — `ERC8183WithAuthorization` uses the base `ERC8183` EIP-712 domain so relayed entrypoints extend the same protocol identity; signers can call `cancelAuthorization` directly to burn one of their own outstanding nonces -- **Hook safety** — `claimRefund` is hookable and intentionally blocking: the refund is paid to `job.client` (which may be a contract) and the `afterAction` hook runs after the transfer so a contract-client can atomically forward funds to the real beneficiary. The trade-off is that a reverting hook can block the permissionless refund; hook whitelisting + ERC-165 checks and admin `batchDetachHook` mitigate this. Pending claims must still be resolved before refund +- **Hook safety** — `claimRefund` is hookable and intentionally blocking: the refund is paid to `job.client` (which may be a contract) and the `afterAction` hook runs after the transfer so a contract-client can atomically forward funds to the real beneficiary. The trade-off (a reverting hook can block the permissionless refund) is handled by governance: hooks are whitelisted only after an audit that they don't block lifecycle paths, with admin `pause()` + `emergencyWithdraw` as the on-chain break-glass recovery. Pending claims must still be resolved before refund See [docs/01-architecture.md](docs/01-architecture.md) for state machine and sequence diagrams. diff --git a/contracts/ERC8183.sol b/contracts/ERC8183.sol index c3a7dca..e0bd24e 100644 --- a/contracts/ERC8183.sol +++ b/contracts/ERC8183.sol @@ -920,11 +920,15 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable /// revert to roll the whole refund back — this is required: a failed forward /// MUST NOT leave funds stranded in the intermediary client contract. /// Trade-off: this removes the unconditional post-expiry refund guarantee — a - /// buggy/reverting hook can block the refund. Mitigations: hooks are - /// admin-whitelisted and ERC-165-checked at attach time, and an admin can sever - /// a hook from in-flight jobs via batchDetachHook (after which claimRefund - /// proceeds with no callbacks). Deployments that need an unconditional refund - /// guarantee should attach no hook (or a hook that never reverts on this path). + /// buggy/reverting hook could otherwise block the refund. The trust model + /// mitigates this: hooks are admin-whitelisted + ERC-165-checked at attach time + /// and are expected to be audited as part of whitelisting to never block + /// lifecycle paths (i.e. never revert on claimRefund except for a genuine + /// forward failure) and to never derive routing/authorization from the + /// caller-supplied optParams on this permissionless path. Break-glass recovery if + /// a hook still blocks a refund: admin pause() + emergencyWithdraw moves the + /// escrow out regardless of hooks; batchDetachHook is the lighter option for a + /// non-forwarding hook (after which claimRefund proceeds with no callbacks). /// @param jobId The expired job to claim refund for /// @param optParams Hook-specific parameters (passed to before/after hooks) function claimRefund(uint256 jobId, bytes calldata optParams) external whenNotPaused nonReentrant { diff --git a/docs/02-hook-system.md b/docs/02-hook-system.md index 5327f1e..93302ba 100644 --- a/docs/02-hook-system.md +++ b/docs/02-hook-system.md @@ -70,11 +70,19 @@ signer; for the permissionless `claimRefund` it is `msg.sender` (whoever trigger > able to revert the whole refund — a failed forward must not strand funds in the intermediary. > > The accepted trade-off: as the permissionless recovery path, a buggy or fail-closed hook -> that reverts on `claimRefund` can block the refund and lock escrowed funds. Mitigations: -> hooks are admin-whitelisted and ERC-165-checked at attach time, and an admin can sever a -> bad hook from in-flight jobs via `batchDetachHook` (after which `claimRefund` succeeds with -> no callbacks). Only whitelist hooks you fully trust and have audited; deployments that don't -> need contract-client forwarding and want an unconditional refund should attach no hook. +> that reverts on `claimRefund` can block the refund and lock escrowed funds. This is handled +> by a governance trust model: +> - Hooks are admin-whitelisted only after an **audit confirming they do not block lifecycle +> paths** — a whitelisted hook MUST NOT revert on `claimRefund` except for a genuine forward +> failure, and MUST NOT derive routing/authorization from the caller-supplied `optParams` +> on this permissionless path (anyone can trigger `claimRefund`). +> - **Break-glass recovery:** if a hook still blocks a refund, the admin can `pause()` and +> `emergencyWithdraw` the escrow regardless of any hook; `batchDetachHook` is the lighter +> option for a non-forwarding hook (after which `claimRefund` succeeds with no callbacks). +> +> Only whitelist hooks you fully trust and have audited. Deployments that don't need +> contract-client forwarding and want an unconditional, trust-minimized refund should attach +> no hook on such jobs. The forwarding flow, concretely (the `ForwardingClient` in the test suite demonstrates it): diff --git a/eip.md b/eip.md index 02430d5..54a7181 100644 --- a/eip.md +++ b/eip.md @@ -253,7 +253,7 @@ When `submit` or `reject` supersedes a pending claim, no claim-specific hook cal #### Hook security - Hooks are **trusted** contracts chosen by the client at job creation. A malicious or buggy hook can revert valid actions or execute arbitrary logic in callbacks. Clients SHOULD audit or use well-known hook implementations. -- **Liveness (BREAKING CHANGE):** A reverting hook can block all hookable actions for that job until `expiredAt`. This is by design — the hook is part of the job's policy. In earlier revisions of this specification `claimRefund` was deliberately **not** hookable, which gave an *unconditional* recovery guarantee: refunds after expiry could never be blocked. This revision makes `claimRefund` hookable (before + after) **intentionally and as a blocking callback**, to support a `job.client` that is a contract standing in for the real beneficiary (e.g. a smart account or router): the refund is paid to that contract and its `afterAction` callback (which runs *after* the refund transfer) atomically forwards the funds to the rightful end-client. The callback MUST be able to revert the whole refund, because a failed forward must not leave funds stranded in the intermediary client contract. The accepted trade-off is that the post-expiry refund is no longer unconditionally live: a buggy or fail-closed hook can revert and block it. Recovery then depends on (a) only attaching hooks that do not revert on `claimRefund`, and (b) an admin severing a misbehaving hook from in-flight jobs (`batchDetachHook` in the reference implementation), after which `claimRefund` succeeds with no callbacks. Deployments that do not need contract-client forwarding and require an unconditional post-expiry refund SHOULD attach no hook (or a non-reverting hook) on such jobs. +- **Liveness (BREAKING CHANGE):** A reverting hook can block all hookable actions for that job until `expiredAt`. This is by design — the hook is part of the job's policy. In earlier revisions of this specification `claimRefund` was deliberately **not** hookable, which gave an *unconditional* recovery guarantee: refunds after expiry could never be blocked. This revision makes `claimRefund` hookable (before + after) **intentionally and as a blocking callback**, to support a `job.client` that is a contract standing in for the real beneficiary (e.g. a smart account or router): the refund is paid to that contract and its `afterAction` callback (which runs *after* the refund transfer) atomically forwards the funds to the rightful end-client. The callback MUST be able to revert the whole refund, because a failed forward must not leave funds stranded in the intermediary client contract. The accepted trade-off is that the post-expiry refund is no longer unconditionally live: a buggy or fail-closed hook can revert and block it. The trust model handles this in two layers: **(1)** hooks are admin-whitelisted, and whitelisting is expected to include an **audit that the hook never blocks lifecycle paths** — it MUST NOT revert on `claimRefund` except for a genuine forward failure, and MUST NOT derive routing or authorization from the caller-supplied `optParams` on this permissionless path (a stranger may trigger `claimRefund`); **(2)** as an on-chain break-glass, the admin can `pause()` and `emergencyWithdraw` the escrow regardless of any hook, and `batchDetachHook` is a lighter option for a non-forwarding hook. Deployments that do not need contract-client forwarding and want an unconditional, trust-minimized refund SHOULD attach no hook (or a non-reverting hook) on such jobs. - **Atomicity:** After-callbacks run after state changes but within the same transaction. If an after-callback reverts, the entire transaction (including the core state change) is rolled back. This is intentional — it enables atomic multi-step flows (e.g. escrow funding + side token transfer must both succeed or both revert). - `onlyERC8183` modifiers on hooks are RECOMMENDED so that hook functions cannot be called directly by external actors. - Hooks SHOULD NOT be upgradeable after a job is created, as this would allow the hook to change behaviour mid-job. @@ -562,7 +562,7 @@ function DOMAIN_SEPARATOR() external view returns (bytes32); - **Tokens:** Use SafeERC-20 or equivalent for [ERC-20](./eip-20.md). - **Evaluator:** MUST be set at creation; if "client completes", pass `evaluator = client`. - **Hook gas limits** (for hooked implementations): Implementations SHOULD impose a gas limit on hook calls (e.g. `call{gas: HOOK_GAS_LIMIT}(...)`) to bound execution cost and prevent hooks from consuming unbounded gas. The specific limit is left to the implementation as gas costs vary across chains. -- Hook contracts are client-supplied and trusted by the client; implementations MUST NOT allow hooks to modify core escrow state directly. Where `claimRefund` is hookable (as in the reference implementation), a reverting hook can block refunds after expiry; the reference implementation mitigates this with hook whitelisting/ERC-165 checks at attach time and an admin `batchDetachHook` recovery path. Implementations requiring an unconditional post-expiry refund SHALL keep `claimRefund` non-hookable. +- Hook contracts are client-supplied and trusted by the client; implementations MUST NOT allow hooks to modify core escrow state directly. Where `claimRefund` is hookable (as in the reference implementation), a reverting hook can block refunds after expiry. The reference implementation accepts this under a governance trust model: hooks are admin-whitelisted only after an audit that they do not block lifecycle paths (and do not act on caller-supplied `optParams` on the permissionless `claimRefund` path), with admin `pause()` + `emergencyWithdraw` as the on-chain break-glass recovery (and `batchDetachHook` for non-forwarding hooks). Implementations requiring an unconditional, trust-minimized post-expiry refund SHALL keep `claimRefund` non-hookable. - Jobs that use **advanced hooks** (e.g. two‑phase escrow / fund‑transfer hooks that custody additional tokens) are expected to have **more revert paths and tighter coupling** to external logic than plain, non‑hooked Agentic Commerce jobs. Such hooks SHOULD be reserved for agents and users who understand and accept this trade‑off; for most simple jobs, a non‑hooked or policy‑only hook is RECOMMENDED. ## Copyright diff --git a/test/ERC8183.t.sol b/test/ERC8183.t.sol index b0c0a31..a8038ac 100644 --- a/test/ERC8183.t.sol +++ b/test/ERC8183.t.sol @@ -1617,6 +1617,38 @@ contract ERC8183Test is Test { assertEq(uint8(core.getJob(jobId).status), uint8(ERC8183.JobStatus.Expired)); } + // Break-glass: if a hook blocks the refund, admin pause() + emergencyWithdraw recovers the + // escrow regardless of the hook. This is the governance backstop for refund liveness. + function test_claimRefund_blockedByHook_adminEmergencyWithdrawRecovers() public { + SelectiveRevertHook hook = new SelectiveRevertHook(ERC8183.claimRefund.selector); + vm.prank(deployer); + core.setHookWhitelist(address(hook), true); + + vm.prank(client); + uint256 jobId = core.createJob(provider, evaluator, _futureExpiry(), "hooked", address(hook), 0, ""); + vm.prank(provider); + core.setBudget(jobId, address(usdc), TEN_USDC, ""); + vm.prank(client); + core.fund(jobId, address(usdc), TEN_USDC, ""); + + vm.warp(block.timestamp + 3601); + + // Hook blocks the permissionless refund. + vm.expectRevert(bytes("blocked")); + core.claimRefund(jobId, ""); + assertEq(usdc.balanceOf(address(core)), TEN_USDC); + + // Break-glass: admin pauses and withdraws the escrow to the client, bypassing the hook. + uint256 balBefore = usdc.balanceOf(client); + vm.startPrank(deployer); + core.pause(); + core.emergencyWithdraw(address(usdc), client, TEN_USDC); + vm.stopPrank(); + + assertEq(usdc.balanceOf(client), balBefore + TEN_USDC); + assertEq(usdc.balanceOf(address(core)), 0); + } + // A contract client forwards the refund to the real beneficiary in its afterAction hook. // This is the intended reason the claimRefund hook is blocking. function test_claimRefund_contractClientForwardsRefundAtomically() public { From 98453609ca34d27b3da182cc8ce530ba22d4554e Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 2 Jul 2026 15:00:35 +0800 Subject: [PATCH 12/32] docs: sync createJob hook sequence diagram with afterAction behavior --- docs/01-architecture.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/01-architecture.md b/docs/01-architecture.md index 2f965d7..835753d 100644 --- a/docs/01-architecture.md +++ b/docs/01-architecture.md @@ -101,8 +101,10 @@ sequenceDiagram participant P as Provider participant E as Evaluator - C->>AC: createJob(provider, evaluator, expiry, desc, hook, agentId) - Note over AC: Status: Open (hook stored, no callback) + C->>AC: createJob(provider, evaluator, expiry, desc, hook, agentId, optParams) + Note over AC: Status: Open (hook attached) + AC->>H: afterAction(jobId, createJob.selector, data) + Note over H: CAN revert to reject the job (no beforeAction on creation) P->>AC: setBudget(jobId, token, amount, optParams) AC->>H: beforeAction(jobId, setBudget.selector, data) From 3236045f9e4cbf734bfb11f835e5ec644c56ee48 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 2 Jul 2026 15:00:35 +0800 Subject: [PATCH 13/32] test: replace RevertingHook mock with SelectiveRevertHook --- test/ERC8183.t.sol | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/test/ERC8183.t.sol b/test/ERC8183.t.sol index a8038ac..45efca2 100644 --- a/test/ERC8183.t.sol +++ b/test/ERC8183.t.sol @@ -70,21 +70,6 @@ contract RecordingHook is IERC8183Hook { } } -/// @notice Reverts on every callback — used to prove hooks can gate transitions. -contract RevertingHook is IERC8183Hook { - function beforeAction(uint256, bytes4, bytes calldata) external pure override { - revert("blocked"); - } - - function afterAction(uint256, bytes4, bytes calldata) external pure override { - revert("blocked"); - } - - function supportsInterface(bytes4 id) external pure override returns (bool) { - return id == type(IERC8183Hook).interfaceId || id == type(IERC165).interfaceId; - } -} - /// @notice Reverts only on a single configured selector — lets other lifecycle calls /// (e.g. createJob/setBudget/fund) proceed while gating one specific action. contract SelectiveRevertHook is IERC8183Hook { @@ -1072,7 +1057,7 @@ contract ERC8183Test is Test { // a reverting afterAction rejects job creation entirely function test_createJob_afterHookRevertRejectsCreation() public { - RevertingHook hook = new RevertingHook(); + SelectiveRevertHook hook = new SelectiveRevertHook(ERC8183.createJob.selector); vm.prank(deployer); core.setHookWhitelist(address(hook), true); From dfcd5fadd9b4c3a63f2bf015910b4fc62f636a41 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 2 Jul 2026 15:34:38 +0800 Subject: [PATCH 14/32] feat: add admin forceRefund as job-scoped break-glass for hook-blocked refunds Refunds the client and expires the job in one atomic step with hook callbacks skipped. Eligibility is identical to claimRefund, so the admin gains no power beyond the permissionless path; requires pause(). Closes the double-refund window left by rescuing via raw emergencyWithdraw, which is now reserved for funds not attributed to any job. --- contracts/ERC8183.sol | 45 ++++++++++++++++++++----- test/ERC8183.t.sol | 78 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 109 insertions(+), 14 deletions(-) diff --git a/contracts/ERC8183.sol b/contracts/ERC8183.sol index 3a57631..6a6c695 100644 --- a/contracts/ERC8183.sol +++ b/contracts/ERC8183.sol @@ -928,13 +928,41 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable /// lifecycle paths (i.e. never revert on claimRefund except for a genuine /// forward failure) and to never derive routing/authorization from the /// caller-supplied optParams on this permissionless path. Break-glass recovery if - /// a hook still blocks a refund: admin pause() + emergencyWithdraw moves the - /// escrow out regardless of hooks; batchDetachHook is the lighter option for a - /// non-forwarding hook (after which claimRefund proceeds with no callbacks). + /// a hook still blocks a refund: batchDetachHook for a non-forwarding hook (after + /// which claimRefund proceeds with no callbacks), or admin forceRefund, which + /// refunds and expires the job in one step while bypassing hooks. emergencyWithdraw + /// is reserved for funds NOT attributed to any job (e.g. stray transfers) — using + /// it for a job-tied refund leaves the job Funded and refundable a second time. /// @param jobId The expired job to claim refund for /// @param optParams Hook-specific parameters (passed to before/after hooks) function claimRefund(uint256 jobId, bytes calldata optParams) external whenNotPaused nonReentrant { - Job storage job = jobs[jobId]; + Job storage job = _validateRefundEligibility(jobId); + + bytes memory data = abi.encode(msg.sender, optParams); + _beforeHook(job.hook, jobId, this.claimRefund.selector, data); + + _expireWithRefund(jobId, job); + + _afterHook(job.hook, jobId, this.claimRefund.selector, data); + } + + /// @notice Admin break-glass: refunds and expires a job whose hook blocks claimRefund, + /// bypassing hook callbacks. Requires the contract to be paused. + /// @dev Grants no power beyond the permissionless path: eligibility is exactly + /// claimRefund's (post-expiry, grace period elapsed for Submitted, pending claims + /// still block) — only the hook calls are skipped. Because the job transitions to + /// Expired atomically with the payout, a rescued job can never be refunded again, + /// which is why this MUST be used instead of emergencyWithdraw for job-tied funds + /// (emergencyWithdraw moves tokens without closing the job's ledger entry). + /// @param jobId The expired job to rescue + function forceRefund(uint256 jobId) external onlyRole(ADMIN_ROLE) whenPaused nonReentrant { + Job storage job = _validateRefundEligibility(jobId); + _expireWithRefund(jobId, job); + } + + /// @dev Shared eligibility checks for claimRefund/forceRefund. + function _validateRefundEligibility(uint256 jobId) internal view returns (Job storage job) { + job = jobs[jobId]; if (jobId == 0 || jobId > jobCounter) revert InvalidJob(); if (job.status != JobStatus.Open && job.status != JobStatus.Funded && job.status != JobStatus.Submitted) revert WrongStatus(); @@ -946,10 +974,11 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable } else { if (block.timestamp < job.expiredAt) revert WrongStatus(); } + } - bytes memory data = abi.encode(msg.sender, optParams); - _beforeHook(job.hook, jobId, this.claimRefund.selector, data); - + /// @dev Shared state transition for claimRefund/forceRefund: expire the job and pay the + /// unsettled remainder to the client. + function _expireWithRefund(uint256 jobId, Job storage job) internal { JobStatus prev = job.status; job.status = JobStatus.Expired; @@ -960,8 +989,6 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable } emit JobExpired(jobId); - - _afterHook(job.hook, jobId, this.claimRefund.selector, data); } function _distributeSettlement( diff --git a/test/ERC8183.t.sol b/test/ERC8183.t.sol index 45efca2..5993f9d 100644 --- a/test/ERC8183.t.sol +++ b/test/ERC8183.t.sol @@ -6,6 +6,7 @@ import {Vm} from "forge-std/Vm.sol"; import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; import {IAccessControl} from "@openzeppelin/contracts/access/IAccessControl.sol"; import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; +import {PausableUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/PausableUpgradeable.sol"; import {ERC8183} from "../contracts/ERC8183.sol"; import {IERC8183Hook} from "../contracts/IERC8183Hook.sol"; @@ -1602,9 +1603,10 @@ contract ERC8183Test is Test { assertEq(uint8(core.getJob(jobId).status), uint8(ERC8183.JobStatus.Expired)); } - // Break-glass: if a hook blocks the refund, admin pause() + emergencyWithdraw recovers the - // escrow regardless of the hook. This is the governance backstop for refund liveness. - function test_claimRefund_blockedByHook_adminEmergencyWithdrawRecovers() public { + // Break-glass: if a hook blocks the refund, admin pause() + forceRefund pays the client + // and expires the job in one step, bypassing the hook. This is the governance backstop + // for refund liveness. + function test_claimRefund_blockedByHook_adminForceRefundRecovers() public { SelectiveRevertHook hook = new SelectiveRevertHook(ERC8183.claimRefund.selector); vm.prank(deployer); core.setHookWhitelist(address(hook), true); @@ -1623,15 +1625,81 @@ contract ERC8183Test is Test { core.claimRefund(jobId, ""); assertEq(usdc.balanceOf(address(core)), TEN_USDC); - // Break-glass: admin pauses and withdraws the escrow to the client, bypassing the hook. + // Break-glass: admin pauses and force-refunds the job, bypassing the hook. uint256 balBefore = usdc.balanceOf(client); vm.startPrank(deployer); core.pause(); - core.emergencyWithdraw(address(usdc), client, TEN_USDC); + core.forceRefund(jobId); vm.stopPrank(); assertEq(usdc.balanceOf(client), balBefore + TEN_USDC); assertEq(usdc.balanceOf(address(core)), 0); + assertEq(uint8(core.getJob(jobId).status), uint8(ERC8183.JobStatus.Expired)); + } + + // Regression: a rescued job can never be refunded a second time — forceRefund expires the + // job atomically with the payout, so a later hook detach + claimRefund cannot double-pay + // out of other jobs' escrow. + function test_forceRefund_rescuedJobCannotBeRefundedAgain() public { + SelectiveRevertHook hook = new SelectiveRevertHook(ERC8183.claimRefund.selector); + vm.prank(deployer); + core.setHookWhitelist(address(hook), true); + + vm.prank(client); + uint256 jobId = core.createJob(provider, evaluator, _futureExpiry(), "hooked", address(hook), 0, ""); + vm.prank(provider); + core.setBudget(jobId, address(usdc), TEN_USDC, ""); + vm.prank(client); + core.fund(jobId, address(usdc), TEN_USDC, ""); + + vm.warp(block.timestamp + 3601); + + vm.startPrank(deployer); + core.pause(); + core.forceRefund(jobId); + core.unpause(); + + // Even after the hook is detached, the permissionless path cannot pay again. + uint256[] memory ids = new uint256[](1); + ids[0] = jobId; + core.batchDetachHook(ids); + vm.stopPrank(); + + vm.expectRevert(ERC8183.WrongStatus.selector); + core.claimRefund(jobId, ""); + } + + function test_forceRefund_revertsWhenNotPaused() public { + uint256 jobId = _createFundedJob(TEN_USDC); + vm.warp(block.timestamp + 3601); + + vm.prank(deployer); + vm.expectRevert(PausableUpgradeable.ExpectedPause.selector); + core.forceRefund(jobId); + } + + function test_forceRefund_revertsForNonAdmin() public { + uint256 jobId = _createFundedJob(TEN_USDC); + vm.warp(block.timestamp + 3601); + + vm.prank(deployer); + core.pause(); + + vm.prank(client); + vm.expectRevert(); + core.forceRefund(jobId); + } + + // forceRefund grants no power beyond claimRefund's eligibility: pre-expiry jobs cannot be + // force-expired by the admin. + function test_forceRefund_revertsBeforeExpiry() public { + uint256 jobId = _createFundedJob(TEN_USDC); + + vm.startPrank(deployer); + core.pause(); + vm.expectRevert(ERC8183.WrongStatus.selector); + core.forceRefund(jobId); + vm.stopPrank(); } // A contract client forwards the refund to the real beneficiary in its afterAction hook. From a6c0dea31fe80ea76459b46bf0f724fbc2df9e95 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 2 Jul 2026 15:34:38 +0800 Subject: [PATCH 15/32] docs: point break-glass guidance at forceRefund; scope emergencyWithdraw to stray funds --- README.md | 2 +- docs/02-hook-system.md | 8 ++++++-- eip.md | 4 ++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 076262e..1389806 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,7 @@ contracts/ - **Pending claim resolution** — after expiry, a Funded job with a pending provider claim cannot be force-refunded until the claim is approved, rejected, or withdrawn; if all parties stay idle, escrow remains parked - **Claim replay guard** — rejected claim hashes stay consumed, so providers must vary `cumulativeAmount`, the deliverable, or `optParams` to refile - **Authorization extension** — `ERC8183WithAuthorization` uses the base `ERC8183` EIP-712 domain so relayed entrypoints extend the same protocol identity; signers can call `cancelAuthorization` directly to burn one of their own outstanding nonces -- **Hook safety** — `claimRefund` is hookable and intentionally blocking: the refund is paid to `job.client` (which may be a contract) and the `afterAction` hook runs after the transfer so a contract-client can atomically forward funds to the real beneficiary. The trade-off (a reverting hook can block the permissionless refund) is handled by governance: hooks are whitelisted only after an audit that they don't block lifecycle paths, with admin `pause()` + `emergencyWithdraw` as the on-chain break-glass recovery. Pending claims must still be resolved before refund +- **Hook safety** — `claimRefund` is hookable and intentionally blocking: the refund is paid to `job.client` (which may be a contract) and the `afterAction` hook runs after the transfer so a contract-client can atomically forward funds to the real beneficiary. The trade-off (a reverting hook can block the permissionless refund) is handled by governance: hooks are whitelisted only after an audit that they don't block lifecycle paths, with admin `pause()` + `forceRefund` as the on-chain break-glass — it refunds the client and expires the job in one step, bypassing hooks, so a rescued job can never be refunded twice (`emergencyWithdraw` is reserved for funds not attributed to any job). Pending claims must still be resolved before refund See [docs/01-architecture.md](docs/01-architecture.md) for state machine and sequence diagrams. diff --git a/docs/02-hook-system.md b/docs/02-hook-system.md index 93302ba..c6755aa 100644 --- a/docs/02-hook-system.md +++ b/docs/02-hook-system.md @@ -77,8 +77,12 @@ signer; for the permissionless `claimRefund` it is `msg.sender` (whoever trigger > failure, and MUST NOT derive routing/authorization from the caller-supplied `optParams` > on this permissionless path (anyone can trigger `claimRefund`). > - **Break-glass recovery:** if a hook still blocks a refund, the admin can `pause()` and -> `emergencyWithdraw` the escrow regardless of any hook; `batchDetachHook` is the lighter -> option for a non-forwarding hook (after which `claimRefund` succeeds with no callbacks). +> `forceRefund` the job — same eligibility as `claimRefund` but with hook callbacks skipped, +> refunding the client and expiring the job atomically (so it can never be refunded twice); +> `batchDetachHook` is the lighter option for a non-forwarding hook (after which +> `claimRefund` succeeds with no callbacks). `emergencyWithdraw` is reserved for funds not +> attributed to any job (e.g. stray transfers) — it moves tokens without closing a job's +> ledger entry, so it MUST NOT be used for job-tied refunds. > > Only whitelist hooks you fully trust and have audited. Deployments that don't need > contract-client forwarding and want an unconditional, trust-minimized refund should attach diff --git a/eip.md b/eip.md index cbaaae5..11270ac 100644 --- a/eip.md +++ b/eip.md @@ -253,7 +253,7 @@ When `submit` or `reject` supersedes a pending claim, no claim-specific hook cal #### Hook security - Hooks are **trusted** contracts chosen by the client at job creation. A malicious or buggy hook can revert valid actions or execute arbitrary logic in callbacks. Clients SHOULD audit or use well-known hook implementations. -- **Liveness (BREAKING CHANGE):** A reverting hook can block all hookable actions for that job until `expiredAt`. This is by design — the hook is part of the job's policy. In earlier revisions of this specification `claimRefund` was deliberately **not** hookable, which gave an *unconditional* recovery guarantee: refunds after expiry could never be blocked. This revision makes `claimRefund` hookable (before + after) **intentionally and as a blocking callback**, to support a `job.client` that is a contract standing in for the real beneficiary (e.g. a smart account or router): the refund is paid to that contract and its `afterAction` callback (which runs *after* the refund transfer) atomically forwards the funds to the rightful end-client. The callback MUST be able to revert the whole refund, because a failed forward must not leave funds stranded in the intermediary client contract. The accepted trade-off is that the post-expiry refund is no longer unconditionally live: a buggy or fail-closed hook can revert and block it. The trust model handles this in two layers: **(1)** hooks are admin-whitelisted, and whitelisting is expected to include an **audit that the hook never blocks lifecycle paths** — it MUST NOT revert on `claimRefund` except for a genuine forward failure, and MUST NOT derive routing or authorization from the caller-supplied `optParams` on this permissionless path (a stranger may trigger `claimRefund`); **(2)** as an on-chain break-glass, the admin can `pause()` and `emergencyWithdraw` the escrow regardless of any hook, and `batchDetachHook` is a lighter option for a non-forwarding hook. Deployments that do not need contract-client forwarding and want an unconditional, trust-minimized refund SHOULD attach no hook (or a non-reverting hook) on such jobs. +- **Liveness (BREAKING CHANGE):** A reverting hook can block all hookable actions for that job until `expiredAt`. This is by design — the hook is part of the job's policy. In earlier revisions of this specification `claimRefund` was deliberately **not** hookable, which gave an *unconditional* recovery guarantee: refunds after expiry could never be blocked. This revision makes `claimRefund` hookable (before + after) **intentionally and as a blocking callback**, to support a `job.client` that is a contract standing in for the real beneficiary (e.g. a smart account or router): the refund is paid to that contract and its `afterAction` callback (which runs *after* the refund transfer) atomically forwards the funds to the rightful end-client. The callback MUST be able to revert the whole refund, because a failed forward must not leave funds stranded in the intermediary client contract. The accepted trade-off is that the post-expiry refund is no longer unconditionally live: a buggy or fail-closed hook can revert and block it. The trust model handles this in two layers: **(1)** hooks are admin-whitelisted, and whitelisting is expected to include an **audit that the hook never blocks lifecycle paths** — it MUST NOT revert on `claimRefund` except for a genuine forward failure, and MUST NOT derive routing or authorization from the caller-supplied `optParams` on this permissionless path (a stranger may trigger `claimRefund`); **(2)** as an on-chain break-glass, the admin can `pause()` and `forceRefund` the job — identical eligibility to `claimRefund` with the hook callbacks skipped, refunding the client and expiring the job atomically so a rescued job can never be refunded twice — and `batchDetachHook` is a lighter option for a non-forwarding hook (`emergencyWithdraw` remains reserved for funds not attributed to any job). Deployments that do not need contract-client forwarding and want an unconditional, trust-minimized refund SHOULD attach no hook (or a non-reverting hook) on such jobs. - **Atomicity:** After-callbacks run after state changes but within the same transaction. If an after-callback reverts, the entire transaction (including the core state change) is rolled back. This is intentional — it enables atomic multi-step flows (e.g. escrow funding + side token transfer must both succeed or both revert). - `onlyERC8183` modifiers on hooks are RECOMMENDED so that hook functions cannot be called directly by external actors. - Hooks SHOULD NOT be upgradeable after a job is created, as this would allow the hook to change behaviour mid-job. @@ -563,7 +563,7 @@ function DOMAIN_SEPARATOR() external view returns (bytes32); - **Tokens:** Use SafeERC-20 or equivalent for [ERC-20](./eip-20.md). - **Evaluator:** MUST be set at creation; if "client completes", pass `evaluator = client`. - **Hook gas limits** (for hooked implementations): Implementations SHOULD impose a gas limit on hook calls (e.g. `call{gas: HOOK_GAS_LIMIT}(...)`) to bound execution cost and prevent hooks from consuming unbounded gas. The specific limit is left to the implementation as gas costs vary across chains. -- Hook contracts are client-supplied and trusted by the client; implementations MUST NOT allow hooks to modify core escrow state directly. Where `claimRefund` is hookable (as in the reference implementation), a reverting hook can block refunds after expiry. The reference implementation accepts this under a governance trust model: hooks are admin-whitelisted only after an audit that they do not block lifecycle paths (and do not act on caller-supplied `optParams` on the permissionless `claimRefund` path), with admin `pause()` + `emergencyWithdraw` as the on-chain break-glass recovery (and `batchDetachHook` for non-forwarding hooks). Implementations requiring an unconditional, trust-minimized post-expiry refund SHALL keep `claimRefund` non-hookable. +- Hook contracts are client-supplied and trusted by the client; implementations MUST NOT allow hooks to modify core escrow state directly. Where `claimRefund` is hookable (as in the reference implementation), a reverting hook can block refunds after expiry. The reference implementation accepts this under a governance trust model: hooks are admin-whitelisted only after an audit that they do not block lifecycle paths (and do not act on caller-supplied `optParams` on the permissionless `claimRefund` path), with an admin `forceRefund` (refund + expire in one step, hooks bypassed, only under `pause()`) as the on-chain break-glass recovery (and `batchDetachHook` for non-forwarding hooks). A rescue MUST close the job's accounting atomically with the payout so a rescued job cannot be refunded twice; a free-form escape hatch like `emergencyWithdraw` SHOULD be reserved for funds not attributed to any job. Implementations requiring an unconditional, trust-minimized post-expiry refund SHALL keep `claimRefund` non-hookable. - Jobs that use **advanced hooks** (e.g. two‑phase escrow / fund‑transfer hooks that custody additional tokens) are expected to have **more revert paths and tighter coupling** to external logic than plain, non‑hooked Agentic Commerce jobs. Such hooks SHOULD be reserved for agents and users who understand and accept this trade‑off; for most simple jobs, a non‑hooked or policy‑only hook is RECOMMENDED. ## Copyright From 136ddf4b7a8ede2a92f4e540339a405ae9fcf443 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 2 Jul 2026 16:24:40 +0800 Subject: [PATCH 16/32] feat: add recipient override to forceRefund A contract-client that depended on its now-blocked forwarding hook may be unable to receive or move the refund, so paying it would re-strand the funds outside the escrow. The override adds no destination power the admin does not already hold via emergencyWithdraw; address(0) defaults to job.client. --- README.md | 2 +- contracts/ERC8183.sol | 33 ++++++++++++++++++++------------- docs/02-hook-system.md | 5 ++++- eip.md | 2 +- test/ERC8183.t.sol | 38 +++++++++++++++++++++++++++++++++----- 5 files changed, 59 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 1389806..6230aa5 100644 --- a/README.md +++ b/README.md @@ -68,7 +68,7 @@ contracts/ - **Pending claim resolution** — after expiry, a Funded job with a pending provider claim cannot be force-refunded until the claim is approved, rejected, or withdrawn; if all parties stay idle, escrow remains parked - **Claim replay guard** — rejected claim hashes stay consumed, so providers must vary `cumulativeAmount`, the deliverable, or `optParams` to refile - **Authorization extension** — `ERC8183WithAuthorization` uses the base `ERC8183` EIP-712 domain so relayed entrypoints extend the same protocol identity; signers can call `cancelAuthorization` directly to burn one of their own outstanding nonces -- **Hook safety** — `claimRefund` is hookable and intentionally blocking: the refund is paid to `job.client` (which may be a contract) and the `afterAction` hook runs after the transfer so a contract-client can atomically forward funds to the real beneficiary. The trade-off (a reverting hook can block the permissionless refund) is handled by governance: hooks are whitelisted only after an audit that they don't block lifecycle paths, with admin `pause()` + `forceRefund` as the on-chain break-glass — it refunds the client and expires the job in one step, bypassing hooks, so a rescued job can never be refunded twice (`emergencyWithdraw` is reserved for funds not attributed to any job). Pending claims must still be resolved before refund +- **Hook safety** — `claimRefund` is hookable and intentionally blocking: the refund is paid to `job.client` (which may be a contract) and the `afterAction` hook runs after the transfer so a contract-client can atomically forward funds to the real beneficiary. The trade-off (a reverting hook can block the permissionless refund) is handled by governance: hooks are whitelisted only after an audit that they don't block lifecycle paths, with admin `pause()` + `forceRefund` as the on-chain break-glass — it pays the refund (to the client, or to an admin-chosen recipient when the client is itself an intermediary contract that cannot receive funds without its blocked hook) and expires the job in one step, bypassing hooks, so a rescued job can never be refunded twice (`emergencyWithdraw` is reserved for funds not attributed to any job). Pending claims must still be resolved before refund See [docs/01-architecture.md](docs/01-architecture.md) for state machine and sequence diagrams. diff --git a/contracts/ERC8183.sol b/contracts/ERC8183.sol index 6a6c695..020e1b3 100644 --- a/contracts/ERC8183.sol +++ b/contracts/ERC8183.sol @@ -941,23 +941,30 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable bytes memory data = abi.encode(msg.sender, optParams); _beforeHook(job.hook, jobId, this.claimRefund.selector, data); - _expireWithRefund(jobId, job); + _expireWithRefund(jobId, job, job.client); _afterHook(job.hook, jobId, this.claimRefund.selector, data); } /// @notice Admin break-glass: refunds and expires a job whose hook blocks claimRefund, /// bypassing hook callbacks. Requires the contract to be paused. - /// @dev Grants no power beyond the permissionless path: eligibility is exactly - /// claimRefund's (post-expiry, grace period elapsed for Submitted, pending claims - /// still block) — only the hook calls are skipped. Because the job transitions to - /// Expired atomically with the payout, a rescued job can never be refunded again, - /// which is why this MUST be used instead of emergencyWithdraw for job-tied funds - /// (emergencyWithdraw moves tokens without closing the job's ledger entry). + /// @dev Grants no state-transition power beyond the permissionless path: eligibility is + /// exactly claimRefund's (post-expiry, grace period elapsed for Submitted, pending + /// claims still block) — only the hook calls are skipped. Because the job + /// transitions to Expired atomically with the payout, a rescued job can never be + /// refunded again, which is why this MUST be used instead of emergencyWithdraw for + /// job-tied funds (emergencyWithdraw moves tokens without closing the job's + /// ledger entry). + /// The destination override exists because job.client may itself be an + /// intermediary contract (smart account, router) that cannot receive or forward + /// funds without its — now blocked — hook: paying it would re-strand the refund + /// outside the escrow's reach. This grants no destination power the admin does + /// not already hold via emergencyWithdraw's free-form (token, to, amount). /// @param jobId The expired job to rescue - function forceRefund(uint256 jobId) external onlyRole(ADMIN_ROLE) whenPaused nonReentrant { + /// @param to Refund recipient; address(0) defaults to job.client + function forceRefund(uint256 jobId, address to) external onlyRole(ADMIN_ROLE) whenPaused nonReentrant { Job storage job = _validateRefundEligibility(jobId); - _expireWithRefund(jobId, job); + _expireWithRefund(jobId, job, to == address(0) ? job.client : to); } /// @dev Shared eligibility checks for claimRefund/forceRefund. @@ -977,15 +984,15 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable } /// @dev Shared state transition for claimRefund/forceRefund: expire the job and pay the - /// unsettled remainder to the client. - function _expireWithRefund(uint256 jobId, Job storage job) internal { + /// unsettled remainder to `recipient` (always job.client on the permissionless path). + function _expireWithRefund(uint256 jobId, Job storage job, address recipient) internal { JobStatus prev = job.status; job.status = JobStatus.Expired; uint256 refundAmount = job.budget - job.settledAmount; if (refundAmount > 0 && (prev == JobStatus.Funded || prev == JobStatus.Submitted)) { - IERC20(job.paymentToken).safeTransfer(job.client, refundAmount); - emit Refunded(jobId, job.client, refundAmount); + IERC20(job.paymentToken).safeTransfer(recipient, refundAmount); + emit Refunded(jobId, recipient, refundAmount); } emit JobExpired(jobId); diff --git a/docs/02-hook-system.md b/docs/02-hook-system.md index c6755aa..b061419 100644 --- a/docs/02-hook-system.md +++ b/docs/02-hook-system.md @@ -78,7 +78,10 @@ signer; for the permissionless `claimRefund` it is `msg.sender` (whoever trigger > on this permissionless path (anyone can trigger `claimRefund`). > - **Break-glass recovery:** if a hook still blocks a refund, the admin can `pause()` and > `forceRefund` the job — same eligibility as `claimRefund` but with hook callbacks skipped, -> refunding the client and expiring the job atomically (so it can never be refunded twice); +> paying the refund and expiring the job atomically (so it can never be refunded twice). The +> recipient defaults to the client but can be overridden: a contract-client that depended on +> its now-blocked hook to forward funds may be unable to receive them, and paying it would +> re-strand the refund (the override adds no power the admin lacks via `emergencyWithdraw`); > `batchDetachHook` is the lighter option for a non-forwarding hook (after which > `claimRefund` succeeds with no callbacks). `emergencyWithdraw` is reserved for funds not > attributed to any job (e.g. stray transfers) — it moves tokens without closing a job's diff --git a/eip.md b/eip.md index 11270ac..5f574fe 100644 --- a/eip.md +++ b/eip.md @@ -253,7 +253,7 @@ When `submit` or `reject` supersedes a pending claim, no claim-specific hook cal #### Hook security - Hooks are **trusted** contracts chosen by the client at job creation. A malicious or buggy hook can revert valid actions or execute arbitrary logic in callbacks. Clients SHOULD audit or use well-known hook implementations. -- **Liveness (BREAKING CHANGE):** A reverting hook can block all hookable actions for that job until `expiredAt`. This is by design — the hook is part of the job's policy. In earlier revisions of this specification `claimRefund` was deliberately **not** hookable, which gave an *unconditional* recovery guarantee: refunds after expiry could never be blocked. This revision makes `claimRefund` hookable (before + after) **intentionally and as a blocking callback**, to support a `job.client` that is a contract standing in for the real beneficiary (e.g. a smart account or router): the refund is paid to that contract and its `afterAction` callback (which runs *after* the refund transfer) atomically forwards the funds to the rightful end-client. The callback MUST be able to revert the whole refund, because a failed forward must not leave funds stranded in the intermediary client contract. The accepted trade-off is that the post-expiry refund is no longer unconditionally live: a buggy or fail-closed hook can revert and block it. The trust model handles this in two layers: **(1)** hooks are admin-whitelisted, and whitelisting is expected to include an **audit that the hook never blocks lifecycle paths** — it MUST NOT revert on `claimRefund` except for a genuine forward failure, and MUST NOT derive routing or authorization from the caller-supplied `optParams` on this permissionless path (a stranger may trigger `claimRefund`); **(2)** as an on-chain break-glass, the admin can `pause()` and `forceRefund` the job — identical eligibility to `claimRefund` with the hook callbacks skipped, refunding the client and expiring the job atomically so a rescued job can never be refunded twice — and `batchDetachHook` is a lighter option for a non-forwarding hook (`emergencyWithdraw` remains reserved for funds not attributed to any job). Deployments that do not need contract-client forwarding and want an unconditional, trust-minimized refund SHOULD attach no hook (or a non-reverting hook) on such jobs. +- **Liveness (BREAKING CHANGE):** A reverting hook can block all hookable actions for that job until `expiredAt`. This is by design — the hook is part of the job's policy. In earlier revisions of this specification `claimRefund` was deliberately **not** hookable, which gave an *unconditional* recovery guarantee: refunds after expiry could never be blocked. This revision makes `claimRefund` hookable (before + after) **intentionally and as a blocking callback**, to support a `job.client` that is a contract standing in for the real beneficiary (e.g. a smart account or router): the refund is paid to that contract and its `afterAction` callback (which runs *after* the refund transfer) atomically forwards the funds to the rightful end-client. The callback MUST be able to revert the whole refund, because a failed forward must not leave funds stranded in the intermediary client contract. The accepted trade-off is that the post-expiry refund is no longer unconditionally live: a buggy or fail-closed hook can revert and block it. The trust model handles this in two layers: **(1)** hooks are admin-whitelisted, and whitelisting is expected to include an **audit that the hook never blocks lifecycle paths** — it MUST NOT revert on `claimRefund` except for a genuine forward failure, and MUST NOT derive routing or authorization from the caller-supplied `optParams` on this permissionless path (a stranger may trigger `claimRefund`); **(2)** as an on-chain break-glass, the admin can `pause()` and `forceRefund` the job — identical eligibility to `claimRefund` with the hook callbacks skipped, paying the refund (to the client by default, or to an admin-chosen recipient when the client is an intermediary contract that cannot receive funds without its blocked hook) and expiring the job atomically so a rescued job can never be refunded twice — and `batchDetachHook` is a lighter option for a non-forwarding hook (`emergencyWithdraw` remains reserved for funds not attributed to any job). Deployments that do not need contract-client forwarding and want an unconditional, trust-minimized refund SHOULD attach no hook (or a non-reverting hook) on such jobs. - **Atomicity:** After-callbacks run after state changes but within the same transaction. If an after-callback reverts, the entire transaction (including the core state change) is rolled back. This is intentional — it enables atomic multi-step flows (e.g. escrow funding + side token transfer must both succeed or both revert). - `onlyERC8183` modifiers on hooks are RECOMMENDED so that hook functions cannot be called directly by external actors. - Hooks SHOULD NOT be upgradeable after a job is created, as this would allow the hook to change behaviour mid-job. diff --git a/test/ERC8183.t.sol b/test/ERC8183.t.sol index 5993f9d..8c08724 100644 --- a/test/ERC8183.t.sol +++ b/test/ERC8183.t.sol @@ -1629,7 +1629,7 @@ contract ERC8183Test is Test { uint256 balBefore = usdc.balanceOf(client); vm.startPrank(deployer); core.pause(); - core.forceRefund(jobId); + core.forceRefund(jobId, address(0)); vm.stopPrank(); assertEq(usdc.balanceOf(client), balBefore + TEN_USDC); @@ -1656,7 +1656,7 @@ contract ERC8183Test is Test { vm.startPrank(deployer); core.pause(); - core.forceRefund(jobId); + core.forceRefund(jobId, address(0)); core.unpause(); // Even after the hook is detached, the permissionless path cannot pay again. @@ -1669,13 +1669,41 @@ contract ERC8183Test is Test { core.claimRefund(jobId, ""); } + // The destination override: when job.client is an intermediary contract whose (blocked) + // forwarding hook was the only way funds could move onward, the admin can pay the + // ultimate beneficiary directly instead of re-stranding the refund in the intermediary. + function test_forceRefund_canRedirectToChosenRecipient() public { + SelectiveRevertHook hook = new SelectiveRevertHook(ERC8183.claimRefund.selector); + vm.prank(deployer); + core.setHookWhitelist(address(hook), true); + + vm.prank(client); + uint256 jobId = core.createJob(provider, evaluator, _futureExpiry(), "hooked", address(hook), 0, ""); + vm.prank(provider); + core.setBudget(jobId, address(usdc), TEN_USDC, ""); + vm.prank(client); + core.fund(jobId, address(usdc), TEN_USDC, ""); + + vm.warp(block.timestamp + 3601); + + address beneficiary = makeAddr("ultimateBeneficiary"); + vm.startPrank(deployer); + core.pause(); + core.forceRefund(jobId, beneficiary); + vm.stopPrank(); + + assertEq(usdc.balanceOf(beneficiary), TEN_USDC); + assertEq(usdc.balanceOf(address(core)), 0); + assertEq(uint8(core.getJob(jobId).status), uint8(ERC8183.JobStatus.Expired)); + } + function test_forceRefund_revertsWhenNotPaused() public { uint256 jobId = _createFundedJob(TEN_USDC); vm.warp(block.timestamp + 3601); vm.prank(deployer); vm.expectRevert(PausableUpgradeable.ExpectedPause.selector); - core.forceRefund(jobId); + core.forceRefund(jobId, address(0)); } function test_forceRefund_revertsForNonAdmin() public { @@ -1687,7 +1715,7 @@ contract ERC8183Test is Test { vm.prank(client); vm.expectRevert(); - core.forceRefund(jobId); + core.forceRefund(jobId, address(0)); } // forceRefund grants no power beyond claimRefund's eligibility: pre-expiry jobs cannot be @@ -1698,7 +1726,7 @@ contract ERC8183Test is Test { vm.startPrank(deployer); core.pause(); vm.expectRevert(ERC8183.WrongStatus.selector); - core.forceRefund(jobId); + core.forceRefund(jobId, address(0)); vm.stopPrank(); } From 07232e42ff17162edc143ea332cf2ec0f209836d Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 16 Jul 2026 12:05:49 +0800 Subject: [PATCH 17/32] docs: revise eip.md wording and add internal cross-links - restore optParams bullet to opaque-payload form (hash-binding stays documented under Claim Settlement); drop stale 'hook only' clause since optParams also reaches IDisburser receivers - drop advisory fee-rounding note; per-delta computation is already normative - clarify under Claim interactions that complete never encounters a pending claim (claims are Funded-only; submit supersedes first) - rename settlement paths to client-initiated / provider-initiated and propagate the vocabulary - hyperlink all intra-document section references --- eip.md | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/eip.md b/eip.md index 5f574fe..20c7012 100644 --- a/eip.md +++ b/eip.md @@ -33,7 +33,7 @@ A **job** has exactly one of six states: | State | Meaning | | ------------- | ----------------------------------------------------------------------------------------------------------------- | | **Open** | Created; budget not yet set or not yet funded. Client may set budget, then fund or reject. | -| **Funded** | Budget escrowed. Provider may submit work or file a settlement claim; client may settle partial amounts directly or approve/reject a pending claim; evaluator may approve/reject a pending claim or reject the job. After `expiredAt`, anyone may trigger refund of the unsettled remainder (`budget - settledAmount`, see Job Data), provided no claim is pending. | +| **Funded** | Budget escrowed. Provider may submit work or file a settlement claim; client may settle partial amounts directly or approve/reject a pending claim; evaluator may approve/reject a pending claim or reject the job. After `expiredAt`, anyone may trigger refund of the unsettled remainder (`budget - settledAmount`, see [Job Data](#job-data)), provided no claim is pending. | | **Submitted** | Provider has submitted work. Only evaluator may complete or reject. After `expiredAt + EVALUATION_GRACE_PERIOD`, anyone may trigger refund. | | **Completed** | Terminal. Unsettled remainder released to provider (minus optional fees). | | **Rejected** | Terminal. Unsettled remainder refunded to client. | @@ -54,7 +54,7 @@ Allowed transitions: No other transitions are valid. -Settlement claims (see Claim Settlement below) do not introduce job states: all claim activity occurs while the job is Funded, and at most one claim is pending per job at any time. +Settlement claims (see [Claim Settlement](#claim-settlement) below) do not introduce job states: all claim activity occurs while the job is Funded, and at most one claim is pending per job at any time. ### Roles @@ -66,16 +66,16 @@ Settlement claims (see Claim Settlement below) do not introduce job states: all Each job SHALL have at least: -- `client`, `provider`, `evaluator` (addresses). **Provider MAY be zero at creation** (see Optional provider below). +- `client`, `provider`, `evaluator` (addresses). **Provider MAY be zero at creation** (see [Optional provider](#optional-provider-set-later) below). - `description` (string) — set at creation (e.g. job brief, scope reference). - `budget` (uint256) - `expiredAt` (uint256 timestamp) - `status` (Open | Funded | Submitted | Completed | Rejected | Expired) - `paymentToken` (address) — the [ERC-20](./eip-20.md) token used for payment on this job, set via `setBudget`. -- `hook` (address) — OPTIONAL. External hook contract called before and after core functions (see Hooks below). MAY be `address(0)` (no hook). +- `hook` (address) — OPTIONAL. External hook contract called before and after core functions (see [Hooks](#hooks-optional) below). MAY be `address(0)` (no hook). - `payoutReceiver` (address) — OPTIONAL. Provider-managed payout recipient. MAY be `address(0)` to pay the provider directly. Set via `setPayoutReceiver`. - `providerAgentId` (uint256) — OPTIONAL. When non-zero, references an agent identity in an [ERC-8004](./eip-8004.md) registry, enabling on-chain identity binding for reputation. Set via `setProvider` (or at creation if provider is known). Default `0` (unset). -- `settledAmount` (uint256) — cumulative gross amount already released through claim settlements (see Claim Settlement below). Initially `0`. MUST be strictly monotonically increasing and MUST NOT exceed `budget`. +- `settledAmount` (uint256) — cumulative gross amount already released through claim settlements (see [Claim Settlement](#claim-settlement) below). Initially `0`. MUST be strictly monotonically increasing and MUST NOT exceed `budget`. All payout and refund computations in this specification are defined over the **unsettled remainder** `budget - settledAmount`. For jobs that never use claim settlement, `settledAmount` remains `0` and every formula reduces to the full budget. @@ -107,9 +107,9 @@ Called by provider only. SHALL revert if caller is not the job's provider, or if - **complete(jobId, reason, optParams?)** Called by evaluator only. SHALL revert if job is not Submitted or caller is not the job's evaluator. SHALL set status to Completed. SHALL transfer the unsettled remainder (`budget - settledAmount`) to the provider-side payout recipient, minus optional platform fee to a configurable treasury and optional evaluator fee paid to the evaluator address. `reason` MAY be `bytes32(0)` or an attestation hash (OPTIONAL). SHALL emit an event including `reason` if provided. `optParams` forwarded to hook if set. - **reject(jobId, reason, optParams?)** -Called by **client or provider when job is Open**, or by **evaluator when job is Funded or Submitted**. SHALL revert if job is not Open, Funded, or Submitted, or if the caller is not authorised for the current status. SHALL set status to Rejected. If Funded or Submitted, SHALL refund the unsettled remainder (`budget - settledAmount`) to client. If a settlement claim is pending, SHALL clear it (see Claim interactions below). `reason` OPTIONAL. SHALL emit an event including `reason` and the caller (rejector) if provided. `optParams` forwarded to hook if set. +Called by **client or provider when job is Open**, or by **evaluator when job is Funded or Submitted**. SHALL revert if job is not Open, Funded, or Submitted, or if the caller is not authorised for the current status. SHALL set status to Rejected. If Funded or Submitted, SHALL refund the unsettled remainder (`budget - settledAmount`) to client. If a settlement claim is pending, SHALL clear it (see [Claim interactions](#claim-interactions) below). `reason` OPTIONAL. SHALL emit an event including `reason` and the caller (rejector) if provided. `optParams` forwarded to hook if set. - **claimRefund(jobId, optParams)** -Callable by anyone when status is Open, Funded, or Submitted. SHALL revert if status is Open or Funded and `block.timestamp < job.expiredAt`. SHALL revert if status is Submitted and `block.timestamp < job.expiredAt + EVALUATION_GRACE_PERIOD` (the grace period protects an evaluator who is mid-review from being censored by a third-party refund call; implementations MAY set the grace period length or omit it). SHALL revert if a settlement claim is pending on a non-Submitted job (claims can only arise while Funded) — pending claims MUST be resolved (approved, rejected, or withdrawn) before an expiry refund. SHALL set status to Expired. If the prior status was Funded or Submitted, SHALL transfer the unsettled remainder (`budget - settledAmount`), if non-zero, to the client. MAY be hookable; if hookable, the `beforeAction`/`afterAction` callbacks fire around the refund and a reverting hook can block it (see the liveness caveat under **Hook security**). Implementations requiring an unconditional post-expiry refund SHALL keep `claimRefund` non-hookable. +Callable by anyone when status is Open, Funded, or Submitted. SHALL revert if status is Open or Funded and `block.timestamp < job.expiredAt`. SHALL revert if status is Submitted and `block.timestamp < job.expiredAt + EVALUATION_GRACE_PERIOD` (the grace period protects an evaluator who is mid-review from being censored by a third-party refund call; implementations MAY set the grace period length or omit it). SHALL revert if a settlement claim is pending on a non-Submitted job (claims can only arise while Funded) — pending claims MUST be resolved (approved, rejected, or withdrawn) before an expiry refund. SHALL set status to Expired. If the prior status was Funded or Submitted, SHALL transfer the unsettled remainder (`budget - settledAmount`), if non-zero, to the client. MAY be hookable; if hookable, the `beforeAction`/`afterAction` callbacks fire around the refund and a reverting hook can block it (see the liveness caveat under [**Hook security**](#hook-security)). Implementations requiring an unconditional post-expiry refund SHALL keep `claimRefund` non-hookable. > **Design rationale — refund forwarding for contract clients.** `claimRefund` is > intentionally hookable *and* its callbacks are intentionally **blocking**, to support a @@ -128,7 +128,7 @@ Callable by anyone when status is Open, Funded, or Submitted. SHALL revert if st > refund into the intermediary while silently dropping the forward. > > The accepted cost is that the post-expiry refund is no longer *unconditionally* live: a -> buggy or fail-closed hook can revert and block it (see **Hook security**). Deployments that +> buggy or fail-closed hook can revert and block it (see [**Hook security**](#hook-security)). Deployments that > do not need contract-client forwarding SHOULD attach no hook (or a hook that never reverts > on the `claimRefund` selector) so the refund stays unconditional. @@ -136,10 +136,10 @@ Callable by anyone when status is Open, Funded, or Submitted. SHALL revert if st While a job is Funded, escrow MAY be released incrementally through **claim settlement**, ahead of (or instead of) terminal completion. Two settlement paths share one ledger: -- the **direct path**: the client unilaterally settles an amount to the provider (`settleClaim`); -- the **claim path**: the provider files a pending claim (`submitClaim`) which the client or evaluator approves (`approveClaim`) or any of the three parties rejects (`rejectClaim`). +- the **client-initiated path**: the client unilaterally settles an amount to the provider (`settleClaim`); +- the **provider-initiated path**: the provider files a pending claim (`submitClaim`) which the client or evaluator approves (`approveClaim`) or any of the three parties rejects (`rejectClaim`). -All settlement functions take a **cumulative amount** — the total gross amount the caller asserts should have been released to date — never a delta. Each settlement SHALL release exactly `cumulativeAmount - settledAmount`, then set `settledAmount = cumulativeAmount`. A settlement SHALL revert if `cumulativeAmount <= settledAmount` (no new settlement) or `cumulativeAmount > budget` (exceeds escrow). Because both paths write the same strictly-increasing `settledAmount`, no interleaving of direct settlements, claim approvals, completion, rejection, and refund can release more than `budget` in total, and the delta paid by a claim approval can only shrink between submission and approval, never grow. +All settlement functions take a **cumulative amount** — the total gross amount the caller asserts should have been released to date — never a delta. Each settlement SHALL release exactly `cumulativeAmount - settledAmount`, then set `settledAmount = cumulativeAmount`. A settlement SHALL revert if `cumulativeAmount <= settledAmount` (no new settlement) or `cumulativeAmount > budget` (exceeds escrow). Because both paths write the same strictly-increasing `settledAmount`, no interleaving of client-initiated settlements, claim approvals, completion, rejection, and refund can release more than `budget` in total, and the delta paid by a claim approval can only shrink between submission and approval, never grow. At most one claim is pending per job. Implementations SHALL bind the pending claim by a hash over `(cumulativeAmount, deliverable, optParams)` so that approval and rejection require presenting the exact claim contents. @@ -148,15 +148,16 @@ Called by **provider** only. Files a pending claim against a Funded job; moves n - **settleClaim(jobId, cumulativeAmount, deliverable, optParams?)** Called by **client** only. Immediate unilateral settlement: SHALL revert if the job is not Funded, has expired, `cumulativeAmount <= settledAmount`, or `cumulativeAmount > budget`. SHALL set `settledAmount = cumulativeAmount` and distribute the delta to the provider-side payout recipient (minus optional platform/evaluator fees). `deliverable` is the client's settlement attestation, not a verified provider claim. A pending claim SHALL NOT block this function (streaming settlement); settling reduces the delta any subsequent approval of that claim would pay. SHALL emit Settled and ClaimSettled. `optParams` forwarded to hook if set. - **approveClaim(jobId, cumulativeAmount, deliverable, optParams?)** -Called by **client or evaluator**. SHALL revert if the job is not Funded, no claim is pending, or the arguments do not exactly match the pending claim — the evaluator MUST NOT be able to release amounts the provider did not claim. SHALL revert if `cumulativeAmount <= settledAmount` (e.g. the claim was already covered by direct settlement) or `cumulativeAmount > budget`. SHALL consume the pending claim, set `settledAmount = cumulativeAmount`, and distribute the delta to the provider-side payout recipient (minus optional fees). Approval is NOT subject to an expiry check: a claim filed before expiry remains approvable, consistent with the evaluation grace period philosophy. SHALL emit Settled and ClaimApproved. `optParams` forwarded to hook if set. +Called by **client or evaluator**. SHALL revert if the job is not Funded, no claim is pending, or the arguments do not exactly match the pending claim — the evaluator MUST NOT be able to release amounts the provider did not claim. SHALL revert if `cumulativeAmount <= settledAmount` (e.g. the claim was already covered by a client-initiated settlement) or `cumulativeAmount > budget`. SHALL consume the pending claim, set `settledAmount = cumulativeAmount`, and distribute the delta to the provider-side payout recipient (minus optional fees). Approval is NOT subject to an expiry check: a claim filed before expiry remains approvable, consistent with the evaluation grace period philosophy. SHALL emit Settled and ClaimApproved. `optParams` forwarded to hook if set. - **rejectClaim(jobId, cumulativeAmount, deliverable, reason, optParams?)** Called by **client, evaluator, or provider** (the provider withdrawing their own stale claim, e.g. to file a corrected one). SHALL revert if the job is not Funded, no claim is pending, or the arguments do not exactly match the pending claim. SHALL consume the pending claim without moving funds. The consumed claim tuple SHALL remain unfilable — a corrected claim must differ in `cumulativeAmount`, `deliverable`, or `optParams`. SHALL emit ClaimRejected. `optParams` forwarded to hook if set. #### Claim interactions - **submit** SHALL supersede any pending claim — the provider is electing the full-completion path for the entire remainder — and SHOULD emit ClaimRejected with a supersession reason so hooks and indexers observe a closed claim lifecycle. +- **complete** requires no supersession logic: claims can only be filed while the job is Funded, and `complete` is callable only once the job is Submitted — by which point `submit` has already superseded any pending claim, so `complete` can never encounter one. - Job-level **reject** SHALL also clear any pending claim (emitting ClaimRejected) before transitioning the job to Rejected. -- **claimRefund** SHALL revert while a claim is pending (claims can only arise while Funded); the claim must be approved, rejected, or withdrawn first. For non-hooked jobs the client can always unblock a refund in two transactions (`rejectClaim`, then `claimRefund`); for hooked jobs, a reverting `rejectClaim` hook can block this path (see Hook security). +- **claimRefund** SHALL revert while a claim is pending (claims can only arise while Funded); the claim must be approved, rejected, or withdrawn first. For non-hooked jobs the client can always unblock a refund in two transactions (`rejectClaim`, then `claimRefund`); for hooked jobs, a reverting `rejectClaim` hook can block this path (see [Hook security](#hook-security)). - Implementations SHALL update `settledAmount` and clear the pending claim **before** any token transfers (checks-effects-interactions). ### Attestation @@ -166,7 +167,7 @@ Called by **client, evaluator, or provider** (the provider withdrawing their own ### Fees -Implementations MAY charge a **platform fee** and/or an **evaluator fee** (both in basis points). The platform fee is paid to a configurable treasury; the evaluator fee is paid to the job's evaluator address. The specification does not require either fee. If present, fees SHALL be computed independently on each settlement delta and on the completion remainder. Note that with integer (floor) division, the aggregate fee across many small settlements MAY be marginally lower than a single fee computed over the total released amount; implementations and integrators SHOULD treat fee totals as rounding-dependent. Fees SHALL NOT be taken on refunds. +Implementations MAY charge a **platform fee** and/or an **evaluator fee** (both in basis points). The platform fee is paid to a configurable treasury; the evaluator fee is paid to the job's evaluator address. The specification does not require either fee. If present, fees SHALL be computed independently on each settlement delta and on the completion remainder. Fees SHALL NOT be taken on refunds. ### Payout Receivers @@ -189,7 +190,7 @@ interface IERC8183Hook { } ``` -The `selector` parameter identifies which core function is being called (e.g. the function selector for `fund`). The `data` parameter contains function-specific parameters encoded as bytes (see Data encoding below). The hook uses the selector to route internally: +The `selector` parameter identifies which core function is being called (e.g. the function selector for `fund`). The `data` parameter contains function-specific parameters encoded as bytes (see [Data encoding](#data-encoding) below). The hook uses the selector to route internally: ```solidity function beforeAction(uint256 jobId, bytes4 selector, bytes calldata data) external { @@ -217,7 +218,7 @@ When a job has a hook set, the core contract SHALL call `hook.beforeAction(...)` | `settleClaim` | Yes | | `approveClaim` | Yes | | `rejectClaim` | Yes | -| `claimRefund` | Yes — but see the liveness caveat under **Hook security** (a reverting hook can block the permissionless refund) | +| `claimRefund` | Yes — but see the liveness caveat under [**Hook security**](#hook-security) (a reverting hook can block the permissionless refund) | #### Data encoding @@ -245,7 +246,7 @@ When `submit` or `reject` supersedes a pending claim, no claim-specific hook cal #### Hook behaviour -- The `optParams` field (`bytes`, OPTIONAL) on each hookable core function is an opaque payload forwarded to the hook via the `data` parameter. Callers that do not use hooks MAY pass empty bytes. The core contract SHALL NOT decode `optParams`; however, the claim-settlement functions hash-bind `optParams` into the claim identity, so approving or rejecting a claim requires presenting the identical `optParams` bytes. +- The `optParams` field (`bytes`, OPTIONAL) on each hookable core function is an opaque payload forwarded to the hook via the `data` parameter. Callers that do not use hooks MAY pass empty bytes. The core contract SHALL NOT interpret `optParams`. - **Before hooks** (`beforeAction`) are called before the core logic executes. A before hook MAY revert to block the action (e.g. enforce custom validation, allowlists, or preconditions). - **After hooks** (`afterAction`) are called after the core logic completes (including state changes and token transfers). An after hook MAY perform side effects (e.g. emit events, update external state, trigger notifications) or revert to roll back the entire transaction. - If `job.hook == address(0)`, the core contract SHALL skip hook calls and execute normally. @@ -382,7 +383,7 @@ Implementations SHOULD emit at least: - **Refunded**(jobId, client, amount) - **Settled**(jobId, cumulativeAmount, delta) — emitted on every settlement regardless of path - **ClaimSubmitted**(jobId, provider, cumulativeAmount, delta, deliverable, optParams) — provider files a pending claim; `optParams` is emitted so the exact claim preimage can be propagated to observers -- **ClaimSettled**(jobId, settler, cumulativeAmount, delta, deliverable) — direct client settlement; `deliverable` is the settler's attestation, not a verified provider claim +- **ClaimSettled**(jobId, settler, cumulativeAmount, delta, deliverable) — client-initiated settlement; `deliverable` is the settler's attestation, not a verified provider claim - **ClaimApproved**(jobId, approver, cumulativeAmount, delta, deliverable) — pending claim approved by client or evaluator - **ClaimRejected**(jobId, rejector, reason) — pending claim rejected, withdrawn, or superseded From 595e2682281bc19592f2b900740363e71d25bfca Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 16 Jul 2026 14:39:49 +0800 Subject: [PATCH 18/32] docs: fold claim settlement into core functions - nest Claim Settlement as a subsection of Core Functions with a bridging sentence; Claim interactions moves one level down - Signed Authorizations: drop the now-redundant 'including the claim-settlement functions' parenthetical - de-duplicate the 'share one ledger' phrasing in the intro --- eip.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/eip.md b/eip.md index 20c7012..ca8b462 100644 --- a/eip.md +++ b/eip.md @@ -132,9 +132,9 @@ Callable by anyone when status is Open, Funded, or Submitted. SHALL revert if st > do not need contract-client forwarding SHOULD attach no hook (or a hook that never reverts > on the `claimRefund` selector) so the refund stays unconditional. -### Claim Settlement +#### Claim Settlement -While a job is Funded, escrow MAY be released incrementally through **claim settlement**, ahead of (or instead of) terminal completion. Two settlement paths share one ledger: +While a job is Funded, escrow MAY be released incrementally through **claim settlement**, ahead of (or instead of) terminal completion. The four claim-settlement functions are core entry points like the lifecycle functions above; they are specified together because two settlement paths share one ledger: - the **client-initiated path**: the client unilaterally settles an amount to the provider (`settleClaim`); - the **provider-initiated path**: the provider files a pending claim (`submitClaim`) which the client or evaluator approves (`approveClaim`) or any of the three parties rejects (`rejectClaim`). @@ -152,7 +152,7 @@ Called by **client or evaluator**. SHALL revert if the job is not Funded, no cla - **rejectClaim(jobId, cumulativeAmount, deliverable, reason, optParams?)** Called by **client, evaluator, or provider** (the provider withdrawing their own stale claim, e.g. to file a corrected one). SHALL revert if the job is not Funded, no claim is pending, or the arguments do not exactly match the pending claim. SHALL consume the pending claim without moving funds. The consumed claim tuple SHALL remain unfilable — a corrected claim must differ in `cumulativeAmount`, `deliverable`, or `optParams`. SHALL emit ClaimRejected. `optParams` forwarded to hook if set. -#### Claim interactions +##### Claim interactions - **submit** SHALL supersede any pending claim — the provider is electing the full-completion path for the entire remainder — and SHOULD emit ClaimRejected with a supersession reason so hooks and indexers observe a closed claim lifecycle. - **complete** requires no supersession logic: claims can only be filed while the job is Funded, and `complete` is callable only once the job is Submitted — by which point `submit` has already superseded any pending claim, so `complete` can never encounter one. @@ -460,7 +460,7 @@ To support gasless execution — where a client, provider, or evaluator signs an **Implementation requirements:** -- Each actor-authorized core function (including the claim-settlement functions, but excluding the permissionless `claimRefund`) SHALL have a `*WithAuthorization` variant accepting the original parameters plus an `Authorization { address signer; uint72 nonce; uint256 deadline; bytes sig; }`. The reference implementation wraps `createJob`'s parameters in a `CreateJobAuthorizationParams` struct to stay within stack limits. +- Each actor-authorized core function (all except the permissionless `claimRefund`) SHALL have a `*WithAuthorization` variant accepting the original parameters plus an `Authorization { address signer; uint72 nonce; uint256 deadline; bytes sig; }`. The reference implementation wraps `createJob`'s parameters in a `CreateJobAuthorizationParams` struct to stay within stack limits. - Each action SHALL have a distinct EIP-712 typehash binding `signer`, all call parameters (dynamic values such as `description` and `optParams` bound by their `keccak256` hash), `nonce`, and `deadline`, so a signature for one action can never execute another. - `completeWithAuthorization` and `rejectWithAuthorization` SHALL additionally bind the job's stored `submittedAt` value in the signed payload. The value is `0` for an Open or Funded job that has not been submitted, and the actual stored timestamp for a Submitted job. - Nonces SHALL be unordered (random-nonce style, as in ERC-3009) and single-use across all action types. The reference implementation packs them as `bytes32((uint256(uint160(signer)) << 96) | uint256(nonce))` in a single used-nonce mapping. From dbd3aa8c4e0a983c7e18cc33498191f76518c6f8 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 16 Jul 2026 15:02:17 +0800 Subject: [PATCH 19/32] docs: tighten hook-liveness prose and deduplicate refund-forwarding rationale - Hook security liveness bullet now defers to the claimRefund design rationale instead of restating it; drops the BREAKING CHANGE label and revision narration, keeps both hook obligations and the full forceRefund break-glass semantics - Security Considerations hook bullet keeps only its unique normative content and links to Hook security for mitigations - split the ledger-invariant and trust-model run-on sentences - flatten flourishes: grace-period wording, x402 pitch, IDisburser probing-cost rationale --- eip.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/eip.md b/eip.md index ca8b462..3b13f59 100644 --- a/eip.md +++ b/eip.md @@ -109,7 +109,7 @@ Called by evaluator only. SHALL revert if job is not Submitted or caller is not - **reject(jobId, reason, optParams?)** Called by **client or provider when job is Open**, or by **evaluator when job is Funded or Submitted**. SHALL revert if job is not Open, Funded, or Submitted, or if the caller is not authorised for the current status. SHALL set status to Rejected. If Funded or Submitted, SHALL refund the unsettled remainder (`budget - settledAmount`) to client. If a settlement claim is pending, SHALL clear it (see [Claim interactions](#claim-interactions) below). `reason` OPTIONAL. SHALL emit an event including `reason` and the caller (rejector) if provided. `optParams` forwarded to hook if set. - **claimRefund(jobId, optParams)** -Callable by anyone when status is Open, Funded, or Submitted. SHALL revert if status is Open or Funded and `block.timestamp < job.expiredAt`. SHALL revert if status is Submitted and `block.timestamp < job.expiredAt + EVALUATION_GRACE_PERIOD` (the grace period protects an evaluator who is mid-review from being censored by a third-party refund call; implementations MAY set the grace period length or omit it). SHALL revert if a settlement claim is pending on a non-Submitted job (claims can only arise while Funded) — pending claims MUST be resolved (approved, rejected, or withdrawn) before an expiry refund. SHALL set status to Expired. If the prior status was Funded or Submitted, SHALL transfer the unsettled remainder (`budget - settledAmount`), if non-zero, to the client. MAY be hookable; if hookable, the `beforeAction`/`afterAction` callbacks fire around the refund and a reverting hook can block it (see the liveness caveat under [**Hook security**](#hook-security)). Implementations requiring an unconditional post-expiry refund SHALL keep `claimRefund` non-hookable. +Callable by anyone when status is Open, Funded, or Submitted. SHALL revert if status is Open or Funded and `block.timestamp < job.expiredAt`. SHALL revert if status is Submitted and `block.timestamp < job.expiredAt + EVALUATION_GRACE_PERIOD` (the grace period protects an evaluator who is mid-review from being censored by a third-party refund call; implementations MAY set the grace period length or omit it). SHALL revert if a settlement claim is pending on a non-Submitted job (claims can only arise while Funded) — pending claims MUST be resolved (approved, rejected, or withdrawn) before an expiry refund. SHALL set status to Expired. If the prior status was Funded or Submitted, SHALL transfer the unsettled remainder (`budget - settledAmount`), if non-zero, to the client. MAY be hookable; a reverting hook can then block the refund (see the liveness caveat under [**Hook security**](#hook-security)). Implementations requiring an unconditional post-expiry refund SHALL keep `claimRefund` non-hookable. > **Design rationale — refund forwarding for contract clients.** `claimRefund` is > intentionally hookable *and* its callbacks are intentionally **blocking**, to support a @@ -139,7 +139,7 @@ While a job is Funded, escrow MAY be released incrementally through **claim sett - the **client-initiated path**: the client unilaterally settles an amount to the provider (`settleClaim`); - the **provider-initiated path**: the provider files a pending claim (`submitClaim`) which the client or evaluator approves (`approveClaim`) or any of the three parties rejects (`rejectClaim`). -All settlement functions take a **cumulative amount** — the total gross amount the caller asserts should have been released to date — never a delta. Each settlement SHALL release exactly `cumulativeAmount - settledAmount`, then set `settledAmount = cumulativeAmount`. A settlement SHALL revert if `cumulativeAmount <= settledAmount` (no new settlement) or `cumulativeAmount > budget` (exceeds escrow). Because both paths write the same strictly-increasing `settledAmount`, no interleaving of client-initiated settlements, claim approvals, completion, rejection, and refund can release more than `budget` in total, and the delta paid by a claim approval can only shrink between submission and approval, never grow. +All settlement functions take a **cumulative amount** — the total gross amount the caller asserts should have been released to date — never a delta. Each settlement SHALL release exactly `cumulativeAmount - settledAmount`, then set `settledAmount = cumulativeAmount`. A settlement SHALL revert if `cumulativeAmount <= settledAmount` (no new settlement) or `cumulativeAmount > budget` (exceeds escrow). Because both paths write the same strictly-increasing `settledAmount`, no interleaving of client-initiated settlements, claim approvals, completion, rejection, and refund can release more than `budget` in total. The delta paid by a claim approval can only shrink between submission and approval, never grow. At most one claim is pending per job. Implementations SHALL bind the pending claim by a hash over `(cumulativeAmount, deliverable, optParams)` so that approval and rejection require presenting the exact claim contents. @@ -148,7 +148,7 @@ Called by **provider** only. Files a pending claim against a Funded job; moves n - **settleClaim(jobId, cumulativeAmount, deliverable, optParams?)** Called by **client** only. Immediate unilateral settlement: SHALL revert if the job is not Funded, has expired, `cumulativeAmount <= settledAmount`, or `cumulativeAmount > budget`. SHALL set `settledAmount = cumulativeAmount` and distribute the delta to the provider-side payout recipient (minus optional platform/evaluator fees). `deliverable` is the client's settlement attestation, not a verified provider claim. A pending claim SHALL NOT block this function (streaming settlement); settling reduces the delta any subsequent approval of that claim would pay. SHALL emit Settled and ClaimSettled. `optParams` forwarded to hook if set. - **approveClaim(jobId, cumulativeAmount, deliverable, optParams?)** -Called by **client or evaluator**. SHALL revert if the job is not Funded, no claim is pending, or the arguments do not exactly match the pending claim — the evaluator MUST NOT be able to release amounts the provider did not claim. SHALL revert if `cumulativeAmount <= settledAmount` (e.g. the claim was already covered by a client-initiated settlement) or `cumulativeAmount > budget`. SHALL consume the pending claim, set `settledAmount = cumulativeAmount`, and distribute the delta to the provider-side payout recipient (minus optional fees). Approval is NOT subject to an expiry check: a claim filed before expiry remains approvable, consistent with the evaluation grace period philosophy. SHALL emit Settled and ClaimApproved. `optParams` forwarded to hook if set. +Called by **client or evaluator**. SHALL revert if the job is not Funded, no claim is pending, or the arguments do not exactly match the pending claim — the evaluator MUST NOT be able to release amounts the provider did not claim. SHALL revert if `cumulativeAmount <= settledAmount` (e.g. the claim was already covered by a client-initiated settlement) or `cumulativeAmount > budget`. SHALL consume the pending claim, set `settledAmount = cumulativeAmount`, and distribute the delta to the provider-side payout recipient (minus optional fees). Approval is NOT subject to an expiry check: a claim filed before expiry remains approvable, for the same reason the evaluation grace period exists — a review in flight when the job expires may still finish. SHALL emit Settled and ClaimApproved. `optParams` forwarded to hook if set. - **rejectClaim(jobId, cumulativeAmount, deliverable, reason, optParams?)** Called by **client, evaluator, or provider** (the provider withdrawing their own stale claim, e.g. to file a corrected one). SHALL revert if the job is not Funded, no claim is pending, or the arguments do not exactly match the pending claim. SHALL consume the pending claim without moving funds. The consumed claim tuple SHALL remain unfilable — a corrected claim must differ in `cumulativeAmount`, `deliverable`, or `optParams`. SHALL emit ClaimRejected. `optParams` forwarded to hook if set. @@ -175,7 +175,7 @@ Implementations MAY charge a **platform fee** and/or an **evaluator fee** (both Implementations MAY support `IDisburser` receivers. If the payout recipient is a contract that advertises `IDisburser` via ERC-165 at payout time, the implementation SHALL transfer the provider-side net amount first, then call `onDisbursement(jobId, selector, token, amount, optParams)`. Callback detection is dynamic: a receiver with no code when set can later deploy or delegate code and receive callbacks if it advertises `IDisburser` when paid. A revert from `onDisbursement` SHALL revert the parent action, including any platform or evaluator fee transfers made in that action. EOAs and contracts that do not advertise `IDisburser` are plain recipients. Implementations SHOULD skip the callback when the net provider-side amount is zero. -Dynamic callback detection adds ERC-165 probing cost to each nonzero payout routed to a contract receiver. This is the cost of preserving payout-time behavior for counterfactual deployments, upgradeable receivers, and delegated EOAs; implementations SHOULD NOT cache receiver interface support unless they also change the documented dynamic semantics. +Dynamic callback detection adds ERC-165 probing cost to each nonzero payout routed to a contract receiver. This preserves payout-time behavior for receivers whose code appears or changes after the job is created; implementations SHOULD NOT cache receiver interface support unless they also change the documented dynamic semantics. ### Hooks (OPTIONAL) @@ -254,7 +254,7 @@ When `submit` or `reject` supersedes a pending claim, no claim-specific hook cal #### Hook security - Hooks are **trusted** contracts chosen by the client at job creation. A malicious or buggy hook can revert valid actions or execute arbitrary logic in callbacks. Clients SHOULD audit or use well-known hook implementations. -- **Liveness (BREAKING CHANGE):** A reverting hook can block all hookable actions for that job until `expiredAt`. This is by design — the hook is part of the job's policy. In earlier revisions of this specification `claimRefund` was deliberately **not** hookable, which gave an *unconditional* recovery guarantee: refunds after expiry could never be blocked. This revision makes `claimRefund` hookable (before + after) **intentionally and as a blocking callback**, to support a `job.client` that is a contract standing in for the real beneficiary (e.g. a smart account or router): the refund is paid to that contract and its `afterAction` callback (which runs *after* the refund transfer) atomically forwards the funds to the rightful end-client. The callback MUST be able to revert the whole refund, because a failed forward must not leave funds stranded in the intermediary client contract. The accepted trade-off is that the post-expiry refund is no longer unconditionally live: a buggy or fail-closed hook can revert and block it. The trust model handles this in two layers: **(1)** hooks are admin-whitelisted, and whitelisting is expected to include an **audit that the hook never blocks lifecycle paths** — it MUST NOT revert on `claimRefund` except for a genuine forward failure, and MUST NOT derive routing or authorization from the caller-supplied `optParams` on this permissionless path (a stranger may trigger `claimRefund`); **(2)** as an on-chain break-glass, the admin can `pause()` and `forceRefund` the job — identical eligibility to `claimRefund` with the hook callbacks skipped, paying the refund (to the client by default, or to an admin-chosen recipient when the client is an intermediary contract that cannot receive funds without its blocked hook) and expiring the job atomically so a rescued job can never be refunded twice — and `batchDetachHook` is a lighter option for a non-forwarding hook (`emergencyWithdraw` remains reserved for funds not attributed to any job). Deployments that do not need contract-client forwarding and want an unconditional, trust-minimized refund SHOULD attach no hook (or a non-reverting hook) on such jobs. +- **Liveness:** A reverting hook can block all hookable actions for that job, including the post-expiry refund. This is the accepted cost of refund forwarding for contract clients; the full argument is the design rationale under `claimRefund` in [Core Functions](#core-functions). The trust model mitigates it in two layers. First, whitelisting is expected to include an audit that the hook never blocks lifecycle paths: a hook MUST NOT revert on `claimRefund` except for a genuine forward failure, and MUST NOT derive routing or authorization from the caller-supplied `optParams` on that permissionless path (a stranger may trigger `claimRefund`). Second, the admin can `pause()` and `forceRefund` the job as an on-chain break-glass. Its eligibility is identical to `claimRefund` with the hook callbacks skipped; the refund goes to the client by default (or to an admin-chosen recipient when the client contract cannot receive funds without its blocked hook), and the job expires atomically with the payout so a rescued job can never be refunded twice. `batchDetachHook` is a lighter option for a non-forwarding hook; `emergencyWithdraw` remains reserved for funds not attributed to any job. Earlier drafts kept `claimRefund` non-hookable, and deployments that want that unconditional, trust-minimized refund SHOULD attach no hook (or a non-reverting hook). - **Atomicity:** After-callbacks run after state changes but within the same transaction. If an after-callback reverts, the entire transaction (including the core state change) is rolled back. This is intentional — it enables atomic multi-step flows (e.g. escrow funding + side token transfer must both succeed or both revert). - `onlyERC8183` modifiers on hooks are RECOMMENDED so that hook functions cannot be called directly by external actors. - Hooks SHOULD NOT be upgradeable after a job is created, as this would allow the hook to change behaviour mid-job. @@ -471,7 +471,7 @@ To support gasless execution — where a client, provider, or evaluator signs an **Token approvals:** For functions that pull tokens (e.g. `fundWithAuthorization`), the signer SHOULD use [ERC-2612](./eip-2612.md) (`permit`) to approve token spending via signature. The facilitator can then call `permit` and `fundWithAuthorization` in a single transaction — no on-chain approval tx needed from the signer. -**x402 compatibility:** This extension enables compatibility with HTTP-native payment protocols such as x402, where an AI agent signs payment intents off-chain and a payment facilitator handles on-chain execution. The agent only needs a private key and tokens — no gas, no RPC management, no chain-specific logic. +**x402 compatibility:** This extension enables compatibility with HTTP-native payment protocols such as x402, where an AI agent signs payment intents off-chain and a payment facilitator handles on-chain execution. The agent needs only a private key and tokens; the facilitator pays gas and submits the transactions. --- @@ -564,7 +564,7 @@ function DOMAIN_SEPARATOR() external view returns (bytes32); - **Tokens:** Use SafeERC-20 or equivalent for [ERC-20](./eip-20.md). - **Evaluator:** MUST be set at creation; if "client completes", pass `evaluator = client`. - **Hook gas limits** (for hooked implementations): Implementations SHOULD impose a gas limit on hook calls (e.g. `call{gas: HOOK_GAS_LIMIT}(...)`) to bound execution cost and prevent hooks from consuming unbounded gas. The specific limit is left to the implementation as gas costs vary across chains. -- Hook contracts are client-supplied and trusted by the client; implementations MUST NOT allow hooks to modify core escrow state directly. Where `claimRefund` is hookable (as in the reference implementation), a reverting hook can block refunds after expiry. The reference implementation accepts this under a governance trust model: hooks are admin-whitelisted only after an audit that they do not block lifecycle paths (and do not act on caller-supplied `optParams` on the permissionless `claimRefund` path), with an admin `forceRefund` (refund + expire in one step, hooks bypassed, only under `pause()`) as the on-chain break-glass recovery (and `batchDetachHook` for non-forwarding hooks). A rescue MUST close the job's accounting atomically with the payout so a rescued job cannot be refunded twice; a free-form escape hatch like `emergencyWithdraw` SHOULD be reserved for funds not attributed to any job. Implementations requiring an unconditional, trust-minimized post-expiry refund SHALL keep `claimRefund` non-hookable. +- Hook contracts are client-supplied and trusted by the client; implementations MUST NOT allow hooks to modify core escrow state directly. Where `claimRefund` is hookable (as in the reference implementation), a reverting hook can block refunds after expiry; see [Hook security](#hook-security) for the whitelist-audit and break-glass mitigations. Any rescue path MUST close the job's accounting atomically with the payout so a rescued job cannot be refunded twice; a free-form escape hatch like `emergencyWithdraw` SHOULD be reserved for funds not attributed to any job. Implementations requiring an unconditional, trust-minimized post-expiry refund SHALL keep `claimRefund` non-hookable. - Jobs that use **advanced hooks** (e.g. two‑phase escrow / fund‑transfer hooks that custody additional tokens) are expected to have **more revert paths and tighter coupling** to external logic than plain, non‑hooked Agentic Commerce jobs. Such hooks SHOULD be reserved for agents and users who understand and accept this trade‑off; for most simple jobs, a non‑hooked or policy‑only hook is RECOMMENDED. ## Copyright From 9f180c3ca7d4b42c9c8bf3963ab13c8888ca43e0 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 16 Jul 2026 16:12:10 +0800 Subject: [PATCH 20/32] docs: define claim deliverable semantics in claim-settlement intro One shared sentence: same bytes32 reference form as submit's deliverable, but identifying the milestone being settled rather than the full work. --- eip.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eip.md b/eip.md index 3b13f59..e18fc5a 100644 --- a/eip.md +++ b/eip.md @@ -141,7 +141,7 @@ While a job is Funded, escrow MAY be released incrementally through **claim sett All settlement functions take a **cumulative amount** — the total gross amount the caller asserts should have been released to date — never a delta. Each settlement SHALL release exactly `cumulativeAmount - settledAmount`, then set `settledAmount = cumulativeAmount`. A settlement SHALL revert if `cumulativeAmount <= settledAmount` (no new settlement) or `cumulativeAmount > budget` (exceeds escrow). Because both paths write the same strictly-increasing `settledAmount`, no interleaving of client-initiated settlements, claim approvals, completion, rejection, and refund can release more than `budget` in total. The delta paid by a claim approval can only shrink between submission and approval, never grow. -At most one claim is pending per job. Implementations SHALL bind the pending claim by a hash over `(cumulativeAmount, deliverable, optParams)` so that approval and rejection require presenting the exact claim contents. +At most one claim is pending per job. Implementations SHALL bind the pending claim by a hash over `(cumulativeAmount, deliverable, optParams)` so that approval and rejection require presenting the exact claim contents. As with `submit`, `deliverable` is a `bytes32` reference (e.g. hash of an off-chain deliverable, IPFS CID, attestation commitment); in a claim it identifies the milestone or phase being settled rather than the full work. - **submitClaim(jobId, cumulativeAmount, deliverable, optParams?)** Called by **provider** only. Files a pending claim against a Funded job; moves no funds. SHALL revert if the job is not Funded, has expired, `deliverable == bytes32(0)`, a claim is already pending, `cumulativeAmount <= settledAmount`, `cumulativeAmount > budget`, or an identical claim tuple `(cumulativeAmount, deliverable, optParams)` was previously filed on this job (replay protection; implementations SHOULD track consumed claim hashes). SHALL emit ClaimSubmitted including `optParams` so the exact claim preimage can be propagated to observers. `optParams` forwarded to hook if set. From 577de19b0c3428950ce03687b1c46ce4c26bcd81 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 16 Jul 2026 17:18:11 +0800 Subject: [PATCH 21/32] feat: emit ForceRefunded event for break-glass attribution A forceRefund previously emitted only the generic Refunded event, so event-stream indexers could not distinguish an admin rescue (possibly redirected to a non-client recipient) from a normal client refund. Emit ForceRefunded(jobId, admin, recipient, amount) alongside Refunded. Resolves audit finding L-02. --- contracts/ERC8183.sol | 17 +++++++++++++++-- test/ERC8183.t.sol | 29 +++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/contracts/ERC8183.sol b/contracts/ERC8183.sol index 020e1b3..2c03f83 100644 --- a/contracts/ERC8183.sol +++ b/contracts/ERC8183.sol @@ -190,6 +190,15 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable address indexed client, uint256 amount ); + /// @notice Emitted alongside Refunded when the admin break-glass forceRefund expires a + /// job, attributing the acting admin and the (possibly overridden) recipient so + /// event-stream indexers can distinguish an admin rescue from a client refund + event ForceRefunded( + uint256 indexed jobId, + address indexed admin, + address indexed recipient, + uint256 amount + ); /// @notice Emitted on each successful partial settlement event Settled( uint256 indexed jobId, @@ -964,7 +973,9 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable /// @param to Refund recipient; address(0) defaults to job.client function forceRefund(uint256 jobId, address to) external onlyRole(ADMIN_ROLE) whenPaused nonReentrant { Job storage job = _validateRefundEligibility(jobId); - _expireWithRefund(jobId, job, to == address(0) ? job.client : to); + address recipient = to == address(0) ? job.client : to; + uint256 amount = _expireWithRefund(jobId, job, recipient); + emit ForceRefunded(jobId, msg.sender, recipient, amount); } /// @dev Shared eligibility checks for claimRefund/forceRefund. @@ -985,7 +996,8 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable /// @dev Shared state transition for claimRefund/forceRefund: expire the job and pay the /// unsettled remainder to `recipient` (always job.client on the permissionless path). - function _expireWithRefund(uint256 jobId, Job storage job, address recipient) internal { + /// Returns the amount actually transferred (0 when nothing was escrowed to refund). + function _expireWithRefund(uint256 jobId, Job storage job, address recipient) internal returns (uint256 refunded) { JobStatus prev = job.status; job.status = JobStatus.Expired; @@ -993,6 +1005,7 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable if (refundAmount > 0 && (prev == JobStatus.Funded || prev == JobStatus.Submitted)) { IERC20(job.paymentToken).safeTransfer(recipient, refundAmount); emit Refunded(jobId, recipient, refundAmount); + refunded = refundAmount; } emit JobExpired(jobId); diff --git a/test/ERC8183.t.sol b/test/ERC8183.t.sol index 8c08724..6f8c495 100644 --- a/test/ERC8183.t.sol +++ b/test/ERC8183.t.sol @@ -1697,6 +1697,35 @@ contract ERC8183Test is Test { assertEq(uint8(core.getJob(jobId).status), uint8(ERC8183.JobStatus.Expired)); } + // Observability: a break-glass rescue must be distinguishable from a normal client refund + // in the event stream alone. forceRefund emits ForceRefunded (attributing the acting admin + // and the chosen recipient) alongside the generic Refunded. + function test_forceRefund_emitsForceRefundedForAttribution() public { + SelectiveRevertHook hook = new SelectiveRevertHook(ERC8183.claimRefund.selector); + vm.prank(deployer); + core.setHookWhitelist(address(hook), true); + + vm.prank(client); + uint256 jobId = core.createJob(provider, evaluator, _futureExpiry(), "hooked", address(hook), 0, ""); + vm.prank(provider); + core.setBudget(jobId, address(usdc), TEN_USDC, ""); + vm.prank(client); + core.fund(jobId, address(usdc), TEN_USDC, ""); + + vm.warp(block.timestamp + 3601); + + address beneficiary = makeAddr("ultimateBeneficiary"); + vm.startPrank(deployer); + core.pause(); + + vm.expectEmit(true, true, false, true); + emit ERC8183.Refunded(jobId, beneficiary, TEN_USDC); + vm.expectEmit(true, true, true, true); + emit ERC8183.ForceRefunded(jobId, deployer, beneficiary, TEN_USDC); + core.forceRefund(jobId, beneficiary); + vm.stopPrank(); + } + function test_forceRefund_revertsWhenNotPaused() public { uint256 jobId = _createFundedJob(TEN_USDC); vm.warp(block.timestamp + 3601); From 6400d7881ce783d8105f28e2ba5006e392242847 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 16 Jul 2026 17:18:51 +0800 Subject: [PATCH 22/32] docs: state gas-exhaustion and no-op-forwarding hook risks explicitly Audit findings M-01/L-03: the liveness trade-off language only named reverting hooks, but a hook that consumes unbounded gas blocks a call identically with no revert to signal it, and the forward-or-revert expectation on refund-forwarding hooks is an audited hook obligation, not a kernel-verified invariant. Say both explicitly in the whitelist audit expectations (eip.md Hook security, claimRefund natspec), extend the disburser receiver caveat to gas exhaustion (L-01), and document the ForceRefunded event in the break-glass paragraph. --- contracts/ERC8183.sol | 16 ++++++++++++++-- eip.md | 4 ++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/contracts/ERC8183.sol b/contracts/ERC8183.sol index 2c03f83..2cdeebe 100644 --- a/contracts/ERC8183.sol +++ b/contracts/ERC8183.sol @@ -474,6 +474,11 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable /// @dev Only receivers with deployed code can receive the optional callback. /// EOAs and contracts that do not advertise IDisburser are plain recipients. + /// The callback is intentionally blocking (a revert rolls back the settlement, + /// per the spec) and is not gas-bounded: a receiver that reverts or consumes + /// unbounded gas in onDisbursement blocks its own settlement. The receiver is + /// provider-chosen, so this is provider-controlled risk — the evaluator can + /// reject instead, which refunds the client through a path with no callback. function _isDisburser(address receiver) internal view returns (bool) { return receiver.code.length > 0 && ERC165Checker.supportsInterface( receiver, @@ -930,12 +935,19 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable /// forward the funds to the rightful end-client. The callbacks therefore MAY /// revert to roll the whole refund back — this is required: a failed forward /// MUST NOT leave funds stranded in the intermediary client contract. + /// Forward-or-revert is the hook's obligation, not a kernel invariant: the + /// kernel does not verify that funds moved onward, so a no-op afterAction + /// finalizes the job (Expired, irreversible) with the refund left in the + /// intermediary. Verifying forwarding behavior is part of the whitelist audit. /// Trade-off: this removes the unconditional post-expiry refund guarantee — a - /// buggy/reverting hook could otherwise block the refund. The trust model + /// buggy, reverting, or gas-exhausting hook could otherwise block the refund + /// (unbounded gas consumption blocks the call exactly like a revert, with no + /// revert reason to signal it). The trust model /// mitigates this: hooks are admin-whitelisted + ERC-165-checked at attach time /// and are expected to be audited as part of whitelisting to never block /// lifecycle paths (i.e. never revert on claimRefund except for a genuine - /// forward failure) and to never derive routing/authorization from the + /// forward failure, never consume unbounded gas on any hooked selector) and to + /// never derive routing/authorization from the /// caller-supplied optParams on this permissionless path. Break-glass recovery if /// a hook still blocks a refund: batchDetachHook for a non-forwarding hook (after /// which claimRefund proceeds with no callbacks), or admin forceRefund, which diff --git a/eip.md b/eip.md index 3b13f59..224568e 100644 --- a/eip.md +++ b/eip.md @@ -254,7 +254,7 @@ When `submit` or `reject` supersedes a pending claim, no claim-specific hook cal #### Hook security - Hooks are **trusted** contracts chosen by the client at job creation. A malicious or buggy hook can revert valid actions or execute arbitrary logic in callbacks. Clients SHOULD audit or use well-known hook implementations. -- **Liveness:** A reverting hook can block all hookable actions for that job, including the post-expiry refund. This is the accepted cost of refund forwarding for contract clients; the full argument is the design rationale under `claimRefund` in [Core Functions](#core-functions). The trust model mitigates it in two layers. First, whitelisting is expected to include an audit that the hook never blocks lifecycle paths: a hook MUST NOT revert on `claimRefund` except for a genuine forward failure, and MUST NOT derive routing or authorization from the caller-supplied `optParams` on that permissionless path (a stranger may trigger `claimRefund`). Second, the admin can `pause()` and `forceRefund` the job as an on-chain break-glass. Its eligibility is identical to `claimRefund` with the hook callbacks skipped; the refund goes to the client by default (or to an admin-chosen recipient when the client contract cannot receive funds without its blocked hook), and the job expires atomically with the payout so a rescued job can never be refunded twice. `batchDetachHook` is a lighter option for a non-forwarding hook; `emergencyWithdraw` remains reserved for funds not attributed to any job. Earlier drafts kept `claimRefund` non-hookable, and deployments that want that unconditional, trust-minimized refund SHOULD attach no hook (or a non-reverting hook). +- **Liveness:** A reverting hook can block all hookable actions for that job, including the post-expiry refund. A hook that consumes unbounded gas blocks identically — no finite caller gas budget covers it, and there is no revert to signal it — so gas exhaustion is equivalent to a revert throughout this section. This is the accepted cost of refund forwarding for contract clients; the full argument is the design rationale under `claimRefund` in [Core Functions](#core-functions). The forward-or-revert expectation on forwarding hooks is likewise a hook obligation, not a kernel invariant: the contract does not verify that a refund moved onward, so a no-op `afterAction` on a contract client finalizes the job with the refund left in the intermediary. The trust model mitigates both in two layers. First, whitelisting is expected to include an audit that the hook never blocks lifecycle paths: a hook MUST NOT revert on `claimRefund` except for a genuine forward failure, MUST NOT consume unbounded gas on any hooked selector, MUST NOT derive routing or authorization from the caller-supplied `optParams` on that permissionless path (a stranger may trigger `claimRefund`), and — when it forwards refunds for a contract client — MUST forward or revert, never no-op. Second, the admin can `pause()` and `forceRefund` the job as an on-chain break-glass. Its eligibility is identical to `claimRefund` with the hook callbacks skipped; the refund goes to the client by default (or to an admin-chosen recipient when the client contract cannot receive funds without its blocked hook), and the job expires atomically with the payout so a rescued job can never be refunded twice. Implementations SHOULD emit a dedicated event for the rescue (the reference implementation emits `ForceRefunded(jobId, admin, recipient, amount)` alongside the generic `Refunded`) so an admin redirect is attributable from the event stream alone. `batchDetachHook` is a lighter option for a non-forwarding hook; `emergencyWithdraw` remains reserved for funds not attributed to any job. Earlier drafts kept `claimRefund` non-hookable, and deployments that want that unconditional, trust-minimized refund SHOULD attach no hook (or a non-reverting hook). - **Atomicity:** After-callbacks run after state changes but within the same transaction. If an after-callback reverts, the entire transaction (including the core state change) is rolled back. This is intentional — it enables atomic multi-step flows (e.g. escrow funding + side token transfer must both succeed or both revert). - `onlyERC8183` modifiers on hooks are RECOMMENDED so that hook functions cannot be called directly by external actors. - Hooks SHOULD NOT be upgradeable after a job is created, as this would allow the hook to change behaviour mid-job. @@ -552,7 +552,7 @@ function DOMAIN_SEPARATOR() external view returns (bytes32); - Evaluator is trusted for completion and rejection once the job is Submitted; a malicious evaluator can complete or reject arbitrarily. Use reputation (e.g. [ERC-8004](./eip-8004.md)) or staking for high-value jobs. - Once Funded, only the evaluator can reject, and only the provider can submit; the client cannot unilaterally withdraw, which protects the provider after they start work. - **Evaluator settlement scope:** the evaluator can approve only the exact pending claim the provider filed, never originate or alter amounts; the client can settle freely but only toward the job's provider out of their own escrow. Widening either authority breaks the trust model — in particular, allowing the evaluator to settle arbitrary amounts would escalate the evaluator from attestor to spender of client escrow. -- **Provider-controlled payout receiver:** `payoutReceiver` controls where provider-side net payouts are sent and is therefore set by the provider while the job is Open. The reference design locks the receiver once the job is Funded so funded jobs cannot be rerouted away from a receiver that downstream custody, financing, or disbursement flows may rely on. Providers SHOULD choose receiver contracts carefully: a receiver that advertises `IDisburser` and reverts in `onDisbursement` will roll back completion or settlement, including platform and evaluator fee transfers, at the cost of blocking the provider's own payout. Evaluators can reject a Funded or Submitted job rather than complete into a reverting receiver, which refunds the client and pays no one. +- **Provider-controlled payout receiver:** `payoutReceiver` controls where provider-side net payouts are sent and is therefore set by the provider while the job is Open. The reference design locks the receiver once the job is Funded so funded jobs cannot be rerouted away from a receiver that downstream custody, financing, or disbursement flows may rely on. Providers SHOULD choose receiver contracts carefully: a receiver that advertises `IDisburser` and reverts (or consumes unbounded gas) in `onDisbursement` will roll back completion or settlement, including platform and evaluator fee transfers, at the cost of blocking the provider's own payout. Evaluators can reject a Funded or Submitted job rather than complete into a reverting receiver, which refunds the client and pays no one. - **Dynamic receiver code:** `IDisburser` detection happens at payout time. A plain EOA receiver can later gain delegated code through mechanisms such as [EIP-7702](./eip-7702.md), and a counterfactual receiver can later deploy code at the selected address. These changes can make callbacks begin firing after the job was funded; because only the provider can select the receiver, this is provider-controlled risk. - **Pending-claim refund blocking is bounded griefing:** a provider's pending claim blocks `claimRefund` on a Funded job past expiry, but on non-hooked jobs the client can always clear it with `rejectClaim` and refund in the next transaction. On hooked jobs a reverting `rejectClaim` hook can keep the claim pinned; clients accepting a hook accept this as part of the job's policy. - **Consumed claim hashes:** rejected or withdrawn claim tuples remain consumed, so a rejected claim cannot be silently refiled and later approved; a refile must visibly differ in `cumulativeAmount`, `deliverable`, or `optParams`. From 30b1a8815998d2dbf4cdf593092fffe3c158cce5 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 16 Jul 2026 17:35:01 +0800 Subject: [PATCH 23/32] docs: trim redundant natspec added in review resolution The normative language lives in eip.md; keep contract comments to one-line pointers. --- contracts/ERC8183.sol | 22 ++++++---------------- test/ERC8183.t.sol | 4 +--- 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/contracts/ERC8183.sol b/contracts/ERC8183.sol index 2cdeebe..56e1c43 100644 --- a/contracts/ERC8183.sol +++ b/contracts/ERC8183.sol @@ -190,9 +190,8 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable address indexed client, uint256 amount ); - /// @notice Emitted alongside Refunded when the admin break-glass forceRefund expires a - /// job, attributing the acting admin and the (possibly overridden) recipient so - /// event-stream indexers can distinguish an admin rescue from a client refund + /// @notice Emitted alongside Refunded when forceRefund pays out, attributing the admin + /// and chosen recipient event ForceRefunded( uint256 indexed jobId, address indexed admin, @@ -474,11 +473,6 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable /// @dev Only receivers with deployed code can receive the optional callback. /// EOAs and contracts that do not advertise IDisburser are plain recipients. - /// The callback is intentionally blocking (a revert rolls back the settlement, - /// per the spec) and is not gas-bounded: a receiver that reverts or consumes - /// unbounded gas in onDisbursement blocks its own settlement. The receiver is - /// provider-chosen, so this is provider-controlled risk — the evaluator can - /// reject instead, which refunds the client through a path with no callback. function _isDisburser(address receiver) internal view returns (bool) { return receiver.code.length > 0 && ERC165Checker.supportsInterface( receiver, @@ -935,18 +929,14 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable /// forward the funds to the rightful end-client. The callbacks therefore MAY /// revert to roll the whole refund back — this is required: a failed forward /// MUST NOT leave funds stranded in the intermediary client contract. - /// Forward-or-revert is the hook's obligation, not a kernel invariant: the - /// kernel does not verify that funds moved onward, so a no-op afterAction - /// finalizes the job (Expired, irreversible) with the refund left in the - /// intermediary. Verifying forwarding behavior is part of the whitelist audit. + /// Forward-or-revert is audited at whitelisting; the kernel does not verify it. /// Trade-off: this removes the unconditional post-expiry refund guarantee — a - /// buggy, reverting, or gas-exhausting hook could otherwise block the refund - /// (unbounded gas consumption blocks the call exactly like a revert, with no - /// revert reason to signal it). The trust model + /// buggy, reverting, or gas-exhausting hook could otherwise block the refund. + /// The trust model /// mitigates this: hooks are admin-whitelisted + ERC-165-checked at attach time /// and are expected to be audited as part of whitelisting to never block /// lifecycle paths (i.e. never revert on claimRefund except for a genuine - /// forward failure, never consume unbounded gas on any hooked selector) and to + /// forward failure, never consume unbounded gas) and to /// never derive routing/authorization from the /// caller-supplied optParams on this permissionless path. Break-glass recovery if /// a hook still blocks a refund: batchDetachHook for a non-forwarding hook (after diff --git a/test/ERC8183.t.sol b/test/ERC8183.t.sol index 6f8c495..5d7a013 100644 --- a/test/ERC8183.t.sol +++ b/test/ERC8183.t.sol @@ -1697,9 +1697,7 @@ contract ERC8183Test is Test { assertEq(uint8(core.getJob(jobId).status), uint8(ERC8183.JobStatus.Expired)); } - // Observability: a break-glass rescue must be distinguishable from a normal client refund - // in the event stream alone. forceRefund emits ForceRefunded (attributing the acting admin - // and the chosen recipient) alongside the generic Refunded. + // A rescue must be distinguishable from a normal refund in the event stream alone. function test_forceRefund_emitsForceRefundedForAttribution() public { SelectiveRevertHook hook = new SelectiveRevertHook(ERC8183.claimRefund.selector); vm.prank(deployer); From 665ed5e4bf81db4ca5ca677f24ae2e499e438573 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 16 Jul 2026 18:04:54 +0800 Subject: [PATCH 24/32] docs: split hook liveness into consideration-per-bullet form Liveness / audit obligations / break-glass as separate bullets, with rationale delegated to the claimRefund design-rationale note. All normative statements (four hook MUST NOTs, rescue-event SHOULD, no-hook SHOULD, gas-exhaustion equivalence, forceRefund semantics) preserved. --- eip.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/eip.md b/eip.md index fb35879..7f31fd3 100644 --- a/eip.md +++ b/eip.md @@ -254,7 +254,9 @@ When `submit` or `reject` supersedes a pending claim, no claim-specific hook cal #### Hook security - Hooks are **trusted** contracts chosen by the client at job creation. A malicious or buggy hook can revert valid actions or execute arbitrary logic in callbacks. Clients SHOULD audit or use well-known hook implementations. -- **Liveness:** A reverting hook can block all hookable actions for that job, including the post-expiry refund. A hook that consumes unbounded gas blocks identically — no finite caller gas budget covers it, and there is no revert to signal it — so gas exhaustion is equivalent to a revert throughout this section. This is the accepted cost of refund forwarding for contract clients; the full argument is the design rationale under `claimRefund` in [Core Functions](#core-functions). The forward-or-revert expectation on forwarding hooks is likewise a hook obligation, not a kernel invariant: the contract does not verify that a refund moved onward, so a no-op `afterAction` on a contract client finalizes the job with the refund left in the intermediary. The trust model mitigates both in two layers. First, whitelisting is expected to include an audit that the hook never blocks lifecycle paths: a hook MUST NOT revert on `claimRefund` except for a genuine forward failure, MUST NOT consume unbounded gas on any hooked selector, MUST NOT derive routing or authorization from the caller-supplied `optParams` on that permissionless path (a stranger may trigger `claimRefund`), and — when it forwards refunds for a contract client — MUST forward or revert, never no-op. Second, the admin can `pause()` and `forceRefund` the job as an on-chain break-glass. Its eligibility is identical to `claimRefund` with the hook callbacks skipped; the refund goes to the client by default (or to an admin-chosen recipient when the client contract cannot receive funds without its blocked hook), and the job expires atomically with the payout so a rescued job can never be refunded twice. Implementations SHOULD emit a dedicated event for the rescue (the reference implementation emits `ForceRefunded(jobId, admin, recipient, amount)` alongside the generic `Refunded`) so an admin redirect is attributable from the event stream alone. `batchDetachHook` is a lighter option for a non-forwarding hook; `emergencyWithdraw` remains reserved for funds not attributed to any job. Earlier drafts kept `claimRefund` non-hookable, and deployments that want that unconditional, trust-minimized refund SHOULD attach no hook (or a non-reverting hook). +- **Liveness:** a reverting hook can block every hookable action, including the post-expiry refund; a hook that consumes unbounded gas blocks identically, so gas exhaustion counts as a revert throughout this section. The contract does not verify that a forwarded refund moved onward. See the design rationale under `claimRefund` in [Core Functions](#core-functions). +- **Audit obligations:** a whitelisted hook MUST NOT revert on `claimRefund` (except for a genuine forward failure), consume unbounded gas on any hooked selector, derive routing or authorization from the caller-supplied `optParams` on that permissionless path, or no-op a refund it is expected to forward. +- **Break-glass:** under `pause()`, admin `forceRefund` applies `claimRefund`'s eligibility with hook callbacks skipped, pays the client (or an admin-chosen recipient when the client contract cannot receive funds), and expires the job atomically — a rescued job cannot be refunded twice. Implementations SHOULD emit a dedicated event (`ForceRefunded(jobId, admin, recipient, amount)` in the reference implementation). `batchDetachHook` suits non-forwarding hooks; `emergencyWithdraw` is reserved for funds not attributed to any job. Deployments that need an unconditional post-expiry refund SHOULD attach no hook (or a non-reverting hook). - **Atomicity:** After-callbacks run after state changes but within the same transaction. If an after-callback reverts, the entire transaction (including the core state change) is rolled back. This is intentional — it enables atomic multi-step flows (e.g. escrow funding + side token transfer must both succeed or both revert). - `onlyERC8183` modifiers on hooks are RECOMMENDED so that hook functions cannot be called directly by external actors. - Hooks SHOULD NOT be upgradeable after a job is created, as this would allow the hook to change behaviour mid-job. From 3d44824e0340ab705ff02a7ec340e2c7bcd29be8 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 16 Jul 2026 18:14:09 +0800 Subject: [PATCH 25/32] docs: sync events with ForceRefunded and trim hook prose - add ForceRefunded to operational events; Refunded recipient may differ under a forceRefund destination override - hook gas cap becomes MAY with a pointer to the audit obligations (reference implementation relies on whitelist audit, not a cap) - replace 'kernel' with 'implementation'; de-triplicate the createJob afterAction-only explanation; point claimRefund's grace-period parenthetical at the State Machine rationale; drop 'canonicalized' --- eip.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/eip.md b/eip.md index 7f31fd3..33486ad 100644 --- a/eip.md +++ b/eip.md @@ -109,7 +109,7 @@ Called by evaluator only. SHALL revert if job is not Submitted or caller is not - **reject(jobId, reason, optParams?)** Called by **client or provider when job is Open**, or by **evaluator when job is Funded or Submitted**. SHALL revert if job is not Open, Funded, or Submitted, or if the caller is not authorised for the current status. SHALL set status to Rejected. If Funded or Submitted, SHALL refund the unsettled remainder (`budget - settledAmount`) to client. If a settlement claim is pending, SHALL clear it (see [Claim interactions](#claim-interactions) below). `reason` OPTIONAL. SHALL emit an event including `reason` and the caller (rejector) if provided. `optParams` forwarded to hook if set. - **claimRefund(jobId, optParams)** -Callable by anyone when status is Open, Funded, or Submitted. SHALL revert if status is Open or Funded and `block.timestamp < job.expiredAt`. SHALL revert if status is Submitted and `block.timestamp < job.expiredAt + EVALUATION_GRACE_PERIOD` (the grace period protects an evaluator who is mid-review from being censored by a third-party refund call; implementations MAY set the grace period length or omit it). SHALL revert if a settlement claim is pending on a non-Submitted job (claims can only arise while Funded) — pending claims MUST be resolved (approved, rejected, or withdrawn) before an expiry refund. SHALL set status to Expired. If the prior status was Funded or Submitted, SHALL transfer the unsettled remainder (`budget - settledAmount`), if non-zero, to the client. MAY be hookable; a reverting hook can then block the refund (see the liveness caveat under [**Hook security**](#hook-security)). Implementations requiring an unconditional post-expiry refund SHALL keep `claimRefund` non-hookable. +Callable by anyone when status is Open, Funded, or Submitted. SHALL revert if status is Open or Funded and `block.timestamp < job.expiredAt`. SHALL revert if status is Submitted and `block.timestamp < job.expiredAt + EVALUATION_GRACE_PERIOD` (see the grace-period rationale under [State Machine](#state-machine)). SHALL revert if a settlement claim is pending on a non-Submitted job (claims can only arise while Funded) — pending claims MUST be resolved (approved, rejected, or withdrawn) before an expiry refund. SHALL set status to Expired. If the prior status was Funded or Submitted, SHALL transfer the unsettled remainder (`budget - settledAmount`), if non-zero, to the client. MAY be hookable; a reverting hook can then block the refund (see the liveness caveat under [**Hook security**](#hook-security)). Implementations requiring an unconditional post-expiry refund SHALL keep `claimRefund` non-hookable. > **Design rationale — refund forwarding for contract clients.** `claimRefund` is > intentionally hookable *and* its callbacks are intentionally **blocking**, to support a @@ -179,7 +179,7 @@ Dynamic callback detection adds ERC-165 probing cost to each nonzero payout rout ### Hooks (OPTIONAL) -Implementations MAY support an optional **hook contract** per job to extend the core protocol without modifying it. The hook address is set at job creation (or `address(0)` for no hook) and stored on the job. A **non-hooked kernel** that ignores the `hook` field (or always sets it to `address(0)`) is fully compliant with this specification. The reference `ERC8183` contract supports both modes in a single contract: jobs with `hook == address(0)` skip all callbacks, and jobs with a whitelisted hook receive `beforeAction` / `afterAction` callbacks on the hookable functions listed below. +Implementations MAY support an optional **hook contract** per job to extend the core protocol without modifying it. The hook address is set at job creation (or `address(0)` for no hook) and stored on the job. A **non-hooked implementation** that ignores the `hook` field (or always sets it to `address(0)`) is fully compliant with this specification. The reference `ERC8183` contract supports both modes in a single contract: jobs with `hook == address(0)` skip all callbacks, and jobs with a whitelisted hook receive `beforeAction` / `afterAction` callbacks on the hookable functions listed below. A hook contract SHALL implement the `IERC8183Hook` interface — just two functions: @@ -202,11 +202,11 @@ function beforeAction(uint256 jobId, bytes4 selector, bytes calldata data) exter } ``` -When a job has a hook set, the core contract SHALL call `hook.beforeAction(...)` and `hook.afterAction(...)` around each hookable function. `createJob` is `afterAction`-only: the hook is attached during creation, so no `beforeAction` can fire (there is no attached hook before the job exists), but `afterAction` fires once the job is created so the hook can initialize per-job bookkeeping or revert to reject the job. Conformant non-hooked kernels simply ignore the field. +When a job has a hook set, the core contract SHALL call `hook.beforeAction(...)` and `hook.afterAction(...)` around each hookable function. `createJob` is `afterAction`-only — no hook is attached before the job exists — so the after-callback is where a hook initializes per-job bookkeeping or reverts to reject the job. Non-hooked implementations simply ignore the field. | Core function | Hookable | | -------------- | -------- | -| `createJob` | **Yes (`afterAction` only)** — hook is attached during creation; only the after-callback fires | +| `createJob` | **Yes (`afterAction` only)** | | `setPayoutReceiver` | Yes | | `setProvider` | Yes | | `setBudget` | Yes | @@ -240,7 +240,7 @@ The `data` parameter passed to hooks contains the core function's parameters enc | `rejectClaim` | `abi.encode(address actor, uint256 cumulativeAmount, bytes32 deliverable, bytes32 reason, bytes optParams)` | | `claimRefund` | `abi.encode(address actor, bytes optParams)` | -`actor` is the principal who authorized the action: `msg.sender` for direct calls, the EIP-712 signer for `*WithAuthorization` calls, and `msg.sender` (whoever triggered the refund) for the permissionless `claimRefund`. For `createJob`, `providerAgentId` is the canonicalized value stored on the job (zero for a providerless job), so it always agrees with `jobs[jobId]`. +`actor` is the principal who authorized the action: `msg.sender` for direct calls, the EIP-712 signer for `*WithAuthorization` calls, and `msg.sender` (whoever triggered the refund) for the permissionless `claimRefund`. For `createJob`, `providerAgentId` is the value stored on the job (zero for a providerless job), so it always agrees with `jobs[jobId]`. When `submit` or `reject` supersedes a pending claim, no claim-specific hook callback fires for the supersession itself; hooks observe the post-supersession claim state in the `submit`/`reject` callbacks. @@ -382,7 +382,7 @@ Implementations SHOULD emit at least: - **Disbursed**(jobId, receiver, selector, amount) — emitted after `IDisburser.onDisbursement` is invoked - **PlatformFeePaid**(jobId, platformTreasury, amount) — only emitted when a non-zero platform fee is taken - **EvaluatorFeePaid**(jobId, evaluator, amount) — only emitted when a non-zero evaluator fee is taken -- **Refunded**(jobId, client, amount) +- **Refunded**(jobId, recipient, amount) — recipient is the client, except under a `forceRefund` destination override (see `ForceRefunded` below) - **Settled**(jobId, cumulativeAmount, delta) — emitted on every settlement regardless of path - **ClaimSubmitted**(jobId, provider, cumulativeAmount, delta, deliverable, optParams) — provider files a pending claim; `optParams` is emitted so the exact claim preimage can be propagated to observers - **ClaimSettled**(jobId, settler, cumulativeAmount, delta, deliverable) — client-initiated settlement; `deliverable` is the settler's attestation, not a verified provider claim @@ -391,7 +391,7 @@ Implementations SHOULD emit at least: Note that `PaymentReleased`, `PlatformFeePaid`, and `EvaluatorFeePaid` fire on each settlement, not only on completion. -Implementations that add admin tooling SHOULD also emit operational events (e.g. `HookWhitelistUpdated`, `PaymentTokenAllowlistUpdated`, `HookDetached`, `PlatformFeeUpdated`, `EvaluatorFeeUpdated`, `EmergencyWithdraw`) so off-chain indexers can track configuration changes. +Implementations that add admin tooling SHOULD also emit operational events (e.g. `HookWhitelistUpdated`, `PaymentTokenAllowlistUpdated`, `HookDetached`, `PlatformFeeUpdated`, `EvaluatorFeeUpdated`, `ForceRefunded`, `EmergencyWithdraw`) so off-chain indexers can track configuration changes. ## Rationale @@ -565,7 +565,7 @@ function DOMAIN_SEPARATOR() external view returns (bytes32); - **Reentrancy:** Functions that transfer tokens SHALL be protected (e.g. reentrancy guard). Claim settlement transfers tokens mid-lifecycle (not only terminally), so effects-before-transfers ordering (update `settledAmount`, clear the pending claim, then transfer) is mandatory, not advisory. - **Tokens:** Use SafeERC-20 or equivalent for [ERC-20](./eip-20.md). - **Evaluator:** MUST be set at creation; if "client completes", pass `evaluator = client`. -- **Hook gas limits** (for hooked implementations): Implementations SHOULD impose a gas limit on hook calls (e.g. `call{gas: HOOK_GAS_LIMIT}(...)`) to bound execution cost and prevent hooks from consuming unbounded gas. The specific limit is left to the implementation as gas costs vary across chains. +- **Hook gas** (for hooked implementations): Implementations MAY impose a per-call gas limit on hook calls. The reference implementation instead relies on the whitelist audit — a hook that consumes unbounded gas is treated as blocking, per the audit obligations under [Hook security](#hook-security). - Hook contracts are client-supplied and trusted by the client; implementations MUST NOT allow hooks to modify core escrow state directly. Where `claimRefund` is hookable (as in the reference implementation), a reverting hook can block refunds after expiry; see [Hook security](#hook-security) for the whitelist-audit and break-glass mitigations. Any rescue path MUST close the job's accounting atomically with the payout so a rescued job cannot be refunded twice; a free-form escape hatch like `emergencyWithdraw` SHOULD be reserved for funds not attributed to any job. Implementations requiring an unconditional, trust-minimized post-expiry refund SHALL keep `claimRefund` non-hookable. - Jobs that use **advanced hooks** (e.g. two‑phase escrow / fund‑transfer hooks that custody additional tokens) are expected to have **more revert paths and tighter coupling** to external logic than plain, non‑hooked Agentic Commerce jobs. Such hooks SHOULD be reserved for agents and users who understand and accept this trade‑off; for most simple jobs, a non‑hooked or policy‑only hook is RECOMMENDED. From d62e262462d47255ffb9f9704a96a488b43d25ef Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 16 Jul 2026 18:16:41 +0800 Subject: [PATCH 26/32] docs: fix state-machine drift against reference implementation - Open row: budget is proposed by the provider via setBudget, not set by the client - add the Open -> Submitted transition for zero-budget jobs, which the submit spec already allows - createJob expiry bound is strict (expiredAt <= now + 5 minutes reverts), not 'at least 5 minutes' --- eip.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/eip.md b/eip.md index 33486ad..f684c49 100644 --- a/eip.md +++ b/eip.md @@ -32,7 +32,7 @@ A **job** has exactly one of six states: | State | Meaning | | ------------- | ----------------------------------------------------------------------------------------------------------------- | -| **Open** | Created; budget not yet set or not yet funded. Client may set budget, then fund or reject. | +| **Open** | Created; budget not yet set or not yet funded. Provider may propose a budget via `setBudget`; client may then fund, and either party may reject. | | **Funded** | Budget escrowed. Provider may submit work or file a settlement claim; client may settle partial amounts directly or approve/reject a pending claim; evaluator may approve/reject a pending claim or reject the job. After `expiredAt`, anyone may trigger refund of the unsettled remainder (`budget - settledAmount`, see [Job Data](#job-data)), provided no claim is pending. | | **Submitted** | Provider has submitted work. Only evaluator may complete or reject. After `expiredAt + EVALUATION_GRACE_PERIOD`, anyone may trigger refund. | | **Completed** | Terminal. Unsettled remainder released to provider (minus optional fees). | @@ -45,6 +45,7 @@ Allowed transitions: - **Open → Funded**: Provider calls `setBudget(jobId, token, amount)` to propose the price and payment token, then client accepts by calling `fund(jobId, expectedToken, expectedBudget)`; contract pulls `job.budget` of the job's payment token from client into escrow. - **Open → Rejected**: Client or provider calls `reject(jobId, reason?)`. - **Open → Expired**: When `block.timestamp >= job.expiredAt`, anyone may call `claimRefund(jobId)`; contract sets state to Expired. No refund to client as job has not been funded yet. +- **Open → Submitted**: zero-budget jobs only (`budget == 0`, no escrow): provider calls `submit(jobId, deliverable)` directly from Open (see `submit` under [Core Functions](#core-functions)). - **Funded → Submitted**: Provider calls `submit(jobId, deliverable)`; signals that work has been completed and is ready for evaluation. - **Funded → Rejected**: Evaluator calls `reject(jobId, reason?)`; contract refunds client. - **Funded → Expired**: When `block.timestamp >= job.expiredAt`, anyone may call `claimRefund(jobId)`; contract sets state to Expired and refunds client. @@ -93,7 +94,7 @@ SHALL revert if `job.provider == address(0)` (provider MUST be set before fundin ### Core Functions - **createJob(provider, evaluator, expiredAt, description, hook?, providerAgentId?)** -Called by client. Creates job in Open with `client = msg.sender`, `provider`, `evaluator`, `expiredAt`, `description`, optional `hook` address, and default `payoutReceiver = address(0)`. SHALL revert if `evaluator` is zero, if `expiredAt` is not at least 5 minutes in the future, if `provider == evaluator`, or if `msg.sender == provider`. **Provider MAY be zero**; if so, client MUST call `setProvider` before `fund`. `hook` MAY be `address(0)` (no hook); if non-zero, the hook MUST be admin-whitelisted and SHOULD advertise support for the `IERC8183Hook` interface via ERC-165. `providerAgentId` is the provider's [ERC-8004](./eip-8004.md) agent identity; if `provider` is non-zero and `providerAgentId` is non-zero, SHALL set `job.providerAgentId = providerAgentId`; the contract MAY verify that `provider` is the owner or operator of that `providerAgentId` on the ERC-8004 registry. Returns `jobId`. +Called by client. Creates job in Open with `client = msg.sender`, `provider`, `evaluator`, `expiredAt`, `description`, optional `hook` address, and default `payoutReceiver = address(0)`. SHALL revert if `evaluator` is zero, if `expiredAt <= block.timestamp + 5 minutes`, if `provider == evaluator`, or if `msg.sender == provider`. **Provider MAY be zero**; if so, client MUST call `setProvider` before `fund`. `hook` MAY be `address(0)` (no hook); if non-zero, the hook MUST be admin-whitelisted and SHOULD advertise support for the `IERC8183Hook` interface via ERC-165. `providerAgentId` is the provider's [ERC-8004](./eip-8004.md) agent identity; if `provider` is non-zero and `providerAgentId` is non-zero, SHALL set `job.providerAgentId = providerAgentId`; the contract MAY verify that `provider` is the owner or operator of that `providerAgentId` on the ERC-8004 registry. Returns `jobId`. - **setPayoutReceiver(jobId, payoutReceiver)** Called by provider. SHALL revert if job is not Open, the job has expired, caller is not the job's provider, `payoutReceiver` is the escrow contract itself, or the payment token is already set and `payoutReceiver == job.paymentToken`. SHALL set the provider-side payout recipient for the job. `payoutReceiver` MAY be `address(0)` to pay the provider directly. Implementations SHOULD emit `PayoutReceiverSet`. - **setProvider(jobId, provider, agentId?)** From c06aef4a8f7a1857a99eabf3dd5f9784bc11ee88 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 16 Jul 2026 18:18:18 +0800 Subject: [PATCH 27/32] docs: align job data types and IDisburser pragma with contracts - expiredAt is uint48 to match the interface and encoding tables - declare submittedAt in Job Data (Signed Authorizations binds it) - IDisburser snippet pragma matches contracts/IDisburser.sol (^0.8.28) --- eip.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/eip.md b/eip.md index f684c49..9a01ace 100644 --- a/eip.md +++ b/eip.md @@ -70,8 +70,9 @@ Each job SHALL have at least: - `client`, `provider`, `evaluator` (addresses). **Provider MAY be zero at creation** (see [Optional provider](#optional-provider-set-later) below). - `description` (string) — set at creation (e.g. job brief, scope reference). - `budget` (uint256) -- `expiredAt` (uint256 timestamp) +- `expiredAt` (uint48 timestamp) - `status` (Open | Funded | Submitted | Completed | Rejected | Expired) +- `submittedAt` (uint48 timestamp) — OPTIONAL for the core protocol; SHOULD be recorded on `submit` for grace-period accounting, REQUIRED when implementing Signed Authorizations (which bind it). Default `0` (not submitted). - `paymentToken` (address) — the [ERC-20](./eip-20.md) token used for payment on this job, set via `setBudget`. - `hook` (address) — OPTIONAL. External hook contract called before and after core functions (see [Hooks](#hooks-optional) below). MAY be `address(0)` (no hook). - `payoutReceiver` (address) — OPTIONAL. Provider-managed payout recipient. MAY be `address(0)` to pay the provider directly. Set via `setPayoutReceiver`. @@ -502,7 +503,7 @@ interface IERC8183Hook is IERC165 { ### IDisburser.sol ```solidity -pragma solidity ^0.8.20; +pragma solidity ^0.8.28; import "@openzeppelin/contracts/utils/introspection/IERC165.sol"; From b26c82e417821929f6e5875048dcb7639f546dd7 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 16 Jul 2026 18:33:22 +0800 Subject: [PATCH 28/32] docs: mark claimRefund optParams optional and drop dangling 'if provided' reason is always emitted in the complete/reject events; the optional marker on claimRefund's optParams matches every other signature --- eip.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/eip.md b/eip.md index 9a01ace..060bad8 100644 --- a/eip.md +++ b/eip.md @@ -107,10 +107,10 @@ Called by client. SHALL revert if job is not Open, caller is not client, **provi - **submit(jobId, deliverable, optParams?)** Called by provider only. SHALL revert if caller is not the job's provider, or if `job.expiredAt > 0` and the job has expired. SHALL revert if job is not Funded, unless the job is Open with `budget == 0` (zero-budget job, no escrow needed). SHALL set status to Submitted and SHOULD record `submittedAt = block.timestamp` for grace-period accounting. `deliverable` (`bytes32`) is a reference to submitted work (e.g. hash of off-chain deliverable, IPFS CID, attestation commitment). SHALL emit an event including `deliverable` (e.g. JobSubmitted). `optParams` forwarded to hook if set. - **complete(jobId, reason, optParams?)** -Called by evaluator only. SHALL revert if job is not Submitted or caller is not the job's evaluator. SHALL set status to Completed. SHALL transfer the unsettled remainder (`budget - settledAmount`) to the provider-side payout recipient, minus optional platform fee to a configurable treasury and optional evaluator fee paid to the evaluator address. `reason` MAY be `bytes32(0)` or an attestation hash (OPTIONAL). SHALL emit an event including `reason` if provided. `optParams` forwarded to hook if set. +Called by evaluator only. SHALL revert if job is not Submitted or caller is not the job's evaluator. SHALL set status to Completed. SHALL transfer the unsettled remainder (`budget - settledAmount`) to the provider-side payout recipient, minus optional platform fee to a configurable treasury and optional evaluator fee paid to the evaluator address. `reason` MAY be `bytes32(0)` or an attestation hash (OPTIONAL). SHALL emit an event including `reason`. `optParams` forwarded to hook if set. - **reject(jobId, reason, optParams?)** -Called by **client or provider when job is Open**, or by **evaluator when job is Funded or Submitted**. SHALL revert if job is not Open, Funded, or Submitted, or if the caller is not authorised for the current status. SHALL set status to Rejected. If Funded or Submitted, SHALL refund the unsettled remainder (`budget - settledAmount`) to client. If a settlement claim is pending, SHALL clear it (see [Claim interactions](#claim-interactions) below). `reason` OPTIONAL. SHALL emit an event including `reason` and the caller (rejector) if provided. `optParams` forwarded to hook if set. -- **claimRefund(jobId, optParams)** +Called by **client or provider when job is Open**, or by **evaluator when job is Funded or Submitted**. SHALL revert if job is not Open, Funded, or Submitted, or if the caller is not authorised for the current status. SHALL set status to Rejected. If Funded or Submitted, SHALL refund the unsettled remainder (`budget - settledAmount`) to client. If a settlement claim is pending, SHALL clear it (see [Claim interactions](#claim-interactions) below). `reason` OPTIONAL. SHALL emit an event including `reason` and the caller (rejector). `optParams` forwarded to hook if set. +- **claimRefund(jobId, optParams?)** Callable by anyone when status is Open, Funded, or Submitted. SHALL revert if status is Open or Funded and `block.timestamp < job.expiredAt`. SHALL revert if status is Submitted and `block.timestamp < job.expiredAt + EVALUATION_GRACE_PERIOD` (see the grace-period rationale under [State Machine](#state-machine)). SHALL revert if a settlement claim is pending on a non-Submitted job (claims can only arise while Funded) — pending claims MUST be resolved (approved, rejected, or withdrawn) before an expiry refund. SHALL set status to Expired. If the prior status was Funded or Submitted, SHALL transfer the unsettled remainder (`budget - settledAmount`), if non-zero, to the client. MAY be hookable; a reverting hook can then block the refund (see the liveness caveat under [**Hook security**](#hook-security)). Implementations requiring an unconditional post-expiry refund SHALL keep `claimRefund` non-hookable. > **Design rationale — refund forwarding for contract clients.** `claimRefund` is From 9d3e77c0212d53ec1215c3c3cd254b784b796830 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 16 Jul 2026 18:35:15 +0800 Subject: [PATCH 29/32] docs: deduplicate setProvider/fund specs and reframe hook gating - Optional-provider section shrinks to the flow description plus a cross-reference; Core Functions setProvider absorbs the ProviderSet emit and future-operator notes it lacked - hook gating at createJob: SHOULD gate attachment for the standard; whitelist + ERC-165 stated as reference-implementation behavior (consistent with Hook security's MAY on allowlists) --- eip.md | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/eip.md b/eip.md index 060bad8..4e07a35 100644 --- a/eip.md +++ b/eip.md @@ -85,21 +85,16 @@ Each job has its own [ERC-20](./eip-20.md) payment token. The token address is s ### Optional provider (set later) -Jobs MAY be created **without a provider** by passing `provider = address(0)` to `createJob`. In that case the client SHALL set the provider later via `setProvider(jobId, provider, agentId?)` before funding. This supports flows such as bidding or assignment after creation. - -- **setProvider(jobId, provider, agentId?)** -Called by **client** only. SHALL revert if job is not Open, the job has expired, the current `job.provider != address(0)`, `provider == address(0)`, `provider == job.client`, or `provider == job.evaluator`. SHALL set `job.provider = provider`. `agentId` is the provider's [ERC-8004](./eip-8004.md) agent identity; if non-zero, the contract MAY verify that `provider` is the owner or operator of that agentId on the ERC-8004 registry, and SHALL set `job.providerAgentId = agentId`. SHALL emit an event (e.g. ProviderSet) including the agentId. Implementations MAY allow an operator role to call setProvider in the future; this specification only requires client-only for the minimal protocol. -- **fund(jobId, expectedToken, expectedBudget)** -SHALL revert if `job.provider == address(0)` (provider MUST be set before funding), if `job.paymentToken != expectedToken`, or if `job.budget != expectedBudget` (front-running protection). +Jobs MAY be created **without a provider** by passing `provider = address(0)` to `createJob`. In that case the client SHALL set the provider later via `setProvider(jobId, provider, agentId?)` before funding — `fund` SHALL revert while `job.provider == address(0)`. This supports flows such as bidding or assignment after creation; see `setProvider` and `fund` under [Core Functions](#core-functions) for the full specifications. ### Core Functions - **createJob(provider, evaluator, expiredAt, description, hook?, providerAgentId?)** -Called by client. Creates job in Open with `client = msg.sender`, `provider`, `evaluator`, `expiredAt`, `description`, optional `hook` address, and default `payoutReceiver = address(0)`. SHALL revert if `evaluator` is zero, if `expiredAt <= block.timestamp + 5 minutes`, if `provider == evaluator`, or if `msg.sender == provider`. **Provider MAY be zero**; if so, client MUST call `setProvider` before `fund`. `hook` MAY be `address(0)` (no hook); if non-zero, the hook MUST be admin-whitelisted and SHOULD advertise support for the `IERC8183Hook` interface via ERC-165. `providerAgentId` is the provider's [ERC-8004](./eip-8004.md) agent identity; if `provider` is non-zero and `providerAgentId` is non-zero, SHALL set `job.providerAgentId = providerAgentId`; the contract MAY verify that `provider` is the owner or operator of that `providerAgentId` on the ERC-8004 registry. Returns `jobId`. +Called by client. Creates job in Open with `client = msg.sender`, `provider`, `evaluator`, `expiredAt`, `description`, optional `hook` address, and default `payoutReceiver = address(0)`. SHALL revert if `evaluator` is zero, if `expiredAt <= block.timestamp + 5 minutes`, if `provider == evaluator`, or if `msg.sender == provider`. **Provider MAY be zero**; if so, client MUST call `setProvider` before `fund`. `hook` MAY be `address(0)` (no hook); if non-zero, implementations SHOULD gate hook attachment (see [Hook security](#hook-security)) — the reference implementation requires the hook to be admin-whitelisted and to advertise support for the `IERC8183Hook` interface via ERC-165. `providerAgentId` is the provider's [ERC-8004](./eip-8004.md) agent identity; if `provider` is non-zero and `providerAgentId` is non-zero, SHALL set `job.providerAgentId = providerAgentId`; the contract MAY verify that `provider` is the owner or operator of that `providerAgentId` on the ERC-8004 registry. Returns `jobId`. - **setPayoutReceiver(jobId, payoutReceiver)** Called by provider. SHALL revert if job is not Open, the job has expired, caller is not the job's provider, `payoutReceiver` is the escrow contract itself, or the payment token is already set and `payoutReceiver == job.paymentToken`. SHALL set the provider-side payout recipient for the job. `payoutReceiver` MAY be `address(0)` to pay the provider directly. Implementations SHOULD emit `PayoutReceiverSet`. - **setProvider(jobId, provider, agentId?)** -Called by client. SHALL revert if job is not Open, has expired, current `job.provider != address(0)`, `provider == address(0)`, `provider == job.client`, or `provider == job.evaluator`. SHALL set `job.provider = provider`. `agentId` is the provider's [ERC-8004](./eip-8004.md) agent identity; if non-zero, SHALL set `job.providerAgentId = agentId`; the contract MAY verify that `provider` is the owner or operator of that agentId on the ERC-8004 registry. +Called by client. SHALL revert if job is not Open, has expired, current `job.provider != address(0)`, `provider == address(0)`, `provider == job.client`, or `provider == job.evaluator`. SHALL set `job.provider = provider`. `agentId` is the provider's [ERC-8004](./eip-8004.md) agent identity; if non-zero, SHALL set `job.providerAgentId = agentId`; the contract MAY verify that `provider` is the owner or operator of that agentId on the ERC-8004 registry. SHALL emit an event (e.g. ProviderSet) including the agentId. Implementations MAY allow an operator role to call setProvider in the future; this specification only requires client-only for the minimal protocol. - **setBudget(jobId, token, amount, optParams?)** Called by the job's provider. Sets `job.paymentToken = token` and `job.budget = amount`. SHALL revert if job is not Open, has expired, caller is not the provider, `token` is the zero address, or a nonzero `payoutReceiver` already equals `token`. Implementations SHOULD restrict `token` to an admin-managed allowlist of tokens with vetted ERC-20 semantics, to reject tokens that would break escrow accounting (e.g. fee-on-transfer, rebasing, transfer-hooked, pausable, or blacklist tokens); the reference implementation reverts with `PaymentTokenNotAllowed` for tokens not on the allowlist. `optParams` forwarded to hook if set. - **fund(jobId, expectedToken, expectedBudget, optParams?)** From be8fab3b8436f9734808a671b99351fc79adcd13 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 16 Jul 2026 20:28:40 +0800 Subject: [PATCH 30/32] docs: restore upstream gas-limit SHOULD and bind whitelist MUST to hookable claimRefund MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings on !13: - the gas-limit bullet predates this fork (present at 65bbbbb); restore its normative SHOULD verbatim and append the reference-impl fact — rescoping it belongs on the discussion thread - audit obligations now open with a conditional requirement: implementations that make claimRefund hookable MUST gate hook attachment behind an audit, closing the gap left by recasting createJob's whitelist MUST as a SHOULD --- eip.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/eip.md b/eip.md index 4e07a35..0c3abfa 100644 --- a/eip.md +++ b/eip.md @@ -252,7 +252,7 @@ When `submit` or `reject` supersedes a pending claim, no claim-specific hook cal - Hooks are **trusted** contracts chosen by the client at job creation. A malicious or buggy hook can revert valid actions or execute arbitrary logic in callbacks. Clients SHOULD audit or use well-known hook implementations. - **Liveness:** a reverting hook can block every hookable action, including the post-expiry refund; a hook that consumes unbounded gas blocks identically, so gas exhaustion counts as a revert throughout this section. The contract does not verify that a forwarded refund moved onward. See the design rationale under `claimRefund` in [Core Functions](#core-functions). -- **Audit obligations:** a whitelisted hook MUST NOT revert on `claimRefund` (except for a genuine forward failure), consume unbounded gas on any hooked selector, derive routing or authorization from the caller-supplied `optParams` on that permissionless path, or no-op a refund it is expected to forward. +- **Audit obligations:** implementations that make `claimRefund` hookable MUST gate hook attachment behind an audit (the reference implementation admin-whitelists hooks). An approved hook MUST NOT revert on `claimRefund` (except for a genuine forward failure), consume unbounded gas on any hooked selector, derive routing or authorization from the caller-supplied `optParams` on that permissionless path, or no-op a refund it is expected to forward. - **Break-glass:** under `pause()`, admin `forceRefund` applies `claimRefund`'s eligibility with hook callbacks skipped, pays the client (or an admin-chosen recipient when the client contract cannot receive funds), and expires the job atomically — a rescued job cannot be refunded twice. Implementations SHOULD emit a dedicated event (`ForceRefunded(jobId, admin, recipient, amount)` in the reference implementation). `batchDetachHook` suits non-forwarding hooks; `emergencyWithdraw` is reserved for funds not attributed to any job. Deployments that need an unconditional post-expiry refund SHOULD attach no hook (or a non-reverting hook). - **Atomicity:** After-callbacks run after state changes but within the same transaction. If an after-callback reverts, the entire transaction (including the core state change) is rolled back. This is intentional — it enables atomic multi-step flows (e.g. escrow funding + side token transfer must both succeed or both revert). - `onlyERC8183` modifiers on hooks are RECOMMENDED so that hook functions cannot be called directly by external actors. @@ -562,7 +562,7 @@ function DOMAIN_SEPARATOR() external view returns (bytes32); - **Reentrancy:** Functions that transfer tokens SHALL be protected (e.g. reentrancy guard). Claim settlement transfers tokens mid-lifecycle (not only terminally), so effects-before-transfers ordering (update `settledAmount`, clear the pending claim, then transfer) is mandatory, not advisory. - **Tokens:** Use SafeERC-20 or equivalent for [ERC-20](./eip-20.md). - **Evaluator:** MUST be set at creation; if "client completes", pass `evaluator = client`. -- **Hook gas** (for hooked implementations): Implementations MAY impose a per-call gas limit on hook calls. The reference implementation instead relies on the whitelist audit — a hook that consumes unbounded gas is treated as blocking, per the audit obligations under [Hook security](#hook-security). +- **Hook gas limits** (for hooked implementations): Implementations SHOULD impose a gas limit on hook calls (e.g. `call{gas: HOOK_GAS_LIMIT}(...)`) to bound execution cost and prevent hooks from consuming unbounded gas. The specific limit is left to the implementation as gas costs vary across chains. The reference implementation does not currently impose one; it relies on the whitelist audit, treating gas exhaustion as blocking per [Hook security](#hook-security). - Hook contracts are client-supplied and trusted by the client; implementations MUST NOT allow hooks to modify core escrow state directly. Where `claimRefund` is hookable (as in the reference implementation), a reverting hook can block refunds after expiry; see [Hook security](#hook-security) for the whitelist-audit and break-glass mitigations. Any rescue path MUST close the job's accounting atomically with the payout so a rescued job cannot be refunded twice; a free-form escape hatch like `emergencyWithdraw` SHOULD be reserved for funds not attributed to any job. Implementations requiring an unconditional, trust-minimized post-expiry refund SHALL keep `claimRefund` non-hookable. - Jobs that use **advanced hooks** (e.g. two‑phase escrow / fund‑transfer hooks that custody additional tokens) are expected to have **more revert paths and tighter coupling** to external logic than plain, non‑hooked Agentic Commerce jobs. Such hooks SHOULD be reserved for agents and users who understand and accept this trade‑off; for most simple jobs, a non‑hooked or policy‑only hook is RECOMMENDED. From cc93e30094f49e9aa9d3b9a513580ae7416af128 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 16 Jul 2026 20:28:40 +0800 Subject: [PATCH 31/32] fix: rename Refunded event param client to recipient forceRefund can pay an admin-chosen recipient, so the declaration's param name was misleading for indexers. Topics hash types only; the rename is ABI-compatible. 97/97 tests pass. --- contracts/ERC8183.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/ERC8183.sol b/contracts/ERC8183.sol index 56e1c43..24f4262 100644 --- a/contracts/ERC8183.sol +++ b/contracts/ERC8183.sol @@ -184,10 +184,10 @@ contract ERC8183 is Initializable, AccessControlUpgradeable, PausableUpgradeable address indexed evaluator, uint256 amount ); - /// @notice Emitted when escrowed funds are returned to the client + /// @notice Emitted when escrowed funds are refunded; recipient is the client except under a forceRefund destination override event Refunded( uint256 indexed jobId, - address indexed client, + address indexed recipient, uint256 amount ); /// @notice Emitted alongside Refunded when forceRefund pays out, attributing the admin From fbc2fc8f7668fcd1038a87fff4c843f7911fcfc2 Mon Sep 17 00:00:00 2001 From: "nicholas.yong" Date: Thu, 16 Jul 2026 20:33:31 +0800 Subject: [PATCH 32/32] docs: fix mermaid parse errors from semicolons in diagram text Mermaid treats ';' as a statement separator even inside message and note text, so both the claim-settlement and payout-receiver sequence diagrams failed to render on GitLab. --- docs/01-architecture.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/01-architecture.md b/docs/01-architecture.md index 835753d..765028c 100644 --- a/docs/01-architecture.md +++ b/docs/01-architecture.md @@ -78,11 +78,11 @@ sequenceDiagram Note over AC: pending claim hash stored C->>AC: approveClaim(jobId, cumulativeAmount, deliverable, optParams) Note over AC: 💸 delta released through payout receiver - AC-->>R: transfer(net); optional onDisbursement(..., approveClaim.selector, ...) + AC-->>R: transfer(net) + optional onDisbursement(..., approveClaim.selector, ...) else Fast path: client-authorized settlement C->>AC: settleClaim(jobId, cumulativeAmount, deliverable, optParams) Note over AC: 💸 delta released immediately through payout receiver - AC-->>R: transfer(net); optional onDisbursement(..., settleClaim.selector, ...) + AC-->>R: transfer(net) + optional onDisbursement(..., settleClaim.selector, ...) end Note over C,AC: Relayed fast path: client signs SettleClaimAuthorization,
relayer calls settleClaimWithAuthorization(...) @@ -148,9 +148,9 @@ sequenceDiagram participant R as PayoutReceiver C->>AC: createJob(..., hook, agentId) - Note over AC: payoutReceiver defaults to address(0); pays provider + Note over AC: payoutReceiver defaults to address(0), pays provider P->>AC: setPayoutReceiver(jobId, newReceiver) - Note over AC: provider-only, Open only; locked once Funded + Note over AC: provider-only while Open, locked once Funded P->>AC: setBudget(jobId, token, amount, "0x") C->>AC: fund(jobId, expectedToken, expectedBudget, "0x") Note over AC: Status: Funded @@ -158,7 +158,7 @@ sequenceDiagram alt Provider claim settlement P->>AC: submitClaim(jobId, amount, deliverable, optParams) E->>AC: approveClaim(jobId, amount, deliverable, optParams) - Note over AC: Pending claim approved;
delta released through payout receiver + Note over AC: Pending claim approved,
delta released through payout receiver else Fast client settlement C->>AC: settleClaim(jobId, amount, deliverable, optParams) Note over AC: Delta released immediately through payout receiver