Dev #184
Dev #184
Conversation
…into feat/trace
retry in verification script
Rescue and pause scripts
feat: reduced low finality to 0 blocks
…ght have 32 bytes address on non-evm chains
fees manager as app gateway
Fix oncomplete hook
Fix payload id
Fix transmitter verification
…ket-protocol into deploy-scripts
…ket-protocol into deploy-scripts
Deploy scripts
WalkthroughRefactors protocol to use switchboardId (uint64) and bytes32 IDs for plugs/targets across contracts, interfaces, events, and tooling. Adds MessageSwitchboard and CCTPSwitchboard subsystems, new Converters, ID packing in payloadId, revamped Socket trigger/execute flows, watcher/request handling updates, deployment metadata and scripts, plus extensive test updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Plug
participant Socket
participant Switchboard
participant Watcher
participant Target
Plug->>Socket: triggerAppGateway(data)
Socket->>Switchboard: processTrigger(plug, triggerId, payload, overrides) [payable]
Note right of Switchboard: Emits TriggerProcessed
Watcher->>Switchboard: attest(digest, proof)
Socket->>Switchboard: getTransmitter(sender, payloadId, transmitterProof)
Socket->>Switchboard: allowPayload(digest, payloadId)?
Switchboard-->>Socket: true/false
Socket->>Target: _execute(...)
sequenceDiagram
participant Caller
participant SocketBatcher
participant Socket
participant CCTP as CCTPSwitchboard
Caller->>SocketBatcher: attestCCTPAndProveAndExecute(execParams, cctpParams, switchboardId)
SocketBatcher->>CCTP: attestVerifyAndProveExecutions(execParams, cctpParams, payloadId)
SocketBatcher->>Socket: execute(executeParams, transmissionParams, refund)
SocketBatcher->>CCTP: syncOut(payloadId, nextBatchRemoteChainSlugs)
sequenceDiagram
participant Owner
participant Switchboard
participant Socket
participant Plug
Owner->>Switchboard: registerSwitchboard()
Switchboard->>Socket: registerSwitchboard() -> returns id
Owner-->>Switchboard: switchboardId stored
Owner->>Plug: initSocket(appGatewayId, socket, switchboardId)
Plug->>Socket: connect(appGatewayId, switchboardId)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 95
🔭 Outside diff range comments (19)
src/finality.ts (1)
12-16: Avoid leaking mutable shared config objects
getFinalityBlocksreturns references to module-level objects. A consumer mutating the returned object will mutate shared config for everyone. This should return a defensive copy.export const getFinalityBlocks = ( chainSlug: ChainSlug ): ChainFinalityBlocks => { - return finalityBlockOverrides[chainSlug] ?? defaultFinalityBlocks; + const blocks = finalityBlockOverrides[chainSlug] ?? defaultFinalityBlocks; + // Return a defensive copy to prevent accidental mutation of shared config. + return { ...blocks }; };hardhat-scripts/utils/overrides.ts (2)
8-14: Use BigNumberish for overrides storage to avoid lossy number conversionschainOverrides currently stores gasLimit/gasPrice as number, which then forces toNumber() elsewhere. This should use BigNumberish end-to-end to prevent precision loss and future overflow issues.
Apply this diff:
export const chainOverrides: { [chainSlug in ChainSlug]?: { type?: number; - gasLimit?: number; - gasPrice?: number; + gasLimit?: BigNumberish; + gasPrice?: BigNumberish; }; } = {
64-70: Stop coercing gasPrice to number; keep as BigNumber/BigNumberishtoNumber() is lossy and can overflow for larger values. The return type already allows BigNumberish. This should return the BigNumber directly.
Apply this diff:
- if (gasPrice == undefined || gasPrice == null) - gasPrice = (await getGasPrice(chainSlug, provider)).toNumber(); + if (gasPrice == undefined || gasPrice == null) + gasPrice = await getGasPrice(chainSlug, provider);Note: This relies on the earlier type change so gasPrice in chainOverrides is also BigNumberish.
hardhat.config.ts (1)
103-104: Apply env-configurable EVMX explorer URLs & confirm Etherscan API compatibilityThe sanity-check script returned
"Unknown module"for both proxy calls, so Hardhat’s verify plugin will fail against this host. This should externalize the URLs to env vars and point at an Etherscan-compatible API (supportingproxy,account,contract, etc.), or swap in a host that does.Files to update:
- hardhat.config.ts at lines 103–104 (and similarly at 142–144)
Apply this diff:
{ network: "EVMX", chainId: EVMX_CHAIN_ID, urls: { - apiURL: "https://explorer-evmx-1dpy1f56o4.t.conduit.xyz/api", - browserURL: "https://explorer-evmx-1dpy1f56o4.t.conduit.xyz/", + apiURL: process.env.EVMX_EXPLORER_API_URL || "https://explorer-evmx-1dpy1f56o4.t.conduit.xyz/api", + browserURL: process.env.EVMX_EXPLORER_BROWSER_URL || "https://explorer-evmx-1dpy1f56o4.t.conduit.xyz/", }, },After updating, confirm verify support by running:
npx hardhat verify --network EVMX <DEPLOYED_CONTRACT_ADDRESS>src/signer.ts (1)
10-25: Refactor nonce generation to use CSPRNG and allow injectionThis should replace the insecure
getNonce()(Date.now + Math.random) with a CSPRNG-backed value and make nonce injectable for deterministic tests.• In
src/signer.ts, updategetNonceto something like:import { randomBytes } from "crypto"; import { BigNumber } from "ethers"; export const getNonce = (): BigNumber => { // 16 bytes of entropy → up to uint128 const buf = randomBytes(16); return BigNumber.from(buf); };• Change the signer function signature to accept an optional override:
-export const signWatcherMultiCallMessage = async ( - watcherContractAddress: string, - evmxChainId: number, - targetContractAddress: string, - calldata: string, - signer: ethers.Signer -) => { - const signatureNonce = getNonce(); +export const signWatcherMultiCallMessage = async ( + watcherContractAddress: string, + evmxChainId: number, + targetContractAddress: string, + calldata: string, + signer: ethers.Signer, + opts?: { nonce?: BigNumberish } +) => { + const signatureNonce = opts?.nonce ?? getNonce();• Confirm ABI encoding types still match your protocol:
evmxChainIdis encoded as"uint32"here. If chain IDs have moved touint64, update the first tuple element to"uint64".targetContractAddressremains"address". If you now use a bytes32 identifier for targets, change that element accordingly.hardhat-scripts/deploy/7.upload.ts (2)
24-37: Validate CONFIG_FILE_NAME before using it as an S3 key.This should reject unsafe characters and enforce a .json key to avoid surprising S3 paths and ensure consistent content-type handling.
Apply this diff to validate and normalize the key:
const getFileName = () => { switch (mode) { case DeploymentMode.LOCAL: - return process.env.CONFIG_FILE_NAME || "pocConfig.json"; + return sanitizeS3Key(process.env.CONFIG_FILE_NAME || "pocConfig.json"); case DeploymentMode.DEV: return "devConfig.json"; case DeploymentMode.STAGE: return "stageConfig.json"; case DeploymentMode.PROD: return "prodConfig.json"; default: throw new Error(`Invalid deployment mode: ${mode}`); } };Add this helper near the top of the file (outside the selected range):
// Only allow simple filenames and enforce .json suffix const sanitizeS3Key = (key: string): string => { const normalized = key.endsWith(".json") ? key : `${key}.json`; if (!/^[A-Za-z0-9._-]+\.json$/.test(normalized)) { throw new Error("Invalid CONFIG_FILE_NAME: only A-Z, a-z, 0-9, ., _, - are allowed and must end with .json"); } return normalized; };
46-47: Stop logging the full S3 config; it includes RPC credentials.getS3Config() includes RPC and WSS endpoints sourced from env vars; these often embed API keys. Printing JSON to stdout will leak secrets to CI logs.
Apply this diff to remove the sensitive log and replace it with a safe message:
async function uploadToS3() { const fileName = getFileName(); const bucketName = getBucketName(); const data = getS3Config(); - console.log(JSON.stringify(data, null, 2)); + console.log(`Uploading ${fileName} to S3 bucket ${bucketName}`); const params = { Bucket: bucketName, Key: fileName, Body: JSON.stringify(data, null, 2), ContentType: "application/json", };hardhat-scripts/s3Config/buildConfig.ts (1)
18-27: Deduplicate supportedChainSlugs in buildConfig.tsThe
S3Configtype already declaresevmxChainSlug, so no type change is needed. However, ifchainsever includesEVMX_CHAIN_ID, the current spread will produce duplicates insupportedChainSlugs. This should use aSetto guarantee uniqueness.• File: hardhat-scripts/s3Config/buildConfig.ts
– Replace the assignment at line 19:-export const getS3Config = () => { - const supportedChainSlugs = [EVMX_CHAIN_ID as ChainSlug, ...chains]; +export const getS3Config = () => { + const supportedChainSlugs = Array.from( + new Set<ChainSlug>([EVMX_CHAIN_ID as ChainSlug, ...chains]) + );hardhat-scripts/constants/types.ts (1)
1-1: Import BigNumberish for the updated typeRequired by the proposed
switchboardIdtype change.Apply this diff:
-import { ChainAddressesObj, ChainSlug } from "../../src"; +import { ChainAddressesObj, ChainSlug } from "../../src"; +import { BigNumberish } from "ethers";hardhat-scripts/deploy/9.setupTransmitter.ts (2)
74-82: Deposit only the credit shortfall, not the full thresholdThis should top up to the threshold instead of always depositing the full threshold. Saves funds and avoids over-wrapping on repeated runs.
Apply this diff:
- .wrap(transmitterAddress, { - ...(await overrides(EVMX_CHAIN_ID as ChainSlug)), - value: TRANSMITTER_CREDIT_THRESHOLD, - }); + .wrap(transmitterAddress, { + ...(await overrides(EVMX_CHAIN_ID as ChainSlug)), + // Top up only the deficit + value: TRANSMITTER_CREDIT_THRESHOLD.sub(credits), + });
90-102: Guard against missing provider and fund only the native shortfall
- provider! can be undefined; hard-failing early is safer than confusing runtime errors.
- Send only the deficit to reach threshold. Prevents over-funding the transmitter across repeated runs.
Apply this diff:
export const checkAndDepositNative = async () => { console.log("Checking and depositing native"); - const nativeBalance = await transmitterSigner.provider!.getBalance( + if (!transmitterSigner.provider) { + throw new Error("Transmitter signer has no provider"); + } + const nativeBalance = await transmitterSigner.provider.getBalance( transmitterAddress ); if (nativeBalance.lt(TRANSMITTER_NATIVE_THRESHOLD)) { console.log("Depositing native for transmitter..."); - const tx = await getWatcherSigner().sendTransaction({ + const tx = await getWatcherSigner().sendTransaction({ to: transmitterAddress, - value: TRANSMITTER_NATIVE_THRESHOLD, + // Top up only the deficit + value: TRANSMITTER_NATIVE_THRESHOLD.sub(nativeBalance), ...(await overrides(EVMX_CHAIN_ID as ChainSlug)), });contracts/protocol/SocketUtils.sol (1)
77-90: Critical: Digest calculation changed to use packed encodingThe switch from
abi.encodetoabi.encodePackedfundamentally alters digest calculation. This creates tightly packed data without padding, resulting in different digest values. This is a breaking change that invalidates all existing signatures.Ensure all components generating or verifying these digests are updated:
- Off-chain signers must use the new packed format
- Any stored/cached digests become invalid
- Signature verification will fail for pre-migration payloads
Consider adding a version field or migration path if backward compatibility is needed.
contracts/evmx/AuctionManager.sol (1)
173-178: Critical: Early return without proper state transitionThe function changes from reverting to returning early when auction status is CLOSED or NOT_STARTED. This could leave the system in an inconsistent state if callers expect the auction to transition states.
- if ( - auctionStatus[requestCount_] == AuctionStatus.CLOSED || - auctionStatus[requestCount_] == AuctionStatus.NOT_STARTED - ) return; + if (auctionStatus[requestCount_] == AuctionStatus.CLOSED) { + revert AuctionClosed(); + } + if (auctionStatus[requestCount_] == AuctionStatus.NOT_STARTED) { + revert AuctionNotOpen(); + }test/TriggerTest.t.sol (1)
54-54: Missing _encodeTriggerId functionThe test calls
_encodeTriggerIdon line 54 but this function has been removed according to the summary. This will cause a compilation error.The _encodeTriggerId function is called but not defined. This needs to be either:
- Re-implemented in the test
- Replaced with the new ID construction approach
- Imported from a helper library
- bytes32 triggerId = _encodeTriggerId(address(arbConfig.socket), arbChainSlug); + // Use the new ID construction approach + bytes32 triggerId = keccak256(abi.encodePacked(address(arbConfig.socket), arbChainSlug));contracts/evmx/base/AppGatewayBase.sol (1)
170-177: Return type change from address to bytes32 breaks interface compatibilityChanging
getOnChainAddressreturn type fromaddresstobytes32is a breaking change that will fail for existing integrations expecting an address type.This needs migration support or a new method name:
+// Legacy support - mark as deprecated +function getOnChainAddressLegacy( + bytes32 contractId_, + uint32 chainSlug_ +) public view returns (address) { + bytes32 addr = getOnChainAddress(contractId_, chainSlug_); + if (addr == bytes32(0)) return address(0); + // Convert bytes32 back to address for legacy callers + return address(uint160(uint256(addr))); +} function getOnChainAddress( bytes32 contractId_, uint32 chainSlug_ ) public view returns (bytes32 onChainAddress) {contracts/evmx/watcher/precompiles/WritePrecompile.sol (1)
109-113: Zero address check bypasses connection verificationWhen
targetis zero, the code assignscontractFactoryPlugwithout verifying if it's configured. This could result in using an unconfigured (zero) factory plug.Add validation:
if (queueParams_.transaction.target == bytes32(0)) { - queueParams_.transaction.target = contractFactoryPlugs[ - queueParams_.transaction.chainSlug - ]; + bytes32 factoryPlug = contractFactoryPlugs[queueParams_.transaction.chainSlug]; + if (factoryPlug == bytes32(0)) revert("Contract factory plug not configured"); + queueParams_.transaction.target = factoryPlug; appGateway_ = address(this);hardhat-scripts/admin/disconnect.ts (1)
293-293: Undefined variablesbTypeThe code references
sbTypewhich is not defined in this file. Based on the changes incontracts/evmx/fees/Credit.sol, this should bedeprecatedSbType.- queueParams.switchboardType = sbType; + queueParams.switchboardType = deprecatedSbType;However, since this is a hardhat script and not a contract, this variable needs to be properly defined or imported. This should reference a constant value representing the switchboard type for the disconnection flow.
contracts/evmx/fees/Credit.sol (1)
282-295: Async function pattern needs error handlingThe
_createRequestfunction is marked asasyncbut there's no visible error handling for the asynchronous operations. Thewatcher__().queue()call could fail.Add proper error handling:
- ) internal async { + ) internal async { _setMaxFees(maxFees_); _setOverrides(consumeFrom_); QueueParams memory queueParams; queueParams.overrideParams = overrideParams; queueParams.transaction = Transaction({ chainSlug: chainSlug_, target: _getFeesPlugAddress(chainSlug_), payload: payload_ }); - queueParams.switchboardType = sbType; + queueParams.switchboardType = deprecatedSbType; - watcher__().queue(queueParams, address(this)); + try watcher__().queue(queueParams, address(this)) { + // Success case handled by async callback + } catch { + revert("Failed to queue request"); + } }contracts/evmx/watcher/Configurations.sol (1)
84-94: Missing validation for plug configurationThe function accepts any plug configuration without validating that the appGatewayId or switchboardId are valid.
function setAppGatewayConfigs(AppGatewayConfig[] calldata configs_) external onlyWatcher { for (uint256 i = 0; i < configs_.length; i++) { + // Validate non-zero values + require(configs_[i].plug != bytes32(0), "Invalid plug"); + require(configs_[i].plugConfig.appGatewayId != bytes32(0), "Invalid app gateway ID"); + // Store the plug configuration for this network and plug _plugConfigs[configs_[i].chainSlug][configs_[i].plug] = configs_[i].plugConfig; emit PlugAdded( configs_[i].plugConfig.appGatewayId, configs_[i].chainSlug, configs_[i].plug ); } }
🧹 Nitpick comments (47)
script/counter/DeployEVMxCounterApp.s.sol (3)
16-23: Stop the broadcast after deployments.Explicitly stopping the broadcast avoids accidentally broadcasting any subsequent operations if edits are made later. This is a safe, low-cost guardrail.
vm.startBroadcast(deployerPrivateKey); // Setting fee payment on Arbitrum Sepolia uint256 fees = 1 ether; CounterAppGateway gateway = new CounterAppGateway(addressResolver, fees); + vm.stopBroadcast(); + console.log("Contracts deployed:");
8-8: Fix the command’s filename casing.The file on disk is “DeployEVMxCounterApp.s.sol”, but the comment uses lowercase “deploy…”. On case-sensitive systems this breaks.
-// source .env && forge script script/counter/deployEVMxCounterApp.s.sol --broadcast --skip-simulation --legacy --gas-price 0 +// source .env && forge script script/counter/DeployEVMxCounterApp.s.sol --broadcast --skip-simulation
18-18: Generalize the network-specific comment.This line references Arbitrum Sepolia, but the fork comes from EVMX_RPC, which may point elsewhere. This should avoid naming a specific network.
- // Setting fee payment on Arbitrum Sepolia + // Set fee parameter for the selected fork (value is expected to be in wei)script/supertoken/TransferSuperToken.s.sol (3)
8-8: Update the run command comment; this script doesn’t broadcast.The flags suggest a tx broadcast, but the script only encodes and logs bytes. This should not advise broadcast-related flags.
Apply this diff:
-// source .env && forge script script/supertoken/TransferSuperToken.s.sol --broadcast --skip-simulation --legacy --gas-price 0 +// Usage (no broadcast): source .env && forge script script/supertoken/TransferSuperToken.s.sol -vvvv
20-27: Make units explicit for srcAmount and make deadline intention clear.This should document the unit to avoid accidental human-readable vs base-unit mistakes. Also clarify the deadline’s time horizon.
Apply this diff:
srcAmount: 100, - deadline: block.timestamp + 1000000 + // Amount in base units (e.g., wei). + // Consider parameterizing via env (e.g., SRC_AMOUNT) for reuse. + deadline: block.timestamp + 1_000_000 // ~11.6 days from the forked head
34-35: Log helpful context alongside the payload.Logging the key fields helps consumers validate what the payload contains without local decoding.
Apply this diff:
- console.logBytes(encodedPayload); + console.log("superToken:", gateway.superToken()); + console.log("forwarderArb:", forwarderArb); + console.log("forwarderOpt:", forwarderOpt); + console.log("user:", user); + console.log("payload length:", encodedPayload.length); + console.logBytes(encodedPayload);hardhat-scripts/utils/overrides.ts (1)
72-82: Prefer EIP-1559 fee data when available; expose maxFeePerGas/maxPriorityFeePerGasHard-coding gasPrice and boosting for Sepolia ignores EIP-1559. This should populate maxFeePerGas and maxPriorityFeePerGas when the provider supports it, and fall back to gasPrice otherwise.
Minimal changes to add EIP-1559 support:
export const overrides = async ( chain: ChainSlug | number ): Promise<{ type?: number | undefined; gasLimit?: BigNumberish | undefined; - gasPrice?: BigNumberish | undefined; + gasPrice?: BigNumberish | undefined; + maxFeePerGas?: BigNumberish | undefined; + maxPriorityFeePerGas?: BigNumberish | undefined; }> => {export const getOverrides = async ( chainSlug: ChainSlug, provider: providers.StaticJsonRpcProvider ) => { let overrides = chainOverrides[chainSlug]; - let gasPrice = overrides?.gasPrice; + let gasPrice = overrides?.gasPrice; + let maxFeePerGas: BigNumberish | undefined; + let maxPriorityFeePerGas: BigNumberish | undefined; @@ - if (gasPrice == undefined || gasPrice == null) - gasPrice = (await getGasPrice(chainSlug, provider)).toNumber(); + if (gasPrice == undefined || gasPrice == null) { + const feeData = await provider.getFeeData(); + // Prefer EIP-1559 if available + if (feeData.maxFeePerGas && feeData.maxPriorityFeePerGas) { + maxFeePerGas = feeData.maxFeePerGas; + maxPriorityFeePerGas = feeData.maxPriorityFeePerGas; + } else { + gasPrice = await getGasPrice(chainSlug, provider); + } + } @@ - return { gasLimit, gasPrice, type }; + return { gasLimit, gasPrice, maxFeePerGas, maxPriorityFeePerGas, type };If you keep the Sepolia 1.5x heuristic (Lines 78–81), apply it to whichever field you actually use (gasPrice or maxFeePerGas) to avoid underpricing on testnets.
script/helpers/DepositCredit.s.sol (2)
12-12: Make token amount clearer; prefer 2e6 literal or env override.Using 2e6 conveys USDC’s 6 decimals. Keeping it configurable via env avoids code edits per run.
Apply this diff for readability:
- uint256 feesAmount = 2000000; // 2 USDC + uint256 feesAmount = 2e6; // 2 USDC (6 decimals)Optionally, allow an env override:
uint256 feesAmount = vm.envOr("FEES_AMOUNT_USDC", uint256(2e6));
28-30: Fix misleading log label: USDC is not wei.This should avoid confusion when reading logs. USDC has 6 decimals.
- console.log("Sender balance in wei:", balance); + console.log("Sender USDC balance (base units, 6 decimals):", balance);package.json (1)
15-24: Add prepack to guarantee dist is built before publishThis should ensure consumers always get a built package even if someone publishes directly without using the custom script.
Apply this diff:
"scripts": { "build": "yarn abi && tsc --project lib.tsconfig.json", "tsc": "tsc --project lib.tsconfig.json", "abi": "hardhat export-abi", "lint": "prettier \"./**\" --write", "lintContracts": "prettier \"./**\" --write --plugin=prettier-plugin-solidity", "compile": "forge build", "deploy": "bash setupInfraContracts.sh", "publish-core": "yarn build && yarn publish --patch --no-git-tag-version", - "trace": "source .env && bash trace.sh" + "trace": "source .env && bash trace.sh", + "prepack": "yarn build" },hardhat-scripts/verify/verify.ts (1)
57-73: Add minimal tracing to see retry progress and failuresThis should log the attempt number and how many items remain, which helps diagnose Etherscan flakiness and rate limiting.
You can augment the loop:
- while (retryCount < 5) { + while (retryCount < 5) { + console.log(`Verify attempt #${retryCount + 1} for ${chainName}`); const unverifiedChainParams: VerifyArgs[] = []; if (pendingParams.length) { const len = pendingParams.length; for (let index = 0; index < len; index++) { const res = await verify(...pendingParams[index]); if (!res) { unverifiedChainParams.push(pendingParams[index]); } } } await storeUnVerifiedParams(unverifiedChainParams, chain, mode); - if (unverifiedChainParams.length == 0) break; + console.log(`Remaining unverified: ${unverifiedChainParams.length}`); + if (unverifiedChainParams.length == 0) break; pendingParams = unverifiedChainParams; retryCount++; await new Promise((resolve) => setTimeout(resolve, 1000)); }hardhat-scripts/constants/constants.ts (1)
12-13: Use constants.HashZero for a canonical bytes32 zero.
ethers.utils.hexZeroPad(constants.AddressZero, 32)yields 0x00…00, but it’s indirect. This should useconstants.HashZerofor clarity and to avoid depending on hex padding semantics.Apply this diff:
-export const BYTES32_ZERO = ethers.utils.hexZeroPad(constants.AddressZero, 32); +export const BYTES32_ZERO = constants.HashZero;contracts/utils/common/IdUtils.sol (1)
5-7: Enhance NatSpec to document bit layout.Explicitly documenting the bit ranges eliminates ambiguity and future regressions.
Apply this diff:
-/// @param payloadPointer_ The payload pointer -/// @param switchboardId_ The switchboard id +/// @param payloadPointer_ The payload pointer (lower 160 bits: [159:0]) +/// @param switchboardId_ The switchboard id (next 64 bits: [223:160]) /// @param chainSlug_ The chain slugcontracts/utils/common/Constants.sol (1)
21-22: Solana chain slug values need documentationThe constants CHAIN_SLUG_SOLANA_MAINNET (10000001) and CHAIN_SLUG_SOLANA_DEVNET (10000002) use a different numbering scheme than typical EVM chain IDs. Document the rationale for this 10000000+ range.
Add documentation:
+// Solana chain slugs use 10000000+ range to avoid conflicts with EVM chain IDs uint32 constant CHAIN_SLUG_SOLANA_MAINNET = 10000001; uint32 constant CHAIN_SLUG_SOLANA_DEVNET = 10000002;src/types.ts (2)
22-26: Use consistent ID type naming conventionThe ID fields mix different naming patterns.
FastSwitchboardId,CCTPSwitchboardId, andMessageSwitchboardIdshould follow the same convention as the address fields.- FastSwitchboardId: string; - CCTPSwitchboardId: string; - MessageSwitchboardId: string; + fastSwitchboardId: string; + cctpSwitchboardId: string; + messageSwitchboardId: string;
31-31: Use specific type for switchboard ID mappingThe
SwitchboardIdToAddressMapshould use a more specific key type since switchboard IDs are uint64 in the contracts.Consider defining a type alias or using a more specific type annotation:
type SwitchboardId = string; // represents uint64 SwitchboardIdToAddressMap: { [switchboardId: SwitchboardId]: string };This improves type safety and makes the expected format clearer.
contracts/protocol/interfaces/ISocketBatcher.sol (1)
19-26: Add parameter documentation for switchboardId_The function documentation should include the new
switchboardId_parameter./** * @notice Attests a payload and executes it * @param executeParams_ The execution parameters + * @param switchboardId_ The ID of the switchboard to use for attestation * @param digest_ The digest of the payload * @param proof_ The proof of the payload * @param transmitterSignature_ The signature of the transmitter + * @param refundAddress_ The address to refund excess fees to * @return The return data after execution */contracts/evmx/plugs/FeesPlug.sol (1)
119-125: Document the new switchboardId parameterThe function lacks documentation for its parameters, especially the new
switchboardId_parameter which is part of the migration to ID-based addressing.Add documentation before the function:
/// @notice Connects the plug to a socket /// @param appGatewayId_ The ID of the app gateway /// @param socket_ The socket address /// @param switchboardId_ The ID of the switchboard to usescript/supertoken/DeployEVMxSuperTokenApp.s.sol (2)
6-6: Relative import path could break when script location changes.This relative import path
../../test/apps/app-gateways/super-token/SuperTokenAppGateway.solmakes the script fragile to directory restructuring. This should use an absolute import path from the project root.-import "../../test/apps/app-gateways/super-token/SuperTokenAppGateway.sol"; +import "test/apps/app-gateways/super-token/SuperTokenAppGateway.sol";
8-8: Remove commented deployment command from production code.This deployment command comment should be moved to documentation or removed from the source file.
-// source .env && forge script script/supertoken/deployEVMxSuperTokenApp.s.sol --broadcast --skip-simulation --legacy --gas-price 0hardhat-scripts/misc-scripts/createLabels.ts (3)
1-1: Align file header/comment with actual path to avoid confusionThe header says scripts/generate-labels.ts, but this file lives at hardhat-scripts/misc-scripts/createLabels.ts. This should not mislead users running the script.
Apply this diff:
-// scripts/generate-labels.ts +// hardhat-scripts/misc-scripts/createLabels.ts
12-15: Ensure clean section replacement and separationIf foundry.toml doesn’t end with a newline, the new [labels] header will attach to the last line. Add a separator.
Apply this diff:
- // Generate new labels section - let labelsSection = "[labels]\n"; + // Generate new labels section + let labelsSection = (foundryContent.endsWith("\n") ? "" : "\n") + "[labels]\n";Also applies to: 20-23
62-64: Document correct CLI usageNode can’t execute TS directly without ts-node. Either ship the compiled JS or document using ts-node. Also the filename in the comment should match.
Apply this diff:
-// Get chainSlug from command line arguments -const chainSlug = process.argv[2]; -generateFoundryLabels(chainSlug); +// Usage: +// npx ts-node hardhat-scripts/misc-scripts/createLabels.ts <chainId?> +// or run the compiled JS equivalent +const chainSlug = process.argv[2]; +generateFoundryLabels(chainSlug);contracts/evmx/interfaces/IAsyncDeployer.sol (1)
23-33: Update NatSpec to reflect bytes32-based chain contract identifierThese functions shifted from address to bytes32 for chainContractAddress_. This should be documented to avoid misuse and to guide encoding (e.g., Converters.toBytes32Format).
Apply this diff:
// Forwarder Management - function getOrDeployForwarderContract( + /// @notice Get or deploy the forwarder for a given chain contract identifier + /// @param chainContractAddress_ Chain-specific contract identifier (bytes32-encoded address/ID) + /// @param chainSlug_ Chain slug/domain identifier + function getOrDeployForwarderContract( bytes32 chainContractAddress_, uint32 chainSlug_ ) external returns (address); - function getForwarderAddress( + /// @notice Resolve the forwarder address for a given chain contract identifier (no deploy) + /// @param chainContractAddress_ Chain-specific contract identifier (bytes32-encoded address/ID) + /// @param chainSlug_ Chain slug/domain identifier + function getForwarderAddress( bytes32 chainContractAddress_, uint32 chainSlug_ ) external view returns (address);script/helpers/DepositCreditMainnet.s.sol (4)
7-8: Remove unused importTestUSDC is not used.
Apply this diff:
-import {TestUSDC} from "../../contracts/evmx/mocks/TestUSDC.sol";
9-11: Fix script header comment to match file and intended usageThe header points to a different script name. Keep the command accurate to avoid operator error.
Apply this diff:
-// source .env && forge script script/helpers/DepositCreditAndNative.s.sol --broadcast --skip-simulation +// source .env && forge script script/helpers/DepositCreditMainnet.s.sol --rpc-url "$ARBITRUM_RPC" --broadcast --skip-simulation
16-35: Stop broadcasting at the end of the scriptThis should call vm.stopBroadcast() to clearly delimit the broadcast window.
Apply this diff:
console.log("Fees Amount:", feesAmount); feesPlug.depositCredit(address(USDCContract), appGateway, feesAmount); + vm.stopBroadcast(); }
28-30: Use require for balance check for consistent styleUse require with message to match the rest of the guard style.
Apply this diff:
- if (balance < feesAmount) { - revert("Sender does not have enough USDC"); - } + require(balance >= feesAmount, "Sender does not have enough USDC");script/helpers/TransferRemainingCredits.s.sol (1)
34-35: Low-level call lacks detailed error reportingThe generic "Transfer failed" message doesn't help diagnose why the transfer failed. The return data could contain useful revert reasons.
Decode and report the actual error:
- (bool success, ) = appGateway.call(data); - require(success, "Transfer failed"); + (bool success, bytes memory returnData) = appGateway.call(data); + if (!success) { + if (returnData.length > 0) { + assembly { + revert(add(32, returnData), mload(returnData)) + } + } else { + revert("Transfer failed without error data"); + } + }contracts/protocol/interfaces/ISwitchboard.sol (1)
40-44: Duplicate @notice tags in documentationThe function has two
@noticetags on lines 33 and 34, which is non-standard documentation practice.Merge the documentation:
/** - * @notice Gets the transmitter for a given payload - * @notice Switchboard are required to implement this function to allow for the verification of the transmitters + * @notice Gets the transmitter for a given payload. Switchboards are required to implement this function to allow for the verification of the transmitters * @param sender_ The sender of the payload * @param payloadId_ The payload ID * @param transmitterSignature_ The transmitter signature * @return The transmitter address */contracts/evmx/helpers/AsyncDeployer.sol (1)
163-164: tryCall error handling loses gas informationUsing
tryCallwithgasleft()and then requiring success doesn't provide information about whether the failure was due to insufficient gas or a revert in the initialization logic.Consider using a more specific gas amount or capturing the return data:
- (bool success, , ) = proxy.tryCall(0, gasleft(), 0, initData_); - require(success, "Initialization failed"); + (bool success, bytes memory returnData, ) = proxy.tryCall(0, gasleft(), 0, initData_); + if (!success) { + if (returnData.length > 0) { + assembly { + revert(add(32, returnData), mload(returnData)) + } + } else { + revert("Initialization failed without error data"); + } + }contracts/evmx/plugs/ContractFactoryPlug.sol (1)
58-59: Conditional logic should validate enum value explicitlyUsing direct equality check with enum is less readable. This should use explicit enum comparison for clarity.
- if (isPlug_ == IsPlug.YES) - IPlug(addr).initSocket(appGatewayId_, msg.sender, switchboardId_); + if (isPlug_ == IsPlug.YES) { + IPlug(addr).initSocket(appGatewayId_, msg.sender, switchboardId_); + }hardhat-scripts/utils/sign.ts (2)
35-35: Unnecessary type casting to stringThe
as stringcast on line 35 is redundant sinceEVMxAddressesObjalready definesWatcheras a string type.- evmxAddresses[Contracts.Watcher] as string, + evmxAddresses[Contracts.Watcher],
51-51: Duplicate unnecessary type castingSame redundant
as stringcast as in line 35.- evmxAddresses[Contracts.Watcher] as string + evmxAddresses[Contracts.Watcher]src/enums.ts (1)
58-62: ID entries should follow consistent naming patternThe new ID entries mix "Id" suffix inconsistently. All switchboard ID entries should use consistent casing.
- FastSwitchboardId = "FastSwitchboardId", + FastSwitchboardID = "FastSwitchboardID", - CCTPSwitchboardId = "CCTPSwitchboardId", + CCTPSwitchboardID = "CCTPSwitchboardID", - MessageSwitchboardId = "MessageSwitchboardId", + MessageSwitchboardID = "MessageSwitchboardID",contracts/evmx/watcher/WatcherStorage.sol (1)
19-23: Storage slot comment needs updateThe comment on line 19 indicates "slot 50 (32 + 32)" but should be "(32 + 32 = 64 bits)" to clarify that two uint32 values are packed into a single 256-bit slot.
- // slot 50 (32 + 32) + // slot 50 (32 + 32 = 64 bits used)contracts/evmx/helpers/Forwarder.sol (1)
24-29: Typo in commentLine 24 has a typo: "kep" should be "kept".
- /// @notice old on-chain address kep for storage compatibility - can be removed after redeployment + /// @notice old on-chain address kept for storage compatibility - can be removed after redeploymenttest/MessageSwitchboardTest.t.sol (1)
26-46: Missing validation of digest creation in testThe test doesn't validate that
_createDigestreturns a valid digest before using it in assertions.Add digest validation:
function testIncrementWithMessageSwitchboard() public { (uint160 payloadPointer, DigestParams memory digestParams) = _getTriggerData( arbPlug, optPlug, arbConfig, optConfig, abi.encodeWithSignature("increment()") ); + + bytes32 digest = _createDigest(digestParams); + require(digest != bytes32(0), "Invalid digest created"); vm.expectEmit(true, true, true, true); emit TriggerProcessed( optConfig.chainSlug, msgSbFees, - _createDigest(digestParams), + digest, digestParams );contracts/protocol/interfaces/ICCTPSwitchboard.sol (1)
8-12: Fix incorrect interface documentationThe NatSpec comment says "ISwitchboard" but this is ICCTPSwitchboard interface.
/** - * @title ISwitchboard + * @title ICCTPSwitchboard * @dev The interface for a switchboard contract that is responsible for verification of payloads if the correct * digest is executed. */contracts/protocol/Socket.sol (2)
54-55: gasLimitBuffer should be immutableThe
gasLimitBufferis set in constructor but never modified. Should be declared as immutable for gas optimization.Declare gasLimitBuffer as immutable in the base contract:
+uint256 public immutable gasLimitBuffer; constructor( uint32 chainSlug_, address owner_, string memory version_ ) SocketUtils(chainSlug_, owner_, version_) { gasLimitBuffer = 105; }
203-204: Unused return value from overrides() callThe
plugOverridesis retrieved but might be empty. Should validate if overrides are actually needed.Consider checking if overrides are non-empty before processing:
bytes memory plugOverrides = IPlug(plug_).overrides(); +// Only process if overrides exist +if (plugOverrides.length == 0) { + plugOverrides = ""; +}contracts/evmx/fees/Credit.sol (1)
297-300: Inconsistent zero check patternThe function checks if
feesPlugs[chainSlug_] == bytes32(0)but this differs from the BYTES32_ZERO constant used elsewhere in the codebase.Use a consistent pattern for zero checks across the codebase. Consider importing and using the BYTES32_ZERO constant for clarity.
EventTopics.md (1)
22-38: Documentation structure needs consistency with table formattingThe Socket event table has inconsistent column widths compared to other sections. The Arguments column is significantly wider due to the new complex signatures, causing misalignment.
Apply consistent column formatting:
-| Event | Arguments | Topic | -| ---------------------------- | --------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------- | +| Event | Arguments | Topic | +| ---------------------------- | ------------------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------- |test/SetupTest.t.sol (2)
499-515: Extract signature creation to reduce duplicationThe signature creation logic with the "\x19Ethereum Signed Message:\n32" prefix and assembly operations is duplicated across multiple test files. This should be centralized.
Create a dedicated signature helper contract or library:
+contract SignatureHelper { + function createSignature(bytes32 digest_, uint256 privateKey_) public pure returns (bytes memory sig) { + bytes32 digest = keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", digest_)); + (uint8 sigV, bytes32 sigR, bytes32 sigS) = vm.sign(privateKey_, digest); + sig = new bytes(65); + bytes1 v32 = bytes1(sigV); + assembly { + mstore(add(sig, 96), v32) + mstore(add(sig, 32), sigR) + mstore(add(sig, 64), sigS) + } + } +}
1458-1464: Helper functions belong in a dedicated utilities contractThese conversion functions are generic utilities that will likely be needed across multiple test files. They should be in a shared location.
Move to a dedicated test utilities contract:
+// In test/utils/Converters.t.sol +library TestConverters { + function addressToBytes32(address addr_) internal pure returns (bytes32) { + return bytes32(uint256(uint160(addr_))); + } + + function bytes32ToAddress(bytes32 addrBytes32_) internal pure returns (address) { + return address(uint160(uint256(addrBytes32_))); + } +}Then import and use in test files:
+import "./utils/Converters.t.sol"; -function addressToBytes32(address addr_) pure returns (bytes32) { - return bytes32(uint256(uint160(addr_))); -} - -function bytes32ToAddress(bytes32 addrBytes32_) pure returns (address) { - return address(uint160(uint256(addrBytes32_))); -}contracts/protocol/switchboard/MessageSwitchboard.sol (1)
98-109: registerSibling function vulnerable to griefingAny address can call
registerSiblingand overwrite their sibling plug mapping without authorization checks. While this only affects the caller's own mapping, it could lead to accidental misconfiguration.Add a confirmation mechanism to prevent accidental overwrites:
function registerSibling(uint32 chainSlug_, bytes32 siblingPlug_) external { if ( siblingSockets[chainSlug_] == bytes32(0) || siblingSwitchboards[chainSlug_] == bytes32(0) ) { revert SiblingNotFound(); } + + // Prevent accidental overwrites + bytes32 existing = siblingPlugs[chainSlug_][msg.sender]; + if (existing != bytes32(0) && existing != siblingPlug_) { + revert SiblingAlreadyRegistered(); + } // Register the sibling for the calling plug siblingPlugs[chainSlug_][msg.sender] = siblingPlug_; emit SiblingRegistered(chainSlug_, msg.sender, siblingPlug_); }FunctionSignatures.md (1)
1-643: Documentation format inconsistency and missing contextThe function signatures table lacks important context about parameter types and return values. This makes it difficult to understand the actual interface changes without cross-referencing the code.
Consider enhancing the documentation format to include parameter and return types:
## AuctionManager -| Function | Signature | -| ---------------------------- | ------------ | -| `addressResolver__` | `0x6a750469` | +| Function | Signature | Parameters | Returns | +| ---------------------------- | ------------ | ---------- | ------- | +| `addressResolver__` | `0x6a750469` | () | address |This would make the documentation more useful for developers integrating with these contracts.
| address owner_, | ||
| bytes32 sbType_ | ||
| ) public reinitializer(1) { | ||
| ) public reinitializer(2) { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify reinitializer version increment is intentional
The reinitializer changed from version 1 to 2. This should only happen when upgrading an already deployed contract. Ensure existing deployments are properly migrated.
🏁 Script executed:
#!/bin/bash
# Check for migration scripts or deployment history
# Look for deployment scripts that handle this upgrade
fd -e sol -e js -e ts | xargs rg -l "FeesManager.*reinitializer|upgrade.*FeesManager"
# Check for other reinitializer(2) usage patterns
ast-grep --pattern 'reinitializer(2)'
# Look for migration documentation
fd README -e md | xargs rg -i "migration|upgrade.*fees"Length of output: 245
Add migration logic for FeesManager reinitializer bump
The constructor was changed to reinitializer(2) without any migration scripts or documented upgrade path. This should only happen when upgrading an existing deployment—existing contracts will skip the new initializer and miss any added state. Provide explicit migration logic and tests to ensure storage slot compatibility:
- contracts/evmx/fees/FeesManager.sol (line 51): implement an upgrade initializer or external migration script to move from version 1 → 2.
- Add tests that deploy the v1 contract, perform the upgrade, then verify storage slots and new state are correctly initialized.
| /// @notice get the switchboard type | ||
| /// @return sbType The switchboard type | ||
| function sbType() external view returns (bytes32); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
New sbType() requirement: ensure all gateways implement and tests assert it
This should be implemented across concrete AppGateways and validated in tests to prevent silent zero-value returns.
Search for implementations missing sbType:
🏁 Script executed:
#!/bin/bash
# Find contracts implementing IAppGateway but missing sbType()
rg -n -A 5 -B 3 $'contract .* is .*IAppGateway'
rg -n -A 2 -B 2 $'function sbType\\s*\\(\\)'Length of output: 1178
🏁 Script executed:
#!/bin/bash
# List all concrete AppGateway contracts extending the abstract base
rg -n 'contract .* is .*AppGatewayBase'Length of output: 1221
🏁 Script executed:
#!/bin/bash
# Display AppGatewayBase.sol contents around potential sbType implementation
sed -n '1,200p' contracts/evmx/base/AppGatewayBase.solLength of output: 7193
🏁 Script executed:
#!/bin/bash
# Check where gateways call the base initializer
rg -n "_initializeAppGateway" -n contracts/evmx
# Inspect AuctionManager initializer and sbType usage
rg -n -A3 -B3 "function initialize" contracts/evmx/AuctionManager.sol
rg -n "sbType" -n contracts/evmx/AuctionManager.solLength of output: 721
Add sbType() assertions in gateway tests
All concrete gateways already inherit the public sbType getter via AppGatewayBase, but no tests currently validate it—silent zero‐value defaults could slip through. This should be covered in every gateway’s test:
• In test/apps/Counter.t.sol: after deploying/initializing CounterAppGateway, assert gateway.sbType() == FAST.
• In test/apps/SuperToken.t.sol: after setup of SuperTokenAppGateway, assert gateway.sbType() == FAST.
• In test/apps/ParallelCounter.t.sol: assert gateway.sbType() == FAST.
• In test/AuctionManager.t.sol: after initializing AuctionManager, assert auctionManager.sbType() == FAST.
• In any future gateway tests, include the same check.
Example snippet to add:
import { FAST } from "../utils/common/Constants.sol";
function test_sbTypeIsFast() public {
- // existing setup...
+ // setup gateway
+ GatewayType gateway = new GatewayType();
+ gateway.initialize(addressResolver);
+
+ // verify sbType
+ assertEq(gateway.sbType(), FAST, "sbType must be FAST");
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In contracts/evmx/interfaces/IAppGateway.sol around lines 44 to 47, the public
sbType getter is present but tests are missing assertions that concrete gateways
return the expected value; add assertions in the specified test files
immediately after each gateway is deployed/initialized: in
test/apps/Counter.t.sol, test/apps/SuperToken.t.sol,
test/apps/ParallelCounter.t.sol, and test/AuctionManager.t.sol assert
gateway.sbType() == FAST (or auctionManager.sbType() == FAST) using the same
FAST constant/import used elsewhere in the test suite; ensure the assertion is
added to any future gateway tests as well so zero-value defaults cannot slip
through.
| function triggerFromChainSlug() external view returns (uint32); | ||
|
|
||
| function triggerFromPlug() external view returns (bytes32); | ||
|
|
||
| function isAppGatewayCalled(bytes32 triggerId) external view returns (bool); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Document and verify new getters across implementations.
These public getters change the interface surface. This should:
- Add brief NatSpec for each to define semantics and storage origin.
- Verify all Watcher implementations conform.
I can add concise NatSpec like:
- triggerFromChainSlug: “Chain slug of the current trigger context”
- triggerFromPlug: “Plug (bytes32) of the current trigger context”
- isAppGatewayCalled: “Whether AppGateway call occurred for triggerId”
Run this to spot implementing contracts needing updates:
🏁 Script executed:
#!/bin/bash
rg -n $'contract\\s+Watcher\\b|is AppGatewayCalled|triggerFromChainSlug|triggerFromPlug|function\\s+triggerFromChainSlug\\(|function\\s+triggerFromPlug\\(' -A 3 -B 2Length of output: 3442
🏁 Script executed:
#!/bin/bash
rg -n "mapping.*isAppGatewayCalled" -A3 -B3 contracts/evmx/watcher/WatcherStorage.solLength of output: 450
Document IWatcher getters with NatSpec
Implementation already includes storage‐level comments in WatcherStorage.sol. This should be reflected in the interface to define semantics and storage origin.
• File: contracts/evmx/interfaces/IWatcher.sol
Add above each function:
- function triggerFromChainSlug() external view returns (uint32);
+ /// @notice Chain slug of the current trigger context stored in WatcherStorage
+ /// @return chainSlug slug of the origin chain for the current trigger
+ function triggerFromChainSlug() external view returns (uint32);
- function triggerFromPlug() external view returns (bytes32);
+ /// @notice Plug (bytes32) of the current trigger context stored in WatcherStorage
+ /// @return plug identifier of the plug for the current trigger
+ function triggerFromPlug() external view returns (bytes32);
- function isAppGatewayCalled(bytes32 triggerId) external view returns (bool);
+ /// @notice Whether AppGateway has been called for the given triggerId
+ /// @param triggerId identifier of the trigger to check
+ /// @return called true if AppGateway call occurred for triggerId
+ function isAppGatewayCalled(bytes32 triggerId) external view returns (bool);No other Watcher implementations exist; WatcherStorage.sol already uses override and carries matching NatSpec.
📝 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.
| function triggerFromChainSlug() external view returns (uint32); | |
| function triggerFromPlug() external view returns (bytes32); | |
| function isAppGatewayCalled(bytes32 triggerId) external view returns (bool); | |
| /// @notice Chain slug of the current trigger context stored in WatcherStorage | |
| /// @return chainSlug slug of the origin chain for the current trigger | |
| function triggerFromChainSlug() external view returns (uint32); | |
| /// @notice Plug (bytes32) of the current trigger context stored in WatcherStorage | |
| /// @return plug identifier of the plug for the current trigger | |
| function triggerFromPlug() external view returns (bytes32); | |
| /// @notice Whether AppGateway has been called for the given triggerId | |
| /// @param triggerId identifier of the trigger to check | |
| /// @return called true if AppGateway call occurred for triggerId | |
| function isAppGatewayCalled(bytes32 triggerId) external view returns (bool); |
🤖 Prompt for AI Agents
In contracts/evmx/interfaces/IWatcher.sol around lines 46 to 51, the three
getter declarations lack NatSpec; add Ethereum NatSpec comments above each
function mirroring the storage-level comments in WatcherStorage.sol to document
semantics and storage origin. For each getter, add a brief description line, an
@return tag describing the returned value and its units/type, and note that the
value is read from WatcherStorage (or the matching storage variable name) so
implementations and callers understand provenance; keep wording identical to the
existing WatcherStorage.sol comments and ensure formatting follows Solidity
NatSpec conventions.
| function increaseFees(uint40 requestCount_, uint256 newFees_) external; | ||
|
|
||
| function setIsValidPlug(bool isValid_, uint32 chainSlug_, address onchainAddress_) external; | ||
| function setIsValidPlug(bool isValid_, uint32 chainSlug_, bytes32 onchainAddress_) external; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Breaking change: on-chain identifier is bytes32; rename param for clarity.
The type change is correct for ID-based plugs, but the name onchainAddress_ now misleads (it’s not an address). Rename to reflect it’s an ID.
Apply this diff:
- function setIsValidPlug(bool isValid_, uint32 chainSlug_, bytes32 onchainAddress_) external;
+ function setIsValidPlug(bool isValid_, uint32 chainSlug_, bytes32 onchainId_) external;Note: Param rename does not affect ABI, but all impls should update for readability.
🏁 Script executed:
#!/bin/bash
rg -n $'setIsValidPlug\\s*\\(' -A 1 -B 2Length of output: 2923
🏁 Script executed:
#!/bin/bash
# Interface IConfigurations
echo "=== contracts/evmx/interfaces/IConfigurations.sol (lines 44–55) ==="
sed -n '44,55p' contracts/evmx/interfaces/IConfigurations.sol
# Implementation Configurations.sol
echo
echo "=== contracts/evmx/watcher/Configurations.sol (lines 120–135) ==="
sed -n '120,135p' contracts/evmx/watcher/Configurations.sol
# Mock precompile
echo
echo "=== test/mock/MockWatcherPrecompile.sol (lines 50–70) ==="
sed -n '50,70p' test/mock/MockWatcherPrecompile.solLength of output: 2372
Rename onchainAddress_ to onchainId_ in IWatcher and all implementations
This should clarify that the third parameter is an identifier, not an address.
This should update:
- contracts/evmx/interfaces/IWatcher.sol: change
bytes32 onchainAddress_→bytes32 onchainId_ - test/mock/MockWatcherPrecompile.sol: change
bytes32 onchainAddress_→bytes32 onchainId_ - contracts/evmx/watcher/Watcher.sol: rename function param
plug_→onchainId_and propagate it to the call - contracts/evmx/base/AppGatewayBase.sol: rename local var
onchainAddress→onchainIdand pass it
Diff examples:
--- a/contracts/evmx/interfaces/IWatcher.sol
@@ -87,7 +87,7 @@
function increaseFees(uint40 requestCount_, uint256 newFees_) external;
- function setIsValidPlug(bool isValid_, uint32 chainSlug_, bytes32 onchainAddress_) external;
+ function setIsValidPlug(bool isValid_, uint32 chainSlug_, bytes32 onchainId_) external;--- a/test/mock/MockWatcherPrecompile.sol
@@ -51,7 +51,7 @@
function increaseFees(uint40 requestCount_, uint256 newFees_) external override {}
- function setIsValidPlug(
- bool isValid_,
- uint32 chainSlug_,
- bytes32 onchainAddress_
- ) external override {}
+ function setIsValidPlug(
+ bool isValid_,
+ uint32 chainSlug_,
+ bytes32 onchainId_
+ ) external override {}--- a/contracts/evmx/watcher/Watcher.sol
@@ -155,7 +155,7 @@
- function setIsValidPlug(bool isValid_, uint32 chainSlug_, bytes32 plug_) external override {
+ function setIsValidPlug(bool isValid_, uint32 chainSlug_, bytes32 onchainId_) external override {
configurations__.setIsValidPlug(isValid_, chainSlug_, onchainId_, msg.sender);
}--- a/contracts/evmx/base/AppGatewayBase.sol
@@ -224,7 +224,7 @@
- bytes32 onchainAddress = getOnChainAddress(contractId_, chainSlug_);
- watcher__().setIsValidPlug(isValid, chainSlug_, onchainAddress);
+ bytes32 onchainId = getOnChainAddress(contractId_, chainSlug_);
+ watcher__().setIsValidPlug(isValid, chainSlug_, onchainId);📝 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.
| function setIsValidPlug(bool isValid_, uint32 chainSlug_, bytes32 onchainAddress_) external; | |
| function setIsValidPlug(bool isValid_, uint32 chainSlug_, bytes32 onchainId_) external; |
🤖 Prompt for AI Agents
In contracts/evmx/interfaces/IWatcher.sol around line 90, rename the third
parameter from bytes32 onchainAddress_ to bytes32 onchainId_; then update all
implementations and call sites accordingly: in
test/mock/MockWatcherPrecompile.sol change the function signature to use
onchainId_; in contracts/evmx/watcher/Watcher.sol rename the function parameter
plug_ to onchainId_ and forward that value to the IWatcher call; in
contracts/evmx/base/AppGatewayBase.sol rename local variable onchainAddress to
onchainId and pass onchainId into the Watcher call so parameter names match
across interface, implementations, and callers.
| function disconnectSocket() external onlyOwner { | ||
| socket__.disconnect(); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add documentation and consider security implications
The new disconnectSocket function lacks documentation. This is a critical operation that forcibly disconnects the socket.
+ /// @notice Forcibly disconnects the socket
+ /// @dev This will disconnect the plug from the socket, preventing any further operations
+ /// @dev Ensure this is only called when absolutely necessary as it may leave the system in an inconsistent state
function disconnectSocket() external onlyOwner {
socket__.disconnect();
}Also verify that disconnecting won't leave fees stuck or cause issues with in-flight transactions.
🏁 Script executed:
#!/bin/bash
# Check for any cleanup or state management around disconnect operations
ast-grep --pattern 'function disconnect() {
$$$
}'
# Look for any disconnect event emissions or state cleanup
rg -A 10 "disconnect\(\)" --type solidityLength of output: 131
🏁 Script executed:
#!/bin/bash
# Search for disconnect implementations in Solidity contracts
rg -n "function disconnect" -g "*.sol"
# Search for any calls to disconnect across Solidity files
rg -n "disconnect(" -g "*.sol"
# Locate the declaration of socket__ to identify its interface and implementation
rg -n "socket__" -g "*.sol"Length of output: 4228
🏁 Script executed:
#!/bin/bash
# Inspect implementation of disconnect in SocketConfig.sol
rg -n -C5 "function disconnect" -g "contracts/protocol/SocketConfig.sol"
# Inspect implementation of disconnect in PlugBase.sol
rg -n -C5 "function disconnect" -g "contracts/protocol/base/PlugBase.sol"
# Check for any state cleanup logic (delete/remove) in these methods
rg -n -C5 "delete" -g "contracts/protocol/SocketConfig.sol"
rg -n -C5 "delete" -g "contracts/protocol/base/PlugBase.sol"
# Search for event emissions related to disconnect
rg -n -C5 "emit" -g "contracts/protocol/SocketConfig.sol"
rg -n -C5 "emit" -g "contracts/protocol/base/PlugBase.sol"Length of output: 6077
🏁 Script executed:
#!/bin/bash
# Show FeesPlug.sol around the disconnectSocket function and class declaration
rg -n -C10 "contract FeesPlug" -g "contracts/evmx/plugs/FeesPlug.sol"
rg -n -C10 "function disconnectSocket" -g "contracts/evmx/plugs/FeesPlug.sol"Length of output: 3352
Add NatSpec, emit proper event, and handle fee cleanup
This should:
- Add NatSpec to
disconnectSocketexplaining its impact. - Call internal
_disconnectSocket()(notsocket__.disconnect()directly) soConnectorPlugDisconnectedis emitted. - Ensure any locked fees are rescued (e.g. via
rescueFunds) before or after disconnect to prevent stuck balances.
Points of attention:
- File:
contracts/evmx/plugs/FeesPlug.solat lines 127–129.
Proposed diff:
/// @notice Disconnects this plug from its socket, preventing further fee operations.
/// @dev Calls internal `_disconnectSocket()` to emit `ConnectorPlugDisconnected`
/// and clear plug state on the socket side. Any remaining fees should be
/// rescued via `rescueFunds` to avoid stuck balances.
- function disconnectSocket() external onlyOwner {
- socket__.disconnect();
- }
+ function disconnectSocket() external onlyOwner {
+ _disconnectSocket();
+ }📝 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.
| function disconnectSocket() external onlyOwner { | |
| socket__.disconnect(); | |
| } | |
| /// @notice Disconnects this plug from its socket, preventing further fee operations. | |
| /// @dev Calls internal `_disconnectSocket()` to emit `ConnectorPlugDisconnected` | |
| /// and clear plug state on the socket side. Any remaining fees should be | |
| /// rescued via `rescueFunds` to avoid stuck balances. | |
| function disconnectSocket() external onlyOwner { | |
| _disconnectSocket(); | |
| } |
🤖 Prompt for AI Agents
In contracts/evmx/plugs/FeesPlug.sol around lines 127–129, the disconnectSocket
function lacks NatSpec, does not use the internal disconnect routine that emits
ConnectorPlugDisconnected, and may leave locked fees stuck; add a NatSpec
comment describing that this disconnects the plug and may reject further
operations, replace the direct socket__.disconnect() call with a call to the
internal _disconnectSocket() so the ConnectorPlugDisconnected event is emitted,
and invoke the fee rescue path (e.g. call rescueFunds or an internal
_rescueLockedFees()) either immediately before or after calling
_disconnectSocket() to ensure any locked fees are returned; preserve the
onlyOwner modifier and error handling.
| SuperTokenAppGateway gateway = SuperTokenAppGateway(vm.envAddress("APP_GATEWAY")); | ||
| address forwarderArb = gateway.forwarderAddresses(gateway.superToken(), 421614); | ||
| address forwarderOpt = gateway.forwarderAddresses(gateway.superToken(), 11155420); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Validate configuration and align chainId typing with the ID refactor.
- This should assert that resolved forwarder addresses are non-zero to fail fast on misconfiguration.
- Per PR-wide refactor, chain identifiers often moved to uint64. Ensure forwarderAddresses’ signature matches and cast accordingly.
Apply this diff to add safety checks (keep the casting step after verifying the signature per the script below):
- SuperTokenAppGateway gateway = SuperTokenAppGateway(vm.envAddress("APP_GATEWAY"));
- address forwarderArb = gateway.forwarderAddresses(gateway.superToken(), 421614);
- address forwarderOpt = gateway.forwarderAddresses(gateway.superToken(), 11155420);
+ ISuperTokenAppGateway gateway = ISuperTokenAppGateway(vm.envAddress("APP_GATEWAY"));
+ // Cast to uint64 if the gateway expects uint64 per the protocol refactor (verify with the script below).
+ address forwarderArb = gateway.forwarderAddresses(gateway.superToken(), 421614);
+ address forwarderOpt = gateway.forwarderAddresses(gateway.superToken(), 11155420);
+ require(forwarderArb != address(0), "forwarderArb not set");
+ require(forwarderOpt != address(0), "forwarderOpt not set");Run this script to confirm the signature of forwarderAddresses and whether it takes uint64:
🏁 Script executed:
#!/bin/bash
# Verify the signature of forwarderAddresses across the repo.
# Expectation: second param is uint64 after the refactor.
set -euo pipefail
echo "Searching for forwarderAddresses signature..."
rg -n -A 3 -B 3 $'forwarderAddresses\\s*\\('
echo
echo "Grepping interface/contract definitions for param types..."
rg -n $'function\\s+forwarderAddresses\\s*\\(' -A 1 -B 1Length of output: 3773
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check for ISuperTokenAppGateway interface
rg -n "interface\s+ISuperTokenAppGateway" -A1 -B1
# Inspect SuperTokenAppGateway contract definition
rg -n "contract\s+SuperTokenAppGateway" -A3 -B3Length of output: 71
Validate forwarder addresses and correct chainSlug typing
- forwarderAddresses uses the IAppGateway signature forwarderAddresses(bytes32,uint32). No uint64 cast is required—the literals 421614 and 11155420 fit in uint32.
- This should assert that both resolved forwarder addresses are non-zero to fail fast on misconfiguration.
Locations to update (script/supertoken/TransferSuperToken.s.sol lines 16–19):
- SuperTokenAppGateway gateway = SuperTokenAppGateway(vm.envAddress("APP_GATEWAY"));
- address forwarderArb = gateway.forwarderAddresses(gateway.superToken(), 421614);
- address forwarderOpt = gateway.forwarderAddresses(gateway.superToken(), 11155420);
+ // Optionally use the IAppGateway interface for looser coupling:
+ // IAppGateway gateway = IAppGateway(vm.envAddress("APP_GATEWAY"));
+ SuperTokenAppGateway gateway = SuperTokenAppGateway(vm.envAddress("APP_GATEWAY"));
+ address forwarderArb = gateway.forwarderAddresses(gateway.superToken(), 421614);
+ address forwarderOpt = gateway.forwarderAddresses(gateway.superToken(), 11155420);
+ require(forwarderArb != address(0), "forwarderArb not set");
+ require(forwarderOpt != address(0), "forwarderOpt not set");📝 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.
| SuperTokenAppGateway gateway = SuperTokenAppGateway(vm.envAddress("APP_GATEWAY")); | |
| address forwarderArb = gateway.forwarderAddresses(gateway.superToken(), 421614); | |
| address forwarderOpt = gateway.forwarderAddresses(gateway.superToken(), 11155420); | |
| // Optionally use the IAppGateway interface for looser coupling: | |
| // IAppGateway gateway = IAppGateway(vm.envAddress("APP_GATEWAY")); | |
| SuperTokenAppGateway gateway = SuperTokenAppGateway(vm.envAddress("APP_GATEWAY")); | |
| address forwarderArb = gateway.forwarderAddresses(gateway.superToken(), 421614); | |
| address forwarderOpt = gateway.forwarderAddresses(gateway.superToken(), 11155420); | |
| require(forwarderArb != address(0), "forwarderArb not set"); | |
| require(forwarderOpt != address(0), "forwarderOpt not set"); |
🤖 Prompt for AI Agents
In script/supertoken/TransferSuperToken.s.sol around lines 16–19, the call to
gateway.forwarderAddresses should match the IAppGateway signature
forwarderAddresses(bytes32,uint32): ensure the numeric chain IDs are passed as
uint32 (e.g. uint32(421614) and uint32(11155420)) rather than a uint64 cast, and
after resolving each forwarder address add an assertion/require that it is
non-zero (require(forwarder != address(0), "forwarder not configured for chain
X")) to fail fast on misconfiguration.
| bytes memory encodedOrder = abi.encode(transferOrder); | ||
| bytes memory encodedPayload = abi.encodeWithSelector( | ||
| bytes4(keccak256("transfer(bytes)")), | ||
| encodedOrder | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use a typed selector instead of a string literal hash for safety.
A typed selector guards against typos and signature drift. This should reference an interface’s selector.
Apply this diff:
- bytes memory encodedPayload = abi.encodeWithSelector(
- bytes4(keccak256("transfer(bytes)")),
- encodedOrder
- );
+ bytes memory encodedPayload = abi.encodeWithSelector(
+ ITransfer.transfer.selector,
+ encodedOrder
+ );Add this minimal interface in this file (outside the selected range), above the contract:
interface ITransfer {
function transfer(bytes calldata data) external;
}🤖 Prompt for AI Agents
In script/supertoken/TransferSuperToken.s.sol around lines 29 to 33, the code
uses bytes4(keccak256("transfer(bytes)")) as the selector; add the minimal
interface `ITransfer` (with function transfer(bytes calldata data) external)
above the contract (outside the selected range) and replace the keccak selector
expression with the typed selector ITransfer.transfer.selector so the selector
is type-checked and guarded against typos/signature drift.
| Events.RequestCompletedWithErrors, | ||
| ]; | ||
|
|
||
| export const configurationsEvents = [Events.PlugAdded]; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
configurationsEvents array needs more events
The configurationsEvents array only includes PlugAdded but configurations typically involve more operations like removal or updates.
🏁 Script executed:
#!/bin/bash
# Check if there are other configuration-related events that should be included
ast-grep --pattern 'event Plug$_($$$)' Length of output: 40
🏁 Script executed:
#!/bin/bash
# Search for configuration-related events in the Configurations contract
rg "event.*Plug|event.*Config" contracts/evmx/Configurations.sol contracts/evmx/interfaces/IConfigurations.sol -A 1Length of output: 191
🏁 Script executed:
#!/bin/bash
set -e
# List all Solidity files to locate the Configurations contract
echo "=== Solidity files in repo ==="
fd -e sol
# Search for interface IConfigurations definitions
echo "=== interface IConfigurations declarations ==="
rg "interface IConfigurations" -n -t sol || echo "No interface IConfigurations found"
# Search for contract Configurations definitions
echo "=== contract Configurations declarations ==="
rg "contract Configurations" -n -t sol || echo "No contract Configurations found"
# Search for events related to configuration (Config) and plugs (Plug)
echo "=== Events containing 'Config' ==="
rg "event\s+\w*Config\w*" -n -t sol || echo "No Config-related events found"
echo "=== Events containing 'Plug' ==="
rg "event\s+\w*Plug\w*" -n -t sol || echo "No Plug-related events found"Length of output: 5368
🏁 Script executed:
#!/bin/bash
set -e
# Inspect event declarations in the watcher Configurations contract
rg -n 'event ' contracts/evmx/watcher/Configurations.solLength of output: 383
Add missing configuration watcher events
This should include all events emitted by the watcher’s Configurations contract.
File: src/events.ts, line 28.
-export const configurationsEvents = [Events.PlugAdded];
+export const configurationsEvents = [
+ Events.PlugAdded,
+ Events.SwitchboardSet,
+ Events.SocketSet,
+ Events.IsValidPlugSet,
+];This ensures handlers react to SwitchboardSet, SocketSet, and IsValidPlugSet as well as PlugAdded.
📝 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.
| export const configurationsEvents = [Events.PlugAdded]; | |
| export const configurationsEvents = [ | |
| Events.PlugAdded, | |
| Events.SwitchboardSet, | |
| Events.SocketSet, | |
| Events.IsValidPlugSet, | |
| ]; |
🤖 Prompt for AI Agents
In src/events.ts around line 28, the configurationsEvents array only contains
Events.PlugAdded but must include all events emitted by the Configurations
contract; update the array to include Events.SwitchboardSet, Events.SocketSet,
and Events.IsValidPlugSet alongside Events.PlugAdded so handlers will react to
SwitchboardSet, SocketSet, IsValidPlugSet, and PlugAdded.
| export const defaultFinalityBlocks: ChainFinalityBlocks = { | ||
| [FinalityBucket.LOW]: 1, | ||
| [FinalityBucket.LOW]: 0, | ||
| [FinalityBucket.MEDIUM]: "safe", | ||
| [FinalityBucket.HIGH]: "finalized", | ||
| }; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
LOW finality = 0 can cause reorg exposure and falsy-defaulting bugs
- Security: 0 means "head-of-chain" (no confirmations). For chains without overrides, this weakens safety and increases reorg risk.
- Correctness: Any consumer using
||to apply defaults will treat0as falsy and silently override it, leading to subtle bugs.
This should either:
- Be explicitly documented as intentional “head” semantics and enforced across consumers (use
??instead of||, and explicitundefinedchecks), or - Remain
1for a safer default if production paths rely on it.
If keeping 0, add an in-file note to make the contract explicit:
export const defaultFinalityBlocks: ChainFinalityBlocks = {
+ // NOTE: LOW=0 intentionally means "head" (no confirmation wait).
+ // Consumers MUST NOT use `||` to default this value, use nullish coalescing `??` instead.
[FinalityBucket.LOW]: 0,
[FinalityBucket.MEDIUM]: "safe",
[FinalityBucket.HIGH]: "finalized",
};Run this script to find risky || usages near finality logic and all call sites of getFinalityBlocks. Inspect and replace || with ?? where 0 is a valid value.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Potential falsy-default patterns near finality-related identifiers:"
rg -n -C 2 --glob '!dist/*' --glob '!build/*' $'(finality|FinalityBucket|getFinalityBlocks)[^\n]{0,120}\\|\\|' || true
echo
echo "All uses of getFinalityBlocks:"
rg -n -A 3 -B 1 --glob '!dist/*' --glob '!build/*' 'getFinalityBlocks\\s*\\(' || true
echo
echo "Broad AST scan for '||' expressions; review matches that involve finality logic:"
ast-grep --pattern $'$_ || $_' --lang ts | rg -n -C 2 '(finality|FinalityBucket|blocks|confirm|finalized|safe)' || trueLength of output: 850
Document LOW=0 as Intentional Head Semantics & Enforce Nullish Coalescing
No internal ||–based defaults around finality were found, but keeping 0 without documentation risks future consumers silently overriding it. This should be addressed in src/finality.ts:
• Add an explicit note beside [FinalityBucket.LOW]: 0
• Require all consumers to use ?? instead of || when defaulting this value
• Alternatively, bump the LOW default to 1 if you want a safer confirmation baseline
Suggested diff:
export const defaultFinalityBlocks: ChainFinalityBlocks = {
+ // NOTE: LOW=0 intentionally means "head" (no confirmation wait).
+ // Consumers MUST use nullish coalescing (`??`) instead of `||` when defaulting this value.
[FinalityBucket.LOW]: 0,
[FinalityBucket.MEDIUM]: "safe",
[FinalityBucket.HIGH]: "finalized",
};🤖 Prompt for AI Agents
In src/finality.ts around lines 6 to 10, the LOW finality bucket is set to 0
without clarification which can be misused by consumers who default with ||; add
an inline comment next to [FinalityBucket.LOW]: 0 that documents this is an
intentional “head semantics” zero value, update any nearby documentation/export
comment to instruct callers to use nullish coalescing (??) when applying
defaults, and either (A) keep 0 and add that guidance or (B) change the default
to 1 if you prefer a non-zero confirmation baseline — choose one approach and
make the code comment and public docs consistent with it.
| export const getNonce = () => { | ||
| const timestamp = Date.now(); | ||
| const random = Math.floor(Math.random() * 1000); | ||
| const nonce = Number(String(timestamp) + String(random)); | ||
| return nonce; | ||
| }; |
There was a problem hiding this comment.
Replace Math.random + string concat with crypto-grade randomness and arithmetic
This should use a CSPRNG and avoid string concatenation (leading zeros are lost and it’s slower). Keep the value within JS safe integer range and preserve the “timestamp + 3-digit suffix” shape.
Apply this diff:
-export const getNonce = () => {
- const timestamp = Date.now();
- const random = Math.floor(Math.random() * 1000);
- const nonce = Number(String(timestamp) + String(random));
- return nonce;
-};
+export const getNonce = (): number => {
+ const timestampMs = Date.now(); // 13 digits for now
+ // Derive a cryptographically secure 0..999 value.
+ const randBytes = ethers.utils.randomBytes(2); // Uint8Array length 2
+ const rand = (((randBytes[0] << 8) | randBytes[1]) % 1000); // 0..999
+ // Equivalent to zero-padded 3-digit suffix without string ops.
+ return timestampMs * 1000 + rand; // stays < 2^53 until ~year 2255
+};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/signer.ts around lines 27 to 32, replace the non-cryptographic
Math.random + string concatenation with a CSPRNG and arithmetic: generate a
0–999 integer suffix using a crypto API (e.g., Node's crypto.randomInt(0,1000)
or web crypto getRandomValues), compute nonce = timestamp * 1000 + suffix
(preserving leading zeros and the "timestamp + 3-digit suffix" shape), ensure
this stays within Number.MAX_SAFE_INTEGER (timestamp*1000 is safe for
Date.now()), and return that numeric nonce; also add the required import/use of
the crypto API for your environment.
| ) internal async { | ||
| _setMaxFees(maxFees_); | ||
| _setOverrides(consumeFrom_); | ||
| _setSchedule(delayInSeconds_); | ||
| then(callbackSelector_, callbackData_); | ||
| watcher__().submitRequest(maxFees_, address(this), consumeFrom_, bytes("")); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Async modifier requires verification
The function signature changed to internal async. Verify that the async modifier is properly defined and that the promise-based flow correctly handles asynchronous execution.
🏁 Script executed:
#!/bin/bash
# Search for async modifier definition and usage patterns
rg "modifier async" -A 5
# Check for other internal async functions to understand the pattern
ast-grep --pattern 'function $_($$$) internal async'Length of output: 399
Inspect async helper implementations for correct hook behavior and promise chaining.
🏁 Script executed:
#!/bin/bash
# Verify definitions of async lifecycle hooks and then(...)
rg "function _preAsync" -A5
rg "function _postAsync" -A5
rg "function then" -A5Length of output: 2486
Relocate then() after the promise is created
Calling then() inside your async‐wrapped function attaches the callback to the previous promise (since _postAsync()—which creates the new promise—is invoked after your body). This should register the callback on the newly created promise.
• AuctionManager.sol (lines 238–243): remove the in-body then(callbackSelector_, callbackData_).
• Either call then(...) immediately after the async function returns, or extend the async modifier to accept the selector/data and invoke IPromise(...).then(...) inside _postAsync().
Example modifier change:
modifier async(bytes4 selector_, bytes memory data_) {
_preAsync();
_;
- _postAsync();
- // body must call then(), but it's too early
+ _postAsync();
+ IPromise(watcher__().latestAsyncPromise()).then(selector_, data_);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In contracts/evmx/AuctionManager.sol around lines 238–243, the in-body
then(callbackSelector_, callbackData_) is attaching the callback to the previous
promise because the new promise is created after the async function body; remove
the then(...) call from inside this async-wrapped function and instead attach
the callback to the promise returned by the call (i.e., call .then(selector,
data) immediately after invoking the async function), or modify the
async/_postAsync machinery to accept the selector/data and call
IPromise(...).then(...) inside _postAsync so the callback is registered on the
newly created promise.
| onCompleteData = abi.encodeWithSelector( | ||
| this.onDeployComplete.selector, | ||
| watcher__().getCurrentRequestCount(), | ||
| abi.encode(chainSlug_) | ||
| ); |
There was a problem hiding this comment.
Critical: Potential reentrancy vulnerability in deployment callback flow
The new onDeployComplete callback mechanism encodes watcher__().getCurrentRequestCount() at deployment time but doesn't validate it during callback execution. This creates a window for request manipulation between deployment initiation and completion.
The deployment flow should validate that the request count hasn't been tampered with:
-function onDeployComplete(uint40, bytes calldata onCompleteData_) external virtual onlyWatcher {
+function onDeployComplete(uint40 requestCount_, bytes calldata onCompleteData_) external virtual onlyWatcher {
+ // Validate the request count matches expected state
+ require(requestCount_ <= watcher__().getCurrentRequestCount(), "Invalid request count");
if (onCompleteData_.length == 0) return;
uint32 chainSlug = abi.decode(onCompleteData_, (uint32));
initializeOnChain(chainSlug);
}📝 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.
| onCompleteData = abi.encodeWithSelector( | |
| this.onDeployComplete.selector, | |
| watcher__().getCurrentRequestCount(), | |
| abi.encode(chainSlug_) | |
| ); | |
| function onDeployComplete(uint40 requestCount_, bytes calldata onCompleteData_) external virtual onlyWatcher { | |
| // Validate the request count matches expected state | |
| require(requestCount_ <= watcher__().getCurrentRequestCount(), "Invalid request count"); | |
| if (onCompleteData_.length == 0) return; | |
| uint32 chainSlug = abi.decode(onCompleteData_, (uint32)); | |
| initializeOnChain(chainSlug); | |
| } |
| forwarderAddresses[contractId][chainSlug] = asyncDeployer__().getOrDeployForwarderContract( | ||
| abi.decode(returnData_, (address)), | ||
| toBytes32Format(abi.decode(returnData_, (address))), | ||
| chainSlug | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Unchecked address-to-bytes32 conversion could silently fail
The toBytes32Format conversion in setAddress doesn't validate that the decoded address is valid before conversion.
Add validation before conversion:
function setAddress(bytes memory data_, bytes memory returnData_) external onlyPromises {
(uint32 chainSlug, bytes32 contractId) = abi.decode(data_, (uint32, bytes32));
+ address decodedAddr = abi.decode(returnData_, (address));
+ require(decodedAddr != address(0), "Invalid deployed address");
forwarderAddresses[contractId][chainSlug] = asyncDeployer__().getOrDeployForwarderContract(
- toBytes32Format(abi.decode(returnData_, (address))),
+ toBytes32Format(decodedAddr),
chainSlug
);
}📝 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.
| forwarderAddresses[contractId][chainSlug] = asyncDeployer__().getOrDeployForwarderContract( | |
| abi.decode(returnData_, (address)), | |
| toBytes32Format(abi.decode(returnData_, (address))), | |
| chainSlug | |
| ); | |
| } | |
| address decodedAddr = abi.decode(returnData_, (address)); | |
| require(decodedAddr != address(0), "Invalid deployed address"); | |
| forwarderAddresses[contractId][chainSlug] = asyncDeployer__().getOrDeployForwarderContract( | |
| toBytes32Format(decodedAddr), | |
| chainSlug | |
| ); | |
| } |
🤖 Prompt for AI Agents
In contracts/evmx/base/AppGatewayBase.sol around lines 145-149, the code decodes
an address from returnData_ and directly passes it into toBytes32Format which
can silently fail for invalid or zero addresses; decode returnData_ into a local
address variable first, validate returnData_ length if applicable and require
the decoded address is non-zero (address != address(0)) before calling
toBytes32Format, and handle the failure path (revert with a clear error or skip
assignment) so only a validated address is converted and used to set
forwarderAddresses.
| bytes32 onchainAddress = getOnChainAddress(contractId_, chainSlug_); | ||
| watcher__().setIsValidPlug(isValid, chainSlug_, onchainAddress); | ||
| } |
There was a problem hiding this comment.
Missing validation for zero bytes32 address
The _setValidPlug function doesn't validate if onchainAddress is zero before passing it to the watcher, which could register invalid plugs.
Add validation:
function _setValidPlug(bool isValid, uint32 chainSlug_, bytes32 contractId_) internal {
bytes32 onchainAddress = getOnChainAddress(contractId_, chainSlug_);
+ require(onchainAddress != bytes32(0), "Invalid onchain address");
watcher__().setIsValidPlug(isValid, chainSlug_, onchainAddress);
}📝 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.
| bytes32 onchainAddress = getOnChainAddress(contractId_, chainSlug_); | |
| watcher__().setIsValidPlug(isValid, chainSlug_, onchainAddress); | |
| } | |
| function _setValidPlug(bool isValid, uint32 chainSlug_, bytes32 contractId_) internal { | |
| bytes32 onchainAddress = getOnChainAddress(contractId_, chainSlug_); | |
| require(onchainAddress != bytes32(0), "Invalid onchain address"); | |
| watcher__().setIsValidPlug(isValid, chainSlug_, onchainAddress); | |
| } |
🤖 Prompt for AI Agents
In contracts/evmx/base/AppGatewayBase.sol around lines 225 to 227, the function
calls watcher__().setIsValidPlug without ensuring onchainAddress is non-zero;
add a check that onchainAddress != bytes32(0) and revert with a clear error
message if it is zero (e.g., require(onchainAddress != bytes32(0), "Invalid
onchain address")), then proceed to call watcher__().setIsValidPlug(isValid,
chainSlug_, onchainAddress).
| bytes32 prevBatchDigestHash = getPrevBatchDigestHash( | ||
| payloadParams.requestCount, | ||
| payloadParams.batchCount | ||
| uint40(payloadParams.payloadPointer >> 120), | ||
| uint40(payloadParams.payloadPointer >> 80) | ||
| ); |
There was a problem hiding this comment.
Bit-shift operations lack validation
The bit-shift operations to extract requestCount and batchCount from payloadPointer don't validate if the values are reasonable. Invalid values could cause issues in getPrevBatchDigestHash.
Add validation:
bytes32 prevBatchDigestHash = getPrevBatchDigestHash(
uint40(payloadParams.payloadPointer >> 120),
uint40(payloadParams.payloadPointer >> 80)
);
+// Validate extracted values are non-zero and reasonable
+uint40 requestCount = uint40(payloadParams.payloadPointer >> 120);
+uint40 batchCount = uint40(payloadParams.payloadPointer >> 80);
+if (requestCount == 0) revert("Invalid request count");
+bytes32 prevBatchDigestHash = getPrevBatchDigestHash(requestCount, batchCount);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In contracts/evmx/watcher/precompiles/WritePrecompile.sol around lines 172-175,
the code directly shifts payloadParams.payloadPointer to derive two uint40
values and passes them to getPrevBatchDigestHash without validating them; change
this to first mask the shifted values to 40 bits into local uint40 variables,
then validate those variables against expected ranges (e.g., non-zero if that's
required, and within any protocol-specific max counts or the current payload
length/known limits), and revert with a clear error message if they are out of
bounds before calling getPrevBatchDigestHash.
| bytes memory data = abi.encodeWithSignature( | ||
| "transferCredits(address,uint256)", | ||
| newAppGateway, | ||
| availableFees | ||
| ); | ||
| (bool success, ) = appGateway.call(data); | ||
| require(success, "Transfer failed"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing validation before credit transfer
The script doesn't validate that availableFees > 0 before attempting the transfer. This could result in unnecessary transaction costs if there are no credits to transfer.
Add validation:
uint256 availableFees = feesManager.getAvailableCredits(appGateway);
console.log("Available fees:", availableFees);
+ require(availableFees > 0, "No available credits to transfer");
bytes memory data = abi.encodeWithSignature(
"transferCredits(address,uint256)",
newAppGateway,
availableFees
);📝 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.
| bytes memory data = abi.encodeWithSignature( | |
| "transferCredits(address,uint256)", | |
| newAppGateway, | |
| availableFees | |
| ); | |
| (bool success, ) = appGateway.call(data); | |
| require(success, "Transfer failed"); | |
| uint256 availableFees = feesManager.getAvailableCredits(appGateway); | |
| console.log("Available fees:", availableFees); | |
| require(availableFees > 0, "No available credits to transfer"); | |
| bytes memory data = abi.encodeWithSignature( | |
| "transferCredits(address,uint256)", | |
| newAppGateway, | |
| availableFees | |
| ); | |
| (bool success, ) = appGateway.call(data); | |
| require(success, "Transfer failed"); |
🤖 Prompt for AI Agents
In script/helpers/TransferRemainingCredits.s.sol around lines 29 to 35, the code
attempts to transfer credits without checking if availableFees is greater than
zero; add a validation that availableFees > 0 before encoding and calling
transferCredits and either revert with a clear message or return early to avoid
paying gas for a no-op transfer; if reverting, use require(availableFees > 0,
"No credits to transfer"); otherwise skip the call when availableFees == 0.
| uint256 availableFees = feesManager.getAvailableCredits(appGateway); | ||
| console.log("Available fees:", availableFees); | ||
| feesManager.transferCredits(appGateway, vm.addr(deployerPrivateKey), availableFees); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Credit transfer logic is correct but lacks safety check.
The script transfers all available credits but should verify that availableFees > 0 before attempting the transfer to avoid unnecessary transactions.
uint256 availableFees = feesManager.getAvailableCredits(appGateway);
console.log("Available fees:", availableFees);
- feesManager.transferCredits(appGateway, vm.addr(deployerPrivateKey), availableFees);
+ if (availableFees > 0) {
+ feesManager.transferCredits(appGateway, vm.addr(deployerPrivateKey), availableFees);
+ } else {
+ console.log("No available credits to withdraw");
+ }📝 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.
| uint256 availableFees = feesManager.getAvailableCredits(appGateway); | |
| console.log("Available fees:", availableFees); | |
| feesManager.transferCredits(appGateway, vm.addr(deployerPrivateKey), availableFees); | |
| uint256 availableFees = feesManager.getAvailableCredits(appGateway); | |
| console.log("Available fees:", availableFees); | |
| if (availableFees > 0) { | |
| feesManager.transferCredits(appGateway, vm.addr(deployerPrivateKey), availableFees); | |
| } else { | |
| console.log("No available credits to withdraw"); | |
| } |
🤖 Prompt for AI Agents
In script/helpers/WithdrawRemainingCredits.s.sol around lines 24 to 26, the code
transfers all available credits without checking if availableFees is greater
than zero; add a guard that checks if availableFees > 0 before calling
feesManager.transferCredits (log a message and skip/return when zero) so you
avoid unnecessary transactions when there are no credits to transfer.
| // Setting fee payment on Arbitrum Sepolia | ||
| uint256 fees = 1 ether; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hardcoded fee value should be configurable.
The fixed 1 ether fee should be sourced from environment variables or configuration to support different deployment environments.
- // Setting fee payment on Arbitrum Sepolia
- uint256 fees = 1 ether;
+ uint256 fees = vm.envUint("GATEWAY_FEES");📝 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.
| // Setting fee payment on Arbitrum Sepolia | |
| uint256 fees = 1 ether; | |
| uint256 fees = vm.envUint("GATEWAY_FEES"); |
🤖 Prompt for AI Agents
In script/supertoken/DeployEVMxSuperTokenApp.s.sol around lines 18-19, the fee
is hardcoded as `1 ether`; change this to read a configurable value (e.g., from
an environment variable or config) with a sensible default. Update the script to
parse the env/config value into a uint256 (apply appropriate unit conversion so
input can be in ether or wei), validate it (non-zero and within expected
bounds), and use that variable instead of the hardcoded literal; also document
the env variable name and default in the script header or README.
| hoax(user); | ||
| feesManager.withdrawCredits( | ||
| feesChainSlug, | ||
| address(feesConfig.testUSDC), | ||
| withdrawAmount, | ||
| feesAmount, | ||
| address(receiver) | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing validation of withdrawal parameters
The test doesn't verify that the withdrawal was successful or that the correct amounts were transferred. This makes the test incomplete as a validation tool.
Add balance checks:
+ uint256 userBalanceBefore = user.balance;
+ uint256 receiverBalanceBefore = receiver.balance;
+
hoax(user);
feesManager.withdrawCredits(
feesChainSlug,
address(feesConfig.testUSDC),
withdrawAmount,
feesAmount,
address(receiver)
);
+
+ // Verify the withdrawal was processed
+ assertEq(user.balance, userBalanceBefore - feesAmount, "Incorrect fee deduction");📝 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.
| hoax(user); | |
| feesManager.withdrawCredits( | |
| feesChainSlug, | |
| address(feesConfig.testUSDC), | |
| withdrawAmount, | |
| feesAmount, | |
| address(receiver) | |
| ); | |
| uint256 userBalanceBefore = user.balance; | |
| uint256 receiverBalanceBefore = receiver.balance; | |
| hoax(user); | |
| feesManager.withdrawCredits( | |
| feesChainSlug, | |
| address(feesConfig.testUSDC), | |
| withdrawAmount, | |
| feesAmount, | |
| address(receiver) | |
| ); | |
| // Verify the withdrawal was processed | |
| assertEq(user.balance, userBalanceBefore - feesAmount, "Incorrect fee deduction"); |
🤖 Prompt for AI Agents
In test/AuctionManager.t.sol around lines 39 to 46, the test calls
feesManager.withdrawCredits but does not assert outcomes; capture relevant
pre-transaction balances (contract/feesManager token balance, receiver token
balance and any on-chain credit/accounting value tracked by feesManager) then
call withdrawCredits and capture post-transaction balances, and add assertions
that (1) the receiver's token balance increased by the expected withdrawAmount
(or withdrawAmount minus fees if applicable), (2) the feesManager/contract token
balance decreased by that same amount, and (3) any internal credits or fees
accounting value (feesAmount or credits ledger) was updated/decreased by the
expected fees/credits; use the test framework's assert functions
(assertEq/assertTrue) to compare before/after values with clear messages.
| executeRequest(); | ||
|
|
||
| // expire bid settle | ||
| executeRequest(); | ||
| } |
There was a problem hiding this comment.
Test lacks assertions for critical state changes
The test executes requests but doesn't verify the expected outcomes. After expiring a bid, the test should validate that the bid was actually expired and the expected state changes occurred.
Add assertions to verify the test objectives:
executeRequest();
// expire bid settle
executeRequest();
+
+ // Add assertions to verify:
+ // 1. The bid was actually expired
+ // 2. Any refunds were processed correctly
+ // 3. The auction state was updated appropriately
+ // Example:
+ // assertEq(auctionManager.getBidStatus(bidId), BidStatus.Expired);
+ // assertEq(receiver.balance, expectedRefundAmount);📝 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.
| executeRequest(); | |
| // expire bid settle | |
| executeRequest(); | |
| } | |
| executeRequest(); | |
| // expire bid settle | |
| executeRequest(); | |
| // Add assertions to verify: | |
| // 1. The bid was actually expired | |
| // 2. Any refunds were processed correctly | |
| // 3. The auction state was updated appropriately | |
| // Example: | |
| // assertEq(auctionManager.getBidStatus(bidId), BidStatus.Expired); | |
| // assertEq(receiver.balance, expectedRefundAmount); | |
| } |
| // slot 51 | ||
| /// @notice switchboard type | ||
| bytes32 public sbType; | ||
| bytes32 public deprecatedSbType; |
There was a problem hiding this comment.
Deprecated field naming convention
The field is renamed to deprecatedSbType but is still actively used in line 293. If this field is truly deprecated, it shouldn't be used in active logic.
Either:
- Remove the "deprecated" prefix if the field is still in use
- Complete the migration to remove usage of this deprecated field
| function handleRevert(bytes32 payloadId_) external override { | ||
| if (watcher__().getPayloadParams(payloadId_).asyncPromise != msg.sender) return; | ||
| emit WithdrawFailed(payloadId_); | ||
| } |
There was a problem hiding this comment.
Missing access control in handleRevert
The handleRevert function only checks if the caller is the async promise but doesn't revert for unauthorized callers. This silently ignores invalid calls.
function handleRevert(bytes32 payloadId_) external override {
- if (watcher__().getPayloadParams(payloadId_).asyncPromise != msg.sender) return;
+ require(
+ watcher__().getPayloadParams(payloadId_).asyncPromise == msg.sender,
+ "Unauthorized caller"
+ );
emit WithdrawFailed(payloadId_);
}📝 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.
| function handleRevert(bytes32 payloadId_) external override { | |
| if (watcher__().getPayloadParams(payloadId_).asyncPromise != msg.sender) return; | |
| emit WithdrawFailed(payloadId_); | |
| } | |
| function handleRevert(bytes32 payloadId_) external override { | |
| require( | |
| watcher__().getPayloadParams(payloadId_).asyncPromise == msg.sender, | |
| "Unauthorized caller" | |
| ); | |
| emit WithdrawFailed(payloadId_); | |
| } |
🤖 Prompt for AI Agents
In contracts/evmx/fees/Credit.sol around lines 317 to 320, handleRevert
currently just returns when msg.sender is not the expected asyncPromise which
silently ignores unauthorized calls; change it to explicitly revert on
unauthorized callers (e.g.,
require(watcher__().getPayloadParams(payloadId_).asyncPromise == msg.sender,
"handleRevert: unauthorized caller")) so only the authorized asyncPromise can
call the function, and emit WithdrawFailed(payloadId_) only after the
authorization check passes.
| function connect(bytes32 appGatewayId_, uint64 switchboardId_) external override { | ||
| if (isValidSwitchboard[switchboardId_] != SwitchboardStatus.REGISTERED) | ||
| revert InvalidSwitchboard(); | ||
|
|
||
| PlugConfig storage _plugConfig = _plugConfigs[msg.sender]; | ||
|
|
||
| PlugConfigEvm storage _plugConfig = _plugConfigs[msg.sender]; | ||
| _plugConfig.appGatewayId = appGatewayId_; | ||
| _plugConfig.switchboard = switchboard_; | ||
| _plugConfig.switchboardId = switchboardId_; | ||
|
|
||
| emit PlugConnected(msg.sender, appGatewayId_, switchboardId_); | ||
| } |
There was a problem hiding this comment.
Incorrect function signature in connect
The function signature doesn't match the interface declaration. The interface ISocket expects connect(bytes32 appGatewayId_, address socket_, uint64 switchboardId_) but this implementation omits the socket_ parameter.
- function connect(bytes32 appGatewayId_, uint64 switchboardId_) external override {
+ function connect(bytes32 appGatewayId_, address socket_, uint64 switchboardId_) external override {
+ // socket_ parameter can be validated or ignored based on requirements
if (isValidSwitchboard[switchboardId_] != SwitchboardStatus.REGISTERED)
revert InvalidSwitchboard();
PlugConfigEvm storage _plugConfig = _plugConfigs[msg.sender];
_plugConfig.appGatewayId = appGatewayId_;
_plugConfig.switchboardId = switchboardId_;
emit PlugConnected(msg.sender, appGatewayId_, switchboardId_);
}📝 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.
| function connect(bytes32 appGatewayId_, uint64 switchboardId_) external override { | |
| if (isValidSwitchboard[switchboardId_] != SwitchboardStatus.REGISTERED) | |
| revert InvalidSwitchboard(); | |
| PlugConfig storage _plugConfig = _plugConfigs[msg.sender]; | |
| PlugConfigEvm storage _plugConfig = _plugConfigs[msg.sender]; | |
| _plugConfig.appGatewayId = appGatewayId_; | |
| _plugConfig.switchboard = switchboard_; | |
| _plugConfig.switchboardId = switchboardId_; | |
| emit PlugConnected(msg.sender, appGatewayId_, switchboardId_); | |
| } | |
| function connect(bytes32 appGatewayId_, address socket_, uint64 switchboardId_) external override { | |
| // socket_ parameter can be validated or ignored based on requirements | |
| if (isValidSwitchboard[switchboardId_] != SwitchboardStatus.REGISTERED) | |
| revert InvalidSwitchboard(); | |
| PlugConfigEvm storage _plugConfig = _plugConfigs[msg.sender]; | |
| _plugConfig.appGatewayId = appGatewayId_; | |
| _plugConfig.switchboardId = switchboardId_; | |
| emit PlugConnected(msg.sender, appGatewayId_, switchboardId_); | |
| } |
🤖 Prompt for AI Agents
In contracts/protocol/SocketConfig.sol around lines 97 to 106, the connect
function signature is missing the address socket_ parameter required by ISocket;
update the function signature to connect(bytes32 appGatewayId_, address socket_,
uint64 switchboardId_) and then use socket_ in place of msg.sender when indexing
_plugConfigs and when emitting PlugConnected(msg.sender, ...) so the stored
PlugConfig and event use the supplied socket address; keep the same switchboard
validation and the override specifier.
| function syncOut(bytes32 payloadId_, uint32[] calldata remoteChainSlugs_) external { | ||
| bytes32 digest = socket__.payloadIdToDigest(payloadId_); | ||
| // not attested | ||
| if (digest == bytes32(0) || !isAttested[digest]) revert NotAttested(); | ||
|
|
||
| // already synced out | ||
| if (isSyncedOut[digest]) return; | ||
| isSyncedOut[digest] = true; | ||
|
|
||
| // not executed | ||
| ExecutionStatus isExecuted = socket__.payloadExecuted(payloadId_); | ||
| if (isExecuted != ExecutionStatus.Executed) revert NotExecuted(); | ||
|
|
||
| bytes memory message = abi.encode(payloadId_, digest); | ||
| for (uint256 i = 0; i < remoteChainSlugs_.length; i++) { | ||
| RemoteEndpoint memory endpoint = chainSlugToRemoteEndpoint[remoteChainSlugs_[i]]; | ||
| messageTransmitter.sendMessage(endpoint.remoteDomain, endpoint.remoteAddress, message); | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing access control on syncOut function
The syncOut function lacks access control, allowing any caller to trigger cross-chain message transmission. This could lead to spam or DoS attacks.
- function syncOut(bytes32 payloadId_, uint32[] calldata remoteChainSlugs_) external {
+ function syncOut(bytes32 payloadId_, uint32[] calldata remoteChainSlugs_) external onlyOwner {
bytes32 digest = socket__.payloadIdToDigest(payloadId_);
// not attested
if (digest == bytes32(0) || !isAttested[digest]) revert NotAttested();📝 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.
| function syncOut(bytes32 payloadId_, uint32[] calldata remoteChainSlugs_) external { | |
| bytes32 digest = socket__.payloadIdToDigest(payloadId_); | |
| // not attested | |
| if (digest == bytes32(0) || !isAttested[digest]) revert NotAttested(); | |
| // already synced out | |
| if (isSyncedOut[digest]) return; | |
| isSyncedOut[digest] = true; | |
| // not executed | |
| ExecutionStatus isExecuted = socket__.payloadExecuted(payloadId_); | |
| if (isExecuted != ExecutionStatus.Executed) revert NotExecuted(); | |
| bytes memory message = abi.encode(payloadId_, digest); | |
| for (uint256 i = 0; i < remoteChainSlugs_.length; i++) { | |
| RemoteEndpoint memory endpoint = chainSlugToRemoteEndpoint[remoteChainSlugs_[i]]; | |
| messageTransmitter.sendMessage(endpoint.remoteDomain, endpoint.remoteAddress, message); | |
| } | |
| } | |
| function syncOut(bytes32 payloadId_, uint32[] calldata remoteChainSlugs_) external onlyOwner { | |
| bytes32 digest = socket__.payloadIdToDigest(payloadId_); | |
| // not attested | |
| if (digest == bytes32(0) || !isAttested[digest]) revert NotAttested(); | |
| // already synced out | |
| if (isSyncedOut[digest]) return; | |
| isSyncedOut[digest] = true; | |
| // not executed | |
| ExecutionStatus isExecuted = socket__.payloadExecuted(payloadId_); | |
| if (isExecuted != ExecutionStatus.Executed) revert NotExecuted(); | |
| bytes memory message = abi.encode(payloadId_, digest); | |
| for (uint256 i = 0; i < remoteChainSlugs_.length; i++) { | |
| RemoteEndpoint memory endpoint = chainSlugToRemoteEndpoint[remoteChainSlugs_[i]]; | |
| messageTransmitter.sendMessage(endpoint.remoteDomain, endpoint.remoteAddress, message); | |
| } | |
| } |
| function proveRemoteExecutions( | ||
| bytes32[] calldata previousPayloadIds_, | ||
| bytes32 payloadId_, | ||
| bytes calldata transmitterSignature_, | ||
| ExecuteParams calldata executeParams_ | ||
| ) public { | ||
| // Calculate prevBatchDigestHash from stored remoteExecutedDigests | ||
| bytes32 prevBatchDigestHash = bytes32(0); | ||
| for (uint256 i = 0; i < previousPayloadIds_.length; i++) { | ||
| if (remoteExecutedDigests[previousPayloadIds_[i]] == bytes32(0)) | ||
| revert RemoteExecutionNotFound(); | ||
| prevBatchDigestHash = keccak256( | ||
| abi.encodePacked(prevBatchDigestHash, remoteExecutedDigests[previousPayloadIds_[i]]) | ||
| ); | ||
| } | ||
| // Check if the calculated prevBatchDigestHash matches the one in executeParams_ | ||
| if (prevBatchDigestHash != executeParams_.prevBatchDigestHash) | ||
| revert PrevBatchDigestHashMismatch(); | ||
|
|
||
| address transmitter = _recoverSigner( | ||
| keccak256(abi.encode(address(socket__), payloadId_)), | ||
| transmitterSignature_ | ||
| ); | ||
|
|
||
| // Construct current digest | ||
| (bytes32 appGatewayId, ) = socket__.getPlugConfig(executeParams_.target); | ||
| bytes32 constructedDigest = _createDigest( | ||
| transmitter, | ||
| payloadId_, | ||
| appGatewayId, | ||
| executeParams_ | ||
| ); | ||
|
|
||
| // Verify the constructed digest matches the stored one | ||
| if (!isAttested[constructedDigest]) revert NotAttested(); | ||
| isRemoteExecuted[payloadId_] = true; | ||
| } |
There was a problem hiding this comment.
Race condition in proveRemoteExecutions
The function calculates prevBatchDigestHash from stored digests but doesn't verify these digests are final. If remote executions are still being processed, the hash calculation could be incorrect.
This should verify all previous payloads are marked as executed before calculating the hash:
function proveRemoteExecutions(
bytes32[] calldata previousPayloadIds_,
bytes32 payloadId_,
bytes calldata transmitterSignature_,
ExecuteParams calldata executeParams_
) public {
// Calculate prevBatchDigestHash from stored remoteExecutedDigests
bytes32 prevBatchDigestHash = bytes32(0);
for (uint256 i = 0; i < previousPayloadIds_.length; i++) {
if (remoteExecutedDigests[previousPayloadIds_[i]] == bytes32(0))
revert RemoteExecutionNotFound();
+ // Verify the previous payload is marked as executed
+ if (!isRemoteExecuted[previousPayloadIds_[i]])
+ revert("Previous payload not executed");
prevBatchDigestHash = keccak256(
abi.encodePacked(prevBatchDigestHash, remoteExecutedDigests[previousPayloadIds_[i]])
);
}📝 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.
| function proveRemoteExecutions( | |
| bytes32[] calldata previousPayloadIds_, | |
| bytes32 payloadId_, | |
| bytes calldata transmitterSignature_, | |
| ExecuteParams calldata executeParams_ | |
| ) public { | |
| // Calculate prevBatchDigestHash from stored remoteExecutedDigests | |
| bytes32 prevBatchDigestHash = bytes32(0); | |
| for (uint256 i = 0; i < previousPayloadIds_.length; i++) { | |
| if (remoteExecutedDigests[previousPayloadIds_[i]] == bytes32(0)) | |
| revert RemoteExecutionNotFound(); | |
| prevBatchDigestHash = keccak256( | |
| abi.encodePacked(prevBatchDigestHash, remoteExecutedDigests[previousPayloadIds_[i]]) | |
| ); | |
| } | |
| // Check if the calculated prevBatchDigestHash matches the one in executeParams_ | |
| if (prevBatchDigestHash != executeParams_.prevBatchDigestHash) | |
| revert PrevBatchDigestHashMismatch(); | |
| address transmitter = _recoverSigner( | |
| keccak256(abi.encode(address(socket__), payloadId_)), | |
| transmitterSignature_ | |
| ); | |
| // Construct current digest | |
| (bytes32 appGatewayId, ) = socket__.getPlugConfig(executeParams_.target); | |
| bytes32 constructedDigest = _createDigest( | |
| transmitter, | |
| payloadId_, | |
| appGatewayId, | |
| executeParams_ | |
| ); | |
| // Verify the constructed digest matches the stored one | |
| if (!isAttested[constructedDigest]) revert NotAttested(); | |
| isRemoteExecuted[payloadId_] = true; | |
| } | |
| function proveRemoteExecutions( | |
| bytes32[] calldata previousPayloadIds_, | |
| bytes32 payloadId_, | |
| bytes calldata transmitterSignature_, | |
| ExecuteParams calldata executeParams_ | |
| ) public { | |
| // Calculate prevBatchDigestHash from stored remoteExecutedDigests | |
| bytes32 prevBatchDigestHash = bytes32(0); | |
| for (uint256 i = 0; i < previousPayloadIds_.length; i++) { | |
| if (remoteExecutedDigests[previousPayloadIds_[i]] == bytes32(0)) | |
| revert RemoteExecutionNotFound(); | |
| // Verify the previous payload is marked as executed | |
| if (!isRemoteExecuted[previousPayloadIds_[i]]) | |
| revert("Previous payload not executed"); | |
| prevBatchDigestHash = keccak256( | |
| abi.encodePacked(prevBatchDigestHash, remoteExecutedDigests[previousPayloadIds_[i]]) | |
| ); | |
| } | |
| // Check if the calculated prevBatchDigestHash matches the one in executeParams_ | |
| if (prevBatchDigestHash != executeParams_.prevBatchDigestHash) | |
| revert PrevBatchDigestHashMismatch(); | |
| address transmitter = _recoverSigner( | |
| keccak256(abi.encode(address(socket__), payloadId_)), | |
| transmitterSignature_ | |
| ); | |
| // Construct current digest | |
| (bytes32 appGatewayId, ) = socket__.getPlugConfig(executeParams_.target); | |
| bytes32 constructedDigest = _createDigest( | |
| transmitter, | |
| payloadId_, | |
| appGatewayId, | |
| executeParams_ | |
| ); | |
| // Verify the constructed digest matches the stored one | |
| if (!isAttested[constructedDigest]) revert NotAttested(); | |
| isRemoteExecuted[payloadId_] = true; | |
| } |
🤖 Prompt for AI Agents
In contracts/protocol/switchboard/CCTPSwitchboard.sol around lines 86-122, the
loop that computes prevBatchDigestHash uses remoteExecutedDigests without
guaranteeing those remote executions are final; add a check inside the loop to
ensure each previousPayloadIds_[i] has been marked executed (e.g.,
require(isRemoteExecuted[previousId], "RemoteExecutionNotFinal") or revert
RemoteExecutionNotFinal()) before reading remoteExecutedDigests[previousId], so
you fail early if a prior execution is still processing and avoid computing an
incorrect hash.
| ## IMessageSwitchboard | ||
|
|
||
| | Event | Arguments | Topic | | ||
| | ----- | --------- | ----- | | ||
|
|
||
| ## ICCTPSwitchboard | ||
|
|
||
| | Event | Arguments | Topic | | ||
| | ----- | --------- | ----- | | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove empty interface sections that add no documentation value
The IMessageSwitchboard and ICCTPSwitchboard sections are empty placeholders that clutter the documentation.
Remove both empty sections:
-## IMessageSwitchboard
-
-| Event | Arguments | Topic |
-| ----- | --------- | ----- |
-
-## ICCTPSwitchboard
-
-| Event | Arguments | Topic |
-| ----- | --------- | ----- |
-📝 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.
| ## IMessageSwitchboard | |
| | Event | Arguments | Topic | | |
| | ----- | --------- | ----- | | |
| ## ICCTPSwitchboard | |
| | Event | Arguments | Topic | | |
| | ----- | --------- | ----- | |
🤖 Prompt for AI Agents
In EventTopics.md around lines 233 to 242, the IMessageSwitchboard and
ICCTPSwitchboard sections are empty and add no documentation value; remove both
entire empty sections (the headers and their empty markdown tables) so the file
contains only meaningful, populated sections and headings remain consistent.
Ensure surrounding content spacing and table of contents (if any) are updated to
reflect the removals.
| if ( | ||
| await isConfigSetOnSocket(plug, socket, ZERO_APP_GATEWAY_ID, switchboard) | ||
| ) { | ||
| if (await isConfigSetOnSocket(plug, socket, BYTES32_ZERO, switchboard)) { |
There was a problem hiding this comment.
Incorrect switchboard parameter in config check
The isConfigSetOnSocket call passes switchboard (an address) as the fourth parameter, but based on the function signature in hardhat-scripts/utils/appConfig.ts, it expects switchboardId (a string).
- if (await isConfigSetOnSocket(plug, socket, BYTES32_ZERO, switchboard)) {
+ const switchboardId = "0"; // Disconnected state uses switchboardId "0"
+ if (await isConfigSetOnSocket(plug, socket, BYTES32_ZERO, switchboardId)) {📝 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.
| if (await isConfigSetOnSocket(plug, socket, BYTES32_ZERO, switchboard)) { | |
| const switchboardId = "0"; // Disconnected state uses switchboardId "0" | |
| if (await isConfigSetOnSocket(plug, socket, BYTES32_ZERO, switchboardId)) { |
🤖 Prompt for AI Agents
In hardhat-scripts/admin/disconnect.ts around line 54, the call to
isConfigSetOnSocket passes the Switchboard contract object/address as the fourth
argument but the function expects a switchboardId string; replace the argument
with the switchboardId string (e.g., a local switchboardId variable) — if
switchboardId is not already available, derive it from the contract/address (for
example use switchboard.address or switchboard.address.toLowerCase() as the id)
and pass that string instead.
| // disconnect old fees plug | ||
| arbConfig.feesPlug.connectSocket( | ||
| bytes32(0), | ||
| address(arbConfig.socket), | ||
| address(arbConfig.switchboard) | ||
| ); | ||
| hoax(socketOwner); | ||
| arbConfig.feesPlug.disconnectSocket(); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Test should verify disconnection state
The test disconnects the fees plug but doesn't verify the disconnection was successful before proceeding.
function testDisconnectFeesPlug() public {
// disconnect old fees plug
hoax(socketOwner);
arbConfig.feesPlug.disconnectSocket();
+
+ // Verify disconnection
+ (bytes32 appGatewayId, uint64 switchboardId) = arbConfig.socket.getPlugConfig(address(arbConfig.feesPlug));
+ assertEq(appGatewayId, bytes32(0), "App gateway ID should be reset");
+ assertEq(switchboardId, 0, "Switchboard ID should be reset");📝 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.
| // disconnect old fees plug | |
| arbConfig.feesPlug.connectSocket( | |
| bytes32(0), | |
| address(arbConfig.socket), | |
| address(arbConfig.switchboard) | |
| ); | |
| hoax(socketOwner); | |
| arbConfig.feesPlug.disconnectSocket(); | |
| function testDisconnectFeesPlug() public { | |
| // disconnect old fees plug | |
| hoax(socketOwner); | |
| arbConfig.feesPlug.disconnectSocket(); | |
| // Verify disconnection | |
| (bytes32 appGatewayId, uint64 switchboardId) = arbConfig.socket.getPlugConfig(address(arbConfig.feesPlug)); | |
| assertEq(appGatewayId, bytes32(0), "App gateway ID should be reset"); | |
| assertEq(switchboardId, 0, "Switchboard ID should be reset"); | |
| } |
🤖 Prompt for AI Agents
In test/FeesTest.t.sol around lines 93-96, after calling
arbConfig.feesPlug.disconnectSocket() the test must assert the plug is actually
disconnected; add a check using the plug's public state getter (for example
assertEq(arbConfig.feesPlug.socket(), address(0)) or
assertFalse(arbConfig.feesPlug.isConnected())), or alternatively call an action
that requires a connected socket and expect it to revert, to ensure the
disconnect succeeded before proceeding.
| (, Transaction memory transaction, , , , ) = abi.decode( | ||
| params.precompileData, | ||
| (address, Transaction, WriteFinality, uint256, uint256, address) | ||
| ); |
There was a problem hiding this comment.
Incorrect type in abi.decode causes compilation error
The decode statement expects address as the last parameter but the comment and context suggest it should be uint64 for switchboard ID.
Fix the type mismatch:
(, Transaction memory transaction, , , , ) = abi.decode(
params.precompileData,
- (address, Transaction, WriteFinality, uint256, uint256, address)
+ (address, Transaction, WriteFinality, uint256, uint256, uint64)
);📝 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.
| (, Transaction memory transaction, , , , ) = abi.decode( | |
| params.precompileData, | |
| (address, Transaction, WriteFinality, uint256, uint256, address) | |
| ); | |
| (, Transaction memory transaction, , , , ) = abi.decode( | |
| params.precompileData, | |
| (address, Transaction, WriteFinality, uint256, uint256, uint64) | |
| ); |
🤖 Prompt for AI Agents
In test/SetupTest.t.sol around lines 1078-1081, the abi.decode tuple uses
address as the final type but the context/comment indicates that value is a
uint64 Switchboard ID; change the last type in the abi.decode type list from
address to uint64 so the decoded tuple matches the expected types (e.g.,
(address, Transaction, WriteFinality, uint256, uint256, uint64)); ensure any
variables receiving that decoded value are declared as uint64 (or explicitly
cast) to resolve the compilation error.
| // Count valid plugs first. In some cases we might have contractIds such that oly a subset is | ||
| // deployed on a chain. for ex, vault on source, and supertoken on destination. | ||
| uint256 validPlugCount = 0; | ||
| for (uint i = 0; i < contractIds_.length; i++) { | ||
| bytes32 plug = appGateway_.getOnChainAddress(contractIds_[i], chainSlug_); | ||
| if (plug != bytes32(0)) { | ||
| validPlugCount++; | ||
| } | ||
| } | ||
|
|
||
| // Create array with exact size needed | ||
| AppGatewayConfig[] memory configs = new AppGatewayConfig[](validPlugCount); | ||
| uint256 configIndex = 0; | ||
|
|
||
| SocketContracts memory socketConfig = getSocketConfig(chainSlug_); | ||
| for (uint i = 0; i < contractIds_.length; i++) { | ||
| address plug = appGateway_.getOnChainAddress(contractIds_[i], chainSlug_); | ||
| bytes32 plug = appGateway_.getOnChainAddress(contractIds_[i], chainSlug_); | ||
| uint64 switchboardId = configurations.switchboards(chainSlug_, appGateway_.sbType()); | ||
| if (plug != bytes32(0)) { | ||
| configs[configIndex] = AppGatewayConfig({ | ||
| plug: plug, | ||
| chainSlug: chainSlug_, | ||
| plugConfig: PlugConfigGeneric({ | ||
| appGatewayId: toBytes32Format(address(appGateway_)), | ||
| switchboardId: switchboardId | ||
| }) | ||
| }); | ||
| configIndex++; | ||
| } | ||
| } | ||
|
|
||
| configs[i] = AppGatewayConfig({ | ||
| plug: plug, | ||
| chainSlug: chainSlug_, | ||
| plugConfig: PlugConfig({ | ||
| appGatewayId: encodeAppGatewayId(address(appGateway_)), | ||
| switchboard: address(socketConfig.switchboard) | ||
| }) | ||
| }); | ||
| // Only call watcher if we have valid configs | ||
| if (validPlugCount > 0) { | ||
| watcherMultiCall( | ||
| address(configurations), | ||
| abi.encodeWithSelector(Configurations.setAppGatewayConfigs.selector, configs) | ||
| ); | ||
| } | ||
| watcherMultiCall( | ||
| address(configurations), | ||
| abi.encodeWithSelector(Configurations.setAppGatewayConfigs.selector, configs) | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize setupGatewayAndPlugs to avoid redundant iterations
The function iterates through contractIds_ twice - once to count valid plugs and again to populate configs. This doubles the computational cost unnecessarily.
Combine into a single pass with dynamic array:
function setupGatewayAndPlugs(
uint32 chainSlug_,
IAppGateway appGateway_,
bytes32[] memory contractIds_
) internal {
- // Count valid plugs first. In some cases we might have contractIds such that oly a subset is
- // deployed on a chain. for ex, vault on source, and supertoken on destination.
- uint256 validPlugCount = 0;
- for (uint i = 0; i < contractIds_.length; i++) {
- bytes32 plug = appGateway_.getOnChainAddress(contractIds_[i], chainSlug_);
- if (plug != bytes32(0)) {
- validPlugCount++;
- }
- }
-
- // Create array with exact size needed
- AppGatewayConfig[] memory configs = new AppGatewayConfig[](validPlugCount);
- uint256 configIndex = 0;
+ AppGatewayConfig[] memory tempConfigs = new AppGatewayConfig[](contractIds_.length);
+ uint256 configCount = 0;
for (uint i = 0; i < contractIds_.length; i++) {
bytes32 plug = appGateway_.getOnChainAddress(contractIds_[i], chainSlug_);
uint64 switchboardId = configurations.switchboards(chainSlug_, appGateway_.sbType());
if (plug != bytes32(0)) {
- configs[configIndex] = AppGatewayConfig({
+ tempConfigs[configCount] = AppGatewayConfig({
plug: plug,
chainSlug: chainSlug_,
plugConfig: PlugConfigGeneric({
appGatewayId: toBytes32Format(address(appGateway_)),
switchboardId: switchboardId
})
});
- configIndex++;
+ configCount++;
}
}
// Only call watcher if we have valid configs
- if (validPlugCount > 0) {
+ if (configCount > 0) {
+ // Resize array to actual count
+ AppGatewayConfig[] memory configs = new AppGatewayConfig[](configCount);
+ for (uint i = 0; i < configCount; i++) {
+ configs[i] = tempConfigs[i];
+ }
watcherMultiCall(
address(configurations),
abi.encodeWithSelector(Configurations.setAppGatewayConfigs.selector, configs)
);
}
}📝 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.
| // Count valid plugs first. In some cases we might have contractIds such that oly a subset is | |
| // deployed on a chain. for ex, vault on source, and supertoken on destination. | |
| uint256 validPlugCount = 0; | |
| for (uint i = 0; i < contractIds_.length; i++) { | |
| bytes32 plug = appGateway_.getOnChainAddress(contractIds_[i], chainSlug_); | |
| if (plug != bytes32(0)) { | |
| validPlugCount++; | |
| } | |
| } | |
| // Create array with exact size needed | |
| AppGatewayConfig[] memory configs = new AppGatewayConfig[](validPlugCount); | |
| uint256 configIndex = 0; | |
| SocketContracts memory socketConfig = getSocketConfig(chainSlug_); | |
| for (uint i = 0; i < contractIds_.length; i++) { | |
| address plug = appGateway_.getOnChainAddress(contractIds_[i], chainSlug_); | |
| bytes32 plug = appGateway_.getOnChainAddress(contractIds_[i], chainSlug_); | |
| uint64 switchboardId = configurations.switchboards(chainSlug_, appGateway_.sbType()); | |
| if (plug != bytes32(0)) { | |
| configs[configIndex] = AppGatewayConfig({ | |
| plug: plug, | |
| chainSlug: chainSlug_, | |
| plugConfig: PlugConfigGeneric({ | |
| appGatewayId: toBytes32Format(address(appGateway_)), | |
| switchboardId: switchboardId | |
| }) | |
| }); | |
| configIndex++; | |
| } | |
| } | |
| configs[i] = AppGatewayConfig({ | |
| plug: plug, | |
| chainSlug: chainSlug_, | |
| plugConfig: PlugConfig({ | |
| appGatewayId: encodeAppGatewayId(address(appGateway_)), | |
| switchboard: address(socketConfig.switchboard) | |
| }) | |
| }); | |
| // Only call watcher if we have valid configs | |
| if (validPlugCount > 0) { | |
| watcherMultiCall( | |
| address(configurations), | |
| abi.encodeWithSelector(Configurations.setAppGatewayConfigs.selector, configs) | |
| ); | |
| } | |
| watcherMultiCall( | |
| address(configurations), | |
| abi.encodeWithSelector(Configurations.setAppGatewayConfigs.selector, configs) | |
| ); | |
| } | |
| function setupGatewayAndPlugs( | |
| uint32 chainSlug_, | |
| IAppGateway appGateway_, | |
| bytes32[] memory contractIds_ | |
| ) internal { | |
| // Allocate a temp array as large as contractIds_, then compact it in place | |
| AppGatewayConfig[] memory tempConfigs = new AppGatewayConfig[](contractIds_.length); | |
| uint256 configCount = 0; | |
| for (uint i = 0; i < contractIds_.length; i++) { | |
| bytes32 plug = appGateway_.getOnChainAddress(contractIds_[i], chainSlug_); | |
| uint64 switchboardId = configurations.switchboards(chainSlug_, appGateway_.sbType()); | |
| if (plug != bytes32(0)) { | |
| tempConfigs[configCount] = AppGatewayConfig({ | |
| plug: plug, | |
| chainSlug: chainSlug_, | |
| plugConfig: PlugConfigGeneric({ | |
| appGatewayId: toBytes32Format(address(appGateway_)), | |
| switchboardId: switchboardId | |
| }) | |
| }); | |
| configCount++; | |
| } | |
| } | |
| // Only call watcher if we have valid configs | |
| if (configCount > 0) { | |
| // Resize array to the actual number of configs | |
| AppGatewayConfig[] memory configs = new AppGatewayConfig[](configCount); | |
| for (uint i = 0; i < configCount; i++) { | |
| configs[i] = tempConfigs[i]; | |
| } | |
| watcherMultiCall( | |
| address(configurations), | |
| abi.encodeWithSelector(Configurations.setAppGatewayConfigs.selector, configs) | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
In test/SetupTest.t.sol around lines 1131 to 1168, remove the two-pass iteration
over contractIds_ and replace it with a single pass: allocate a temporary
AppGatewayConfig[] memory tmp = new AppGatewayConfig[](contractIds_.length),
iterate once over contractIds_, for each non-zero plug build and store the
AppGatewayConfig into tmp[index++] and increment count, then after the loop
allocate the final AppGatewayConfig[] memory configs = new
AppGatewayConfig[](count) and copy the populated entries from tmp[0..count-1]
into configs; finally call watcherMultiCall only if count > 0. Ensure you keep
computing switchboardId inside the loop and update variable names accordingly.
4 participants
Summary by CodeRabbit
New Features
Refactor
Chores
Tests