Skip to content

feat: doctor guards silo isolation and lock-and-lock lanes #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aelmanaa merged 1 commit into from
Sep 22, 2026
Merged

feat: doctor guards silo isolation and lock-and-lock lanes #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions docs/gotchas/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,22 @@ link to.
must return `0x` (otherwise the endpoint is not running `eth_call` at all). Check `eth_chainId`
matches the chain you meant, because public RPC directories carry chainId collisions.

<a id="silos-need-no-second-route"></a>
- **A silo isolates a chain only while that chain has no DIRECT lane to a differently-boxed sibling.** A
Siloed 2.0 pool maps each remote chain to an `ERC20LockBox`, and a release is paid out of the box mapped
to the chain the message CAME from. A remote-to-remote lane between two DIFFERENT boxes moves supply
without moving liquidity: value that entered through box A leaves from the other chain, whose box never
received it. Measured on staging (1.0 in each box, 1.0 supply on each remote): a 0.4 hop between the two
remotes left both boxes untouched at 1.0 while the supplies moved to 0.6 and 1.4, so 0.4 of one chain's
tokens are backed by the other chain's box. Nothing reverts at the time - the first return larger than
the box does, with `InsufficientBalance(requested, available)`, and until then the drift is invisible.
**Routing through the hub is fine and is the intended topology**: the inbound leg releases from one box
and the outbound leg locks into the other, so liquidity and supply move together (measured: a hub-routed
hop left every box equal to its own chain's supply). Chains that should trade directly belong on the
SAME box. `make doctor` FAILs such a pair when it can see both sides - it reads DECLARED lanes from each
peer's project store, so a peer store you do not keep locally is reported as unchecked, not as clean,
and the on-chain lane itself is only removed by `RemoveChain`.

<a id="a-successful-call-can-still-revert-your-frame"></a>
- **A successful call can still revert your frame.** `try C(a).f() returns (string memory)` routes a
REVERT to its catch, but the return data is decoded in the CALLER's frame after the call already
Expand Down
7 changes: 7 additions & 0 deletions docs/operations/liquidity.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,13 @@ LOCK_BOXES=AVALANCHE_TESTNET_FUJI=<box>,ETHEREUM_TESTNET_SEPOLIA_BASE_1=<box2> \
(Safe mode warns instead, since the batch may map it).
- `configureLockBoxes` never removes an entry, and does not check that the pool may use the box;
`ConfigureLockBoxes` refuses when it may not.
- **A silo isolates a chain only while that chain has no DIRECT lane to a differently-boxed sibling.** Two
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
[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
under `roles.lockboxes`, and `roles-check` audits each one.
Expand Down
5 changes: 5 additions & 0 deletions docs/operations/pools.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,11 @@ forge script \
The token already exists on both chains independently. Each chain needs its own `ERC20LockBox` and
`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).

```bash
# 1. Deploy ERC20LockBox on Ethereum Sepolia
forge script \
Expand Down
34 changes: 34 additions & 0 deletions docs/troubleshooting/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,40 @@ after a `forge script` run, with no `--broadcast`.
re-adding a lane silently reactivates stale config.
- **Verify.** `make doctor` reports the lane with a remote pool present.

## `make doctor` FAILs that two chains "hold their liquidity in different lock boxes"

- **Diagnosis.** The Siloed 2.0 pool maps those two remotes to different `ERC20LockBox`es, and the two
declare a lane to each other, so a token locked into one box is released from the other. Read the
mapping with `PROJECT_GROUP=<g> forge script script/configure/siloed/GetSiloedPoolState.s.sol
--rpc-url "$(bash script/config/rpc-url.sh <siloed chain>)"` (the chain comes from the RPC, not an env
var) and the lane with
`jq '.lanes' project/[<g>/]<remote>.json`. Either side declaring the lane is enough to break the
backing.
- **Fix.** Decide which the two chains are. Meant to be isolated: **`RemoveChain` on both remote pools
first** - that is the write that stops the traffic - and only then `make remove-lane LOCAL=<a>
REMOTE=<b> BOTH=1`, which edits the declaration. Meant to trade freely: put them on one box - move the
liquidity (`WithdrawFromLockBox` then `DepositToLockBox`) and `ConfigureLockBoxes` the second chain onto
the first one's box. `configureLockBoxes` is set-only, so the remapping is the write; the emptied box
stays deployed.
- **Verify.** Read the CHAIN, not the declaration: `cast call <remote pool> "isSupportedChain(uint64)(bool)"
<other selector> --rpc-url <remote rpc>` must answer `false` on both sides. Removing only the
declaration silences this finding while the lane keeps draining - doctor reads declared lanes, so a
green run after `remove-lane` alone proves nothing. Then `make doctor CHAIN=<siloed chain>` returns
VERIFIED. See [the gotcha](../gotchas/index.md#silos-need-no-second-route).

## A release into a Siloed 2.0 pool reverts with `InsufficientBalance`

- **Diagnosis.** The box mapped to the source chain holds less than the message asks for. Two causes:
the silo was never funded to that level, or two remotes on different boxes have a lane to each other
and have been trading over it, which moves supply without moving liquidity (the gotcha above). Compare
each box's balance against its chain's supply: `cast call <token> 'balanceOf(address)(uint256)' <box>`
on the pool's chain vs `cast call <token> 'totalSupply()(uint256)' --rpc-url <remote rpc>`. A box
holding MORE than its chain's supply, with another box holding less, is the cross-lane case.
- **Fix.** Fund the box (`DepositToLockBox`) to cover the in-flight amount, then remove the cross lane or
consolidate the two chains onto one box so the drift cannot recur. `make preflight SOURCE_CHAIN=<src>
DEST_CHAIN=<dst> TOKEN=<token> AMOUNT=<wei> RECEIVER=<addr>` reproduces the revert before a real send.
- **Verify.** The same `make preflight` returns `GO:`, and `make doctor CHAIN=<siloed chain>` is VERIFIED.

## 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
Expand Down
233 changes: 232 additions & 1 deletion script/config/VerifyChain.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1782,8 +1782,11 @@ contract VerifyChain is Script {
) {
_reconcileSiloedLockBoxes(pool);
}
bool localLockRelease =
versionKnown && PoolVersion._isLockReleaseFamily(PoolVersion._typePrefixOf(typeAndVersion));
uint64[] memory declaredSelectors = new uint64[](declared.length);
for (uint256 i = 0; i < declared.length; i++) {
if (localLockRelease) _warnLockAndLock(declared[i]);
declaredSelectors[i] = _checkDeclaredLaneOnChain(projectJson, pool, version, declared[i]);
}
bool reverseChecked = _checkOnChainLanesDeclared(name, pool, declaredSelectors);
Expand Down Expand Up @@ -2210,7 +2213,19 @@ contract VerifyChain is Script {
);
}
} catch {
_fail(string.concat("lockboxes: ", label, " does not answer getAllAuthorizedCallers()"));
// A custom box may implement only ILockBox (deposit/withdraw/isTokenSupported), where
// getAllAuthorizedCallers is absent by design: unverifiable, not broken.
if (token != address(0) && _answersIsTokenSupported(box, token)) {
_warn(
string.concat(
"lockboxes: ",
label,
" does not expose getAllAuthorizedCallers() - authorization NOT verified; confirm the pool may call it"
)
);
} else {
_fail(string.concat("lockboxes: ", label, " does not answer getAllAuthorizedCallers()"));
}
}
if (token != address(0)) {
try s_probe.lockBoxSupports(box, token) returns (bool ok) {
Expand All @@ -2229,6 +2244,7 @@ contract VerifyChain is Script {
);
}
}
_checkSiloIsolation(boxes, chains);
if (s_fails == failsBefore && s_warns == warnsBefore) {
_pass(
string.concat(
Expand All @@ -2242,6 +2258,221 @@ contract VerifyChain is Script {
}
}

/// @dev Lock-release on BOTH ends of a lane is a liquidity bridge, not a mint/burn mesh: a transfer
/// locks on one side and pays out of the other side's own liquidity, which drains in the busy
/// direction until someone rebalances. Legitimate (a canonical token that exists on both chains),
/// so it WARNs once per lane rather than failing - unlike two silos of one pool talking to each
/// other, which is never sound and FAILs above. Peer type comes from the peer's own project store.
function _warnLockAndLock(string memory remote) private {
string memory key = _peerPoolKey(remote);
if (bytes(key).length == 0 || !_contains(key, "LockReleaseTokenPool_")) return;
// A SILOED peer is a different shape: it pays from the lock box mapped to this chain, not from one
// pooled balance, so the rebalancing duty is per silo. Same hazard, different place to look.
bool siloedPeer = _contains(key, "SiloedLockReleaseTokenPool_");
_warn(
string.concat(
"lanes: ",
remote,
siloedPeer ? " runs a siloed lock-release pool too (" : " runs a lock-release pool too (",
key,
siloedPeer
? ") - this lane pays each release out of the lock box that peer maps to this chain, so it"
" drains one way and needs rebalancing per silo (docs/operations/liquidity.md)."
: ") - this lane pays each release out of the destination's own liquidity, so it drains one"
" way and needs rebalancing; fund and monitor both sides (docs/operations/liquidity.md)."
)
);
}

/// @dev The peer's `deployments{}` key naming its ACTIVE pool (the key carries the pool type and
/// version), or "" when the peer store is absent, unreadable, or names no pool.
function _peerPoolKey(string memory remote) private view returns (string memory) {
string memory path = ProjectStore._path(remote);
if (!vm.exists(path)) return "";
string memory json;
try s_probe.readFileFor(path) returns (string memory data) {
json = data;
} catch {
return "";
}
string memory active;
try s_probe.parseString(json, ".addresses.active.tokenPool") returns (string memory a) {
active = a;
} catch {
return "";
}
string[] memory keys;
try s_probe.keysOf(json, ".addresses.deployments") returns (string[] memory k) {
keys = k;
} catch {
return "";
}
for (uint256 i = 0; i < keys.length; i++) {
// Bracket notation: version keys (`..._2.0.0`) carry dots that dot-path notation mis-splits.
try s_probe.parseString(json, string.concat(".addresses.deployments[\"", keys[i], "\"]")) returns (
string memory v
) {
if (_eqIgnoreCase(v, active)) return keys[i];
} catch {}
}
return "";
}

function _eqIgnoreCase(string memory a, string memory b) private pure returns (bool) {
bytes memory x = bytes(a);
bytes memory y = bytes(b);
if (x.length != y.length) return false;
for (uint256 i = 0; i < x.length; i++) {
bytes1 cx = (x[i] >= "A" && x[i] <= "Z") ? bytes1(uint8(x[i]) + 32) : x[i];
bytes1 cy = (y[i] >= "A" && y[i] <= "Z") ? bytes1(uint8(y[i]) + 32) : y[i];
if (cx != cy) return false;
}
return true;
}

/// @dev A silo isolates a chain only while that chain has no OTHER route into the mesh. Two remotes
/// served by DIFFERENT boxes that also talk to each other break it: tokens locked into box A can
/// leave via the other chain and come back demanding a release from box B, which never received
/// them - the transfer reverts and box A holds liquidity no supply can claim any more. Chains
/// mapped to the SAME box share liquidity by design and are unaffected.
/// @dev Reads each remote's DECLARED lanes from its own project store (the mesh rung's evidence), not
/// the remote chain: this run is forked to the local chain only.
function _checkSiloIsolation(SiloedLockReleaseTokenPool.LockBoxConfig[] memory boxes, uint64[] memory supported)
private
{
// Resolve each box once: the catalog and every peer store are read one time, not once per pair.
string[] memory names = new string[](boxes.length);
string[][] memory lanes = new string[][](boxes.length);
bool[] memory usable = new bool[](boxes.length);
for (uint256 i = 0; i < boxes.length; i++) {
// A stale entry for a chain the pool no longer serves already WARNs above; it carries no route.
if (!_containsSelector(supported, boxes[i].remoteChainSelector)) continue;
// Only a chain that could be isolated FROM another box raises an isolation question at all.
if (!_sharesNoBoxWithSomeSibling(boxes, supported, i)) continue;
names[i] = _chainNameBySelector(boxes[i].remoteChainSelector);
if (bytes(names[i]).length == 0) {
_warn(
string.concat(
"lockboxes: silo isolation NOT checked for ",
_selectorLabel(boxes[i].remoteChainSelector),
" - no chain file names that selector (make add-chain), so its lanes cannot be read"
)
);
continue;
}
(uint8 status, string[] memory declared) = _declaredLaneNamesStatus(names[i]);
if (status == LANES_UNREADABLE) {
_fail(
string.concat(
"lockboxes: silo isolation NOT checked for ",
names[i],
" - its project store exists but does not parse; repair it (make fmt-config) and re-run"
)
);
continue;
}
if (status == LANES_ABSENT) {
_warn(
string.concat(
"lockboxes: silo isolation NOT checked for ",
names[i],
" - no project store here, so a lane it declares to another silo cannot be seen"
)
);
continue;
}
lanes[i] = declared;
usable[i] = true;
}
// One finding per PAIR (i < j), and a lane declared from either side proves the route exists.
for (uint256 i = 0; i < boxes.length; i++) {
if (!usable[i]) continue;
string memory nameI = names[i];
string[] memory lanesI = lanes[i];
for (uint256 j = i + 1; j < boxes.length; j++) {
if (boxes[i].lockBox == boxes[j].lockBox) continue;
if (!usable[j]) continue;
string memory nameJ = names[j];
if (!_namesContain(lanesI, nameJ) && !_namesContain(lanes[j], nameI)) continue;
_fail(
string.concat(
"lockboxes: ",
nameI,
" and ",
nameJ,
" hold their liquidity in different lock boxes (",
vm.toString(boxes[i].lockBox),
" vs ",
vm.toString(boxes[j].lockBox),
") yet declare a lane to each other - tokens locked for one are released from the",
" other's box, so the silos stop backing their own chain. Remove that lane, or map",
" both chains to one box (configure/siloed/ConfigureLockBoxes.s.sol)."
)
);
}
}
}

/// @dev Whether some other live box in the set holds different liquidity from box `i` - the only case
/// where a lane between the two would break the backing, and so the only case worth reporting on.
function _sharesNoBoxWithSomeSibling(
SiloedLockReleaseTokenPool.LockBoxConfig[] memory boxes,
uint64[] memory supported,
uint256 i
) private pure returns (bool) {
for (uint256 j = 0; j < boxes.length; j++) {
if (j == i || boxes[j].lockBox == boxes[i].lockBox) continue;
if (_containsSelector(supported, boxes[j].remoteChainSelector)) return true;
}
return false;
}

uint8 internal constant LANES_OK = 0;
uint8 internal constant LANES_ABSENT = 1;
uint8 internal constant LANES_UNREADABLE = 2;

/// @dev The remote names a chain declares lanes to, from its own project store, WITH why the answer is
/// empty: a store this operator does not keep (ABSENT) reads differently from one that does not
/// parse (UNREADABLE). Collapsing both into "no lanes" turns "could not look" into "looks fine".
function _declaredLaneNamesStatus(string memory name) private view returns (uint8, string[] memory) {
string memory path = ProjectStore._path(name);
string[] memory none = new string[](0);
if (!vm.exists(path)) return (LANES_ABSENT, none);
string memory json;
try s_probe.readFileFor(path) returns (string memory data) {
json = data;
} catch {
return (LANES_UNREADABLE, none);
}
try s_probe.hasKey(json, ".lanes") returns (bool has) {
if (!has) return (LANES_OK, none);
} catch {
return (LANES_UNREADABLE, none);
}
try s_probe.keysOf(json, ".lanes") returns (string[] memory keys) {
return (LANES_OK, keys);
} catch {
return (LANES_UNREADABLE, none);
}
}

/// @dev Whether the address answers the ILockBox token query at all - the tell that it is a lock box
/// with a narrower ABI rather than a wrong or dead address.
function _answersIsTokenSupported(address box, address token) private view returns (bool) {
try s_probe.lockBoxSupports(box, token) returns (bool) {
return true;
} catch {
return false;
}
}

function _namesContain(string[] memory names, string memory name) private pure returns (bool) {
for (uint256 i = 0; i < names.length; i++) {
if (keccak256(bytes(names[i])) == keccak256(bytes(name))) return true;
}
return false;
}

function _hasLockBox(SiloedLockReleaseTokenPool.LockBoxConfig[] memory boxes, uint64 selector)
private
pure
Expand Down
Loading
Loading