-
Notifications
You must be signed in to change notification settings - Fork 8
Deploy scripts #185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deploy scripts #185
Conversation
…ket-protocol into deploy-scripts
WalkthroughThis update introduces a comprehensive migration from using switchboard addresses to using switchboard IDs (uint64) throughout the codebase, affecting contract interfaces, deployment scripts, configuration files, and test utilities. The changes also add new contracts, events, errors, and function signatures related to the MessageSwitchboard module, update deployment metadata, and revise supporting scripts and types for consistency with the new switchboard ID paradigm. Changes
Sequence Diagram(s)sequenceDiagram
participant Deployer
participant Socket
participant MessageSwitchboard
participant FastSwitchboard
participant CCTPSwitchboard
Deployer->>Socket: registerSwitchboard(switchboardAddress)
Socket->>Socket: assign switchboardId (uint64)
Deployer->>MessageSwitchboard: deploy(chainSlug, socket, owner)
Deployer->>Socket: registerSwitchboard(MessageSwitchboard)
Deployer->>FastSwitchboard: deploy(chainSlug, socket, owner)
Deployer->>Socket: registerSwitchboard(FastSwitchboard)
Deployer->>CCTPSwitchboard: deploy(chainSlug, socket, owner)
Deployer->>Socket: registerSwitchboard(CCTPSwitchboard)
Deployer->>MessageSwitchboard: setSiblingConfig(remoteChain, sbId, socketAddr, sbAddr)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
🔭 Outside diff range comments (1)
hardhat-scripts/deploy/2.roles.ts (1)
85-87
: Update watcher role assignment logic for MessageSwitchboardThe current logic only assigns the watcher address to FastSwitchboard and CCTPSwitchboard for WATCHER_ROLE. MessageSwitchboard should follow the same pattern.
const targetAddress = - [Contracts.FastSwitchboard, Contracts.CCTPSwitchboard].includes( + [Contracts.FastSwitchboard, Contracts.CCTPSwitchboard, Contracts.MessageSwitchboard].includes( contractName as Contracts ) && roleName === ROLES.WATCHER_ROLE ? watcher : signer.address;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (27)
Errors.md
(3 hunks)EventTopics.md
(6 hunks)FunctionSignatures.md
(11 hunks)contracts/protocol/SocketBatcher.sol
(3 hunks)contracts/protocol/interfaces/ISocketBatcher.sol
(1 hunks)deployments/dev_addresses.json
(1 hunks)deployments/dev_verification.json
(1 hunks)foundry.toml
(1 hunks)hardhat-scripts/admin/disable-sb.ts
(2 hunks)hardhat-scripts/admin/disconnect.ts
(4 hunks)hardhat-scripts/config/config.ts
(2 hunks)hardhat-scripts/constants/constants.ts
(1 hunks)hardhat-scripts/constants/feeConstants.ts
(1 hunks)hardhat-scripts/constants/types.ts
(1 hunks)hardhat-scripts/deploy/1.deploy.ts
(2 hunks)hardhat-scripts/deploy/2.roles.ts
(1 hunks)hardhat-scripts/deploy/3.configureChains.ts
(9 hunks)hardhat-scripts/deploy/6.connect.ts
(5 hunks)hardhat-scripts/deploy/9.setupTransmitter.ts
(1 hunks)hardhat-scripts/utils/appConfig.ts
(2 hunks)hardhat-scripts/utils/overrides.ts
(1 hunks)hardhat-scripts/utils/sign.ts
(4 hunks)hardhat.config.ts
(1 hunks)src/enums.ts
(1 hunks)src/signer.ts
(2 hunks)src/types.ts
(1 hunks)test/SetupTest.t.sol
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
hardhat-scripts/admin/disable-sb.ts (1)
hardhat-scripts/constants/constants.ts (1)
FAST_SWITCHBOARD_TYPE
(9-9)
hardhat-scripts/deploy/1.deploy.ts (1)
hardhat-scripts/utils/deployUtils.ts (1)
getOrDeploy
(38-77)
hardhat-scripts/admin/disconnect.ts (5)
hardhat-scripts/utils/appConfig.ts (1)
isConfigSetOnSocket
(4-16)hardhat-scripts/constants/constants.ts (1)
BYTES32_ZERO
(12-12)hardhat-scripts/utils/overrides.ts (1)
overrides
(43-54)hardhat-scripts/utils/address.ts (1)
toBytes32FormatHexString
(38-52)hardhat-scripts/config/config.ts (1)
EVMX_CHAIN_ID
(100-100)
hardhat-scripts/utils/appConfig.ts (2)
hardhat-scripts/config/config.ts (1)
watcher
(96-96)hardhat-scripts/utils/address.ts (1)
toBytes32Format
(54-61)
hardhat-scripts/utils/sign.ts (5)
hardhat-scripts/utils/address.ts (1)
getAddresses
(7-23)hardhat-scripts/config/config.ts (2)
mode
(6-8)EVMX_CHAIN_ID
(100-100)src/types.ts (1)
EVMxAddressesObj
(33-49)src/signer.ts (1)
signWatcherMultiCallMessage
(3-25)hardhat-scripts/utils/deployUtils.ts (1)
getInstance
(126-130)
hardhat-scripts/deploy/3.configureChains.ts (7)
hardhat-scripts/utils/deployUtils.ts (4)
DeployParams
(31-36)getInstance
(126-130)storeAddresses
(143-170)updateContractSettings
(323-362)hardhat-scripts/config/config.ts (2)
mode
(6-8)EVMX_CHAIN_ID
(100-100)hardhat-scripts/constants/types.ts (1)
DeploymentAddresses
(3-5)hardhat-scripts/constants/constants.ts (3)
FAST_SWITCHBOARD_TYPE
(9-9)CCTP_SWITCHBOARD_TYPE
(10-10)MSG_SB_FEES
(14-14)hardhat-scripts/utils/address.ts (1)
toBytes32FormatHexString
(38-52)src/types.ts (1)
ChainAddressesObj
(18-31)hardhat-scripts/utils/overrides.ts (1)
overrides
(43-54)
🪛 LanguageTool
EventTopics.md
[grammar] ~39-~39: Use correct spacing
Context: ...f12463c30019b4ed354ba8bc7bc26f28` | ## IFastSwitchboard | Event | Arguments | Topic | | ----- | ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~42-~42: Use hyphens correctly
Context: ...guments | Topic | | ----- | --------- | ----- | ## SocketBatcher | Event ...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~233-~233: Use correct spacing
Context: ...0e689022da30a08905ba5ced053219eb` | ## IMessageSwitchboard | Event | Arguments | Topic | | ----- | ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~236-~236: Use hyphens correctly
Context: ...guments | Topic | | ----- | --------- | ----- | ## ICCTPSwitchboard | Event | Arguments |...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~265-~265: There might be a mistake here.
Context: ...441b19729b9601c5fc17ae1cf99a8a52` | ## MessageSwitchboard | Event | Argum...
(QB_NEW_EN_OTHER)
FunctionSignatures.md
[grammar] ~530-~530: There might be a mistake here.
Context: ...wnership |
0xf2fde38b` | ## MessageSwitchboard | Function | Signa...
(QB_NEW_EN_OTHER)
🔇 Additional comments (59)
hardhat-scripts/constants/feeConstants.ts (1)
23-23
: Verify DEV fee pool address on-chain
- File: hardhat-scripts/constants/feeConstants.ts (line 23)
- Confirm that the new address
0x13A3018920c7b56B20dd34E29C298121025E6de4
has non-empty contract code on the DEV network using a valid RPC endpoint. For example:cast code 0x13A3018920c7b56B20dd34E29C298121025E6de4 \ --rpc-url https://sepolia.infura.io/v3/<YOUR_PROJECT_ID>- Verify the deployed contract’s functionality matches the intended fee pool behavior in DEV.
hardhat-scripts/utils/overrides.ts (1)
36-40
: LGTM - Overrides removal for EVMX chain.Commenting out the explicit gas and transaction type overrides for EVMX chain is appropriate, allowing the system to use default values or runtime calculations. This aligns with the standardization efforts across the codebase.
hardhat-scripts/constants/types.ts (1)
19-19
: Good naming change for clarity.The property name change from
switchboard
toswitchboardId
improves clarity and aligns with the migration to switchboard ID-based identification throughout the codebase.src/signer.ts (2)
10-10
: Good improvement for nonce uniqueness.Replacing the simple timestamp with the new
getNonce()
function improves collision resistance by adding randomness to the timestamp-based nonce generation.
28-33
: Nonce generation implementation is robust.The nonce generation approach effectively combines timestamp with random component to minimize collision probability. The implementation correctly concatenates the values as strings before converting to number.
hardhat.config.ts (1)
142-143
: Explorer endpoints reachable: update approvedThe browser endpoint returned HTTP 200 and the API endpoint returned HTTP 400 (indicating the API is reachable but missing required query parameters). Both URLs are functional as expected—no further changes needed.
src/types.ts (1)
23-26
: LGTM: Type extension supports switchboard ID migrationThe addition of switchboard contract addresses and their corresponding IDs aligns well with the migration from address-based to ID-based switchboard identification. The naming convention is consistent and all properties are appropriately typed as strings.
hardhat-scripts/config/config.ts (1)
38-38
: Configuration refinement for DEV chainsCommenting out BASE_SEPOLIA from DEV deployment chains appears to be a deliberate configuration refinement.
hardhat-scripts/constants/constants.ts (1)
12-12
: Good refactoring: More generic constant namingReplacing
ZERO_APP_GATEWAY_ID
withBYTES32_ZERO
provides better semantic clarity and reusability across different contexts.foundry.toml (1)
13-35
: All contract addresses verified for chain ID 14323-✅ Every address listed in foundry.toml was found in deployments/dev_addresses.json
-✅ deployments/dev_addresses.json contains the correct chain ID (14323)No further action needed.
hardhat-scripts/deploy/2.roles.ts (1)
20-20
: Good addition: MessageSwitchboard role configurationAdding MessageSwitchboard with the same roles as other switchboard contracts maintains consistency in the role-based access control system.
hardhat-scripts/deploy/9.setupTransmitter.ts (1)
24-25
: LGTM! Logical improvement to setup sequence.The reordering to call
checkAndDepositNative()
beforeapproveAuctionManager()
is a sensible improvement that ensures the transmitter has sufficient native tokens before attempting to approve the auction manager, potentially preventing transaction failures.src/enums.ts (1)
58-62
: LGTM! Consistent enum additions for switchboard ID migration.The new enum entries support the system-wide migration from switchboard addresses to switchboard IDs. The naming convention is consistent and aligns with the broader refactoring effort.
hardhat-scripts/admin/disable-sb.ts (1)
1-1
: LGTM! Clean import removal.Removing the unused
constants
import improves code cleanliness.hardhat-scripts/deploy/1.deploy.ts (2)
345-353
: LGTM! Consistent MessageSwitchboard deployment.The MessageSwitchboard deployment follows the established pattern used for other switchboards, with appropriate constructor parameters and consistent error handling.
468-474
: LGTM! Improved deployment logging.The additional console log provides valuable debugging information by showing both the proxy address and implementation address after deployment, which aids in troubleshooting and verification.
hardhat-scripts/utils/appConfig.ts (2)
8-15
: LGTM! Proper migration to switchboard ID.The parameter name change from
switchboard
toswitchboardId
and the updated comparison logic correctly implement the migration from address-based to ID-based switchboard identification. The string comparison approach is appropriate for uint64 IDs.
23-32
: LGTM! Consistent switchboard ID implementation.The changes maintain consistency with the
isConfigSetOnSocket
function, properly converting both stored values and input parameters to strings for comparison. This approach works correctly for uint64 switchboard IDs.hardhat-scripts/utils/sign.ts (4)
2-2
: LGTM: Improved type safety with EVMxAddressesObj importThe addition of the
EVMxAddressesObj
type import enhances type safety and aligns with the broader migration to using typed address objects throughout the codebase.
33-35
: LGTM: Enhanced type safety with direct typed accessThe replacement of nested address access with a more direct and typed approach using
EVMxAddressesObj
improves code readability and type safety. The extraction and casting pattern is consistent with modern TypeScript practices.
47-51
: LGTM: Consistent typed access pattern and improved formattingThe implementation maintains consistency with the
signWatcherMessage
function by using the same typed address extraction pattern. The addition of a blank line before the comment improves code readability.
65-65
: Minor formatting improvementThe addition of a blank line before the comment enhances code readability and follows good formatting practices.
test/SetupTest.t.sol (2)
970-970
: LGTM: Correct migration to switchboard IDThe change from passing the switchboard address to passing the switchboard ID aligns perfectly with the updated
ISocketBatcher.attestAndExecute
interface. This ensures consistency with the protocol-wide migration to usinguint64
switchboard IDs.
1013-1013
: LGTM: Consistent switchboard ID usageThe modification to use
switchboardId()
instead of the switchboard address is consistent with the interface changes and maintains alignment with the updatedattestCCTPAndProveAndExecute
function signature.contracts/protocol/interfaces/ISocketBatcher.sol (1)
21-21
: LGTM: Clean migration to switchboard ID parameterThe parameter type change from
address switchboard_
touint64 switchboardId_
is well-executed and aligns with the protocol-wide migration to using switchboard IDs instead of direct addresses. This improves abstraction by allowing the implementing contract to resolve switchboard addresses internally.hardhat-scripts/deploy/6.connect.ts (6)
48-49
: LGTM: Clear variable rename reflecting ID usageThe variable rename from
switchboard
toswitchboardId
accurately reflects the change from using addresses to using IDs, improving code clarity and consistency with the new paradigm.
53-53
: LGTM: Updated function call with switchboard IDThe function call correctly uses the new
switchboardId
parameter, maintaining consistency with the updatedisConfigSetOnSocket
function signature.
59-62
: LGTM: Plug connection updated to use switchboard IDThe
connectSocket
function call correctly uses theswitchboardId
parameter instead of the switchboard address, aligning with the updated contract interface.
116-116
: LGTM: Consistent ID usage in EVMx configurationThe use of
switchboardId
from the address object maintains consistency with the migration pattern throughout the deployment scripts.
131-131
: LGTM: Updated configuration check with switchboard IDThe
isConfigSetOnEVMx
function call correctly passes theswitchboardId
parameter, maintaining consistency with the updated utility function signature.
140-140
: LGTM: Consistent switchboard ID in app configurationThe configuration object correctly uses
switchboardId
instead of the address, ensuring compatibility with the updated configuration structure.deployments/dev_verification.json (1)
5-122
: Deployment address verification summary
- All primary contract addresses are valid Ethereum checksums (40-hex characters, prefixed with “0x”).
- No unintended duplicate primary addresses were detected.
- One address (
0x90B2f8766F47E0A108D85DC43Ddb45A1d5e486FB
) appears multiple times as a dependency (SchedulePrecompile, ReadPrecompile, PromiseResolver)—this is expected since it points to the Watcher contract.- SchedulePrecompile parameters (BigNumber hex values and timing fields) have been updated as intended.
Please manually confirm on network 7625382 (e.g., via Etherscan or your node) that:
- Each contract’s bytecode at these addresses matches the expected build artifacts.
- The updated SchedulePrecompile configuration values align with your system requirements.
hardhat-scripts/admin/disconnect.ts (5)
4-8
: LGTM!The import changes correctly reflect the migration from
ZERO_APP_GATEWAY_ID
toBYTES32_ZERO
, aligning with the codebase-wide shift to use bytes32 zero values for representing empty identifiers.
15-15
: Import addition looks good.The
toBytes32FormatHexString
utility import is necessary for converting addresses to bytes32 format in the updated configuration logic.
60-62
: Function call update looks correct.The change from
connectSocket
todisconnectSocket
without parameters is appropriate for the disconnect operation, with only transaction overrides being passed.
147-151
: Logic is inverted for fees plug disconnect check.The condition checks if the current fees plug equals
BYTES32_ZERO
and returns early. For a disconnect operation, it should proceed only if the fees plug is NOT already disconnected.- if (currentFeesPlug.toString() === BYTES32_ZERO.toString()) { + if (currentFeesPlug.toString() !== BYTES32_ZERO.toString()) { console.log(`Fees plug already disconnected on ${chain}`); return; }Likely an incorrect or invalid review comment.
54-57
: Logic appears to be inverted for disconnect check.The function checks if the config is already set to
BYTES32_ZERO
and returns early if true. However, since this is a disconnect operation, shouldn't it proceed only if the plug is currently connected (i.e., config is NOT set toBYTES32_ZERO
)?The current logic would skip disconnecting plugs that are actually connected.
Consider inverting the condition:
- if (await isConfigSetOnSocket(plug, socket, BYTES32_ZERO, switchboard)) { + if (!(await isConfigSetOnSocket(plug, socket, BYTES32_ZERO, switchboard))) { console.log(`${plugContract} Socket Config on ${chain} already disconnected!`); return; }Likely an incorrect or invalid review comment.
Errors.md (3)
52-55
: Error refactoring improves clarity.Splitting the compound error
SwitchboardExistsOrDisabled()
intoSwitchboardExists()
and addingPlugNotConnected()
provides better error granularity and aligns with the new disconnect functionality.
71-76
: New error section properly documented.The addition of
NotSupported()
error forMessagePlugBase.sol
is appropriate for handling unsupported operations in the base contract.
95-104
: MessageSwitchboard errors properly defined.The new error definitions for
MessageSwitchboard.sol
are comprehensive and appropriate. Note thatAlreadyAttested()
andWatcherNotFound()
are shared withFastSwitchboard.sol
, indicating common switchboard functionality.deployments/dev_addresses.json (3)
3-13
: Chain 421614 configuration properly updated.The addition of switchboard IDs (FastSwitchboardId: "1", CCTPSwitchboardId: "2", MessageSwitchboardId: "3") and the new MessageSwitchboard address align with the migration to ID-based switchboard identification.
15-40
: EVMX chain configuration properly added.The new chain 14323 configuration includes all necessary EVMX-specific contracts (Watcher, RequestHandler, Configurations, etc.) with valid addresses and appropriate startBlock.
42-52
: Chain 11155420 configuration consistently updated.The updates follow the same pattern as other chains with appropriate switchboard IDs and MessageSwitchboard address addition.
contracts/protocol/SocketBatcher.sol (3)
43-51
: Switchboard ID parameter change improves security.The migration from
address switchboard_
touint64 switchboardId_
centralizes switchboard address resolution throughsocket__.switchboardAddresses()
, preventing the use of arbitrary switchboard addresses and improving security.
67-73
: Consistent switchboard ID implementation.The changes properly implement the switchboard ID pattern, with address resolution at line 69 and correct usage in
createPayloadId
at line 72.
75-79
: Switchboard address usage is correct.The resolved switchboard address is properly used in both
attestVerifyAndProveExecutions
(lines 75-79) andsyncOut
(line 90) calls, maintaining the correct execution flow.Also applies to: 90-90
hardhat-scripts/deploy/3.configureChains.ts (9)
27-27
: Required imports added for deployment tracking.The addition of
DeployParams
andstoreAddresses
imports supports the new deployment tracking functionality.Also applies to: 33-33
56-61
: Deployment tracking structure properly initialized.The
DeployParams
object provides a clean way to track deployment information throughout the configuration process.
67-74
: FastSwitchboard registration correctly implemented.The registration process properly captures and stores the switchboard ID in the deployment addresses.
76-93
: Switchboard registrations follow consistent pattern.Both CCTPSwitchboard and MessageSwitchboard registrations properly capture and store their respective IDs, maintaining consistency with the FastSwitchboard pattern.
100-105
: Updated function call properly passes switchboard IDs.The
setOnchainContracts
call now correctly passes the registered switchboard IDs as parameters, aligning with the ID-based identification system.
114-115
: Configuration finalization properly implemented.The addition of
setSiblingConfig
for MessageSwitchboard setup andstoreAddresses
to persist deployment data ensures complete configuration.
137-138
: Switchboard configuration correctly uses IDs.The
setOnchainContracts
function properly uses the passed switchboard IDs in thesetSwitchboard
calls, maintaining consistency with the ID-based system.Also applies to: 157-160, 169-171
209-261
: MessageSwitchboard sibling configuration well implemented.The
setSiblingConfig
function properly:
- Handles missing remote configurations gracefully
- Uses consistent address formatting with
toBytes32FormatHexString
- Applies appropriate transaction overrides
- Provides clear logging for debugging
331-358
: Switchboard registration properly returns ID.The
registerSb
function correctly:
- Checks for existing registration (non-zero ID)
- Registers the switchboard if needed
- Retrieves and returns the switchboard ID as a string
EventTopics.md (3)
22-38
: Confirm topic hashes after parameter-type changeFor all updated
Socket
events the ABI parameter list changed fromaddress
/bytes32
touint64 switchboardId
.
That alters the event signature and therefore the topic hash. Please re-generate the keccak256 topic hashes from the compiler and cross-check the values in this table to avoid mismatches that would break indexers / log parsing.
195-196
: Argument naming drift
SwitchboardSet
now mixessbType: bytes32
withswitchboardId: uint64
.
All other docs moved away from raw addresses/bytes32. Double-check the contract – if it still emits a bytes32 identifier for type that’s fine, but if this was meant to be an enum/int then update the doc.
265-279
: Inconsistent identifier usage in MessageSwitchboard eventsThe paradigm everywhere else is
switchboardId: uint64
.
SiblingConfigSet
still listsswitchboard: bytes32
. If the implementation indeed works with IDs, change the argument toswitchboardId
. Otherwise note clearly why a bytes32 is required (e.g., hashed address).FunctionSignatures.md (1)
61-70
: Collision check for new Socket methodsNew signatures (
disconnect
,gasLimitBuffer
, etc.) look fine but please runforge inspect <Contract> sigs
(or similar) to ensure no 4-byte collision with existing functions – especially in upgradeable contracts where behaviour clashes are catastrophic.
| `PlugConnected` | `(plug: address, appGatewayId: bytes32, switchboardId: uint64)` | `0xb2a45daaee4fc6ced936700efec176684095e7b77c4cb1419b578ecc6f2ebce6` | | ||
| `PlugDisconnected` | `(plug: address)` | `0x474a53f61630e976f47075b6029ba8d55d0563151bdb9222c5dbdc88f7af6f51` | | ||
| `RoleGranted` | `(role: bytes32, grantee: address)` | `0x2ae6a113c0ed5b78a53413ffbb7679881f11145ccfba4fb92e863dfcd5a1d2f3` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Missing indexed
indication
PlugConnected
and PlugDisconnected
frequently emit plug
as indexed address
in Solidity so that subscribers can filter by plug. Consider explicitly noting indexing (plug: indexed address
) in the arguments column like the rest of the doc for consistency.
🤖 Prompt for AI Agents
In EventTopics.md around lines 30 to 32, the event argument types for
`PlugConnected` and `PlugDisconnected` do not indicate that `plug` is indexed,
which is common in Solidity for filtering. Update the argument types to
explicitly mark `plug` as `indexed address` to maintain consistency and clarity
in the documentation.
## IFastSwitchboard | ||
|
||
| Event | Arguments | Topic | | ||
| ----- | --------- | ----- | | ||
| `AppGatewayCallRequested` | `(triggerId: bytes32, appGatewayId: bytes32, switchboard: address, plug: address, overrides: bytes, payload: bytes)` | `0x5c88d65ab8ba22a57e582bd8ddfa9801cc0ca6be6cb3182baaedc705a612419e` | | ||
| `ExecutionFailed` | `(payloadId: bytes32, exceededMaxCopy: bool, returnData: bytes)` | `0x385334bc68a32c4d164625189adc7633e6074eb1b837fb4d11d768245151e4ce` | | ||
| `ExecutionSuccess` | `(payloadId: bytes32, exceededMaxCopy: bool, returnData: bytes)` | `0x324d63a433b21a12b90e79cd2ba736b2a5238be6165e03b750fa4a7d5193d5d9` | | ||
| `OwnershipHandoverCanceled` | `(pendingOwner: address)` | `0xfa7b8eab7da67f412cc9575ed43464468f9bfbae89d1675917346ca6d8fe3c92` | | ||
| `OwnershipHandoverRequested` | `(pendingOwner: address)` | `0xdbf36a107da19e49527a7176a1babf963b4b0ff8cde35ee35d6cd8f1f9ac7e1d` | | ||
| `OwnershipTransferred` | `(oldOwner: address, newOwner: address)` | `0x8be0079c531659141344cd1fd0a4f28419497f9722a3daafe3b4186f6b6457e0` | | ||
| `PlugConnected` | `(plug: address, appGatewayId: bytes32, switchboard: address)` | `0x90c5924e27cfb6e3a688e729083681f30494ae2615ae14aac3bc807a0c436a88` | | ||
| `RoleGranted` | `(role: bytes32, grantee: address)` | `0x2ae6a113c0ed5b78a53413ffbb7679881f11145ccfba4fb92e863dfcd5a1d2f3` | | ||
| `RoleRevoked` | `(role: bytes32, revokee: address)` | `0x155aaafb6329a2098580462df33ec4b7441b19729b9601c5fc17ae1cf99a8a52` | | ||
| `SocketFeeManagerUpdated` | `(oldSocketFeeManager: address, newSocketFeeManager: address)` | `0xdcb02e10d5220346a4638aa2826eaab1897306623bc40a427049e4ebd12255b4` | | ||
| `SwitchboardAdded` | `(switchboard: address)` | `0x1595852923edfbbf906f09fc8523e4cfb022a194773c4d1509446b614146ee88` | | ||
| `SwitchboardDisabled` | `(switchboard: address)` | `0x1b4ee41596b4e754e5665f01ed6122b356f7b36ea0a02030804fac7fa0fdddfc` | | ||
| `SwitchboardEnabled` | `(switchboard: address)` | `0x6909a9974e3eec619bc479ba882d30a5ef1219b72ab1ce6a354516e91be317b8` | | ||
|
||
## SocketBatcher |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Empty section may confuse readers
## IFastSwitchboard
contains only the table header. Either populate it with the events (e.g. Attested
) or remove the section until implemented to avoid the impression of incomplete documentation.
-| Event | Arguments | Topic |
-| ----- | --------- | ----- |
+| Event | Arguments | Topic |
+| ----- | --------- | ----- |
+| *None* (contract emits no events) | | |
📝 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.
## IFastSwitchboard | |
| Event | Arguments | Topic | | |
| ----- | --------- | ----- | | |
| `AppGatewayCallRequested` | `(triggerId: bytes32, appGatewayId: bytes32, switchboard: address, plug: address, overrides: bytes, payload: bytes)` | `0x5c88d65ab8ba22a57e582bd8ddfa9801cc0ca6be6cb3182baaedc705a612419e` | | |
| `ExecutionFailed` | `(payloadId: bytes32, exceededMaxCopy: bool, returnData: bytes)` | `0x385334bc68a32c4d164625189adc7633e6074eb1b837fb4d11d768245151e4ce` | | |
| `ExecutionSuccess` | `(payloadId: bytes32, exceededMaxCopy: bool, returnData: bytes)` | `0x324d63a433b21a12b90e79cd2ba736b2a5238be6165e03b750fa4a7d5193d5d9` | | |
| `OwnershipHandoverCanceled` | `(pendingOwner: address)` | `0xfa7b8eab7da67f412cc9575ed43464468f9bfbae89d1675917346ca6d8fe3c92` | | |
| `OwnershipHandoverRequested` | `(pendingOwner: address)` | `0xdbf36a107da19e49527a7176a1babf963b4b0ff8cde35ee35d6cd8f1f9ac7e1d` | | |
| `OwnershipTransferred` | `(oldOwner: address, newOwner: address)` | `0x8be0079c531659141344cd1fd0a4f28419497f9722a3daafe3b4186f6b6457e0` | | |
| `PlugConnected` | `(plug: address, appGatewayId: bytes32, switchboard: address)` | `0x90c5924e27cfb6e3a688e729083681f30494ae2615ae14aac3bc807a0c436a88` | | |
| `RoleGranted` | `(role: bytes32, grantee: address)` | `0x2ae6a113c0ed5b78a53413ffbb7679881f11145ccfba4fb92e863dfcd5a1d2f3` | | |
| `RoleRevoked` | `(role: bytes32, revokee: address)` | `0x155aaafb6329a2098580462df33ec4b7441b19729b9601c5fc17ae1cf99a8a52` | | |
| `SocketFeeManagerUpdated` | `(oldSocketFeeManager: address, newSocketFeeManager: address)` | `0xdcb02e10d5220346a4638aa2826eaab1897306623bc40a427049e4ebd12255b4` | | |
| `SwitchboardAdded` | `(switchboard: address)` | `0x1595852923edfbbf906f09fc8523e4cfb022a194773c4d1509446b614146ee88` | | |
| `SwitchboardDisabled` | `(switchboard: address)` | `0x1b4ee41596b4e754e5665f01ed6122b356f7b36ea0a02030804fac7fa0fdddfc` | | |
| `SwitchboardEnabled` | `(switchboard: address)` | `0x6909a9974e3eec619bc479ba882d30a5ef1219b72ab1ce6a354516e91be317b8` | | |
## SocketBatcher | |
## IFastSwitchboard | |
| Event | Arguments | Topic | | |
| ----- | --------- | ----- | | |
| *None* (contract emits no events) | | | | |
## SocketBatcher |
🧰 Tools
🪛 LanguageTool
[grammar] ~39-~39: Use correct spacing
Context: ...f12463c30019b4ed354ba8bc7bc26f28` | ## IFastSwitchboard | Event | Arguments | Topic | | ----- | ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~42-~42: Use hyphens correctly
Context: ...guments | Topic | | ----- | --------- | ----- | ## SocketBatcher | Event ...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
🤖 Prompt for AI Agents
In EventTopics.md around lines 39 to 44, the IFastSwitchboard section contains
only a table header without any event entries, which may confuse readers. Either
add the relevant events such as 'Attested' with their arguments and topics to
complete the table or remove the entire IFastSwitchboard section until it can be
properly implemented to avoid giving the impression of incomplete documentation.
| `RequestSubmitted` | `(hasWrite: bool, requestCount: uint40, totalEstimatedWatcherFees: uint256, requestParams: tuple, payloadParamsArray: tuple[])` | `0xb730ca5523e3f80e88b4bb71e1e78d447553069cd9a7143bb0032b957135b530` | | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Table row exceeds typical width
RequestSubmitted
has a very wide arguments cell. For readability consider wrapping tuple fields onto multiple lines or using Markdown <br>
.
🤖 Prompt for AI Agents
In EventTopics.md around lines 217 to 218, the table row for `RequestSubmitted`
has an arguments cell that is too wide, reducing readability. To fix this, break
the tuple fields into multiple lines or insert Markdown line breaks (`<br>`)
within the cell to wrap the content and improve visual clarity.
## IMessageSwitchboard | ||
|
||
| Event | Arguments | Topic | | ||
| ----- | --------- | ----- | | ||
| `AppGatewayCallFailed` | `(triggerId: bytes32)` | `0xcaf8475fdade8465ea31672463949e6cf1797fdcdd11eeddbbaf857e1e5907b7` | | ||
| `CalledAppGateway` | `(triggerId: bytes32)` | `0xf659ffb3875368f54fb4ab8f5412ac4518af79701a48076f7a58d4448e4bdd0b` | | ||
| `Initialized` | `(version: uint64)` | `0xc7f505b2f371ae2175ee4913f4499e1f2633a7b5936321eed1cdaeb6115181d2` | | ||
| `OwnershipHandoverCanceled` | `(pendingOwner: address)` | `0xfa7b8eab7da67f412cc9575ed43464468f9bfbae89d1675917346ca6d8fe3c92` | | ||
| `OwnershipHandoverRequested` | `(pendingOwner: address)` | `0xdbf36a107da19e49527a7176a1babf963b4b0ff8cde35ee35d6cd8f1f9ac7e1d` | | ||
| `OwnershipTransferred` | `(oldOwner: address, newOwner: address)` | `0x8be0079c531659141344cd1fd0a4f28419497f9722a3daafe3b4186f6b6457e0` | | ||
| `TriggerFailed` | `(triggerId: bytes32)` | `0x4386783bb0f7cad4ba12f033dbec03dc3441e7757a122f3097a7a4d945c98040` | | ||
| `TriggerFeesSet` | `(triggerFees: uint256)` | `0x7df3967b7c8727af5ac0ee9825d88aafeb899d769bc428b91f8967fa0b623084` | | ||
| `TriggerSucceeded` | `(triggerId: bytes32)` | `0x92d20fbcbf31370b8218e10ed00c5aad0e689022da30a08905ba5ced053219eb` | | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Another empty section
## IMessageSwitchboard
is empty. Populate or remove as above to keep docs tidy.
🧰 Tools
🪛 LanguageTool
[grammar] ~233-~233: Use correct spacing
Context: ...0e689022da30a08905ba5ced053219eb` | ## IMessageSwitchboard | Event | Arguments | Topic | | ----- | ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~236-~236: Use hyphens correctly
Context: ...guments | Topic | | ----- | --------- | ----- | ## ICCTPSwitchboard | Event | Arguments |...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
🤖 Prompt for AI Agents
In EventTopics.md around lines 233 to 237, the section titled ##
IMessageSwitchboard is empty. To fix this, either add the relevant event,
arguments, and topic details for IMessageSwitchboard in the table format used
above or remove the entire section to maintain clean and tidy documentation.
| `setGasLimitBuffer` | `0x1f0cfa78` | | ||
| `setMaxCopyBytes` | `0x4fc7d6e9` | | ||
| `setSocketFeeManager` | `0x25bd97e5` | | ||
| `simulate` | `0x91bf8275` | | ||
| `socketFeeManager` | `0xde5b8838` | | ||
| `switchboardAddresses` | `0x9cf0af93` | | ||
| `switchboardIdCounter` | `0x5f850dfd` | | ||
| `switchboardIds` | `0x91db23d3` | | ||
| `transferOwnership` | `0xf2fde38b` | | ||
| `triggerAppGateway` | `0x29e654e1` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Document view/pure state mutability
For setGasLimitBuffer
(mutative) vs gasLimitBuffer
(view) the names are clear, but the doc could help newcomers by suffixing (view)
/ (payable)
where relevant as done in some public ABI references.
🤖 Prompt for AI Agents
In FunctionSignatures.md around lines 81 to 90, the function signatures list
lacks explicit state mutability annotations like (view) or (payable) that
clarify whether functions are read-only or mutative. Update the documentation by
adding appropriate suffixes such as (view) for read-only functions and (payable)
for payable functions next to each function name to improve clarity for
newcomers.
## MessageSwitchboard | ||
|
||
| Function | Signature | | ||
| ---------------------------- | ------------ | | ||
| `allowPayload` | `0x31c23f66` | | ||
| `attest` | `0x4cc621d2` | | ||
| `cancelOwnershipHandover` | `0x54d1f13d` | | ||
| `chainSlug` | `0xb349ba65` | | ||
| `completeOwnershipHandover` | `0xf04e283e` | | ||
| `getSwitchboardFees` | `0x99de43bf` | | ||
| `getTransmitter` | `0x73e7d880` | | ||
| `grantRole` | `0x2f2ff15d` | | ||
| `hasRole` | `0x91d14854` | | ||
| `isAttested` | `0xc13c2396` | | ||
| `owner` | `0x8da5cb5b` | | ||
| `ownershipHandoverExpiresAt` | `0xfee81cf4` | | ||
| `payloadCounter` | `0x550ce1d5` | | ||
| `processTrigger` | `0x7f3352bc` | | ||
| `registerSibling` | `0x4f58b88c` | | ||
| `registerSwitchboard` | `0x74f5b1fc` | | ||
| `renounceOwnership` | `0x715018a6` | | ||
| `requestOwnershipHandover` | `0x25692962` | | ||
| `rescueFunds` | `0x6ccae054` | | ||
| `revokeRole` | `0xd547741f` | | ||
| `setSiblingConfig` | `0xacfbfc03` | | ||
| `setSwitchboardFees` | `0x476bfc49` | | ||
| `siblingPlugs` | `0x25fc3bcc` | | ||
| `siblingSockets` | `0xce3483a5` | | ||
| `siblingSwitchboards` | `0x51de49c9` | | ||
| `socket__` | `0xc6a261d2` | | ||
| `switchboardFees` | `0xb2a71ee7` | | ||
| `switchboardId` | `0xd3be4120` | | ||
| `transferOwnership` | `0xf2fde38b` | | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Large new surface area – recommend automated API export
MessageSwitchboard
adds >20 externals. To minimise doc drift, consider generating this markdown from solidity-abi json via a CI script rather than manual edits.
🧰 Tools
🪛 LanguageTool
[grammar] ~530-~530: There might be a mistake here.
Context: ...wnership |
0xf2fde38b` | ## MessageSwitchboard | Function | Signa...
(QB_NEW_EN_OTHER)
🤖 Prompt for AI Agents
In FunctionSignatures.md around lines 530 to 563, the MessageSwitchboard section
manually lists over 20 function signatures, which risks documentation drift. To
fix this, implement a CI script that automatically generates this markdown table
from the solidity-abi JSON output. This script should parse the ABI, extract
function names and selectors, and update the markdown file during the build or
deployment process to keep the documentation in sync with the codebase.
"0", | ||
"setSwitchboard", | ||
[chain, FAST_SWITCHBOARD_TYPE, constants.AddressZero], | ||
[chain, FAST_SWITCHBOARD_TYPE, 0], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify parameter type consistency.
The change from constants.AddressZero
to string "0"
(line 71) and numeric 0
(line 73) creates inconsistency in zero value representation. Ensure that the updateContractSettings
function and the underlying contract expect these specific formats.
🏁 Script executed:
#!/bin/bash
# Description: Check the updateContractSettings function signature and contract expectations
# Expected: Verification of parameter types and zero value handling
# Search for updateContractSettings function definition
ast-grep --pattern 'function updateContractSettings($$$) {
$$$
}'
# Search for setSwitchboard function in contracts to understand expected parameter types
ast-grep --pattern 'function setSwitchboard($$$) {
$$$
}'
Length of output: 131
🏁 Script executed:
#!/bin/bash
set -eux
# Locate the disable-sb.ts script
SCRIPT_PATH=$(fd disable-sb.ts)
echo "Found disable-sb.ts at: $SCRIPT_PATH"
# Show the context around the setSwitchboard call in disable-sb.ts
rg -n -C3 '"setSwitchboard"' "$SCRIPT_PATH"
# Search for updateContractSettings usage or definition in TypeScript files
rg -n "updateContractSettings" -g "*.ts"
# Search for setSwitchboard usage or definition in TypeScript files
rg -n "setSwitchboard" -g "*.ts"
# Search for function definitions in Solidity contracts
rg -n "function updateContractSettings" -g "*.sol"
rg -n "function setSwitchboard" -g "*.sol"
Length of output: 2257
🏁 Script executed:
#!/bin/bash
set -eux
# Show the signature and parameter list for updateContractSettings in deployUtils.ts
rg -n -C10 "export const updateContractSettings" -g "hardhat-scripts/utils/deployUtils.ts"
# Show the implementation where the method name and args are passed through to the contract
rg -n -C10 "contract\..*\[methodName\]" -g "hardhat-scripts/utils/deployUtils.ts" || true
Length of output: 1588
Use AddressZero
for zero‐address parameters
To ensure the zero value matches the contract’s address
type and to keep both the required value and setter argument consistent, replace the literal "0"
and numeric 0
with ethers.constants.AddressZero
:
• In hardhat-scripts/admin/disable-sb.ts
around line 71:
- "switchboards",
- [chain, FAST_SWITCHBOARD_TYPE],
- "0",
- "setSwitchboard",
- [chain, FAST_SWITCHBOARD_TYPE, 0],
+ "switchboards",
+ [chain, FAST_SWITCHBOARD_TYPE],
+ constants.AddressZero,
+ "setSwitchboard",
+ [chain, FAST_SWITCHBOARD_TYPE, constants.AddressZero],
• Add at the top of the file:
import { constants } from "ethers";
This ensures both the requiredValue
and the value passed into setSwitchboard
are valid Ethereum addresses.
🤖 Prompt for AI Agents
In hardhat-scripts/admin/disable-sb.ts around lines 71 to 73, replace the
literal string "0" and numeric 0 used as zero-address parameters with
ethers.constants.AddressZero to ensure they are valid Ethereum addresses. To do
this, import { constants } from "ethers" at the top of the file, then use
constants.AddressZero in place of "0" and 0 in the relevant code lines for
requiredValue and the argument passed to setSwitchboard.
const appGatewayId = BYTES32_ZERO; | ||
const switchboardId = "0"; | ||
const plugContract = Contracts.FeesPlug; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Consider using consistent types for switchboardId.
The switchboardId
is set to string "0"
here, but based on the codebase changes, switchboard IDs are typically uint64
. Consider using a numeric 0
or ensuring type consistency across the codebase.
- const switchboardId = "0";
+ const switchboardId = 0;
📝 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.
const appGatewayId = BYTES32_ZERO; | |
const switchboardId = "0"; | |
const plugContract = Contracts.FeesPlug; | |
const appGatewayId = BYTES32_ZERO; | |
const switchboardId = 0; | |
const plugContract = Contracts.FeesPlug; |
🤖 Prompt for AI Agents
In hardhat-scripts/admin/disconnect.ts around lines 118 to 120, the variable
switchboardId is assigned the string "0" but should be a numeric value to
maintain type consistency with uint64 IDs used elsewhere. Change the assignment
of switchboardId from the string "0" to the numeric 0 to align with the expected
type and ensure consistent handling across the codebase.
export const EVM_CHAIN_ID_MAP: Record<DeploymentMode, number> = { | ||
[DeploymentMode.LOCAL]: 7625382, | ||
[DeploymentMode.DEV]: 7625382, | ||
[DeploymentMode.DEV]: 14323, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify the chain ID change for DEV environment
The chain ID for DEV deployment has changed from 7625382
to 14323
. This is a significant change that affects deployment targeting and contract verification.
Ensure this chain ID change is coordinated with:
- RPC endpoint configurations
- Contract verification services
- Integration testing environments
- Deployment address mappings
🏁 Script executed:
#!/bin/bash
# Verify if the new chain ID 14323 is properly configured across deployment files
echo "Checking for chain ID 14323 in deployment configurations..."
fd -e json . deployments/ --exec grep -l "14323" {}
echo -e "\nChecking for old chain ID 7625382 references..."
rg "7625382" --type json deployments/
Length of output: 411
Fix remaining legacy chain ID in verification configs
The new DEV chain ID (14323) is present in deployments/dev_addresses.json
, but deployments/dev_verification.json
still references the old ID (7625382
). Update this file so that your verification scripts target the correct chain.
• File to update:
- deployments/dev_verification.json
• Change needed (example snippet):
- "7625382": [
+ "14323": [
// existing verification entries…
]
After updating, rerun your verification workflow to confirm all references to 7625382
are removed and replaced with 14323
.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In hardhat-scripts/config/config.ts at line 90, the DEV chain ID is updated to
14323, but the file deployments/dev_verification.json still uses the old chain
ID 7625382. Open deployments/dev_verification.json and replace all instances of
7625382 with 14323 to ensure consistency. After making this change, rerun the
verification scripts to confirm the update is effective.
); | ||
export const BYTES32_ZERO = ethers.utils.hexZeroPad(constants.AddressZero, 32); | ||
|
||
export const MSG_SB_FEES = "100000000"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Document the fee structure for MSG_SB_FEES
The new constant MSG_SB_FEES = "100000000"
introduces a fee value for message switchboard operations. Consider adding a comment to document the fee structure or units (e.g., wei, gwei).
+// Message Switchboard fees in wei
export const MSG_SB_FEES = "100000000";
📝 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 MSG_SB_FEES = "100000000"; | |
// Message Switchboard fees in wei | |
export const MSG_SB_FEES = "100000000"; |
🤖 Prompt for AI Agents
In hardhat-scripts/constants/constants.ts at line 14, the constant MSG_SB_FEES
is defined without any comment explaining the fee structure or units. Add a
clear comment above this line to document what MSG_SB_FEES represents and
specify the units of the fee value (such as wei or gwei) to improve code clarity
and maintainability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
EventTopics.md (3)
22-32
: Still missingindexed
flag forplug
argument
Same point raised in a previous review; the newPlugConnected
/PlugDisconnected
rows do not annotateplug
asindexed
.
39-44
: EmptyIFastSwitchboard
section persistsTable header is present with no rows. Populate or remove to avoid implying incomplete docs.
233-237
: EmptyIMessageSwitchboard
section persistsSame issue as above; either list events or drop the section for now.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
Errors.md
(3 hunks)EventTopics.md
(6 hunks)FunctionSignatures.md
(7 hunks)
🧰 Additional context used
🪛 LanguageTool
Errors.md
[grammar] ~72-~72: Use correct spacing
Context: ...imulationFailed()|
0x2fbab3ac` | ## protocol/base/MessagePlugBase.sol | Error | Signature | | --...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~76-~76: There might be a mistake here.
Context: ...- | ------------ | | NotSupported()
| 0xa0387940
| ## protocol/switchboard/CCTPSwitchboard.so...
(QB_NEW_EN_OTHER)
[grammar] ~96-~96: Use correct spacing
Context: ...WatcherNotFound()|
0xa278e4ad` | ## protocol/switchboard/MessageSwitchboard.sol | Error | Signat...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
EventTopics.md
[grammar] ~39-~39: Use correct spacing
Context: ...f12463c30019b4ed354ba8bc7bc26f28` | ## IFastSwitchboard | Event | Arguments | Topic | | ----- | ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~233-~233: Use correct spacing
Context: ...0e689022da30a08905ba5ced053219eb` | ## IMessageSwitchboard | Event | Arguments | Topic | | ----- | ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~236-~236: Use hyphens correctly
Context: ...guments | Topic | | ----- | --------- | ----- | ## ICCTPSwitchboard | Event | Arguments |...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~265-~265: There might be a mistake here.
Context: ...441b19729b9601c5fc17ae1cf99a8a52` | ## MessageSwitchboard | Event | Argum...
(QB_NEW_EN_OTHER)
FunctionSignatures.md
[grammar] ~504-~504: Use correct spacing
Context: ...tions |
0x6f30514c` | ## FastSwitchboard | Function | Signatu...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~529-~529: There might be a mistake here.
Context: ...wnership |
0xf2fde38b` | ## MessageSwitchboard | Function | Signa...
(QB_NEW_EN_OTHER)
[grammar] ~563-~563: Use correct spacing
Context: ...wnership |
0xf2fde38b` | ## ReadPrecompile | Function | Signa...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~576-~576: There might be a mistake here.
Context: ...f| |
watcher__ |
0x300bb063` | ## SchedulePrecompile | Function ...
(QB_NEW_EN_OTHER)
🔇 Additional comments (3)
Errors.md (1)
53-57
: Confirm removal and new signature
SwitchboardExistsOrDisabled()
is no longer listed whilePlugNotConnected()
was added.
Please double-check that the contract actually removed the former and thatPlugNotConnected()
’s 4-byte selector (0x411d0255
) matches the compiled ABI; docs & code must stay in lock-step.EventTopics.md (1)
265-279
: Argument types may be out of sync with ID-based refactorIn
SiblingConfigSet
the last column isswitchboard: bytes32
, yet the codebase migrated touint64 switchboardId
.
SiblingRegistered
still exposesplugAddress: address
while most places now usebytes32 plug
.Please verify the actual event definitions in
MessageSwitchboard.sol
; update the docs or contracts so the types are consistent.FunctionSignatures.md (1)
531-535
: Inconsistent selector forattest
MessageSwitchboard.attest
shows selector0x4cc621d2
whereas bothFastSwitchboard
andCCTPSwitchboard
use0x63671b60
.
If the parameter list is identical this divergence is unexpected and signals drift between docs and ABI.
## protocol/switchboard/MessageSwitchboard.sol | ||
|
||
| Error | Signature | | ||
| ----------------------------- | ------------ | | ||
| `AlreadyAttested()` | `0x35d90805` | | ||
| `WatcherNotFound()` | `0xa278e4ad` | | ||
| `SiblingNotFound()` | `0xb3b47851` | | ||
| `InvalidTargetVerification()` | `0xe9377a19` | | ||
| `InvalidMsgValue()` | `0x1841b4e1` | | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Duplicate error codes across switchboards
AlreadyAttested()
(0x35d90805
) and WatcherNotFound()
(0xa278e4ad
) now appear in both FastSwitchboard and MessageSwitchboard tables.
Unless the underlying libraries purposely define separate copies, consider documenting them once (e.g. under a shared “switchboard-common” section) to avoid reader confusion and future drift.
🧰 Tools
🪛 LanguageTool
[grammar] ~96-~96: Use correct spacing
Context: ...WatcherNotFound()|
0xa278e4ad` | ## protocol/switchboard/MessageSwitchboard.sol | Error | Signat...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🤖 Prompt for AI Agents
In Errors.md around lines 96 to 105, the error codes AlreadyAttested() and
WatcherNotFound() are duplicated in both FastSwitchboard and MessageSwitchboard
sections. To fix this, consolidate these common errors into a single shared
section named "switchboard-common" and reference this section from both
switchboard tables to avoid duplication and potential confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (6)
Errors.md (1)
90-104
: Duplicate error codes were not deduped
AlreadyAttested()
andWatcherNotFound()
are still listed under both FastSwitchboard and MessageSwitchboard despite the earlier feedback.
Please consolidate common switchboard errors into a shared section to avoid future drift.EventTopics.md (3)
30-32
: Consider markingplug
asindexed
for consistencyNearly every consumer filters by
plug
– documenting it asindexed address
(like the other docs) avoids confusion.
39-43
: EmptyIFastSwitchboard
section still presentLeaving a header with no rows looks unfinished. Either populate with events or drop the section until implemented.
233-237
:IMessageSwitchboard
section is still emptySame issue as above – please remove or fill.
FunctionSignatures.md (2)
55-93
: State-mutability suffixes still missingPrevious review asked to annotate
(view)
/(payable)
for read-only vs mutative functions (e.g.gasLimitBuffer
vssetGasLimitBuffer
). Docs remain unchanged – adding the suffixes greatly helps API consumers.
530-563
: AutomateMessageSwitchboard
ABI table to avoid driftManually listing >30 selectors is error-prone. Recommend generating this section from the compiled ABI in CI (see previous suggestion).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
Errors.md
(3 hunks)EventTopics.md
(2 hunks)FunctionSignatures.md
(3 hunks)contracts/evmx/base/AppGatewayBase.sol
(1 hunks)contracts/evmx/fees/Credit.sol
(1 hunks)contracts/evmx/watcher/precompiles/WritePrecompile.sol
(1 hunks)deployments/dev_verification.json
(1 hunks)src/signer.ts
(2 hunks)
🧰 Additional context used
🪛 LanguageTool
EventTopics.md
[grammar] ~20-~20: Use correct spacing
Context: ...441b19729b9601c5fc17ae1cf99a8a52` | ## Socket | Event | Argumen...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~39-~39: Use correct spacing
Context: ...f12463c30019b4ed354ba8bc7bc26f28` | ## IFastSwitchboard | Event | Arguments | Topic | | ----- | ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~42-~42: Use hyphens correctly
Context: ...guments | Topic | | ----- | --------- | ----- | ## SocketBatcher | Event ...
(QB_NEW_EN_OTHER_ERROR_IDS_29)
[grammar] ~52-~52: Use correct spacing
Context: ...19497f9722a3daafe3b4186f6b6457e0` | ## SocketFeeManager | Event | Argumen...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~63-~63: Use correct spacing
Context: ...04ad46e9d97c543ef9a8dfea3e407053` | ## FeesManager | Event | Argume...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~82-~82: Use correct spacing
Context: ...4821087a31cb43a7851dc6ff743d9be7` | ## FeesPool | Event | Argumen...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~94-~94: Use correct spacing
Context: ...441b19729b9601c5fc17ae1cf99a8a52` | ## AddressResolver | Event | Argum...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~109-~109: Use correct spacing
Context: ...fa43a7a14a1529204ced5b6350052bb0` | ## AsyncDeployer | Event | Argumen...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~121-~121: Use correct spacing
Context: ...19497f9722a3daafe3b4186f6b6457e0` | ## AsyncPromise | Event | Arguments | ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~125-~125: There might be a mistake here.
Context: ...| Initialized
| (version: uint64)
| 0xc7f505b2f371ae2175ee4913f4499e1f2633a7b5936321eed1cdaeb6115181d2
| ## DeployForwarder | Event ...
(QB_NEW_EN_OTHER)
[grammar] ~136-~136: Use correct spacing
Context: ...19497f9722a3daafe3b4186f6b6457e0` | ## Forwarder | Event | Arguments | ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~140-~140: There might be a mistake here.
Context: ...| Initialized
| (version: uint64)
| 0xc7f505b2f371ae2175ee4913f4499e1f2633a7b5936321eed1cdaeb6115181d2
| ## ProxyFactory | Event | Argume...
(QB_NEW_EN_OTHER)
[grammar] ~150-~150: Use correct spacing
Context: ...514c6b50e1e5ad30040a4df2b12791c7` | ## TestUSDC | Event | Arguments ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~155-~155: There might be a mistake here.
Context: ... to: address, amount: uint256) |
0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef` | ## ContractFactoryPlug | Event ...
(QB_NEW_EN_OTHER)
[grammar] ~169-~169: Use correct spacing
Context: ...441b19729b9601c5fc17ae1cf99a8a52` | ## FeesPlug | Event | Argumen...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~184-~184: Use correct spacing
Context: ...3f8fa5846e26231d30ae79a417d3d262` | ## Configurations | Event | Argumen...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~197-~197: Use correct spacing
Context: ...8538b9eb87aa1db45d412e7165d6850a` | ## PromiseResolver | Event | Arguments ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~205-~205: There might be a mistake here.
Context: ...6ad3e6001ea6fb7d12ea0241ba13c4ba` | ## RequestHandler | Event | Argum...
(QB_NEW_EN_OTHER)
[grammar] ~219-~219: Use correct spacing
Context: ...7553069cd9a7143bb0032b957135b530` | ## Watcher | Event | Argumen...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~233-~233: Use correct spacing
Context: ...0e689022da30a08905ba5ced053219eb` | ## IMessageSwitchboard | Event | Arguments | Topic | | ----- | ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~265-~265: There might be a mistake here.
Context: ...441b19729b9601c5fc17ae1cf99a8a52` | ## MessageSwitchboard | Event | Argum...
(QB_NEW_EN_OTHER)
[grammar] ~280-~280: Use correct spacing
Context: ...3b52c84111559988d28d6872185caa32` | ## ReadPrecompile | Event | Arguments ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~288-~288: Use correct spacing
Context: ...fcf5950ca8e559150e764eced73e794a` | ## SchedulePrecompile | Event | Argum...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~299-~299: Use correct spacing
Context: ...04e045f2ed9a4fa442665935cb73e26b` | ## WritePrecompile | Event | Argu...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
FunctionSignatures.md
[grammar] ~53-~53: Use correct spacing
Context: ...ds |
0x9133f232` | ## Socket | Function | Signatu...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~94-~94: Use correct spacing
Context: ... | 0x54fd4d50
| ## SocketFeeManager | Function | Signatu...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~114-~114: Use correct spacing
Context: ...wnership |
0xf2fde38b` | ## FeesManager | Function | Sig...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~159-~159: Use correct spacing
Context: ... | 0x023276f0
| ## FeesPool | Function | Signatu...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~174-~174: There might be a mistake here.
Context: ...38b| |
withdraw |
0xf3fef3a3` | ## AddressResolver | Function ...
(QB_NEW_EN_OTHER)
[grammar] ~202-~202: Use correct spacing
Context: ... |
0x300bb063` | ## AsyncDeployer | Function | Sign...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~232-~232: Use correct spacing
Context: ... | 0x300bb063
| ## AsyncPromise | Function | Signature | |...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~254-~254: Use correct spacing
Context: ...watcher__ |
0x300bb063` | ## DeployForwarder | Function | Signatu...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~276-~276: Use correct spacing
Context: ... |
0x300bb063` | ## Forwarder | Function | Signature | |...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~290-~290: There might be a mistake here.
Context: ... 0x6ccae054
| | watcher__
| 0x300bb063
| ## ProxyFactory | Function ...
(QB_NEW_EN_OTHER)
[grammar] ~305-~305: There might be a mistake here.
Context: ...c4| |
upgradeAndCall |
0x9623609d` | ## TestUSDC | Function | Signat...
(QB_NEW_EN_OTHER)
[grammar] ~324-~324: There might be a mistake here.
Context: ...| 0xa9059cbb
| | transferFrom
| 0x23b872dd
| ## ContractFactoryPlug | Function ...
(QB_NEW_EN_OTHER)
[grammar] ~350-~350: Use correct spacing
Context: ...wnership |
0xf2fde38b` | ## Configurations | Function | Signatu...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~374-~374: Use correct spacing
Context: ... |
0x300bb063` | ## PromiseResolver | Function | Signature | | -...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~381-~381: There might be a problem here.
Context: ... | 0xbf8484b8
| | watcher__
| 0x300bb063
| ## RequestHandler | Function | Sig...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~420-~420: Use correct spacing
Context: ... | 0x300bb063
| ## Watcher | Function | Signatu...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~505-~505: Use correct spacing
Context: ...tions |
0x6f30514c` | ## FastSwitchboard | Function | Signatu...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~530-~530: There might be a mistake here.
Context: ...wnership |
0xf2fde38b` | ## MessageSwitchboard | Function | Signa...
(QB_NEW_EN_OTHER)
[grammar] ~564-~564: Use correct spacing
Context: ...wnership |
0xf2fde38b` | ## ReadPrecompile | Function | Signa...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~577-~577: There might be a mistake here.
Context: ...f| |
watcher__ |
0x300bb063` | ## SchedulePrecompile | Function ...
(QB_NEW_EN_OTHER)
[grammar] ~598-~598: Use correct spacing
Context: ... | 0x300bb063
| ## WritePrecompile | Function | Signa...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🔇 Additional comments (5)
contracts/evmx/base/AppGatewayBase.sol (1)
175-176
: No functional change – readability improvedSplitting the
IForwarder(...).getOnChainAddress()
call across two lines slightly boosts readability without impacting execution. Looks good as-is.contracts/evmx/fees/Credit.sol (1)
73-73
: Formatting-only adjustment acknowledgedThe single-line inheritance list is stylistic; no logic, storage-layout, or initializer-order impact detected.
src/signer.ts (1)
10-10
: LGTM! Improved nonce uniqueness.The change from
Date.now()
togetNonce()
enhances nonce uniqueness, reducing the risk of collisions when multiple signatures are generated in rapid succession.deployments/dev_verification.json (1)
23-38
: Parameter array now mixes 1 e11 & 1 e13 – confirm the units/order
SchedulePrecompile
’s constructor args changed to[ watcher, 86400, 0x02540be400, 0xe8d4a51000, 3600 ]
That puts the smaller
0x02540be400
(1 × 10¹¹) before the larger0xe8d4a51000
(1 × 10¹³).
If these are “minFee” / “maxFee” the order looks swapped compared to previous deployments. Double-check before redeploying ‑ an inverted range will brick fee calculations on-chain.contracts/evmx/watcher/precompiles/WritePrecompile.sol (1)
163-166
: Approve type change: switchboards returns uint64
- contracts/evmx/interfaces/IConfigurations.sol shows
function switchboards(uint32 chainSlug_, bytes32 sbType_) external view returns (uint64);
- The
abi.decode(..., uint64)
in WritePrecompile.sol now correctly matches the encoding.- The decoded uint64 is unused here but kept for interface consistency.
No further changes needed.
"0x8DA71DE564096Ea4A43E794E1dEcA8F1692B69D7", | ||
"FeesManager", | ||
"contracts/evmx/fees/FeesManager.sol", | ||
[] | ||
], | ||
[ | ||
"0x872bb254118a2210e3C491918133F2ab4D7Bc362", | ||
"Watcher", | ||
"contracts/evmx/watcher/Watcher.sol", | ||
[] | ||
], | ||
[ | ||
"0x7D6F2A4aDf7e5Cfcf9627CC7FCA1d39fD19C07fc", | ||
"SchedulePrecompile", | ||
"contracts/evmx/watcher/precompiles/SchedulePrecompile.sol", | ||
[ | ||
"0xD5b30DC89D96ee7303Dc2726491996B46089F693", | ||
86400, | ||
{ | ||
"type": "BigNumber", | ||
"hex": "0xe8d4a51000" | ||
}, | ||
{ | ||
"type": "BigNumber", | ||
"hex": "0xe8d4a51000" | ||
}, | ||
300 | ||
] | ||
], | ||
[ | ||
"0x254Dc9e0623426A79F02D2001E367cd32B50aaaA", | ||
"ReadPrecompile", | ||
"contracts/evmx/watcher/precompiles/ReadPrecompile.sol", | ||
[ | ||
"0xD5b30DC89D96ee7303Dc2726491996B46089F693", | ||
{ | ||
"type": "BigNumber", | ||
"hex": "0xe8d4a51000" | ||
}, | ||
300 | ||
] | ||
], | ||
[ | ||
"0xD3aEb53da0a72788C16eAf5a23a5aBae6708C073", | ||
"WritePrecompile", | ||
"contracts/evmx/watcher/precompiles/WritePrecompile.sol", | ||
[] | ||
], | ||
[ | ||
"0xcfFda1dF8668266E6A77809EcA9CCA8A632ecaF3", | ||
"PromiseResolver", | ||
"contracts/evmx/watcher/PromiseResolver.sol", | ||
["0xD5b30DC89D96ee7303Dc2726491996B46089F693"] | ||
], | ||
[ | ||
"0x3d9578B252ed1F5A66348Cc40E482dacc32Ae790", | ||
"RequestHandler", | ||
"contracts/evmx/watcher/RequestHandler.sol", | ||
[] | ||
], | ||
[ | ||
"0x0d2646fC08af29A7799Af435c5ABBA1b020C4dC7", | ||
"Configurations", | ||
"contracts/evmx/watcher/Configurations.sol", | ||
[] | ||
], | ||
[ | ||
"0xCe95fca954a0BF43c299c79d5152f2c164C02b7A", | ||
"DeployForwarder", | ||
"contracts/evmx/helpers/DeployForwarder.sol", | ||
[] | ||
], | ||
[ | ||
"0x42109F6212765ABeb589f9b2c14Bee4b8DB3e638", | ||
"AuctionManager", | ||
"contracts/evmx/AuctionManager.sol", | ||
[] | ||
], | ||
[ | ||
"0xFa9B9271A4153eEABa76ae10bfc4F128651c25D7", | ||
"Watcher", | ||
"contracts/evmx/watcher/Watcher.sol", | ||
[] | ||
], | ||
[ | ||
"0x80CFbD3B6134Fb2D2B7d21FC132a9F7c115e7B72", | ||
"AsyncDeployer", | ||
"contracts/evmx/helpers/AsyncDeployer.sol", | ||
[] | ||
"0x13A3018920c7b56B20dd34E29C298121025E6de4", | ||
"FeesPool", | ||
"contracts/evmx/fees/FeesPool.sol", | ||
["0xb62505feacC486e809392c65614Ce4d7b051923b"] | ||
], | ||
[ | ||
"0x6975302A1B7aF61d89F85a13855B66D15221Cf8D", | ||
"0xC3f93140EF2f57a87FDdCe7DdADB544873b67C7D", | ||
"FeesManager", | ||
"contracts/evmx/fees/FeesManager.sol", | ||
[] | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple FeesManager
entries for the same chain cause ambiguity
FeesManager
appears twice (lines 5-9 and 17-21) with different addresses for the same network ID.
Down-stream tooling that consumes this file (e.g. hardhat-deploy
’s verify
) will not know which address to verify and may silently pick the first match.
Either delete the stale record or add an explicit tag
/alias
field so each instance can be referenced unambiguously.
🤖 Prompt for AI Agents
In deployments/dev_verification.json around lines 5 to 21, there are multiple
entries for "FeesManager" with different addresses under the same network ID,
causing ambiguity for downstream tools. To fix this, either remove the outdated
or duplicate "FeesManager" entry or add a unique "tag" or "alias" field to each
entry so they can be referenced unambiguously by verification tools.
"0xBD3d78023FAf3590fFcF65013b5cC058Efeb13C8", | ||
"FeesManager", | ||
"contracts/evmx/fees/FeesManager.sol", | ||
[] | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Third FeesManager
entry likely a leftover
A third FeesManager
entry appears here with no constructor args. If this address is from an earlier dry-run, please remove it to keep the artefacts deterministic.
🤖 Prompt for AI Agents
In deployments/dev_verification.json around lines 102 to 106, there is a
redundant third entry for "FeesManager" with no constructor arguments, likely
leftover from a previous dry-run. Remove this extra entry entirely to maintain
deterministic deployment artifacts and avoid confusion.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Risk of precision loss and nonce collisions with current approach.
The string concatenation approach has several issues:
- Precision loss: Concatenating timestamp (13 digits) + random (up to 3 digits) creates numbers that may exceed JavaScript's
MAX_SAFE_INTEGER
, leading to precision loss. - Limited entropy: Random range of 0-999 provides only 1000 possible values per millisecond.
- Potential overflow: Large concatenated numbers may not be handled safely.
Consider these safer alternatives:
Option 1: Bitwise combination (recommended)
export const getNonce = () => {
- const timestamp = Date.now();
- const random = Math.floor(Math.random() * 1000);
- const nonce = Number(String(timestamp) + String(random));
- return nonce;
+ const timestamp = Date.now();
+ const random = Math.floor(Math.random() * 0x100000); // 20-bit random
+ return (timestamp << 20) | random; // Combine via bitwise OR
};
Option 2: Simple addition with larger random range
export const getNonce = () => {
const timestamp = Date.now();
- const random = Math.floor(Math.random() * 1000);
- const nonce = Number(String(timestamp) + String(random));
- return nonce;
+ const randomOffset = Math.floor(Math.random() * 1000000);
+ return timestamp * 1000000 + randomOffset;
};
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/signer.ts around lines 27 to 32, the current nonce generation by
concatenating timestamp and random number risks precision loss and collisions
due to exceeding JavaScript's MAX_SAFE_INTEGER and limited randomness. To fix
this, replace the concatenation with a safer method such as combining timestamp
and random number using bitwise operations or by adding a larger random number
range to the timestamp, ensuring the result stays within safe integer limits and
increases entropy for uniqueness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
hardhat-scripts/deploy/3.configureChains.ts (1)
340-367
: Validate switchboard registration before returning IDThe function returns the switchboard ID without validating that registration was successful. Consider adding validation to ensure the ID is valid.
if (Number(switchboardId) == 0) { const registerTx = await switchboard.registerSwitchboard({ ...(await overrides(chain)), }); console.log(`Registering Switchboard ${sbAddress}: ${registerTx.hash}`); await registerTx.wait(); switchboardId = await switchboard.switchboardId(); console.log(`Switchboard ID: ${switchboardId}`); + + // Verify registration was successful + if (Number(switchboardId) == 0) { + throw new Error(`Failed to register switchboard at ${sbAddress}`); + } } return switchboardId;
♻️ Duplicate comments (2)
hardhat-scripts/config/config.ts (1)
90-91
: Critical: Verify chain ID changes across all deployment files.The chain ID changes (DEV: 7625382→14323, STAGE: 43→14323) are significant and require careful verification. A past review identified that verification files weren't updated for similar changes.
Ensure these chain ID changes are coordinated across:
- Deployment address mappings (deployments/dev_addresses.json, deployments/stage_addresses.json)
- Verification configurations (deployments/dev_verification.json, deployments/stage_verification.json)
- RPC endpoint configurations
- Integration testing environments
#!/bin/bash # Description: Verify chain ID consistency across deployment files echo "Checking for new chain ID 14323 in deployment files..." rg "14323" deployments/ --type json echo -e "\nChecking for old chain IDs in verification files..." rg "7625382|43" deployments/ --type json echo -e "\nListing all deployment and verification files to check manually..." fd "\.(json)$" deployments/deployments/dev_verification.json (1)
6-22
: Multiple FeesManager entries still presentThe issue with multiple FeesManager entries for chain 7625382 persists. There are three entries with different addresses, which will cause ambiguity for verification tools.
Also applies to: 103-107
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (9)
deployments/dev_addresses.json
(1 hunks)deployments/dev_verification.json
(1 hunks)foundry.toml
(1 hunks)hardhat-scripts/config/config.ts
(2 hunks)hardhat-scripts/constants/feeConstants.ts
(1 hunks)hardhat-scripts/deploy/3.configureChains.ts
(9 hunks)hardhat-scripts/s3Config/buildConfig.ts
(1 hunks)package.json
(2 hunks)src/types.ts
(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: sebastiantf
PR: SocketDotTech/socket-ll-contracts#277
File: src/bridges/gnosis-bridge-router/GnosisBridgeRouterImpl.sol:235-241
Timestamp: 2025-07-28T16:22:57.704Z
Learning: In the GnosisBridgeRouterImpl contract, the team chose to skip receiverAddress validation in the swapAndBridge function to save gas costs, consistent with their gas optimization priorities throughout the Socket bridge contracts.
package.json (1)
Learnt from: sebastiantf
PR: SocketDotTech/socket-ll-contracts#277
File: package.json:63-63
Timestamp: 2025-07-28T15:58:48.507Z
Learning: In package.json, sebastiantf prefers adding solhint-plugin-prettier as a dev dependency for IDE integration and warnings rather than incorporating solhint into the lint script for build-time execution.
src/types.ts (1)
Learnt from: sebastiantf
PR: SocketDotTech/socket-ll-contracts#277
File: src/bridges/gnosis-bridge-router/GnosisBridgeRouterImpl.sol:235-241
Timestamp: 2025-07-28T16:22:57.704Z
Learning: In the GnosisBridgeRouterImpl contract, the team chose to skip receiverAddress validation in the swapAndBridge function to save gas costs, consistent with their gas optimization priorities throughout the Socket bridge contracts.
hardhat-scripts/deploy/3.configureChains.ts (2)
Learnt from: sebastiantf
PR: SocketDotTech/socket-ll-contracts#277
File: src/bridges/gnosis-bridge-router/GnosisBridgeRouterImpl.sol:235-241
Timestamp: 2025-07-28T16:22:57.704Z
Learning: In the GnosisBridgeRouterImpl contract, the team chose to skip receiverAddress validation in the swapAndBridge function to save gas costs, consistent with their gas optimization priorities throughout the Socket bridge contracts.
Learnt from: sebastiantf
PR: SocketDotTech/socket-ll-contracts#277
File: src/bridges/gnosis-bridge-router/GnosisBridgeRouterImpl.sol:126-131
Timestamp: 2025-07-28T16:22:33.706Z
Learning: In the Socket bridge contracts, gas optimization is prioritized over additional validation checks. The team prefers to skip validation checks like require(msg.value == amount)
and require(receiverAddress != address(0))
in native token bridging functions to minimize gas costs for users, even if it means accepting some additional risk.
🧬 Code Graph Analysis (1)
hardhat-scripts/s3Config/buildConfig.ts (1)
hardhat-scripts/config/config.ts (1)
EVMX_CHAIN_ID
(100-100)
🔇 Additional comments (7)
package.json (2)
12-12
: LGTM! Version bump aligns with deployment changes.The version increment from 1.1.32 to 1.1.37 is consistent with the deployment script updates mentioned in the PR objectives.
47-47
: No known security vulnerabilities in Node.js “pg” v8.16.3
- We’ve reviewed Snyk, CVEDetails, npm advisories, and other public databases—there are no direct CVEs or security advisories affecting the “pg” driver at v8.16.3 as of July 28, 2025.
- Keep an eye on any future advisories (including transitive dependencies) by subscribing to official channels (npm, Snyk, Node.js security releases).
hardhat-scripts/constants/feeConstants.ts (1)
23-24
: Confirm unified FeesPool address for DEV & STAGEThe address
0x13A3018920c7b56B20dd34E29C298121025E6de4
is now used for both DEV and STAGE in:
- hardhat-scripts/constants/feeConstants.ts
- deployments/dev_verification.json & deployments/dev_addresses.json
- foundry.toml (labeled as
"FeesPool"
)Please verify:
- The FeesPool contract is indeed deployed at this address on both DEV and STAGE networks.
- If your deployment workflow maintains a
deployments/stage_addresses.json
(or similar), ensure the FeesPool entry there matches this address.foundry.toml (1)
13-36
: All address mappings verified and in syncVerified that every entry in the foundry.toml
[labels]
section appears in the deployment config files:
- AddressResolver & AddressResolverImpl found in deployments/dev_addresses.json
- FeesPool also present in deployments/dev_verification.json
No inconsistencies detected—ready to merge.hardhat-scripts/deploy/3.configureChains.ts (1)
218-269
: LGTM! Well-structured sibling configuration setupThe
setSiblingConfig
function properly handles the MessageSwitchboard sibling configuration with appropriate error handling and logging for missing remote configurations.deployments/dev_addresses.json (2)
2-26
: Missing switchboard-ID metadata for the new chain 14323Every other chain entry that participates in the new “switchboard-ID” architecture now contains:
•
CCTPSwitchboard
,FastSwitchboard
,MessageSwitchboard
addresses
• The corresponding*Id
keys (FastSwitchboardId
, etc.)
• ASwitchboardIdToAddressMap
sectionThe freshly-added 14323 block only lists the legacy contracts and omits all of the above.
If deployment on 14323 genuinely does not use any switchboards, add an explicit empty structure so downstream scripts do not crash on missing keys; otherwise populate the correct values."14323": { ... "WritePrecompileImpl": "0xc2Ca571f4d4C2008Da4Bd750BaD3d50A5705ffF8", + "FastSwitchboard": "", + "FastSwitchboardId": "", + "CCTPSwitchboard": "", + "CCTPSwitchboardId": "", + "MessageSwitchboard": "", + "MessageSwitchboardId": "", + "SwitchboardIdToAddressMap": {} },Please verify and update before this lands in
main
.
46-63
: Double-checkstartBlock
accuracy after redeploy
startBlock
for chain 11155420 is set to30946234
, while 421614 uses178158322
. Given the recent contract redeployments and ID migration, be sure these blocks correspond to the FIRST transaction that emitted the new switchboard events; indexing services rely on this value to avoid missing historical data.If uncertain, rerun the deployment script with
--print-start-block
and update accordingly.
"421614": { | ||
"CCTPSwitchboard": "0x74A4aa989515b088A1aC33C5D59897f69cA66B91", | ||
"ContractFactoryPlug": "0x25190648330361f35d00a0D41BD347de8E1B838C", | ||
"FastSwitchboard": "0x39d21e679312Bf0e3681bd0254c587E3528dd2a3", | ||
"FeesPlug": "0xe9BDa44a39F8d29eaF1956EB05442551794871f3", | ||
"Socket": "0x86264607bAD260e9032add9e4E2DA74f71E354E0", | ||
"SocketBatcher": "0x5B83E4104E37c7ECDf6Aeb62a4E204E4c63ac8D5", | ||
"startBlock": 157984273 | ||
}, | ||
"7625382": { | ||
"AddressResolver": "0x67790E222c41b0E787C278e757b7c40f03Fa5709", | ||
"AddressResolverImpl": "0x89C928379fED43B7117b852931e2968ce39C8380", | ||
"AsyncDeployer": "0x1C70bc3043667e884222B8835E0Ae554eb512810", | ||
"AsyncDeployerImpl": "0x09a762309c63a4e19cd3d822aA340Fe964Ba9C92", | ||
"AuctionManager": "0xC12aDF88dfc116CAF88816d150FE498843dABEEe", | ||
"AuctionManagerImpl": "0x55b76897b3BF6ED04188cbaa7DC21ae14b35D3eE", | ||
"Configurations": "0x377431bD1A3321C401542C8B1EC6E0c23E125042", | ||
"ConfigurationsImpl": "0xDe5DedAe6e17f906D1269D5e84BEfB06F3926310", | ||
"DeployForwarder": "0xd48218b2DafF9063177b0c6Bae229ec6C5f086a9", | ||
"DeployForwarderImpl": "0x8e178161BB3B36a28C15DFBe3142afF8757B8993", | ||
"ERC1967Factory": "0x870fCA8803bEFd119B1317AFB6794F97af7e515e", | ||
"FeesManager": "0x761A9024D267006061ec943d02e3949678906f3E", | ||
"FeesManagerImpl": "0x29C583B64FD2d7b70f8F6253C2a28D60af364Cb5", | ||
"FeesPool": "0x9De353dD1131aB4e502590D3a1832652FA316268", | ||
"PromiseResolver": "0x73b1B3dF6C71e0aa912f9d6933920D4461ae9718", | ||
"ReadPrecompile": "0x58f49313816c1876417EE53De8F5de047359fB2C", | ||
"RequestHandler": "0x63a6D7096b5a2F5c9Ce7D8632A7A2034A85b7F01", | ||
"RequestHandlerImpl": "0x593f4844ceEA828bC6d9D78A0ef7Ce64F42190dC", | ||
"SchedulePrecompile": "0xF77d2059a66026Efac11334D30372429553CAaC3", | ||
"startBlock": 8626651, | ||
"Watcher": "0xe4D1B4B8c0eEE90ac1f5314e758446CBa201BBA8", | ||
"WatcherImpl": "0x7726e559A5129A9174f89F7E2029f7212B66dD13", | ||
"WritePrecompile": "0xd8be408E271EEe9d3D0f28305bB9b6003589E1A9", | ||
"WritePrecompileImpl": "0xE24c4b0f67f566Fa558b3FE85f1780CD330f1F4D" | ||
"CCTPSwitchboard": "0xaF912b7eaD59f5d8c8179f8606A3fa93459a612C", | ||
"CCTPSwitchboardId": "2", | ||
"ContractFactoryPlug": "0x2784a207d51DD8c1A5323C05Fb868B68a854326C", | ||
"FastSwitchboard": "0xd8DDEA0c49C0bcbFD31272c262a4b31FfB44a8Bc", | ||
"FastSwitchboardId": "1", | ||
"FeesPlug": "0x3CFe3e7eCc62232B3d29454DCABeE4f2B921e3a2", | ||
"MessageSwitchboard": "0x0C049f6Dd92eA5dA05058b9B309755Cb347587C9", | ||
"MessageSwitchboardId": "3", | ||
"Socket": "0x90c5210fcFbFF7DA5753F3ddAEBee4102b8d6A9B", | ||
"SocketBatcher": "0xd726FeF96aD21AAD24443FAE933DDa9c79716a93", | ||
"startBlock": 178158322, | ||
"SwitchboardIdToAddressMap": { | ||
"1": "0xd8DDEA0c49C0bcbFD31272c262a4b31FfB44a8Bc", | ||
"2": "0xaF912b7eaD59f5d8c8179f8606A3fa93459a612C", | ||
"3": "0x0C049f6Dd92eA5dA05058b9B309755Cb347587C9" | ||
} | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Store switchboard IDs as numbers, not strings
FastSwitchboardId
, CCTPSwitchboardId
, MessageSwitchboardId
and the keys inside SwitchboardIdToAddressMap
are currently JSON strings ("1"
, "2"
…).
The contracts treat these values as uint64
, and several hardhat scripts parse them with BigNumber.from(id)
. Keeping them as JSON numbers avoids an unnecessary string → number
coercion and prevents subtle bugs (e.g. accidental leading-zero or hex parsing).
- "FastSwitchboardId": "1",
- "CCTPSwitchboardId": "2",
- "MessageSwitchboardId": "3",
+ "FastSwitchboardId": 1,
+ "CCTPSwitchboardId": 2,
+ "MessageSwitchboardId": 3,
"SwitchboardIdToAddressMap": {
- "1": "0xd8DDEA0c49C0bcbFD31272c262a4b31FfB44a8Bc",
- "2": "0xaF912b7eaD59f5d8c8179f8606A3fa93459a612C",
- "3": "0x0C049f6Dd92eA5dA05058b9B309755Cb347587C9"
+ 1: "0xd8DDEA0c49C0bcbFD31272c262a4b31FfB44a8Bc",
+ 2: "0xaF912b7eaD59f5d8c8179f8606A3fa93459a612C",
+ 3: "0x0C049f6Dd92eA5dA05058b9B309755Cb347587C9"
}
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In deployments/dev_addresses.json between lines 28 and 45, the switchboard ID
values such as FastSwitchboardId, CCTPSwitchboardId, MessageSwitchboardId, and
the keys in SwitchboardIdToAddressMap are stored as JSON strings. Change these
string values to JSON numbers by removing the quotes around the numeric IDs to
ensure they are treated as numbers, matching the uint64 type expected by the
contracts and avoiding unnecessary string-to-number conversions.
ChainSlug.ARBITRUM_SEPOLIA, | ||
ChainSlug.OPTIMISM_SEPOLIA, | ||
ChainSlug.BASE_SEPOLIA, | ||
// ChainSlug.BASE_SEPOLIA, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Document the reasoning for excluding BASE_SEPOLIA.
The exclusion of ChainSlug.BASE_SEPOLIA
from LOCAL and DEV deployment modes should be documented to clarify whether this is temporary or a permanent architectural decision.
Also applies to: 38-38
🏁 Script executed:
#!/bin/bash
# Description: Check if BASE_SEPOLIA exclusion affects existing deployments or configurations
echo "Checking for BASE_SEPOLIA references in deployment files..."
rg "BASE_SEPOLIA" deployments/ --type json
echo -e "\nChecking for BASE_SEPOLIA in other configuration files..."
rg "BASE_SEPOLIA" hardhat-scripts/ --type ts
Length of output: 672
Document BASE_SEPOLIA exclusion in config.ts
No other deployment or config files reference ChainSlug.BASE_SEPOLIA
outside of hardhat-scripts/config/config.ts
, so commenting it out has no upstream impact. Please add a clear comment explaining why BASE_SEPOLIA
is excluded (e.g. deprecated network, testing isolation, etc.) so future maintainers understand whether this is a temporary or permanent change.
Affected locations:
- hardhat-scripts/config/config.ts: lines where
// ChainSlug.BASE_SEPOLIA,
appears (around line 32 and line 38)
🤖 Prompt for AI Agents
In hardhat-scripts/config/config.ts around lines 32 and 38, add a clear comment
next to the commented out `ChainSlug.BASE_SEPOLIA` explaining why it is
excluded, such as it being deprecated or reserved for testing isolation. This
will help future maintainers understand if the exclusion is temporary or
permanent.
chains: {}, | ||
testnetChainSlugs: testnetChains, | ||
mainnetChainSlugs: mainnetChains, | ||
evmxChainSlug: EVMX_CHAIN_ID as ChainSlug, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
LGTM! Enhanced S3Config with chain-specific information.
The addition of evmxChainSlug
property provides better chain identification in the S3 configuration, aligning with the switchboard ID migration mentioned in the PR summary.
🏁 Script executed:
#!/bin/bash
# Description: Verify that S3Config type definition includes the evmxChainSlug property
echo "Checking S3Config type definition..."
ast-grep --pattern 'interface S3Config {
$$$
}'
echo -e "\nChecking for evmxChainSlug in type definitions..."
rg "evmxChainSlug" --type ts src/types.ts
Length of output: 316
Ensure evmxChainSlug
uses ChainSlug
in the S3Config interface
The runtime code casts EVMX_CHAIN_ID
to ChainSlug
, but in src/types.ts
the S3Config
interface still declares:
evmxChainSlug: number;
To keep static types in sync with your runtime cast, please update the interface:
• In src/types.ts
under interface S3Config
, change:
- evmxChainSlug: number;
+ evmxChainSlug: ChainSlug;
• Ensure you import ChainSlug
from its module:
import { ChainSlug } from '…';
This will preserve type safety and align the interface with the actual value.
🤖 Prompt for AI Agents
In src/types.ts around the S3Config interface, update the type of evmxChainSlug
from number to ChainSlug to match the runtime cast in
hardhat-scripts/s3Config/buildConfig.ts line 26. Import ChainSlug from its
module at the top of src/types.ts. This ensures the static type definition
aligns with the actual runtime type and maintains type safety.
FastSwitchboardId: string; | ||
CCTPSwitchboardId: string; | ||
MessageSwitchboardId: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Consider using number type for switchboard IDs
The switchboard ID properties are typed as string
, but based on the contract interfaces using uint64
for switchboard IDs, these should be typed as number
for better type safety and consistency.
- FastSwitchboardId: string;
- CCTPSwitchboardId: string;
- MessageSwitchboardId: string;
+ FastSwitchboardId: number;
+ CCTPSwitchboardId: number;
+ MessageSwitchboardId: number;
📝 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.
FastSwitchboardId: string; | |
CCTPSwitchboardId: string; | |
MessageSwitchboardId: string; | |
FastSwitchboardId: number; | |
CCTPSwitchboardId: number; | |
MessageSwitchboardId: number; |
🤖 Prompt for AI Agents
In src/types.ts around lines 24 to 26, the switchboard ID properties are
currently typed as string, but they should be changed to number to align with
the contract interfaces that use uint64 for these IDs. Update the type
annotations of FastSwitchboardId, CCTPSwitchboardId, and MessageSwitchboardId
from string to number to improve type safety and consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🔭 Outside diff range comments (3)
hardhat-scripts/deploy/6.connect.ts (2)
48-63
: Add validation for switchboardId before using it.The code retrieves
switchboardId
but doesn't validate if it exists or is valid. Consider adding a check to ensure the switchboardId is present and not "0" before proceeding with the connection.// Get switchboard id and app gateway addresses const switchboardId = addr[Contracts.FastSwitchboardId]; + if (!switchboardId || switchboardId === "0") { + console.log(`FastSwitchboardId not found or invalid for ${plugContract} on ${chain}`); + return; + } const appGatewayId = getAppGatewayId(plugContract, addresses); checkIfAppGatewayIdExists(appGatewayId, "AppGatewayId");
120-141
: Validate switchboardId and optimize operation order.Similar to the
connectPlug
function, the switchboardId should be validated. Also, consider checking the switchboardId before converting the plug address to bytes32 format to avoid unnecessary operations.const appGatewayId = getAppGatewayId(plugContract, addresses); const switchboardId = addr[Contracts.FastSwitchboardId]; - const plugBytes32Hex = toBytes32FormatHexString(addr[plugContract]); checkIfAppGatewayIdExists(appGatewayId, "AppGatewayId"); + if (!switchboardId || switchboardId === "0") { + console.log(`FastSwitchboardId not found or invalid for ${plugContract} on ${chain}`); + continue; + } + const plugBytes32Hex = toBytes32FormatHexString(addr[plugContract]);hardhat-scripts/deploy/3.configureChains.ts (1)
303-322
: Check for remote switchboard address before conversion.The code converts the remote switchboard address to bytes32 format before checking if it exists, which could cause errors if the address is undefined.
for (const remoteChainSlug of remoteChainSlugs) { + const remoteSwitchboardAddress = addresses[remoteChainSlug]?.[Contracts.CCTPSwitchboard]; + if (!remoteSwitchboardAddress) { + console.log( + `Remote switchboard address not found for ${remoteChainSlug}` + ); + continue; + } const remoteSwitchboardAddress = toBytes32FormatHexString( - addresses[remoteChainSlug]?.[Contracts.CCTPSwitchboard]! + remoteSwitchboardAddress ); const currentRemoteEndpoint = await switchboard.chainSlugToRemoteEndpoint( remoteChainSlug ); if (currentRemoteEndpoint.remoteAddress == remoteSwitchboardAddress) { console.log(`Remote endpoint ${remoteChainSlug} already exists`); continue; } - - if (!remoteSwitchboardAddress) { - console.log( - `Remote switchboard address not found for ${remoteChainSlug}` - ); - continue; - }
♻️ Duplicate comments (2)
hardhat-scripts/config/config.ts (1)
32-32
: Document the BASE_SEPOLIA exclusion reasoning.The commented-out
ChainSlug.BASE_SEPOLIA
in LOCAL deployment should include a clear comment explaining why it's excluded (e.g., deprecated network, testing isolation, temporary exclusion).FunctionSignatures.md (1)
544-577
: MessageSwitchboard documentation is comprehensive.The new MessageSwitchboard contract section properly documents all 31 functions with their signatures. The previous suggestion about automating markdown generation from Solidity ABI remains valid for long-term maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
FunctionSignatures.md
(3 hunks)deployments/stage_addresses.json
(1 hunks)deployments/stage_verification.json
(1 hunks)foundry.toml
(1 hunks)hardhat-scripts/config/config.ts
(2 hunks)hardhat-scripts/constants/feeConstants.ts
(1 hunks)hardhat-scripts/deploy/3.configureChains.ts
(9 hunks)hardhat-scripts/deploy/6.connect.ts
(4 hunks)hardhat-scripts/utils/overrides.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: sebastiantf
PR: SocketDotTech/socket-ll-contracts#277
File: src/bridges/gnosis-bridge-router/GnosisBridgeRouterImpl.sol:235-241
Timestamp: 2025-07-28T16:22:57.704Z
Learning: In the GnosisBridgeRouterImpl contract, the team chose to skip receiverAddress validation in the swapAndBridge function to save gas costs, consistent with their gas optimization priorities throughout the Socket bridge contracts.
hardhat-scripts/utils/overrides.ts (2)
Learnt from: sebastiantf
PR: SocketDotTech/socket-ll-contracts#277
File: src/bridges/gnosis-bridge-router/GnosisBridgeRouterImpl.sol:86-97
Timestamp: 2025-07-28T16:22:11.571Z
Learning: In the Socket bridge contracts, gas optimization is prioritized over code deduplication. When suggesting refactoring to reduce code duplication (like extracting helper functions), consider that function calls have gas overhead and users prefer to keep gas costs minimal even if it means having some duplicated code.
Learnt from: shreykeny
PR: SocketDotTech/bungee-backend#576
File: src/modules/router/routers/staked-oft/dummyData.config.ts:1522-1522
Timestamp: 2025-07-28T16:59:04.164Z
Learning: In the SocketDotTech/bungee-backend OFT dummy data configuration, the lzNativeFee values are denominated in each chain's native token, not ETH. For example, the Plume chain's fee of '140000000000000000000' represents 140 PLUME tokens, not ETH, which is reasonable for that network's fee structure.
hardhat-scripts/constants/feeConstants.ts (1)
Learnt from: shreykeny
PR: SocketDotTech/bungee-backend#576
File: src/modules/router/routers/staked-oft/dummyData.config.ts:1522-1522
Timestamp: 2025-07-28T16:59:04.164Z
Learning: In the SocketDotTech/bungee-backend OFT dummy data configuration, the lzNativeFee values are denominated in each chain's native token, not ETH. For example, the Plume chain's fee of '140000000000000000000' represents 140 PLUME tokens, not ETH, which is reasonable for that network's fee structure.
hardhat-scripts/deploy/6.connect.ts (1)
Learnt from: sebastiantf
PR: SocketDotTech/socket-ll-contracts#277
File: src/bridges/gnosis-bridge-router/GnosisBridgeRouterImpl.sol:235-241
Timestamp: 2025-07-28T16:22:57.704Z
Learning: In the GnosisBridgeRouterImpl contract, the team chose to skip receiverAddress validation in the swapAndBridge function to save gas costs, consistent with their gas optimization priorities throughout the Socket bridge contracts.
hardhat-scripts/deploy/3.configureChains.ts (2)
Learnt from: sebastiantf
PR: SocketDotTech/socket-ll-contracts#277
File: src/bridges/gnosis-bridge-router/GnosisBridgeRouterImpl.sol:235-241
Timestamp: 2025-07-28T16:22:57.704Z
Learning: In the GnosisBridgeRouterImpl contract, the team chose to skip receiverAddress validation in the swapAndBridge function to save gas costs, consistent with their gas optimization priorities throughout the Socket bridge contracts.
Learnt from: sebastiantf
PR: SocketDotTech/socket-ll-contracts#277
File: src/bridges/gnosis-bridge-router/GnosisBridgeRouterImpl.sol:126-131
Timestamp: 2025-07-28T16:22:33.706Z
Learning: In the Socket bridge contracts, gas optimization is prioritized over additional validation checks. The team prefers to skip validation checks like require(msg.value == amount)
and require(receiverAddress != address(0))
in native token bridging functions to minimize gas costs for users, even if it means accepting some additional risk.
FunctionSignatures.md (3)
Learnt from: sebastiantf
PR: SocketDotTech/socket-ll-contracts#277
File: src/bridges/gnosis-bridge-router/GnosisBridgeRouterImpl.sol:86-97
Timestamp: 2025-07-28T16:22:11.571Z
Learning: In the Socket bridge contracts, gas optimization is prioritized over code deduplication. When suggesting refactoring to reduce code duplication (like extracting helper functions), consider that function calls have gas overhead and users prefer to keep gas costs minimal even if it means having some duplicated code.
Learnt from: sebastiantf
PR: SocketDotTech/socket-ll-contracts#277
File: src/bridges/gnosis-bridge-router/GnosisBridgeRouterImpl.sol:126-131
Timestamp: 2025-07-28T16:22:33.706Z
Learning: In the Socket bridge contracts, gas optimization is prioritized over additional validation checks. The team prefers to skip validation checks like require(msg.value == amount)
and require(receiverAddress != address(0))
in native token bridging functions to minimize gas costs for users, even if it means accepting some additional risk.
Learnt from: sebastiantf
PR: SocketDotTech/socket-ll-contracts#277
File: src/bridges/gnosis-bridge-router/GnosisBridgeRouterImpl.sol:235-241
Timestamp: 2025-07-28T16:22:57.704Z
Learning: In the GnosisBridgeRouterImpl contract, the team chose to skip receiverAddress validation in the swapAndBridge function to save gas costs, consistent with their gas optimization priorities throughout the Socket bridge contracts.
🪛 LanguageTool
FunctionSignatures.md
[grammar] ~53-~53: Use correct spacing
Context: ...ds |
0x9133f232` | ## Socket | Function | Signatu...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~94-~94: Use correct spacing
Context: ... | 0x54fd4d50
| ## SocketFeeManager | Function | Signatu...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~114-~114: Use correct spacing
Context: ...wnership |
0xf2fde38b` | ## FeesManager | Function | Sig...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~173-~173: Use correct spacing
Context: ... | 0x023276f0
| ## FeesPool | Function | Signatu...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~188-~188: There might be a mistake here.
Context: ...38b| |
withdraw |
0xf3fef3a3` | ## AddressResolver | Function ...
(QB_NEW_EN_OTHER)
[grammar] ~216-~216: Use correct spacing
Context: ... |
0x300bb063` | ## AsyncDeployer | Function | Sign...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~246-~246: Use correct spacing
Context: ... | 0x300bb063
| ## AsyncPromise | Function | Signature | |...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~268-~268: Use correct spacing
Context: ...watcher__ |
0x300bb063` | ## DeployForwarder | Function | Signatu...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~290-~290: Use correct spacing
Context: ... |
0x300bb063` | ## Forwarder | Function | Signature | |...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~304-~304: There might be a mistake here.
Context: ... 0x6ccae054
| | watcher__
| 0x300bb063
| ## ProxyFactory | Function ...
(QB_NEW_EN_OTHER)
[grammar] ~319-~319: There might be a mistake here.
Context: ...c4| |
upgradeAndCall |
0x9623609d` | ## TestUSDC | Function | Signat...
(QB_NEW_EN_OTHER)
[grammar] ~338-~338: There might be a mistake here.
Context: ...| 0xa9059cbb
| | transferFrom
| 0x23b872dd
| ## ContractFactoryPlug | Function ...
(QB_NEW_EN_OTHER)
[grammar] ~364-~364: Use correct spacing
Context: ...wnership |
0xf2fde38b` | ## Configurations | Function | Signatu...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~388-~388: Use correct spacing
Context: ... |
0x300bb063` | ## PromiseResolver | Function | Signature | | -...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~395-~395: There might be a problem here.
Context: ... | 0xbf8484b8
| | watcher__
| 0x300bb063
| ## RequestHandler | Function | Sig...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~434-~434: Use correct spacing
Context: ... | 0x300bb063
| ## Watcher | Function | Signatu...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~519-~519: Use correct spacing
Context: ...tions |
0x6f30514c` | ## FastSwitchboard | Function | Signatu...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~544-~544: There might be a mistake here.
Context: ...wnership |
0xf2fde38b` | ## MessageSwitchboard | Function | Signa...
(QB_NEW_EN_OTHER)
[grammar] ~578-~578: Use correct spacing
Context: ...wnership |
0xf2fde38b` | ## ReadPrecompile | Function | Signa...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~591-~591: There might be a mistake here.
Context: ...f| |
watcher__ |
0x300bb063` | ## SchedulePrecompile | Function ...
(QB_NEW_EN_OTHER)
[grammar] ~612-~612: Use correct spacing
Context: ... | 0x300bb063
| ## WritePrecompile | Function | Signa...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🔇 Additional comments (7)
hardhat-scripts/constants/feeConstants.ts (1)
23-24
: LGTM! Fee pool addresses updated for DEV and STAGE environments.The fee pool address updates align with the broader switchboard ID migration. Ensure these new addresses are properly coordinated with the deployment configurations in
deployments/dev_addresses.json
anddeployments/stage_addresses.json
.foundry.toml (1)
13-35
: All foundry.toml addresses verified in deployments JSON files.
- Every address listed in
foundry.toml
was found indeployments/*.json
.- Configuration aligns with the switchboard contract updates.
No further action required.
hardhat-scripts/utils/overrides.ts (2)
31-31
: Verify BASE chain gas handling after removing explicit overrides.The commented-out
gasLimit
forChainSlug.BASE
removes explicit gas configuration. Ensure this doesn't cause transaction failures or suboptimal gas usage.
37-39
: EVMX gas-override removal is safe
- Inspected
chainOverrides[EVMX_CHAIN_ID]
inhardhat-scripts/utils/overrides.ts
: the only gas settings (type
,gasLimit
,gasPrice
) are commented out.- Searched all Hardhat deploy & admin scripts for any other
gasLimit
orgasPrice
overrides on EVMX—none found.- All EVMX transactions now fall back to ethers/hardhat’s default gas estimation, so removing static overrides has no adverse impact on cost or reliability.
deployments/stage_verification.json (1)
3-11
: Confirm intentional reduction in stage verification entriesThe
stage_verification.json
now only includes theAsyncDeployer
contract for chain14323
(and an empty array for8453
), whereas indev_verification.json
chain7625382
holds many verified contracts. Please confirm that:
- Stage is meant to verify only
AsyncDeployer
on14323
(and no other contracts).- Omitting the other chains’ verification data in stage is intentional and not accidental data loss.
deployments/stage_addresses.json (1)
1-134
: Configuration structure looks good!The switchboard ID assignments are consistent across all chains (1=Fast, 2=CCTP, 3=Message) and the
SwitchboardIdToAddressMap
correctly maps these IDs to their respective addresses. The EVMx chain (14323) appropriately lacks switchboard contracts.hardhat-scripts/deploy/3.configureChains.ts (1)
219-285
: Verify MSG_SB_FEES value in constantsFound in hardhat-scripts/constants/constants.ts:
• export const MSG_SB_FEES = "100000000";Please confirm that
"100000000"
is the correct fee amount for all MessageSwitchboard sibling configurations across your supported chains.
[DeploymentMode.DEV]: 14323, | ||
[DeploymentMode.STAGE]: 14323, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify chain ID consistency across all deployment files.
Both DEV and STAGE environments now use chain ID 14323
. This unification is significant and requires verification that all related configuration files are updated consistently.
🏁 Script executed:
#!/bin/bash
# Description: Verify chain ID 14323 consistency across deployment and verification files
echo "Checking deployment files for chain ID 14323..."
rg "14323" deployments/ --type json
echo -e "\nChecking for any remaining references to old chain IDs..."
rg "7625382|43" deployments/ --type json
echo -e "\nVerifying verification files match deployment files..."
find deployments/ -name "*_verification.json" -exec echo "=== {} ===" \; -exec cat {} \;
Length of output: 6459
Remove legacy chain IDs from dev_verification.json
The DEV verification file still lists entries under the old chain ID (7625382
), even though both DEV and STAGE have been unified to 14323
. This will cause your verification step to skip the new 14323
deployments and continue targeting the old chain.
• File to fix:
– deployments/dev_verification.json
• Remove the entire "7625382": […]
block
• If any of those contracts still need on-chain verification, move them into the "14323": […]
array
• (Optional) When grepping for chain IDs in CI scripts, anchor your regex to the start of the line (e.g. ^ *"7625382":
) to avoid matching "43"
inside hex addresses.
🤖 Prompt for AI Agents
In hardhat-scripts/config/config.ts around lines 90 to 91, the legacy chain ID
7625382 is still present in dev_verification.json, causing verification to
target the old chain instead of the unified 14323. Remove the entire "7625382"
block from deployments/dev_verification.json and move any contracts needing
verification into the "14323" array. Optionally, update CI scripts to anchor
regex searches for chain IDs to the start of the line to avoid false matches.
const fastSwitchboardId = await registerSb( | ||
chain, | ||
chainAddresses[Contracts.FastSwitchboard], | ||
signer, | ||
socketContract | ||
); | ||
deployUtils.addresses[Contracts.FastSwitchboardId] = | ||
fastSwitchboardId.toString(); | ||
deployUtils.addresses.SwitchboardIdToAddressMap[ | ||
fastSwitchboardId.toString() | ||
] = chainAddresses[Contracts.FastSwitchboard]; | ||
|
||
await registerSb( | ||
const cctpSwitchboardId = await registerSb( | ||
chain, | ||
chainAddresses[Contracts.CCTPSwitchboard], | ||
signer, | ||
socketContract | ||
); | ||
deployUtils.addresses[Contracts.CCTPSwitchboardId] = | ||
cctpSwitchboardId.toString(); | ||
deployUtils.addresses.SwitchboardIdToAddressMap[ | ||
cctpSwitchboardId.toString() | ||
] = chainAddresses[Contracts.CCTPSwitchboard]; | ||
|
||
const messageSwitchboardId = await registerSb( | ||
chain, | ||
chainAddresses[Contracts.MessageSwitchboard], | ||
signer, | ||
socketContract | ||
); | ||
deployUtils.addresses[Contracts.MessageSwitchboardId] = | ||
messageSwitchboardId.toString(); | ||
deployUtils.addresses.SwitchboardIdToAddressMap[ | ||
messageSwitchboardId.toString() | ||
] = chainAddresses[Contracts.MessageSwitchboard]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add validation for switchboard addresses before registration.
The code attempts to register switchboards without checking if their addresses exist. This could lead to runtime errors if a switchboard address is missing.
+ if (!chainAddresses[Contracts.FastSwitchboard]) {
+ console.log(`FastSwitchboard address not found for chain ${chain}`);
+ } else {
const fastSwitchboardId = await registerSb(
chain,
chainAddresses[Contracts.FastSwitchboard],
signer,
socketContract
);
deployUtils.addresses[Contracts.FastSwitchboardId] =
fastSwitchboardId.toString();
deployUtils.addresses.SwitchboardIdToAddressMap[
fastSwitchboardId.toString()
] = chainAddresses[Contracts.FastSwitchboard];
+ }
Apply similar validation for CCTPSwitchboard and MessageSwitchboard registrations.
📝 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.
const fastSwitchboardId = await registerSb( | |
chain, | |
chainAddresses[Contracts.FastSwitchboard], | |
signer, | |
socketContract | |
); | |
deployUtils.addresses[Contracts.FastSwitchboardId] = | |
fastSwitchboardId.toString(); | |
deployUtils.addresses.SwitchboardIdToAddressMap[ | |
fastSwitchboardId.toString() | |
] = chainAddresses[Contracts.FastSwitchboard]; | |
await registerSb( | |
const cctpSwitchboardId = await registerSb( | |
chain, | |
chainAddresses[Contracts.CCTPSwitchboard], | |
signer, | |
socketContract | |
); | |
deployUtils.addresses[Contracts.CCTPSwitchboardId] = | |
cctpSwitchboardId.toString(); | |
deployUtils.addresses.SwitchboardIdToAddressMap[ | |
cctpSwitchboardId.toString() | |
] = chainAddresses[Contracts.CCTPSwitchboard]; | |
const messageSwitchboardId = await registerSb( | |
chain, | |
chainAddresses[Contracts.MessageSwitchboard], | |
signer, | |
socketContract | |
); | |
deployUtils.addresses[Contracts.MessageSwitchboardId] = | |
messageSwitchboardId.toString(); | |
deployUtils.addresses.SwitchboardIdToAddressMap[ | |
messageSwitchboardId.toString() | |
] = chainAddresses[Contracts.MessageSwitchboard]; | |
if (!chainAddresses[Contracts.FastSwitchboard]) { | |
console.log(`FastSwitchboard address not found for chain ${chain}`); | |
} else { | |
const fastSwitchboardId = await registerSb( | |
chain, | |
chainAddresses[Contracts.FastSwitchboard], | |
signer, | |
socketContract | |
); | |
deployUtils.addresses[Contracts.FastSwitchboardId] = | |
fastSwitchboardId.toString(); | |
deployUtils.addresses.SwitchboardIdToAddressMap[ | |
fastSwitchboardId.toString() | |
] = chainAddresses[Contracts.FastSwitchboard]; | |
} | |
const cctpSwitchboardId = await registerSb( | |
chain, | |
chainAddresses[Contracts.CCTPSwitchboard], | |
signer, | |
socketContract | |
); | |
deployUtils.addresses[Contracts.CCTPSwitchboardId] = | |
cctpSwitchboardId.toString(); | |
deployUtils.addresses.SwitchboardIdToAddressMap[ | |
cctpSwitchboardId.toString() | |
] = chainAddresses[Contracts.CCTPSwitchboard]; | |
const messageSwitchboardId = await registerSb( | |
chain, | |
chainAddresses[Contracts.MessageSwitchboard], | |
signer, | |
socketContract | |
); | |
deployUtils.addresses[Contracts.MessageSwitchboardId] = | |
messageSwitchboardId.toString(); | |
deployUtils.addresses.SwitchboardIdToAddressMap[ | |
messageSwitchboardId.toString() | |
] = chainAddresses[Contracts.MessageSwitchboard]; |
🤖 Prompt for AI Agents
In hardhat-scripts/deploy/3.configureChains.ts between lines 67 and 101, the
code registers switchboards without validating if their addresses exist, which
can cause runtime errors. Add checks before each registerSb call to verify that
the corresponding switchboard address in chainAddresses is defined and valid. If
an address is missing, handle the error appropriately, such as throwing an error
or skipping registration, to prevent runtime failures. Apply this validation
consistently for FastSwitchboard, CCTPSwitchboard, and MessageSwitchboard
registrations.
}); | ||
|
||
if (Number(sb) == 0) { | ||
if (Number(switchboardId) == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use strict equality and handle edge cases in switchboard ID comparison.
The current comparison using Number(switchboardId) == 0
could behave unexpectedly if switchboardId is undefined, null, or an invalid string.
- if (Number(switchboardId) == 0) {
+ if (!switchboardId || switchboardId === "0") {
📝 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 (Number(switchboardId) == 0) { | |
if (!switchboardId || switchboardId === "0") { |
🤖 Prompt for AI Agents
In hardhat-scripts/deploy/3.configureChains.ts at line 375, replace the loose
equality check using Number(switchboardId) == 0 with a strict equality check and
add validation to ensure switchboardId is defined and a valid number before
comparison. This prevents unexpected behavior when switchboardId is undefined,
null, or an invalid string.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores