From 8d5d6f49954c1ca8bfe701b337ce0b98bf56e953 Mon Sep 17 00:00:00 2001 From: Ameesha Agrawal Date: Mon, 24 Nov 2025 14:38:32 +0530 Subject: [PATCH 1/8] feat: n/n watcher --- contracts/protocol/Socket.sol | 1 - .../protocol/switchboard/EVMxSwitchboard.sol | 79 +++++++++++++++--- .../switchboard/MessageSwitchboard.sol | 82 +++++++++++++++++-- .../protocol/switchboard/SwitchboardBase.sol | 5 +- contracts/utils/common/Errors.sol | 6 +- 5 files changed, 149 insertions(+), 24 deletions(-) diff --git a/contracts/protocol/Socket.sol b/contracts/protocol/Socket.sol index 271f785f..277e2f7f 100644 --- a/contracts/protocol/Socket.sol +++ b/contracts/protocol/Socket.sol @@ -14,7 +14,6 @@ using LibCall for address; */ contract Socket is SocketUtils { // --- State Variables --- - /// @notice Mapping of payload id to execution status (Executed/Reverted) mapping(bytes32 => ExecutionStatus) public executionStatus; diff --git a/contracts/protocol/switchboard/EVMxSwitchboard.sol b/contracts/protocol/switchboard/EVMxSwitchboard.sol index e6183533..7e2c07f2 100644 --- a/contracts/protocol/switchboard/EVMxSwitchboard.sol +++ b/contracts/protocol/switchboard/EVMxSwitchboard.sol @@ -27,15 +27,24 @@ contract EVMxSwitchboard is SwitchboardBase { /// @notice Default deadline for payload execution (1 day) uint256 public defaultDeadline = 1 days; - /// @notice Mapping of digest to attestation status (true if attested by watcher) - mapping(bytes32 => bool) public isAttested; + /// @notice TotalWatchers registered + uint256 public totalWatchers; + + /// @notice Mapping of watcher address to digest to attestation status (true if attested by watcher) + mapping(address => mapping(bytes32 => bool)) public isAttested; + + /// @notice Mapping of digest to attestation count + mapping(bytes32 => uint256) public attestations; + + /// @notice Mapping of digest to validity status (true if digest is attested by enough watchers) + mapping(bytes32 => bool) public isValid; /// @notice Mapping of plug address to app gateway ID mapping(address => bytes32) public plugAppGatewayIds; // @notice Mapping of payload ID to plug address mapping(bytes32 => address) public payloadIdToPlug; - + // --- Events --- /// @notice Event emitted when watcher attests a payload @@ -88,15 +97,20 @@ contract EVMxSwitchboard is SwitchboardBase { * Payload is uniquely identified by digest. Once attested, payload can be executed. */ function attest(bytes32 digest_, bytes calldata proof_) public virtual { - // Prevent double attestation - if (isAttested[digest_]) revert AlreadyAttested(); address watcher = _recoverSigner( keccak256(abi.encodePacked(toBytes32Format(address(this)), chainSlug, digest_)), proof_ ); if (!_hasRole(WATCHER_ROLE, watcher)) revert WatcherNotFound(); - isAttested[digest_] = true; + // Prevent double attestation + if (isAttested[watcher][digest_]) revert AlreadyAttested(); + isAttested[watcher][digest_] = true; + attestations[digest_]++; + + // Mark digest as valid if enough attestations are reached + if (attestations[digest_] >= totalWatchers) isValid[digest_] = true; + emit Attested(digest_, watcher); } @@ -118,10 +132,10 @@ contract EVMxSwitchboard is SwitchboardBase { bytes32 appGatewayId = abi.decode(source_, (bytes32)); if (plugAppGatewayIds[target_] != appGatewayId) revert InvalidSource(); // Return true only if digest is attested - return isAttested[digest_]; + return isValid[digest_]; } - /** + /** * @inheritdoc ISwitchboard * @notice Processes a payload request and creates payload ID * @param plug_ The source plug address @@ -170,13 +184,13 @@ contract EVMxSwitchboard is SwitchboardBase { * @param plug_ The address of the plug * @param feesData_ Encoded fees data (type + data) * @dev Currently we don't support increasing fees for payloads in EVMxSwitchboard, but we will in the future. - * Currently only emitting the event. Verifications happen off-chain on evmx. + * Currently only emitting the event. Verifications happen off-chain on evmx. */ function increaseFeesForPayload( bytes32 payloadId_, address plug_, bytes calldata feesData_ - ) external override payable onlySocket { + ) external payable override onlySocket { if (payloadIdToPlug[payloadId_] != plug_) revert InvalidSource(); emit FeesIncreased(payloadId_, plug_, feesData_); } @@ -200,7 +214,10 @@ contract EVMxSwitchboard is SwitchboardBase { * @param isReverting_ True if payload should be marked as reverting * @dev Only callable by owner. Used to mark payloads that are known to revert. */ - function setRevertingPayload(bytes32 payloadId_, bool isReverting_) external onlyRole(WATCHER_ROLE) { + function setRevertingPayload( + bytes32 payloadId_, + bool isReverting_ + ) external onlyRole(WATCHER_ROLE) { revertingPayloadIds[payloadId_] = isReverting_; emit RevertingPayloadIdset(payloadId_, isReverting_); } @@ -227,4 +244,44 @@ contract EVMxSwitchboard is SwitchboardBase { ) external view override returns (bytes memory plugConfig_) { plugConfig_ = abi.encode(plugAppGatewayIds[plug_]); } + + /** + * @notice adds a watcher + * @param watcher_ watcher address + */ + function grantWatcherRole(address watcher_) external onlyRole(GOVERNANCE_ROLE) { + if (_hasRole(WATCHER_ROLE, watcher_)) revert WatcherFound(); + _grantRole(WATCHER_ROLE, watcher_); + + ++totalWatchers; + } + + /** + * @notice removes a watcher + * @param watcher_ watcher address + */ + function revokeWatcherRole(address watcher_) external onlyRole(GOVERNANCE_ROLE) { + if (!_hasRole(WATCHER_ROLE, watcher_)) revert WatcherNotFound(); + _revokeRole(WATCHER_ROLE, watcher_); + + --totalWatchers; + } + + /** + * @dev Overriding this function from AccessControl to make sure owner can't grant Watcher Role directly, and should + * only use grantWatcherRole function instead. This is to make sure watcher count remains correct + */ + function grantRole(bytes32 role_, address grantee_) external override onlyOwner { + if (role_ == WATCHER_ROLE) revert InvalidRole(); + _grantRole(role_, grantee_); + } + + /** + * @dev Overriding this function from AccessControl to make sure owner can't revoke Watcher Role directly, and should + * only use revokeWatcherRole function instead. This is to make sure watcher count remains correct + */ + function revokeRole(bytes32 role_, address grantee_) external override onlyOwner { + if (role_ == WATCHER_ROLE) revert InvalidRole(); + _revokeRole(role_, grantee_); + } } diff --git a/contracts/protocol/switchboard/MessageSwitchboard.sol b/contracts/protocol/switchboard/MessageSwitchboard.sol index dd271a00..6a44abb6 100644 --- a/contracts/protocol/switchboard/MessageSwitchboard.sol +++ b/contracts/protocol/switchboard/MessageSwitchboard.sol @@ -25,8 +25,17 @@ contract MessageSwitchboard is SwitchboardBase, ReentrancyGuard { /// @notice Default deadline for payload execution (1 day) uint256 public defaultDeadline = 1 days; - /// @notice Mapping of digest to attestation status (true if attested by watcher) - mapping(bytes32 => bool) public isAttested; + /// @notice Mapping of sibling chain slug to totalWatchers registered + mapping(uint32 => uint256) public totalWatchers; + + /// @notice Mapping of watcher address to digest to attestation status (true if attested by watcher) + mapping(address => mapping(bytes32 => bool)) public isAttested; + + /// @notice Mapping of digest to attestation count + mapping(bytes32 => uint256) public attestations; + + /// @notice Mapping of digest to validity status (true if digest is attested by enough watchers) + mapping(bytes32 => bool) public isValid; /// @notice Mapping of destination chain slug to sibling socket address (bytes32 format) mapping(uint32 => bytes32) public siblingSockets; @@ -58,7 +67,7 @@ contract MessageSwitchboard is SwitchboardBase, ReentrancyGuard { // --- Events --- /// @notice Event emitted when watcher attests a payload - event Attested(bytes32 payloadId, bytes32 digest, address watcher); + event Attested(bytes32 digest, address watcher); /// @notice Event emitted when message is sent outbound event MessageOutbound( @@ -412,13 +421,19 @@ contract MessageSwitchboard is SwitchboardBase, ReentrancyGuard { proof_ ); // Verify watcher has WATCHER_ROLE - if (!_hasRole(WATCHER_ROLE, watcher)) revert WatcherNotFound(); + bytes32 role = keccak256(abi.encode(WATCHER_ROLE, chainSlug)); + if (!_hasRole(role, watcher)) revert WatcherNotFound(); // Prevent double attestation - if (isAttested[digest]) revert AlreadyAttested(); - isAttested[digest] = true; + // Prevent double attestation + if (isAttested[watcher][digest]) revert AlreadyAttested(); + isAttested[watcher][digest] = true; + attestations[digest]++; + + // Mark digest as valid if enough attestations are reached + if (attestations[digest] >= totalWatchers[chainSlug]) isValid[digest] = true; - emit Attested(digest_.payloadId, digest, watcher); + emit Attested(digest, watcher); } /** @@ -439,7 +454,8 @@ contract MessageSwitchboard is SwitchboardBase, ReentrancyGuard { abi.encodePacked(toBytes32Format(address(this)), chainSlug, payloadId_, nonce_) ); address watcher = _recoverSigner(digest, signature_); - if (!_hasRole(WATCHER_ROLE, watcher)) revert WatcherNotFound(); + bytes32 role = keccak256(abi.encode(WATCHER_ROLE, chainSlug)); + if (!_hasRole(role, watcher)) revert WatcherNotFound(); if (usedNonces[watcher][nonce_]) revert NonceAlreadyUsed(); usedNonces[watcher][nonce_] = true; @@ -666,7 +682,7 @@ contract MessageSwitchboard is SwitchboardBase, ReentrancyGuard { (uint32 siblingChainSlug, bytes32 siblingPlug) = _decodePackedSource(sibling_); if (siblingPlugs[siblingChainSlug][target_] != siblingPlug) revert InvalidSource(); // digest has enough attestations - return isAttested[digest_]; + return isValid[digest_]; } /** @@ -732,4 +748,52 @@ contract MessageSwitchboard is SwitchboardBase, ReentrancyGuard { uint32 chainSlug_ = abi.decode(extraData_, (uint32)); plugConfig_ = abi.encode(siblingPlugs[chainSlug_][plug_]); } + + /** + * @notice adds a watcher + * @param watcher_ watcher address + */ + function grantWatcherRole( + uint32 siblingChainSlug_, + address watcher_ + ) external onlyRole(GOVERNANCE_ROLE) { + bytes32 role = keccak256(abi.encode(WATCHER_ROLE, siblingChainSlug_)); + if (_hasRole(role, watcher_)) revert WatcherFound(); + _grantRole(role, watcher_); + + ++totalWatchers[siblingChainSlug_]; + } + + /** + * @notice removes a watcher + * @param watcher_ watcher address + */ + function revokeWatcherRole( + uint32 siblingChainSlug_, + address watcher_ + ) external onlyRole(GOVERNANCE_ROLE) { + bytes32 role = keccak256(abi.encode(WATCHER_ROLE, siblingChainSlug_)); + if (!_hasRole(role, watcher_)) revert WatcherNotFound(); + _revokeRole(role, watcher_); + + --totalWatchers[siblingChainSlug_]; + } + + /** + * @dev Overriding this function from AccessControl to make sure owner can't grant Watcher Role directly, and should + * only use grantWatcherRole function instead. This is to make sure watcher count remains correct + */ + function grantRole(bytes32 role_, address grantee_) external override onlyOwner { + if (role_ != GOVERNANCE_ROLE && role_ != RESCUE_ROLE && role_ != FEE_UPDATER_ROLE) revert InvalidRole(); + _grantRole(role_, grantee_); + } + + /** + * @dev Overriding this function from AccessControl to make sure owner can't revoke Watcher Role directly, and should + * only use revokeWatcherRole function instead. This is to make sure watcher count remains correct + */ + function revokeRole(bytes32 role_, address grantee_) external override onlyOwner { + if (role_ != GOVERNANCE_ROLE && role_ != RESCUE_ROLE && role_ != FEE_UPDATER_ROLE) revert InvalidRole(); + _revokeRole(role_, grantee_); + } } diff --git a/contracts/protocol/switchboard/SwitchboardBase.sol b/contracts/protocol/switchboard/SwitchboardBase.sol index 9b5db580..130e7bbf 100644 --- a/contracts/protocol/switchboard/SwitchboardBase.sol +++ b/contracts/protocol/switchboard/SwitchboardBase.sol @@ -5,7 +5,7 @@ import {ECDSA} from "solady/utils/ECDSA.sol"; import "../interfaces/ISocket.sol"; import "../interfaces/ISwitchboard.sol"; import "../../utils/AccessControl.sol"; -import {RESCUE_ROLE} from "../../utils/common/AccessRoles.sol"; +import {RESCUE_ROLE, GOVERNANCE_ROLE} from "../../utils/common/AccessRoles.sol"; import "../../utils/common/Errors.sol"; import "../../utils/RescueFundsLib.sol"; @@ -41,9 +41,10 @@ abstract contract SwitchboardBase is ISwitchboard, AccessControl { if (chainSlug_ == 0) revert InvalidChainSlug(); if (address(socket_) == address(0)) revert InvalidSocket(); if (owner_ == address(0)) revert InvalidOwner(); + + _initializeOwner(owner_); chainSlug = chainSlug_; socket__ = socket_; - _initializeOwner(owner_); switchboardId = socket__.registerSwitchboard(); } diff --git a/contracts/utils/common/Errors.sol b/contracts/utils/common/Errors.sol index 56cbe034..2cb8612c 100644 --- a/contracts/utils/common/Errors.sol +++ b/contracts/utils/common/Errors.sol @@ -160,4 +160,8 @@ error InvalidOwner(); /// @notice Thrown when socket is invalid error InvalidSocket(); /// @notice Thrown when switchboard id is invalid -error InvalidSwitchboardId(); \ No newline at end of file +error InvalidSwitchboardId(); +/// @notice Thrown when role is invalid +error InvalidRole(); +/// @notice Thrown when watcher is already found +error WatcherFound(); \ No newline at end of file From c7c5ff56bc9733760bd22407209dfdef1013ba74 Mon Sep 17 00:00:00 2001 From: Ameesha Agrawal Date: Mon, 24 Nov 2025 14:38:43 +0530 Subject: [PATCH 2/8] fix: tests --- test/SetupTest.t.sol | 8 +- .../switchboard/FastSwitchboard.t.sol | 118 ++++++++++-------- .../switchboard/MessageSwitchboard.t.sol | 9 +- 3 files changed, 80 insertions(+), 55 deletions(-) diff --git a/test/SetupTest.t.sol b/test/SetupTest.t.sol index 009e797e..387ed0eb 100644 --- a/test/SetupTest.t.sol +++ b/test/SetupTest.t.sol @@ -231,10 +231,14 @@ contract DeploySetup is SetupStore { socket.grantRole(SWITCHBOARD_DISABLER_ROLE, address(socketOwner)); // switchboard - switchboard.grantRole(WATCHER_ROLE, watcherEOA); switchboard.grantRole(RESCUE_ROLE, address(socketOwner)); + switchboard.grantRole(GOVERNANCE_ROLE, address(socketOwner)); + switchboard.grantWatcherRole(watcherEOA); - messageSwitchboard.grantRole(WATCHER_ROLE, watcherEOA); + messageSwitchboard.grantRole(GOVERNANCE_ROLE, address(socketOwner)); + messageSwitchboard.grantRole(RESCUE_ROLE, address(socketOwner)); + messageSwitchboard.grantWatcherRole(arbChainSlug, watcherEOA); + messageSwitchboard.grantWatcherRole(optChainSlug, watcherEOA); gasStation.grantRole(RESCUE_ROLE, address(socketOwner)); gasStation.whitelistToken(address(socketConfig.testUSDC)); diff --git a/test/protocol/switchboard/FastSwitchboard.t.sol b/test/protocol/switchboard/FastSwitchboard.t.sol index 32ad383e..64626d67 100644 --- a/test/protocol/switchboard/FastSwitchboard.t.sol +++ b/test/protocol/switchboard/FastSwitchboard.t.sol @@ -18,7 +18,6 @@ import "../../Utils.t.sol"; * @dev Base contract for EVMxSwitchboard tests with common setup and helper methods */ contract EVMxSwitchboardTestBase is Test, Utils { - // Test constants uint32 constant CHAIN_SLUG = 1; uint32 constant OTHER_CHAIN_SLUG = 2; @@ -28,7 +27,7 @@ contract EVMxSwitchboardTestBase is Test, Utils { address owner = address(0x1000); address plugOwner = address(0x2000); address watcher = address(0x3000); - + // Private key for watcher signing uint256 watcherPrivateKey = 0x1111111111111111111111111111111111111111111111111111111111111111; @@ -46,11 +45,19 @@ contract EVMxSwitchboardTestBase is Test, Utils { socket = new Socket(CHAIN_SLUG, owner); // Deploy switchboards - evmxSwitchboard = new EVMxSwitchboard(CHAIN_SLUG, socket, owner, EVMX_CHAIN_SLUG, WATCHER_ID); + evmxSwitchboard = new EVMxSwitchboard( + CHAIN_SLUG, + socket, + owner, + EVMX_CHAIN_SLUG, + WATCHER_ID + ); messageSwitchboard = new MessageSwitchboard(CHAIN_SLUG, socket, owner); hoax(owner); - evmxSwitchboard.grantRole(WATCHER_ROLE, getWatcherAddress()); + evmxSwitchboard.grantRole(GOVERNANCE_ROLE, owner); + hoax(owner); + evmxSwitchboard.grantWatcherRole(getWatcherAddress()); // Get switchboard ID switchboardId = evmxSwitchboard.switchboardId(); @@ -72,18 +79,19 @@ contract EVMxSwitchboardTestBase is Test, Utils { bytes memory payload_, bytes memory source_ ) internal view returns (ExecutionParams memory) { - return ExecutionParams({ - callType: WRITE, - payloadId: payloadId_, - deadline: block.timestamp + 3600, - gasLimit: 100000, - value: 0, - payload: payload_, - target: target_, - prevBatchDigestHash: bytes32(0), - source: source_, - extraData: bytes("") - }); + return + ExecutionParams({ + callType: WRITE, + payloadId: payloadId_, + deadline: block.timestamp + 3600, + gasLimit: 100000, + value: 0, + payload: payload_, + target: target_, + prevBatchDigestHash: bytes32(0), + source: source_, + extraData: bytes("") + }); } /** @@ -98,30 +106,32 @@ contract EVMxSwitchboardTestBase is Test, Utils { uint256 gasLimit_, uint256 value_ ) internal view returns (ExecutionParams memory) { - return ExecutionParams({ - callType: WRITE, - payloadId: payloadId_, - deadline: deadline_, - gasLimit: gasLimit_, - value: value_, - payload: payload_, - target: target_, - prevBatchDigestHash: bytes32(0), - source: source_, - extraData: bytes("") - }); + return + ExecutionParams({ + callType: WRITE, + payloadId: payloadId_, + deadline: deadline_, + gasLimit: gasLimit_, + value: value_, + payload: payload_, + target: target_, + prevBatchDigestHash: bytes32(0), + source: source_, + extraData: bytes("") + }); } /** * @dev Helper to create TransmissionParams with default values */ function _createTransmissionParams() internal pure returns (TransmissionParams memory) { - return TransmissionParams({ - socketFees: 0, - refundAddress: address(0), - extraData: bytes(""), - transmitterProof: bytes("") - }); + return + TransmissionParams({ + socketFees: 0, + refundAddress: address(0), + extraData: bytes(""), + transmitterProof: bytes("") + }); } /** @@ -131,12 +141,13 @@ contract EVMxSwitchboardTestBase is Test, Utils { uint256 socketFees_, address refundAddress_ ) internal pure returns (TransmissionParams memory) { - return TransmissionParams({ - socketFees: socketFees_, - refundAddress: refundAddress_, - extraData: bytes(""), - transmitterProof: bytes("") - }); + return + TransmissionParams({ + socketFees: socketFees_, + refundAddress: refundAddress_, + extraData: bytes(""), + transmitterProof: bytes("") + }); } /** @@ -152,7 +163,11 @@ contract EVMxSwitchboardTestBase is Test, Utils { /** * @dev Helper to create default payload and overrides for processPayload tests */ - function _createPayloadAndOverrides() internal pure returns (bytes memory payload, bytes memory overrides) { + function _createPayloadAndOverrides() + internal + pure + returns (bytes memory payload, bytes memory overrides) + { payload = abi.encode("test"); overrides = abi.encode(uint256(0)); // Pass 0 to use default deadline } @@ -180,7 +195,6 @@ contract EVMxSwitchboardTestBase is Test, Utils { * @dev Tests for payload ID verification in Socket.execute() and EVMxSwitchboard payload creation */ contract SocketPayloadIdVerificationTest is EVMxSwitchboardTestBase { - // ============================================ // TESTS - Socket.execute() Payload ID Verification // ============================================ @@ -415,7 +429,7 @@ contract SocketPayloadIdVerificationTest is EVMxSwitchboardTestBase { vm.prank(getWatcherAddress()); evmxSwitchboard.attest(digest, signature); - assertTrue(evmxSwitchboard.isAttested(digest), "Digest should be attested"); + assertTrue(evmxSwitchboard.isValid(digest), "Digest should be valid"); } function test_Attest_AlreadyAttested_Reverts() public { @@ -434,7 +448,7 @@ contract SocketPayloadIdVerificationTest is EVMxSwitchboardTestBase { function test_Attest_InvalidWatcher_Reverts() public { bytes32 digest = keccak256(abi.encode("test payload")); - + // Create signature with invalid private key (non-watcher) bytes32 signatureDigest = keccak256( abi.encodePacked(toBytes32Format(address(evmxSwitchboard)), CHAIN_SLUG, digest) @@ -454,7 +468,7 @@ contract SocketPayloadIdVerificationTest is EVMxSwitchboardTestBase { function test_AllowPayload_InvalidSource_Reverts() public { bytes32 digest = keccak256(abi.encode("test")); bytes32 appGatewayId = toBytes32Format(address(0x1234)); - + // Set up plug config with different appGatewayId bytes memory plugConfig = abi.encode(toBytes32Format(address(0x5678))); vm.prank(address(socket)); @@ -469,7 +483,7 @@ contract SocketPayloadIdVerificationTest is EVMxSwitchboardTestBase { function test_AllowPayload_ValidSource_ReturnsTrue() public { bytes32 digest = keccak256(abi.encode("test")); bytes32 appGatewayId = toBytes32Format(address(0x1234)); - + // Set up plug config bytes memory plugConfig = abi.encode(appGatewayId); vm.prank(address(socket)); @@ -489,7 +503,7 @@ contract SocketPayloadIdVerificationTest is EVMxSwitchboardTestBase { function test_AllowPayload_NotAttested_ReturnsFalse() public { bytes32 digest = keccak256(abi.encode("test")); bytes32 appGatewayId = toBytes32Format(address(0x1234)); - + // Set up plug config bytes memory plugConfig = abi.encode(appGatewayId); vm.prank(address(socket)); @@ -614,7 +628,11 @@ contract SocketPayloadIdVerificationTest is EVMxSwitchboardTestBase { vm.prank(owner); evmxSwitchboard.setDefaultDeadline(newDeadline); - assertEq(evmxSwitchboard.defaultDeadline(), newDeadline, "Default deadline should be updated"); + assertEq( + evmxSwitchboard.defaultDeadline(), + newDeadline, + "Default deadline should be updated" + ); } function test_SetDefaultDeadline_OnlyOwner() public { @@ -657,7 +675,7 @@ contract SocketPayloadIdVerificationTest is EVMxSwitchboardTestBase { function test_ProcessPayload_WithCustomDeadline() public { MockPlug triggerPlug = _createTriggerPlug(); bytes memory payload = abi.encode("test"); - + // Use custom deadline (not 0) uint256 customDeadline = block.timestamp + 2 days; bytes memory overrides = abi.encode(customDeadline); @@ -684,7 +702,7 @@ contract SocketPayloadIdVerificationTest is EVMxSwitchboardTestBase { function test_ProcessPayload_WithZeroDeadline_UsesDefault() public { MockPlug triggerPlug = _createTriggerPlug(); bytes memory payload = abi.encode("test"); - + // Pass 0 as deadline - should use default bytes memory overrides = abi.encode(uint256(0)); diff --git a/test/protocol/switchboard/MessageSwitchboard.t.sol b/test/protocol/switchboard/MessageSwitchboard.t.sol index 79e49ee7..5ca236b3 100644 --- a/test/protocol/switchboard/MessageSwitchboard.t.sol +++ b/test/protocol/switchboard/MessageSwitchboard.t.sol @@ -46,9 +46,12 @@ contract MessageSwitchboardTest is Test, Utils { // Setup roles - grant watcher role to the address derived from watcherPrivateKey address actualWatcherAddress = getWatcherAddress(); address actualFeeUpdaterAddress = getFeeUpdaterAddress(); + vm.startPrank(owner); - messageSwitchboard.grantRole(WATCHER_ROLE, actualWatcherAddress); + messageSwitchboard.grantRole(GOVERNANCE_ROLE, owner); + messageSwitchboard.grantRole(RESCUE_ROLE, owner); messageSwitchboard.grantRole(FEE_UPDATER_ROLE, actualFeeUpdaterAddress); + messageSwitchboard.grantWatcherRole(DST_CHAIN, actualWatcherAddress); vm.stopPrank(); uint32 switchboardId = messageSwitchboard.switchboardId(); @@ -702,11 +705,11 @@ contract MessageSwitchboardTest is Test, Utils { // Register this digest as attested (simulating the flow) vm.prank(getWatcherAddress()); vm.expectEmit(true, false, true, true); - emit MessageSwitchboard.Attested(payloadId, digest, getWatcherAddress()); + emit MessageSwitchboard.Attested(digest, getWatcherAddress()); messageSwitchboard.attest(digestParams, signature); // Verify it's attested - assertTrue(messageSwitchboard.isAttested(digest)); + assertTrue(messageSwitchboard.isValid(digest)); } // NOTE: test_attest_InvalidTarget_Reverts() was removed because the attest() function From 426f352c592c490155d8fe6ebf5a70f577058619 Mon Sep 17 00:00:00 2001 From: Ameesha Agrawal Date: Mon, 24 Nov 2025 15:18:00 +0530 Subject: [PATCH 3/8] fix: tests --- .../switchboard/MessageSwitchboard.sol | 39 +++-- .../switchboard/MessageSwitchboard.t.sol | 144 +++++++++++++----- 2 files changed, 131 insertions(+), 52 deletions(-) diff --git a/contracts/protocol/switchboard/MessageSwitchboard.sol b/contracts/protocol/switchboard/MessageSwitchboard.sol index 614cae9f..6abe660e 100644 --- a/contracts/protocol/switchboard/MessageSwitchboard.sol +++ b/contracts/protocol/switchboard/MessageSwitchboard.sol @@ -162,8 +162,25 @@ contract MessageSwitchboard is SwitchboardBase, ReentrancyGuard { function setRevertingPayload( bytes32 payloadId_, - bool isReverting_ - ) external onlyRole(WATCHER_ROLE) { + bool isReverting_, + uint256 nonce_, + bytes calldata signature_ + ) external { + bytes32 digest = keccak256( + abi.encodePacked( + toBytes32Format(address(this)), + chainSlug, + payloadId_, + isReverting_, + nonce_ + ) + ); + + address watcher = _recoverSigner(digest, signature_); + bytes32 role = keccak256(abi.encode(WATCHER_ROLE, chainSlug)); + if (!_hasRole(role, watcher)) revert WatcherNotFound(); + _validateAndUseNonce(this.setRevertingPayload.selector, watcher, nonce_); + revertingPayloadIds[payloadId_] = isReverting_; emit RevertingPayloadIdset(payloadId_, isReverting_); } @@ -407,14 +424,14 @@ contract MessageSwitchboard is SwitchboardBase, ReentrancyGuard { if (!_hasRole(role, watcher)) revert WatcherNotFound(); // Prevent double attestation - if (isAttested[watcher][digest]) revert AlreadyAttested(); - isAttested[watcher][digest] = true; - attestations[digest]++; + if (isAttested[watcher][digest_]) revert AlreadyAttested(); + isAttested[watcher][digest_] = true; + attestations[digest_]++; - // Mark digest as valid if enough attestations are reached - if (attestations[digest] >= totalWatchers[chainSlug]) isValid[digest] = true; + // Mark digest_ as valid if enough attestations are reached + if (attestations[digest_] >= totalWatchers[chainSlug]) isValid[digest_] = true; - emit Attested(digest, watcher); + emit Attested(digest_, watcher); } /** @@ -711,7 +728,8 @@ contract MessageSwitchboard is SwitchboardBase, ReentrancyGuard { * only use grantWatcherRole function instead. This is to make sure watcher count remains correct */ function grantRole(bytes32 role_, address grantee_) external override onlyOwner { - if (role_ != GOVERNANCE_ROLE && role_ != RESCUE_ROLE && role_ != FEE_UPDATER_ROLE) revert InvalidRole(); + if (role_ != GOVERNANCE_ROLE && role_ != RESCUE_ROLE && role_ != FEE_UPDATER_ROLE) + revert InvalidRole(); _grantRole(role_, grantee_); } @@ -720,7 +738,8 @@ contract MessageSwitchboard is SwitchboardBase, ReentrancyGuard { * only use revokeWatcherRole function instead. This is to make sure watcher count remains correct */ function revokeRole(bytes32 role_, address grantee_) external override onlyOwner { - if (role_ != GOVERNANCE_ROLE && role_ != RESCUE_ROLE && role_ != FEE_UPDATER_ROLE) revert InvalidRole(); + if (role_ != GOVERNANCE_ROLE && role_ != RESCUE_ROLE && role_ != FEE_UPDATER_ROLE) + revert InvalidRole(); _revokeRole(role_, grantee_); } } diff --git a/test/protocol/switchboard/MessageSwitchboard.t.sol b/test/protocol/switchboard/MessageSwitchboard.t.sol index 08378964..6b582d04 100644 --- a/test/protocol/switchboard/MessageSwitchboard.t.sol +++ b/test/protocol/switchboard/MessageSwitchboard.t.sol @@ -27,7 +27,8 @@ contract MessageSwitchboardTest is Test, Utils { // Private keys for signing uint256 watcherPrivateKey = 0x1111111111111111111111111111111111111111111111111111111111111111; - uint256 feeUpdaterPrivateKey = 0x5555555555555555555555555555555555555555555555555555555555555555; + uint256 feeUpdaterPrivateKey = + 0x5555555555555555555555555555555555555555555555555555555555555555; // Contracts Socket socket; @@ -49,10 +50,9 @@ contract MessageSwitchboardTest is Test, Utils { messageSwitchboard.grantRole(GOVERNANCE_ROLE, owner); messageSwitchboard.grantRole(RESCUE_ROLE, owner); messageSwitchboard.grantRole(FEE_UPDATER_ROLE, actualFeeUpdaterAddress); - messageSwitchboard.grantWatcherRole(DST_CHAIN, actualWatcherAddress); + messageSwitchboard.grantWatcherRole(SRC_CHAIN, actualWatcherAddress); vm.stopPrank(); - switchboardId = messageSwitchboard.switchboardId(); // Now create plugs with the registered switchboard ID srcPlug = new MockPlug(address(socket), switchboardId); @@ -270,7 +270,15 @@ contract MessageSwitchboardTest is Test, Utils { bytes32 payloadId, bytes memory payload ) internal view returns (DigestParams memory) { - return _createDigestParams(payloadId, payload, address(dstPlug), 100000, 0, block.timestamp + 3600); + return + _createDigestParams( + payloadId, + payload, + address(dstPlug), + 100000, + 0, + block.timestamp + 3600 + ); } /** @@ -297,7 +305,8 @@ contract MessageSwitchboardTest is Test, Utils { bytes32 siblingPlug = messageSwitchboard.siblingPlugs(dstChainSlug, plug_); // Contract uses overrides.deadline, or block.timestamp + defaultDeadline if deadline is 0 - uint256 deadline = block.timestamp + (deadline_ > 0 ? deadline_ : messageSwitchboard.defaultDeadline()); + uint256 deadline = block.timestamp + + (deadline_ > 0 ? deadline_ : messageSwitchboard.defaultDeadline()); return DigestParams({ @@ -341,10 +350,18 @@ contract MessageSwitchboardTest is Test, Utils { * @param nonce The nonce to include in the signature * @return signature The watcher signature */ - function _createWatcherSignature(bytes32 payloadId, uint256 nonce) internal view returns (bytes memory) { + function _createWatcherSignature( + bytes32 payloadId, + uint256 nonce + ) internal view returns (bytes memory) { // markRefundEligible signs: keccak256(abi.encodePacked(switchboardAddress, chainSlug, payloadId, nonce)) bytes32 digest = keccak256( - abi.encodePacked(toBytes32Format(address(messageSwitchboard)), SRC_CHAIN, payloadId, nonce) + abi.encodePacked( + toBytes32Format(address(messageSwitchboard)), + SRC_CHAIN, + payloadId, + nonce + ) ); return createSignature(digest, watcherPrivateKey); } @@ -358,8 +375,11 @@ contract MessageSwitchboardTest is Test, Utils { return _createWatcherSignature(payloadId, 0); } - function _createSetMinMsgValueFeesBatchSignature(uint32[] memory chainSlugs, uint256[] memory minFees, uint256 nonce) internal view returns (bytes memory) { - + function _createSetMinMsgValueFeesBatchSignature( + uint32[] memory chainSlugs, + uint256[] memory minFees, + uint256 nonce + ) internal view returns (bytes memory) { bytes32 digest = keccak256( abi.encodePacked( toBytes32Format(address(messageSwitchboard)), @@ -380,7 +400,7 @@ contract MessageSwitchboardTest is Test, Utils { function _approvePlugForSponsor() internal { vm.prank(sponsor); address[] memory plugs = new address[](1); - plugs[0] = address(srcPlug); + plugs[0] = address(srcPlug); messageSwitchboard.approvePlugs(plugs); } @@ -447,7 +467,11 @@ contract MessageSwitchboardTest is Test, Utils { _setupSiblingConfig(); vm.expectEmit(true, true, true, true); - emit MessageSwitchboard.PlugConfigUpdated(address(srcPlug), DST_CHAIN, toBytes32Format(address(dstPlug))); + emit MessageSwitchboard.PlugConfigUpdated( + address(srcPlug), + DST_CHAIN, + toBytes32Format(address(dstPlug)) + ); srcPlug.registerSibling(DST_CHAIN, address(dstPlug)); bytes memory plugConfig = messageSwitchboard.getPlugConfig( @@ -503,7 +527,10 @@ contract MessageSwitchboardTest is Test, Utils { // Expect MessageOutbound event first (contract emits this before PayloadRequested) // Calculate expected digestParams and digest // Decode deadline from overrides (86400 in this test) - (, , , , , uint256 deadline) = abi.decode(overrides, (uint8, uint32, uint256, uint256, address, uint256)); + (, , , , , uint256 deadline) = abi.decode( + overrides, + (uint8, uint32, uint256, uint256, address, uint256) + ); DigestParams memory expectedDigestParams = _createExpectedDigestParamsForProcessPayload( expectedPayloadId, DST_CHAIN, @@ -631,7 +658,10 @@ contract MessageSwitchboardTest is Test, Utils { // Expect MessageOutbound event first (contract emits this before PayloadRequested) // Calculate expected digestParams and digest // Decode deadline from overrides (version 2: uint8, uint32, uint256, uint256, uint256, address, uint256) - (, , , , , , uint256 deadline) = abi.decode(overrides, (uint8, uint32, uint256, uint256, uint256, address, uint256)); + (, , , , , , uint256 deadline) = abi.decode( + overrides, + (uint8, uint32, uint256, uint256, uint256, address, uint256) + ); DigestParams memory expectedDigestParams = _createExpectedDigestParamsForProcessPayload( expectedPayloadId, DST_CHAIN, @@ -872,9 +902,11 @@ contract MessageSwitchboardTest is Test, Utils { // Verify marked eligible (, , bool isEligible, , ) = messageSwitchboard.payloadFees(payloadId); assertTrue(isEligible); - + // Verify nonce was used - uint256 namespacedNonce = uint256(keccak256(abi.encodePacked(messageSwitchboard.markRefundEligible.selector, nonce))); + uint256 namespacedNonce = uint256( + keccak256(abi.encodePacked(messageSwitchboard.markRefundEligible.selector, nonce)) + ); assertTrue(messageSwitchboard.usedNonces(getWatcherAddress(), namespacedNonce)); } @@ -1282,12 +1314,18 @@ contract MessageSwitchboardTest is Test, Utils { function test_setRevertingPayload_Success() public { bytes32 payloadId = bytes32(uint256(0x1234)); bool isReverting = true; + uint256 nonce = 1; - vm.expectEmit(true, false, false, true); + bytes32 digest = keccak256( + abi.encodePacked(toBytes32Format(address(messageSwitchboard)), SRC_CHAIN, payloadId, isReverting, nonce) + ); + bytes memory signature = createSignature(digest, watcherPrivateKey); + + vm.expectEmit(true, true, false, true); emit MessageSwitchboard.RevertingPayloadIdset(payloadId, isReverting); vm.prank(getWatcherAddress()); - messageSwitchboard.setRevertingPayload(payloadId, isReverting); + messageSwitchboard.setRevertingPayload(payloadId, isReverting, nonce, signature); assertTrue(messageSwitchboard.revertingPayloadIds(payloadId)); } @@ -1295,26 +1333,58 @@ contract MessageSwitchboardTest is Test, Utils { function test_setRevertingPayload_NotOwner_Reverts() public { bytes32 payloadId = bytes32(uint256(0x1234)); bool isReverting = true; + uint256 nonce = 1; + + bytes32 digest = keccak256( + abi.encodePacked(address(messageSwitchboard), SRC_CHAIN, payloadId, isReverting, nonce) + ); + bytes memory signature = createSignature(digest, feeUpdaterPrivateKey); - vm.prank(address(0x9999)); vm.expectRevert(); - messageSwitchboard.setRevertingPayload(payloadId, isReverting); + messageSwitchboard.setRevertingPayload(payloadId, isReverting, nonce, signature); } function test_setRevertingPayload_SetToFalse() public { bytes32 payloadId = bytes32(uint256(0x1234)); - + uint256 nonce = 1; + bool isReverting = true; + bytes32 digest = keccak256( + abi.encodePacked( + toBytes32Format(address(messageSwitchboard)), + SRC_CHAIN, + payloadId, + isReverting, + nonce + ) + ); + + bytes memory signature = createSignature(digest, watcherPrivateKey); + // First set to true vm.prank(getWatcherAddress()); - messageSwitchboard.setRevertingPayload(payloadId, true); + messageSwitchboard.setRevertingPayload(payloadId, isReverting, nonce, signature); assertTrue(messageSwitchboard.revertingPayloadIds(payloadId)); + nonce++; + isReverting = false; + digest = keccak256( + abi.encodePacked( + toBytes32Format(address(messageSwitchboard)), + SRC_CHAIN, + payloadId, + isReverting, + nonce + ) + ); + + signature = createSignature(digest, watcherPrivateKey); + // Then set to false vm.expectEmit(true, false, false, true); - emit MessageSwitchboard.RevertingPayloadIdset(payloadId, false); + emit MessageSwitchboard.RevertingPayloadIdset(payloadId, isReverting); vm.prank(getWatcherAddress()); - messageSwitchboard.setRevertingPayload(payloadId, false); + messageSwitchboard.setRevertingPayload(payloadId, isReverting, nonce, signature); assertFalse(messageSwitchboard.revertingPayloadIds(payloadId)); } @@ -1487,7 +1557,11 @@ contract MessageSwitchboardTest is Test, Utils { bytes memory plugConfig = abi.encode(SRC_CHAIN, toBytes32Format(address(dstPlug))); vm.expectEmit(true, true, true, true); - emit MessageSwitchboard.PlugConfigUpdated(address(srcPlug), SRC_CHAIN, toBytes32Format(address(dstPlug))); + emit MessageSwitchboard.PlugConfigUpdated( + address(srcPlug), + SRC_CHAIN, + toBytes32Format(address(dstPlug)) + ); vm.prank(address(socket)); messageSwitchboard.updatePlugConfig(address(srcPlug), plugConfig); @@ -1575,6 +1649,7 @@ contract MessageSwitchboardTest is Test, Utils { abi.encodePacked(toBytes32Format(address(messageSwitchboard)), SRC_CHAIN, digest) ); bytes memory signature = createSignature(signatureDigest, watcherPrivateKey); + vm.prank(getWatcherAddress()); messageSwitchboard.attest(digest, signature); @@ -1604,12 +1679,7 @@ contract MessageSwitchboardTest is Test, Utils { bytes memory source = abi.encodePacked(SRC_CHAIN, toBytes32Format(address(srcPlug))); // allowPayload should return false for non-attested digest - bool result = messageSwitchboard.allowPayload( - digest, - payloadId, - address(dstPlug), - source - ); + bool result = messageSwitchboard.allowPayload(digest, payloadId, address(dstPlug), source); assertFalse(result); } @@ -1638,12 +1708,7 @@ contract MessageSwitchboardTest is Test, Utils { messageSwitchboard.attest(digest, signature); // allowPayload uses _decodePackedSource internally - bool result = messageSwitchboard.allowPayload( - digest, - payloadId, - address(dstPlug), - packed - ); + bool result = messageSwitchboard.allowPayload(digest, payloadId, address(dstPlug), packed); assertTrue(result); } @@ -1661,11 +1726,6 @@ contract MessageSwitchboardTest is Test, Utils { // allowPayload will call _decodePackedSource which should revert vm.expectRevert("Invalid packed length"); - messageSwitchboard.allowPayload( - digest, - payloadId, - address(dstPlug), - invalidSource - ); + messageSwitchboard.allowPayload(digest, payloadId, address(dstPlug), invalidSource); } } From 8554de9b3bb666b61f11ea21f39c608507972590 Mon Sep 17 00:00:00 2001 From: Ameesha Agrawal Date: Mon, 24 Nov 2025 15:18:56 +0530 Subject: [PATCH 4/8] fix: lint --- .prettierignore | 3 +- auditor-docs/AUDIT_FOCUS_AREAS.md | 133 +++++++++++-- auditor-docs/AUDIT_PREP_SUMMARY.md | 64 +++++-- auditor-docs/CONTRACTS_REFERENCE.md | 147 ++++++++------- auditor-docs/FAQ.md | 174 +++++++++++++++--- auditor-docs/MESSAGE_FLOW.md | 70 ++++++- auditor-docs/README.md | 80 ++++++-- auditor-docs/SECURITY_MODEL.md | 170 +++++++++++------ auditor-docs/SETUP_GUIDE.md | 58 +++++- auditor-docs/SYSTEM_OVERVIEW.md | 32 +++- auditor-docs/TESTING_COVERAGE.md | 132 ++++++++++--- contracts/evmx/base/AppGatewayBase.sol | 3 +- contracts/evmx/helpers/ForwarderSolana.sol | 10 +- contracts/protocol/Socket.sol | 19 +- contracts/protocol/SocketConfig.sol | 2 +- contracts/protocol/SocketUtils.sol | 4 +- contracts/protocol/base/MessagePlugBase.sol | 2 +- .../protocol/switchboard/EVMxSwitchboard.sol | 8 +- contracts/utils/common/Constants.sol | 1 - contracts/utils/common/Errors.sol | 2 +- test/PausableTest.t.sol | 3 +- test/encode.t.sol | 101 +++++----- test/protocol/Socket.t.sol | 14 +- .../switchboard/MessageSwitchboard.t.sol | 8 +- 24 files changed, 916 insertions(+), 324 deletions(-) diff --git a/.prettierignore b/.prettierignore index 9999ea6a..d31c61eb 100644 --- a/.prettierignore +++ b/.prettierignore @@ -36,4 +36,5 @@ setupInfraContracts.sh testScript.sh trace.sh coverage-report/ -internal-audit/ \ No newline at end of file +internal-audit/ +foundry.lock \ No newline at end of file diff --git a/auditor-docs/AUDIT_FOCUS_AREAS.md b/auditor-docs/AUDIT_FOCUS_AREAS.md index 6bfad5b9..acf1d669 100644 --- a/auditor-docs/AUDIT_FOCUS_AREAS.md +++ b/auditor-docs/AUDIT_FOCUS_AREAS.md @@ -3,15 +3,18 @@ ## Priority 1: Critical Functions ### Socket.execute() - Main Entry Point + **File**: `contracts/protocol/Socket.sol` (lines 46-74) **Why Critical**: + - Handles all inbound payload execution - Processes value transfers - Makes external calls to untrusted contracts - Single point of failure for cross-chain execution **Key Validations to Verify**: + - Deadline enforcement - Replay protection via executionStatus - msg.value sufficiency check @@ -19,6 +22,7 @@ - Call type restriction (WRITE only) **Security Pattern**: CEI (Checks-Effects-Interactions) + - executionStatus set BEFORE external call to plug - payloadIdToDigest stored BEFORE external call - Different payloadIds during reentrancy are legitimate @@ -27,16 +31,19 @@ --- -### Socket._execute() - Payload Execution +### Socket.\_execute() - Payload Execution + **File**: `contracts/protocol/Socket.sol` (lines 122-161) **Why Critical**: + - Performs untrusted external call to plug - Handles value transfer to plug - Manages execution success/failure - Collects network fees **Key Checks**: + - Gas limit validation: `gasleft() >= (gasLimit * gasLimitBuffer) / 100` - gasLimit type: uint64 (prevents overflow) - External call isolation (tryCall usage) @@ -44,6 +51,7 @@ - State changes before external calls **Post-Execution Flow**: + - Success: NetworkFeeCollector.collectNetworkFee() (trusted contract) - Failure: Full refund to refundAddress or msg.sender @@ -52,17 +60,21 @@ --- ### Switchboard.processPayload() - Payload Creation -**Files**: + +**Files**: + - `contracts/protocol/switchboard/MessageSwitchboard.sol` (lines 165-238) - `contracts/protocol/switchboard/FastSwitchboard.sol` (lines 146-178) **Why Critical**: + - Creates unique payload IDs - Stores fee information - Validates sibling configuration - Emits events for off-chain watchers **Key Checks**: + - Counter overflow protection (uint64) - Sibling validation completeness - Fee tracking accuracy @@ -74,39 +86,47 @@ --- ### Switchboard.allowPayload() - Verification Gate + **Files**: + - `contracts/protocol/switchboard/MessageSwitchboard.sol` (lines 667-677) - `contracts/protocol/switchboard/FastSwitchboard.sol` (lines 111-122) **Why Critical**: + - Final authorization check before execution - Validates source-target pairing - Checks attestation status - Cannot be bypassed **Key Checks**: + - Source validation logic correctness - Attestation requirement enforcement - No bypass conditions exist --- -### SocketUtils._createDigest() - Parameter Binding +### SocketUtils.\_createDigest() - Parameter Binding + **File**: `contracts/protocol/SocketUtils.sol` (lines 70-100) **Why Critical**: + - Binds all execution parameters to single hash - Used for attestation verification - Prevents parameter manipulation **Key Checks**: + - Length prefix usage for variable fields (payload, source, extraData) - Inclusion of all critical parameters - Proper encoding preventing collisions - Deterministic hashing **Important**: Length prefixes prevent collision attacks where: -- `payload="AAAA", source="BB"` + +- `payload="AAAA", source="BB"` - `payload="AAA", source="ABB"` - Would hash to same value without length prefixes @@ -116,7 +136,8 @@ ### ETH Transfer Locations -#### 1. Socket._execute() → Plug +#### 1. Socket.\_execute() → Plug + ```solidity executionParams.target.tryCall( executionParams.value, // ← Value transferred here @@ -125,18 +146,23 @@ executionParams.target.tryCall( executionParams.payload ) ``` -**Verify**: + +**Verify**: + - Value comes from msg.value - Validated in execute(): `msg.value >= executionParams.value + socketFees` - Isolated execution environment --- -#### 2. Socket._handleSuccessfulExecution() → NetworkFeeCollector +#### 2. Socket.\_handleSuccessfulExecution() → NetworkFeeCollector + ```solidity networkFeeCollector.collectNetworkFee{value: transmissionParams.socketFees}(...) ``` -**Verify**: + +**Verify**: + - Called after execution completes - socketFees portion of msg.value - State updated before external call @@ -144,11 +170,14 @@ networkFeeCollector.collectNetworkFee{value: transmissionParams.socketFees}(...) --- -#### 3. Socket._handleFailedExecution() → Refund Address +#### 3. Socket.\_handleFailedExecution() → Refund Address + ```solidity SafeTransferLib.safeTransferETH(receiver, msg.value) ``` + **Verify**: + - Full msg.value refunded on failure - Correct recipient (refundAddress or msg.sender) - executionStatus set to Reverted first @@ -158,10 +187,13 @@ SafeTransferLib.safeTransferETH(receiver, msg.value) --- #### 4. MessageSwitchboard.refund() → Refund Address + ```solidity SafeTransferLib.safeTransferETH(fees.refundAddress, feesToRefund) ``` + **Verify**: + - ReentrancyGuard applied ✓ - isRefunded flag set before transfer ✓ - nativeFees zeroed before transfer ✓ @@ -172,13 +204,16 @@ SafeTransferLib.safeTransferETH(fees.refundAddress, feesToRefund) --- #### 5. MessageSwitchboard.processPayload() - Fee Storage + ```solidity payloadFees[payloadId] = PayloadFees({ nativeFees: msg.value, ... }) ``` + **Verify**: + - msg.value properly tracked - Sufficient fees checked against minimums - Cannot be decreased except via refund @@ -188,6 +223,7 @@ payloadFees[payloadId] = PayloadFees({ ### Fee Accounting Checks **Verify These Invariants**: + 1. Total ETH in = Total ETH out (no leakage) 2. Fee increases are monotonic (only up, never down) 3. Refunds only happen once per payload @@ -200,28 +236,37 @@ payloadFees[payloadId] = PayloadFees({ ### Socket → Switchboard Calls #### 1. getTransmitter() + **File**: `contracts/protocol/Socket.sol` (line 92) + ```solidity address transmitter = ISwitchboard(switchboardAddress).getTransmitter(...) ``` + **Note**: Returns address(0) if no signature. Switchboard is trusted per system assumptions. --- #### 2. allowPayload() + **File**: `contracts/protocol/Socket.sol` (line 105) + ```solidity bool allowed = ISwitchboard(switchboardAddress).allowPayload(...) ``` + **Critical**: Switchboards are trusted by plugs who choose to connect to them. --- #### 3. processPayload() + **File**: `contracts/protocol/Socket.sol` (line 259) + ```solidity payloadId = ISwitchboard(switchboardAddress).processPayload{value: value_}(...) ``` + **Verify**: Switchboard receives value, creates unique payloadId --- @@ -229,20 +274,27 @@ payloadId = ISwitchboard(switchboardAddress).processPayload{value: value_}(...) ### Socket → Plug Calls #### 1. overrides() + **File**: `contracts/protocol/Socket.sol` (line 256) + ```solidity bytes memory plugOverrides = IPlug(plug_).overrides() ``` + **Note**: View function, safe --- #### 2. Execution Call + **File**: `contracts/protocol/Socket.sol` (line 137) + ```solidity executionParams.target.tryCall(value, gasLimit, maxCopyBytes, payload) ``` + **Security**: + - Reentrancy allowed but safe (CEI pattern followed) - Gas griefing mitigated (gas limit enforced) - Always reverts scenario acceptable (plug's responsibility) @@ -257,6 +309,7 @@ executionParams.target.tryCall(value, gasLimit, maxCopyBytes, payload) ### Watcher Attestation Signatures **MessageSwitchboard.attest()** + ```solidity digest_to_sign = keccak256(abi.encodePacked( toBytes32Format(address(this)), // ← Switchboard address @@ -267,6 +320,7 @@ watcher = _recoverSigner(digest_to_sign, proof) ``` **Protection Against Replay**: + - ✓ Includes contract address (prevents cross-contract replay) - ✓ Includes chainSlug (prevents cross-chain replay) - ✓ chainSlug typically = block.chainid (additional protection) @@ -279,6 +333,7 @@ watcher = _recoverSigner(digest_to_sign, proof) ### Transmitter Signatures **SwitchboardBase.getTransmitter()** + ```solidity digest_to_sign = keccak256(abi.encodePacked( address(socket__), @@ -287,7 +342,8 @@ digest_to_sign = keccak256(abi.encodePacked( transmitter = _recoverSigner(digest_to_sign, transmitterSignature_) ``` -**Note**: +**Note**: + - Transmitter signature is optional (returns address(0) if not provided) - Used for accountability and reputation tracking - Does NOT affect authorization (only attestation matters) @@ -297,26 +353,30 @@ transmitter = _recoverSigner(digest_to_sign, transmitterSignature_) ### Nonce-Based Signatures **Functions Using Nonces**: + 1. `markRefundEligible(payloadId, nonce, signature)` 2. `setMinMsgValueFees(chainSlug, minFees, nonce, signature)` 3. `setMinMsgValueFeesBatch(chainSlugs, minFees, nonce, signature)` **Nonce Management**: + - ✓ Namespace isolation per function type (using function selectors) - ✓ Nonces cannot be replayed within same namespace - ✓ Off-chain uses UUIDv4 (128-bit) for nonce generation - ✓ Collision extremely unlikely **Implementation**: + ```solidity function _validateAndUseNonce(bytes4 selector_, address signer_, uint256 nonce_) internal { - uint256 namespacedNonce = uint256(keccak256(abi.encodePacked(selector_, nonce_))); - if (usedNonces[signer_][namespacedNonce]) revert NonceAlreadyUsed(); - usedNonces[signer_][namespacedNonce] = true; + uint256 namespacedNonce = uint256(keccak256(abi.encodePacked(selector_, nonce_))); + if (usedNonces[signer_][namespacedNonce]) revert NonceAlreadyUsed(); + usedNonces[signer_][namespacedNonce] = true; } ``` **Function Selectors Used**: + - `markRefundEligible`: `this.markRefundEligible.selector` - `setMinMsgValueFees` & `setMinMsgValueFeesBatch`: `this.setMinMsgValueFees.selector` (shared namespace) @@ -325,11 +385,13 @@ function _validateAndUseNonce(bytes4 selector_, address signer_, uint256 nonce_) ### Signature Format All signatures use Ethereum Signed Message format: + ```solidity "\x19Ethereum Signed Message:\n32" + digest ``` **Verify**: + - Consistent usage across all contracts - No raw signature verification (all prefixed) - Using Solady's ECDSA.recover (assumed secure) @@ -339,9 +401,11 @@ All signatures use Ethereum Signed Message format: ## Priority 5: Replay Protection Mechanisms ### 1. Execution Status + **Location**: `Socket.sol` - `executionStatus[bytes32 payloadId]` **Mechanism**: + ```solidity if (executionStatus[payloadId] == ExecutionStatus.Executed) revert PayloadAlreadyExecuted(); @@ -349,6 +413,7 @@ executionStatus[payloadId] = ExecutionStatus.Executed; ``` **Verify**: + - Check happens before any external calls ✓ - Status set before execution ✓ - No way to reset status ✓ @@ -356,15 +421,18 @@ executionStatus[payloadId] = ExecutionStatus.Executed; --- ### 2. Attestation One-Way + **Location**: Both switchboards - `isAttested[bytes32 digest]` **Mechanism**: + ```solidity if (isAttested[digest]) revert AlreadyAttested(); isAttested[digest] = true; ``` **Verify**: + - Cannot un-attest a digest ✓ - Check happens early in attestation flow ✓ @@ -373,9 +441,11 @@ isAttested[digest] = true; --- ### 3. Nonce System + **Location**: `MessageSwitchboard.sol` - `usedNonces[address][uint256]` **Mechanism**: + ```solidity uint256 namespacedNonce = uint256(keccak256(abi.encodePacked(selector_, nonce_))); if (usedNonces[signer][namespacedNonce]) revert NonceAlreadyUsed(); @@ -383,6 +453,7 @@ usedNonces[signer][namespacedNonce] = true; ``` **Verify**: + - Nonce checked before performing action ✓ - No nonce reuse possible ✓ - Namespace isolation prevents cross-function replay ✓ @@ -390,9 +461,11 @@ usedNonces[signer][namespacedNonce] = true; --- ### 4. Payload ID Uniqueness + **Mechanism**: Counter-based with chain/switchboard encoding **Verify**: + - Counters only increment (never decrement) ✓ - Counter overflow handling (uint64) - not a realistic concern ✓ - Payload ID includes source and destination info ✓ @@ -402,15 +475,18 @@ usedNonces[signer][namespacedNonce] = true; ## Priority 6: Gas Handling ### Gas Limit Validation + **Location**: `Socket.sol:130` + ```solidity -if (gasleft() < (executionParams_.gasLimit * gasLimitBuffer) / 100) +if (gasleft() < (executionParams_.gasLimit * gasLimitBuffer) / 100) revert LowGasLimit(); ``` **Type**: gasLimit is uint64 **Overflow Analysis**: + - `uint64.max * 105 / 100` = fits within uint256 ✓ - No overflow risk ✓ - Allows flexibility for different chains (Ethereum: 30M, Mantle: 4B) @@ -420,7 +496,9 @@ if (gasleft() < (executionParams_.gasLimit * gasLimitBuffer) / 100) --- ### Gas Limit Forwarding + **Location**: `Socket.sol:137-142` + ```solidity (success, exceededMaxCopy, returnData) = executionParams.target.tryCall( executionParams.value, @@ -431,6 +509,7 @@ if (gasleft() < (executionParams_.gasLimit * gasLimitBuffer) / 100) ``` **Verify**: + - tryCall properly limits gas ✓ - Doesn't forward more gas than available ✓ - 63/64 rule respected by EVM ✓ @@ -438,7 +517,9 @@ if (gasleft() < (executionParams_.gasLimit * gasLimitBuffer) / 100) --- ### Return Data Limitation + **Location**: `Socket.sol:118` and config + ```solidity maxCopyBytes = 2048 (default) ``` @@ -446,6 +527,7 @@ maxCopyBytes = 2048 (default) **Purpose**: Prevent DOS from excessive return data copying **Verify**: + - Properly limits memory allocation ✓ - exceededMaxCopy flag set correctly ✓ - Events still emitted even when exceeded ✓ @@ -455,11 +537,13 @@ maxCopyBytes = 2048 (default) ## Priority 7: Configuration Management ### Switchboard Registration + **Function**: `SocketConfig.registerSwitchboard()` **Design Decision**: No contract existence check **Rationale**: + - Switchboards are trusted by plugs who choose to connect - Plugs verify switchboard implementation before connecting - Invalid switchboards simply won't work (plug's responsibility) @@ -469,9 +553,11 @@ maxCopyBytes = 2048 (default) --- ### Plug Connection + **Function**: `SocketConfig.connect()` **Transaction Ordering**: + - Switchboard status checked at entry - Status could change in same block (different tx) - Low probability: only when exploit found @@ -486,22 +572,26 @@ maxCopyBytes = 2048 (default) ### Payload Execution **Edge Case 1**: Plug always reverts + - executionStatus set to Reverted ✓ - msg.value refunded ✓ - Cannot retry execution ✓ - **Impact**: Funds returned, no loss **Edge Case 2**: Plug consumes all gas + - tryCall limits gas, execution fails ✓ - Status set to Reverted ✓ - **Verify**: Gas checks prevent complete exhaustion ✓ **Edge Case 3**: Deadline expires during execution + - Deadline checked before execution starts ✓ - Not checked during execution ✓ - **Impact**: Payload could execute slightly after deadline (acceptable) **Edge Case 4**: Multiple transmitters race to execute + - First transaction sets execution status ✓ - Later transactions revert (already executed) ✓ - **Impact**: Wasted gas for losing transmitters (acceptable) @@ -511,16 +601,19 @@ maxCopyBytes = 2048 (default) ### Fee Management **Edge Case 1**: Fees increased after attestation + - Allowed by design ✓ - Doesn't invalidate attestation ✓ - **Impact**: Can incentivize execution of slow payloads ✓ **Edge Case 2**: Refund claimed before execution attempted + - Only possible if watcher marks eligible ✓ - Watcher shouldn't mark if execution possible ✓ - **Impact**: Payload never executes (intentional) **Edge Case 3**: Fee increase causes overflow + - Solidity 0.8+ prevents overflow with revert ✓ - **Impact**: Cannot increase fees beyond max ✓ @@ -529,6 +622,7 @@ maxCopyBytes = 2048 (default) ### Griefing Vectors **Transmitter Griefing**: Malicious plug could make payload look valid (passes simulation) then revert in production + - **Mitigation**: Transmitters blacklist bad plugs - **Market Solution**: Reputation systems - **Impact**: LOW - Market-based solution adequate @@ -538,29 +632,34 @@ maxCopyBytes = 2048 (default) ## Suggested Testing Scenarios ### Reentrancy Tests + 1. Malicious plug calls Socket.sendPayload() during execution ✓ (safe - new payload) 2. Malicious plug calls Socket.execute() during execution ✓ (safe - different payloadId) 3. Refund recipient attempts reentrancy during refund ✓ (protected by ReentrancyGuard) ### Replay Tests + 1. Attempt to execute same payloadId twice ✓ (blocked by executionStatus) 2. Attempt to attest same digest twice ✓ (blocked by isAttested) 3. Attempt to reuse nonce within namespace ✓ (blocked by usedNonces) 4. Attempt to reuse nonce across functions ✓ (namespace isolation prevents) ### Gas Tests + 1. Execute with gasLimit = 0 (should handle gracefully) 2. Execute with gasLimit = type(uint64).max (should not overflow) 3. Execute with minimal gas (just above threshold) 4. Payload that consumes exactly gasLimit ### Value Tests + 1. Execute with msg.value = executionParams.value + socketFees (exact) 2. Execute with msg.value < required (should revert) 3. Execute with msg.value > required (excess stays in Socket) 4. Increase fees with msg.value causing nativeFees overflow (should revert) ### Signature Tests + 1. Invalid signature format 2. Signature from non-watcher address 3. Nonce reuse within namespace (should revert) @@ -571,23 +670,27 @@ maxCopyBytes = 2048 (default) ## Security Properties to Verify ### Correctness Properties + - ✓ Every executed payload was properly attested - ✓ Every executed payload came from authorized source - ✓ Every payload executes at most once - ✓ Execution respects all specified parameters (gas, value, deadline) ### Safety Properties + - ✓ User funds never lost or stolen - ✓ Fees properly accounted for - ✓ Refunds only issued for unexecuted payloads - ✓ No unauthorized state modifications ### Liveness Properties + - ✓ Valid payloads can eventually execute (if attested) - ✓ Plugs can always disconnect - ✓ Governance can always pause in emergency ### Economic Properties + - ✓ Transmitters incentivized to deliver payloads - ✓ Griefing attacks mitigated by market mechanisms - ✓ Fee increases benefit protocol/transmitters @@ -607,6 +710,7 @@ maxCopyBytes = 2048 (default) ## Summary The Socket Protocol follows security best practices with: + - ✅ CEI (Checks-Effects-Interactions) pattern throughout - ✅ Replay protection at multiple levels - ✅ Namespace-isolated nonces @@ -615,6 +719,7 @@ The Socket Protocol follows security best practices with: - ✅ One-time execution with clear finality Main audit focus should be on: + 1. Value flow tracking 2. Signature verification completeness 3. Edge case handling diff --git a/auditor-docs/AUDIT_PREP_SUMMARY.md b/auditor-docs/AUDIT_PREP_SUMMARY.md index c13205f8..25d79e6a 100644 --- a/auditor-docs/AUDIT_PREP_SUMMARY.md +++ b/auditor-docs/AUDIT_PREP_SUMMARY.md @@ -9,8 +9,9 @@ This document summarizes the pre-audit review conducted on Socket Protocol's cor ## Pre-Audit Review Results ### Contracts Reviewed + - ✅ Socket.sol (286 lines) -- ✅ SocketUtils.sol (210 lines) +- ✅ SocketUtils.sol (210 lines) - ✅ SocketConfig.sol (203 lines) - ✅ MessageSwitchboard.sol (763 lines) - ✅ FastSwitchboard.sol (244 lines) @@ -27,21 +28,25 @@ This document summarizes the pre-audit review conducted on Socket Protocol's cor ### ✅ Design Patterns Validated **1. Checks-Effects-Interactions (CEI) Pattern** + - **Status**: ✅ Properly implemented throughout -- **Key Functions**: execute(), _execute(), processPayload() +- **Key Functions**: execute(), \_execute(), processPayload() - **Result**: Reentrancy protection without ReentrancyGuard overhead **2. Replay Protection** + - **Status**: ✅ Multi-layer protection in place - **Mechanisms**: executionStatus, isAttested, nonce system - **Result**: No double-execution or replay possible **3. Gas Limit Handling** + - **Status**: ✅ Appropriate for multi-chain deployment - **Type**: uint64 (prevents overflow, supports high-throughput chains) - **Result**: Flexible without hardcoded limits **4. Signature Verification** + - **Status**: ✅ Includes necessary anti-replay components - **Protection**: address(this), chainSlug (= block.chainid typically) - **Result**: Cross-chain replay prevented @@ -51,25 +56,24 @@ This document summarizes the pre-audit review conducted on Socket Protocol's cor ### 🔧 Improvements Implemented **1. Nonce Namespace Isolation** ✅ IMPLEMENTED + - **Issue**: Single nonce mapping shared across different function types - **Solution**: Function selector-based namespace isolation - **Implementation**: `_validateAndUseNonce(bytes4 selector, address signer, uint256 nonce)` - **Benefit**: Prevents cross-function nonce exhaustion, cleaner off-chain management **Code Added**: + ```solidity -function _validateAndUseNonce( - bytes4 selector_, - address signer_, - uint256 nonce_ -) internal { - uint256 namespacedNonce = uint256(keccak256(abi.encodePacked(selector_, nonce_))); - if (usedNonces[signer_][namespacedNonce]) revert NonceAlreadyUsed(); - usedNonces[signer_][namespacedNonce] = true; +function _validateAndUseNonce(bytes4 selector_, address signer_, uint256 nonce_) internal { + uint256 namespacedNonce = uint256(keccak256(abi.encodePacked(selector_, nonce_))); + if (usedNonces[signer_][namespacedNonce]) revert NonceAlreadyUsed(); + usedNonces[signer_][namespacedNonce] = true; } ``` **Rationale for Function Selectors**: + - Deterministic encoding (same on-chain and off-chain) - Gas efficient (bytes4 vs string) - Type-safe (compiler verification) @@ -81,42 +85,52 @@ function _validateAndUseNonce( The following items were initially flagged but determined to be non-issues after analysis: **1. Reentrancy in Execution Flow** + - **Reason**: CEI pattern properly followed, different payloadIds are independent - **Verdict**: Safe by design **2. Gas Limit Overflow** -- **Reason**: uint64 * 105 / 100 fits within uint256, no overflow + +- **Reason**: uint64 \* 105 / 100 fits within uint256, no overflow - **Verdict**: Not an issue **3. Deadline Validation (Max Limit)** + - **Reason**: Application-layer responsibility, different apps need different deadlines - **Verdict**: Intentional design decision **4. msg.value Full Refund on Failure** + - **Reason**: Transmitters should simulate; external reimbursement exists - **Verdict**: Acceptable trade-off **5. increaseFeesForPayload Validation** + - **Reason**: Multi-layer validation (Socket + Switchboard + off-chain) - **Verdict**: Properly secured **6. Counter Overflow Risk** + - **Reason**: uint64 = 18 quintillion, not realistically exploitable - **Verdict**: Acceptable **7. Double Attestation Race** + - **Reason**: Transactions execute serially, not concurrently - **Verdict**: Not possible **8. Transaction Ordering "Race"** + - **Reason**: Block-level ordering, not race condition; low probability, low impact - **Verdict**: Acceptable **9. Cross-Contract Reentrancy** + - **Reason**: CEI pattern + unique payloadIds per call - **Verdict**: Safe by design **10. Signature Replay Across Chains** + - **Reason**: chainSlug = block.chainid (typically), unique per chain - **Verdict**: Properly protected @@ -127,22 +141,27 @@ The following items were initially flagged but determined to be non-issues after ### Trust Model 1. **Switchboards are Trusted by Plugs** + - Anyone can register, but plugs choose whom to trust - Plug's responsibility to verify switchboard implementation 2. **NetworkFeeCollector is Trusted by Socket** + - Set by governance - Called after successful execution for fee collection 3. **Target Plugs are Trusted by Source Plugs** + - Source specifies destination plug - Cross-chain trust established at application level 4. **simulate() is Off-Chain Only** + - Gated by OFF_CHAIN_CALLER (0xDEAD) - Used for gas estimation by transmitters 5. **Watchers Act Honestly** + - At least one honest watcher per payload - Verify source chain correctly - Respect finality before attesting @@ -157,6 +176,7 @@ The following items were initially flagged but determined to be non-issues after ## Security Properties Verified ### Core Invariants + - ✓ Each payload executes at most once - ✓ Execution status transitions are one-way - ✓ Digests are immutable once stored @@ -166,6 +186,7 @@ The following items were initially flagged but determined to be non-issues after - ✓ Source validation prevents unauthorized execution ### Protection Mechanisms + - ✓ CEI pattern throughout execution flow - ✓ Replay protection via executionStatus mapping - ✓ Nonce management with namespace isolation @@ -180,27 +201,32 @@ The following items were initially flagged but determined to be non-issues after ### High-Priority Test Scenarios **1. Reentrancy Tests** + - Malicious plug calls sendPayload() during execution (should create new payload) - Malicious plug calls execute() with different payloadId (should succeed) - Refund recipient attempts reentrancy (should be blocked by ReentrancyGuard) **2. Replay Protection** + - Attempt double execution of same payloadId (should revert) - Attempt double attestation of same digest (should revert) - Reuse nonce within namespace (should revert) - Reuse nonce across namespaces (should succeed with isolation) **3. Gas Limit Edge Cases** + - gasLimit = 0 (should handle) - gasLimit = type(uint64).max (should not overflow) - gasLimit exceeds block limit (should naturally fail) **4. Value Flow** + - Exact msg.value (should succeed) - Insufficient msg.value (should revert) - Excess msg.value (stays in contract) **5. Fee Management** + - Increase fees causing overflow (should revert) - Refund double-claim (should revert) - Unauthorized fee increase (should revert) @@ -210,6 +236,7 @@ The following items were initially flagged but determined to be non-issues after ## Documentation Status ### Files Created/Updated + - ✅ SYSTEM_OVERVIEW.md - Updated with assumptions - ✅ CONTRACTS_REFERENCE.md - Comprehensive reference - ✅ MESSAGE_FLOW.md - Detailed flow documentation @@ -228,30 +255,36 @@ The following items were initially flagged but determined to be non-issues after ### File: MessageSwitchboard.sol **Change 1: Added Nonce Validation Utility** + - Location: ~Line 354 - Added: `_validateAndUseNonce()` internal function - Purpose: DRY principle, namespace isolation **Change 2: Updated markRefundEligible()** + - Location: ~Line 459 - Changed: From inline nonce check to utility function call - Namespace: `this.markRefundEligible.selector` **Change 3: Updated setMinMsgValueFees()** + - Location: ~Line 500 - Changed: From inline nonce check to utility function call - Namespace: `this.setMinMsgValueFees.selector` **Change 4: Updated setMinMsgValueFeesBatch()** + - Location: ~Line 533 - Changed: From inline nonce check to utility function call - Namespace: `this.setMinMsgValueFees.selector` (shares namespace) **Change 5: Added Missing Event** + - Added: `event DefaultDeadlineSet(uint256 defaultDeadline);` - Purpose: Complete event coverage -**Net Result**: +**Net Result**: + - Reduced code duplication - Improved maintainability - Added namespace isolation @@ -264,19 +297,23 @@ The following items were initially flagged but determined to be non-issues after ### For Auditors to Evaluate 1. **Gas Limit Flexibility** + - No hardcoded max supports diverse chains - Could extreme values cause unforeseen issues? 2. **Switchboard Trust Model** + - Is plug-level trust verification sufficient? - Should protocol add reputation mechanisms? 3. **Fee Economic Sustainability** + - External transmitter reimbursement model - Market-based griefing protection - Are these adequate long-term? 4. **Upgrade Strategy** + - Currently no upgrade mechanism - Security issues require redeployment - Is this acceptable for critical infrastructure? @@ -307,12 +344,14 @@ The following items were initially flagged but determined to be non-issues after ## Summary Socket Protocol demonstrates: + - ✅ Strong security patterns (CEI, replay protection) - ✅ Clear trust boundaries - ✅ Appropriate trade-offs for cross-chain infrastructure - ✅ Well-documented assumptions and design decisions The protocol is **audit-ready** with: + - Solid architectural foundation - Security-first design - Clear documentation for auditors @@ -326,4 +365,3 @@ The protocol is **audit-ready** with: **Protocol Version**: [Version] **Pre-Audit Review**: Complete ✅ **Status**: Ready for formal audit - diff --git a/auditor-docs/CONTRACTS_REFERENCE.md b/auditor-docs/CONTRACTS_REFERENCE.md index 6fb79fd5..205d96eb 100644 --- a/auditor-docs/CONTRACTS_REFERENCE.md +++ b/auditor-docs/CONTRACTS_REFERENCE.md @@ -2,16 +2,16 @@ ## Contract Inventory -| Contract | LOC | Purpose | Inheritance | Key External Calls | -|----------|-----|---------|-------------|-------------------| -| Socket.sol | 286 | Main execution & routing | SocketUtils | ISwitchboard, IPlug, INetworkFeeCollector | -| SocketUtils.sol | 210 | Utilities & verification | SocketConfig | ISwitchboard | -| SocketConfig.sol | 203 | Configuration management | AccessControl, Pausable | ISwitchboard | -| MessageSwitchboard.sol | 740 | Message-based verification | SwitchboardBase, ReentrancyGuard | ISocket | -| FastSwitchboard.sol | 244 | Fast EVMX verification | SwitchboardBase | ISocket | -| SwitchboardBase.sol | 115 | Base switchboard logic | ISwitchboard, AccessControl | ISocket | -| IdUtils.sol | 75 | Payload ID utilities | None | None (pure functions) | -| OverrideParamsLib.sol | 148 | Parameter builder | None | None (pure functions) | +| Contract | LOC | Purpose | Inheritance | Key External Calls | +| ---------------------- | --- | -------------------------- | -------------------------------- | ----------------------------------------- | +| Socket.sol | 286 | Main execution & routing | SocketUtils | ISwitchboard, IPlug, INetworkFeeCollector | +| SocketUtils.sol | 210 | Utilities & verification | SocketConfig | ISwitchboard | +| SocketConfig.sol | 203 | Configuration management | AccessControl, Pausable | ISwitchboard | +| MessageSwitchboard.sol | 740 | Message-based verification | SwitchboardBase, ReentrancyGuard | ISocket | +| FastSwitchboard.sol | 244 | Fast EVMX verification | SwitchboardBase | ISocket | +| SwitchboardBase.sol | 115 | Base switchboard logic | ISwitchboard, AccessControl | ISocket | +| IdUtils.sol | 75 | Payload ID utilities | None | None (pure functions) | +| OverrideParamsLib.sol | 148 | Parameter builder | None | None (pure functions) | --- @@ -22,27 +22,28 @@ **Purpose**: Core contract for cross-chain payload execution and transmission. Main entry point for both inbound (execute) and outbound (sendPayload) operations. **Key State Variables**: + - `executionStatus[bytes32]`: Tracks whether payload has been executed/reverted - `payloadIdToDigest[bytes32]`: Stores digest for each payload ID **Critical Functions**: + - `execute()`: Executes incoming payload from remote chain - Validates deadline, call type, plug connection, msg.value - Verifies digest through switchboard - Prevents replay attacks via execution status - Calls target plug with payload - - `sendPayload()`: Sends payload to remote chain - Verifies plug is connected - Gets plug overrides configuration - Delegates to switchboard for processing - - `fallback()`: Alternative entry point for sendPayload - Double-encodes return value for raw calldata compatibility **Access Control**: Inherits from SocketConfig (RESCUE_ROLE, PAUSER_ROLE, UNPAUSER_ROLE) **External Dependencies**: + - Calls switchboard for verification (`allowPayload`, `getTransmitter`) - Calls plug for overrides (`IPlug.overrides()`) - Calls network fee collector for fee collection @@ -54,18 +55,18 @@ **Purpose**: Provides utility functions for digest creation, simulation, and verification helpers. **Key State Variables**: + - `OFF_CHAIN_CALLER`: Special address (0xDEAD) for off-chain simulations - `chainSlug`: Immutable chain identifier **Critical Functions**: + - `_createDigest()`: Creates deterministic hash of execution parameters - Uses length prefixes for variable-length fields (payload, source, extraData) - Includes transmitter, payloadId, deadline, gasLimit, value, target - - `simulate()`: Off-chain only - tests payload execution for gas estimation - Only callable by OFF_CHAIN_CALLER - Returns success/failure and return data - - `_verifyPlugSwitchboard()`: Validates plug connection and switchboard status - `_verifyPayloadId()`: Validates payload routing information - `increaseFeesForPayload()`: Allows plugs to top up fees for pending payloads @@ -79,6 +80,7 @@ **Purpose**: Manages socket configuration including switchboard registration, plug connections, and system parameters. **Key State Variables**: + - `switchboardIdCounter`: Incrementing counter for switchboard IDs - `switchboardStatus[uint32]`: Tracks REGISTERED/DISABLED status - `plugSwitchboardIds[address]`: Maps plugs to their connected switchboards @@ -87,21 +89,21 @@ - `maxCopyBytes`: Maximum bytes to copy from return data **Critical Functions**: + - `registerSwitchboard()`: Assigns unique ID to switchboard - Called by switchboard contract - Sets status to REGISTERED - Increments counter - - `connect()`: Connects plug to switchboard - Validates switchboard is registered - Stores plug-switchboard mapping - Forwards config to switchboard if provided - - `disconnect()`: Removes plug connection - `disableSwitchboard()`: Governance can disable switchboards - `enableSwitchboard()`: Governance can re-enable switchboards **Access Control**: + - GOVERNANCE_ROLE: Enable switchboards, set parameters - SWITCHBOARD_DISABLER_ROLE: Disable switchboards @@ -112,6 +114,7 @@ **Purpose**: Full-featured switchboard with watcher attestations, fee management (native + sponsored), refunds, and cross-chain routing. **Key State Variables**: + - `payloadCounter`: Incrementing counter for payload IDs - `isAttested[bytes32]`: Tracks attested digests - `siblingSockets[uint32]`: Destination chain socket addresses @@ -124,42 +127,39 @@ - `minMsgValueFees[uint32]`: Minimum fees per destination chain **Critical Functions**: + - `processPayload()`: Handles outbound payload requests - Decodes overrides (version 1: native, version 2: sponsored) - Validates sibling configuration exists - Creates digest and payload ID - Tracks fees for refund eligibility - Emits MessageOutbound event - - `attest()`: Watchers attest to payloads - Verifies watcher signature - Checks watcher has WATCHER_ROLE - Marks digest as attested - - `allowPayload()`: Verifies payload can execute - Checks source plug matches expected sibling - Checks digest is attested - - `markRefundEligible()`: Watchers mark payloads for refund - Validates watcher signature with nonce - Prevents nonce replay - - `refund()`: Claims refund for eligible payloads - Protected by ReentrancyGuard - Transfers native fees back to refund address - - `increaseFeesForPayload()`: Top up fees - Supports both native and sponsored flows - - `setMinMsgValueFees()`: Updates minimum fees - Requires FEE_UPDATER_ROLE signature with nonce **Access Control**: + - WATCHER_ROLE: Attest payloads, mark refunds - FEE_UPDATER_ROLE: Update fee parameters - onlySocket: Called by Socket for payload processing **Fee Flows**: + - Native: User pays ETH when sending payload - Sponsored: Sponsor pre-approves plugs, maxFees tracked off-chain @@ -170,6 +170,7 @@ **Purpose**: Simplified switchboard for fast finality using EVMX chain verification. **Key State Variables**: + - `evmxChainSlug`: EVMX chain identifier for verification - `watcherId`: Watcher ID for EVMX verification - `payloadCounter`: Incrementing counter @@ -179,28 +180,28 @@ - `defaultDeadline`: Default execution deadline (1 day) **Critical Functions**: + - `processPayload()`: Creates payload with EVMX verification - Validates EVMX config is set - Decodes deadline from overrides (or uses default) - Creates payload ID with: source=(chainSlug, switchboardId), verification=(evmxChainSlug, watcherId) - Emits PayloadRequested - - `attest()`: Watchers attest digest - Similar to MessageSwitchboard but simpler - Verifies watcher signature - - `allowPayload()`: Checks attestation and source - Validates app gateway ID matches - Returns attestation status - - `updatePlugConfig()`: Sets plug's app gateway ID - `setEvmxConfig()`: Owner configures EVMX chain and watcher **Access Control**: + - WATCHER_ROLE: Attest payloads - onlyOwner: Configure EVMX, set defaults **Differences from MessageSwitchboard**: + - No fee management (fees handled on EVMX) - Simpler attestation model - App gateway ID based routing vs. sibling plug mapping @@ -212,20 +213,20 @@ **Purpose**: Abstract base providing common functionality for all switchboards. **Key State Variables**: + - `socket__`: Immutable reference to Socket contract - `chainSlug`: Chain identifier - `switchboardId`: Assigned by Socket during registration - `revertingPayloadIds[bytes32]`: Marks payloads as known reverting **Critical Functions**: + - `registerSwitchboard()`: Calls Socket to get unique ID - Only callable by owner - Must be called after deployment - - `getTransmitter()`: Recovers transmitter from signature - Returns address(0) if no signature provided - Uses Ethereum signed message format - - `_recoverSigner()`: Internal ECDSA recovery - Adds "\x19Ethereum Signed Message:\n32" prefix - Uses Solady's ECDSA library @@ -243,10 +244,10 @@ **No State Variables** (all pure functions) **Functions**: + - `createPayloadId()`: Encodes components into bytes32 - Takes: sourceChainSlug, sourceId, verificationChainSlug, verificationId, pointer - Bit layout: [Source: 64][Verification: 64][Pointer: 64][Reserved: 64] - - `decodePayloadId()`: Extracts all components from bytes32 - `getVerificationInfo()`: Extracts verification chain and ID - `getSourceInfo()`: Extracts source chain and ID @@ -262,6 +263,7 @@ **No State Variables** (all pure functions) **Functions**: + - `clear()`: Creates new OverrideParams with defaults - `setRead()`, `setParallel()`, `setWriteFinality()`: Set flags - `setGasLimit()`, `setValue()`, `setMaxFees()`: Set numeric values @@ -275,6 +277,7 @@ ## Contract Interactions ### Execution Flow (Inbound) + ``` Transmitter → Socket.execute() ├─> SocketUtils._verifyPlugSwitchboard() @@ -288,6 +291,7 @@ Transmitter → Socket.execute() ``` ### Sending Flow (Outbound) + ``` Plug → Socket.sendPayload() ├─> SocketUtils._verifyPlugSwitchboard() @@ -297,6 +301,7 @@ Plug → Socket.sendPayload() ``` ### Registration Flow + ``` Switchboard → Socket.registerSwitchboard() └─> Assign ID, set status REGISTERED @@ -310,46 +315,49 @@ Plug → Socket.connect(switchboardId, config) ## Key Data Structures ### ExecutionParams + ```solidity struct ExecutionParams { - bytes4 callType; // WRITE, READ, or SCHEDULE - uint256 deadline; // Execution deadline timestamp - uint256 gasLimit; // Gas limit for execution - address target; // Target plug address - uint256 value; // Native value to send - bytes32 payloadId; // Unique payload identifier - bytes32 prevBatchDigestHash; // For batch processing - bytes source; // Encoded source info - bytes payload; // Call data - bytes extraData; // Additional data + bytes4 callType; // WRITE, READ, or SCHEDULE + uint256 deadline; // Execution deadline timestamp + uint256 gasLimit; // Gas limit for execution + address target; // Target plug address + uint256 value; // Native value to send + bytes32 payloadId; // Unique payload identifier + bytes32 prevBatchDigestHash; // For batch processing + bytes source; // Encoded source info + bytes payload; // Call data + bytes extraData; // Additional data } ``` ### TransmissionParams + ```solidity struct TransmissionParams { - uint256 socketFees; // Fees for Socket/transmitter - address refundAddress; // Where to refund on failure - bytes extraData; // Additional parameters - bytes transmitterProof; // Transmitter signature + uint256 socketFees; // Fees for Socket/transmitter + address refundAddress; // Where to refund on failure + bytes extraData; // Additional parameters + bytes transmitterProof; // Transmitter signature } ``` ### DigestParams + ```solidity struct DigestParams { - bytes32 socket; // Destination socket address - bytes32 transmitter; // Transmitter address - bytes32 payloadId; // Unique identifier - uint256 deadline; // Execution deadline - bytes4 callType; // Call type - uint256 gasLimit; // Gas limit - uint256 value; // Native value - bytes32 target; // Target address - bytes32 prevBatchDigestHash; - bytes payload; // Payload data - bytes source; // Source information - bytes extraData; // Extra data + bytes32 socket; // Destination socket address + bytes32 transmitter; // Transmitter address + bytes32 payloadId; // Unique identifier + uint256 deadline; // Execution deadline + bytes4 callType; // Call type + uint256 gasLimit; // Gas limit + uint256 value; // Native value + bytes32 target; // Target address + bytes32 prevBatchDigestHash; + bytes payload; // Payload data + bytes source; // Source information + bytes extraData; // Extra data } ``` @@ -357,16 +365,16 @@ struct DigestParams { ## Access Control Roles -| Role | Purpose | Holders | -|------|---------|---------| -| Owner | Full admin control | Deployer initially | -| GOVERNANCE_ROLE | Enable switchboards, set parameters | Multi-sig/DAO | -| SWITCHBOARD_DISABLER_ROLE | Emergency disable switchboards | Security team | -| RESCUE_ROLE | Recover stuck funds | Governance | -| PAUSER_ROLE | Pause socket operations | Emergency responders | -| UNPAUSER_ROLE | Unpause socket operations | Governance | -| WATCHER_ROLE | Attest payloads | Off-chain watcher nodes | -| FEE_UPDATER_ROLE | Update fee parameters | Fee oracle | +| Role | Purpose | Holders | +| ------------------------- | ----------------------------------- | ----------------------- | +| Owner | Full admin control | Deployer initially | +| GOVERNANCE_ROLE | Enable switchboards, set parameters | Multi-sig/DAO | +| SWITCHBOARD_DISABLER_ROLE | Emergency disable switchboards | Security team | +| RESCUE_ROLE | Recover stuck funds | Governance | +| PAUSER_ROLE | Pause socket operations | Emergency responders | +| UNPAUSER_ROLE | Unpause socket operations | Governance | +| WATCHER_ROLE | Attest payloads | Off-chain watcher nodes | +| FEE_UPDATER_ROLE | Update fee parameters | Fee oracle | --- @@ -374,16 +382,15 @@ struct DigestParams { ```solidity // Call Types -bytes4 constant READ = bytes4(keccak256("READ")); -bytes4 constant WRITE = bytes4(keccak256("WRITE")); -bytes4 constant SCHEDULE = bytes4(keccak256("SCHEDULE")); +bytes4 constant READ = bytes4(keccak256('READ')); +bytes4 constant WRITE = bytes4(keccak256('WRITE')); +bytes4 constant SCHEDULE = bytes4(keccak256('SCHEDULE')); // Switchboard Types -bytes32 constant FAST = keccak256("FAST"); -bytes32 constant CCTP = keccak256("CCTP"); +bytes32 constant FAST = keccak256('FAST'); +bytes32 constant CCTP = keccak256('CCTP'); // Limits uint256 constant PAYLOAD_SIZE_LIMIT = 24_500; uint16 constant MAX_COPY_BYTES = 2048; ``` - diff --git a/auditor-docs/FAQ.md b/auditor-docs/FAQ.md index 6f2a1255..9ac79356 100644 --- a/auditor-docs/FAQ.md +++ b/auditor-docs/FAQ.md @@ -5,34 +5,40 @@ ### Core Assumptions **A1: Switchboards are trusted by Plugs/Apps** + - Anyone can register as a switchboard on Socket - Plugs only connect to switchboards they have verified and trust - Invalid or malicious switchboards only affect plugs that choose to connect to them - Users must perform due diligence before connecting **A2: NetworkFeeCollector is trusted by Socket** + - Socket calls networkFeeCollector.collectNetworkFee() after successful execution - No reentrancy concerns as the collector is a trusted contract - Governance sets the networkFeeCollector address **A3: Target Plugs are trusted by Source Plugs** + - Source plugs specify and trust their sibling plugs on destination chains - Invalid target plug configurations only affect the plug that set them - Cross-chain trust is established at plug level, not protocol level **A4: simulate() function is for off-chain use only** + - Gated by OFF_CHAIN_CALLER address (0xDEAD) - Only used by off-chain services for gas estimation and revert checking - Not accessible on mainnet (msg.sender can never be 0xDEAD in normal operation) - Results used by transmitters to avoid failed transactions **A5: Watchers act honestly** + - At least one honest watcher per payload is assumed - Watchers verify source chain state correctly before attesting - Watchers respect finality periods before attesting - Compromised watcher can DOS (refuse to attest) but not forge invalid payloads **A6: Transmitters are rational economic actors** + - Should call simulate() before sending transactions - External reimbursement mechanisms exist for failed deliveries - May blacklist/whitelist plugs based on historical behavior @@ -79,6 +85,7 @@ See `PAYLOAD_ID_ARCHITECTURE.md` for detailed explanation. - **Gas Efficiency**: No need to track retry counts or conditions If a payload fails due to temporary conditions, the application layer can: + - Send a new payload with updated parameters - Use the refund mechanism (MessageSwitchboard) - Build retry logic in the plug contract itself @@ -87,9 +94,10 @@ If a payload fails due to temporary conditions, the application layer can: ### Q4: What's the difference between FastSwitchboard and MessageSwitchboard? -**Answer**: +**Answer**: **FastSwitchboard**: + - Optimized for speed via EVMX verification - Simpler fee model (fees managed on EVMX) - App gateway ID-based routing @@ -97,6 +105,7 @@ If a payload fails due to temporary conditions, the application layer can: - Best for: High-throughput, fast finality needs **MessageSwitchboard**: + - Full-featured with native and sponsored fees - Complex fee management with refunds - Sibling plug mapping for routing @@ -132,6 +141,7 @@ This maintains compatibility with raw calls while providing proper ABI-decodable **Answer**: Reentrancy is allowed but safe due to the Checks-Effects-Interactions (CEI) pattern. **During Execution**: + ```solidity // State updated FIRST executionStatus[payloadId] = Executed; @@ -147,11 +157,13 @@ if (success && networkFeeCollector != address(0)) { ``` **If Plug Reenters**: + - Calls `execute()` with different payload → New unique payloadId, safe ✓ - Calls `sendPayload()` → Creates new unique payloadId, safe ✓ - Calls `execute()` with same payload → Reverts (PayloadAlreadyExecuted) ✓ **During Refund**: + - Protected by Solady's ReentrancyGuard ✓ - State updated before transfer ✓ @@ -164,16 +176,19 @@ if (success && networkFeeCollector != address(0)) { **Answer**: Impact depends on the switchboard type: **FastSwitchboard**: + - Single compromised watcher can attest malicious payloads - Relies on EVMX chain consensus (multiple validators) - System security = EVMX security **MessageSwitchboard**: + - Can configure multiple watchers (M-of-N threshold) - Single compromised watcher cannot authorize alone - System security depends on watcher set size and threshold **Mitigation Strategies**: + - Use multiple independent watcher nodes - Implement watcher rotation - Monitor watcher behavior off-chain @@ -186,12 +201,14 @@ if (success && networkFeeCollector != address(0)) { **Answer**: No, for several reasons: **What Governance CAN do**: + - Pause the contract (prevents new operations) - Disable switchboards (prevents new connections) - Change network fee collector - Update gas/copy byte limits **What Governance CANNOT do**: + - Modify past execution status - Change payloadIdToDigest mappings - Execute payloads without valid attestation @@ -199,6 +216,7 @@ if (success && networkFeeCollector != address(0)) { - Cancel attested payloads User funds are protected by: + - Immutable execution logic - Cryptographic attestation requirements - Replay protection @@ -213,26 +231,31 @@ User funds are protected by: **Answer**: Multiple layers of protection: **Isolation**: + - External call via tryCall with gas limit - Return data limited to maxCopyBytes - Value transfer limited to executionParams.value **State Protection**: + - Execution status set BEFORE plug call - Digest stored BEFORE plug call - Reentrancy guard (recommended) **Economic Disincentives**: + - Malicious behavior only affects the malicious plug - Cannot impact other plugs' payloads - Reverting payloads lose fees (fail to execute) **What Malicious Plug Can Do**: + - Revert its own executions - Consume all provided gas - Attempt reentrancy (should fail) **What Malicious Plug Cannot Do**: + - Execute payloads multiple times - Access other plugs' funds - Forge attestations @@ -245,6 +268,7 @@ User funds are protected by: **Answer**: Multiple mechanisms: **In Signature Digest**: + ```solidity digest = keccak256(abi.encodePacked( toBytes32Format(address(this)), // Contract address @@ -254,6 +278,7 @@ digest = keccak256(abi.encodePacked( ``` **Protection Layers**: + 1. **Contract Address**: Different addresses on different chains 2. **Chain Slug**: Explicit chain identifier in signature 3. **Payload ID**: Includes source and destination chain info @@ -267,21 +292,25 @@ digest = keccak256(abi.encodePacked( ### Q10: What happens if a payload deadline passes before execution? -**Answer**: +**Answer**: **Before Execution Starts**: + ```solidity if (executionParams_.deadline < block.timestamp) revert DeadlinePassed(); ``` + - Payload cannot be executed - Reverts immediately - Funds not lost (not yet transferred) **During Execution**: + - Deadline not checked during plug execution - Payload could finish slightly after deadline **After Deadline**: + - Payload remains unexecutable - In MessageSwitchboard: Eligible for refund (watcher must mark) - In FastSwitchboard: Fees not refunded (managed on EVMX) @@ -295,17 +324,20 @@ if (executionParams_.deadline < block.timestamp) revert DeadlinePassed(); **Answer**: No, by design: **Current Behavior**: + - Payloads can be executed in any order - First transmitter to call execute() wins - `prevBatchDigestHash` exists in params but not enforced **Why Not Enforced**: + - Cross-chain messaging is inherently async - Different chain finality times - Transmitter competition for fees - Simpler implementation **Application-Level Solutions**: + - Plugs should handle out-of-order messages - Use nonces/sequence numbers in payload data - Build state machines that accept messages in any order @@ -318,23 +350,28 @@ if (executionParams_.deadline < block.timestamp) revert DeadlinePassed(); **Answer**: Two-step process: **Step 1: Mark Eligible** + ```solidity messageSwitchboard.markRefundEligible(payloadId, nonce, signature) ``` + - Requires watcher signature - Watcher verifies payload won't execute (e.g., deadline passed) - Sets `isRefundEligible = true` **Step 2: Claim Refund** + ```solidity messageSwitchboard.refund(payloadId) ``` + - Anyone can call (if eligible) - Protected by ReentrancyGuard - Transfers nativeFees to refundAddress - Sets `isRefunded = true` **Conditions for Eligibility**: + - Payload has not executed - Deadline passed or other non-executable condition - Watcher has attested to eligibility @@ -348,17 +385,20 @@ messageSwitchboard.refund(payloadId) **Answer**: Optional accountability mechanism: **If Provided**: + - Signature over (socket address + payloadId) - Proves which transmitter delivered payload - Enables reputation systems - Allows dispute resolution **If Not Provided** (empty bytes): + - Returns address(0) - Execution still works - Anonymous delivery **Use Cases**: + - Track transmitter performance - Reward reliable transmitters - Slash misbehaving transmitters (off-chain) @@ -373,12 +413,14 @@ messageSwitchboard.refund(payloadId) **Answer**: To prevent overflow issues while maintaining flexibility: **With uint64**: + - Max value: 18,446,744,073,709,551,616 (18 quintillion) - Calculation: `uint64.max * 105 / 100` fits within uint256 ✓ - Supports high-throughput chains (Ethereum: 30M, Mantle: 4B for ERC20) - Prevents type(uint256).max attacks **Why No Hardcoded Max**: + - Different chains have vastly different gas models - Future chains may have even higher limits - Natural failure if insufficient gas provided @@ -393,11 +435,13 @@ messageSwitchboard.refund(payloadId) **Answer**: Not concurrent races, but transaction ordering matters. **Concurrent Execution**: ❌ Impossible + - Transactions execute serially within a block - No parallel thread execution - State changes are atomic per transaction **Transaction Ordering**: ✓ Possible + ``` Block N contains: Tx1: plug.connect(switchboardId) @@ -405,6 +449,7 @@ Block N contains: ``` **Execution Order**: + - If Tx1 first: plug connects, then switchboard disabled (plug can disconnect) - If Tx2 first: switchboard disabled, plug connection fails @@ -420,7 +465,7 @@ Block N contains: ```solidity // User specifies gasLimit for plug execution -executionParams.gasLimit = 200_000; +executionParams.gasLimit = 200_000; // Socket needs extra gas for its own operations: // - Verification logic @@ -434,6 +479,7 @@ requiredGas = (200_000 * 105) / 100 = 210_000 **Default Buffer**: 105 (5% overhead) **Why Needed**: + - Socket operations consume gas before/after plug call - Prevents "out of gas" errors in Socket logic - Ensures clean error handling @@ -449,6 +495,7 @@ requiredGas = (200_000 * 105) / 100 = 210_000 **Answer**: Depends on switchboard type: **MessageSwitchboard (Native Fees)**: + ``` User pays: msg.value ├─ executionParams.value → Plug @@ -457,6 +504,7 @@ User pays: msg.value ``` **MessageSwitchboard (Sponsored)**: + ``` User pays: 0 ETH (msg.value = 0) Sponsor: Pre-approved plug, maxFees tracked off-chain @@ -464,6 +512,7 @@ Fees: Managed by off-chain system, charged to sponsor ``` **FastSwitchboard**: + ``` Fees: Managed entirely on EVMX chain Socket/FastSwitchboard: No fee handling @@ -476,17 +525,20 @@ Socket/FastSwitchboard: No fee handling **Answer**: Yes, via `increaseFeesForPayload()`: **Purpose**: + - Incentivize slow payloads - Increase priority - Adjust for changing gas prices **Restrictions**: + - Only the source plug can increase fees - Can only increase, not decrease - Native fees: Add more ETH - Sponsored fees: Update maxFees value **Effect**: + - Does not invalidate attestation - Off-chain watchers/transmitters see updated fees - Makes execution more attractive @@ -498,20 +550,22 @@ Socket/FastSwitchboard: No fee handling **Answer**: Function selectors create isolated nonce spaces to prevent cross-function replay. **Implementation**: + ```solidity function _validateAndUseNonce( - bytes4 selector_, // Function selector for namespace - address signer_, - uint256 nonce_ + bytes4 selector_, // Function selector for namespace + address signer_, + uint256 nonce_ ) internal { - // Namespace nonce with function selector - uint256 namespacedNonce = uint256(keccak256(abi.encodePacked(selector_, nonce_))); - if (usedNonces[signer_][namespacedNonce]) revert NonceAlreadyUsed(); - usedNonces[signer_][namespacedNonce] = true; + // Namespace nonce with function selector + uint256 namespacedNonce = uint256(keccak256(abi.encodePacked(selector_, nonce_))); + if (usedNonces[signer_][namespacedNonce]) revert NonceAlreadyUsed(); + usedNonces[signer_][namespacedNonce] = true; } ``` **Usage**: + ```solidity // Different functions, different namespaces _validateAndUseNonce(this.markRefundEligible.selector, watcher, nonce); @@ -519,6 +573,7 @@ _validateAndUseNonce(this.setMinMsgValueFees.selector, feeUpdater, nonce); ``` **Benefits**: + - ✓ Same nonce value can be used across different functions - ✓ Prevents accidental cross-function replay - ✓ Cleaner off-chain nonce management @@ -533,11 +588,13 @@ _validateAndUseNonce(this.setMinMsgValueFees.selector, feeUpdater, nonce); **Answer**: Deterministic encoding and gas efficiency. **Problem with Strings**: + - Encoding can differ between Solidity and off-chain code - Variable length increases gas cost - Potential for encoding mismatches **Benefits of Function Selectors**: + - ✓ Fixed size (bytes4 = 4 bytes) - ✓ Deterministically computed: `keccak256("functionName(params)")[:4]` - ✓ Same computation on-chain and off-chain @@ -545,11 +602,12 @@ _validateAndUseNonce(this.setMinMsgValueFees.selector, feeUpdater, nonce); - ✓ Lower gas cost **Example**: + ```javascript // Off-chain (JavaScript/TypeScript) -const selector = ethers.utils.id("markRefundEligible(bytes32,uint256,bytes)").slice(0, 10); +const selector = ethers.utils.id('markRefundEligible(bytes32,uint256,bytes)').slice(0, 10); const namespacedNonce = ethers.utils.keccak256( - ethers.utils.solidityPack(["bytes4", "uint256"], [selector, nonce]) + ethers.utils.solidityPack(['bytes4', 'uint256'], [selector, nonce]), ); ``` @@ -560,17 +618,19 @@ const namespacedNonce = ethers.utils.keccak256( **Answer**: Multiple safeguards: **Fee Storage**: + ```solidity struct PayloadFees { - uint256 nativeFees; // Immutable except increase/refund - address refundAddress; // Set at creation - bool isRefundEligible; // Only watcher can set - bool isRefunded; // One-time flag - address plug; // Ownership tracking + uint256 nativeFees; // Immutable except increase/refund + address refundAddress; // Set at creation + bool isRefundEligible; // Only watcher can set + bool isRefunded; // One-time flag + address plug; // Ownership tracking } ``` **Protections**: + 1. Only source plug can increase fees 2. Refunds only to specified refundAddress 3. Refunds only when watcher-approved @@ -578,6 +638,7 @@ struct PayloadFees { 5. Fees in successful execution go to NetworkFeeCollector (governance-set) **Cannot**: + - Decrease fees - Redirect refund address - Claim refund without watcher signature @@ -587,22 +648,25 @@ struct PayloadFees { ### Q18: What happens to excess msg.value? -**Answer**: +**Answer**: **On Successful Execution**: + ```solidity msg.value = executionParams.value + socketFees + excess ├─ executionParams.value → Plug -├─ socketFees → NetworkFeeCollector +├─ socketFees → NetworkFeeCollector └─ excess → Stays in Socket contract ⚠️ ``` **On Failed Execution**: + ```solidity msg.value (all) → Refunded to refundAddress ``` -**Recommendation**: +**Recommendation**: + - Send exact amount (value + socketFees) - Or accept that excess stays in Socket - Use `rescueFunds()` if significant amounts stuck @@ -618,6 +682,7 @@ msg.value (all) → Refunded to refundAddress **Answer**: Prevents collision attacks: **Without Length Prefixes**: + ```solidity // Collision possible: payload1 = "AAAA", source1 = "BB" @@ -626,6 +691,7 @@ payload2 = "AAA", source2 = "ABB" ``` **With Length Prefixes**: + ```solidity // Unique hashes: digest1 = keccak256(uint32(4) + "AAAA" + uint32(2) + "BB") @@ -633,6 +699,7 @@ digest2 = keccak256(uint32(3) + "AAA" + uint32(3) + "ABB") ``` **Applied To**: + - payload (variable length) - source (variable length) - extraData (variable length) @@ -646,12 +713,14 @@ digest2 = keccak256(uint32(3) + "AAA" + uint32(3) + "ABB") **Answer**: Security mechanism against DOS: **Problem**: + ```solidity // Malicious plug returns huge data return new bytes(10_000_000); // Would consume excessive gas to copy ``` **Solution**: + ```solidity maxCopyBytes = 2048; // Default 2KB @@ -662,6 +731,7 @@ maxCopyBytes = 2048; // Default 2KB ``` **Benefits**: + - Prevents DOS via excessive memory allocation - Predictable gas costs - Still allows reasonable return data @@ -675,18 +745,21 @@ maxCopyBytes = 2048; // Default 2KB **Answer**: Solady's tryCall provides safe external call handling: **Features**: + ```solidity -(bool success, bool exceededMaxCopy, bytes memory returnData) = +(bool success, bool exceededMaxCopy, bytes memory returnData) = target.tryCall(value, gasLimit, maxCopyBytes, payload); ``` **Advantages over raw call**: + - Explicit gas limit forwarding - Return data size limiting (DOS protection) - Doesn't revert on failure (returns success flag) - Safely handles all failure modes **Why Not Raw Call**: + ```solidity (bool success, bytes memory data) = target.call{value: value, gas: gasLimit}(payload); // Issues: @@ -702,21 +775,25 @@ maxCopyBytes = 2048; // Default 2KB **Answer**: Call types define execution context: **WRITE** (Currently Only Supported): + - State-changing operations - Executed on destination chain - Default for cross-chain messages **READ** (Not Yet Implemented): + - View/pure functions - Would read state without changes - Planned for future versions **SCHEDULE** (Not Yet Implemented): + - Delayed execution - Would schedule for future block - Planned for EVMX integration **Current Check**: + ```solidity if (executionParams_.callType != WRITE) revert InvalidCallType(); ``` @@ -734,6 +811,7 @@ uint32 public immutable chainSlug; ``` **Reasons**: + - Each Socket instance tied to specific chain - Cannot migrate Socket to different chain - Prevents misconfiguration @@ -741,6 +819,7 @@ uint32 public immutable chainSlug; - Gas optimization (immutable vs storage) **If Chain Slug Needs to Change**: + - Deploy new Socket contract - Cannot modify existing deployment - By design - prevents critical errors @@ -751,20 +830,23 @@ uint32 public immutable chainSlug; ### Q24: What happens if a plug connects then immediately disconnects? -**Answer**: +**Answer**: **State After Disconnect**: + ```solidity plugSwitchboardIds[plug] = 0; // Cleared in Socket // But: switchboard still has plug config stored ``` **Implications**: + - Cannot send new payloads (not connected) - Existing attested payloads still executable - Switchboard retains config (stale data) -**Cleanup**: +**Cleanup**: + - Switchboard config not automatically cleared - May want to call `updatePlugConfig(plug, "")` to clear - Low impact - just storage inefficiency @@ -776,12 +858,14 @@ plugSwitchboardIds[plug] = 0; // Cleared in Socket **Answer**: Yes, and it's an intended emergency mechanism: **Process**: + ```solidity socketConfig.disableSwitchboard(switchboardId); // Status: REGISTERED → DISABLED ``` **Effect on Connected Plugs**: + - Plugs remain connected (mapping not cleared) - New payloads fail (processPayload checks status) - Existing attested payloads still executable @@ -800,12 +884,14 @@ socketConfig.disableSwitchboard(switchboardId); **Scenario**: EVMX chain offline or compromised **Impact**: + - FastSwitchboard payloads cannot be attested - MessageSwitchboard unaffected (independent) - Plugs can disconnect from FastSwitchboard - Can connect to MessageSwitchboard instead **Mitigation**: + - Deploy multiple switchboard types - Don't rely solely on FastSwitchboard - Have fallback verification method @@ -816,22 +902,25 @@ socketConfig.disableSwitchboard(switchboardId); ### Q27: What happens at payload counter overflow? -**Answer**: +**Answer**: **Scenario**: `payloadCounter = type(uint64).max`, then processPayload() called **Behavior**: + ```solidity payloadCounter++ // Overflows in Solidity 0.8+ // Reverts with panic(0x11) - arithmetic overflow ``` **Impact**: + - Cannot create new payloads on this switchboard - DOS condition - Existing payloads unaffected -**Likelihood**: +**Likelihood**: + - 2^64 = 18,446,744,073,709,551,616 payloads needed - At 1000 payloads/second = 584 million years @@ -846,6 +935,7 @@ payloadCounter++ // Overflows in Solidity 0.8+ **Answer**: Design patterns: **Pattern 1: Idempotent Operations** + ```solidity // Make operations safe to replay function inbound(bytes memory data) external { @@ -857,6 +947,7 @@ function inbound(bytes memory data) external { ``` **Pattern 2: Sequence Numbers** + ```solidity uint256 public expectedNonce; function inbound(bytes memory data) external { @@ -869,6 +960,7 @@ function inbound(bytes memory data) external { ``` **Pattern 3: State Machine** + ```solidity enum State { A, B, C } State public state; @@ -887,6 +979,7 @@ function inbound(bytes memory data) external { **Answer**: Multi-step process: **Step 1: Simulate on Destination** + ```solidity // Off-chain: Call Socket.simulate() on destination chain SimulateParams[] memory params = [...]; @@ -895,12 +988,14 @@ SimulationResult[] memory results = socket.simulate(params); ``` **Step 2: Add Safety Buffer** + ```solidity uint256 estimatedGas = results[0].gasUsed; uint256 gasLimit = (estimatedGas * 150) / 100; // 50% buffer ``` **Step 3: Include in Overrides** + ```solidity bytes memory overrides = abi.encode( version, @@ -913,6 +1008,7 @@ socket.sendPayload{value: fees + value}(overrides, payload); ``` **Best Practices**: + - Always add buffer (at least 20%) - Test on destination chain - Monitor actual vs estimated @@ -925,16 +1021,19 @@ socket.sendPayload{value: fees + value}(overrides, payload); **Answer**: Partially: **Source Chain** (Non-EVM → EVM): + - Possible with appropriate switchboard - Switchboard must verify non-EVM chain proofs - Source encoding in bytes format **Destination Chain** (EVM → Non-EVM): + - Socket must be on EVM chain - Target must be EVM contract - Current: EVM-only execution **Solana Support**: + - Structs defined for Solana integration - `SolanaInstruction`, `SolanaReadRequest` in Structs.sol - Not fully implemented in current contracts @@ -950,12 +1049,14 @@ socket.sendPayload{value: fees + value}(overrides, payload); **Answer**: Application-level responsibility, not protocol concern. **Rationale**: + - Different applications have different time requirements - Some need hours, others need weeks or months - Protocol shouldn't impose business logic constraints - If app sets far-future deadline, it's their design choice **Application Responsibility**: + - Apps should handle stale state appropriately - Can implement their own deadline logic - Can check conditions before execution @@ -969,20 +1070,24 @@ socket.sendPayload{value: fees + value}(overrides, payload); **Answer**: Balance between simplicity and transmitter incentives. **Current Design**: + - Failed execution → Full refund to refundAddress - Transmitter loses gas cost for failed transaction **Rationale**: + 1. **Transmitters Should Simulate**: Off-chain simulate() function available 2. **External Reimbursement**: Transmitters compensated externally for failures 3. **Market Solution**: Bad plugs get blacklisted by transmitters 4. **Simplicity**: No complex partial refund logic needed **Griefing Vector**: Malicious plug could pass simulation but revert in production + - **Mitigation**: Market-based reputation system - **Impact**: Low - transmitters adapt behavior **Alternative Considered**: Keep socketFees even on failure + - **Downside**: Legitimate failures (network issues, gas spikes) penalize users - **Current**: More user-friendly, relies on transmitter rationality @@ -995,23 +1100,25 @@ socket.sendPayload{value: fees + value}(overrides, payload); **Gas Cost**: ReentrancyGuard adds ~2,500 gas per protected function **Why It's Safe**: + ```solidity // Checks-Effects-Interactions pattern function execute(...) { // CHECKS if (deadline < block.timestamp) revert; if (executionStatus[id] == Executed) revert; - + // EFFECTS executionStatus[id] = Executed; payloadIdToDigest[id] = digest; - + // INTERACTIONS target.tryCall(...); // Reentrancy here is safe } ``` **Reentrancy Scenarios**: + 1. Same payloadId → Reverts (status already Executed) 2. Different payloadId → New execution, independent state 3. sendPayload() → Creates new payload, no state conflict @@ -1027,12 +1134,14 @@ function execute(...) { **Answer**: Multi-layer validation prevents abuse. **Validation Layers**: + 1. **Socket Layer**: `_verifyPlugSwitchboard(msg.sender)` - ensures plug is connected 2. **onlySocket Modifier**: Only Socket can call switchboard 3. **Plug Ownership**: Switchboard checks `payloadFees[id].plug == plug_` 4. **Off-Chain**: Watchers verify before applying fee updates **Attack Attempt**: + ```solidity // Attacker tries to increase fees for someone else's payload attacker.increaseFeesForPayload(victimPayloadId, feeData) @@ -1051,38 +1160,45 @@ attacker.increaseFeesForPayload(victimPayloadId, feeData) ### Q34: Areas We'd Like Feedback On **1. Gas Limit Flexibility**: + - No hardcoded maximum gas limit to support diverse chains - Is this appropriate, or should we have a configurable max per chain? - Could extremely high gasLimit values cause issues we haven't considered? **2. Switchboard Trust Model**: + - Is the trust assumption on switchboards acceptable for production? - Should we add on-chain reputation/bonding mechanisms? - How should plugs evaluate switchboard trustworthiness? **3. Fee Economic Model**: + - Native fee model: Is external transmitter reimbursement sufficient? - Griefing attacks: Should protocol provide on-chain mitigation? - Fee market: Will competition drive efficient delivery? **4. Counter Exhaustion**: + - uint64 payloadCounter: ~18 quintillion payloads - Should we add explicit handling for counter approaching max? - Is revert-on-overflow the right approach, or should we allow rollover? **5. Upgrade Path**: + - Contracts currently not upgradeable - Is this appropriate for critical infrastructure? - If security issue found, migration path is deploy-new-contracts - Should we consider proxy pattern for critical contracts? **6. Cross-Chain State Synchronization**: + - Protocol assumes eventual consistency - No built-in ordering enforcement - Is this appropriate for all use cases? - Should we provide optional ordering mechanisms? **7. Edge Case Handling**: + - Plug that always reverts: Acceptable? (Currently: yes, funds refunded) - Excessive return data: Limited to maxCopyBytes (Currently: 2KB) - Deadline precision: Uses block.timestamp (±15 seconds) @@ -1093,16 +1209,19 @@ attacker.increaseFeesForPayload(victimPayloadId, feeData) ## Contact & Support **For Audit Questions**: + - Open issue in repository with [AUDIT] tag - Email: [audit-support@example.com] - Discord: [#auditor-support channel] **For Technical Clarifications**: + - Reference this FAQ first - Check other documentation files - Ask in audit communication channel **For Security Issues**: + - DO NOT post publicly - Email: [security@example.com] - Use PGP key if available @@ -1115,4 +1234,3 @@ This FAQ is maintained during the audit process. If you have questions not cover **Last Updated**: [Date] **Version**: 1.0 - diff --git a/auditor-docs/MESSAGE_FLOW.md b/auditor-docs/MESSAGE_FLOW.md index 4f26faf3..cae256b7 100644 --- a/auditor-docs/MESSAGE_FLOW.md +++ b/auditor-docs/MESSAGE_FLOW.md @@ -17,11 +17,13 @@ This document details the step-by-step flows for cross-chain message passing thr ### Detailed Steps #### Step 1: Plug Initiates Send + ``` Plug calls: socket.sendPayload(callData) OR fallback() ``` **Checks Performed**: + - Socket is not paused - Plug has sufficient balance for msg.value (if any) @@ -30,11 +32,13 @@ Plug calls: socket.sendPayload(callData) OR fallback() --- #### Step 2: Socket Validates Plug Connection + ``` Function: Socket._sendPayload() → SocketUtils._verifyPlugSwitchboard() ``` **Checks Performed**: + - `plugSwitchboardIds[plug] != 0` (plug is connected) - `switchboardStatus[switchboardId] == REGISTERED` (switchboard is active) @@ -43,6 +47,7 @@ Function: Socket._sendPayload() → SocketUtils._verifyPlugSwitchboard() --- #### Step 3: Socket Retrieves Plug Overrides + ``` Call: IPlug(plug).overrides() ``` @@ -50,6 +55,7 @@ Call: IPlug(plug).overrides() **Purpose**: Plug specifies destination chain, gas limit, deadline, fees, etc. **Format**: Depends on switchboard type + - FastSwitchboard: `abi.encode(deadline)` or empty for default - MessageSwitchboard: Version-based encoding (see below) @@ -72,6 +78,7 @@ Sequence: ``` **State Changes**: + - `payloadCounter` increments - `payloadIdToPlug[payloadId]` set @@ -82,11 +89,11 @@ Sequence: ``` Sequence: 1. Decode overrides based on version: - + Version 1 (Native Fees): - (uint8, uint32 dstChainSlug, uint256 gasLimit, uint256 value, + (uint8, uint32 dstChainSlug, uint256 gasLimit, uint256 value, address refundAddress, uint256 deadline) - + Version 2 (Sponsored): (uint8, uint32 dstChainSlug, uint256 gasLimit, uint256 value, uint256 maxFees, address sponsor, uint256 deadline) @@ -98,21 +105,21 @@ Sequence: 3. Create digest and payload ID: - Get dstSwitchboardId from siblingSwitchboardIds[dstChainSlug] - - Create payload ID: source=(chainSlug, switchboardId), + - Create payload ID: source=(chainSlug, switchboardId), verification=(dstChainSlug, dstSwitchboardId), pointer=payloadCounter++ - Build DigestParams with destination socket/plug addresses - Hash digest 4. Handle fees: - + If Sponsored: - Check sponsorApprovals[sponsor][plug] == true - Store sponsoredPayloadFees[payloadId] = (maxFees, plug) - Emit MessageOutbound with isSponsored=true - + If Native: - Check msg.value >= minMsgValueFees[dstChainSlug] + value - - Store payloadFees[payloadId] = (nativeFees=msg.value, refundAddress, + - Store payloadFees[payloadId] = (nativeFees=msg.value, refundAddress, isRefundEligible=false, isRefunded=false, plug) - Emit MessageOutbound with isSponsored=false @@ -120,6 +127,7 @@ Sequence: ``` **State Changes**: + - `payloadCounter` increments - `payloadFees[payloadId]` or `sponsoredPayloadFees[payloadId]` set - Native fees stored in contract balance @@ -129,6 +137,7 @@ Sequence: #### Step 5: Off-Chain Processing (Not in Scope) Watchers monitoring source chain: + 1. See PayloadRequested event 2. Validate payload and source 3. Submit attestation to destination chain switchboard @@ -146,28 +155,33 @@ Watchers monitoring source chain: ### Detailed Steps #### Step 1: Transmitter Submits Execution + ``` Transmitter calls: socket.execute(executionParams, transmissionParams) ``` **executionParams** contains: + - payloadId, target, payload, gasLimit, value, deadline - callType (must be WRITE) - source (encoded source chain + plug) - prevBatchDigestHash, extraData **transmissionParams** contains: + - socketFees (amount for transmitter/protocol) - refundAddress (where to refund on failure) - transmitterProof (optional signature) - extraData **Requirements**: + - `msg.value >= executionParams.value + transmissionParams.socketFees` --- #### Step 2: Socket Validates Execution Request + ``` Function: Socket.execute() ``` @@ -175,16 +189,19 @@ Function: Socket.execute() **Validations (in order)**: 1. **Deadline Check**: + ``` if (executionParams.deadline < block.timestamp) revert DeadlinePassed() ``` 2. **Call Type Check**: + ``` if (executionParams.callType != WRITE) revert InvalidCallType() ``` 3. **Plug Connection**: + ``` _verifyPlugSwitchboard(executionParams.target) → Checks plug is connected and switchboard is REGISTERED @@ -192,12 +209,14 @@ Function: Socket.execute() ``` 4. **Value Check**: + ``` if (msg.value < executionParams.value + transmissionParams.socketFees) revert InsufficientMsgValue() ``` 5. **Payload ID Routing**: + ``` _verifyPayloadId(executionParams.payloadId, switchboardAddress) → Extract verification chain slug and switchboard ID from payloadId @@ -215,6 +234,7 @@ Function: Socket.execute() --- #### Step 3: Verify Digest Through Switchboard + ``` Function: Socket._verify() ``` @@ -222,6 +242,7 @@ Function: Socket._verify() **Sequence**: 1. **Recover Transmitter**: + ``` address transmitter = switchboard.getTransmitter( msg.sender, @@ -229,15 +250,18 @@ Function: Socket._verify() transmissionParams.transmitterProof ) ``` + - If no proof provided, returns address(0) - If proof provided, recovers signer from signature 2. **Create Digest**: + ``` bytes32 digest = _createDigest(transmitter, executionParams) ``` - + Digest includes (with length prefixes for variable fields): + - socket address, transmitter, payloadId, deadline - callType, gasLimit, value, target - prevBatchDigestHash @@ -246,6 +270,7 @@ Function: Socket._verify() - uint32(extraData.length) + extraData 3. **Store Digest**: + ``` payloadIdToDigest[payloadId] = digest ``` @@ -264,6 +289,7 @@ Function: Socket._verify() --- #### Step 4: Execute on Target Plug + ``` Function: Socket._execute() ``` @@ -271,15 +297,18 @@ Function: Socket._execute() **Sequence**: 1. **Gas Check**: + ``` if (gasleft() < (executionParams.gasLimit * gasLimitBuffer) / 100) revert LowGasLimit() ``` + - gasLimitBuffer typically 105 (5% overhead) 2. **External Call**: + ``` - (bool success, bool exceededMaxCopy, bytes memory returnData) = + (bool success, bool exceededMaxCopy, bytes memory returnData) = executionParams.target.tryCall( executionParams.value, executionParams.gasLimit, @@ -287,12 +316,14 @@ Function: Socket._execute() executionParams.payload ) ``` + - Uses Solady's LibCall.tryCall() - Limits return data to maxCopyBytes (default 2048) 3. **Handle Result**: **If Success**: + ``` _handleSuccessfulExecution() → Emit ExecutionSuccess(payloadId, exceededMaxCopy, returnData) @@ -304,6 +335,7 @@ Function: Socket._execute() ``` **If Failure**: + ``` _handleFailedExecution() → Set executionStatus[payloadId] = Reverted @@ -312,6 +344,7 @@ Function: Socket._execute() ``` **State Changes**: + - `executionStatus[payloadId]` = Executed or Reverted - `payloadIdToDigest[payloadId]` = digest - Fees transferred (success) or refunded (failure) @@ -327,6 +360,7 @@ Watcher calls: fastSwitchboard.attest(digest, proof) ``` **Sequence**: + 1. Check `!isAttested[digest]` (prevent double attestation) 2. Recover watcher from signature: ``` @@ -342,6 +376,7 @@ Watcher calls: fastSwitchboard.attest(digest, proof) 5. Emit `Attested(digest, watcher)` **allowPayload Check**: + ``` 1. Decode source: bytes32 appGatewayId = abi.decode(source) 2. Check plugAppGatewayIds[target] == appGatewayId @@ -357,6 +392,7 @@ Watcher calls: messageSwitchboard.attest(digestParams, proof) ``` **Sequence**: + 1. Create digest from DigestParams: `digest = _createDigest(digestParams)` 2. Recover watcher from signature: ``` @@ -373,6 +409,7 @@ Watcher calls: messageSwitchboard.attest(digestParams, proof) 6. Emit `Attested(payloadId, digest, watcher)` **allowPayload Check**: + ``` 1. Decode source: (uint32 srcChainSlug, bytes32 srcPlug) = _decodePackedSource(source) 2. Check siblingPlugs[srcChainSlug][target] == srcPlug @@ -390,6 +427,7 @@ Plug calls: socket.increaseFeesForPayload(payloadId, feesData) {value: amount} ``` **Sequence**: + 1. Socket validates plug is connected: `_verifyPlugSwitchboard(msg.sender)` 2. Socket forwards to switchboard: ``` @@ -401,6 +439,7 @@ Plug calls: socket.increaseFeesForPayload(payloadId, feesData) {value: amount} ``` **MessageSwitchboard Processing**: + ``` 1. Decode feesType from feesData (first byte) 2. If feesType == 1 (Native): @@ -415,6 +454,7 @@ Plug calls: socket.increaseFeesForPayload(payloadId, feesData) {value: amount} ``` **FastSwitchboard Processing**: + ``` 1. Check payloadIdToPlug[payloadId] == plug 2. Emit FeesIncreased (event only, no state change) @@ -432,6 +472,7 @@ Anyone calls: messageSwitchboard.markRefundEligible(payloadId, nonce, signature) ``` **Sequence**: + 1. Check `!payloadFees[payloadId].isRefundEligible` 2. Check `payloadFees[payloadId].nativeFees > 0` 3. Create digest: @@ -459,6 +500,7 @@ Anyone calls: messageSwitchboard.refund(payloadId) ``` **Sequence** (protected by ReentrancyGuard): + 1. Check `payloadFees[payloadId].isRefundEligible == true` 2. Check `payloadFees[payloadId].isRefunded == false` 3. Cache `feesToRefund = payloadFees[payloadId].nativeFees` @@ -478,6 +520,7 @@ Switchboard calls: socket.registerSwitchboard() ``` **Sequence**: + 1. Check `switchboardAddressToId[msg.sender] == 0` (not already registered) 2. Assign ID: `switchboardId = switchboardIdCounter++` 3. Store mappings: @@ -496,6 +539,7 @@ Plug calls: socket.connect(switchboardId, plugConfig) ``` **Sequence**: + 1. Validate `switchboardId != 0` 2. Check `switchboardStatus[switchboardId] == REGISTERED` 3. Store: `plugSwitchboardIds[msg.sender] = switchboardId` @@ -516,6 +560,7 @@ Plug calls: socket.disconnect() ``` **Sequence**: + 1. Check `plugSwitchboardIds[msg.sender] != 0` (is connected) 2. Set `plugSwitchboardIds[msg.sender] = 0` 3. Emit `PlugDisconnected(msg.sender)` @@ -566,6 +611,7 @@ disconnected ↔ connected (bidirectional via connect/disconnect) ## 7. Critical Checkpoints ### Execution Must Pass + 1. ✓ Contract not paused 2. ✓ Deadline not passed 3. ✓ Call type is WRITE @@ -579,6 +625,7 @@ disconnected ↔ connected (bidirectional via connect/disconnect) 11. ✓ Digest is attested ### Sending Must Pass + 1. ✓ Contract not paused 2. ✓ Plug is connected 3. ✓ Switchboard is REGISTERED @@ -592,24 +639,27 @@ disconnected ↔ connected (bidirectional via connect/disconnect) ## 8. Event Emission Order ### Successful Execution + ``` 1. ExecutionSuccess(payloadId, exceededMaxCopy, returnData) 2. [NetworkFeeCollector events - if configured] ``` ### Failed Execution + ``` 1. ExecutionFailed(payloadId, exceededMaxCopy, returnData) ``` ### Payload Sending + ``` 1. MessageOutbound (MessageSwitchboard only) 2. PayloadRequested ``` ### Attestation + ``` 1. Attested(digest/payloadId, watcher) ``` - diff --git a/auditor-docs/README.md b/auditor-docs/README.md index 3414b033..8b14882c 100644 --- a/auditor-docs/README.md +++ b/auditor-docs/README.md @@ -17,9 +17,11 @@ Welcome to the Socket Protocol auditor documentation package. This collection of ## 📚 Documentation Index ### 0. [AUDIT_PREP_SUMMARY.md](./AUDIT_PREP_SUMMARY.md) - **NEW** + **Pre-audit review results** and improvements made. **Contents**: + - Validated security patterns (CEI, replay protection) - Nonce namespace isolation improvement implemented - Issues analyzed and dismissed with rationale @@ -30,9 +32,11 @@ Welcome to the Socket Protocol auditor documentation package. This collection of **Read this if**: You want to understand what was already reviewed and improved. ### 1. [SYSTEM_OVERVIEW.md](./SYSTEM_OVERVIEW.md) + **Start here** for a high-level understanding of the protocol. **Contents**: + - Protocol purpose and value proposition - High-level architecture diagram - Core components (Socket, Switchboards, Plugs, Watchers) @@ -45,9 +49,11 @@ Welcome to the Socket Protocol auditor documentation package. This collection of --- ### 2. [CONTRACTS_REFERENCE.md](./CONTRACTS_REFERENCE.md) + **Complete reference** for all contracts in scope. **Contents**: + - Contract inventory table with LOC and purpose - Detailed descriptions of each contract - Key state variables and functions @@ -60,9 +66,11 @@ Welcome to the Socket Protocol auditor documentation package. This collection of --- ### 3. [MESSAGE_FLOW.md](./MESSAGE_FLOW.md) + **Step-by-step flows** through the system. **Contents**: + - Outbound flow (sending payloads) - Inbound flow (executing payloads) - Attestation flows per switchboard type @@ -76,9 +84,11 @@ Welcome to the Socket Protocol auditor documentation package. This collection of --- ### 4. [SECURITY_MODEL.md](./SECURITY_MODEL.md) + **Security properties and assumptions**. **Contents**: + - Trusted vs untrusted entities - Access control matrix - Critical invariants that must hold @@ -92,9 +102,11 @@ Welcome to the Socket Protocol auditor documentation package. This collection of --- ### 5. [AUDIT_FOCUS_AREAS.md](./AUDIT_FOCUS_AREAS.md) + **Priority areas for audit attention**. **Contents**: + - Critical functions ranked by priority - Value flow points (all ETH transfers) - Cross-contract interaction risks @@ -109,9 +121,11 @@ Welcome to the Socket Protocol auditor documentation package. This collection of --- ### 6. [SETUP_GUIDE.md](./SETUP_GUIDE.md) + **Get the codebase running**. **Contents**: + - Environment setup (Node.js, Foundry) - Build and compile instructions - Running tests (Foundry and Hardhat) @@ -125,9 +139,11 @@ Welcome to the Socket Protocol auditor documentation package. This collection of --- ### 7. [TESTING_COVERAGE.md](./TESTING_COVERAGE.md) + **Existing tests and coverage gaps**. **Contents**: + - Current test organization - Existing test coverage summary - Coverage metrics by contract @@ -141,9 +157,11 @@ Welcome to the Socket Protocol auditor documentation package. This collection of --- ### 8. [FAQ.md](./FAQ.md) + **Answers to common questions**. **Contents**: + - Architecture and design rationale - Security and trust questions - Operations and behavior clarifications @@ -161,34 +179,41 @@ Welcome to the Socket Protocol auditor documentation package. This collection of ### For First-Time Auditors **Step 1**: Read [SYSTEM_OVERVIEW.md](./SYSTEM_OVERVIEW.md) + - Understand the big picture - Learn the key components - Grasp the trust model **Step 2**: Skim [CONTRACTS_REFERENCE.md](./CONTRACTS_REFERENCE.md) + - Get familiar with contract names and purposes - Note the contract interaction patterns **Step 3**: Follow [SETUP_GUIDE.md](./SETUP_GUIDE.md) + - Set up your environment - Compile the contracts - Run the test suite **Step 4**: Dive into [AUDIT_FOCUS_AREAS.md](./AUDIT_FOCUS_AREAS.md) + - Start with Priority 1 functions - Check value flow points - Verify signature mechanisms **Step 5**: Trace flows using [MESSAGE_FLOW.md](./MESSAGE_FLOW.md) + - Follow an execution from start to finish - Understand state changes at each step **Step 6**: Review [SECURITY_MODEL.md](./SECURITY_MODEL.md) + - Verify invariants hold - Check attack surface areas - Validate access controls **Step 7**: Reference [TESTING_COVERAGE.md](./TESTING_COVERAGE.md) & [FAQ.md](./FAQ.md) as needed + - Check what's already tested - Find answers to specific questions @@ -198,16 +223,16 @@ Welcome to the Socket Protocol auditor documentation package. This collection of ### Contracts In Scope (8 files) -| Contract | LOC | Complexity | Priority | -|----------|-----|------------|----------| -| Socket.sol | 286 | High | P0 - Critical | -| SocketUtils.sol | 210 | Medium | P0 - Critical | -| MessageSwitchboard.sol | 740 | High | P0 - Critical | -| FastSwitchboard.sol | 244 | Medium | P1 - High | -| SocketConfig.sol | 203 | Medium | P1 - High | -| SwitchboardBase.sol | 115 | Low | P2 - Medium | -| IdUtils.sol | 75 | Low | P2 - Medium | -| OverrideParamsLib.sol | 148 | Low | P3 - Low | +| Contract | LOC | Complexity | Priority | +| ---------------------- | --- | ---------- | ------------- | +| Socket.sol | 286 | High | P0 - Critical | +| SocketUtils.sol | 210 | Medium | P0 - Critical | +| MessageSwitchboard.sol | 740 | High | P0 - Critical | +| FastSwitchboard.sol | 244 | Medium | P1 - High | +| SocketConfig.sol | 203 | Medium | P1 - High | +| SwitchboardBase.sol | 115 | Low | P2 - Medium | +| IdUtils.sol | 75 | Low | P2 - Medium | +| OverrideParamsLib.sol | 148 | Low | P3 - Low | **Total Lines of Code**: ~2,000 LOC @@ -216,13 +241,15 @@ Welcome to the Socket Protocol auditor documentation package. This collection of ### Key Areas of Focus 🔴 **Critical** (Must Review): + - Socket.execute() - Main execution entry point -- Socket._execute() - External call to plugs +- Socket.\_execute() - External call to plugs - Digest creation and verification - Replay protection mechanisms - Value transfers and fee handling 🟠 **High** (Should Review): + - Switchboard attestation flows - Fee increase and refund logic - Nonce management @@ -230,6 +257,7 @@ Welcome to the Socket Protocol auditor documentation package. This collection of - Configuration management 🟡 **Medium** (Nice to Review): + - Payload ID encoding/decoding - Parameter builder utilities - Event emissions @@ -244,22 +272,27 @@ These assumptions are fundamental to the protocol's security model: ### Trust Model 1. **Switchboards are Trusted by Plugs** + - Anyone can register, plugs choose whom to trust - Plug's responsibility to verify switchboard before connecting 2. **NetworkFeeCollector is Trusted by Socket** + - Set by governance, called after successful execution - No reentrancy concerns (trusted entity) 3. **Target Plugs are Trusted by Source Plugs** + - Source specifies destination plug - Invalid target only affects the configuring plug 4. **simulate() is Off-Chain Only** + - Gated by OFF_CHAIN_CALLER (0xDEAD) - Used for gas estimation, not accessible on mainnet 5. **Watchers Act Honestly** + - At least one honest watcher assumed per payload - Verify source chain correctly, respect finality @@ -271,16 +304,19 @@ These assumptions are fundamental to the protocol's security model: ### Design Tradeoffs 1. **Payload Execution is One-Time Only** + - No retry mechanism for failed payloads - Simplicity & security over retry complexity - Application layer can send new payloads if needed 2. **No Built-in Ordering Enforcement** + - Payloads can execute in any order - Asynchronous cross-chain messaging by nature - Applications must handle out-of-order delivery 3. **No Maximum Gas Limit** + - Supports diverse chains (Ethereum: 30M, Mantle: 4B) - Flexibility over restrictive limits - Natural failure if insufficient gas provided @@ -293,10 +329,12 @@ These assumptions are fundamental to the protocol's security model: ### Security Patterns 1. **CEI (Checks-Effects-Interactions)** + - State updated before external calls - Reentrancy allowed but safe 2. **Multi-Layer Replay Protection** + - executionStatus prevents double execution - isAttested prevents double attestation - Namespace-isolated nonces prevent cross-function replay @@ -320,27 +358,32 @@ These assumptions are fundamental to the protocol's security model: ### Recommended Tools **Static Analysis**: + - Slither - Mythril - Aderyn **Dynamic Analysis**: + - Foundry (fuzzing & invariant testing) - Echidna - Manticore **Gas Analysis**: + - Foundry gas reports - Hardhat gas reporter ### External Dependencies **Solady Library** (`lib/solady/`): + - Gas-optimized implementations - Widely used and audited - Key modules: LibCall, ECDSA, SafeTransferLib, ReentrancyGuard **Forge Standard Library** (`lib/forge-std/`): + - Testing utilities only - Not deployed on-chain @@ -351,16 +394,19 @@ These assumptions are fundamental to the protocol's security model: ### Questions During Audit **Technical Questions**: + 1. Check [FAQ.md](./FAQ.md) first 2. Review relevant documentation sections 3. Open issue with [AUDIT-QUESTION] tag **Clarifications Needed**: + 1. Consult [CONTRACTS_REFERENCE.md](./CONTRACTS_REFERENCE.md) 2. Review [MESSAGE_FLOW.md](./MESSAGE_FLOW.md) 3. Request clarification via designated channel **Security Concerns**: + 1. Note in your audit report 2. Verify with [SECURITY_MODEL.md](./SECURITY_MODEL.md) 3. Discuss in secure audit channel @@ -368,6 +414,7 @@ These assumptions are fundamental to the protocol's security model: ### Feedback Welcome We appreciate feedback on: + - Documentation clarity and completeness - Missing information or unclear explanations - Suggested improvements to the protocol @@ -405,15 +452,18 @@ We appreciate feedback on: **Suggested Schedule**: - **Week 1**: Setup, overview, and architecture review + - Days 1-2: Environment setup and documentation review - Days 3-5: High-level architecture and flow tracing - **Week 2**: Deep dive into critical functions + - Days 1-2: Socket.sol and SocketUtils.sol - Days 3-4: MessageSwitchboard.sol - Day 5: FastSwitchboard.sol - **Week 3**: Security analysis and testing + - Days 1-2: Attack surface analysis - Days 3-4: Writing additional tests - Day 5: Fuzzing and invariant testing @@ -429,26 +479,31 @@ We appreciate feedback on: ### For Auditors New to Cross-Chain Protocols **Day 1**: Conceptual Understanding + - Read SYSTEM_OVERVIEW.md thoroughly - Understand the problem Socket solves - Learn about switchboard architecture **Day 2**: Contract Familiarity + - Skim all contracts in CONTRACTS_REFERENCE.md - Draw your own architecture diagram - Identify entry and exit points **Day 3**: Flow Tracing + - Pick one successful execution path - Trace it step-by-step using MESSAGE_FLOW.md - Note all state changes **Day 4**: Security Focus + - Read SECURITY_MODEL.md - List all trust assumptions - Identify attack vectors **Day 5**: Hands-On + - Set up environment per SETUP_GUIDE.md - Run tests - Try breaking things @@ -462,11 +517,13 @@ We appreciate feedback on: ### Key Addresses (Example - Update for Actual Deployment) **Ethereum Sepolia**: + - Socket: `0x...` - MessageSwitchboard: `0x...` - FastSwitchboard: `0x...` **Arbitrum Sepolia**: + - Socket: `0x...` - MessageSwitchboard: `0x...` @@ -538,4 +595,3 @@ If you need any clarification or additional information, please don't hesitate t **Protocol Version**: [Version] **Audit Firm**: [Firm Name] **Point of Contact**: [Name/Email] - diff --git a/auditor-docs/SECURITY_MODEL.md b/auditor-docs/SECURITY_MODEL.md index 63b23b73..df582d36 100644 --- a/auditor-docs/SECURITY_MODEL.md +++ b/auditor-docs/SECURITY_MODEL.md @@ -5,9 +5,11 @@ ### Trusted Entities #### 1. **Governance** + **Trust Level**: High **Capabilities**: + - Enable/re-enable switchboards via `enableSwitchboard()` - Set network fee collector address - Set gas limit buffer (minimum 100%) @@ -15,6 +17,7 @@ - Grant/revoke roles to other addresses **Cannot Do**: + - Directly execute payloads - Access user funds - Modify past execution status @@ -25,41 +28,49 @@ --- #### 2. **Watchers** + **Trust Level**: High (Critical for security) **Capabilities**: + - Attest to payload digests via `attest()` - Mark payloads as refund eligible - Sign off-chain for fee updates (FEE_UPDATER_ROLE) **Cannot Do**: + - Execute payloads directly - Withdraw fees - Modify switchboard configuration - Change payload content after attestation -**Assumption**: +**Assumption**: + - At least one honest watcher per payload - Watchers verify source chain state correctly - Watchers respect finality before attesting - Will not attest to invalid payloads -**Attack Vector if Compromised**: +**Attack Vector if Compromised**: + - Could attest to malicious payloads - Could refuse to attest legitimate payloads (liveness failure) --- #### 3. **Switchboard Owners** + **Trust Level**: Medium-High **Capabilities**: + - Configure EVMX settings (FastSwitchboard) - Set default deadlines - Mark payloads as reverting - Grant WATCHER_ROLE to addresses **Cannot Do**: + - Modify payload content - Access fees directly - Override Socket validation @@ -69,9 +80,11 @@ --- #### 4. **Socket Owner (Initial)** + **Trust Level**: High (Initial deployment only) **Capabilities**: + - Deploy contracts with correct parameters - Set initial role holders - Transfer ownership to governance @@ -83,9 +96,11 @@ ### Untrusted Entities #### 1. **Plugs (Application Contracts)** + **Trust Level**: None (Fully adversarial) **Behavior**: + - May be malicious or buggy - Can attempt reentrancy - Can revert on execution @@ -93,6 +108,7 @@ - Can emit misleading events **Protections**: + - Isolated execution environment - Gas limits enforced - Execution status prevents replay @@ -102,15 +118,18 @@ --- #### 2. **Transmitters** + **Trust Level**: Low (Economic actors) **Behavior**: + - Rational economic actors seeking fees - May try to extract MEV - May deliver payloads in any order - May delay delivery **Protections**: + - Cannot forge attestations (requires watcher signature) - Cannot modify payload content (digest verification) - Deadlines prevent indefinite delays @@ -121,14 +140,17 @@ --- #### 3. **Fee Payers** + **Trust Level**: None **Behavior**: + - May underpay fees - May try to DOS system with spam - May attempt double-spending **Protections**: + - Minimum fee requirements enforced - Insufficient fees cause revert - No refund on successful execution @@ -136,13 +158,16 @@ --- #### 4. **Sponsor Accounts** + **Trust Level**: None (User-controlled) **Behavior**: + - May approve malicious plugs - May revoke approvals mid-flight **Protections**: + - Explicit approval required via `approvePlug()` - Only affects sponsored payloads - Cannot affect native fee payloads @@ -151,27 +176,27 @@ ## Access Control Matrix -| Function | Contract | Roles Required | Restriction | -|----------|----------|----------------|-------------| -| `execute()` | Socket | None | Not paused, valid params | -| `sendPayload()` | Socket | None | Not paused, connected plug | -| `connect()` | Socket | None (msg.sender = plug) | Valid switchboard | -| `disconnect()` | Socket | None (msg.sender = plug) | Currently connected | -| `registerSwitchboard()` | Socket | None (msg.sender = switchboard) | Not already registered | -| `disableSwitchboard()` | SocketConfig | SWITCHBOARD_DISABLER_ROLE | - | -| `enableSwitchboard()` | SocketConfig | GOVERNANCE_ROLE | - | -| `setNetworkFeeCollector()` | SocketConfig | GOVERNANCE_ROLE | - | -| `setGasLimitBuffer()` | SocketConfig | GOVERNANCE_ROLE | >= 100 | -| `setMaxCopyBytes()` | SocketConfig | GOVERNANCE_ROLE | - | -| `pause()` | SocketUtils | PAUSER_ROLE | - | -| `unpause()` | SocketUtils | UNPAUSER_ROLE | - | -| `rescueFunds()` | SocketUtils/Switchboards | RESCUE_ROLE | - | -| `attest()` | Switchboards | WATCHER_ROLE | Valid signature | -| `markRefundEligible()` | MessageSwitchboard | WATCHER_ROLE | Valid signature + nonce | -| `refund()` | MessageSwitchboard | None | Must be eligible | -| `setMinMsgValueFees()` | MessageSwitchboard | FEE_UPDATER_ROLE | Valid signature + nonce | -| `setEvmxConfig()` | FastSwitchboard | onlyOwner | - | -| `setRevertingPayload()` | Switchboards | onlyOwner | - | +| Function | Contract | Roles Required | Restriction | +| -------------------------- | ------------------------ | ------------------------------- | -------------------------- | +| `execute()` | Socket | None | Not paused, valid params | +| `sendPayload()` | Socket | None | Not paused, connected plug | +| `connect()` | Socket | None (msg.sender = plug) | Valid switchboard | +| `disconnect()` | Socket | None (msg.sender = plug) | Currently connected | +| `registerSwitchboard()` | Socket | None (msg.sender = switchboard) | Not already registered | +| `disableSwitchboard()` | SocketConfig | SWITCHBOARD_DISABLER_ROLE | - | +| `enableSwitchboard()` | SocketConfig | GOVERNANCE_ROLE | - | +| `setNetworkFeeCollector()` | SocketConfig | GOVERNANCE_ROLE | - | +| `setGasLimitBuffer()` | SocketConfig | GOVERNANCE_ROLE | >= 100 | +| `setMaxCopyBytes()` | SocketConfig | GOVERNANCE_ROLE | - | +| `pause()` | SocketUtils | PAUSER_ROLE | - | +| `unpause()` | SocketUtils | UNPAUSER_ROLE | - | +| `rescueFunds()` | SocketUtils/Switchboards | RESCUE_ROLE | - | +| `attest()` | Switchboards | WATCHER_ROLE | Valid signature | +| `markRefundEligible()` | MessageSwitchboard | WATCHER_ROLE | Valid signature + nonce | +| `refund()` | MessageSwitchboard | None | Must be eligible | +| `setMinMsgValueFees()` | MessageSwitchboard | FEE_UPDATER_ROLE | Valid signature + nonce | +| `setEvmxConfig()` | FastSwitchboard | onlyOwner | - | +| `setRevertingPayload()` | Switchboards | onlyOwner | - | --- @@ -180,9 +205,11 @@ These properties must ALWAYS hold true: ### 1. Execution Uniqueness + ``` ∀ payloadId: executionStatus[payloadId] ∈ {NotExecuted, Executed, Reverted} ``` + Once set to Executed or Reverted, status cannot change. **Consequence**: No payload can be executed twice. @@ -190,9 +217,11 @@ Once set to Executed or Reverted, status cannot change. --- ### 2. Digest Immutability + ``` ∀ payloadId: payloadIdToDigest[payloadId] is write-once ``` + Once digest is stored, it cannot be modified. **Consequence**: Execution parameters cannot be changed after verification. @@ -200,9 +229,11 @@ Once digest is stored, it cannot be modified. --- ### 3. Attestation Permanence + ``` ∀ digest: isAttested[digest] = true ⟹ always true ``` + Attestations cannot be revoked. **Consequence**: Attested payloads remain attested forever. @@ -210,6 +241,7 @@ Attestations cannot be revoked. --- ### 4. Switchboard ID Uniqueness + ``` ∀ address A: switchboardAddressToId[A] assigned once and never changes ∀ id: switchboardAddresses[id] assigned once and never changes @@ -220,6 +252,7 @@ Attestations cannot be revoked. --- ### 5. Monotonic Counters + ``` payloadCounter only increases (never decreases or resets) switchboardIdCounter only increases @@ -230,6 +263,7 @@ switchboardIdCounter only increases --- ### 6. Fee Conservation (Native) + ``` payloadFees[id].nativeFees can only: - Increase via increaseFeesForPayload() @@ -241,6 +275,7 @@ payloadFees[id].nativeFees can only: --- ### 7. Refund Single-Use + ``` payloadFees[id].isRefunded = true ⟹ payloadFees[id].nativeFees = 0 ``` @@ -250,6 +285,7 @@ payloadFees[id].isRefunded = true ⟹ payloadFees[id].nativeFees = 0 --- ### 8. Execution Value Constraint + ``` At execute(): msg.value >= executionParams.value + transmissionParams.socketFees ``` @@ -259,6 +295,7 @@ At execute(): msg.value >= executionParams.value + transmissionParams.socketFees --- ### 9. Payload ID Routing + ``` ∀ payload executed on chainSlug C via switchboard S: getVerificationInfo(payloadId) = (C, S.switchboardId) @@ -269,6 +306,7 @@ At execute(): msg.value >= executionParams.value + transmissionParams.socketFees --- ### 10. Source Validation + ``` ∀ payload with source S executing on target T: switchboard.allowPayload() validates S matches expected source for T @@ -282,14 +320,14 @@ At execute(): msg.value >= executionParams.value + transmissionParams.socketFees ### 1. External Call Points (High Risk) -| Location | Called Contract | Protection | -|----------|----------------|------------| -| Socket._execute() | Plug (target) | Gas limit, tryCall, execution status set first | -| Socket._handleSuccessfulExecution() | NetworkFeeCollector | After execution status set | -| Socket._sendPayload() | Plug.overrides() | View function, no state change | -| Socket._verify() | Switchboard (allowPayload) | Before execution, read-only | -| SocketConfig.connect() | Switchboard.updatePlugConfig() | Plug is msg.sender | -| MessageSwitchboard.refund() | refundAddress | ReentrancyGuard, state updated first | +| Location | Called Contract | Protection | +| ------------------------------------ | ------------------------------ | ---------------------------------------------- | +| Socket.\_execute() | Plug (target) | Gas limit, tryCall, execution status set first | +| Socket.\_handleSuccessfulExecution() | NetworkFeeCollector | After execution status set | +| Socket.\_sendPayload() | Plug.overrides() | View function, no state change | +| Socket.\_verify() | Switchboard (allowPayload) | Before execution, read-only | +| SocketConfig.connect() | Switchboard.updatePlugConfig() | Plug is msg.sender | +| MessageSwitchboard.refund() | refundAddress | ReentrancyGuard, state updated first | **Key Risk**: Reentrancy through plug execution @@ -297,12 +335,12 @@ At execute(): msg.value >= executionParams.value + transmissionParams.socketFees ### 2. Value Transfer Points (High Risk) -| Location | Recipient | Amount | Condition | -|----------|-----------|--------|-----------| -| Socket._execute() | Plug | executionParams.value | During execution | -| Socket._handleSuccessfulExecution() | NetworkFeeCollector | socketFees | After successful execution | -| Socket._handleFailedExecution() | refundAddress/msg.sender | msg.value | On execution failure | -| MessageSwitchboard.refund() | fees.refundAddress | nativeFees | When refund eligible | +| Location | Recipient | Amount | Condition | +| ------------------------------------ | ------------------------ | --------------------- | -------------------------- | +| Socket.\_execute() | Plug | executionParams.value | During execution | +| Socket.\_handleSuccessfulExecution() | NetworkFeeCollector | socketFees | After successful execution | +| Socket.\_handleFailedExecution() | refundAddress/msg.sender | msg.value | On execution failure | +| MessageSwitchboard.refund() | fees.refundAddress | nativeFees | When refund eligible | **Key Risk**: Incorrect refund logic or missing reentrancy protection @@ -310,13 +348,13 @@ At execute(): msg.value >= executionParams.value + transmissionParams.socketFees ### 3. Signature Verification Points (Critical) -| Location | Signer Role | Digest Components | -|----------|-------------|-------------------| -| SwitchboardBase.getTransmitter() | Transmitter (optional) | socket address + payloadId | -| FastSwitchboard.attest() | WATCHER_ROLE | switchboard address + chainSlug + digest | -| MessageSwitchboard.attest() | WATCHER_ROLE | switchboard address + chainSlug + digest | -| MessageSwitchboard.markRefundEligible() | WATCHER_ROLE | switchboard + chainSlug + payloadId + nonce | -| MessageSwitchboard.setMinMsgValueFees() | FEE_UPDATER_ROLE | switchboard + chainSlug + params + nonce | +| Location | Signer Role | Digest Components | +| --------------------------------------- | ---------------------- | ------------------------------------------- | +| SwitchboardBase.getTransmitter() | Transmitter (optional) | socket address + payloadId | +| FastSwitchboard.attest() | WATCHER_ROLE | switchboard address + chainSlug + digest | +| MessageSwitchboard.attest() | WATCHER_ROLE | switchboard address + chainSlug + digest | +| MessageSwitchboard.markRefundEligible() | WATCHER_ROLE | switchboard + chainSlug + payloadId + nonce | +| MessageSwitchboard.setMinMsgValueFees() | FEE_UPDATER_ROLE | switchboard + chainSlug + params + nonce | **Key Risk**: Signature replay, malleability, or missing components in digest @@ -325,6 +363,7 @@ At execute(): msg.value >= executionParams.value + transmissionParams.socketFees ### 4. State Modification Points #### High Impact State Changes + - `executionStatus[payloadId]`: Replay protection - `payloadIdToDigest[payloadId]`: Parameter binding - `isAttested[digest]`: Authorization @@ -332,6 +371,7 @@ At execute(): msg.value >= executionParams.value + transmissionParams.socketFees - `usedNonces[signer][nonce]`: Replay protection #### Configuration Changes + - `switchboardStatus[id]`: Can disable verification path - `plugSwitchboardIds[plug]`: Routes plug to switchboard - `siblingPlugs[chain][plug]`: Controls source validation @@ -340,13 +380,13 @@ At execute(): msg.value >= executionParams.value + transmissionParams.socketFees ### 5. Arithmetic Operations -| Location | Operation | Overflow Risk | -|----------|-----------|---------------| -| Socket._execute() | gasLimit * gasLimitBuffer / 100 | Medium (large gasLimit) | -| MessageSwitchboard._increaseNativeFees() | nativeFees += msg.value | Low (Solidity 0.8+) | -| MessageSwitchboard.processPayload() | minFees + value | Low (Solidity 0.8+) | -| IdUtils.createPayloadId() | Bit shifting | None (explicit positions) | -| Payload counters | counter++ | Low (uint64 sufficient) | +| Location | Operation | Overflow Risk | +| ----------------------------------------- | -------------------------------- | ------------------------- | +| Socket.\_execute() | gasLimit \* gasLimitBuffer / 100 | Medium (large gasLimit) | +| MessageSwitchboard.\_increaseNativeFees() | nativeFees += msg.value | Low (Solidity 0.8+) | +| MessageSwitchboard.processPayload() | minFees + value | Low (Solidity 0.8+) | +| IdUtils.createPayloadId() | Bit shifting | None (explicit positions) | +| Payload counters | counter++ | Low (uint64 sufficient) | **Key Risk**: Gas limit arithmetic with extreme values @@ -354,10 +394,10 @@ At execute(): msg.value >= executionParams.value + transmissionParams.socketFees ### 6. Nonce Management -| Function | Nonce Space | Collision Risk | -|----------|-------------|----------------| -| markRefundEligible() | usedNonces[watcher][nonce] | Cross-function collision | -| setMinMsgValueFees() | usedNonces[feeUpdater][nonce] | Cross-function collision | +| Function | Nonce Space | Collision Risk | +| ------------------------- | ----------------------------- | ------------------------ | +| markRefundEligible() | usedNonces[watcher][nonce] | Cross-function collision | +| setMinMsgValueFees() | usedNonces[feeUpdater][nonce] | Cross-function collision | | setMinMsgValueFeesBatch() | usedNonces[feeUpdater][nonce] | Cross-function collision | **Key Risk**: Same nonce mapping shared across different function types @@ -367,37 +407,45 @@ At execute(): msg.value >= executionParams.value + transmissionParams.socketFees ## Known Limitations ### 1. Execution Ordering + - Payloads can be executed in any order - `prevBatchDigestHash` exists but not enforced on-chain - Applications must handle out-of-order execution ### 2. Deadline Granularity + - Deadlines use block.timestamp (manipulable by ±15 seconds) - Not suitable for time-critical applications requiring second-level precision ### 3. Gas Estimation + - Actual gas usage may vary from estimated gasLimit - gasLimitBuffer provides cushion but not guaranteed ### 4. Return Data Limitation + - Return data limited to maxCopyBytes (default 2048) - Large return data truncated with exceededMaxCopy flag ### 5. Finality Assumptions + - Protocol assumes source chain finality before attestation - Reorg on source chain could invalidate attested payloads - Watchers responsible for respecting finality ### 6. Switchboard Trust + - Socket trusts switchboard's allowPayload() decision - Malicious switchboard could authorize invalid payloads - Users must verify switchboard implementation before connecting ### 7. No Built-in Rate Limiting + - No on-chain rate limits for payload submission - Spam protection relies on fees and gas costs ### 8. Single Switchboard Per Plug + - Each plug connects to exactly one switchboard - Cannot use multiple switchboards simultaneously - Must disconnect and reconnect to switch @@ -407,6 +455,7 @@ At execute(): msg.value >= executionParams.value + transmissionParams.socketFees ## Security Assumptions Summary ### Must Hold for Security + 1. ✓ At least one honest watcher per payload 2. ✓ Watchers respect source chain finality 3. ✓ Switchboard verification logic is correct @@ -414,11 +463,13 @@ At execute(): msg.value >= executionParams.value + transmissionParams.socketFees 5. ✓ External contracts (Solady) are secure ### Design Tradeoffs + - **Flexibility vs. Complexity**: Multiple switchboard types increase attack surface - **Speed vs. Security**: FastSwitchboard trades off for speed - **Decentralization vs. Performance**: Watcher set must be managed ### Responsibility Boundaries + - **Protocol**: Routing, replay protection, digest verification - **Switchboards**: Attestation verification, source validation - **Plugs**: Application logic, parameter construction @@ -430,23 +481,28 @@ At execute(): msg.value >= executionParams.value + transmissionParams.socketFees ## Emergency Response Capabilities ### Immediate (PAUSER_ROLE) + - Pause Socket: Stops all `execute()` and `sendPayload()` operations - Existing in-flight payloads not affected ### Fast (SWITCHBOARD_DISABLER_ROLE) + - Disable specific switchboard: Prevents new connections - Existing connections remain but can be individually disconnected by plugs ### Governance (GOVERNANCE_ROLE) + - Re-enable disabled switchboards - Update fee collector (including setting to address(0) to disable) - Adjust gas parameters ### Fund Recovery (RESCUE_ROLE) + - Recover accidentally sent tokens/ETH - Cannot access user funds in proper flow ### No Emergency Stop For + - Cannot cancel already executed payloads - Cannot revoke attestations - Cannot modify past execution status @@ -457,6 +513,7 @@ At execute(): msg.value >= executionParams.value + transmissionParams.socketFees ## Threat Model Summary ### In Scope Threats + - ✓ Malicious plugs attempting reentrancy - ✓ Replay attacks on payloads - ✓ Signature replay attacks @@ -467,6 +524,7 @@ At execute(): msg.value >= executionParams.value + transmissionParams.socketFees - ✓ Nonce exhaustion attacks ### Out of Scope (Trusted Components) + - Watcher infrastructure security - Off-chain monitoring systems - EVMX chain implementation @@ -474,7 +532,7 @@ At execute(): msg.value >= executionParams.value + transmissionParams.socketFees - Network-level DOS attacks ### Partially In Scope + - Economic attacks (fee griefing) - mitigated by design - Front-running - limited impact due to commit-reveal via attestation - MEV extraction - not prevented but contained - diff --git a/auditor-docs/SETUP_GUIDE.md b/auditor-docs/SETUP_GUIDE.md index e39ee9a0..5034906a 100644 --- a/auditor-docs/SETUP_GUIDE.md +++ b/auditor-docs/SETUP_GUIDE.md @@ -5,12 +5,14 @@ ### Prerequisites **Required Software**: + - Node.js >= 18.x - Yarn or npm - Foundry (for Solidity testing) - Git **Installation Commands**: + ```bash # Install Node.js (if not installed) # Visit: https://nodejs.org/ @@ -30,6 +32,7 @@ cast --version ### Repository Setup **Clone and Install**: + ```bash # Clone repository git clone @@ -45,6 +48,7 @@ forge install ``` **Project Structure**: + ``` socket-protocol/ ├── contracts/ @@ -70,6 +74,7 @@ socket-protocol/ ### Using Foundry **Compile Contracts**: + ```bash # Clean previous build forge clean @@ -85,6 +90,7 @@ forge build --force ``` **Compilation Output**: + - Artifacts in: `out/` - Build info in: `artifacts/build-info/` @@ -93,6 +99,7 @@ forge build --force ### Using Hardhat **Compile Contracts**: + ```bash # Clean and compile npx hardhat clean @@ -103,6 +110,7 @@ npx hardhat compile contracts/protocol/Socket.sol ``` **Compilation Output**: + - Artifacts in: `artifacts/` - Typechain types in: `typechain-types/` @@ -113,6 +121,7 @@ npx hardhat compile contracts/protocol/Socket.sol ### Foundry Tests **Run All Tests**: + ```bash # Run all tests forge test @@ -131,6 +140,7 @@ forge test --match-test testExecuteSuccess ``` **Test Coverage**: + ```bash # Generate coverage report forge coverage @@ -148,6 +158,7 @@ open coverage/index.html ### Hardhat Tests **Run Tests**: + ```bash # Run all tests npx hardhat test @@ -166,6 +177,7 @@ REPORT_GAS=true npx hardhat test ### Slither **Installation**: + ```bash pip3 install slither-analyzer # or @@ -173,6 +185,7 @@ pip install slither-analyzer ``` **Run Analysis**: + ```bash # Analyze all contracts slither . @@ -192,6 +205,7 @@ slither . --exclude low,informational ### Mythril **Installation**: + ```bash pip3 install mythril # or via Docker @@ -199,6 +213,7 @@ docker pull mythril/myth ``` **Run Analysis**: + ```bash # Analyze contract myth analyze contracts/protocol/Socket.sol @@ -232,6 +247,7 @@ depth = 15 ``` **Key Settings**: + - Solidity version: 0.8.28 - Optimizer: Enabled with 200 runs - Fuzz runs: 256 (can be increased for thorough testing) @@ -254,6 +270,7 @@ forge-std/=lib/forge-std/src/ ### Environment Variables Create `.env` file: + ```bash # RPC URLs ETHEREUM_SEPOLIA_RPC=https://sepolia.infura.io/v3/YOUR_KEY @@ -274,6 +291,7 @@ ARBISCAN_API_KEY=your_arbiscan_key ### Deploy Socket **Using Foundry Script**: + ```bash # Deploy to testnet forge script script/deploy/DeploySocket.s.sol \ @@ -291,6 +309,7 @@ forge script script/deploy/DeploySocket.s.sol \ ### Deploy Switchboard **Using Foundry Script**: + ```bash # Deploy MessageSwitchboard forge script script/deploy/DeployMessageSwitchboard.s.sol \ @@ -310,15 +329,18 @@ forge script script/deploy/DeployFastSwitchboard.s.sol \ ### Deployment Parameters **Socket Deployment**: + - `chainSlug`: Unique chain identifier (e.g., 1 for Ethereum, 42161 for Arbitrum) - `owner`: Initial owner address (should be multi-sig in production) **MessageSwitchboard Deployment**: + - `chainSlug`: Same as Socket - `socket`: Address of deployed Socket contract - `owner`: Switchboard owner (can be same as Socket owner) **FastSwitchboard Deployment**: + - `chainSlug`: Same as Socket - `socket`: Address of deployed Socket contract - `owner`: Switchboard owner @@ -328,6 +350,7 @@ forge script script/deploy/DeployFastSwitchboard.s.sol \ ### Post-Deployment Configuration **1. Register Switchboard**: + ```solidity // Switchboard calls Socket socket.registerSwitchboard() @@ -335,11 +358,13 @@ socket.registerSwitchboard() ``` **2. Set EVMX Config (FastSwitchboard)**: + ```solidity fastSwitchboard.setEvmxConfig(evmxChainSlug, watcherId) ``` **3. Grant Roles**: + ```solidity // Grant WATCHER_ROLE to watcher addresses switchboard.grantRole(WATCHER_ROLE, watcherAddress) @@ -349,6 +374,7 @@ socket.grantRole(GOVERNANCE_ROLE, governanceAddress) ``` **4. Set Sibling Config (MessageSwitchboard)**: + ```solidity // Configure destination chains messageSwitchboard.setSiblingConfig( @@ -366,6 +392,7 @@ messageSwitchboard.setSiblingConfig( ### Verify on Etherscan **Using Foundry**: + ```bash forge verify-contract \ --chain-id 11155111 \ @@ -376,6 +403,7 @@ forge verify-contract \ ``` **Using Hardhat**: + ```bash npx hardhat verify \ --network sepolia \ @@ -390,6 +418,7 @@ npx hardhat verify \ ### Foundry **Inspect Contract**: + ```bash # Get contract size forge build --sizes @@ -402,6 +431,7 @@ cast 4byte 0x6a761202 ``` **Interact with Contracts**: + ```bash # Read contract cast call 0xSocketAddress "chainSlug()(uint32)" --rpc-url $RPC_URL @@ -418,6 +448,7 @@ cast logs --address 0xSocketAddress --rpc-url $RPC_URL ### Debugging **Run Tests with Traces**: + ```bash # Show execution traces forge test -vvvv @@ -430,12 +461,14 @@ forge test --debug testExecuteSuccess ``` **Forge Debugger**: + ```bash # Enter interactive debugger forge test --match-test testName --debug ``` **Commands in debugger**: + - `s` - step over - `n` - step into - `c` - continue @@ -448,20 +481,15 @@ forge test --match-test testName --debug ### Key Files for Audit **Priority 1 (Critical)**: + 1. `contracts/protocol/Socket.sol` - Main execution contract 2. `contracts/protocol/SocketUtils.sol` - Digest creation & verification 3. `contracts/protocol/switchboard/MessageSwitchboard.sol` - Full-featured switchboard 4. `contracts/protocol/switchboard/FastSwitchboard.sol` - Fast switchboard -**Priority 2 (Important)**: -5. `contracts/protocol/SocketConfig.sol` - Configuration management -6. `contracts/protocol/switchboard/SwitchboardBase.sol` - Base functionality -7. `contracts/utils/common/IdUtils.sol` - Payload ID utilities +**Priority 2 (Important)**: 5. `contracts/protocol/SocketConfig.sol` - Configuration management 6. `contracts/protocol/switchboard/SwitchboardBase.sol` - Base functionality 7. `contracts/utils/common/IdUtils.sol` - Payload ID utilities -**Priority 3 (Supporting)**: -8. `contracts/utils/OverrideParamsLib.sol` - Parameter builder -9. `contracts/utils/common/Structs.sol` - Data structures -10. `contracts/utils/common/Errors.sol` - Error definitions +**Priority 3 (Supporting)**: 8. `contracts/utils/OverrideParamsLib.sol` - Parameter builder 9. `contracts/utils/common/Structs.sol` - Data structures 10. `contracts/utils/common/Errors.sol` - Error definitions --- @@ -472,6 +500,7 @@ forge test --match-test testName --debug **Location**: `lib/solady/` **Key Used Modules**: + - `LibCall.sol` - Safe external call handling - `ECDSA.sol` - Signature verification - `SafeTransferLib.sol` - Safe ETH/token transfers @@ -495,12 +524,14 @@ forge test --match-test testName --debug ### Compilation Issues **Issue**: "Compiler version mismatch" + ```bash # Solution: Install correct version foundryup --version 0.8.28 ``` **Issue**: "Stack too deep" + ```bash # Solution: Enable via-ir forge build --via-ir @@ -511,12 +542,14 @@ forge build --via-ir ### Test Issues **Issue**: "Fuzz test failing intermittently" + ```bash # Solution: Increase runs or set specific seed forge test --fuzz-runs 1000 --fuzz-seed 42 ``` **Issue**: "Invariant test failing" + ```bash # Solution: Check invariant properties and increase depth forge test --invariant-runs 256 --invariant-depth 20 @@ -527,12 +560,14 @@ forge test --invariant-runs 256 --invariant-depth 20 ### RPC Issues **Issue**: "Rate limited" + ```bash # Solution: Use dedicated RPC endpoint or local node forge test --fork-url http://localhost:8545 ``` **Issue**: "Chain fork failing" + ```bash # Solution: Specify block number forge test --fork-url $RPC_URL --fork-block-number 12345678 @@ -545,6 +580,7 @@ forge test --fork-url $RPC_URL --fork-block-number 12345678 ### Contract Addresses (Example Testnet) **Sepolia**: + ``` Socket: 0x... (to be deployed) MessageSwitchboard: 0x... (to be deployed) @@ -552,6 +588,7 @@ FastSwitchboard: 0x... (to be deployed) ``` **Arbitrum Sepolia**: + ``` Socket: 0x... (to be deployed) MessageSwitchboard: 0x... (to be deployed) @@ -562,6 +599,7 @@ MessageSwitchboard: 0x... (to be deployed) ### Role Addresses **Production Setup Recommendation**: + - Owner: Multi-sig wallet (e.g., Gnosis Safe) - GOVERNANCE_ROLE: DAO/Multi-sig - WATCHER_ROLE: Off-chain watcher nodes (multiple) @@ -575,15 +613,17 @@ MessageSwitchboard: 0x... (to be deployed) ## Additional Resources **Documentation**: + - Solidity Docs: https://docs.soliditylang.org/ - Foundry Book: https://book.getfoundry.sh/ - Solady Docs: https://github.com/Vectorized/solady **Security Resources**: + - Smart Contract Security Best Practices: https://consensys.github.io/smart-contract-best-practices/ - DeFi Security Tools: https://github.com/crytic/building-secure-contracts **Questions?** + - Open issue in repository - Contact: [team contact info] - diff --git a/auditor-docs/SYSTEM_OVERVIEW.md b/auditor-docs/SYSTEM_OVERVIEW.md index 40c1d596..fc60be98 100644 --- a/auditor-docs/SYSTEM_OVERVIEW.md +++ b/auditor-docs/SYSTEM_OVERVIEW.md @@ -52,12 +52,14 @@ Socket Protocol is a cross-chain messaging infrastructure that enables secure co ## Core Components ### 1. Socket (Entry Point) + - Central contract on each chain - Handles payload execution (inbound) and submission (outbound) - Manages plug connections to switchboards - Enforces execution rules (deadlines, gas limits, replay protection) ### 2. Switchboards (Verification Layer) + - Verify payload authenticity through attestations - Multiple implementations: - **FastSwitchboard**: EVMX-based fast finality @@ -66,18 +68,21 @@ Socket Protocol is a cross-chain messaging infrastructure that enables secure co - Maintain plug configurations and routing information ### 3. Plugs (Application Layer) + - User-deployed smart contracts - Connect to Socket via specific switchboard - Implement application logic for cross-chain interactions - Call `socket.sendPayload()` to initiate cross-chain messages ### 4. Watchers (Off-Chain) + - Monitor source chain for payload requests - Attest to payloads on destination chain - Sign attestations for switchboard verification - NOT in audit scope (off-chain infrastructure) ### 5. Transmitters (Off-Chain) + - Deliver payloads to destination chain - Call `socket.execute()` with execution parameters - Optionally sign for additional verification @@ -88,24 +93,29 @@ Socket Protocol is a cross-chain messaging infrastructure that enables secure co ### System Assumptions 1. **Switchboards are trusted by Plugs/Apps** + - Anyone can register as a switchboard - Plugs only connect to switchboards they trust - Users verify switchboard implementation before connecting 2. **NetworkFeeCollector is trusted by Socket** + - Socket calls networkFeeCollector after successful execution - No reentrancy concerns as collector is trusted 3. **Target Plugs are trusted by Source Plugs** + - Source plugs specify and trust their sibling plugs on destination chains - Invalid target plugs only affect the plug that configured them 4. **simulate() function is for off-chain use only** + - Gated by OFF_CHAIN_CALLER address (0xDEAD) - Only used by off-chain services for gas estimation - Not accessible on mainnet 5. **Watchers act honestly** + - At least one honest watcher per payload - Watchers verify source chain state correctly - Watchers respect finality before attesting @@ -118,52 +128,64 @@ Socket Protocol is a cross-chain messaging infrastructure that enables secure co ## Key Design Decisions ### Modular Switchboard Architecture + **Decision**: Socket delegates verification to pluggable switchboard contracts rather than implementing verification directly. -**Rationale**: +**Rationale**: + - Different applications need different security/speed tradeoffs - Allows upgrading verification mechanisms without changing core Socket - Enables competition between verification methods ### Payload ID Structure + **Decision**: Payload IDs encode source, verification, and counter information in a single bytes32. **Format**: `[Source: 64 bits][Verification: 64 bits][Counter: 64 bits][Reserved: 64 bits]` **Rationale**: + - Uniquely identifies payloads across all chains - Enables routing validation (correct source → correct destination) - Self-describing without additional lookups ### Two-Phase Execution + **Decision**: Separate payload creation (source chain) from execution (destination chain). **Rationale**: + - Asynchronous cross-chain messaging - Allows off-chain attestation layer - Enables retry mechanisms and fee adjustments ### Digest-Based Verification + **Decision**: All execution parameters are hashed into a digest that switchboards attest to. **Rationale**: + - Single attestation covers all parameters - Prevents parameter manipulation after attestation - Length-prefixed encoding prevents collision attacks ### One-Time Execution + **Decision**: Payloads can only be executed once, even if they fail. **Rationale**: + - Simplicity: No complex retry logic needed - Determinism: Clear finality for each payload - Security: Prevents replay attacks and complex re-execution scenarios - Application Layer: Apps can send new payloads if needed ### No Ordering Enforcement + **Decision**: Payloads can execute in any order on destination chain. **Rationale**: + - Cross-chain messaging is inherently asynchronous - Different chain finality times make ordering impractical - Transmitter competition for fees @@ -172,6 +194,7 @@ Socket Protocol is a cross-chain messaging infrastructure that enables secure co ## Scope Boundaries ### In Scope (Smart Contracts) + - ✅ Socket.sol - Main execution contract - ✅ SocketUtils.sol - Utility functions - ✅ SocketConfig.sol - Configuration management @@ -182,6 +205,7 @@ Socket Protocol is a cross-chain messaging infrastructure that enables secure co - ✅ OverrideParamsLib.sol - Parameter builder library ### Out of Scope + - ❌ Off-chain watcher infrastructure - ❌ Off-chain transmitter infrastructure - ❌ Frontend/API layers @@ -192,6 +216,7 @@ Socket Protocol is a cross-chain messaging infrastructure that enables secure co ## Security Properties ### Critical Invariants (Must Always Hold) + 1. ✓ Each payload executes at most once 2. ✓ Execution status transitions are one-way (cannot revert from Executed) 3. ✓ Digests are immutable once stored @@ -201,6 +226,7 @@ Socket Protocol is a cross-chain messaging infrastructure that enables secure co 7. ✓ Source validation prevents unauthorized executions ### Design Patterns Used + - ✅ **Checks-Effects-Interactions (CEI)**: State updated before external calls - ✅ **Replay Protection**: executionStatus prevents double execution - ✅ **Nonce Management**: Namespace-isolated nonces prevent cross-function replay @@ -218,12 +244,14 @@ Socket Protocol is a cross-chain messaging infrastructure that enables secure co ## Integration Points ### For Plug Developers + 1. Deploy plug contract 2. Call `socket.connect(switchboardId, config)` 3. Send payloads via `socket.sendPayload(data)` or fallback 4. Implement inbound handler for receiving executions ### For Switchboard Developers + 1. Inherit from `SwitchboardBase` 2. Implement `allowPayload()` verification logic 3. Implement `processPayload()` for outbound handling @@ -240,12 +268,14 @@ Socket Protocol is a cross-chain messaging infrastructure that enables secure co ## Economic Model ### Fee Flows + - **Socket Fees**: Paid to transmitters/protocol for execution - **Native Fees**: Paid in ETH on source chain - **Sponsored Fees**: Pre-approved spending by sponsor accounts - **Refunds**: Eligible if payload never executed (watcher-approved) ### Fee Management + - Fees tracked per payload - Can be increased before execution - Refund mechanism for failed deliveries diff --git a/auditor-docs/TESTING_COVERAGE.md b/auditor-docs/TESTING_COVERAGE.md index 5b61faba..261fbe6d 100644 --- a/auditor-docs/TESTING_COVERAGE.md +++ b/auditor-docs/TESTING_COVERAGE.md @@ -28,6 +28,7 @@ test/ ### Socket.sol Tests **execute() Function**: + - ✅ Successful execution flow - ✅ Deadline validation (reverts if passed) - ✅ Call type validation (only WRITE allowed) @@ -39,6 +40,7 @@ test/ - ✅ Execution with network fee collection **sendPayload() Function**: + - ✅ Basic payload submission - ✅ Plug disconnected scenario - ✅ Paused contract scenario @@ -46,6 +48,7 @@ test/ - ✅ Fallback function alternative **State Management**: + - ✅ executionStatus transitions - ✅ payloadIdToDigest storage - ✅ Pause/unpause functionality @@ -55,12 +58,14 @@ test/ ### SocketConfig.sol Tests **Switchboard Registration**: + - ✅ Register new switchboard - ✅ Duplicate registration prevention - ✅ Counter increment verification - ✅ Status set to REGISTERED **Plug Connection/Disconnection**: + - ✅ Connect to valid switchboard - ✅ Connect with configuration - ✅ Connect to invalid/disabled switchboard (reverts) @@ -68,11 +73,13 @@ test/ - ✅ Disconnect when not connected (reverts) **Switchboard Management**: + - ✅ Disable switchboard (authorized) - ✅ Enable switchboard (authorized) - ✅ Access control enforcement **Parameter Updates**: + - ✅ Set gas limit buffer (valid values) - ✅ Set max copy bytes - ✅ Set network fee collector @@ -82,6 +89,7 @@ test/ ### MessageSwitchboard.sol Tests **processPayload()**: + - ✅ Native fee flow (version 1) - ✅ Sponsored fee flow (version 2) - ✅ Sibling validation @@ -91,6 +99,7 @@ test/ - ✅ Payload counter increment **Attestation**: + - ✅ Valid watcher attestation - ✅ Invalid watcher (no role) rejection - ✅ Double attestation prevention @@ -98,6 +107,7 @@ test/ - ✅ Source validation in allowPayload **Fee Management**: + - ✅ Increase native fees - ✅ Increase sponsored fees - ✅ Unauthorized fee increase (reverts) @@ -106,6 +116,7 @@ test/ - ✅ Refund double-claim prevention **Configuration**: + - ✅ Set sibling config - ✅ Update plug config - ✅ Sponsor approve/revoke plug @@ -117,6 +128,7 @@ test/ ### FastSwitchboard.sol Tests **processPayload()**: + - ✅ Basic payload creation - ✅ EVMX config validation - ✅ Deadline handling @@ -124,11 +136,13 @@ test/ - ✅ payloadIdToPlug mapping **Attestation**: + - ✅ Valid watcher attestation - ✅ Invalid watcher rejection - ✅ allowPayload with app gateway validation **Configuration**: + - ✅ Set EVMX config - ✅ Update plug config (app gateway) - ✅ Set default deadline @@ -138,6 +152,7 @@ test/ ### Integration Tests **End-to-End Flows**: + - ✅ Full outbound flow: plug → socket → switchboard → event - ✅ Full inbound flow: execute → verify → call plug → fees - ✅ Cross-switchboard scenarios @@ -148,16 +163,19 @@ test/ ## Coverage Metrics **Overall Coverage** (estimated): + - Line Coverage: ~85% - Branch Coverage: ~80% - Function Coverage: ~90% **High Coverage Areas**: + - Core execution logic: >95% - Access control: >90% - State transitions: >90% **Lower Coverage Areas**: + - Edge cases with extreme values: ~60% - Complex error conditions: ~70% - Rare configuration scenarios: ~65% @@ -171,6 +189,7 @@ test/ #### Reentrancy Attack Tests **Test 1: Reentrant Plug During Execution** + ```solidity // Scenario: Malicious plug calls back into Socket contract MaliciousPlug { @@ -187,6 +206,7 @@ contract MaliciousPlug { --- **Test 2: Reentrant Fee Collection** + ```solidity // Scenario: NetworkFeeCollector attempts reentrancy contract MaliciousFeeCollector { @@ -202,12 +222,13 @@ contract MaliciousFeeCollector { --- **Test 3: Reentrant Refund Recipient** + ```solidity // Scenario: Refund recipient attempts reentrancy contract MaliciousRefundRecipient { - receive() external payable { - messageSwitchboard.refund(payloadId); // Should fail - } + receive() external payable { + messageSwitchboard.refund(payloadId); // Should fail + } } ``` @@ -218,6 +239,7 @@ contract MaliciousRefundRecipient { #### Gas Limit Edge Cases **Test 4: Maximum Gas Limit** + ```solidity // Execute with gasLimit = type(uint256).max executionParams.gasLimit = type(uint256).max; @@ -228,6 +250,7 @@ executionParams.gasLimit = type(uint256).max; --- **Test 5: Zero Gas Limit** + ```solidity // Execute with gasLimit = 0 executionParams.gasLimit = 0; @@ -238,6 +261,7 @@ executionParams.gasLimit = 0; --- **Test 6: Gas Limit Overflow in Calculation** + ```solidity // gasLimit * gasLimitBuffer might overflow executionParams.gasLimit = type(uint256).max / 104; // Just under overflow @@ -248,6 +272,7 @@ executionParams.gasLimit = type(uint256).max / 104; // Just under overflow --- **Test 7: Exact Gas Boundary** + ```solidity // Provide exactly the required gas (no buffer) uint256 exactGas = (executionParams.gasLimit * gasLimitBuffer) / 100; @@ -260,6 +285,7 @@ uint256 exactGas = (executionParams.gasLimit * gasLimitBuffer) / 100; #### Value Handling Tests **Test 8: Exact msg.value Requirement** + ```solidity // msg.value = executionParams.value + socketFees (exact) ``` @@ -269,6 +295,7 @@ uint256 exactGas = (executionParams.gasLimit * gasLimitBuffer) / 100; --- **Test 9: Excess msg.value** + ```solidity // msg.value > executionParams.value + socketFees ``` @@ -278,6 +305,7 @@ uint256 exactGas = (executionParams.gasLimit * gasLimitBuffer) / 100; --- **Test 10: Failed Execution Refund Recipient** + ```solidity // Test with refundAddress = address(0) // Test with refundAddress = valid address @@ -293,6 +321,7 @@ uint256 exactGas = (executionParams.gasLimit * gasLimitBuffer) / 100; #### Signature Replay Tests **Test 11: Cross-Chain Signature Replay** + ```solidity // Use same signature on different chain (if multi-chain deployment) ``` @@ -302,6 +331,7 @@ uint256 exactGas = (executionParams.gasLimit * gasLimitBuffer) / 100; --- **Test 12: Cross-Function Nonce Replay** + ```solidity // Use nonce from markRefundEligible in setMinMsgValueFees watcher signs: markRefundEligible(payloadId, nonce=1, sig) @@ -313,6 +343,7 @@ watcher signs: markRefundEligible(payloadId, nonce=1, sig) --- **Test 13: Attestation Signature Malleability** + ```solidity // Try (r, s) and (r, -s) signature variants ``` @@ -322,6 +353,7 @@ watcher signs: markRefundEligible(payloadId, nonce=1, sig) --- **Test 14: Invalid Signature Format** + ```solidity // Provide signature with wrong length // Provide all-zero signature @@ -336,6 +368,7 @@ watcher signs: markRefundEligible(payloadId, nonce=1, sig) #### Execution Status Tests **Test 15: Concurrent Execution Attempts** + ```solidity // Two transmitters try to execute same payloadId in same block // (Requires forking or simulation) @@ -346,6 +379,7 @@ watcher signs: markRefundEligible(payloadId, nonce=1, sig) --- **Test 16: Execute After Reverted** + ```solidity // First execution fails (sets status to Reverted) // Attempt second execution @@ -356,6 +390,7 @@ watcher signs: markRefundEligible(payloadId, nonce=1, sig) --- **Test 17: Status Transition Validation** + ```solidity // Verify status can only transition: // NotExecuted → Executed @@ -369,6 +404,7 @@ watcher signs: markRefundEligible(payloadId, nonce=1, sig) #### Fee Accounting Tests **Test 18: Fee Increase Overflow** + ```solidity // Set nativeFees to near max payloadFees[id].nativeFees = type(uint256).max - 100; @@ -381,6 +417,7 @@ increaseFeesForPayload{value: 200}(...); --- **Test 19: Fee Accounting Conservation** + ```solidity // Track: total ETH in contract = sum of all payloadFees // After refund: verify conservation @@ -392,6 +429,7 @@ increaseFeesForPayload{value: 200}(...); --- **Test 20: Refund Edge Cases** + ```solidity // Refund with nativeFees = 0 (should revert) // Refund when not eligible (should revert) @@ -406,6 +444,7 @@ increaseFeesForPayload{value: 200}(...); #### Switchboard Management Tests **Test 21: Connect to Disabled Switchboard** + ```solidity // Register switchboard // Disable switchboard @@ -417,6 +456,7 @@ increaseFeesForPayload{value: 200}(...); --- **Test 22: Execute with Disabled Switchboard** + ```solidity // Plug connected to switchboard // Switchboard gets disabled @@ -428,6 +468,7 @@ increaseFeesForPayload{value: 200}(...); --- **Test 23: EOA as Switchboard** + ```solidity // Register EOA as switchboard // Plug connects to it @@ -441,6 +482,7 @@ increaseFeesForPayload{value: 200}(...); #### Role Management Tests **Test 24: Role Escalation Attempt** + ```solidity // Non-admin tries to grant themselves admin role // Non-watcher tries to attest @@ -451,6 +493,7 @@ increaseFeesForPayload{value: 200}(...); --- **Test 25: Role Transfer** + ```solidity // Transfer ownership // Old owner can no longer perform owner actions @@ -464,6 +507,7 @@ increaseFeesForPayload{value: 200}(...); #### Payload ID Tests **Test 26: Payload ID Collision** + ```solidity // Create many payloads, check for duplicate IDs // With same source/dest but different counters @@ -474,6 +518,7 @@ increaseFeesForPayload{value: 200}(...); --- **Test 27: Payload ID Routing Validation** + ```solidity // Create payload for chainA // Attempt to execute on chainB @@ -484,6 +529,7 @@ increaseFeesForPayload{value: 200}(...); --- **Test 28: Counter Boundary** + ```solidity // Set counter to near max (type(uint64).max - 1) // Create multiple payloads @@ -496,6 +542,7 @@ increaseFeesForPayload{value: 200}(...); #### Source Validation Tests **Test 29: Invalid Source Format** + ```solidity // Provide source with wrong encoding // Provide source with wrong length @@ -506,6 +553,7 @@ increaseFeesForPayload{value: 200}(...); --- **Test 30: Source Mismatch** + ```solidity // Payload from plugA on chainX // Source claims plugB on chainY @@ -520,6 +568,7 @@ increaseFeesForPayload{value: 200}(...); #### Socket ↔ Switchboard Tests **Test 31: Malicious Switchboard** + ```solidity // Switchboard always returns true for allowPayload // Switchboard returns address(0) for getTransmitter @@ -531,6 +580,7 @@ increaseFeesForPayload{value: 200}(...); --- **Test 32: Switchboard State Inconsistency** + ```solidity // Switchboard says payload is attested // But never actually called attest() @@ -543,6 +593,7 @@ increaseFeesForPayload{value: 200}(...); #### Socket ↔ Plug Tests **Test 33: Plug Always Reverts** + ```solidity // Plug.inbound() always reverts // Multiple payloads to same plug @@ -553,6 +604,7 @@ increaseFeesForPayload{value: 200}(...); --- **Test 34: Plug Consumes All Gas** + ```solidity // Plug has infinite loop or expensive operation ``` @@ -562,6 +614,7 @@ increaseFeesForPayload{value: 200}(...); --- **Test 35: Plug Returns Large Data** + ```solidity // Plug returns data > maxCopyBytes ``` @@ -573,6 +626,7 @@ increaseFeesForPayload{value: 200}(...); ### Priority 7: Economic & Incentive Tests **Test 36: Fee Griefing** + ```solidity // Attacker creates many payloads with minimum fee // Clogs system or causes transmitter losses @@ -583,6 +637,7 @@ increaseFeesForPayload{value: 200}(...); --- **Test 37: Transmitter Competition** + ```solidity // Multiple transmitters race to execute // First gets reward @@ -593,6 +648,7 @@ increaseFeesForPayload{value: 200}(...); --- **Test 38: Sponsor Approval Manipulation** + ```solidity // Sponsor approves plug // Plug creates sponsored payload @@ -608,28 +664,31 @@ increaseFeesForPayload{value: 200}(...); ### Critical Invariants **Invariant 1: Execution Uniqueness** + ```solidity // Property: ∀ payloadId, executed at most once function invariant_executionUniqueness() public { - // Track all executed payloadIds - // Verify no duplicates + // Track all executed payloadIds + // Verify no duplicates } ``` --- **Invariant 2: Fee Conservation** + ```solidity // Property: Total ETH in = Total ETH out + Contract Balance function invariant_feeConservation() public { - // Sum all payloadFees.nativeFees - // Should equal contract balance + // Sum all payloadFees.nativeFees + // Should equal contract balance } ``` --- **Invariant 3: Refund Single-Use** + ```solidity // Property: If isRefunded = true, then nativeFees = 0 function invariant_refundSingleUse() public { @@ -641,22 +700,24 @@ function invariant_refundSingleUse() public { --- **Invariant 4: Status Monotonicity** + ```solidity // Property: Status never regresses function invariant_statusMonotonic() public { - // NotExecuted can → Executed or Reverted - // Executed/Reverted never change + // NotExecuted can → Executed or Reverted + // Executed/Reverted never change } ``` --- **Invariant 5: Counter Monotonicity** + ```solidity // Property: Counters only increase function invariant_counterMonotonic() public { - // payloadCounter only increases - // switchboardIdCounter only increases + // payloadCounter only increases + // switchboardIdCounter only increases } ``` @@ -667,6 +728,7 @@ function invariant_counterMonotonic() public { ### Fuzz Testing Configuration **Foundry Fuzzing**: + ```toml [profile.default.fuzz] runs = 10000 @@ -678,40 +740,43 @@ max_test_rejects = 100000 ### Key Fuzz Targets **Fuzz 1: execute() Parameters** + ```solidity function testFuzz_execute( - uint256 gasLimit, - uint256 value, - uint256 deadline, - uint256 socketFees + uint256 gasLimit, + uint256 value, + uint256 deadline, + uint256 socketFees ) public { - // Bound inputs to reasonable ranges - gasLimit = bound(gasLimit, 0, 10_000_000); - value = bound(value, 0, 100 ether); - deadline = bound(deadline, block.timestamp, block.timestamp + 365 days); - socketFees = bound(socketFees, 0, 10 ether); - - // Test execution with fuzzed params + // Bound inputs to reasonable ranges + gasLimit = bound(gasLimit, 0, 10_000_000); + value = bound(value, 0, 100 ether); + deadline = bound(deadline, block.timestamp, block.timestamp + 365 days); + socketFees = bound(socketFees, 0, 10 ether); + + // Test execution with fuzzed params } ``` --- **Fuzz 2: Digest Creation** + ```solidity function testFuzz_digestCreation( - bytes calldata payload, - bytes calldata source, - bytes calldata extraData + bytes calldata payload, + bytes calldata source, + bytes calldata extraData ) public { - // Test digest with various lengths and content - // Should always produce deterministic hash + // Test digest with various lengths and content + // Should always produce deterministic hash } ``` --- **Fuzz 3: Payload ID Encoding/Decoding** + ```solidity function testFuzz_payloadId( uint32 srcChain, @@ -732,18 +797,22 @@ function testFuzz_payloadId( ### Known Gaps 1. **Limited Gas Exhaustion Testing** + - Need more tests with boundary gas values - Test gas griefing scenarios 2. **Cross-Chain Replay Scenarios** + - If deployed on multiple chains, test signature replay - Test chainSlug protections 3. **Race Condition Coverage** + - Limited concurrent transaction testing - Need forking tests for realistic conditions 4. **Economic Attack Vectors** + - Fee manipulation strategies - Transmitter incentive edge cases @@ -783,26 +852,31 @@ function testFuzz_payloadId( ## Test Execution Guide ### Run All Tests + ```bash forge test ``` ### Run with Coverage + ```bash forge coverage ``` ### Run Specific Test Suite + ```bash forge test --match-path test/Socket.t.sol ``` ### Run Fuzz Tests with High Runs + ```bash forge test --fuzz-runs 10000 ``` ### Run Invariant Tests + ```bash forge test --match-test invariant ``` @@ -812,18 +886,20 @@ forge test --match-test invariant ## Expected Test Outcomes ### All Tests Should Pass + - ✅ Unit tests: 100% pass rate - ✅ Integration tests: 100% pass rate - ✅ Invariant tests: No violations - ✅ Fuzz tests: No unexpected failures ### Coverage Targets + - 📊 Line coverage: >90% - 📊 Branch coverage: >85% - 📊 Function coverage: >95% ### Performance Benchmarks + - ⚡ execute() gas: <300k gas - ⚡ sendPayload() gas: <200k gas - ⚡ attest() gas: <100k gas - diff --git a/contracts/evmx/base/AppGatewayBase.sol b/contracts/evmx/base/AppGatewayBase.sol index 49bd8bc8..576538ad 100644 --- a/contracts/evmx/base/AppGatewayBase.sol +++ b/contracts/evmx/base/AppGatewayBase.sol @@ -112,7 +112,8 @@ abstract contract AppGatewayBase is AddressResolverUtil, IAppGateway { } // todo: different for solana, need to handle here - onChainAddress = IForwarder(forwarderAddresses[contractId_][chainSlug_]).getOnChainAddress(); + onChainAddress = IForwarder(forwarderAddresses[contractId_][chainSlug_]) + .getOnChainAddress(); } //////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/contracts/evmx/helpers/ForwarderSolana.sol b/contracts/evmx/helpers/ForwarderSolana.sol index eb3e5820..24667a0a 100644 --- a/contracts/evmx/helpers/ForwarderSolana.sol +++ b/contracts/evmx/helpers/ForwarderSolana.sol @@ -29,10 +29,7 @@ contract ForwarderSolana is ForwarderStorage, Initializable, AddressResolverUtil /// that handles calls to multiple Solana contracts. /// @param chainSlug_ chain slug on which the contract is deployed /// @param addressResolver_ address resolver contract - function initialize( - uint32 chainSlug_, - address addressResolver_ - ) public initializer { + function initialize(uint32 chainSlug_, address addressResolver_) public initializer { if (chainSlug_ == CHAIN_SLUG_SOLANA_MAINNET || chainSlug_ == CHAIN_SLUG_SOLANA_DEVNET) { chainSlug = chainSlug_; } else { @@ -66,10 +63,7 @@ contract ForwarderSolana is ForwarderStorage, Initializable, AddressResolverUtil /// @notice Fallback function to process the contract calls to onChainAddress /// @dev It queues the calls in the middleware and deploys the promise contract - function callSolana( - bytes memory solanaPayload, - bytes32 targetContract - ) external { + function callSolana(bytes memory solanaPayload, bytes32 targetContract) external { if (address(addressResolver__) == address(0)) { revert AddressResolverNotSet(); } diff --git a/contracts/protocol/Socket.sol b/contracts/protocol/Socket.sol index 50defdce..7891adff 100644 --- a/contracts/protocol/Socket.sol +++ b/contracts/protocol/Socket.sol @@ -29,8 +29,12 @@ contract Socket is SocketUtils { * @param gasLimitBuffer_ The gas limit buffer percentage (e.g., 105 = 5% buffer) * @param maxCopyBytes_ The maximum bytes to copy from return data (default: 2048 = 2KB) */ - constructor(uint32 chainSlug_, address owner_, uint256 gasLimitBuffer_, uint16 maxCopyBytes_) SocketUtils(chainSlug_, owner_, gasLimitBuffer_, maxCopyBytes_) { - } + constructor( + uint32 chainSlug_, + address owner_, + uint256 gasLimitBuffer_, + uint16 maxCopyBytes_ + ) SocketUtils(chainSlug_, owner_, gasLimitBuffer_, maxCopyBytes_) {} /** * @notice Executes a payload that has been delivered by transmitters and authenticated by switchboards @@ -46,7 +50,6 @@ contract Socket is SocketUtils { ExecutionParams memory executionParams_, TransmissionParams calldata transmissionParams_ ) external payable whenNotPaused returns (bool, bytes memory) { - if (executionParams_.deadline < block.timestamp) revert DeadlinePassed(); if (executionParams_.callType != WRITE) revert InvalidCallType(); @@ -241,7 +244,7 @@ contract Socket is SocketUtils { /** * @notice Fallback function that forwards all calls to Socket's sendPayload * @return ABI encoded payload ID as bytes - * @dev The calldata is passed as-is to the switchboard. + * @dev The calldata is passed as-is to the switchboard. * Solidity does not ABI-encode dynamic returns in fallback functions. The fallback return is raw returndata, so we must manually wrap a `bytes32` into ABI-encoded `bytes` (offset + length + data). * We use double encoding: `abi.encode(abi.encode(payloadId))` to create proper ABI structure. * @dev If using .call() ((bool success, bytes memory returnData) = address(socket).call(payload)), returnData will be raw returndata, so we need to decode twice to get the payloadId. @@ -249,12 +252,12 @@ contract Socket is SocketUtils { */ fallback(bytes calldata) external payable returns (bytes memory) { bytes32 payloadId = _sendPayload(msg.sender, msg.value, msg.data); - + // same as return abi.encode(abi.encode(payloadId)); uses less gas - uint256 offset = 0x20; // Points to length field (32 bytes from start) - uint256 length = 0x20; // Length of bytes32 is 32 bytes + uint256 offset = 0x20; // Points to length field (32 bytes from start) + uint256 length = 0x20; // Length of bytes32 is 32 bytes return abi.encodePacked(offset, length, payloadId); - } + } /** * @notice Reverts when ETH is sent directly to the contract diff --git a/contracts/protocol/SocketConfig.sol b/contracts/protocol/SocketConfig.sol index 4e162fb5..2cee9a81 100644 --- a/contracts/protocol/SocketConfig.sol +++ b/contracts/protocol/SocketConfig.sol @@ -143,7 +143,7 @@ abstract contract SocketConfig is ISocket, AccessControl, Pausable { bytes memory extraData_ ) external view returns (uint32 switchboardId, bytes memory plugConfig) { switchboardId = plugSwitchboardIds[plugAddress_]; - if (switchboardId==0) return (0, bytes("")); + if (switchboardId == 0) return (0, bytes("")); plugConfig = ISwitchboard(switchboardAddresses[switchboardId]).getPlugConfig( plugAddress_, extraData_ diff --git a/contracts/protocol/SocketUtils.sol b/contracts/protocol/SocketUtils.sol index 77504129..08c84f6b 100644 --- a/contracts/protocol/SocketUtils.sol +++ b/contracts/protocol/SocketUtils.sol @@ -23,7 +23,7 @@ abstract contract SocketUtils is SocketConfig { /// @notice Maximum bytes to copy from return data (default: 2048 = 2KB) /// @dev Prevents unbounded return data attacks by limiting copied bytes - uint16 public immutable maxCopyBytes; + uint16 public immutable maxCopyBytes; /// @notice Gas limit buffer percentage (e.g., 105 = 5% buffer) /// @dev Accounts for gas used by current contract execution overhead @@ -58,7 +58,7 @@ abstract contract SocketUtils is SocketConfig { * @return The keccak256 hash of the encoded payload * @dev Creates a deterministic digest from all execution parameters. Uses length prefixes for variable-length fields * (payload, source, extraData) to prevent collision attacks. Fixed-size fields are packed directly, - * variable fields are prefixed with their length. using encodePacked instead of encode for bytes fields + * variable fields are prefixed with their length. using encodePacked instead of encode for bytes fields * to make it cross-chain compatible. */ function _createDigest( diff --git a/contracts/protocol/base/MessagePlugBase.sol b/contracts/protocol/base/MessagePlugBase.sol index 04ec8330..680fc317 100644 --- a/contracts/protocol/base/MessagePlugBase.sol +++ b/contracts/protocol/base/MessagePlugBase.sol @@ -24,7 +24,7 @@ abstract contract MessagePlugBase is PlugBase { constructor(address socket_, uint32 switchboardId_) { if (socket_ == address(0)) revert InvalidSocket(); if (switchboardId_ == 0) revert InvalidSwitchboardId(); - + _setSocket(socket_); switchboardId = switchboardId_; switchboard = socket__.switchboardAddresses(switchboardId_); diff --git a/contracts/protocol/switchboard/EVMxSwitchboard.sol b/contracts/protocol/switchboard/EVMxSwitchboard.sol index 9b619729..230a3221 100644 --- a/contracts/protocol/switchboard/EVMxSwitchboard.sol +++ b/contracts/protocol/switchboard/EVMxSwitchboard.sol @@ -22,8 +22,8 @@ contract EVMxSwitchboard is SwitchboardBase { uint32 public immutable watcherId; /// @notice Counter for generating unique payload IDs - /// @dev If we deploy a new set of Socket contracts, we need to start counter from last value to avoid - /// replay attacks. + /// @dev If we deploy a new set of Socket contracts, we need to start counter from last value to avoid + /// replay attacks. uint64 public payloadCounter; /// @notice Default deadline for payload execution (1 day) @@ -151,11 +151,11 @@ contract EVMxSwitchboard is SwitchboardBase { if (evmxChainSlug == 0 || watcherId == 0) revert EvmxConfigNotSet(); bytes memory overrides; - if (overrides_.length==0) { + if (overrides_.length == 0) { overrides = abi.encode(block.timestamp + defaultDeadline); } else { uint256 deadline = abi.decode(overrides_, (uint256)); - overrides = abi.encode(block.timestamp + (deadline > 0 ? deadline: defaultDeadline)); + overrides = abi.encode(block.timestamp + (deadline > 0 ? deadline : defaultDeadline)); } payloadId = createPayloadId( diff --git a/contracts/utils/common/Constants.sol b/contracts/utils/common/Constants.sol index 38e21368..747def8b 100644 --- a/contracts/utils/common/Constants.sol +++ b/contracts/utils/common/Constants.sol @@ -16,4 +16,3 @@ uint256 constant GAS_LIMIT_BUFFER = 105; // 5% buffer uint32 constant CHAIN_SLUG_SOLANA_MAINNET = 10000001; uint32 constant CHAIN_SLUG_SOLANA_DEVNET = 10000002; - diff --git a/contracts/utils/common/Errors.sol b/contracts/utils/common/Errors.sol index 2cb8612c..e9c9068a 100644 --- a/contracts/utils/common/Errors.sol +++ b/contracts/utils/common/Errors.sol @@ -164,4 +164,4 @@ error InvalidSwitchboardId(); /// @notice Thrown when role is invalid error InvalidRole(); /// @notice Thrown when watcher is already found -error WatcherFound(); \ No newline at end of file +error WatcherFound(); diff --git a/test/PausableTest.t.sol b/test/PausableTest.t.sol index c38a8e5a..3bf67bc0 100644 --- a/test/PausableTest.t.sol +++ b/test/PausableTest.t.sol @@ -6,7 +6,7 @@ import "../contracts/protocol/Socket.sol"; import "../contracts/evmx/watcher/Watcher.sol"; import "../contracts/evmx/helpers/AddressResolver.sol"; import "../contracts/utils/common/AccessRoles.sol"; - import "../contracts/utils/Pausable.sol"; +import "../contracts/utils/Pausable.sol"; import "../contracts/utils/AccessControl.sol"; import "../contracts/utils/common/Constants.sol"; import "solady/utils/ERC1967Factory.sol"; @@ -29,7 +29,6 @@ contract PausableTest is Test { Socket socket; Watcher watcher; - AddressResolver addressResolver; function setUp() public { diff --git a/test/encode.t.sol b/test/encode.t.sol index 0cd99833..18fb31a5 100644 --- a/test/encode.t.sol +++ b/test/encode.t.sol @@ -15,10 +15,7 @@ import "solady/utils/ERC1967Factory.sol"; * @notice Unit tests for pause/unpause functionality with PAUSER_ROLE and UNPAUSER_ROLE */ contract PausableTest is Test { - - - function setUp() public { - } + function setUp() public {} function test_encode_source() public { uint32 sourceChainSlug = 1; @@ -36,9 +33,11 @@ contract PausableTest is Test { /// @param packed The packed bytes from abi.encodePacked(sourceChainSlug, sourceId) /// @return sourceChainSlug The decoded chain slug (uint32) /// @return sourceId The decoded source ID (bytes32) - function decodePackedSource(bytes memory packed) public pure returns (uint32 sourceChainSlug, bytes32 sourceId) { + function decodePackedSource( + bytes memory packed + ) public pure returns (uint32 sourceChainSlug, bytes32 sourceId) { require(packed.length >= 36, "Invalid packed length"); - + // Method 1: Using assembly (most efficient) // In bytes memory, first 32 bytes contain the length // Data starts at offset 32 @@ -47,7 +46,7 @@ contract PausableTest is Test { let firstWord := mload(add(packed, 32)) // Extract uint32 from rightmost 4 bytes (shift right by 224 bits = 28 bytes) sourceChainSlug := shr(224, firstWord) - + // Read next 32 bytes starting at offset 36 (skip 4 bytes for uint32) sourceId := mload(add(packed, 36)) } @@ -57,14 +56,16 @@ contract PausableTest is Test { /// @param packed The packed bytes from abi.encodePacked(sourceChainSlug, sourceId) /// @return sourceChainSlug The decoded chain slug (uint32) /// @return sourceId The decoded source ID (bytes32) - function decodePackedSourceNoAssembly(bytes memory packed) public pure returns (uint32 sourceChainSlug, bytes32 sourceId) { + function decodePackedSourceNoAssembly( + bytes memory packed + ) public pure returns (uint32 sourceChainSlug, bytes32 sourceId) { require(packed.length >= 36, "Invalid packed length"); - + // Extract first 4 bytes directly - Solidity allows bytes4(bytes memory) bytes4 slugBytes = bytes4(packed); // Convert bytes4 to uint32 (direct cast works) sourceChainSlug = uint32(slugBytes); - + // Extract next 32 bytes (bytes 4-35) and convert to bytes32 // Build bytes32 by combining bytes in big-endian order uint256 idValue; @@ -79,17 +80,17 @@ contract PausableTest is Test { uint32 sourceChainSlug = 1; bytes32 sourceId = bytes32(uint256(2)); bytes memory packed = abi.encodePacked(sourceChainSlug, sourceId); - + // Test assembly version (uint32 decodedSlug, bytes32 decodedId) = decodePackedSource(packed); assertEq(decodedSlug, sourceChainSlug, "Chain slug mismatch (assembly)"); assertEq(decodedId, sourceId, "Source ID mismatch (assembly)"); - + // Test pure Solidity version (uint32 decodedSlug2, bytes32 decodedId2) = decodePackedSourceNoAssembly(packed); assertEq(decodedSlug2, sourceChainSlug, "Chain slug mismatch (no assembly)"); assertEq(decodedId2, sourceId, "Source ID mismatch (no assembly)"); - + console.log("Decoded chain slug (assembly):", decodedSlug); console.log("Decoded chain slug (no assembly):", decodedSlug2); console.logBytes32(decodedId); @@ -100,37 +101,37 @@ contract PausableTest is Test { function test_fallback_double_encoding() public { // Deploy the mock fallback contract MockFallbackContract mock = new MockFallbackContract(); - + bytes32 expectedPayloadId = bytes32(uint256(0x123456789abcdef)); - + // Call the fallback function with some dummy data (bool success, bytes memory returnData) = address(mock).call{value: 0}( abi.encodeWithSignature("someRandomFunction(uint256)", 42) ); - + require(success, "Call failed"); - + console.log("\n=== Fallback Double Encoding Test ==="); console.log("Expected payloadId:"); console.logBytes32(expectedPayloadId); - + console.log("\nRaw return data from fallback:"); console.logBytes(returnData); console.log("Return data length:", returnData.length); - + // Decode using double decode: first decode gets the inner encoded bytes, second gets the bytes32 // This is the reverse of: abi.encode(abi.encode(payloadId)) bytes memory innerEncoded = abi.decode(returnData, (bytes)); bytes32 decodedPayloadId = abi.decode(innerEncoded, (bytes32)); - + // Alternative: one-liner version // bytes32 decodedPayloadId = abi.decode(abi.decode(returnData, (bytes)), (bytes32)); - + console.log("\nDecoded payloadId:"); console.logBytes32(decodedPayloadId); - + assertEq(decodedPayloadId, expectedPayloadId, "PayloadId mismatch after decoding"); - + console.log("\nSuccessfully decoded payloadId from double-encoded fallback return"); } @@ -138,50 +139,52 @@ contract PausableTest is Test { function test_fallback_single_vs_double_encoding() public { MockFallbackSingleEncode singleEncode = new MockFallbackSingleEncode(); MockFallbackDoubleEncode doubleEncode = new MockFallbackDoubleEncode(); - + bytes32 expectedPayloadId = bytes32(uint256(0x123456789abcdef)); - + console.log("\n=== Comparing Single vs Double Encoding ==="); - + // Test single encoding (bool success1, bytes memory returnData1) = address(singleEncode).call( abi.encodeWithSignature("dummy()") ); require(success1, "Single encode call failed"); - + console.log("\nSingle encode return data:"); console.logBytes(returnData1); console.log("Length:", returnData1.length); - + // Test double encoding (bool success2, bytes memory returnData2) = address(doubleEncode).call( abi.encodeWithSignature("dummy()") ); require(success2, "Double encode call failed"); - + console.log("\nDouble encode return data:"); console.logBytes(returnData2); console.log("Length:", returnData2.length); - + // Try to decode both console.log("\n--- Decoding Results ---"); - + // Single encoding: abi.encode(bytes32) just gives you the bytes32 padded to 32 bytes // Can decode directly as bytes32 bytes32 decoded1 = abi.decode(returnData1, (bytes32)); console.log("Decoded from single encoding:"); console.logBytes32(decoded1); assertEq(decoded1, expectedPayloadId, "Single encoding decoded correctly (raw bytes32)"); - + // Double encoding: abi.encode(abi.encode(bytes32)) wraps it in ABI structure (offset + length + data) // Need to decode twice: first to get inner bytes, then to get bytes32 bytes32 decoded2 = abi.decode(abi.decode(returnData2, (bytes)), (bytes32)); console.log("Decoded from double encoding:"); console.logBytes32(decoded2); assertEq(decoded2, expectedPayloadId, "Double encoding decoded correctly (ABI-encoded)"); - + console.log("\nBoth methods decoded successfully"); - console.log("Note: Single encoding returns 32 bytes, double encoding returns 96 bytes (offset + length + data)"); + console.log( + "Note: Single encoding returns 32 bytes, double encoding returns 96 bytes (offset + length + data)" + ); } /// @notice Test using interface call (like GasStation.sol) vs raw .call() @@ -191,11 +194,11 @@ contract PausableTest is Test { function test_interface_call_vs_raw_call() public { MockSocketWithFallback mockSocket = new MockSocketWithFallback(); bytes32 expectedPayloadId = bytes32(uint256(0x123456789abcdef)); - + console.log("\n=== Interface Call vs Raw Call Test ==="); console.log("Expected payloadId:"); console.logBytes32(expectedPayloadId); - + // Method 1: Using interface (like GasStation.sol does) // This is how GasStation.sol calls Socket's fallback IMockDepositInterface mockInterface = IMockDepositInterface(address(mockSocket)); @@ -205,12 +208,12 @@ contract PausableTest is Test { 1000, 100 ); - + console.log("Returned payloadId:", returnedPayloadId); // console.log("\nMethod 1 - Interface call (GasStation.sol pattern):"); // console.logBytes(returnedPayloadId); // console.log("Note: Solidity auto-decoded the outer ABI layer!"); - + // Method 2: Using raw .call() // (bool success, bytes memory rawCallReturn) = address(mockSocket).call( // abi.encodeWithSignature( @@ -222,37 +225,36 @@ contract PausableTest is Test { // ) // ); // require(success, "Raw call failed"); - + // console.log("\nMethod 2 - Raw .call():"); // console.logBytes(rawCallReturn); // console.log("Return data length:", rawCallReturn.length); // console.log("Note: Raw bytes from fallback, no auto-decode"); - + // // Now decode both // console.log("\n--- Decoding Results ---"); - + // // CRITICAL: For interface call, Solidity ALREADY decoded the outer ABI layer! // // So returnedBytes contains abi.encode(payloadId) which is just the bytes32 // // We only need ONE decode // bytes32 decodedFromInterface = bytes32(returnedPayloadId); // console.log("Decoded from interface call (ONE decode):"); // console.logBytes32(decodedFromInterface); - + // // For raw call: No auto-decode, the full double-encoded data is returned // // We need TWO decodes to unwrap: abi.encode(abi.encode(payloadId)) // bytes32 decodedFromRawCall = bytes32(uint256(abi.decode(rawCallReturn, (uint256)))); // console.log("Decoded from raw call (TWO decodes):"); // console.logBytes32(decodedFromRawCall); - + // // Both should give us the expected payloadId // assertEq(decodedFromInterface, expectedPayloadId, "Interface call decode mismatch"); // assertEq(decodedFromRawCall, expectedPayloadId, "Raw call decode mismatch"); - + // console.log("\nConclusion: Interface calls need 1 decode, raw calls need 2 decodes!"); } - function test_gas_encode() public { - + function test_gas_encode() public { bytes32 payloadId = bytes32(uint256(0x123456789abcdef)); uint256 start_gas = gasleft(); // To mimic abi.encode(abi.encode(payloadId)), we need to create the ABI structure: @@ -260,8 +262,8 @@ contract PausableTest is Test { // - Length: 0x20 (32 bytes, the length of the bytes32) // - Data: the actual payloadId (32 bytes) // Note: Order is offset FIRST, then length, then data - uint256 offset = 0x20; // Points to length field (32 bytes from start) - uint256 length = 0x20; // Length of bytes32 is 32 bytes + uint256 offset = 0x20; // Points to length field (32 bytes from start) + uint256 length = 0x20; // Length of bytes32 is 32 bytes bytes memory encoded = abi.encodePacked(offset, length, payloadId); uint256 end_gas = gasleft(); @@ -274,11 +276,10 @@ contract PausableTest is Test { console.logBytes(encoded); console.log("Gas used:", start_gas - end_gas); console.log("Encoded length:", encoded.length); - + // Verify they match assertEq(encoded.length, encoded2.length, "Length mismatch"); assertEq(keccak256(encoded), keccak256(encoded2), "Encoded bytes don't match"); - } } diff --git a/test/protocol/Socket.t.sol b/test/protocol/Socket.t.sol index 6edca666..68ddea1e 100644 --- a/test/protocol/Socket.t.sol +++ b/test/protocol/Socket.t.sol @@ -22,7 +22,12 @@ import "../Utils.t.sol"; * @dev Wrapper contract to expose internal functions for testing */ contract SocketTestWrapper is Socket { - constructor(uint32 chainSlug_, address owner_, uint256 gasLimitBuffer_, uint16 maxCopyBytes_) Socket(chainSlug_, owner_, gasLimitBuffer_, maxCopyBytes_) {} + constructor( + uint32 chainSlug_, + address owner_, + uint256 gasLimitBuffer_, + uint16 maxCopyBytes_ + ) Socket(chainSlug_, owner_, gasLimitBuffer_, maxCopyBytes_) {} // Expose internal functions for testing function createDigest( @@ -275,7 +280,12 @@ contract SocketTestBase is Test, Utils { mockPlug = new SimpleMockPlug(); mockFeeManager = new MockFeeManager(); mockTarget = new MockTarget(); - socketWrapper = new SocketTestWrapper(TEST_CHAIN_SLUG, socketOwner, GAS_LIMIT_BUFFER, MAX_COPY_BYTES); + socketWrapper = new SocketTestWrapper( + TEST_CHAIN_SLUG, + socketOwner, + GAS_LIMIT_BUFFER, + MAX_COPY_BYTES + ); // Set up initial state vm.startPrank(socketOwner); diff --git a/test/protocol/switchboard/MessageSwitchboard.t.sol b/test/protocol/switchboard/MessageSwitchboard.t.sol index 6b582d04..ea308b76 100644 --- a/test/protocol/switchboard/MessageSwitchboard.t.sol +++ b/test/protocol/switchboard/MessageSwitchboard.t.sol @@ -1317,7 +1317,13 @@ contract MessageSwitchboardTest is Test, Utils { uint256 nonce = 1; bytes32 digest = keccak256( - abi.encodePacked(toBytes32Format(address(messageSwitchboard)), SRC_CHAIN, payloadId, isReverting, nonce) + abi.encodePacked( + toBytes32Format(address(messageSwitchboard)), + SRC_CHAIN, + payloadId, + isReverting, + nonce + ) ); bytes memory signature = createSignature(digest, watcherPrivateKey); From abace8f61fa99c0f6da3296ebea77f1110abce58 Mon Sep 17 00:00:00 2001 From: Ameesha Agrawal Date: Mon, 24 Nov 2025 15:26:39 +0530 Subject: [PATCH 5/8] fix: evmx sb --- .../protocol/switchboard/EVMxSwitchboard.sol | 22 +++++++++++++-- .../switchboard/EVMxSwitchboard.t.sol | 28 +++++++++++++++++-- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/contracts/protocol/switchboard/EVMxSwitchboard.sol b/contracts/protocol/switchboard/EVMxSwitchboard.sol index 230a3221..b82c684d 100644 --- a/contracts/protocol/switchboard/EVMxSwitchboard.sol +++ b/contracts/protocol/switchboard/EVMxSwitchboard.sol @@ -47,6 +47,8 @@ contract EVMxSwitchboard is SwitchboardBase { // @notice Mapping of payload ID to plug address mapping(bytes32 => address) public payloadIdToPlug; + /// @notice Mapping of watcher address to nonce to usage status (prevents replay attacks) + mapping(address => mapping(uint256 => bool)) public usedNonces; // --- Events --- /// @notice Event emitted when watcher attests a payload @@ -208,8 +210,24 @@ contract EVMxSwitchboard is SwitchboardBase { */ function setRevertingPayload( bytes32 payloadId_, - bool isReverting_ - ) external onlyRole(WATCHER_ROLE) { + bool isReverting_, + uint256 nonce_, + bytes calldata signature_ + ) external { + bytes32 digest = keccak256( + abi.encodePacked( + toBytes32Format(address(this)), + chainSlug, + payloadId_, + isReverting_, + nonce_ + ) + ); + + address watcher = _recoverSigner(digest, signature_); + if (!_hasRole(WATCHER_ROLE, watcher)) revert WatcherNotFound(); + usedNonces[watcher][nonce_] = true; + revertingPayloadIds[payloadId_] = isReverting_; emit RevertingPayloadIdset(payloadId_, isReverting_); } diff --git a/test/protocol/switchboard/EVMxSwitchboard.t.sol b/test/protocol/switchboard/EVMxSwitchboard.t.sol index 0d815978..f7e1e6f6 100644 --- a/test/protocol/switchboard/EVMxSwitchboard.t.sol +++ b/test/protocol/switchboard/EVMxSwitchboard.t.sol @@ -594,12 +594,23 @@ contract SocketPayloadIdVerificationTest is EVMxSwitchboardTestBase { function test_SetRevertingPayload_Success() public { bytes32 payloadId = bytes32(uint256(0x1234)); bool isReverting = true; + uint256 nonce = 1; + bytes32 digest = keccak256( + abi.encodePacked( + toBytes32Format(address(evmxSwitchboard)), + CHAIN_SLUG, + payloadId, + isReverting, + nonce + ) + ); + bytes memory signature = createSignature(digest, watcherPrivateKey); vm.expectEmit(true, false, false, true); emit EVMxSwitchboard.RevertingPayloadIdset(payloadId, isReverting); vm.prank(getWatcherAddress()); - evmxSwitchboard.setRevertingPayload(payloadId, isReverting); + evmxSwitchboard.setRevertingPayload(payloadId, isReverting, nonce, signature); // Verify it was set (check via allowPayload or directly if there's a getter) // Note: revertingPayloadIds is internal, so we can't directly check it @@ -609,10 +620,21 @@ contract SocketPayloadIdVerificationTest is EVMxSwitchboardTestBase { function test_SetRevertingPayload_OnlyOwner() public { bytes32 payloadId = bytes32(uint256(0x1234)); bool isReverting = true; + + uint256 nonce = 1; + bytes32 digest = keccak256( + abi.encodePacked( + toBytes32Format(address(evmxSwitchboard)), + CHAIN_SLUG, + payloadId, + isReverting, + nonce + ) + ); + bytes memory signature = createSignature(digest, uint256(0x1234)); - vm.prank(address(0x9999)); vm.expectRevert(); - evmxSwitchboard.setRevertingPayload(payloadId, isReverting); + evmxSwitchboard.setRevertingPayload(payloadId, isReverting, nonce, signature); } // ============================================ From 95952c0c780f3c57ebe9bbf07234b3926c162c90 Mon Sep 17 00:00:00 2001 From: Ameesha Agrawal Date: Mon, 24 Nov 2025 15:39:38 +0530 Subject: [PATCH 6/8] fix: rename --- .../protocol/switchboard/EVMxSwitchboard.sol | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/contracts/protocol/switchboard/EVMxSwitchboard.sol b/contracts/protocol/switchboard/EVMxSwitchboard.sol index b82c684d..f55da5f7 100644 --- a/contracts/protocol/switchboard/EVMxSwitchboard.sol +++ b/contracts/protocol/switchboard/EVMxSwitchboard.sol @@ -18,8 +18,8 @@ contract EVMxSwitchboard is SwitchboardBase { /// @notice EVMX chain slug for payload verification uint32 public immutable evmxChainSlug; - /// @notice Watcher ID for payload verification - uint32 public immutable watcherId; + /// @notice EVMX watcher ID for payload verification + uint32 public immutable evmxWatcherId; /// @notice Counter for generating unique payload IDs /// @dev If we deploy a new set of Socket contracts, we need to start counter from last value to avoid @@ -29,7 +29,7 @@ contract EVMxSwitchboard is SwitchboardBase { /// @notice Default deadline for payload execution (1 day) uint256 public defaultDeadline = 1 days; - /// @notice TotalWatchers registered + /// @notice TotalWatchers registered which are responsible for attesting payloads from evmx uint256 public totalWatchers; /// @notice Mapping of watcher address to digest to attestation status (true if attested by watcher) @@ -67,7 +67,7 @@ contract EVMxSwitchboard is SwitchboardBase { event PlugConfigUpdated(address indexed plug, bytes32 appGatewayId); /// @notice Event emitted when EVMX config is set - event EvmxConfigSet(uint32 evmxChainSlug, uint32 watcherId); + event EvmxConfigSet(uint32 evmxChainSlug, uint32 evmxWatcherId); /// @notice Event emitted when payload is requested event PayloadRequested( @@ -85,10 +85,10 @@ contract EVMxSwitchboard is SwitchboardBase { ISocket socket_, address owner_, uint32 evmxChainSlug_, - uint32 watcherId_ + uint32 evmxWatcherId_ ) SwitchboardBase(chainSlug_, socket_, owner_) { evmxChainSlug = evmxChainSlug_; - watcherId = watcherId_; + evmxWatcherId = evmxWatcherId_; } // --- External Functions --- @@ -150,7 +150,7 @@ contract EVMxSwitchboard is SwitchboardBase { bytes calldata payload_, bytes calldata overrides_ ) external payable override onlySocket returns (bytes32 payloadId) { - if (evmxChainSlug == 0 || watcherId == 0) revert EvmxConfigNotSet(); + if (evmxChainSlug == 0 || evmxWatcherId == 0) revert EvmxConfigNotSet(); bytes memory overrides; if (overrides_.length == 0) { @@ -164,7 +164,7 @@ contract EVMxSwitchboard is SwitchboardBase { chainSlug, switchboardId, evmxChainSlug, - watcherId, + evmxWatcherId, payloadCounter++ ); From dc293f2e2d2c1e79b7213c86ccd28dac3232b56e Mon Sep 17 00:00:00 2001 From: Ameesha Agrawal Date: Mon, 24 Nov 2025 15:45:11 +0530 Subject: [PATCH 7/8] fix: move payload id to digest map --- contracts/protocol/Socket.sol | 5 ----- contracts/protocol/interfaces/ISocket.sol | 7 ------- contracts/protocol/switchboard/EVMxSwitchboard.sol | 1 - contracts/protocol/switchboard/MessageSwitchboard.sol | 4 ++++ test/protocol/Socket.t.sol | 10 ---------- 5 files changed, 4 insertions(+), 23 deletions(-) diff --git a/contracts/protocol/Socket.sol b/contracts/protocol/Socket.sol index 7891adff..7ff49fa3 100644 --- a/contracts/protocol/Socket.sol +++ b/contracts/protocol/Socket.sol @@ -17,9 +17,6 @@ contract Socket is SocketUtils { /// @notice Mapping of payload id to execution status (Executed/Reverted) mapping(bytes32 => ExecutionStatus) public executionStatus; - /// @notice Mapping of payload id to its digest for verification - mapping(bytes32 => bytes32) public payloadIdToDigest; - // --- Constructor --- /** @@ -95,8 +92,6 @@ contract Socket is SocketUtils { ); bytes32 digest = _createDigest(transmitter, executionParams_); - payloadIdToDigest[executionParams_.payloadId] = digest; - if ( !ISwitchboard(switchboardAddress).allowPayload( digest, diff --git a/contracts/protocol/interfaces/ISocket.sol b/contracts/protocol/interfaces/ISocket.sol index b20ff38c..eecc719b 100644 --- a/contracts/protocol/interfaces/ISocket.sol +++ b/contracts/protocol/interfaces/ISocket.sol @@ -125,13 +125,6 @@ interface ISocket { */ function chainSlug() external view returns (uint32); - /** - * @notice Returns the digest of a payload - * @param payloadId_ The payload ID - * @return digest The digest hash for verification - */ - function payloadIdToDigest(bytes32 payloadId_) external view returns (bytes32); - /** * @notice Returns the switchboard address for a given switchboard ID * @param switchboardId_ The switchboard ID diff --git a/contracts/protocol/switchboard/EVMxSwitchboard.sol b/contracts/protocol/switchboard/EVMxSwitchboard.sol index f55da5f7..3fe7ab2a 100644 --- a/contracts/protocol/switchboard/EVMxSwitchboard.sol +++ b/contracts/protocol/switchboard/EVMxSwitchboard.sol @@ -167,7 +167,6 @@ contract EVMxSwitchboard is SwitchboardBase { evmxWatcherId, payloadCounter++ ); - payloadIdToPlug[payloadId] = plug_; emit PayloadRequested(payloadId, plug_, switchboardId, overrides, payload_); } diff --git a/contracts/protocol/switchboard/MessageSwitchboard.sol b/contracts/protocol/switchboard/MessageSwitchboard.sol index 6abe660e..b084a87d 100644 --- a/contracts/protocol/switchboard/MessageSwitchboard.sol +++ b/contracts/protocol/switchboard/MessageSwitchboard.sol @@ -64,6 +64,9 @@ contract MessageSwitchboard is SwitchboardBase, ReentrancyGuard { /// @notice Mapping of fee updater address to nonce to usage status (prevents replay attacks) mapping(address => mapping(uint256 => bool)) public usedNonces; + /// @notice Mapping of payload id to its digest for verification + mapping(bytes32 => bytes32) public payloadIdToDigest; + // --- Events --- /// @notice Event emitted when watcher attests a payload @@ -214,6 +217,7 @@ contract MessageSwitchboard is SwitchboardBase, ReentrancyGuard { bytes32 payloadId_ ) = _createDigestAndPayloadId(plug_, overrides, payload_); payloadId = payloadId_; + payloadIdToDigest[payloadId] = digest; if (overrides.isSponsored) { // Sponsored flow - validate sponsor has approved this plug diff --git a/test/protocol/Socket.t.sol b/test/protocol/Socket.t.sol index 68ddea1e..bbd4e9ef 100644 --- a/test/protocol/Socket.t.sol +++ b/test/protocol/Socket.t.sol @@ -657,16 +657,6 @@ contract SocketExecuteTestPart2 is SocketTestBase { assertEq(returnData.length, 2048, "Return data should be truncated to maxCopyBytes (2048)"); } - function test_Execute_StoresDigest() public { - bytes32 payloadId = executionParams.payloadId; - - hoax(transmitter); - socket.execute{value: 1 ether}(executionParams, transmissionParams); - - bytes32 storedDigest = socket.payloadIdToDigest(payloadId); - assertTrue(storedDigest != bytes32(0), "Digest should be stored"); - } - function test_Execute_TryCallRevert_ValueStaysInSocketAndGetsRefunded() public { // Create a plug that will revert when called SimpleMockPlug revertingPlug = new SimpleMockPlug(); From 1f3daccc7d04e82e61e22616652429bddc80f282 Mon Sep 17 00:00:00 2001 From: Ameesha Agrawal Date: Mon, 24 Nov 2025 18:32:37 +0530 Subject: [PATCH 8/8] fix: check used nonce --- contracts/protocol/Socket.sol | 2 +- contracts/protocol/switchboard/EVMxSwitchboard.sol | 7 ++++--- contracts/protocol/switchboard/MessageSwitchboard.sol | 6 +++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/contracts/protocol/Socket.sol b/contracts/protocol/Socket.sol index 7ff49fa3..2df3668d 100644 --- a/contracts/protocol/Socket.sol +++ b/contracts/protocol/Socket.sol @@ -83,7 +83,7 @@ contract Socket is SocketUtils { address switchboardAddress, ExecutionParams memory executionParams_, bytes memory transmitterProof_ - ) internal { + ) internal view { // NOTE: the first un-trusted call in the system address transmitter = ISwitchboard(switchboardAddress).getTransmitter( msg.sender, diff --git a/contracts/protocol/switchboard/EVMxSwitchboard.sol b/contracts/protocol/switchboard/EVMxSwitchboard.sol index 3fe7ab2a..34f7fdf6 100644 --- a/contracts/protocol/switchboard/EVMxSwitchboard.sol +++ b/contracts/protocol/switchboard/EVMxSwitchboard.sol @@ -33,7 +33,7 @@ contract EVMxSwitchboard is SwitchboardBase { uint256 public totalWatchers; /// @notice Mapping of watcher address to digest to attestation status (true if attested by watcher) - mapping(address => mapping(bytes32 => bool)) public isAttested; + mapping(address => mapping(bytes32 => bool)) public isAttestedByWatcher; /// @notice Mapping of digest to attestation count mapping(bytes32 => uint256) public attestations; @@ -108,8 +108,8 @@ contract EVMxSwitchboard is SwitchboardBase { if (!_hasRole(WATCHER_ROLE, watcher)) revert WatcherNotFound(); // Prevent double attestation - if (isAttested[watcher][digest_]) revert AlreadyAttested(); - isAttested[watcher][digest_] = true; + if (isAttestedByWatcher[watcher][digest_]) revert AlreadyAttested(); + isAttestedByWatcher[watcher][digest_] = true; attestations[digest_]++; // Mark digest as valid if enough attestations are reached @@ -225,6 +225,7 @@ contract EVMxSwitchboard is SwitchboardBase { address watcher = _recoverSigner(digest, signature_); if (!_hasRole(WATCHER_ROLE, watcher)) revert WatcherNotFound(); + if (usedNonces[watcher][nonce_]) revert NonceAlreadyUsed(); usedNonces[watcher][nonce_] = true; revertingPayloadIds[payloadId_] = isReverting_; diff --git a/contracts/protocol/switchboard/MessageSwitchboard.sol b/contracts/protocol/switchboard/MessageSwitchboard.sol index b084a87d..b23c6dde 100644 --- a/contracts/protocol/switchboard/MessageSwitchboard.sol +++ b/contracts/protocol/switchboard/MessageSwitchboard.sol @@ -29,7 +29,7 @@ contract MessageSwitchboard is SwitchboardBase, ReentrancyGuard { mapping(uint32 => uint256) public totalWatchers; /// @notice Mapping of watcher address to digest to attestation status (true if attested by watcher) - mapping(address => mapping(bytes32 => bool)) public isAttested; + mapping(address => mapping(bytes32 => bool)) public isAttestedByWatcher; /// @notice Mapping of digest to attestation count mapping(bytes32 => uint256) public attestations; @@ -428,8 +428,8 @@ contract MessageSwitchboard is SwitchboardBase, ReentrancyGuard { if (!_hasRole(role, watcher)) revert WatcherNotFound(); // Prevent double attestation - if (isAttested[watcher][digest_]) revert AlreadyAttested(); - isAttested[watcher][digest_] = true; + if (isAttestedByWatcher[watcher][digest_]) revert AlreadyAttested(); + isAttestedByWatcher[watcher][digest_] = true; attestations[digest_]++; // Mark digest_ as valid if enough attestations are reached