From 8fbcf750526fcbc287ad1ff50fd0cc423b5ebb69 Mon Sep 17 00:00:00 2001 From: aelmanaa Date: Tue, 22 Sep 2026 12:54:29 +0200 Subject: [PATCH] feat: refuse a lane with lock-release liquidity on both ends --- docs/operations/lanes-and-remotes.md | 6 ++ docs/operations/liquidity.md | 5 +- docs/operations/pools.md | 8 +- docs/primitives/catalog.json | 1 + .../token-admin-registry/ApplyChainUpdates.md | 1 + docs/troubleshooting/index.md | 16 ++++ script/setup/ApplyChainUpdates.s.sol | 60 ++++++++++++ script/utils/PoolVersion.s.sol | 7 ++ src/utils/RegistryWriter.sol | 22 +++++ test/setup/ApplyChainUpdatesLockAndLock.t.sol | 93 +++++++++++++++++++ 10 files changed, 214 insertions(+), 5 deletions(-) create mode 100644 test/setup/ApplyChainUpdatesLockAndLock.t.sol diff --git a/docs/operations/lanes-and-remotes.md b/docs/operations/lanes-and-remotes.md index f3a386d..73288aa 100644 --- a/docs/operations/lanes-and-remotes.md +++ b/docs/operations/lanes-and-remotes.md @@ -21,6 +21,12 @@ Configure the route from the local pool to a destination chain. Run once per dir Dry run first: omit `--broadcast` to simulate the apply against the fork and print exactly what it would do without sending a transaction. Add `--broadcast` once the output looks right. +Two lanes are refused before anything is sent. A siloed 2.0 pool with no lock box for that destination +(`LockBoxNotConfigured`) - map one first. And a destination whose own project store names a lock-release +pool (`LockAndLockLane`): both ends would pay releases out of their own liquidity, so the lane drains one +way from the moment it applies. `ACK_LOCK_AND_LOCK=true` applies it anyway, for a bridge you intend to fund +and monitor; `MODE=safe` warns instead of refusing, since the batch may never be signed. + ```bash # Configure Ethereum Sepolia -> Mantle Sepolia DEST_CHAIN=MANTLE_SEPOLIA \ diff --git a/docs/operations/liquidity.md b/docs/operations/liquidity.md index 6ef7429..62739e4 100644 --- a/docs/operations/liquidity.md +++ b/docs/operations/liquidity.md @@ -183,8 +183,9 @@ LOCK_BOXES=AVALANCHE_TESTNET_FUJI=,ETHEREUM_TESTNET_SEPOLIA_BASE_1= \ remotes served by DIFFERENT boxes must not have a lane to each other: supply moves while liquidity does not, so one box ends up holding tokens no supply can claim and the other cannot cover its own chain. Routing between them THROUGH the hub is fine - that path releases from one box and locks into the other. - Chains that should trade directly belong on the SAME box. `make doctor` fails such a pair when it can - read both peers' project stores, and says so when it cannot; see + Chains that should trade directly belong on the SAME box. `ApplyChainUpdates` refuses to wire a lane + whose peer also runs a lock-release pool (`ACK_LOCK_AND_LOCK=true` overrides), and `make doctor` fails + such a pair when it can read both peers' project stores, saying so when it cannot; see [the gotcha](../gotchas/index.md#silos-need-no-second-route) for the measured drift. - `make doctor` fails a supported chain with no box, a box that does not authorize the pool, or a box for another token, and warns about a box still mapped to a removed chain. `snapshot-chain` records the boxes diff --git a/docs/operations/pools.md b/docs/operations/pools.md index 5f7f189..a01b69a 100644 --- a/docs/operations/pools.md +++ b/docs/operations/pools.md @@ -143,9 +143,11 @@ The token already exists on both chains independently. Each chain needs its own `LockReleaseTokenPool`. This is a liquidity bridge, not a mint/burn mesh: every release is paid out of the destination's own -liquidity, so the busy direction drains and somebody has to rebalance. Fund and monitor both sides; -`make doctor` warns once per such lane. It is a different thing from two silos of ONE pool talking to each -other, which never balances and is refused - see [siloed pools](liquidity.md#siloed-pools). +liquidity, so the busy direction drains and somebody has to rebalance. **`ApplyChainUpdates` refuses such a +lane by default** (`LockAndLockLane`), because it starts draining the moment it is applied; pass +`ACK_LOCK_AND_LOCK=true` once you have decided to fund and monitor both sides. `make doctor` warns once per +lane that already exists. It is a different thing from two silos of ONE pool talking to each other, which +never balances - see [siloed pools](liquidity.md#siloed-pools). ```bash # 1. Deploy ERC20LockBox on Ethereum Sepolia diff --git a/docs/primitives/catalog.json b/docs/primitives/catalog.json index 5445b6b..512c0bb 100644 --- a/docs/primitives/catalog.json +++ b/docs/primitives/catalog.json @@ -917,6 +917,7 @@ "writes_onchain": true, "destructive": true, "inputs": [ + "ACK_LOCK_AND_LOCK", "DEST_CHAIN", "DEST_CHAIN_FAMILY", "DEST_CHAIN_SELECTOR", diff --git a/docs/primitives/token-admin-registry/ApplyChainUpdates.md b/docs/primitives/token-admin-registry/ApplyChainUpdates.md index 3177622..ac8b222 100644 --- a/docs/primitives/token-admin-registry/ApplyChainUpdates.md +++ b/docs/primitives/token-admin-registry/ApplyChainUpdates.md @@ -19,6 +19,7 @@ Configures cross-chain lanes on the source TokenPool by calling applyChainUpdate | Env var | Description | | --- | --- | +| `ACK_LOCK_AND_LOCK` | See the script header. | | `DEST_CHAIN` | See the script header. | | `DEST_CHAIN_FAMILY` | See the script header. | | `DEST_CHAIN_SELECTOR` | See the script header. | diff --git a/docs/troubleshooting/index.md b/docs/troubleshooting/index.md index 47ce453..e7b79e7 100644 --- a/docs/troubleshooting/index.md +++ b/docs/troubleshooting/index.md @@ -226,6 +226,22 @@ after a `forge script` run, with no `--broadcast`. DEST_CHAIN= TOKEN= AMOUNT= RECEIVER=` reproduces the revert before a real send. - **Verify.** The same `make preflight` returns `GO:`, and `make doctor CHAIN=` is VERIFIED. +## `LockAndLockLane:` ... `runs a lock-release pool too` when applying a lane + +- **Diagnosis.** Both ends of this lane hold their own liquidity: the send locks tokens on the source and + the delivery pays out of the destination's own balance, so the busy direction drains until someone moves + tokens by hand. `ApplyChainUpdates` reads the peer's pool TYPE from its project store + (`jq -r '.addresses.deployments | keys[]' project/[/].json`) - the key carries the type, and + a peer running `...LockReleaseTokenPool_...` is the trigger. Between two silos of ONE pool it is worse: + the lock boxes stop backing their own chains. +- **Fix.** Decide which lane you meant. A mint/burn mesh: deploy a BurnMint pool on the peer (or route the + traffic through a chain that has one) and re-run. A deliberate, funded liquidity bridge: re-run with + `ACK_LOCK_AND_LOCK=true`, then fund both sides and watch them. Two silos of one pool: map both chains to + the SAME lock box instead (`ConfigureLockBoxes`), see + [the gotcha](../gotchas/index.md#silos-need-no-second-route). +- **Verify.** The lane applies (`SENDING (unconfirmed)`), and `make doctor CHAIN=` reports the + lock-and-lock WARN once per such lane - it stays a warning, because a funded bridge is legitimate. + ## Advanced forensics: a leg that failed then recovered The REST API and `ccip-cli show` report only the final `SUCCESS` and hide an earlier failed attempt. To diff --git a/script/setup/ApplyChainUpdates.s.sol b/script/setup/ApplyChainUpdates.s.sol index 6cfa114..6ef1fd4 100644 --- a/script/setup/ApplyChainUpdates.s.sol +++ b/script/setup/ApplyChainUpdates.s.sol @@ -16,6 +16,7 @@ import {PoolVersions} from "../../src/PoolVersions.sol"; import {CctActions, ITokenPoolV150} from "../../src/actions/CctActions.sol"; import {EoaExecutor} from "../../src/base/EoaExecutor.s.sol"; import {ProjectStore} from "../../src/utils/ProjectStore.sol"; +import {RegistryWriter} from "../../src/utils/RegistryWriter.sol"; /// @notice Configures cross-chain lanes on the source TokenPool by calling applyChainUpdates. /// Sets the remote pool(s), remote token, and optional rate limiter configs per destination chain. @@ -75,6 +76,8 @@ import {ProjectStore} from "../../src/utils/ProjectStore.sol"; /// _TOKEN - EVM address of the token on the destination chain /// /// Environment Variables (non-EVM destinations): +/// ACK_LOCK_AND_LOCK - true to apply a lane whose peer also runs a lock-release pool +/// (refused by default: both ends pay from their own liquidity) /// DEST_CHAIN_FAMILY - "svm"/"solana" or "aptos" (default: "evm") /// (auto-detected from the destination's config chainFamily) /// DEST_CHAIN_SELECTOR - uint64 chain selector for the destination chain @@ -230,6 +233,7 @@ contract ApplyChainUpdates is EoaExecutor { (PoolVersions.Version poolVersion, string memory poolTypeAndVersion) = PoolVersion._resolve(poolAddress); console.log(string.concat("Pool contract: ", poolTypeAndVersion)); _requireLockBoxes(poolAddress, poolVersion, poolTypeAndVersion, chainUpdates); + _refuseLockAndLock(poolTypeAndVersion, chainUpdates); _executeCalls( _buildLaneUpdateCalls(poolVersion, poolAddress, chainSelectorRemovals, chainUpdates, shouldRemove) ); @@ -786,6 +790,7 @@ contract ApplyChainUpdates is EoaExecutor { (PoolVersions.Version poolVersion, string memory poolTypeAndVersion) = PoolVersion._resolve(poolAddress); console.log(string.concat("Pool contract: ", poolTypeAndVersion)); _requireLockBoxes(poolAddress, poolVersion, poolTypeAndVersion, chainUpdates); + _refuseLockAndLock(poolTypeAndVersion, chainUpdates); bool[] memory replaceExisting = new bool[](1); replaceExisting[0] = chainAlreadyConfigured; _executeCalls( @@ -796,6 +801,61 @@ contract ApplyChainUpdates is EoaExecutor { /// @dev A Siloed 2.0 pool reverts every transfer on a lane with no lock box, so the box comes first /// (upstream tooling enforces the same order). Safe mode only warns: the batch may map it. + /// @dev Lock-release on BOTH ends of a lane never reconciles: the send locks into this chain's + /// liquidity and the delivery pays out of the peer's, so the two balances drift apart in the + /// busy direction until somebody moves tokens by hand. Between two silos of ONE pool it is + /// worse - the boxes stop backing their own chains (see the doctor's lockboxes rung). Refused + /// here rather than reported later, because the lane starts draining the moment it is applied. + /// `ACK_LOCK_AND_LOCK=true` is the deliberate override for a funded, monitored liquidity + /// bridge (docs/operations/pools.md). Safe mode warns instead: the batch may not be signed. + function _refuseLockAndLock(string memory typeAndVersion, TokenPool.ChainUpdate[] memory updates) internal view { + bool acknowledged = vm.envOr("ACK_LOCK_AND_LOCK", false); + for (uint256 i = 0; i < updates.length; i++) { + string memory reason = lockAndLockReasonForTest(typeAndVersion, updates[i].remoteChainSelector); + if (bytes(reason).length == 0) continue; + if (acknowledged) { + console.log(string.concat(unicode"⚠️ ", reason, " - applying anyway (ACK_LOCK_AND_LOCK=true).")); + continue; + } + if (SafeMode._isSafeMode(_executionMode())) { + console.log(string.concat(unicode"⚠️ ", reason, " - review before signing the batch.")); + continue; + } + revert(reason); + } + } + + /// @dev Test seam: `run()` builds the HelperConfig, so a unit test that calls the guard directly + /// has to build it too. + function initHelperConfigForTest() public { + helperConfig = new HelperConfig(); + } + + /// @notice Why wiring a lane to `remoteChainSelector` would put lock-release liquidity on BOTH ends, + /// or "" when it would not. Empty unless this pool is lock-release family AND the peer's store names a + /// lock-release pool: a mint/burn end absorbs the imbalance, and a peer this operator does not manage + /// is not evidence of a bridge. + function lockAndLockReasonForTest(string memory localTypeAndVersion, uint64 remoteChainSelector) + public + view + returns (string memory) + { + if (!PoolVersion._isLockReleaseFamily(PoolVersion._typePrefixOf(localTypeAndVersion))) return ""; + string memory peer = helperConfig.getSelectorNameBySelector(remoteChainSelector); + if (bytes(peer).length == 0) return ""; + string memory key = RegistryWriter._activeDeploymentKey(peer, "tokenPool"); + if (bytes(key).length == 0 || !PoolVersion._isLockReleaseKey(key)) return ""; + return string.concat( + "LockAndLockLane: ", + peer, + " runs a lock-release pool too (", + key, + "), so this lane pays every release out of the destination's own liquidity and drains one way;", + " fund and monitor both sides, or wire the lane through a mint/burn chain.", + " Set ACK_LOCK_AND_LOCK=true to apply it anyway (docs/operations/pools.md)." + ); + } + function _requireLockBoxes( address pool, PoolVersions.Version version, diff --git a/script/utils/PoolVersion.s.sol b/script/utils/PoolVersion.s.sol index 02908d7..5349a27 100644 --- a/script/utils/PoolVersion.s.sol +++ b/script/utils/PoolVersion.s.sol @@ -245,6 +245,13 @@ library PoolVersion { return keccak256(bytes(typePrefix)) == keccak256(bytes(SILOED_LOCK_RELEASE_TYPE)); } + /// @notice Whether a `deployments{}` key names a lock-release-family pool. The key shape is + /// `{symbol}_{PoolType}_{version}`, so the type substring is the evidence; `SiloedLockRelease...` + /// matches too, and is the worse case of the same hazard. + function _isLockReleaseKey(string memory deploymentKey) internal pure returns (bool) { + return _contains(deploymentKey, "LockReleaseTokenPool_"); + } + function _isLockReleaseFamily(string memory typePrefix) internal pure returns (bool) { return keccak256(bytes(typePrefix)) == keccak256(bytes(LOCK_RELEASE_TYPE)) || _isSiloed(typePrefix); } diff --git a/src/utils/RegistryWriter.sol b/src/utils/RegistryWriter.sol index 1b836d9..b15e976 100644 --- a/src/utils/RegistryWriter.sol +++ b/src/utils/RegistryWriter.sol @@ -384,6 +384,28 @@ library RegistryWriter { } /// @notice The `active` role whose pointer holds `value` (case-insensitive), or "" when none does. + /// @notice The `deployments{}` key naming the artifact `active.` points at, or "" when the + /// store is absent, the role unset, or no deployment matches. The key carries the pool TYPE and + /// version, which is the only record of what a peer chain runs that a script on THIS chain can read. + function _activeDeploymentKey(string memory selectorName, string memory role) + internal + view + returns (string memory) + { + (string[] memory aKeys, string[] memory aVals, string[] memory dKeys, string[] memory dVals) = + _loadMaps(selectorName); + string memory active; + for (uint256 i = 0; i < aKeys.length; i++) { + if (keccak256(bytes(aKeys[i])) == keccak256(bytes(role))) active = aVals[i]; + } + if (bytes(active).length == 0) return ""; + bytes32 want = keccak256(bytes(_lower(active))); + for (uint256 i = 0; i < dVals.length; i++) { + if (keccak256(bytes(_lower(dVals[i]))) == want) return dKeys[i]; + } + return ""; + } + function _activeRoleFor(string memory selectorName, string memory value) internal view returns (string memory) { (string[] memory aKeys, string[] memory aVals,,) = _loadMaps(selectorName); bytes32 want = keccak256(bytes(_lower(value))); diff --git a/test/setup/ApplyChainUpdatesLockAndLock.t.sol b/test/setup/ApplyChainUpdatesLockAndLock.t.sol new file mode 100644 index 0000000..c6dcaa8 --- /dev/null +++ b/test/setup/ApplyChainUpdatesLockAndLock.t.sol @@ -0,0 +1,93 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {ApplyChainUpdates} from "../../script/setup/ApplyChainUpdates.s.sol"; +import {ProjectStore} from "../../src/utils/ProjectStore.sol"; +import {LaneReconcileScratch} from "../config/VerifyChainLaneReconcile.t.sol"; + +/// @notice ApplyChainUpdates refuses to wire a lane whose peer also runs a lock-release pool: both ends +/// would pay releases out of their own liquidity, so the lane drains one way from the moment it applies. +/// The peer's pool TYPE is only knowable from its project store's deployments key, so these cases turn on +/// what that store says - including saying nothing. +contract ApplyChainUpdatesLockAndLockTest is LaneReconcileScratch { + string[5] internal SUITES = ["lockrelease", "siloed", "burnmint", "nostore", "localbm"]; + uint64 internal constant PEER_SELECTOR_BASE = 9_911_000_000_000_000_001; + + function setUp() public { + for (uint256 i = 0; i < SUITES.length; i++) { + _cleanupScratchOne(string.concat("zz-scratch-lal-", SUITES[i])); + } + } + + /// @dev A peer chain whose store names `poolKey` as its active pool, and the reason the script gives + /// for a lane to it ("" = allowed). + function _reasonFor(string memory suite, uint256 seed, string memory poolKey) + internal + returns (string memory reason) + { + return _reasonFor(suite, seed, poolKey, "LockReleaseTokenPool 2.0.0"); + } + + /// @dev `localType` is what THIS chain's pool reports; the guard only speaks for a lock-release local. + function _reasonFor(string memory suite, uint256 seed, string memory poolKey, string memory localType) + internal + returns (string memory reason) + { + string memory peer = string.concat("zz-scratch-lal-", suite); + uint64 selector = uint64(PEER_SELECTOR_BASE + seed * 1_000_000); + _writeScratchChain(peer, 991_100 + seed, selector); + + if (bytes(poolKey).length > 0) { + ProjectStore._seedIfAbsent(peer); + address peerPool = address(uint160(0xB0B0000 + seed)); + vm.writeJson( + string.concat( + "{\"active\":{\"tokenPool\":\"", + vm.toString(peerPool), + "\"},\"deployments\":{\"", + poolKey, + "\":\"", + vm.toString(peerPool), + "\"}}" + ), + _projPath(peer), + ".addresses" + ); + } + + ApplyChainUpdates script = new ApplyChainUpdates(); + script.initHelperConfigForTest(); + reason = script.lockAndLockReasonForTest(localType, selector); + _cleanupScratchOne(peer); + } + + function test_Refuses_PeerRunsLockRelease() public { + string memory reason = _reasonFor("lockrelease", 1, "ZZ_LockReleaseTokenPool_2.0.0"); + assertTrue(bytes(reason).length > 0, "a lock-release peer must be refused"); + assertTrue(vm.indexOf(reason, "LockAndLockLane") != type(uint256).max, reason); + assertTrue(vm.indexOf(reason, "ACK_LOCK_AND_LOCK=true") != type(uint256).max, "names the override"); + } + + /// @dev Two silos of one pool are the worse case of the same hazard, and the key still carries the type. + function test_Refuses_PeerRunsSiloedLockRelease() public { + string memory reason = _reasonFor("siloed", 2, "ZZ_SiloedLockReleaseTokenPool_2.0.0"); + assertTrue(bytes(reason).length > 0, "a siloed lock-release peer must be refused too"); + } + + function test_Allows_PeerRunsBurnMint() public { + assertEq(_reasonFor("burnmint", 3, "ZZ_BurnMintTokenPool_2.0.0"), "", "mint/burn peers are the normal mesh"); + } + + /// @dev Mint/burn on THIS side absorbs the imbalance - lock-release facing burn-mint is the normal + /// CCT shape, not a bridge, so the guard must stay quiet however the peer is configured. + function test_Allows_LocalPoolIsBurnMint() public { + string memory reason = _reasonFor("localbm", 5, "ZZ_LockReleaseTokenPool_2.0.0", "BurnMintTokenPool 2.0.0"); + assertEq(reason, "", "a burn-mint local pool is never half of a lock-and-lock lane"); + } + + /// @dev A peer this operator does not manage says nothing about the peer's pool, and absence of + /// evidence must not block a legitimate lane. + function test_Allows_PeerWithNoStore() public { + assertEq(_reasonFor("nostore", 4, ""), "", "no peer store, no claim"); + } +}