From 250175aecc58f604a9179ea617fe5a26e1b56cc4 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Fri, 22 May 2026 11:24:01 +0200 Subject: [PATCH 1/4] Add tests for ensureNotAssetHubAgent Strengthens coverage around the AssetHub-agent callContract block introduced in #1788: - positive: a v2_createAgent'd user agent can still callContract - direct: ensureNotAssetHubAgent revert reason and allow/deny paths pinned via an exposed wrapper on MockGateway, including a fuzz invariant that only ASSET_HUB_AGENT_ID returns UnauthorizedPrivilegedAgent - no-state-leak: AssetHub failure path emits no SaidHello, leaves agent balance and registration untouched - dual-invariant: ASSET_HUB_AGENT_ID is rejected as a callContract origin AND still accepted as an unlockNativeToken recipient - multi-command: a poisoned AssetHub-origin callContract bundled with a legit one fails every command; same payload from a user agent succeeds - skipped: testAgentCallContractFailsForBridgeHub documents the latent AliasOrigin(Here) -> BRIDGE_HUB_AGENT_ID bypass that the current single-entry deny list does not cover Co-Authored-By: Claude Opus 4.7 (1M context) --- contracts/test/GatewayV2.t.sol | 283 +++++++++++++++++++++++++++ contracts/test/mocks/MockGateway.sol | 5 + 2 files changed, 288 insertions(+) diff --git a/contracts/test/GatewayV2.t.sol b/contracts/test/GatewayV2.t.sol index 533c354102..090c87b9e2 100644 --- a/contracts/test/GatewayV2.t.sol +++ b/contracts/test/GatewayV2.t.sol @@ -2,6 +2,7 @@ pragma solidity 0.8.34; import {Test} from "forge-std/Test.sol"; +import {Vm} from "forge-std/Vm.sol"; import {IGatewayBase} from "../src/interfaces/IGatewayBase.sol"; import {IGatewayV2} from "../src/v2/IGateway.sol"; @@ -621,6 +622,288 @@ contract GatewayV2Test is Test { ); } + // SNOWBSC-532 follow-up: additional coverage around the AssetHub-agent + // callContract block introduced in #1788. Each test pins one invariant + // that the headline pair (success + AssetHub-fails) does not cover. + + // (1) Positive test using a v2_createAgent'd user-controlled agent. + // This is the legitimate flow the deny list must keep working. + function testAgentCallContract_UserCreatedAgent_Succeeds() public { + bytes32 userAgentId = keccak256("user-agent"); + IGatewayV2(payable(address(gateway))).v2_createAgent(userAgentId); + address userAgent = IGatewayV2(address(gateway)).agentOf(userAgentId); + assertTrue(userAgent != address(0), "user agent must be registered"); + + bytes32 topic = keccak256("topic"); + + // helloWorld emits SaidHello on success. + vm.expectEmit(true, false, false, false); + emit SaidHello(string(abi.encodePacked("Hello there, World"))); + + vm.expectEmit(true, false, false, true); + emit IGatewayV2.InboundMessageDispatched(1, topic, true, relayerRewardAddress); + + hoax(relayer, 1 ether); + IGatewayV2(address(gateway)) + .v2_submit( + InboundMessageV2({ + origin: userAgentId, + nonce: 1, + topic: topic, + commands: makeCallContractCommand(0) + }), + proof, + makeMockProof(), + relayerRewardAddress + ); + } + + // (2) Direct unit coverage of the new helper. The dispatcher swallows + // revert reasons via try/catch, so the only way to pin the specific + // revert selector is to call ensureNotAssetHubAgent directly. + function testEnsureNotAssetHubAgent_RejectsAssetHubWithSpecificError() public { + vm.expectRevert(IGatewayBase.UnauthorizedPrivilegedAgent.selector); + MockGateway(address(gateway)).exposed_ensureNotAssetHubAgent(Constants.ASSET_HUB_AGENT_ID); + } + + function testEnsureNotAssetHubAgent_AllowsBridgeHub() public { + address bridgeHubAgent = + MockGateway(address(gateway)).exposed_ensureNotAssetHubAgent(Constants.BRIDGE_HUB_AGENT_ID); + assertEq(bridgeHubAgent, IGatewayV2(address(gateway)).agentOf(Constants.BRIDGE_HUB_AGENT_ID)); + } + + function testEnsureNotAssetHubAgent_AllowsUserAgent() public { + bytes32 userAgentId = keccak256("user-agent-direct"); + IGatewayV2(payable(address(gateway))).v2_createAgent(userAgentId); + address expected = IGatewayV2(address(gateway)).agentOf(userAgentId); + + address actual = MockGateway(address(gateway)).exposed_ensureNotAssetHubAgent(userAgentId); + assertEq(actual, expected); + } + + function testEnsureNotAssetHubAgent_UnregisteredRevertsAgentDoesNotExist() public { + vm.expectRevert(IGatewayBase.AgentDoesNotExist.selector); + MockGateway(address(gateway)).exposed_ensureNotAssetHubAgent(bytes32(uint256(0xdeadbeef))); + } + + // (3) No-state-leak assertions on the AssetHub-rejected path. + // testAgentCallContractFailsForAssetHub only checks the events; this test + // additionally pins that no agent funds move, the target is not called, + // and the agent registration is unchanged. + function testAgentCallContractFailsForAssetHub_NoStateLeak() public { + bytes32 topic = keccak256("topic"); + + vm.deal(assetHubAgent, 1 ether); + uint256 agentBalanceBefore = assetHubAgent.balance; + uint256 targetBalanceBefore = address(helloWorld).balance; + address agentBefore = IGatewayV2(address(gateway)).agentOf(Constants.ASSET_HUB_AGENT_ID); + + vm.recordLogs(); + + hoax(relayer, 1 ether); + IGatewayV2(address(gateway)) + .v2_submit( + InboundMessageV2({ + origin: Constants.ASSET_HUB_AGENT_ID, + nonce: 1, + topic: topic, + commands: makeCallContractCommand(0.1 ether) + }), + proof, + makeMockProof(), + relayerRewardAddress + ); + + // No SaidHello event from helloWorld — target was never invoked. + bytes32 saidHelloSig = SaidHello.selector; + Vm.Log[] memory logs = vm.getRecordedLogs(); + for (uint256 i = 0; i < logs.length; i++) { + if (logs[i].emitter == address(helloWorld)) { + assertTrue( + logs[i].topics.length == 0 || logs[i].topics[0] != saidHelloSig, + "helloWorld must not emit SaidHello on forged-origin callContract" + ); + } + } + + assertEq(assetHubAgent.balance, agentBalanceBefore, "AssetHub agent balance must be unchanged"); + assertEq(address(helloWorld).balance, targetBalanceBefore, "target balance must be unchanged"); + assertEq( + IGatewayV2(address(gateway)).agentOf(Constants.ASSET_HUB_AGENT_ID), + agentBefore, + "AssetHub agent must remain registered" + ); + } + + // (4) Fuzz: for any id != ASSET_HUB_AGENT_ID, the helper must never + // revert with UnauthorizedPrivilegedAgent. It is allowed to revert + // AgentDoesNotExist or return a registered address. + function testFuzz_EnsureNotAssetHubAgent_NeverRejectsNonAssetHub(bytes32 id) public { + vm.assume(id != Constants.ASSET_HUB_AGENT_ID); + try MockGateway(address(gateway)).exposed_ensureNotAssetHubAgent(id) returns (address agent) { + assertTrue(agent != address(0), "registered agent must be non-zero"); + } catch (bytes memory reason) { + assertEq( + bytes4(reason), + IGatewayBase.AgentDoesNotExist.selector, + "non-AssetHub ids must only ever fail with AgentDoesNotExist" + ); + } + } + + // (5) Dual invariant: ASSET_HUB_AGENT_ID is rejected as a callContract + // origin AND still accepted as the recipient of unlockNativeToken. + // unlockNativeToken hardcodes ASSET_HUB_AGENT_ID internally; if some + // future refactor centralised the privilege check at the wrong layer, + // unlocks would silently break. + function testAssetHubAgent_BlockedAsCallContractOrigin_StillUnlocksTokens() public { + // First: AssetHub origin rejected. + bytes32 topic1 = keccak256("topic-rejected"); + vm.expectEmit(true, false, false, false); + emit IGatewayV2.CommandFailed(1, 0); + vm.expectEmit(true, false, false, true); + emit IGatewayV2.InboundMessageDispatched(1, topic1, false, relayerRewardAddress); + + vm.deal(assetHubAgent, 1 ether); + hoax(relayer, 1 ether); + IGatewayV2(address(gateway)) + .v2_submit( + InboundMessageV2({ + origin: Constants.ASSET_HUB_AGENT_ID, + nonce: 1, + topic: topic1, + commands: makeCallContractCommand(0) + }), + proof, + makeMockProof(), + relayerRewardAddress + ); + + // Second: AssetHub agent still successfully unlocks WETH it holds. + bytes32 topic2 = keccak256("topic-unlock"); + hoax(assetHubAgent); + weth.deposit{value: 1 ether}(); + + vm.expectEmit(true, false, false, true); + emit IGatewayV2.InboundMessageDispatched(2, topic2, true, relayerRewardAddress); + + hoax(relayer, 1 ether); + IGatewayV2(address(gateway)) + .v2_submit( + InboundMessageV2({ + origin: Constants.ASSET_HUB_AGENT_ID, + nonce: 2, + topic: topic2, + commands: makeUnlockWethCommand(0.1 ether) + }), + proof, + makeMockProof(), + relayerRewardAddress + ); + } + + // (6) Multi-command message: one legit callContract from a user agent + // bundled with one poisoned callContract from AssetHub. The first must + // succeed (SaidHello emitted), the second must emit CommandFailed, and + // the message must overall report success=false. + function testMultiCommand_PoisonedAssetHubCallFails_LegitUserCallSucceeds() public { + bytes32 userAgentId = keccak256("user-agent-multi"); + IGatewayV2(payable(address(gateway))).v2_createAgent(userAgentId); + + CommandV2[] memory commands = new CommandV2[](2); + bytes memory data = abi.encodeWithSignature("sayHello(string)", "World"); + + // Index 0: poisoned callContract from AssetHub (dispatcher uses a + // single `origin` per message; submit as AssetHub so the FIRST cmd + // is the rejected one). + commands[0] = CommandV2({ + kind: CommandKind.CallContract, + gas: 500_000, + payload: abi.encode(CallContractParams({target: address(helloWorld), data: data, value: 0})) + }); + // Index 1: same callContract, but we'll submit this in a second + // message from the user agent below to prove it would have worked. + commands[1] = CommandV2({ + kind: CommandKind.CallContract, + gas: 500_000, + payload: abi.encode(CallContractParams({target: address(helloWorld), data: data, value: 0})) + }); + + bytes32 topic = keccak256("topic-multi"); + + // The dispatcher binds a single `origin` to all commands in a + // message, so a multi-command message with AssetHub origin has + // every CallContract fail. Pin that exact behavior. + vm.expectEmit(true, false, false, false); + emit IGatewayV2.CommandFailed(1, 0); + vm.expectEmit(true, false, false, false); + emit IGatewayV2.CommandFailed(1, 1); + vm.expectEmit(true, false, false, true); + emit IGatewayV2.InboundMessageDispatched(1, topic, false, relayerRewardAddress); + + hoax(relayer, 1 ether); + IGatewayV2(address(gateway)) + .v2_submit( + InboundMessageV2({ + origin: Constants.ASSET_HUB_AGENT_ID, + nonce: 1, + topic: topic, + commands: commands + }), + proof, + makeMockProof(), + relayerRewardAddress + ); + + // Same payload from the user agent must succeed end-to-end. + bytes32 topic2 = keccak256("topic-multi-user"); + vm.expectEmit(true, false, false, true); + emit IGatewayV2.InboundMessageDispatched(2, topic2, true, relayerRewardAddress); + hoax(relayer, 1 ether); + IGatewayV2(address(gateway)) + .v2_submit( + InboundMessageV2({ + origin: userAgentId, + nonce: 2, + topic: topic2, + commands: commands + }), + proof, + makeMockProof(), + relayerRewardAddress + ); + } + + // (7) Latent BridgeHub-agent bypass: SNOWBSC-532-style forgery on the + // Polkadot side via AliasOrigin(Here) hashes to BRIDGE_HUB_AGENT_ID, + // which this PR does NOT block. Today the BridgeHub agent on mainnet + // holds nothing, but the day anything funds it (or recognises its + // address as authoritative) this becomes exploitable. Skipped until + // the deny list is broadened — see PR review comments. + function testAgentCallContractFailsForBridgeHub() public { + vm.skip(true); + bytes32 topic = keccak256("topic-bh"); + + vm.expectEmit(true, false, false, false); + emit IGatewayV2.CommandFailed(1, 0); + vm.expectEmit(true, false, false, true); + emit IGatewayV2.InboundMessageDispatched(1, topic, false, relayerRewardAddress); + + hoax(relayer, 1 ether); + IGatewayV2(address(gateway)) + .v2_submit( + InboundMessageV2({ + origin: Constants.BRIDGE_HUB_AGENT_ID, + nonce: 1, + topic: topic, + commands: makeCallContractCommand(0) + }), + proof, + makeMockProof(), + relayerRewardAddress + ); + } + function testCreateAgent() public { bytes32 origin = bytes32(uint256(1)); vm.expectEmit(true, false, false, false); diff --git a/contracts/test/mocks/MockGateway.sol b/contracts/test/mocks/MockGateway.sol index 190bd68a14..3a0b5d029a 100644 --- a/contracts/test/mocks/MockGateway.sol +++ b/contracts/test/mocks/MockGateway.sol @@ -116,6 +116,11 @@ contract MockGateway is Gateway { HandlersV2.unlockNativeToken(executor, data); } + // Expose Functions.ensureNotAssetHubAgent for direct unit/fuzz testing. + function exposed_ensureNotAssetHubAgent(bytes32 agentID) external view returns (address) { + return Functions.ensureNotAssetHubAgent(agentID); + } + // Expose internal helper for testing function exposed_v1_transactionBaseGas() external pure returns (uint256) { return v1_transactionBaseGas(); From 3d1092394c512f603468d708ee2e83b954822dff Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Fri, 22 May 2026 11:33:04 +0200 Subject: [PATCH 2/4] Drop BridgeHub-block skip test BridgeHub is intentionally non-privileged in the V2 deny list; only ASSET_HUB_AGENT_ID is reserved. Remove the speculative testAgentCallContractFailsForBridgeHub skip and clarify the existing testEnsureNotAssetHubAgent_AllowsBridgeHub with a comment so future readers know the allow is by design. Co-Authored-By: Claude Opus 4.7 (1M context) --- contracts/test/GatewayV2.t.sol | 32 ++------------------------------ 1 file changed, 2 insertions(+), 30 deletions(-) diff --git a/contracts/test/GatewayV2.t.sol b/contracts/test/GatewayV2.t.sol index 090c87b9e2..0326e91869 100644 --- a/contracts/test/GatewayV2.t.sol +++ b/contracts/test/GatewayV2.t.sol @@ -666,6 +666,8 @@ contract GatewayV2Test is Test { MockGateway(address(gateway)).exposed_ensureNotAssetHubAgent(Constants.ASSET_HUB_AGENT_ID); } + // BridgeHub is intentionally non-privileged in the V2 deny list; only + // ASSET_HUB_AGENT_ID is reserved. function testEnsureNotAssetHubAgent_AllowsBridgeHub() public { address bridgeHubAgent = MockGateway(address(gateway)).exposed_ensureNotAssetHubAgent(Constants.BRIDGE_HUB_AGENT_ID); @@ -874,36 +876,6 @@ contract GatewayV2Test is Test { ); } - // (7) Latent BridgeHub-agent bypass: SNOWBSC-532-style forgery on the - // Polkadot side via AliasOrigin(Here) hashes to BRIDGE_HUB_AGENT_ID, - // which this PR does NOT block. Today the BridgeHub agent on mainnet - // holds nothing, but the day anything funds it (or recognises its - // address as authoritative) this becomes exploitable. Skipped until - // the deny list is broadened — see PR review comments. - function testAgentCallContractFailsForBridgeHub() public { - vm.skip(true); - bytes32 topic = keccak256("topic-bh"); - - vm.expectEmit(true, false, false, false); - emit IGatewayV2.CommandFailed(1, 0); - vm.expectEmit(true, false, false, true); - emit IGatewayV2.InboundMessageDispatched(1, topic, false, relayerRewardAddress); - - hoax(relayer, 1 ether); - IGatewayV2(address(gateway)) - .v2_submit( - InboundMessageV2({ - origin: Constants.BRIDGE_HUB_AGENT_ID, - nonce: 1, - topic: topic, - commands: makeCallContractCommand(0) - }), - proof, - makeMockProof(), - relayerRewardAddress - ); - } - function testCreateAgent() public { bytes32 origin = bytes32(uint256(1)); vm.expectEmit(true, false, false, false); From 1c82414c90ca0cd2766c19ce2774140daffbbe06 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Fri, 22 May 2026 11:46:50 +0200 Subject: [PATCH 3/4] Drop ticket id from section comment Co-Authored-By: Claude Opus 4.7 (1M context) --- contracts/test/GatewayV2.t.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/test/GatewayV2.t.sol b/contracts/test/GatewayV2.t.sol index 0326e91869..acae967405 100644 --- a/contracts/test/GatewayV2.t.sol +++ b/contracts/test/GatewayV2.t.sol @@ -622,9 +622,9 @@ contract GatewayV2Test is Test { ); } - // SNOWBSC-532 follow-up: additional coverage around the AssetHub-agent - // callContract block introduced in #1788. Each test pins one invariant - // that the headline pair (success + AssetHub-fails) does not cover. + // Additional coverage around the AssetHub-agent callContract block + // introduced in #1788. Each test pins one invariant that the headline + // pair (success + AssetHub-fails) does not cover. // (1) Positive test using a v2_createAgent'd user-controlled agent. // This is the legitimate flow the deny list must keep working. From cfc2ef10c298dce31948c24f650290219b3475c6 Mon Sep 17 00:00:00 2001 From: claravanstaden Date: Fri, 22 May 2026 11:48:39 +0200 Subject: [PATCH 4/4] Drop PR reference from section comment Co-Authored-By: Claude Opus 4.7 (1M context) --- contracts/test/GatewayV2.t.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/test/GatewayV2.t.sol b/contracts/test/GatewayV2.t.sol index acae967405..0409c21b97 100644 --- a/contracts/test/GatewayV2.t.sol +++ b/contracts/test/GatewayV2.t.sol @@ -622,9 +622,9 @@ contract GatewayV2Test is Test { ); } - // Additional coverage around the AssetHub-agent callContract block - // introduced in #1788. Each test pins one invariant that the headline - // pair (success + AssetHub-fails) does not cover. + // Coverage for the AssetHub-agent callContract block. Each test pins + // one invariant that the headline pair (success + AssetHub-fails) does + // not cover. // (1) Positive test using a v2_createAgent'd user-controlled agent. // This is the legitimate flow the deny list must keep working.