diff --git a/README.md b/README.md index 326ada8..5b2d52e 100644 --- a/README.md +++ b/README.md @@ -119,6 +119,7 @@ operations page or guide with the exact commands. reconcile with `make roles-check`. See [roles](docs/roles.md). - **Configure v2 pool features.** [Rate limits](docs/operations/rate-limits.md), [CCV](docs/operations/ccv.md), [hooks and allowlist](docs/operations/hooks-allowlist.md), + [policy engine](docs/operations/policy-engine.md), [fees](docs/operations/fees.md), [finality](docs/operations/finality.md), [dynamic config](docs/operations/dynamic-config.md). - **LockRelease liquidity.** The rebalancer and ERC20 LockBox models, side by side, in diff --git a/docs/guides/index.md b/docs/guides/index.md index 19526d7..6970974 100644 --- a/docs/guides/index.md +++ b/docs/guides/index.md @@ -20,6 +20,7 @@ concept page, or a dedicated guide. Grouping is for navigation only. - [Required verifiers (CCV) per lane](../operations/ccv.md). - [Advanced Pool Hooks and the additional-CCV threshold](../operations/hooks-allowlist.md). +- [ACE Policy Engine on the hooks](../operations/policy-engine.md). - [Per-lane token-transfer fees](../operations/fees.md). - [Fast finality and fast-finality rate limits](../operations/finality.md). - [Rate limits (set, pause, remove) and the version deltas](../operations/rate-limits.md). diff --git a/docs/operations/hooks-allowlist.md b/docs/operations/hooks-allowlist.md index 69681b1..b711d7a 100644 --- a/docs/operations/hooks-allowlist.md +++ b/docs/operations/hooks-allowlist.md @@ -20,6 +20,9 @@ callers on hooks and lock boxes. Scripts under `script/configure/allowlist/` and [`UpdateAuthorizedCallers`](../primitives/authorized-callers/UpdateAuthorizedCallers.md), [`GetAuthorizedCallers`](../primitives/authorized-callers/GetAuthorizedCallers.md). +For the policy engine surface of the same hooks contract, see +[Policy engine](policy-engine.md). + ## Deploy Advanced Pool Hooks Use this for enhanced security features like allowlists, CCV management, policy engine integration, and diff --git a/docs/operations/policy-engine.md b/docs/operations/policy-engine.md new file mode 100644 index 0000000..8373158 --- /dev/null +++ b/docs/operations/policy-engine.md @@ -0,0 +1,123 @@ +--- +type: reference +--- + +# Policy engine + +> After applying a change here, sync your declared source of truth (the project store) and reconcile it +> against live with `make doctor CHAIN=`: see +> [Applying config and reconciling with doctor](../config-architecture.md#applying-config-and-reconciling-with-doctor). +> For this page specifically the sync command is `make snapshot-chain CHAIN=`: `hooks.policyEngine` +> is a governance-critical `roles{}` field, and the old declaration fails `make roles-check` until it is +> re-snapshotted. + +Connect an `AdvancedPoolHooks` contract to a [Chainlink ACE](https://docs.chain.link/ace) Policy Engine +on the same chain, or disconnect the engine. Scripts under `script/configure/policy-engine/`. Primitive +pages: [`SetPolicyEngine`](../primitives/policy-engine/SetPolicyEngine.md), +[`GetPolicyEngine`](../primitives/policy-engine/GetPolicyEngine.md). + +The hook forwards each transfer to the engine for evaluation before the source pool locks or burns +tokens (`preflightCheck`) and before the destination pool releases or mints tokens +(`postflightCheck`). If a policy rejects the call, the hook reverts and the transfer does not proceed. +For the Platform-side workflow (target detection, the built-in `CCIP-AdvancedPoolHooks` contract type, +policy instances, protections, and extractor mappings), see +[Protect CCIP Token Pools with ACE](https://docs.chain.link/ace/guides/policy-manager/ccip-token-pools). + +## Before you set an engine + +Two preconditions must hold before the swap lands, or every transfer on the affected lanes stops: + +- **Provision the engine for this hooks address first.** Extractors, policies, and the per-target + default all live on the engine, keyed by the hooks address; nothing carries over from the old + engine. With the ACE default of reject, an unprovisioned engine rejects every `ccipSend` from the + moment the swap lands (`PolicyRunRejected`). Provision the new engine (extractor, policies, + `setTargetDefaultPolicyAllow`) while the old engine (or none) is still attached, then swap. +- **Budget `destGasOverhead` for `postflightCheck` on every lane into this chain.** `postflightCheck` + runs inside `releaseOrMint` on the destination, within the gas the source pool quotes as + `destGasOverhead` for that lane. The FeeQuoter default is 90,000, and one policy alone can exceed it + (a local measurement of one `VolumePolicy` in `postflightCheck` runs ~97,000 gas; a live run with + Sanctions, KYC and Volume policies measured ~217,000). A short budget ends in an out-of-gas inside + the pool call, and the message goes to FAILURE with `TokenHandlingError(token, 0x)` (empty bytes, + unlike a real policy rejection, which carries `PolicyRunRejected`) and needs a manual execution. + Raise it on the source pool of every lane into the engine's chain with + [`UpdateTokenTransferFeeConfig`](fees.md) (`DEST_GAS_OVERHEAD=360000`, `DEST_BYTES_OVERHEAD` of at + least 32), and declare it in `lanes..v2.feeConfig`. The larger overhead is charged to the + sender. + +Also note the ACE version difference: from ACE 1.1.1 the engine's `run()` reverts `TargetNotAttached` +for a target that is not attached, while 1.0.0 does not gate on it. A local test against 1.0.0 can +therefore pass a flow a live 1.1.1+ engine rejects. + +## Set the policy engine + +Point the hooks at the engine deployed on the same chain. The hook calls `attach()` on the engine, +which starts ACE target detection. Only the hooks owner can call `setPolicyEngine`; when a Safe owns +the hooks, run it with `MODE=safe` (see [governance modes](../governance-modes.md)). + +```bash +POOL_HOOKS=0x... \ + POLICY_ENGINE=0x... \ + forge script \ + script/configure/policy-engine/SetPolicyEngine.s.sol \ + --rpc-url \ + $ETHEREUM_SEPOLIA_RPC_URL \ + --account \ + $KEYSTORE_NAME \ + --broadcast +``` + +The zero address disconnects the engine and stops policy checks: + +```bash +POOL_HOOKS=0x... \ + POLICY_ENGINE=0x0000000000000000000000000000000000000000 \ + forge script \ + script/configure/policy-engine/SetPolicyEngine.s.sol \ + --rpc-url \ + $ETHEREUM_SEPOLIA_RPC_URL \ + --account \ + $KEYSTORE_NAME \ + --broadcast +``` + +The script refuses two inputs before broadcasting: a same-address update (a silent no-op on the hook +that would report success) and an engine address with no code on this chain (the hook's `attach()` call +would revert with empty data). + +`setPolicyEngine` detaches the old engine first and reverts `PolicyEngineDetachReverted` when the old +engine's `detach()` reverts. The recovery path is the same script with `ALLOW_FAILED_DETACH=true`, +which calls `setPolicyEngineAllowFailedDetach` on the hook: + +```bash +POOL_HOOKS=0x... \ + POLICY_ENGINE=0x... \ + ALLOW_FAILED_DETACH=true \ + forge script \ + script/configure/policy-engine/SetPolicyEngine.s.sol \ + --rpc-url \ + $ETHEREUM_SEPOLIA_RPC_URL \ + --account \ + $KEYSTORE_NAME \ + --broadcast +``` + +The old engine keeps the hook listed as attached after a forced move, so pointing back at it later +reverts `TargetAlreadyAttached` until that engine drops the hook. + +After the swap, re-snapshot the declared roles: `make snapshot-chain CHAIN=` records +`hooks.policyEngine` in `roles{}`. Until then, `make roles-check` reports the moved governance slot, +which the [roles runbook](../roles.md#the-drift-response-runbook) says to treat as a potential +compromise. + +## Get the policy engine + +Reads the engine address currently set on the hooks contract, or reports that none is set (policy +checks disabled). + +```bash +POOL_HOOKS=0x... \ + forge script \ + script/configure/policy-engine/GetPolicyEngine.s.sol \ + --rpc-url \ + $ETHEREUM_SEPOLIA_RPC_URL +``` diff --git a/docs/primitives/_meta.json b/docs/primitives/_meta.json index 686ce38..259a583 100644 --- a/docs/primitives/_meta.json +++ b/docs/primitives/_meta.json @@ -47,5 +47,23 @@ "INBOUND_RATE_LIMIT_RATE": "uint128 token-bucket refill rate (inbound).", "INBOUND_RATE_LIMIT_ENABLED": "true/false; defaults to true when CAPACITY or RATE are set. false stands alone as a disable." } + }, + "SetPolicyEngine": { + "destructive": true, + "when_to_use": "Connect an AdvancedPoolHooks contract to an ACE Policy Engine on the same chain, or disconnect the engine with the zero address. The hook calls attach() on the new engine, which starts ACE target detection.", + "preconditions": "The executing account is the hooks owner (MODE=safe when a Safe owns it). The engine is deployed on the same chain and already provisioned for this hooks address: extractor, policies, and setTargetDefaultPolicyAllow. Every lane into this chain quotes a destGasOverhead that covers postflightCheck (UpdateTokenTransferFeeConfig).", + "postconditions": "getPolicyEngine returns the new engine. Run make snapshot-chain: hooks.policyEngine is a governance-critical roles{} field and the old declaration fails roles-check until re-snapshotted.", + "failure_modes": "DESTRUCTIVE: the zero address turns compliance checks off, and an unprovisioned engine rejects every transfer (PolicyRunRejected). An engine address with no code and a same-address update are refused before broadcasting. Reverts PolicyEngineDetachReverted when the old engine's detach() reverts; rerun with ALLOW_FAILED_DETACH=true. Pointing back at an engine left behind that way reverts TargetAlreadyAttached. Too little destGasOverhead fails inbound messages with TokenHandlingError(token, 0x).", + "inputs": { + "POLICY_ENGINE": "Address of the ACE Policy Engine on the same chain, or the zero address to disconnect the engine and stop policy checks.", + "POOL_HOOKS": "Address of the AdvancedPoolHooks contract. Resolves via the standard ladder: inline alias > {CHAIN}_POOL_HOOKS env > registry active.poolHooks.", + "ALLOW_FAILED_DETACH": "true calls setPolicyEngineAllowFailedDetach, tolerating a reverting detach() on the old engine (default false)." + } + }, + "GetPolicyEngine": { + "when_to_use": "Read which ACE Policy Engine an AdvancedPoolHooks contract points at, or confirm none is set (policy checks disabled).", + "inputs": { + "POOL_HOOKS": "Address of the AdvancedPoolHooks contract. Resolves via the standard ladder: inline alias > {CHAIN}_POOL_HOOKS env > registry active.poolHooks." + } } } diff --git a/docs/primitives/catalog.json b/docs/primitives/catalog.json index 5445b6b..407ae12 100644 --- a/docs/primitives/catalog.json +++ b/docs/primitives/catalog.json @@ -2,7 +2,7 @@ "generatedBy": "script/docs/gen-primitives.mjs", "note": "Generated from the script/ tree and the Makefile. Do not edit by hand; run `npm run docs:catalog`.", "counts": { - "primitives": 69, + "primitives": 71, "makeTargets": 31 }, "primitives": [ @@ -444,6 +444,39 @@ "AMOUNT" ] }, + { + "name": "GetPolicyEngine", + "script": "script/configure/policy-engine/GetPolicyEngine.s.sol", + "group": "policy-engine", + "description": "Script to read the ACE Policy Engine address currently set on an AdvancedPoolHooks contract.", + "modes": [ + "read" + ], + "read_only": true, + "writes_onchain": false, + "destructive": false, + "inputs": [ + "POOL_HOOKS" + ] + }, + { + "name": "SetPolicyEngine", + "script": "script/configure/policy-engine/SetPolicyEngine.s.sol", + "group": "policy-engine", + "description": "Script to point an AdvancedPoolHooks contract at an ACE Policy Engine on the same chain, or disconnect the engine with the zero address.", + "modes": [ + "eoa", + "safe" + ], + "read_only": false, + "writes_onchain": true, + "destructive": true, + "inputs": [ + "ALLOW_FAILED_DETACH", + "POLICY_ENGINE", + "POOL_HOOKS" + ] + }, { "name": "GetCurrentRateLimits", "script": "script/configure/rate-limiter/GetCurrentRateLimits.s.sol", diff --git a/docs/primitives/index.md b/docs/primitives/index.md index 8b31442..fb3cd38 100644 --- a/docs/primitives/index.md +++ b/docs/primitives/index.md @@ -91,6 +91,11 @@ at [`catalog.json`](catalog.json). These pages are generated from the scripts an - [AcceptOwnership](ownership/AcceptOwnership.md) - Completes a two-step ownership transfer initiated by TransferOwnership for any Ownable contract (a token pool, pool hooks, or a lockbox). _(write)_ - [TransferOwnership](ownership/TransferOwnership.md) - Initiates a two-step ownership transfer for any Ownable contract (a token pool, pool hooks, or a lockbox). _(write)_ +## policy-engine + +- [GetPolicyEngine](policy-engine/GetPolicyEngine.md) - Script to read the ACE Policy Engine address currently set on an AdvancedPoolHooks contract. _(read-only)_ +- [SetPolicyEngine](policy-engine/SetPolicyEngine.md) - Script to point an AdvancedPoolHooks contract at an ACE Policy Engine on the same chain, or disconnect the engine with the zero address. _(write, destructive)_ + ## rate-limiter - [GetCurrentRateLimits](rate-limiter/GetCurrentRateLimits.md) - Reads and displays the current rate limiter state for a TokenPool, compatible with v1 and v2 pools. _(read-only)_ diff --git a/docs/primitives/policy-engine/GetPolicyEngine.md b/docs/primitives/policy-engine/GetPolicyEngine.md new file mode 100644 index 0000000..5754325 --- /dev/null +++ b/docs/primitives/policy-engine/GetPolicyEngine.md @@ -0,0 +1,32 @@ +--- +name: GetPolicyEngine +script: script/configure/policy-engine/GetPolicyEngine.s.sol +group: policy-engine +type: reference +modes: [read] +read_only: true +writes_onchain: false +destructive: false +--- + +# GetPolicyEngine + +Script to read the ACE Policy Engine address currently set on an AdvancedPoolHooks contract. + +**When to use.** Read which ACE Policy Engine an AdvancedPoolHooks contract points at, or confirm none is set (policy checks disabled). + +## Inputs + +| Env var | Description | +| --- | --- | +| `POOL_HOOKS` | Address of the AdvancedPoolHooks contract. Resolves via the standard ladder: inline alias > {CHAIN}_POOL_HOOKS env > registry active.poolHooks. | + +## Reference + +- Script: [`script/configure/policy-engine/GetPolicyEngine.s.sol`](../../../script/configure/policy-engine/GetPolicyEngine.s.sol) +- Modes: read +- Read-only: true | Writes on-chain: false | Destructive: false + +_This page is generated from the script by `script/docs/gen-primitives.mjs`. Edit the script's +`@notice` for the description, or `docs/primitives/_meta.json` for the authored context; do not edit +this file by hand._ diff --git a/docs/primitives/policy-engine/SetPolicyEngine.md b/docs/primitives/policy-engine/SetPolicyEngine.md new file mode 100644 index 0000000..b43d1a2 --- /dev/null +++ b/docs/primitives/policy-engine/SetPolicyEngine.md @@ -0,0 +1,46 @@ +--- +name: SetPolicyEngine +script: script/configure/policy-engine/SetPolicyEngine.s.sol +group: policy-engine +type: reference +modes: [eoa, safe] +read_only: false +writes_onchain: true +destructive: true +--- + +# SetPolicyEngine + +Script to point an AdvancedPoolHooks contract at an ACE Policy Engine on the same chain, or disconnect the engine with the zero address. + +**When to use.** Connect an AdvancedPoolHooks contract to an ACE Policy Engine on the same chain, or disconnect the engine with the zero address. The hook calls attach() on the new engine, which starts ACE target detection. + +## Inputs + +| Env var | Description | +| --- | --- | +| `ALLOW_FAILED_DETACH` | true calls setPolicyEngineAllowFailedDetach, tolerating a reverting detach() on the old engine (default false). | +| `POLICY_ENGINE` | Address of the ACE Policy Engine on the same chain, or the zero address to disconnect the engine and stop policy checks. | +| `POOL_HOOKS` | Address of the AdvancedPoolHooks contract. Resolves via the standard ladder: inline alias > {CHAIN}_POOL_HOOKS env > registry active.poolHooks. | + +## Preconditions + +The executing account is the hooks owner (MODE=safe when a Safe owns it). The engine is deployed on the same chain and already provisioned for this hooks address: extractor, policies, and setTargetDefaultPolicyAllow. Every lane into this chain quotes a destGasOverhead that covers postflightCheck (UpdateTokenTransferFeeConfig). + +## Postconditions + +getPolicyEngine returns the new engine. Run make snapshot-chain: hooks.policyEngine is a governance-critical roles{} field and the old declaration fails roles-check until re-snapshotted. + +## Known failure modes + +DESTRUCTIVE: the zero address turns compliance checks off, and an unprovisioned engine rejects every transfer (PolicyRunRejected). An engine address with no code and a same-address update are refused before broadcasting. Reverts PolicyEngineDetachReverted when the old engine's detach() reverts; rerun with ALLOW_FAILED_DETACH=true. Pointing back at an engine left behind that way reverts TargetAlreadyAttached. Too little destGasOverhead fails inbound messages with TokenHandlingError(token, 0x). + +## Reference + +- Script: [`script/configure/policy-engine/SetPolicyEngine.s.sol`](../../../script/configure/policy-engine/SetPolicyEngine.s.sol) +- Modes: eoa, safe +- Read-only: false | Writes on-chain: true | Destructive: true + +_This page is generated from the script by `script/docs/gen-primitives.mjs`. Edit the script's +`@notice` for the description, or `docs/primitives/_meta.json` for the authored context; do not edit +this file by hand._ diff --git a/package-lock.json b/package-lock.json index 8329dfe..55d5fe8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14,7 +14,8 @@ "@chainlink/contracts-ccip-1.6.0": "npm:@chainlink/contracts-ccip@1.6.0", "@chainlink/contracts-ccip-1.6.1": "npm:@chainlink/contracts-ccip@1.6.1", "@openzeppelin/contracts-4.8.3": "npm:@openzeppelin/contracts@4.8.3", - "@openzeppelin/contracts-5.3.0": "npm:@openzeppelin/contracts@5.3.0" + "@openzeppelin/contracts-5.3.0": "npm:@openzeppelin/contracts@5.3.0", + "@openzeppelin/contracts-upgradeable": "5.0.2" }, "devDependencies": { "solhint": "^6.2.4", @@ -35,6 +36,12 @@ "solady": "0.0.182" } }, + "node_modules/@arbitrum/nitro-contracts/node_modules/@openzeppelin/contracts": { + "version": "4.7.3", + "resolved": "https://registry.npmjs.org/@openzeppelin/contracts/-/contracts-4.7.3.tgz", + "integrity": "sha512-dGRS0agJzu8ybo44pCIf3xBaPQN/65AIXNgK8+4gzKd5kbvlqyxryUYVLJv7fK98Seyd2hDZzVEHSWAh0Bt1Yw==", + "license": "MIT" + }, "node_modules/@arbitrum/nitro-contracts/node_modules/@openzeppelin/contracts-upgradeable": { "version": "4.7.3", "resolved": "https://registry.npmjs.org/@openzeppelin/contracts-upgradeable/-/contracts-upgradeable-4.7.3.tgz", @@ -206,6 +213,12 @@ "integrity": "sha512-xSmezSupL+y9VkHZJGDoCBpmnB2ogM13ccaYDWqJTfS3dbuHkgjuwDFUmaFauBCboQMGB/S5UqUl2y54X99BmA==", "license": "MIT" }, + "node_modules/@chainlink/contracts-ccip-1.6.0/node_modules/@openzeppelin/contracts-upgradeable": { + "version": "4.9.6", + "resolved": "https://registry.npmjs.org/@openzeppelin/contracts-upgradeable/-/contracts-upgradeable-4.9.6.tgz", + "integrity": "sha512-m4iHazOsOCv1DgM7eD7GupTJ+NFVujRZt1wzddDPSVGpWdKq1SKkla5htKG7+IS4d2XOCtzkUNwRZ7Vq5aEUMA==", + "license": "MIT" + }, "node_modules/@chainlink/contracts-ccip-1.6.0/node_modules/@scroll-tech/contracts": { "version": "0.1.0", "resolved": "https://registry.npmjs.org/@scroll-tech/contracts/-/contracts-0.1.0.tgz", @@ -310,6 +323,12 @@ "integrity": "sha512-xSmezSupL+y9VkHZJGDoCBpmnB2ogM13ccaYDWqJTfS3dbuHkgjuwDFUmaFauBCboQMGB/S5UqUl2y54X99BmA==", "license": "MIT" }, + "node_modules/@chainlink/contracts-ccip-1.6.1/node_modules/@openzeppelin/contracts-upgradeable": { + "version": "4.9.6", + "resolved": "https://registry.npmjs.org/@openzeppelin/contracts-upgradeable/-/contracts-upgradeable-4.9.6.tgz", + "integrity": "sha512-m4iHazOsOCv1DgM7eD7GupTJ+NFVujRZt1wzddDPSVGpWdKq1SKkla5htKG7+IS4d2XOCtzkUNwRZ7Vq5aEUMA==", + "license": "MIT" + }, "node_modules/@chainlink/contracts-ccip-1.6.1/node_modules/@scroll-tech/contracts": { "version": "0.1.0", "resolved": "https://registry.npmjs.org/@scroll-tech/contracts/-/contracts-0.1.0.tgz", @@ -331,6 +350,12 @@ "node": ">=8" } }, + "node_modules/@chainlink/contracts/node_modules/@openzeppelin/contracts-upgradeable": { + "version": "4.9.6", + "resolved": "https://registry.npmjs.org/@openzeppelin/contracts-upgradeable/-/contracts-upgradeable-4.9.6.tgz", + "integrity": "sha512-m4iHazOsOCv1DgM7eD7GupTJ+NFVujRZt1wzddDPSVGpWdKq1SKkla5htKG7+IS4d2XOCtzkUNwRZ7Vq5aEUMA==", + "license": "MIT" + }, "node_modules/@changesets/apply-release-plan": { "version": "7.1.0", "resolved": "https://registry.npmjs.org/@changesets/apply-release-plan/-/apply-release-plan-7.1.0.tgz", @@ -893,7 +918,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "@ethersproject/abstract-signer": "^5.8.0", "@ethersproject/basex": "^5.8.0", @@ -924,7 +948,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "@ethersproject/abstract-signer": "^5.8.0", "@ethersproject/address": "^5.8.0", @@ -1011,7 +1034,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "@ethersproject/bytes": "^5.8.0", "@ethersproject/sha2": "^5.8.0" @@ -1174,7 +1196,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "@ethersproject/bignumber": "^5.8.0", "@ethersproject/bytes": "^5.8.0", @@ -1247,7 +1268,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "@ethersproject/bignumber": "^5.8.0", "@ethersproject/constants": "^5.8.0", @@ -1269,7 +1289,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "@ethersproject/abstract-provider": "^5.8.0", "@ethersproject/abstract-signer": "^5.8.0", @@ -1326,7 +1345,6 @@ } ], "license": "MIT", - "peer": true, "dependencies": { "@ethersproject/bytes": "^5.8.0", "@ethersproject/hash": "^5.8.0", @@ -1477,6 +1495,12 @@ "@openzeppelin/contracts-upgradeable": "4.7.3" } }, + "node_modules/@offchainlabs/upgrade-executor/node_modules/@openzeppelin/contracts": { + "version": "4.7.3", + "resolved": "https://registry.npmjs.org/@openzeppelin/contracts/-/contracts-4.7.3.tgz", + "integrity": "sha512-dGRS0agJzu8ybo44pCIf3xBaPQN/65AIXNgK8+4gzKd5kbvlqyxryUYVLJv7fK98Seyd2hDZzVEHSWAh0Bt1Yw==", + "license": "MIT" + }, "node_modules/@offchainlabs/upgrade-executor/node_modules/@openzeppelin/contracts-upgradeable": { "version": "4.7.3", "resolved": "https://registry.npmjs.org/@openzeppelin/contracts-upgradeable/-/contracts-upgradeable-4.7.3.tgz", @@ -1484,10 +1508,11 @@ "license": "MIT" }, "node_modules/@openzeppelin/contracts": { - "version": "4.7.3", - "resolved": "https://registry.npmjs.org/@openzeppelin/contracts/-/contracts-4.7.3.tgz", - "integrity": "sha512-dGRS0agJzu8ybo44pCIf3xBaPQN/65AIXNgK8+4gzKd5kbvlqyxryUYVLJv7fK98Seyd2hDZzVEHSWAh0Bt1Yw==", - "license": "MIT" + "version": "5.0.2", + "resolved": "https://registry.npmjs.org/@openzeppelin/contracts/-/contracts-5.0.2.tgz", + "integrity": "sha512-ytPc6eLGcHHnapAZ9S+5qsdomhjo6QBHTDRRBFfTxXIpsicMhVPouPgmUPebZZZGX7vt9USA+Z+0M0dSVtSUEA==", + "license": "MIT", + "peer": true }, "node_modules/@openzeppelin/contracts-4.7.3": { "name": "@openzeppelin/contracts", @@ -1532,10 +1557,13 @@ "license": "MIT" }, "node_modules/@openzeppelin/contracts-upgradeable": { - "version": "4.9.6", - "resolved": "https://registry.npmjs.org/@openzeppelin/contracts-upgradeable/-/contracts-upgradeable-4.9.6.tgz", - "integrity": "sha512-m4iHazOsOCv1DgM7eD7GupTJ+NFVujRZt1wzddDPSVGpWdKq1SKkla5htKG7+IS4d2XOCtzkUNwRZ7Vq5aEUMA==", - "license": "MIT" + "version": "5.0.2", + "resolved": "https://registry.npmjs.org/@openzeppelin/contracts-upgradeable/-/contracts-upgradeable-5.0.2.tgz", + "integrity": "sha512-0MmkHSHiW2NRFiT9/r5Lu4eJq5UJ4/tzlOgYXNAIj/ONkQTVnz22pLxDvp4C4uZ9he7ZFvGn3Driptn1/iU7tQ==", + "license": "MIT", + "peerDependencies": { + "@openzeppelin/contracts": "5.0.2" + } }, "node_modules/@pnpm/config.env-replace": { "version": "1.1.0", @@ -1656,6 +1684,7 @@ "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.16.0.tgz", "integrity": "sha512-UVJyE9MttOsBQIDKw1skb9nAwQuR5wuGD3+82K6JgJlm/Y+KI92oNsMNGZCYdDsVtRHSak0pcV5Dno5+4jh9sw==", "license": "MIT", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -1676,14 +1705,14 @@ "version": "3.0.0", "resolved": "https://registry.npmjs.org/aes-js/-/aes-js-3.0.0.tgz", "integrity": "sha512-H7wUZRn8WpTq9jocdxQ2c8x2sKo9ZVmzfRE13GiNJXfp7NcKYEdvl3vspKjXox6RIG2VtaRe4JFvxG4rqp2Zuw==", - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/ajv": { "version": "6.14.0", "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.14.0.tgz", "integrity": "sha512-IWrosm/yrn43eiKqkfkHis7QioDleaXQHdDVPKg0FSwwd/DuvyX79TZnFOnYpB7dcsFAMmtFztZuXPDvSePkFw==", "license": "MIT", + "peer": true, "dependencies": { "fast-deep-equal": "^3.1.1", "fast-json-stable-stringify": "^2.0.0", @@ -3764,8 +3793,7 @@ "version": "3.0.1", "resolved": "https://registry.npmjs.org/scrypt-js/-/scrypt-js-3.0.1.tgz", "integrity": "sha512-cdwTTnqPu0Hyvf5in5asVdZocVDTNRmR7XEcJuIzMjJeSHybHl7vpB66AzwTaIg6CLSbtjcxc8fqcySfnTkccA==", - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/semver": { "version": "7.7.4", diff --git a/package.json b/package.json index 74abfcc..f8482df 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,8 @@ "@chainlink/contracts-ccip-1.6.0": "npm:@chainlink/contracts-ccip@1.6.0", "@chainlink/contracts-ccip-1.6.1": "npm:@chainlink/contracts-ccip@1.6.1", "@openzeppelin/contracts-4.8.3": "npm:@openzeppelin/contracts@4.8.3", - "@openzeppelin/contracts-5.3.0": "npm:@openzeppelin/contracts@5.3.0" + "@openzeppelin/contracts-5.3.0": "npm:@openzeppelin/contracts@5.3.0", + "@openzeppelin/contracts-upgradeable": "5.0.2" }, "devDependencies": { "solhint": "^6.2.4", diff --git a/script/configure/policy-engine/GetPolicyEngine.s.sol b/script/configure/policy-engine/GetPolicyEngine.s.sol new file mode 100644 index 0000000..23111cd --- /dev/null +++ b/script/configure/policy-engine/GetPolicyEngine.s.sol @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {Script, console} from "forge-std/Script.sol"; +import {HelperConfig} from "../../HelperConfig.s.sol"; +import {AdvancedPoolHooks} from "@chainlink/contracts-ccip/contracts/pools/AdvancedPoolHooks.sol"; + +/** + * @title GetPolicyEngine + * @notice Script to read the ACE Policy Engine address currently set on an AdvancedPoolHooks contract. + * + * Usage: + * POOL_HOOKS=0x... forge script script/configure/policy-engine/GetPolicyEngine.s.sol --rpc-url $ETHEREUM_SEPOLIA_RPC_URL + */ +contract GetPolicyEngine is Script { + HelperConfig public helperConfig; + + function run() external { + helperConfig = new HelperConfig(); + uint256 chainId = block.chainid; + string memory chainName = helperConfig.getChainName(chainId); + + // POOL_HOOKS alias > {CHAIN}_POOL_HOOKS > registry active.poolHooks (no manual export needed). + address hooksAddress = vm.envOr("POOL_HOOKS", helperConfig.getDeployedPoolHooks(chainId)); + require( + hooksAddress != address(0), + string.concat( + "AdvancedPoolHooks not deployed. Set POOL_HOOKS env var or ", + helperConfig.getNetworkConfig(chainId).chainNameIdentifier, + "_POOL_HOOKS." + ) + ); + + console.log(""); + console.log("========================================"); + console.log(unicode"🔎 Get Policy Engine"); + console.log("========================================"); + console.log(string.concat("Chain: ", chainName)); + console.log(string.concat("Pool Hooks: ", vm.toString(hooksAddress))); + console.log(string.concat("Action: ", "View policy engine")); + console.log("========================================"); + console.log(""); + + address policyEngine = AdvancedPoolHooks(hooksAddress).getPolicyEngine(); + if (policyEngine == address(0)) { + console.log("No policy engine is set on these hooks."); + console.log(" Policy checks are disabled: preflightCheck/postflightCheck skip the engine."); + console.log(" Set one with SetPolicyEngine.s.sol (POLICY_ENGINE=
)."); + } else { + console.log(unicode"✅ Policy Engine:"); + console.log(string.concat(" ", vm.toString(policyEngine))); + } + + console.log(""); + console.log("========================================"); + console.log(string.concat("Pool Hooks: ", helperConfig.getExplorerUrl(chainId, "/address/", hooksAddress))); + console.log("========================================"); + console.log(""); + } +} diff --git a/script/configure/policy-engine/SetPolicyEngine.s.sol b/script/configure/policy-engine/SetPolicyEngine.s.sol new file mode 100644 index 0000000..22f2d92 --- /dev/null +++ b/script/configure/policy-engine/SetPolicyEngine.s.sol @@ -0,0 +1,134 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {console} from "forge-std/Script.sol"; +import {HelperConfig} from "../../HelperConfig.s.sol"; +import {AdvancedPoolHooks} from "@chainlink/contracts-ccip/contracts/pools/AdvancedPoolHooks.sol"; +import {CctActions} from "../../../src/actions/CctActions.sol"; +import {EoaExecutor} from "../../../src/base/EoaExecutor.s.sol"; + +/** + * @title SetPolicyEngine + * @notice Script to point an AdvancedPoolHooks contract at an ACE Policy Engine on the same chain, + * or disconnect the engine with the zero address. + * @dev Calls setPolicyEngine(newPolicyEngine) as the hooks owner. The hook calls attach() on the new + * engine, which starts ACE target detection. When an old engine is set, setPolicyEngine calls + * detach() on it first and reverts PolicyEngineDetachReverted if that call reverts; rerun with + * ALLOW_FAILED_DETACH=true to call setPolicyEngineAllowFailedDetach instead. + * + * Usage: + * POOL_HOOKS=0x... POLICY_ENGINE=0x... forge script script/configure/policy-engine/SetPolicyEngine.s.sol --rpc-url $ETHEREUM_SEPOLIA_RPC_URL --account $KEYSTORE_NAME --broadcast + * POOL_HOOKS=0x... POLICY_ENGINE=0x0000000000000000000000000000000000000000 forge script script/configure/policy-engine/SetPolicyEngine.s.sol --rpc-url $ETHEREUM_SEPOLIA_RPC_URL --account $KEYSTORE_NAME --broadcast + * + * Environment variables: + * POOL_HOOKS - address of the AdvancedPoolHooks contract (alias > {CHAIN}_POOL_HOOKS > registry) + * POLICY_ENGINE - address of the ACE Policy Engine on the same chain, or the zero address to disconnect + * ALLOW_FAILED_DETACH - true calls setPolicyEngineAllowFailedDetach, tolerating a reverting detach() + * on the old engine (default false) + */ +contract SetPolicyEngine is EoaExecutor { + HelperConfig public helperConfig; + + /// @dev The env-reading entrypoint: resolves the inputs and hands them to `runWith` below, + /// which carries all the behaviour. Tests drive `runWith` directly rather than exporting + /// POOL_HOOKS / POLICY_ENGINE, because `vm.setEnv` writes the whole forge PROCESS + /// environment and forge runs suites in parallel (see the note above + /// `BaseForkTest.deployTokenAndPoolFixture`). `runWith` rather than an overloaded `run`: + /// `forge script ` with no `--sig` refuses a contract whose ABI holds two `run` + /// entries, and no script in this repo overloads `run`. + function run() external { + helperConfig = new HelperConfig(); + uint256 chainId = block.chainid; + // POOL_HOOKS alias > {CHAIN}_POOL_HOOKS > registry active.poolHooks (no manual export needed). + address hooksAddress = vm.envOr("POOL_HOOKS", helperConfig.getDeployedPoolHooks(chainId)); + runWith(hooksAddress, vm.envAddress("POLICY_ENGINE"), vm.envOr("ALLOW_FAILED_DETACH", false)); + } + + /// @notice Point the `AdvancedPoolHooks` at `hooksAddress` at the ACE Policy Engine + /// `newPolicyEngine`, or disconnect with the zero address. Drive it directly with + /// `--sig "runWith(address,address,bool)"` to bypass the environment entirely. + function runWith(address hooksAddress, address newPolicyEngine, bool allowFailedDetach) public { + if (address(helperConfig) == address(0)) { + helperConfig = new HelperConfig(); + } + uint256 chainId = block.chainid; + string memory chainName = helperConfig.getChainName(chainId); + + require( + hooksAddress != address(0), + string.concat( + "AdvancedPoolHooks not deployed. Set POOL_HOOKS env var or ", + helperConfig.getNetworkConfig(chainId).chainNameIdentifier, + "_POOL_HOOKS." + ) + ); + + // Read the current engine before the write so the report can name what is being replaced. + address currentEngine = AdvancedPoolHooks(hooksAddress).getPolicyEngine(); + _checkNewEngine(currentEngine, newPolicyEngine); + + console.log(""); + console.log("========================================"); + console.log(unicode"🔗 Set Policy Engine"); + console.log("========================================"); + console.log(string.concat("Chain: ", chainName)); + console.log(string.concat("Pool Hooks: ", vm.toString(hooksAddress))); + console.log(string.concat("Action: ", "Set policy engine")); + console.log("========================================"); + console.log(""); + console.log(string.concat("Current Policy Engine: ", vm.toString(currentEngine))); + console.log(string.concat("New Policy Engine: ", vm.toString(newPolicyEngine))); + if (newPolicyEngine == address(0)) { + console.log(" The zero address disconnects the engine and stops policy checks."); + } + if (allowFailedDetach) { + console.log(" ALLOW_FAILED_DETACH=true: a reverting detach() on the old engine is tolerated."); + console.log(" The old engine keeps this hook listed as attached, so pointing back at it"); + console.log(" later reverts TargetAlreadyAttached until that engine drops the hook."); + } + console.log(""); + + _executeCalls( + allowFailedDetach + ? CctActions._setPolicyEngineAllowFailedDetach(hooksAddress, newPolicyEngine) + : CctActions._setPolicyEngine(hooksAddress, newPolicyEngine) + ); + + _logOperationOutcome(string.concat("set the policy engine on ", chainName)); + // hooks.policyEngine is a governance-critical roles{} field; the old declaration fails + // roles-check until re-snapshotted. + console.log( + string.concat( + _callsWereApplied() ? "Next: " : "After the Safe executes: ", + "make snapshot-chain CHAIN=", + helperConfig.getSelectorName(chainId), + " (records hooks.policyEngine in roles{})" + ) + ); + console.log("========================================"); + console.log(string.concat("Pool Hooks: ", helperConfig.getExplorerUrl(chainId, "/address/", hooksAddress))); + console.log("========================================"); + console.log(""); + } + + /// @dev A same-address update is a silent no-op on the hook, and a codeless engine makes attach() + /// revert with empty data; refuse both before broadcasting. + function _checkNewEngine(address currentEngine, address newPolicyEngine) internal view { + require( + newPolicyEngine != currentEngine, + string.concat( + "POLICY_ENGINE equals the current engine (", + vm.toString(currentEngine), + "). setPolicyEngine would be a no-op. Pass a different address." + ) + ); + require( + newPolicyEngine == address(0) || newPolicyEngine.code.length > 0, + string.concat( + "POLICY_ENGINE ", + vm.toString(newPolicyEngine), + " has no code on this chain. Pass the Policy Engine deployed on this chain, or 0x0 to disconnect." + ) + ); + } +} diff --git a/script/docs/gen-primitives.mjs b/script/docs/gen-primitives.mjs index f857551..5fdfe6c 100644 --- a/script/docs/gen-primitives.mjs +++ b/script/docs/gen-primitives.mjs @@ -59,6 +59,7 @@ const GROUPS = [ ["script/configure/allowlist", "allowlist"], ["script/configure/authorized-callers", "authorized-callers"], ["script/configure/ccv", "ccv"], + ["script/configure/policy-engine", "policy-engine"], ["script/configure/dynamic-config", "dynamic-config"], ["script/configure/fee-config", "fee-config"], ["script/configure/finality-config", "finality-config"], diff --git a/src/actions/CctActions.sol b/src/actions/CctActions.sol index 40a4f3d..ca3a1aa 100644 --- a/src/actions/CctActions.sol +++ b/src/actions/CctActions.sol @@ -473,6 +473,27 @@ library CctActions { return _one(hooks, abi.encodeCall(AdvancedPoolHooks.setThresholdAmount, (amount))); } + /// @notice Point an `AdvancedPoolHooks` (v2) at an ACE Policy Engine on the same chain, or + /// `address(0)` to disconnect the engine and stop policy checks. + /// @dev `setPolicyEngine` detaches the old engine first and reverts `PolicyEngineDetachReverted` + /// when the old engine's `detach()` reverts; `setPolicyEngineAllowFailedDetach` on the hook + /// itself is the on-chain recovery path for that case. Targets the hooks contract; the + /// executing account must be the hooks owner. + function _setPolicyEngine(address hooks, address newPolicyEngine) internal pure returns (Call[] memory) { + return _one(hooks, abi.encodeCall(AdvancedPoolHooks.setPolicyEngine, (newPolicyEngine))); + } + + /// @notice `_setPolicyEngine` that tolerates the old engine's `detach()` reverting. + /// @dev The old engine keeps the hook listed as attached, so pointing back at it later reverts + /// `TargetAlreadyAttached` until that engine drops the hook. + function _setPolicyEngineAllowFailedDetach(address hooks, address newPolicyEngine) + internal + pure + returns (Call[] memory) + { + return _one(hooks, abi.encodeCall(AdvancedPoolHooks.setPolicyEngineAllowFailedDetach, (newPolicyEngine))); + } + /// @notice Apply allowlist updates (`removes`, `adds`) on an `AdvancedPoolHooks` (v2) or a v1 pool - /// both expose the identical `applyAllowListUpdates(address[],address[])` selector. /// @dev IMMUTABILITY TRAP: on `AdvancedPoolHooks`, `allowlistEnabled` is fixed at deploy time from diff --git a/test/actions/PolicyEngineActions.t.sol b/test/actions/PolicyEngineActions.t.sol new file mode 100644 index 0000000..336fafe --- /dev/null +++ b/test/actions/PolicyEngineActions.t.sol @@ -0,0 +1,109 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {AdvancedPoolHooks} from "@chainlink/contracts-ccip/contracts/pools/AdvancedPoolHooks.sol"; +import {MockPolicyEngine} from "@chainlink/contracts-ccip/contracts/test/mocks/MockPolicyEngine.sol"; +import {CctActions} from "../../src/actions/CctActions.sol"; +import {DeployAdvancedPoolHooks} from "../../script/configure/allowlist/DeployAdvancedPoolHooks.s.sol"; +import {BaseForkTest} from "../BaseForkTest.t.sol"; + +/// @notice Fork parity tests for the policy-engine action layer. Hooks are deployed from the +/// repo's own `DeployAdvancedPoolHooks` script (driven by `script/input/advanced-pool-hooks.json`, +/// with the allowlist overridden via the `ALLOWLIST` env var); engine changes are exercised +/// through the `CctActions` builder and asserted via `getPolicyEngine` plus the mock +/// engine's own attached-target view. +/// @dev Engine writes go through the action layer (`_exec`), not the `SetPolicyEngine` script's +/// env interface: `vm.setEnv` writes the whole forge PROCESS environment and forge runs +/// suites in parallel, so a `POOL_HOOKS` set here races every other suite's fixture pins +/// (see the note above `BaseForkTest.deployTokenAndPoolFixture`). The script itself is a thin +/// wrapper over the same builder, so exercising the builder proves the script's calldata. +contract PolicyEngineActionsForkTest is BaseForkTest { + // A FIXED allowlist value shared by every test that runs the deploy script. `vm.setEnv` is process-wide, + // so keeping the value identical across suites makes the deploy deterministic under parallel runs. + address internal constant ALLOWED = address(0x00000000000000000000000000000000000000A1); + + address internal token; + address internal pool; + address internal owner; + + function setUp() public override { + super.setUp(); + (token, pool) = deployTokenAndPoolFixture(); + owner = _scriptBroadcaster(); + } + + /// @dev Runs the repo's DeployAdvancedPoolHooks script with a non-empty allowlist (ALLOWLIST env + /// override) and returns the deployed hooks address, recovered from the broadcaster's CREATE + /// nonce (the deployment file the script writes is racy under parallel suites). `POLICY_ENGINE` + /// is pinned to the zero address for the same reason `deployTokenAndPoolFixture` pins + /// `POOL_HOOKS`: a value left in the process environment by an earlier test would wire the + /// fresh hooks to a stale engine address. + function _deployHooks(address allowed) internal returns (AdvancedPoolHooks hooks) { + vm.setEnv("ALLOWLIST", vm.toString(allowed)); + vm.setEnv("POLICY_ENGINE", vm.toString(address(0))); + uint256 nonceBefore = vm.getNonce(owner); + new DeployAdvancedPoolHooks().run(); + hooks = AdvancedPoolHooks(vm.computeCreateAddress(owner, nonceBefore)); + assertGt(address(hooks).code.length, 0, "hooks not deployed at computed address"); + } + + // ───────────────────────────────────────────────────────────────────────── + // setPolicyEngine: attach, swap, disconnect, asserted via getPolicyEngine + the mock's view + // ───────────────────────────────────────────────────────────────────────── + + function test_PolicyEngine_SetSwapAndDisconnect() public { + AdvancedPoolHooks hooks = _deployHooks(ALLOWED); + MockPolicyEngine engine1 = new MockPolicyEngine(); + MockPolicyEngine engine2 = new MockPolicyEngine(); + + // No engine initially. + assertEq(hooks.getPolicyEngine(), address(0), "no engine initially"); + + // Attach: the hook calls attach() on the engine, so the engine records it as a target. + _exec(owner, CctActions._setPolicyEngine(address(hooks), address(engine1))); + assertEq(hooks.getPolicyEngine(), address(engine1), "engine1 set"); + assertTrue(engine1.isAttached(address(hooks)), "engine1 records the hook as attached"); + + // Swap: the hook detaches engine1 and attaches engine2. + _exec(owner, CctActions._setPolicyEngine(address(hooks), address(engine2))); + assertEq(hooks.getPolicyEngine(), address(engine2), "engine2 set"); + assertFalse(engine1.isAttached(address(hooks)), "engine1 detached"); + assertTrue(engine2.isAttached(address(hooks)), "engine2 records the hook as attached"); + + // Disconnect: the zero address detaches engine2 and stops policy checks. + _exec(owner, CctActions._setPolicyEngine(address(hooks), address(0))); + assertEq(hooks.getPolicyEngine(), address(0), "engine disconnected"); + assertFalse(engine2.isAttached(address(hooks)), "engine2 detached"); + } + + // ───────────────────────────────────────────────────────────────────────── + // Same-address no-op: the hook returns without a transaction; the state is unchanged. + // ───────────────────────────────────────────────────────────────────────── + + function test_PolicyEngine_SameAddressIsNoOp() public { + AdvancedPoolHooks hooks = _deployHooks(ALLOWED); + MockPolicyEngine engine = new MockPolicyEngine(); + + _exec(owner, CctActions._setPolicyEngine(address(hooks), address(engine))); + assertEq(hooks.getPolicyEngine(), address(engine), "engine set"); + + // Setting the same address again changes nothing: no detach, no attach, no event. + _exec(owner, CctActions._setPolicyEngine(address(hooks), address(engine))); + assertEq(hooks.getPolicyEngine(), address(engine), "engine unchanged"); + assertTrue(engine.isAttached(address(hooks)), "engine still attached"); + } + + // ───────────────────────────────────────────────────────────────────────── + // Action-layer parity: the builder encodes the exact selector the script hands the executor. + // ───────────────────────────────────────────────────────────────────────── + + function test_PolicyEngine_ActionBuilderMatchesSelector() public { + AdvancedPoolHooks hooks = _deployHooks(ALLOWED); + MockPolicyEngine engine = new MockPolicyEngine(); + + CctActions.Call[] memory set = CctActions._setPolicyEngine(address(hooks), address(engine)); + assertEq(set.length, 1, "one call"); + assertEq(set[0].target, address(hooks), "targets the hooks"); + assertEq(bytes4(set[0].data), AdvancedPoolHooks.setPolicyEngine.selector, "setPolicyEngine selector"); + } +} diff --git a/test/actions/PolicyEngineReviewFixes.t.sol b/test/actions/PolicyEngineReviewFixes.t.sol new file mode 100644 index 0000000..7e91431 --- /dev/null +++ b/test/actions/PolicyEngineReviewFixes.t.sol @@ -0,0 +1,246 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {AdvancedPoolHooks} from "@chainlink/contracts-ccip/contracts/pools/AdvancedPoolHooks.sol"; +import { + MockPolicyEngine, + MockPolicyEngineRevertingDetach +} from "@chainlink/contracts-ccip/contracts/test/mocks/MockPolicyEngine.sol"; +import {PolicyEngine} from "@chainlink/policy-management/core/PolicyEngine.sol"; +import {VolumePolicy} from "@chainlink/policy-management/policies/VolumePolicy.sol"; +import {IPolicyEngine} from "@chainlink/policy-management/interfaces/IPolicyEngine.sol"; +import {IExtractor} from "@chainlink/policy-management/interfaces/IExtractor.sol"; +import {Pool} from "@chainlink/contracts-ccip/contracts/libraries/Pool.sol"; +import {IAdvancedPoolHooks} from "@chainlink/contracts-ccip/contracts/interfaces/IAdvancedPoolHooks.sol"; +import {AuthorizedCallers} from "@chainlink/contracts/src/v0.8/shared/access/AuthorizedCallers.sol"; +import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; +import {CctActions} from "../../src/actions/CctActions.sol"; +import {DeployAdvancedPoolHooks} from "../../script/configure/allowlist/DeployAdvancedPoolHooks.s.sol"; +import {SetPolicyEngine} from "../../script/configure/policy-engine/SetPolicyEngine.s.sol"; +import {BaseForkTest} from "../BaseForkTest.t.sol"; + +/// @dev Test-local extractor: yields the postflightCheck localAmount as the "amount" parameter, the +/// shape the Platform's built-in CCIP extractor provides for these selectors. +contract PostflightAmountExtractor is IExtractor { + bytes32 public constant PARAM_AMOUNT = keccak256("amount"); + + function typeAndVersion() external pure returns (string memory) { + return "PostflightAmountExtractor 1.0.0"; + } + + function extract(IPolicyEngine.Payload calldata payload) external pure returns (IPolicyEngine.Parameter[] memory) { + (, uint256 localAmount,) = abi.decode(payload.data, (Pool.ReleaseOrMintInV1, uint256, bytes4)); + IPolicyEngine.Parameter[] memory result = new IPolicyEngine.Parameter[](1); + result[0] = IPolicyEngine.Parameter(PARAM_AMOUNT, abi.encode(localAmount)); + return result; + } +} + +/// @title PolicyEngineReviewFixes +/// @notice Validates the fixes for review findings F2, F3 and F4 through the SetPolicyEngine script +/// harness (`runWith`, the parameterised entrypoint that bypasses the process-wide env) and +/// the new `_setPolicyEngineAllowFailedDetach` builder. The findings themselves are proven +/// in PolicyEngineReviewProofs.t.sol. +contract PolicyEngineReviewFixesForkTest is BaseForkTest { + // A FIXED allowlist value shared by every test that runs the deploy script (vm.setEnv is + // process-wide, so an identical value keeps the deploy deterministic under parallel suites). + address internal constant ALLOWED = address(0x00000000000000000000000000000000000000A1); + + address internal owner; + + function setUp() public override { + super.setUp(); + owner = _scriptBroadcaster(); + } + + /// @dev Deploys hooks through the repo's deploy script with the shared fixed allowlist and the + /// engine pinned to zero (the same race-avoidance pins PolicyEngineActionsForkTest uses). + function _deployHooks() internal returns (AdvancedPoolHooks hooks) { + vm.setEnv("ALLOWLIST", vm.toString(ALLOWED)); + vm.setEnv("POLICY_ENGINE", vm.toString(address(0))); + uint256 nonceBefore = vm.getNonce(owner); + new DeployAdvancedPoolHooks().run(); + hooks = AdvancedPoolHooks(vm.computeCreateAddress(owner, nonceBefore)); + assertGt(address(hooks).code.length, 0, "hooks not deployed at computed address"); + } + + // ───────────────────────────────────────────────────────────────────────── + // F2 fix: the pre-broadcast refusals (codeless engine, same address) + // ───────────────────────────────────────────────────────────────────────── + + /// @dev A codeless POLICY_ENGINE is refused before broadcasting, with a named reason instead of + /// the empty-data revert the hook would produce. + function test_F2_fix_refusesCodelessEngine() public { + AdvancedPoolHooks hooks = _deployHooks(); + address eoa = makeAddr("codeless-engine"); + SetPolicyEngine script = new SetPolicyEngine(); + vm.expectRevert( + bytes( + string.concat( + "POLICY_ENGINE ", + vm.toString(eoa), + " has no code on this chain. Pass the Policy Engine deployed on this chain, or 0x0 to disconnect." + ) + ) + ); + script.runWith(address(hooks), eoa, false); + } + + /// @dev A deployed engine and the zero address both pass the pre-broadcast checks. + function test_F2_fix_acceptsDeployedEngineAndZero() public { + AdvancedPoolHooks hooks = _deployHooks(); + MockPolicyEngine engine = new MockPolicyEngine(); + SetPolicyEngine script = new SetPolicyEngine(); + + script.runWith(address(hooks), address(engine), false); + assertEq(hooks.getPolicyEngine(), address(engine), "a deployed engine is accepted"); + + script.runWith(address(hooks), address(0), false); + assertEq(hooks.getPolicyEngine(), address(0), "the zero address is accepted (disconnect)"); + } + + /// @dev A same-address update is still refused (the pre-existing check, unchanged by the fix). + function test_F2_fix_stillRefusesSameAddress() public { + AdvancedPoolHooks hooks = _deployHooks(); + MockPolicyEngine engine = new MockPolicyEngine(); + SetPolicyEngine script = new SetPolicyEngine(); + + script.runWith(address(hooks), address(engine), false); + + vm.expectRevert( + bytes( + string.concat( + "POLICY_ENGINE equals the current engine (", + vm.toString(address(engine)), + "). setPolicyEngine would be a no-op. Pass a different address." + ) + ) + ); + script.runWith(address(hooks), address(engine), false); + } + + // ───────────────────────────────────────────────────────────────────────── + // F3 fix: provision, then swap (the order the doc recommends) + // ───────────────────────────────────────────────────────────────────────── + + /// @dev Provisioning the new engine BEFORE the hook attaches works: setExtractor, + /// addPolicy(, ...) and setTargetDefaultPolicyAllow(, true) all succeed on a + /// not-yet-attached target, and after the swap a within-limits amount passes while an + /// over-limit amount reverts PolicyRunRejected. + function test_F3_fix_provisionThenSwap() public { + AdvancedPoolHooks hooks = _deployHooks(); + PolicyEngine engine = PolicyEngine(Clones.clone(address(new PolicyEngine()))); + engine.initialize(false, address(this)); + + // Provision for the hooks address BEFORE the hook attaches. + engine.setExtractor(AdvancedPoolHooks.postflightCheck.selector, address(new PostflightAmountExtractor())); + VolumePolicy policy = VolumePolicy(Clones.clone(address(new VolumePolicy()))); + policy.initialize(address(engine), address(this), abi.encode(0, 100)); + bytes32[] memory names = new bytes32[](1); + names[0] = keccak256("amount"); + engine.addPolicy(address(hooks), AdvancedPoolHooks.postflightCheck.selector, address(policy), names); + engine.setTargetDefaultPolicyAllow(address(hooks), true); + + // The test contract stands in for the pool as the postflightCheck caller. + address[] memory adds = new address[](1); + adds[0] = address(this); + vm.prank(hooks.owner()); + AuthorizedCallers(address(hooks)) + .applyAuthorizedCallerUpdates( + AuthorizedCallers.AuthorizedCallerArgs({addedCallers: adds, removedCallers: new address[](0)}) + ); + + // Swap: the provisioned engine accepts 50 and rejects 500. + _exec(owner, CctActions._setPolicyEngine(address(hooks), address(engine))); + Pool.ReleaseOrMintInV1 memory in_ = _releaseOrMintIn(); + + vm.prank(address(this)); + hooks.postflightCheck(in_, 50, bytes4(0)); + + vm.prank(address(this)); + vm.expectRevert( + abi.encodeWithSelector( + IPolicyEngine.PolicyRunRejected.selector, + address(policy), + "amount outside allowed volume limits", + _postflightPayload(500) + ) + ); + hooks.postflightCheck(in_, 500, bytes4(0)); + } + + // ───────────────────────────────────────────────────────────────────────── + // F4 fix: the ALLOW_FAILED_DETACH switch and its builder + // ───────────────────────────────────────────────────────────────────────── + + /// @dev The builder encodes setPolicyEngineAllowFailedDetach exactly, and executing it against an + /// engine whose detach() reverts emits PolicyEngineDetachFailed(bad, DetachNotSupported()) + /// and attaches the new engine. + function test_F4_fix_allowFailedDetachBuilder() public { + AdvancedPoolHooks hooks = _deployHooks(); + MockPolicyEngineRevertingDetach bad = new MockPolicyEngineRevertingDetach(); + MockPolicyEngine good = new MockPolicyEngine(); + + CctActions.Call[] memory calls = CctActions._setPolicyEngineAllowFailedDetach(address(hooks), address(good)); + assertEq(calls.length, 1, "one call"); + assertEq( + bytes4(calls[0].data), + AdvancedPoolHooks.setPolicyEngineAllowFailedDetach.selector, + "setPolicyEngineAllowFailedDetach selector" + ); + + _exec(owner, CctActions._setPolicyEngine(address(hooks), address(bad))); + vm.expectEmit(true, false, false, true); + emit AdvancedPoolHooks.PolicyEngineDetachFailed(address(bad), abi.encodeWithSignature("DetachNotSupported()")); + _exec(owner, calls); + assertEq(hooks.getPolicyEngine(), address(good), "the new engine is attached"); + assertTrue(good.isAttached(address(hooks)), "the new engine records the hook as attached"); + } + + /// @dev The script's ALLOW_FAILED_DETACH=true switch routes to the allow-failed-detach builder: + /// with a reverting old engine, the run completes and the new engine lands. + function test_F4_fix_scriptSwitchRoutesToAllowFailedDetach() public { + AdvancedPoolHooks hooks = _deployHooks(); + MockPolicyEngineRevertingDetach bad = new MockPolicyEngineRevertingDetach(); + MockPolicyEngine good = new MockPolicyEngine(); + + _exec(owner, CctActions._setPolicyEngine(address(hooks), address(bad))); + + SetPolicyEngine script = new SetPolicyEngine(); + script.runWith(address(hooks), address(good), true); + assertEq(hooks.getPolicyEngine(), address(good), "the switch must land the new engine"); + } + + // ───────────────────────────────────────────────────────────────────────── + // Helpers + // ───────────────────────────────────────────────────────────────────────── + + /// @dev A minimal inbound payload for a direct postflightCheck call, as the pool would make from + /// inside releaseOrMint. + function _releaseOrMintIn() internal pure returns (Pool.ReleaseOrMintInV1 memory) { + return Pool.ReleaseOrMintInV1({ + originalSender: abi.encode(address(0xB0B)), + remoteChainSelector: 8236463271206331221, + receiver: address(0xB0B), + sourceDenominatedAmount: 1e18, + localToken: address(0), + sourcePoolAddress: abi.encode(address(0x1111111111111111111111111111111111111111)), + sourcePoolData: abi.encode(uint256(18)), + offchainTokenData: "" + }); + } + + /// @dev The exact Payload the hook builds for postflightCheck with the given localAmount, for + /// the expectRevert equality check. + function _postflightPayload(uint256 localAmount) internal view returns (IPolicyEngine.Payload memory) { + bytes memory full = + abi.encodeCall(IAdvancedPoolHooks.postflightCheck, (_releaseOrMintIn(), localAmount, bytes4(0))); + bytes memory stripped = new bytes(full.length - 4); + for (uint256 i = 0; i < stripped.length; i++) { + stripped[i] = full[i + 4]; + } + return IPolicyEngine.Payload({ + selector: AdvancedPoolHooks.postflightCheck.selector, sender: address(this), data: stripped, context: "" + }); + } +} diff --git a/test/actions/PolicyEngineReviewProofs.t.sol b/test/actions/PolicyEngineReviewProofs.t.sol new file mode 100644 index 0000000..ca73d66 --- /dev/null +++ b/test/actions/PolicyEngineReviewProofs.t.sol @@ -0,0 +1,324 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {AdvancedPoolHooks} from "@chainlink/contracts-ccip/contracts/pools/AdvancedPoolHooks.sol"; +import {IAdvancedPoolHooks} from "@chainlink/contracts-ccip/contracts/interfaces/IAdvancedPoolHooks.sol"; +import { + MockPolicyEngine, + MockPolicyEngineRevertingDetach +} from "@chainlink/contracts-ccip/contracts/test/mocks/MockPolicyEngine.sol"; +import {PolicyEngine} from "@chainlink/policy-management/core/PolicyEngine.sol"; +import {VolumePolicy} from "@chainlink/policy-management/policies/VolumePolicy.sol"; +import {IPolicyEngine} from "@chainlink/policy-management/interfaces/IPolicyEngine.sol"; +import {IExtractor} from "@chainlink/policy-management/interfaces/IExtractor.sol"; +import {Pool} from "@chainlink/contracts-ccip/contracts/libraries/Pool.sol"; +import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; +import {AuthorizedCallers} from "@chainlink/contracts/src/v0.8/shared/access/AuthorizedCallers.sol"; +import {CctActions} from "../../src/actions/CctActions.sol"; +import {DeployAdvancedPoolHooks} from "../../script/configure/allowlist/DeployAdvancedPoolHooks.s.sol"; +import {RolesSnapshot} from "../../src/roles/RolesSnapshot.sol"; +import {RolesAuditor} from "../../src/roles/RolesAuditor.sol"; +import {BaseForkTest} from "../BaseForkTest.t.sol"; + +/// @dev Test-local extractor: yields the postflightCheck localAmount as the "amount" parameter, the +/// shape the Platform's built-in CCIP extractor provides for these selectors. +contract PostflightAmountExtractor is IExtractor { + bytes32 public constant PARAM_AMOUNT = keccak256("amount"); + + function typeAndVersion() external pure returns (string memory) { + return "PostflightAmountExtractor 1.0.0"; + } + + function extract(IPolicyEngine.Payload calldata payload) external pure returns (IPolicyEngine.Parameter[] memory) { + (, uint256 localAmount,) = abi.decode(payload.data, (Pool.ReleaseOrMintInV1, uint256, bytes4)); + IPolicyEngine.Parameter[] memory result = new IPolicyEngine.Parameter[](1); + result[0] = IPolicyEngine.Parameter(PARAM_AMOUNT, abi.encode(localAmount)); + return result; + } +} + +/// @title PolicyEngineReviewProofs +/// @notice Review findings F1-F4 and F6, each proven against the real AdvancedPoolHooks bytecode and +/// (where the finding is about engine behavior) the real ACE 1.0.0 PolicyEngine and +/// VolumePolicy, not the mock. The fixes are validated in PolicyEngineReviewFixes.t.sol. +contract PolicyEngineReviewProofsForkTest is BaseForkTest { + // A FIXED allowlist value shared by every test that runs the deploy script (vm.setEnv is + // process-wide, so an identical value keeps the deploy deterministic under parallel suites). + address internal constant ALLOWED = address(0x00000000000000000000000000000000000000A1); + + // The FeeQuoter default destGasOverhead a source pool quotes when no lane fee config is set. + uint32 internal constant DEFAULT_DEST_GAS_OVERHEAD = 90_000; + + address internal owner; + RolesSnapshot internal snap; + RolesAuditor internal auditor; + + function setUp() public override { + super.setUp(); + owner = _scriptBroadcaster(); + snap = new RolesSnapshot(); + auditor = new RolesAuditor(); + } + + /// @dev Deploys hooks through the repo's deploy script with the shared fixed allowlist and the + /// engine pinned to zero (the same race-avoidance pins PolicyEngineActionsForkTest uses). + function _deployHooks() internal returns (AdvancedPoolHooks hooks) { + vm.setEnv("ALLOWLIST", vm.toString(ALLOWED)); + vm.setEnv("POLICY_ENGINE", vm.toString(address(0))); + uint256 nonceBefore = vm.getNonce(owner); + new DeployAdvancedPoolHooks().run(); + hooks = AdvancedPoolHooks(vm.computeCreateAddress(owner, nonceBefore)); + assertGt(address(hooks).code.length, 0, "hooks not deployed at computed address"); + } + + /// @dev Deploys the REAL ACE 1.0.0 engine as a minimal proxy (the implementation's constructor + /// disables initializers, so initialize must run on a clone) with the given default posture. + function _deployRealEngine(bool defaultAllow) internal returns (PolicyEngine engine) { + PolicyEngine impl = new PolicyEngine(); + engine = PolicyEngine(Clones.clone(address(impl))); + engine.initialize(defaultAllow, address(this)); + } + + /// @dev Provisions the engine for the hooks' postflightCheck: an extractor that yields the + /// localAmount as "amount", a VolumePolicy bound to (min, max), and the policy registered + /// for the hooks address on the postflightCheck selector, mapped to "amount". + function _provisionVolumePolicy(PolicyEngine engine, address hooks, uint256 min, uint256 max) internal { + engine.setExtractor(AdvancedPoolHooks.postflightCheck.selector, address(new PostflightAmountExtractor())); + VolumePolicy policy = VolumePolicy(Clones.clone(address(new VolumePolicy()))); + policy.initialize(address(engine), address(this), abi.encode(min, max)); + bytes32[] memory names = new bytes32[](1); + names[0] = keccak256("amount"); + engine.addPolicy(hooks, AdvancedPoolHooks.postflightCheck.selector, address(policy), names); + } + + /// @dev The compiled hooks gate preflightCheck/postflightCheck on the authorized-caller set + /// unconditionally, so a test driving postflightCheck directly must first authorize itself + /// (the pool would be the caller in production; here the test stands in for it). + function _authorizeSelf(AdvancedPoolHooks hooks) internal { + address[] memory adds = new address[](1); + adds[0] = address(this); + vm.prank(hooks.owner()); + AuthorizedCallers(address(hooks)) + .applyAuthorizedCallerUpdates( + AuthorizedCallers.AuthorizedCallerArgs({addedCallers: adds, removedCallers: new address[](0)}) + ); + } + + // ───────────────────────────────────────────────────────────────────────── + // F1: after SetPolicyEngine, the declared roles{} is stale until re-snapshotted + // ───────────────────────────────────────────────────────────────────────── + + /// @dev hooks.policyEngine is a governance-critical roles{} field (RolesAuditor._auditHooks). + /// Attach the hooks to the fixture pool, snapshot roles{} with no engine, move the engine + /// through the action layer, and the STALE declaration must FAIL the audit naming + /// hooks.policyEngine - the drift the roles runbook says to treat as a potential + /// compromise. A fresh RolesSnapshot.build (what make snapshot-chain runs) clears it. + function test_F1_setPolicyEngineDriftsDeclaredRoles() public { + (, address pool) = deployTokenAndPoolFixture(); + AdvancedPoolHooks hooks = _deployHooks(); + _exec(owner, CctActions._updateAdvancedPoolHooks(pool, address(hooks))); + MockPolicyEngine engine = new MockPolicyEngine(); + + string memory baseJson = vm.readFile("config/chains/ethereum-testnet-sepolia.json"); + string memory projectJson = _fixtureProjectJson(pool); + string memory roles = snap.build("ethereum-testnet-sepolia", baseJson, projectJson); + assertTrue( + vm.keyExistsJson(roles, ".hooks.policyEngine"), "precondition: the snapshot declares hooks.policyEngine" + ); + RolesAuditor.Result memory clean = auditor.auditJson("ethereum-testnet-sepolia", _wrap(roles)); + assertEq(clean.fails, 0, "precondition: the engine-less declaration reconciles clean"); + + _exec(owner, CctActions._setPolicyEngine(address(hooks), address(engine))); + + RolesAuditor.Result memory stale = auditor.auditJson("ethereum-testnet-sepolia", _wrap(roles)); + assertGt(stale.fails, 0, "the stale declaration must FAIL after the engine moved"); + assertTrue(vm.contains(stale.failedFields, "hooks.policyEngine"), "the FAIL must name hooks.policyEngine"); + + // The named remedy: a fresh build (what make snapshot-chain runs) records the new engine. + string memory fresh = snap.build("ethereum-testnet-sepolia", baseJson, projectJson); + assertEq( + vm.parseJsonAddress(fresh, ".hooks.policyEngine"), + address(engine), + "a fresh snapshot must record the new engine" + ); + RolesAuditor.Result memory reconciled = auditor.auditJson("ethereum-testnet-sepolia", _wrap(fresh)); + assertEq(reconciled.fails, 0, "the re-snapshotted declaration must reconcile clean"); + } + + // ───────────────────────────────────────────────────────────────────────── + // F2: a codeless POLICY_ENGINE reverts with empty data + // ───────────────────────────────────────────────────────────────────────── + + /// @dev attach() returns nothing, so the hook's call to a codeless address (an EOA or a mistyped + /// engine) reverts with no selector and no reason string. + function test_F2_codelessEngineRevertsWithEmptyData() public { + AdvancedPoolHooks hooks = _deployHooks(); + address eoa = makeAddr("codeless-engine"); + vm.expectRevert(bytes("")); + _exec(owner, CctActions._setPolicyEngine(address(hooks), eoa)); + } + + // ───────────────────────────────────────────────────────────────────────── + // F3: an unprovisioned engine rejects every transfer from the moment the swap lands + // ───────────────────────────────────────────────────────────────────────── + + /// @dev Extractors, policies and the per-target default live on the engine keyed by the hooks + /// address; nothing carries over from the old engine. With the ACE default of reject, the + /// real 1.0.0 engine rejects the hook's postflightCheck with PolicyRunRejected the moment + /// the swap lands. + function test_F3_proof_unprovisionedEngineRejectsEveryTransfer() public { + AdvancedPoolHooks hooks = _deployHooks(); + _authorizeSelf(hooks); + PolicyEngine engine = _deployRealEngine(false); + + _exec(owner, CctActions._setPolicyEngine(address(hooks), address(engine))); + + vm.prank(address(this)); + vm.expectRevert( + abi.encodeWithSelector( + IPolicyEngine.PolicyRunRejected.selector, + address(0), + "no policy allowed the action and default is reject", + _postflightPayload() + ) + ); + hooks.postflightCheck(_releaseOrMintIn(), 1e18, bytes4(0)); + } + + // ───────────────────────────────────────────────────────────────────────── + // F4: a reverting detach() blocks every later setPolicyEngine; the forced path strands the hook + // ───────────────────────────────────────────────────────────────────────── + + /// @dev With an engine whose detach() reverts, every later setPolicyEngine reverts + /// PolicyEngineDetachReverted; only setPolicyEngineAllowFailedDetach moves off it. + function test_F4_detachRevertBlocksSetPolicyEngine() public { + AdvancedPoolHooks hooks = _deployHooks(); + MockPolicyEngineRevertingDetach bad = new MockPolicyEngineRevertingDetach(); + MockPolicyEngine good = new MockPolicyEngine(); + + _exec(owner, CctActions._setPolicyEngine(address(hooks), address(bad))); + + vm.prank(owner); + vm.expectRevert( + abi.encodeWithSelector( + AdvancedPoolHooks.PolicyEngineDetachReverted.selector, + address(bad), + abi.encodeWithSignature("DetachNotSupported()") + ) + ); + hooks.setPolicyEngine(address(good)); + + _exec(owner, CctActions._setPolicyEngineAllowFailedDetach(address(hooks), address(good))); + assertEq(hooks.getPolicyEngine(), address(good), "the forced path must move to the new engine"); + } + + /// @dev The old engine keeps the hook listed as attached after a forced move (its detach() + /// reverted, so targetAttached stayed true), so pointing back at it reverts + /// TargetAlreadyAttached on the real engine's attach(). The reverting detach is forced + /// with vm.mockCallRevert so the REAL engine's own attach() tracking produces the revert. + function test_F4_returningToAForceDetachedEngineReverts() public { + AdvancedPoolHooks hooks = _deployHooks(); + PolicyEngine bad = _deployRealEngine(true); + PolicyEngine good = _deployRealEngine(true); + + _exec(owner, CctActions._setPolicyEngine(address(hooks), address(bad))); + vm.mockCallRevert( + address(bad), + abi.encodeWithSelector(IPolicyEngine.detach.selector), + abi.encodeWithSignature("DetachNotSupported()") + ); + _exec(owner, CctActions._setPolicyEngineAllowFailedDetach(address(hooks), address(good))); + vm.clearMockedCalls(); + + vm.prank(owner); + vm.expectRevert(abi.encodeWithSelector(IPolicyEngine.TargetAlreadyAttached.selector, address(hooks))); + hooks.setPolicyEngine(address(bad)); + } + + // ───────────────────────────────────────────────────────────────────────── + // F6: postflightCheck gas exceeds the 90,000 destGasOverhead default once an engine runs + // ───────────────────────────────────────────────────────────────────────── + + /// @dev postflightCheck runs inside releaseOrMint on the destination, within the gas the source + /// pool quotes as destGasOverhead for that lane. Measures the hook's postflightCheck + /// subtree with no engine and with the real engine + one VolumePolicy: one cheap policy + /// alone exceeds the 90,000 FeeQuoter default, before the mint itself. + function test_F6_postflightEngineRunGas() public { + AdvancedPoolHooks hooks = _deployHooks(); + _authorizeSelf(hooks); + Pool.ReleaseOrMintInV1 memory in_ = _releaseOrMintIn(); + + uint256 noEngineStart = gasleft(); + vm.prank(address(this)); + hooks.postflightCheck(in_, 1e18, bytes4(0)); + uint256 noEngineGas = noEngineStart - gasleft(); + + PolicyEngine engine = _deployRealEngine(true); + _provisionVolumePolicy(engine, address(hooks), 0, 1e18); + _exec(owner, CctActions._setPolicyEngine(address(hooks), address(engine))); + + uint256 withEngineStart = gasleft(); + vm.prank(address(this)); + hooks.postflightCheck(in_, 1e18, bytes4(0)); + uint256 withEngineGas = withEngineStart - gasleft(); + + assertGt(noEngineGas, 0, "sanity: the no-engine run consumed gas"); + assertGt(withEngineGas, noEngineGas, "the engine run must cost more than the no-engine run"); + assertGt( + withEngineGas, + DEFAULT_DEST_GAS_OVERHEAD, + "one VolumePolicy in postflightCheck alone must exceed the 90,000 destGasOverhead default" + ); + emit log_named_uint("postflightCheck gas without an engine", noEngineGas); + emit log_named_uint("postflightCheck gas with the real engine + one VolumePolicy", withEngineGas); + } + + // ───────────────────────────────────────────────────────────────────────── + // Helpers + // ───────────────────────────────────────────────────────────────────────── + + /// @dev The EXPLICIT projectJson a build() resolves the fixture token+pool from (the same + /// isolation seam RolesAuthorityTest uses: no process-global env, no real project file). + function _fixtureProjectJson(address pool) internal returns (string memory) { + address token = deployTokenFixture(); + return string.concat( + "{\"roles\":{\"token\":{\"address\":\"", + vm.toString(token), + "\"},\"pool\":{\"address\":\"", + vm.toString(pool), + "\"}}}" + ); + } + + function _wrap(string memory rolesJson) internal pure returns (string memory) { + return string.concat("{\"roles\":", rolesJson, "}"); + } + + /// @dev A minimal inbound payload for a direct postflightCheck call, as the pool would make from + /// inside releaseOrMint. + function _releaseOrMintIn() internal pure returns (Pool.ReleaseOrMintInV1 memory) { + return Pool.ReleaseOrMintInV1({ + originalSender: abi.encode(address(0xB0B)), + remoteChainSelector: 8236463271206331221, + receiver: address(0xB0B), + sourceDenominatedAmount: 1e18, + localToken: address(0), + sourcePoolAddress: abi.encode(address(0x1111111111111111111111111111111111111111)), + sourcePoolData: abi.encode(uint256(18)), + offchainTokenData: "" + }); + } + + /// @dev The exact Payload the hook builds for postflightCheck (selector, sender, data minus the + /// 4-byte prefix, context = offchainTokenData), for the expectRevert equality check. + function _postflightPayload() internal view returns (IPolicyEngine.Payload memory) { + bytes memory full = abi.encodeCall(IAdvancedPoolHooks.postflightCheck, (_releaseOrMintIn(), 1e18, bytes4(0))); + bytes memory stripped = new bytes(full.length - 4); + for (uint256 i = 0; i < stripped.length; i++) { + stripped[i] = full[i + 4]; + } + return IPolicyEngine.Payload({ + selector: AdvancedPoolHooks.postflightCheck.selector, sender: address(this), data: stripped, context: "" + }); + } +} diff --git a/test/governance/SafeMode.t.sol b/test/governance/SafeMode.t.sol index 509f541..a4de7a7 100644 --- a/test/governance/SafeMode.t.sol +++ b/test/governance/SafeMode.t.sol @@ -15,6 +15,8 @@ import {AcceptAdminRole} from "../../script/setup/AcceptAdminRole.s.sol"; import {CctActions, ITokenPoolV150, ILockReleaseV1Liquidity} from "../../src/actions/CctActions.sol"; import {PoolVersions} from "../../src/PoolVersions.sol"; import {AdvancedPoolHooks} from "@chainlink/contracts-ccip/contracts/pools/AdvancedPoolHooks.sol"; +import {MockPolicyEngine} from "@chainlink/contracts-ccip/contracts/test/mocks/MockPolicyEngine.sol"; +import {Ownable2Step} from "@chainlink/contracts/src/v0.8/shared/access/Ownable2Step.sol"; import {IERC20} from "@openzeppelin/contracts@5.3.0/token/ERC20/IERC20.sol"; import {EoaExecutor} from "../../src/base/EoaExecutor.s.sol"; import {ISafe, SafeCanonical} from "../../src/base/ISafe.sol"; @@ -305,6 +307,10 @@ contract SafeModeForkTest is BaseForkTest { address[] memory feeTokens = new address[](1); feeTokens[0] = token; _assertBatchRoundTrip("withdraw-fee-tokens", CctActions._withdrawFeeTokens(pool, feeTokens, deployer)); + _assertBatchRoundTrip("set-policy-engine", CctActions._setPolicyEngine(pool, deployer)); + _assertBatchRoundTrip( + "set-policy-engine-allow-failed-detach", CctActions._setPolicyEngineAllowFailedDetach(pool, deployer) + ); } /// @dev PR #9 (lanes-as-data / version-dispatched pool ops / CCV config / v1.x LockRelease liquidity / @@ -492,6 +498,32 @@ contract SafeModeForkTest is BaseForkTest { } } + /// @dev PR (policy-engine scripts) added two action-layer builders (`_setPolicyEngine`, + /// `_setPolicyEngineAllowFailedDetach`); the byte-equality contract is "for EVERY builder in + /// the catalog", so both round-trip in the catalog test above. This is the end-to-end leg: + /// hooks owned by a 2-of-3 Safe take `setPolicyEngine` through the full Mode B ceremony + /// (`execTransaction`), the engine records the hook as attached, and the former EOA owner is + /// refused. The hooks are deployed directly (not via the deploy script, whose env seam races + /// parallel suites from inside this suite's setUp). + function test_ModeB_SetPolicyEngine_SafeOwnedHooks() public { + AdvancedPoolHooks hooks = new AdvancedPoolHooks(new address[](0), 0, address(0), new address[](0)); + // The test contract owns the fresh hooks (it ran the constructor); hand them to the Safe. + hooks.transferOwnership(address(safe)); + _runSafeDirect("modeb-accept-hooks-ownership", CctActions._acceptOwnership(address(hooks))); + assertEq(hooks.owner(), address(safe), "the Safe must own the hooks after the Mode B accept"); + + MockPolicyEngine engine = new MockPolicyEngine(); + _runSafeDirect("modeb-set-policy-engine", CctActions._setPolicyEngine(address(hooks), address(engine))); + + assertEq(hooks.getPolicyEngine(), address(engine), "the engine must be set by the Safe"); + assertTrue(engine.isAttached(address(hooks)), "the engine must record the hook as attached"); + + // The former EOA owner is refused: only the hooks owner may call setPolicyEngine. + vm.prank(deployer); + vm.expectRevert(Ownable2Step.OnlyCallableByOwner.selector); + hooks.setPolicyEngine(address(0)); + } + /// @dev The registration pair (claim + accept) executes atomically as ONE Safe MultiSend batch: the /// claim sets the Safe (the token's CCIP admin) as pending administrator, so the accept in the /// SAME batch succeeds.