feat: updated payloadId implementation #211
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (9)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request refactors the payload identification system across the protocol. It replaces the trigger-based architecture with a unified 256-bit payload ID scheme encoding origin, verification, pointer, and reserved fields. Key changes include renaming Changes
Sequence Diagram(s)sequenceDiagram
participant Plug
participant Socket
participant FSB as FastSwitchboard
participant Watcher
participant Relayer
Plug->>Socket: sendPayload(data) payable
activate Socket
Socket->>FSB: processPayload(plug, payload, overrides)
activate FSB
FSB->>FSB: validate EVMX config set
FSB->>FSB: create payloadId = createPayloadId(origin, verification, pointer)
FSB->>FSB: increment triggerPayloadCounter
FSB-->>Socket: return payloadId
deactivate FSB
Socket-->>Plug: return payloadId
deactivate Socket
Note over Relayer: Off-chain indexing
Relayer->>Socket: execute(ExecuteParams{payloadId, ...})
activate Socket
Socket->>Socket: verify payloadId via getVerificationInfo()
alt Valid destination
Socket->>FSB: _verify(payloadId, ...)
activate FSB
FSB-->>Socket: ✓
deactivate FSB
Socket->>Socket: execute payload
else Invalid chainSlug
Socket-->>Relayer: revert InvalidVerificationChainSlug
else Invalid switchboardId
Socket-->>Relayer: revert InvalidVerificationSwitchboardId
end
deactivate Socket
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
tHeMaskedMan981
changed the base branch from
feat/message-switchboard
to
phase-1
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
hardhat.config.ts (1)
18-88: Restore non-zero private keys for configured networks
constants.HashZerois not a valid secp256k1 private key. Hardhat instantiates anethers.Walletfor each entry inaccounts, andnew Wallet(HashZero)throwsinvalid private keyat runtime. This should keep using a real signer (e.g.,SOCKET_PRIVATE_KEY) or fail fast when it is missing, otherwise every network that hitsgetChainConfigwill crash before any deployment or test runs.-import { constants } from "ethers"; +// no additional import needed -const privateKey: HardhatNetworkAccountUserConfig = process.env - .SOCKET_PRIVATE_KEY as unknown as HardhatNetworkAccountUserConfig; +const socketPrivateKey = process.env.SOCKET_PRIVATE_KEY; +if (!socketPrivateKey || /^0x0+$/.test(socketPrivateKey.replace(/^0x/, ""))) { + throw new Error("SOCKET_PRIVATE_KEY must be set to a non-zero hex private key"); +} function getChainConfig(chainSlug: ChainSlug): NetworkUserConfig { return { - accounts: [`${constants.HashZero}`], + accounts: [ + socketPrivateKey.startsWith("0x") ? socketPrivateKey : `0x${socketPrivateKey}`, + ], chainId: ChainSlugToId[chainSlug], url: getJsonRpcUrl(chainSlug), }; } @@ EVMX: { - accounts: [`0x${privateKey}`], + accounts: [ + socketPrivateKey.startsWith("0x") ? socketPrivateKey : `0x${socketPrivateKey}`, + ], chainId: EVMX_CHAIN_ID, url: process.env.EVMX_RPC, },deprecated/test/mock/MockFastSwitchboard.sol (1)
48-54: Signature mismatch breaks compilationThis should implement the new
ISwitchboard.processPayloadsignature exactly. The extratriggerId_parameter and missing return value leave the override invalid, so the contract will not compile after the interface change.Apply this diff to realign the mock:
-function processPayload( - address plug_, - bytes32 triggerId_, - bytes calldata payload_, - bytes calldata overrides_ -) external payable override { - // Simple implementation that just accepts the trigger - // In a real switchboard, this would process the trigger -} +function processPayload( + address plug_, + bytes calldata payload_, + bytes calldata overrides_ +) external payable override returns (bytes32 payloadId_) { + // Simple implementation that just accepts the payload + payloadId_ = keccak256(abi.encodePacked(plug_, payload_, overrides_)); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
FunctionSignatures.md(2 hunks)PAYLOAD_ID_ARCHITECTURE.md(1 hunks)contracts/evmx/watcher/Watcher.sol(1 hunks)contracts/protocol/Socket.sol(7 hunks)contracts/protocol/interfaces/ISocket.sol(2 hunks)contracts/protocol/interfaces/ISwitchboard.sol(1 hunks)contracts/protocol/switchboard/FastSwitchboard.sol(4 hunks)contracts/protocol/switchboard/MessageSwitchboard.sol(7 hunks)contracts/utils/common/IdUtils.sol(1 hunks)contracts/utils/common/Structs.sol(1 hunks)deprecated/test/mock/MockFastSwitchboard.sol(1 hunks)deprecated/test/protocol/switchboards/FastSwitchboardTest.t.sol(2 hunks)hardhat.config.ts(2 hunks)test/SetupTest.t.sol(7 hunks)test/SocketPayloadIdVerification.t.sol(1 hunks)test/mocks/MockPlug.sol(1 hunks)test/switchboard/MessageSwitchboard.t.sol(21 hunks)
🧰 Additional context used
🪛 LanguageTool
PAYLOAD_ID_ARCHITECTURE.md
[grammar] ~24-~24: Ensure spelling is correct
Context: ...cher (on EVMX) - Verified by: Watcher offchain (links source) - Verification: `dst...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~36-~36: Ensure spelling is correct
Context: ...astSwitchboard - Verified by: Watcher offchain (verifies source) - Verification: `...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~120-~120: Ensure spelling is correct
Context: ...tches local config 2. Source (Watcher offchain): Verifies origin component matches e...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (4)
FunctionSignatures.md (1)
365-393: Docs now mirror the ABI renameThe signature table now matches the
processPayloadentry point exposed by the switchboards, so downstream consumers will see the correct selector. This should ship with the contract rename.test/mocks/MockPlug.sol (1)
41-41: Mock aligns with new outbound APISwitching to
socket__.sendPayloadkeeps the return type while exercising the same payable flow the contracts now expose. This should keep the mock in sync with the production interface.deprecated/test/protocol/switchboards/FastSwitchboardTest.t.sol (1)
24-151: Test coverage keeps pace with the renameUpdating both the override and the call site to
processPayloadensures the event emission path is still exercised after the contract rename. This should maintain the existing assertion coverage.contracts/utils/common/Structs.sol (1)
73-82: ExecuteParams now transports payloadIdReplacing
payloadPointerwithpayloadIdkeeps the 32-byte layout while matching the new payload-ID pipeline, so the execution path reads the same identifier produced by the switchboards.
| prevBatchDigestHash: bytes32(0), // No longer using triggerId | ||
| extraData:"0x" |
There was a problem hiding this comment.
Fix extraData to true empty bytes
This should store an empty byte array. The literal "0x" encodes two bytes (0x30, 0x78), so the digest we hash now includes those extra characters. Off-chain signers using empty extraData will compute a different digest and their attestations will be rejected. Swap to hex"" (or bytes("")) so the digest stays compatible.
- prevBatchDigestHash: bytes32(0), // No longer using triggerId
- extraData:"0x"
+ prevBatchDigestHash: bytes32(0), // No longer using triggerId
+ extraData: hex""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| prevBatchDigestHash: bytes32(0), // No longer using triggerId | |
| extraData:"0x" | |
| prevBatchDigestHash: bytes32(0), // No longer using triggerId | |
| extraData: hex"" |
🤖 Prompt for AI Agents
contracts/protocol/switchboard/MessageSwitchboard.sol around lines 336-337: the
extraData field is currently set to the string literal "0x", which encodes the
characters '0' and 'x' into the digest; replace it with a true empty bytes value
(e.g. hex"" or bytes("")) so the serialized digest matches off‑chain signers
expecting empty extraData; update the assignment to use hex"" (or bytes("")) and
run tests/attestation flow to confirm digest compatibility.
| prevBatchDigestHash: bytes32(0), // No longer using triggerId | ||
| extraData: "0x" // Contract now sets extraData to empty | ||
| }); |
There was a problem hiding this comment.
Align test fixture with empty extraData
This should mirror the production struct. After fixing extraData to actual empty bytes, these fixtures need to expect hex""; otherwise the digest constructed in tests will diverge from the contract.
- prevBatchDigestHash: bytes32(0), // No longer using triggerId
- extraData: "0x" // Contract now sets extraData to empty
+ prevBatchDigestHash: bytes32(0), // No longer using triggerId
+ extraData: hex"" // Contract now sets extraData to empty📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| prevBatchDigestHash: bytes32(0), // No longer using triggerId | |
| extraData: "0x" // Contract now sets extraData to empty | |
| }); | |
| prevBatchDigestHash: bytes32(0), // No longer using triggerId | |
| extraData: hex"" // Contract now sets extraData to empty | |
| }); |
🤖 Prompt for AI Agents
In test/SetupTest.t.sol around lines 899 to 901, the test fixture uses
extraData: "0x" but the production contract sets extraData to empty bytes
(hex""); update the fixture and any expected digest construction to use empty
bytes (hex"") instead of the string "0x" so the test encodes the same byte
sequence as the contract and the computed digest matches production.
Summary by CodeRabbit
New Features
API Updates
triggerAppGatewaytosendPayload(returns payload ID).processTriggertoprocessPayload(updated parameter and return signatures).PayloadRequestedevent for payload tracking.Documentation