-
Notifications
You must be signed in to change notification settings - Fork 8
Script and proxy changes #196
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
base: dev
Are you sure you want to change the base?
Conversation
Itchy Palm Devnet
Folder rearrange
Folder rearrange
…socket-protocol into fees-deposit-hook
feat: hyper gas limti
…socket-protocol into fees-deposit-hook
Caution Review failedFailed to post review comments WalkthroughBroad refactor introducing tokenized credits (ERC20-based), SUSDC plug, richer events/APIs, off-chain simulation, upgraded watcher/promise architecture, and expanded chain/config tooling. Significant interface changes ripple through plugs, fees, watcher, protocol, deployments, scripts, and tests. New production/stage/dev deployment data added; extensive test suites and mocks introduced. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant FeesPlug
participant FeesManager as Credit (ERC20)
participant Receiver as IReceiver (optional)
User->>FeesPlug: depositCredit(token, receiver, amount, data)
FeesPlug->>FeesManager: deposit(chainSlug, token, receiver, native=0, credit=amount, data)
alt receiver implements IReceiver
FeesManager-->>Receiver: onTransfer(chainSlug, token, credit, native, data)
end
FeesManager-->>User: emit FeesDeposited(..., data)
sequenceDiagram
autonumber
actor Transmitter
participant Socket
participant Switchboard
participant Plug
participant FeeMgr as SocketFeeManager
Transmitter->>Socket: execute(payloadId, executeParams, transmissionParams)
Socket->>Switchboard: verify digest/allowPayload
Socket->>Plug: external call (payload)
alt success
Socket->>FeeMgr: payAndCheckFees(executeParams, transmissionParams) [payable]
Socket-->>Transmitter: return (ok, data)
else failure
Socket-->>Transmitter: refund ETH to refundAddress or msg.sender
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
✨ Finishing touches
🧪 Generate unit tests
Comment |
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: 28
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
setupInfraContracts.sh (1)
1-5
: Preserve backward compatibility for skip flag.Changing the argument to
"skip"
drops support for existing callers still passing"skip-compile"
, so those flows now compile unexpectedly (or fail when they relied on the skip). This should keep accepting the old token while adding the new alias.-if [ "$1" = "skip" ]; then +if [ "$1" = "skip" ] || [ "$1" = "skip-compile" ]; thensrc/signer.ts (1)
10-10
: Update nonce type in WatcherMultiCallParams
Thenonce
property inWatcherMultiCallParams
(hardhat-scripts/constants/types.ts:10) is defined asnumber
butgetNonce()
now returns astring
. Change it tostring
.contracts/evmx/fees/Credit.sol (1)
170-204
: Fix double-mint on failed native withdraw.When
feesPool.withdraw
fails we call_mint
withcreditAmount_
a second time, so the user receives twice the intended credits whiletokenOnChainBalances
only increased by the original amounts. This should only mint the fallbacknativeAmount_
, keep the bookkeeping totals in sync, and forward the adjusted amounts to callbacks/events.- tokenOnChainBalances[chainSlug_][token_] += creditAmount_ + nativeAmount_; - - // Mint tokens to the user - _mint(depositTo_, creditAmount_); - - if (nativeAmount_ > 0) { - // if native transfer fails, add to credit - bool success = feesPool.withdraw(depositTo_, nativeAmount_); - - if (!success) { - _mint(depositTo_, creditAmount_); - nativeAmount_ = 0; - creditAmount_ += nativeAmount_; - } - } - - if (data_.length > 0) - IReceiver(depositTo_).onTransfer( - chainSlug_, - token_, - creditAmount_, - nativeAmount_, - data_ - ); - - emit Deposited(chainSlug_, token_, depositTo_, creditAmount_, nativeAmount_); + tokenOnChainBalances[chainSlug_][token_] += creditAmount_ + nativeAmount_; + + uint256 finalCreditAmount = creditAmount_; + uint256 finalNativeAmount = nativeAmount_; + + // Mint base credits to the user + _mint(depositTo_, finalCreditAmount); + + if (finalNativeAmount > 0) { + // if native transfer fails, convert it to credits + bool success = feesPool.withdraw(depositTo_, finalNativeAmount); + + if (!success) { + _mint(depositTo_, finalNativeAmount); + finalCreditAmount += finalNativeAmount; + finalNativeAmount = 0; + } + } + + if (data_.length > 0) { + IReceiver(depositTo_).onTransfer( + chainSlug_, + token_, + finalCreditAmount, + finalNativeAmount, + data_ + ); + } + + emit Deposited(chainSlug_, token_, depositTo_, finalCreditAmount, finalNativeAmount);hardhat.config.ts (2)
35-43
: Fix private key handling to avoid double 0x/undefined accounts.This should not prepend "0x" blindly and should guard missing env; current code can produce
0xundefined
or0x0x...
.Apply:
-const privateKey: HardhatNetworkAccountUserConfig = process.env - .SOCKET_SIGNER_KEY as unknown as HardhatNetworkAccountUserConfig; +const rawPk = process.env.SOCKET_SIGNER_KEY || ""; +const privateKey: string = rawPk.replace(/^0x/i, "");And:
- accounts: [`0x${privateKey}`], + accounts: privateKey ? [`0x${privateKey}`] : [],Also applies to: 38-43
98-100
: Use chainId (not slug) for the hardhat network.This should map Hardhat slug → chainId; using the slug directly risks a wrong local chain id.
Apply:
- hardhat: { - chainId: hardhatChainNameToSlug[HardhatChainName.HARDHAT], - }, + hardhat: { + chainId: ChainSlugToId[hardhatChainNameToSlug[HardhatChainName.HARDHAT]], + },contracts/protocol/SocketBatcher.sol (1)
92-104
: Avoid rolling back a successful execute if syncOut fails.Wrap the external
syncOut
call in a try/catch so a sync failure doesn’t revert the main execution:- ICCTPSwitchboard(switchboard).syncOut(payloadId, cctpParams_.nextBatchRemoteChainSlugs); - return (success, returnData); + try ICCTPSwitchboard(switchboard).syncOut(payloadId, cctpParams_.nextBatchRemoteChainSlugs) { + } catch { + } + return (success, returnData);Confirm if strict atomicity is required; otherwise this enforces best-effort syncing.
hardhat-scripts/deploy/1.deploy.ts (1)
101-109
: Await the watcher signer.getWatcherSigner()
is awaited everywhere else because it returns a promise. Assigning it here withoutawait
leavessigner
as a Promise, sosigner.address
,.connect(signer)
, and every downstream use will throw at runtime. This should add the missingawait
when fetching the watcher signer.contracts/evmx/AuctionManager.sol (1)
165-169
: Fix_startAuction
guard. The guard currently reverts unless the status is alreadyOPEN
, so_startAuction
always reverts when the status isNOT_STARTED
/RESTARTED
—exactly the cases where we need to open the auction. As written, bidding can never progress. This should allow the pre-open states through (or drop the guard) before setting the status toOPEN
.
🧹 Nitpick comments (14)
test/apps/ParallelCounter.t.sol (1)
44-83
: Consider loop-based retrieval for reduced repetition.The four getOnChainAndForwarderAddresses calls follow an identical pattern. If you want to reduce repetition, you could iterate over arrays of chainSlugs and counterIds. However, for test clarity, the current explicit approach is acceptable.
package.json (1)
59-61
: Move@types/uuid
todevDependencies
.Type definitions are compile-time only. This should stay in
devDependencies
so consumers do not install unused runtime packages."devDependencies": { "@aws-sdk/client-s3": "^3.670.0", + "@types/uuid": "^10.0.0", "@nomicfoundation/hardhat-verify": "^2.0.14",
- "dependencies": { - "@types/uuid": "^10.0.0" - } + "dependencies": {}src/chain-enums/eventBlockRange.ts (1)
3-6
: Consider making block range values configurable.The hardcoded value of 1000 for both HYPEREVM and SEI might not suit all chains' characteristics (block time, reorg depth). If these values need adjustment per environment or chain behavior, extract them to a config file or environment variable.
hardhat-scripts/test/gas-fees.ts (1)
22-49
: Remove duplicate gas price logging and consider upgrading ethers.Two issues:
- Gas price is logged twice (lines 24-28 and 45-49), which is redundant.
- This script uses ethers v2 API (
ethers.providers.JsonRpcProvider
,ethers.utils.formatUnits
), which is historical. The current maintained version is v6 with breaking API changes.Apply this diff to remove the duplicate:
console.log( "Max Priority Fee Per Gas:", ethers.utils.formatUnits(feeData.maxPriorityFeePerGas || 0, "gwei"), "gwei" ); - - console.log( - "Gas Price:", - ethers.utils.formatUnits(gasPrice, "gwei"), - "gwei" - );For the ethers version: If this project is actively maintained, upgrade to ethers v6 for security and provider updates. The v6 migration guide covers API changes (Provider construction, unit handling, async semantics). Based on learnings.
contracts/protocol/base/PlugBase.sol (2)
18-19
: Consider using bool for isSocketInitialized.The
isSocketInitialized
state variable is declared asuint256
but used as a boolean flag (0 = false, 1 = true). Usingbool
is more gas-efficient, clearer, and idiomatic unless there's a specific reason to useuint256
(e.g., storage packing optimization).
24-25
: Consider adding parameters to ConnectorPlugDisconnected event.The
ConnectorPlugDisconnected
event has no parameters, which limits off-chain observability. Adding indexed parameters likeplug
address andsocket
address would improve traceability for disconnect events.Example:
event ConnectorPlugDisconnected(address indexed plug, address indexed socket);Then emit with:
emit ConnectorPlugDisconnected(address(this), address(socket__));script/helpers/DepositCreditMainnet.s.sol (1)
34-34
: Updated call matches new signature.The empty
bytes("")
parameter correctly adapts to the expandeddepositCredit
signature.Consider parameterizing the
data
argument via environment variable or script argument if future use cases require passing metadata with deposits.src/enums.ts (1)
103-106
: LGTM!GasPriceType enum correctly defines the two main Ethereum transaction pricing models and integrates with the ChainGasPriceType mapping.
Consider maintaining consistent alphabetical ordering across all Contracts enum members for easier navigation:
export enum Contracts { - Socket = "Socket", - FeesPlug = "FeesPlug", - SUSDC = "SUSDC", - ContractFactoryPlug = "ContractFactoryPlug", + AddressResolver = "AddressResolver", + AsyncDeployer = "AsyncDeployer", + AsyncPromise = "AsyncPromise", + AsyncPromiseImpl = "AsyncPromiseImpl", + AuctionManager = "AuctionManager", + CCTPSwitchboard = "CCTPSwitchboard", + CCTPSwitchboardId = "CCTPSwitchboardId", + Configurations = "Configurations", + ContractFactoryPlug = "ContractFactoryPlug", + DeployForwarder = "DeployForwarder", FastSwitchboard = "FastSwitchboard", FastSwitchboardId = "FastSwitchboardId", - CCTPSwitchboard = "CCTPSwitchboard", - CCTPSwitchboardId = "CCTPSwitchboardId", + FeesManager = "FeesManager", + FeesPlug = "FeesPlug", + FeesPool = "FeesPool", + Forwarder = "Forwarder", + ForwarderImpl = "ForwarderImpl", MessageSwitchboard = "MessageSwitchboard", MessageSwitchboardId = "MessageSwitchboardId", - SocketBatcher = "SocketBatcher", - SocketFeeManager = "SocketFeeManager", - AddressResolver = "AddressResolver", - Watcher = "Watcher", - RequestHandler = "RequestHandler", - Configurations = "Configurations", PromiseResolver = "PromiseResolver", - AuctionManager = "AuctionManager", - FeesManager = "FeesManager", - WritePrecompile = "WritePrecompile", ReadPrecompile = "ReadPrecompile", + RequestHandler = "RequestHandler", SchedulePrecompile = "SchedulePrecompile", - FeesPool = "FeesPool", - AsyncDeployer = "AsyncDeployer", - DeployForwarder = "DeployForwarder", - Forwarder = "Forwarder", - ForwarderImpl = "ForwarderImpl", - AsyncPromise = "AsyncPromise", - AsyncPromiseImpl = "AsyncPromiseImpl", + Socket = "Socket", + SocketBatcher = "SocketBatcher", + SocketFeeManager = "SocketFeeManager", + SUSDC = "SUSDC", + Watcher = "Watcher", + WritePrecompile = "WritePrecompile", }contracts/protocol/SocketBatcher.sol (1)
39-66
: Complete NatSpec and keep interface modular.This should document switchboardId_ and refundAddress_ in attestAndExecute; and IFastSwitchboard should live under interfaces/ for reuse.
Apply:
- * @param transmitterProof_ The signature of the transmitter + * @param transmitterProof_ The signature of the transmitter + * @param switchboardId_ The switchboard id to use for attestation + * @param refundAddress_ Refund address for any excess valueAnd extract IFastSwitchboard to contracts/interfaces/IFastSwitchboard.sol with the same contents, then import it here.
hardhat-scripts/deploy/4.configureEVMx.ts (2)
155-165
: Avoid shadowing ‘chains’ identifier for clarity.This should rename locals to prevent confusion with imported chains.
Apply:
- if (maxFeesUpdateArray.length > 0) { - const chains = maxFeesUpdateArray.map((item) => item.chainSlug); - const maxFees = maxFeesUpdateArray.map((item) => item.maxFees); - let tx = await feesManagerContract.setChainMaxFees(chains, maxFees); + if (maxFeesUpdateArray.length > 0) { + const chainSlugsToUpdate = maxFeesUpdateArray.map((i) => i.chainSlug); + const maxFeesToSet = maxFeesUpdateArray.map((i) => i.maxFees); + let tx = await feesManagerContract.setChainMaxFees(chainSlugsToUpdate, maxFeesToSet); console.log( - `Setting Chain Max Fees for chains: ${chains.join(", ")} tx hash: ${ + `Setting Chain Max Fees for chains: ${chainSlugsToUpdate.join(", ")} tx hash: ${ tx.hash }` );
141-151
: Use consistent units in logs.This should log the token unit correctly; formatEther implies 18 decimals. If these fees are in SUSDC (18d), keep as SUSDC; if USDC (6d), use formatUnits(fees, 6).
hardhat-scripts/deploy/2.roles.ts (1)
147-155
: Commented parallel processing code should be removed or enabled.The parallel processing implementation using
pLimit
is commented out. Either enable it if concurrency is desired, or remove the commented code and the unusedpLimit
import (line 5) to keep the codebase clean.hardhat-scripts/verify/verify.ts (2)
34-36
: Align VerifyParams type with numeric ChainSlug keysKeys are now chain slugs (numbers), but the type uses HardhatChainName (strings). This should match the JSON shape to prevent misuse.
Apply:
-export type VerifyParams = { - [chain in HardhatChainName]?: VerifyArgs[]; -}; +export type VerifyParams = Partial<Record<ChainSlug, VerifyArgs[]>>;
54-59
: Avoid parallelizing verifyChain with p‑limit; hre.changeNetwork is globalRunning verifyChain concurrently will race network switches on the single global HRE and produce incorrect verifications. If you need parallelism, spawn separate node processes per chain/network; otherwise keep this sequential.
If you want, I can draft a small runner that spawns
hardhat verify
in child processes per chain with a configurable concurrency limit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (128)
.gitignore
(1 hunks)Errors.md
(1 hunks)EventTopics.md
(6 hunks)FunctionSignatures.md
(2 hunks)contracts/evmx/AuctionManager.sol
(4 hunks)contracts/evmx/base/AppGatewayBase.sol
(2 hunks)contracts/evmx/fees/Credit.sol
(13 hunks)contracts/evmx/fees/FeesManager.sol
(5 hunks)contracts/evmx/interfaces/IAuctionManager.sol
(1 hunks)contracts/evmx/interfaces/IERC20.sol
(1 hunks)contracts/evmx/interfaces/IFeesManager.sol
(2 hunks)contracts/evmx/interfaces/IFeesPlug.sol
(2 hunks)contracts/evmx/interfaces/IPromise.sol
(1 hunks)contracts/evmx/interfaces/IReceiver.sol
(1 hunks)contracts/evmx/interfaces/ISUSDC.sol
(1 hunks)contracts/evmx/plugs/FeesPlug.sol
(3 hunks)contracts/evmx/plugs/SUSDC.sol
(1 hunks)contracts/evmx/watcher/Configurations.sol
(2 hunks)contracts/evmx/watcher/PromiseResolver.sol
(3 hunks)contracts/evmx/watcher/RequestHandler.sol
(5 hunks)contracts/evmx/watcher/Trigger.sol
(3 hunks)contracts/evmx/watcher/Watcher.sol
(2 hunks)contracts/evmx/watcher/WatcherBase.sol
(2 hunks)contracts/evmx/watcher/precompiles/SchedulePrecompile.sol
(3 hunks)contracts/evmx/watcher/precompiles/WritePrecompile.sol
(1 hunks)contracts/protocol/Socket.sol
(10 hunks)contracts/protocol/SocketBatcher.sol
(2 hunks)contracts/protocol/SocketConfig.sol
(3 hunks)contracts/protocol/SocketFeeManager.sol
(2 hunks)contracts/protocol/SocketUtils.sol
(4 hunks)contracts/protocol/base/PlugBase.sol
(3 hunks)contracts/protocol/interfaces/ICCTPSwitchboard.sol
(1 hunks)contracts/protocol/interfaces/IMessageHandler.sol
(1 hunks)contracts/protocol/interfaces/IMessageTransmitter.sol
(1 hunks)contracts/protocol/interfaces/IPlug.sol
(1 hunks)contracts/protocol/interfaces/ISocket.sol
(3 hunks)contracts/protocol/interfaces/ISocketBatcher.sol
(2 hunks)contracts/protocol/interfaces/ISocketFeeManager.sol
(2 hunks)contracts/protocol/interfaces/ISwitchboard.sol
(1 hunks)contracts/protocol/switchboard/FastSwitchboard.sol
(1 hunks)contracts/protocol/switchboard/SwitchboardBase.sol
(2 hunks)contracts/utils/RescueFundsLib.sol
(2 hunks)contracts/utils/common/Errors.sol
(1 hunks)deployments/dev_addresses.json
(1 hunks)deployments/dev_verification.json
(1 hunks)deployments/prod_addresses.json
(1 hunks)deployments/prod_verification.json
(1 hunks)deployments/stage_addresses.json
(1 hunks)deployments/stage_verification.json
(1 hunks)foundry.toml
(1 hunks)hardhat-scripts/addChain/utils.ts
(1 hunks)hardhat-scripts/admin/disconnect.ts
(1 hunks)hardhat-scripts/config/config.ts
(2 hunks)hardhat-scripts/constants/constants.ts
(2 hunks)hardhat-scripts/constants/fee.ts
(1 hunks)hardhat-scripts/constants/feeConstants.ts
(1 hunks)hardhat-scripts/constants/types.ts
(0 hunks)hardhat-scripts/deploy/1.deploy.ts
(12 hunks)hardhat-scripts/deploy/2.roles.ts
(4 hunks)hardhat-scripts/deploy/3.configureChains.ts
(12 hunks)hardhat-scripts/deploy/4.configureEVMx.ts
(4 hunks)hardhat-scripts/deploy/6.connect.ts
(4 hunks)hardhat-scripts/deploy/7.upload.ts
(1 hunks)hardhat-scripts/deploy/9.setupTransmitter.ts
(4 hunks)hardhat-scripts/deploy/UpgradeForwarder.ts
(1 hunks)hardhat-scripts/deploy/UpgradePromise.ts
(1 hunks)hardhat-scripts/deploy/WhitelistFeesReceiver.ts
(1 hunks)hardhat-scripts/deploy/deployTestUSDC.ts
(1 hunks)hardhat-scripts/s3Config/buildConfig.ts
(4 hunks)hardhat-scripts/test/chainTest.ts
(1 hunks)hardhat-scripts/test/gas-fees.ts
(1 hunks)hardhat-scripts/utils/address.ts
(2 hunks)hardhat-scripts/utils/appConfig.ts
(2 hunks)hardhat-scripts/utils/deployUtils.ts
(4 hunks)hardhat-scripts/utils/gatewayId.ts
(1 hunks)hardhat-scripts/utils/overrides.ts
(4 hunks)hardhat-scripts/verify/verify.ts
(3 hunks)hardhat.config.ts
(2 hunks)package.json
(3 hunks)script/counter/DeployEVMxCounterApp.s.sol
(1 hunks)script/counter/WithdrawFeesArbitrumFeesPlug.s.sol
(1 hunks)script/helpers/CheckDepositedCredits.s.sol
(1 hunks)script/helpers/DepositCredit.s.sol
(1 hunks)script/helpers/DepositCreditAndNative.s.sol
(1 hunks)script/helpers/DepositCreditMainnet.s.sol
(1 hunks)script/helpers/TransferRemainingCredits.s.sol
(1 hunks)script/helpers/WithdrawRemainingCredits.s.sol
(1 hunks)setupInfraContracts.sh
(2 hunks)src/chain-enums/chainId.ts
(1 hunks)src/chain-enums/chainSlug.ts
(1 hunks)src/chain-enums/chainSlugToHardhatChainName.ts
(1 hunks)src/chain-enums/chainSlugToId.ts
(1 hunks)src/chain-enums/chainSlugToKey.ts
(1 hunks)src/chain-enums/currency.ts
(1 hunks)src/chain-enums/ethLikeChains.ts
(1 hunks)src/chain-enums/eventBlockRange.ts
(1 hunks)src/chain-enums/gasPriceType.ts
(1 hunks)src/chain-enums/hardhatChainName.ts
(1 hunks)src/chain-enums/hardhatChainNameToSlug.ts
(1 hunks)src/chain-enums/mainnetIds.ts
(1 hunks)src/chain-enums/native-tokens.ts
(1 hunks)src/chain-enums/opStackChains.ts
(1 hunks)src/chain-enums/testnetIds.ts
(1 hunks)src/enums.ts
(3 hunks)src/finality.ts
(1 hunks)src/signer.ts
(2 hunks)src/types.ts
(4 hunks)test/SetupTest.t.sol
(17 hunks)test/SocketUSDC.sol
(1 hunks)test/Utils.t.sol
(1 hunks)test/Watcher.t.sol
(0 hunks)test/apps/ParallelCounter.t.sol
(2 hunks)test/apps/app-gateways/counter/CounterAppGateway.sol
(4 hunks)test/evmx/AuctionManager.t.sol
(2 hunks)test/evmx/FeesTest.t.sol
(7 hunks)test/evmx/ProxyMigration.t.sol
(1 hunks)test/evmx/ProxyStorage.t.sol
(1 hunks)test/evmx/Watcher.t.sol
(1 hunks)test/mock/MockERC721.sol
(1 hunks)test/mock/MockFastSwitchboard.sol
(2 hunks)test/mock/MockFeesManager.sol
(1 hunks)test/mock/MockPlug.sol
(1 hunks)test/protocol/Socket.t.sol
(1 hunks)test/protocol/SocketFeeManager.t.sol
(1 hunks)test/protocol/TriggerTest.t.sol
(1 hunks)test/protocol/switchboards/FastSwitchboardTest.t.sol
(1 hunks)test/protocol/switchboards/MessageSwitchboardTest.t copy.sol
(1 hunks)tsconfig.json
(1 hunks)
💤 Files with no reviewable changes (2)
- test/Watcher.t.sol
- hardhat-scripts/constants/types.ts
🧰 Additional context used
🧬 Code graph analysis (20)
hardhat-scripts/deploy/WhitelistFeesReceiver.ts (6)
hardhat-scripts/constants/types.ts (1)
DeploymentAddresses
(3-5)hardhat-scripts/utils/address.ts (1)
getAddresses
(8-27)hardhat-scripts/config/config.ts (2)
mode
(10-12)EVMX_CHAIN_ID
(181-181)src/types.ts (1)
EVMxAddressesObj
(35-51)hardhat-scripts/utils/sign.ts (1)
getWatcherSigner
(11-14)hardhat-scripts/utils/deployUtils.ts (1)
getInstance
(131-135)
hardhat-scripts/deploy/deployTestUSDC.ts (4)
hardhat-scripts/config/config.ts (4)
logConfig
(247-261)EVMX_CHAIN_ID
(181-181)mode
(10-12)socketOwner
(184-184)hardhat-scripts/utils/sign.ts (2)
getWatcherSigner
(11-14)getSocketSigner
(16-19)hardhat-scripts/constants/feeConstants.ts (1)
tokens
(5-342)hardhat-scripts/utils/deployUtils.ts (1)
deployContractWithArgs
(80-101)
hardhat-scripts/utils/deployUtils.ts (1)
hardhat-scripts/utils/overrides.ts (1)
overrides
(94-105)
hardhat-scripts/constants/fee.ts (2)
hardhat-scripts/config/config.ts (2)
EVMX_CHAIN_ID
(181-181)mainnetChains
(197-218)hardhat-scripts/constants/constants.ts (1)
FEE_MANAGER_WRITE_MAX_FEES
(15-15)
hardhat-scripts/deploy/UpgradePromise.ts (7)
hardhat-scripts/constants/types.ts (1)
DeploymentAddresses
(3-5)hardhat-scripts/utils/address.ts (1)
getAddresses
(8-27)hardhat-scripts/config/config.ts (2)
mode
(10-12)EVMX_CHAIN_ID
(181-181)src/types.ts (1)
ChainAddressesObj
(18-33)hardhat-scripts/utils/sign.ts (1)
getWatcherSigner
(11-14)hardhat-scripts/utils/deployUtils.ts (4)
DeployParams
(32-37)getOrDeploy
(39-78)getInstance
(131-135)storeAddresses
(168-195)hardhat-scripts/utils/overrides.ts (1)
overrides
(94-105)
hardhat-scripts/verify/verify.ts (3)
hardhat-scripts/config/config.ts (3)
chains
(180-180)EVMX_CHAIN_ID
(181-181)mode
(10-12)src/chain-enums/chainSlugToKey.ts (1)
ChainSlugToKey
(4-83)hardhat-scripts/utils/deployUtils.ts (2)
verify
(103-129)storeUnVerifiedParams
(197-222)
hardhat-scripts/s3Config/buildConfig.ts (6)
hardhat-scripts/config/config.ts (6)
testnetChains
(190-195)mainnetChains
(197-218)EVMX_CHAIN_ID
(181-181)IndexerHighChains
(226-237)IndexerLowChains
(239-241)cronOnlyChains
(220-223)src/chain-enums/eventBlockRange.ts (1)
ChainEventBlockRange
(3-6)hardhat-scripts/s3Config/utils.ts (1)
getChainType
(11-23)src/finality.ts (1)
getFinalityBlocks
(12-16)src/chain-enums/currency.ts (1)
Currency
(4-29)src/chain-enums/gasPriceType.ts (1)
ChainGasPriceType
(4-10)
hardhat-scripts/deploy/1.deploy.ts (3)
hardhat-scripts/config/config.ts (5)
skipEVMXDeployment
(268-268)WRITE_EXPIRY_TIME
(276-276)READ_EXPIRY_TIME
(277-277)SCHEDULE_EXPIRY_TIME
(278-278)socketOwner
(184-184)hardhat-scripts/constants/constants.ts (1)
FEE_MANAGER_WRITE_MAX_FEES
(15-15)hardhat-scripts/utils/deployUtils.ts (2)
getInstance
(131-135)getOrDeploy
(39-78)
hardhat-scripts/deploy/3.configureChains.ts (6)
hardhat-scripts/config/config.ts (2)
EVMX_CHAIN_ID
(181-181)getFeesPlugChains
(172-177)hardhat-scripts/utils/overrides.ts (2)
overrides
(94-105)getReadOverrides
(139-143)hardhat-scripts/utils/deployUtils.ts (1)
getInstance
(131-135)hardhat-scripts/constants/feeConstants.ts (2)
tokens
(5-342)getFeeTokens
(351-353)hardhat-scripts/utils/sign.ts (1)
getWatcherSigner
(11-14)hardhat-scripts/utils/address.ts (1)
toBytes32FormatHexString
(42-56)
hardhat-scripts/utils/overrides.ts (1)
hardhat-scripts/config/config.ts (1)
EVMX_CHAIN_ID
(181-181)
hardhat-scripts/deploy/4.configureEVMx.ts (5)
src/types.ts (1)
EVMxAddressesObj
(35-51)hardhat-scripts/utils/deployUtils.ts (1)
getInstance
(131-135)hardhat-scripts/utils/sign.ts (1)
getWatcherSigner
(11-14)hardhat-scripts/config/config.ts (2)
chains
(180-180)EVMX_CHAIN_ID
(181-181)hardhat-scripts/constants/fee.ts (1)
getMaxFees
(11-15)
hardhat-scripts/deploy/9.setupTransmitter.ts (2)
hardhat-scripts/config/config.ts (3)
transmitter
(183-183)TRANSMITTER_CREDIT_THRESHOLD
(297-297)TRANSMITTER_NATIVE_THRESHOLD
(298-298)hardhat-scripts/utils/sign.ts (1)
getWatcherSigner
(11-14)
hardhat.config.ts (3)
hardhat-scripts/s3Config/buildConfig.ts (1)
getChainConfig
(51-72)src/chain-enums/chainSlugToId.ts (1)
ChainSlugToId
(4-83)hardhat-scripts/config/config.ts (1)
EVMX_CHAIN_ID
(181-181)
hardhat-scripts/utils/appConfig.ts (3)
hardhat-scripts/utils/overrides.ts (1)
getReadOverrides
(139-143)hardhat-scripts/config/config.ts (2)
watcher
(182-182)EVMX_CHAIN_ID
(181-181)hardhat-scripts/utils/address.ts (1)
toBytes32Format
(58-65)
hardhat-scripts/deploy/6.connect.ts (6)
hardhat-scripts/utils/appConfig.ts (1)
isConfigSetOnSocket
(6-21)hardhat-scripts/utils/overrides.ts (1)
overrides
(94-105)hardhat-scripts/utils/address.ts (1)
getAddresses
(8-27)hardhat-scripts/config/config.ts (2)
mode
(10-12)chains
(180-180)hardhat-scripts/constants/types.ts (1)
DeploymentAddresses
(3-5)hardhat-scripts/utils/sign.ts (1)
getSocketSigner
(16-19)
hardhat-scripts/admin/disconnect.ts (2)
hardhat-scripts/utils/appConfig.ts (1)
isConfigSetOnSocket
(6-21)hardhat-scripts/constants/constants.ts (1)
BYTES32_ZERO
(12-12)
hardhat-scripts/deploy/UpgradeForwarder.ts (7)
hardhat-scripts/constants/types.ts (1)
DeploymentAddresses
(3-5)hardhat-scripts/utils/address.ts (1)
getAddresses
(8-27)hardhat-scripts/config/config.ts (2)
mode
(10-12)EVMX_CHAIN_ID
(181-181)src/types.ts (1)
ChainAddressesObj
(18-33)hardhat-scripts/utils/sign.ts (1)
getWatcherSigner
(11-14)hardhat-scripts/utils/deployUtils.ts (4)
DeployParams
(32-37)getOrDeploy
(39-78)getInstance
(131-135)storeAddresses
(168-195)hardhat-scripts/utils/overrides.ts (1)
overrides
(94-105)
hardhat-scripts/test/chainTest.ts (1)
src/chain-enums/chainSlugToHardhatChainName.ts (1)
chainSlugToHardhatChainName
(4-83)
hardhat-scripts/constants/feeConstants.ts (2)
src/types.ts (1)
TokenMap
(81-90)hardhat-scripts/config/config.ts (1)
mode
(10-12)
hardhat-scripts/deploy/2.roles.ts (2)
hardhat-scripts/utils/overrides.ts (2)
getReadOverrides
(139-143)overrides
(94-105)hardhat-scripts/config/config.ts (1)
chains
(180-180)
🪛 Shellcheck (0.11.0)
setupInfraContracts.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
🔇 Additional comments (95)
test/evmx/ProxyStorage.t.sol (1)
4-4
: LGTM!The import path update correctly reflects the test directory reorganization. No functional changes to the test logic.
src/finality.ts (2)
22-22
: Validate MAINNET LOW finality change (src/finality.ts:22)
Reducing LOW finality from 6 → 2 blocks (~24 s) raises reorg risk. This should be validated against the protocol’s risk tolerance and ensure downstream logic handles the lower threshold safely.
27-31
: Review POLYGON_MAINNET finality thresholds.
- LOW: 256 → 5, MEDIUM: 512 → 5, HIGH: 1000 → 5; all risk tiers collapsed.
- 5 blocks (~10 s) is excessively aggressive, raising reorg risk.
This should be verified against protocol security requirements; if intentional, restore distinct thresholds for LOW/MEDIUM/HIGH.
contracts/protocol/interfaces/ICCTPSwitchboard.sol (2)
48-49
: Documentation improvement clarifies parameter plurality.The updated comments make it explicit that
messages_
andattestations_
are lists, improving API clarity.
56-66
: ICCTPSwitchboard signature fully propagated
All implementers and call sites supply the new thirdpayloadId_
argument, matching the updated interface.src/signer.ts (1)
27-31
: No ABI encoding issues with hex string nonce. ethers.utils.defaultAbiCoder.encode accepts hex-prefixed strings as BigNumberish for uint256, so getNonce’s return is valid.test/apps/ParallelCounter.t.sol (2)
40-42
: Delegation to helper function is clean.The refactor successfully addresses stack depth concerns by extracting forwarder validation logic.
105-120
: Single forwarder assertion logic is correct.The function cleanly validates both chainSlug and onChainAddress with descriptive error messages.
test/evmx/ProxyMigration.t.sol (1)
5-5
: LGTM!The import path correction aligns with the test directory restructure.
contracts/protocol/interfaces/IPlug.sol (1)
16-16
: LGTM!The documentation clarifies the encoding format dependency on the watcher system, which is helpful context for implementers.
test/protocol/switchboards/MessageSwitchboardTest.t copy.sol (1)
4-5
: LGTM!The import path updates align with the test directory restructure.
script/counter/WithdrawFeesArbitrumFeesPlug.s.sol (1)
20-20
: LGTM!The API change from
getAvailableCredits
tobalanceOf
aligns with the ERC20-based credits refactor. The new naming is more standard and the semantics remain equivalent.contracts/evmx/base/AppGatewayBase.sol (2)
229-232
: LGTM!The API rename from
approveAppGatewayWithSignature
toapproveWithSignature
simplifies the interface. Return value handling remains correct.
245-245
: LGTM!The API rename from
approveAppGateway
toapprove
is cleaner and aligns with standard ERC20-like approval patterns.contracts/evmx/interfaces/IReceiver.sol (1)
1-14
: LGTM!The
IReceiver
interface provides a clean callback mechanism for transfer notifications. The function signature includes appropriate parameters for chain context, token, amounts, and arbitrary data, aligning with the enhanced deposit/credit flows in the PR.script/counter/DeployEVMxCounterApp.s.sol (1)
8-8
: Verify removal of--legacy
and--gas-price 0
is intentional
This should match other deployment scripts or be justified: dropping--legacy
changes transaction type on older EVMs and removing--gas-price 0
uses market gas prices. Confirm EVMx and test networks support the new flags.script/helpers/DepositCredit.s.sol (1)
33-33
: LGTM!The depositCredit call correctly passes an empty bytes parameter, aligning with the expanded API signature.
hardhat-scripts/addChain/utils.ts (1)
6-6
: LGTM!The path update correctly reflects the reorganized chain-enum directory structure.
hardhat-scripts/utils/gatewayId.ts (1)
16-20
: LGTM!The SUSDC case correctly reuses the FeesManager address lookup path, consistent with FeesPlug behavior.
contracts/protocol/interfaces/ISwitchboard.sol (1)
20-21
: LGTM!The added documentation clarifies the processTrigger function's usage and overridability for implementers.
test/evmx/AuctionManager.t.sol (2)
4-5
: LGTM!The import paths correctly reflect the updated test directory structure.
35-35
: LGTM!The approval call correctly uses the updated
approve
method, aligning with the refactored FeesManager API.foundry.toml (1)
13-36
: LGTM!The updated label addresses correctly reflect the new plug-based deployment architecture, including the new APP_GATEWAY entry.
hardhat-scripts/utils/address.ts (2)
4-4
: LGTM!The prod_addresses import correctly supports the new PROD deployment mode.
21-23
: LGTM!The PROD case correctly returns production addresses, extending deployment mode support.
test/protocol/TriggerTest.t.sol (1)
4-6
: LGTM!The import paths correctly reflect the updated test directory structure.
src/chain-enums/testnetIds.ts (1)
33-34
: LGTM!Adding MONAD_TESTNET and RISE_TESTNET to the testnet array aligns with the broader chain expansion across the codebase.
src/chain-enums/opStackChains.ts (1)
24-25
: LGTM!Adding INK and UNICHAIN to the OP Stack chain array is consistent with the expanded chain support.
contracts/evmx/watcher/WatcherBase.sol (2)
8-8
: LGTM!Importing centralized Errors.sol enables custom error reverts in the modifiers below.
18-29
: LGTM!Switching from
require()
with strings to custom error reverts (OnlyWatcherAllowed()
,OnlyRequestHandlerAllowed()
,OnlyPromiseResolverAllowed()
) saves gas and improves clarity.src/chain-enums/ethLikeChains.ts (1)
29-32
: LGTM!Adding FLOW, CAMP, PLUME, and RISE_TESTNET to the ETH-like chains array is consistent with the expanded chain support.
contracts/evmx/watcher/Watcher.sol (2)
10-10
: LGTM!Adding the CoreContractsSet event improves observability when core contract addresses are configured.
37-37
: LGTM!Emitting CoreContractsSet after updating state is correct and provides a clear signal for off-chain indexers.
src/chain-enums/hardhatChainNameToSlug.ts (1)
63-82
: LGTM!Extending the HardhatChainName to ChainSlug mapping with the new chains is consistent with the expanded chain support across the codebase.
contracts/evmx/watcher/Trigger.sol (2)
7-7
: LGTM!Adding IERC20 import enables the ERC20-based fee transfer pattern used in the updated
_callAppGateways
function.
51-51
: LGTM!Moving the
isAppGatewayCalled
flag assignment inside the success branch is correct. This prevents marking failed triggers as successfully called.contracts/evmx/interfaces/IPromise.sol (1)
15-19
: LGTM!The new
callbackSelector()
andcallbackData()
accessors cleanly expand the IPromise interface to expose promise callback metadata. The signatures and documentation are correct.contracts/protocol/interfaces/ISocketFeeManager.sol (1)
6-37
: LGTM!The documentation updates clarify parameter roles and return values without altering the interface's functional behavior. The expanded comments improve the API's usability.
script/helpers/DepositCreditAndNative.s.sol (1)
33-38
: LGTM!The call to
depositCreditAndNative
is correctly updated to include the newbytes memory data_
parameter. Passingbytes("")
as a placeholder is appropriate when no additional data is required.hardhat-scripts/admin/disconnect.ts (1)
54-56
: LGTM!The
isConfigSetOnSocket
call is correctly updated to include thechain
parameter, enabling per-chain configuration checks. This aligns with the broader per-chain awareness introduced in the PR.contracts/protocol/base/PlugBase.sol (1)
76-88
: LGTM!The
initSocket
function provides a standardized initialization entry point for plugs, correctly using thesocketInitializer
modifier to prevent re-initialization. The documentation clearly explains the security rationale for calling this function.Errors.md (1)
176-178
: LGTM!The new error definitions follow the existing documentation pattern. The signature hashes and naming are consistent with the contract's access-control checks mentioned in the AI summary.
contracts/protocol/switchboard/FastSwitchboard.sol (1)
64-72
: LGTM!Adding
virtual
enables test scaffolding to overrideprocessTrigger
for verification purposes, and the@inheritdoc
tag properly references the interface documentation.test/protocol/SocketFeeManager.t.sol (1)
4-7
: LGTM!Import paths correctly adjusted for test file reorganization. No functional changes.
contracts/utils/RescueFundsLib.sol (1)
8-19
: Minimal interface declarations acceptable.The IERC721 and IERC165 interfaces include only the methods needed for rescue operations.
src/enums.ts (2)
56-56
: LGTM!SUSDC enum addition supports the new SUSDC plug architecture visible throughout the PR.
79-82
: LGTM!Forwarder and AsyncPromise enum additions support the expanded deployment and proxy architecture.
src/chain-enums/gasPriceType.ts (1)
1-10
: Approve ChainGasPriceType mapping ChainGasPriceType correctly assigns LEGACY pricing and consumers default toGasPriceType.EIP1559
for unmapped chains.deployments/prod_addresses.json (1)
1-290
: Verify duplicate addresses across deployments.
Multiple chains reuse the same addresses (e.g., 0x4023941D9AB563b1c4d447B3f2A9dd2F1eF19fCA appears 35×; 0x3B1f4ABA1667EeB992B623E7c6d119728cEd3b15 appears 20×; 0xd90a33b0414F5C0De5F315428190598945BbEde2 appears 18×). Confirm this reuse is intentional (e.g., CREATE2) and validate via on-chain explorers that each address hosts the expected contract on its respective network.src/chain-enums/chainSlug.ts (1)
64-83
: LGTM: Enum expansion is correct.The new chain slugs follow the existing pattern and map to corresponding ChainId values. The additions are additive and do not introduce breaking changes.
hardhat-scripts/deploy/UpgradePromise.ts (4)
1-2
: LGTM: dotenv usage is correct.The dotenv config is loaded at the top of the file, ensuring environment variables are available before other imports and logic. This follows best practices.
33-48
: LGTM: Implementation deployment and comparison logic is correct.The script correctly handles both initial deployment (when
oldImpl
is undefined) and upgrade scenarios (whenoldImpl
exists). The case-insensitive address comparison and early return are appropriate.
64-68
: LGTM: Address storage and logging are correct.The updated deployment addresses are persisted and the success log is informative.
70-75
: LGTM: Process exit handling is correct.The script exits cleanly on success and logs errors before exiting with code 1 on failure. This is standard practice for deployment scripts.
src/chain-enums/currency.ts (1)
4-29
: LGTM: Currency mapping refactor is correct.The Currency mapping has been updated to use explicit NativeTokens constants instead of string keys. The new chain entries (SONIC, HYPEREVM, SEI, BERA, FLOW, CAMP, PLUME) follow the existing pattern. The correctness of the NativeTokens enum values is outside the scope of this file.
hardhat-scripts/deploy/deployTestUSDC.ts (5)
1-2
: LGTM: dotenv usage is correct.The dotenv config is loaded at the top of the file, ensuring environment variables are available before other imports and logic. This follows best practices.
17-21
: LGTM: Main function structure is correct.The main function calls logConfig, logBalances, and deployTestUSDC in sequence, which is a standard pattern for deployment scripts.
23-24
: LGTM: chainsToDeploy definition is correct.The script is configured to deploy TestUSDC to MANTA_PACIFIC. The commented-out alternative suggests flexibility for future expansion to multiple chains.
26-47
: LGTM: logBalances function is correct.The function retrieves and logs deployer balances concurrently using Promise.all. The logic is straightforward and correct.
80-85
: LGTM: Process exit handling is correct.The script exits cleanly on success and logs errors before exiting with code 1 on failure. This is standard practice for deployment scripts.
contracts/evmx/watcher/precompiles/SchedulePrecompile.sol (3)
5-5
: LGTM: Import statement is correct.The IPromise interface is imported to access localInvoker, callbackSelector, and callbackData from the asyncPromise.
37-44
: LGTM: ScheduleRequested event expansion is correct.The event now includes localInvoker, callbackSelector, and callbackData, providing richer context for scheduled payloads. This aligns with the broader event surface expansions across the PR.
140-150
: Verify asyncPromise implements IPromise.Line 140 casts
payloadParams.asyncPromise
toIPromise
without validation. IfasyncPromise
does not implement the IPromise interface, the calls tolocalInvoker()
,callbackSelector()
, andcallbackData()
will revert at runtime.Ensure that
asyncPromise
is guaranteed to be a valid IPromise contract. If not, add a validation check:require( IPromise(payloadParams.asyncPromise).localInvoker() != address(0), "Invalid asyncPromise" );Or verify that the asyncPromise is always a valid IPromise address in the calling context (e.g., in RequestHandler).
deployments/prod_verification.json (1)
1-150
: LGTM: JSON structure is correct.The deployment verification file is syntactically correct and follows a consistent structure. The entries map chain IDs to deployment records (address, contract name, source path, constructor args).
Note: Keys "169" and "747" have identical deployment entries except for the Socket contract's chainSlug argument (169 vs 747). Verify that this duplication is intentional (e.g., deploying the same contracts to different networks with the same addresses).
hardhat-scripts/utils/appConfig.ts (2)
6-21
: LGTM: Per-chain read overrides are correct.The
isConfigSetOnSocket
function now accepts achain
parameter and applies per-chain read overrides usinggetReadOverrides(chain)
. This aligns with the broader per-chain configuration changes in the PR.
30-36
: Ignore plug parameter-format check
getPlugConfigs
expectsbytes32
;toBytes32Format(plug)
returns anumber[]
(validBytesLike
). Code is correct.Likely an incorrect or invalid review comment.
FunctionSignatures.md (2)
19-21
: Approve selector documentation Documented selectors matchendAuction(bytes,bytes)
→ 0x7426f0f6 andexpireBid(bytes,bytes)
→ 0x33b5b234.
116-171
: Manual selector verification required
Ensure the 4-byte selectors in FunctionSignatures.md (Lines 116–171) exactly match the keccak256 hashes of the corresponding function signatures in contracts/evmx/fees/FeesManager.sol.hardhat.config.ts (1)
109-156
: Explorer apiURL entries reachable Verified all configured Etherscan and Blockscout endpoints returned HTTP responses (200/400), confirming valid apiURL configuration.src/chain-enums/hardhatChainName.ts (1)
60-79
: All new HardhatChainName entries are present in both mappings
Mapping coverage for ZERO, ZKSYNC, ARENA_Z, INK, SONIC, BERA, B3, UNICHAIN, MONAD_TESTNET, SCROLL, SONEIUM, SWELLCHAIN, WORLD_CHAIN, KATANA, HYPEREVM, SEI, FLOW, CAMP, PLUME, RISE_TESTNET is confirmed in hardhatChainNameToSlug and chainSlugToHardhatChainName.deployments/stage_addresses.json (1)
2-13
: Verify address checksums and mapping parity
This should be validated to avoid silent misconfigs: ensure every 0x address in deployments/stage_addresses.json is a valid EIP-55 checksum and each SwitchboardIdToAddressMap["1"] exactly matches its FastSwitchboard entry.contracts/protocol/SocketConfig.sol (4)
58-61
: LGTM: New events improve observability.The new
GasLimitBufferUpdated
andMaxCopyBytesUpdated
events follow the existing event pattern and will help track configuration changes.
112-115
: LGTM: Event emission added.The
SocketFeeManagerUpdated
event is now properly emitted when updating the socket fee manager, consistent with other configuration setters.
152-155
: LGTM: Event emission added.The
GasLimitBufferUpdated
event is now properly emitted after updating the gas limit buffer.
162-165
: LGTM: Event emission added.The
MaxCopyBytesUpdated
event is now properly emitted after updating the max copy bytes.hardhat-scripts/deploy/2.roles.ts (2)
72-76
: LGTM: Transaction logging improved.Moving the
txHash
log after transaction creation provides the actual hash rather than logging before it's available. This is a clear improvement.
58-65
: Confirm MANTLE-only read overrides in hasRole checkgetReadOverrides supplies overrides only for ChainSlug.MANTLE (returns {} otherwise). Ensure this matches requirements for role checks across all chains.
contracts/protocol/interfaces/ISocket.sol (1)
98-130
: LGTM: New view functions expand the public API.The five new view functions (
payloadExecuted
,chainSlug
,payloadIdToDigest
,triggerCounter
,switchboardAddresses
) provide necessary state access for payload tracking and switchboard management. These additions align with the expanded event surface described in the PR.hardhat-scripts/constants/feeConstants.ts (2)
351-353
: LGTM: Token address extraction correctly implemented.The
getFeeTokens
function now extracts theaddress
field from token objects, which is the correct transformation for the updated token structure.
5-342
: Validate hardcoded USDC addresses across all chains/modes
This should cross-check each address against official deployments (e.g., Etherscan, deployment docs) to prevent misconfigurations or failed transactions.src/types.ts (5)
22-27
: LGTM: Switchboard fields made optional.Making
CCTPSwitchboard
,MessageSwitchboard
, and their IDs optional correctly reflects that these switchboards may not be deployed on all chains. This improves type safety for conditional deployments.
30-30
: LGTM: SUSDC field added.The optional
SUSDC
field supports the new SUSDC plug integration mentioned in the PR objectives.
56-64
: LGTM: S3Config extended with new fields.The additions (
tokens
,indexerHighChains
,indexerLowChains
,cronOnlyChains
) expand the configuration capabilities for chain categorization and token management.
75-77
: LGTM: ChainConfig extended with chain-specific metadata.The
nativeToken
andgasPriceType
fields provide necessary per-chain configuration for gas price handling and native token identification.
81-90
: LGTM: TokenMap type properly defined.The
TokenMap
type structure correctly models token metadata per deployment mode and chain, with appropriate fields for token information (name, symbol, address, decimals).hardhat-scripts/utils/overrides.ts (5)
10-17
: LGTM: ChainOverride type well-defined.The
ChainOverride
type consolidates all override options, including the newgasPriceMultiplier
field, providing a clear structure for per-chain configuration.
8-8
: LGTM: Reasonable default gas price multiplier.The
DEFAULT_GAS_PRICE_MULTIPLIER
of 1.05 provides a 5% buffer above the base gas price, which is a sensible default for transaction inclusion while avoiding excessive overpayment.
114-124
: LGTM: Dynamic gas price computation improves flexibility.The dynamic gas price calculation using
provider.getGasPrice()
and applying the multiplier is superior to hardcoded values. This adapts to network conditions while still allowing per-chain overrides.
139-143
: getReadOverrides only applies to MANTLE.The
getReadOverrides
function only returns overrides for MANTLE chain and returns an empty object for all others. This is now used in role checks in2.roles.ts
. Confirm this MANTLE-specific behavior is intentional for read operations and doesn't mask issues on other chains.
59-62
: Verify BERA chain gas price override
This sets base 3 gwei plus multiplier 2 (6 gwei total). Confirm this matches BERA’s documented requirements.test/evmx/FeesTest.t.sol (4)
4-5
: LGTM: Import paths updated for test directory restructure.The import paths have been correctly updated from
./apps/
and./SetupTest.t.sol
to../apps/
and../SetupTest.t.sol
, reflecting a test directory reorganization.
14-14
: LGTM: counterPlug setup properly initialized.The
counterPlug
address is correctly retrieved from the deployed counter gateway's forwarder addresses, providing the necessary test fixture.Also applies to: 36-37
39-141
: LGTM: Comprehensive SUSDC flow test.The
testDepositAndMintSusdc
test provides thorough coverage of the new SUSDC functionality:
- Deposits USDC and mints SUSDC on EVMx with data execution
- Burns SUSDC on EVMx and mints on the destination chain
- Burns SUSDC on chain and mints back on EVMx
- Withdraws USDC by burning SUSDC
The test includes appropriate balance assertions at each step and validates the data payload execution (feesManagerSwitch). This is a valuable addition to the test suite.
144-144
: LGTM: API updated from approveAppGateway to approve.The function calls have been correctly updated from
approveAppGateway
toapprove
, reflecting the interface changes in the FeesManager contract.Also applies to: 222-222, 275-275
src/chain-enums/chainSlugToKey.ts (1)
63-82
: Enum definitions verified All newly added ChainSlug and HardhatChainName entries exist in their enums.
function setSusdcToken(uint32 chainSlug_, bytes32 susdcToken_) external onlyOwner { | ||
forwarderAddresses[susdcToken][chainSlug_] = asyncDeployer__().getOrDeployForwarderContract( | ||
susdcToken_, | ||
chainSlug_ | ||
); | ||
_setValidPlug(true, chainSlug_, susdcToken); | ||
emit SusdcTokenSet(chainSlug_, susdcToken_); | ||
} |
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.
Persist the SUSDC plug id before using it.
Here susdcToken
stays at its default zero value, so _setValidPlug
and later burn()
calls operate on plug id 0x0
while the event exposes the configured id. This should assign the provided susdcToken_
into storage and use that id when populating forwarderAddresses
and marking the plug valid.
- forwarderAddresses[susdcToken][chainSlug_] = asyncDeployer__().getOrDeployForwarderContract(
- susdcToken_,
- chainSlug_
- );
- _setValidPlug(true, chainSlug_, susdcToken);
+ susdcToken = susdcToken_;
+ forwarderAddresses[susdcToken_][chainSlug_] = asyncDeployer__().getOrDeployForwarderContract(
+ susdcToken_,
+ chainSlug_
+ );
+ _setValidPlug(true, chainSlug_, susdcToken_);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
function setSusdcToken(uint32 chainSlug_, bytes32 susdcToken_) external onlyOwner { | |
forwarderAddresses[susdcToken][chainSlug_] = asyncDeployer__().getOrDeployForwarderContract( | |
susdcToken_, | |
chainSlug_ | |
); | |
_setValidPlug(true, chainSlug_, susdcToken); | |
emit SusdcTokenSet(chainSlug_, susdcToken_); | |
} | |
function setSusdcToken(uint32 chainSlug_, bytes32 susdcToken_) external onlyOwner { | |
// persist the new plug id before using it | |
susdcToken = susdcToken_; | |
forwarderAddresses[susdcToken_][chainSlug_] = asyncDeployer__().getOrDeployForwarderContract( | |
susdcToken_, | |
chainSlug_ | |
); | |
_setValidPlug(true, chainSlug_, susdcToken_); | |
emit SusdcTokenSet(chainSlug_, susdcToken_); | |
} |
🤖 Prompt for AI Agents
In contracts/evmx/fees/Credit.sol around lines 138 to 145, the function
setSusdcToken uses the storage variable susdcToken (still zero) when calling
_setValidPlug and when emitting the event, causing operations to target plug id
0x0; persist the provided susdcToken_ into storage first (assign susdcToken =
susdcToken_), then use that stored value (or susdcToken_) when populating
forwarderAddresses and calling _setValidPlug, and emit the event with the
persisted plug id so subsequent burn()/validation uses the correct id.
if (balanceOf(consumeFrom_) < credits_) revert InsufficientCreditsAvailable(); | ||
|
||
UserCredits storage userCredit = userCredits[consumeFrom_]; | ||
userCredit.blockedCredits += credits_; | ||
userBlockedCredits[consumeFrom_] += credits_; | ||
requestBlockedCredits[requestCount_] = credits_; | ||
emit CreditsBlocked(requestCount_, consumeFrom_, credits_); |
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.
Fix available-balance check in blockCredits.
This should refuse a new block when the caller’s balance is already tied up in other in-flight requests, otherwise _burn
will revert once multiple batches unblock and try to burn more than the user still holds. Require the free balance (balance - already blocked
) before adding more:
- if (balanceOf(consumeFrom_) < credits_) revert InsufficientCreditsAvailable();
-
- userBlockedCredits[consumeFrom_] += credits_;
+ uint256 blocked = userBlockedCredits[consumeFrom_];
+ if (balanceOf(consumeFrom_) < blocked + credits_) revert InsufficientCreditsAvailable();
+
+ userBlockedCredits[consumeFrom_] = blocked + credits_;
📝 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 (balanceOf(consumeFrom_) < credits_) revert InsufficientCreditsAvailable(); | |
UserCredits storage userCredit = userCredits[consumeFrom_]; | |
userCredit.blockedCredits += credits_; | |
userBlockedCredits[consumeFrom_] += credits_; | |
requestBlockedCredits[requestCount_] = credits_; | |
emit CreditsBlocked(requestCount_, consumeFrom_, credits_); | |
// ensure we only block against the user’s free balance | |
uint256 blocked = userBlockedCredits[consumeFrom_]; | |
if (balanceOf(consumeFrom_) < blocked + credits_) revert InsufficientCreditsAvailable(); | |
userBlockedCredits[consumeFrom_] = blocked + credits_; | |
requestBlockedCredits[requestCount_] = credits_; | |
emit CreditsBlocked(requestCount_, consumeFrom_, credits_); |
🤖 Prompt for AI Agents
In contracts/evmx/fees/FeesManager.sol around lines 107 to 111, the current
check uses balanceOf(consumeFrom_) alone and allows blocking more credits than
the user's free balance (balance minus already blocked), which will later cause
_burn to revert; change the guard to compute the available/free balance as
balanceOf(consumeFrom_) minus userBlockedCredits[consumeFrom_] and revert if
that available amount is less than credits_, then proceed to increment
userBlockedCredits and set requestBlockedCredits as before (ensure the
subtraction is done safely or with an appropriate unchecked block if you are
certain it cannot underflow).
_burn(msg.sender, amount); | ||
IFeesManager(address(socket__)).mint(receiver_, amount, data_); | ||
emit TokensBurned(msg.sender, receiver_, amount, data_); | ||
} |
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.
Burn calls the wrong contract.
socket__
is the Socket contract, not the fees manager. Casting it to IFeesManager
and calling mint
will always revert because Socket does not expose that method. This should fetch the actual fees manager address (e.g. socket__.feesManager()
) and invoke IFeesManager
there instead.
- _burn(msg.sender, amount);
- IFeesManager(address(socket__)).mint(receiver_, amount, data_);
+ _burn(msg.sender, amount);
+ address feesManager = socket__.feesManager();
+ IFeesManager(feesManager).mint(receiver_, amount, data_);
📝 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.
_burn(msg.sender, amount); | |
IFeesManager(address(socket__)).mint(receiver_, amount, data_); | |
emit TokensBurned(msg.sender, receiver_, amount, data_); | |
} | |
_burn(msg.sender, amount); | |
address feesManager = socket__.feesManager(); | |
IFeesManager(feesManager).mint(receiver_, amount, data_); | |
emit TokensBurned(msg.sender, receiver_, amount, data_); | |
} |
🤖 Prompt for AI Agents
In contracts/evmx/plugs/SUSDC.sol around lines 88 to 91, the code casts socket__
(the Socket contract) to IFeesManager and calls mint, which will revert because
Socket does not implement that method; instead fetch the actual fees manager
address from the socket (e.g. address feesManager = socket__.feesManager()),
cast that address to IFeesManager and call
IFeesManager(feesManager).mint(receiver_, amount, data_); ensure the socket has
a public feesManager() accessor and import/use the correct IFeesManager
interface if not already.
if (queueParams_.transaction.chainSlug == 5000) { | ||
// Mantle default gas limit | ||
queueParams_.overrideParams.gasLimit = 8_000_000_000; | ||
} else if (queueParams_.transaction.chainSlug == 1329) { | ||
// Sei default gas limit | ||
queueParams_.overrideParams.gasLimit = 8_000_000; | ||
} else if (queueParams_.transaction.chainSlug == 999) { | ||
// HyperEVM default gas limit | ||
queueParams_.overrideParams.gasLimit = 1_500_000; | ||
} else { | ||
queueParams_.overrideParams.gasLimit = 10_000_000; // other chains default gas limit | ||
} |
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 | 🟠 Major
Replace magic numbers with ChainId constants.
Hardcoding chain slugs (5000, 1329, 999) reduces maintainability. If ChainId enum or constants exist for Mantle, Sei, and HyperEVM, reference them instead.
Apply this diff if ChainId constants are available:
- if (queueParams_.transaction.chainSlug == 5000) {
- // Mantle default gas limit
+ if (queueParams_.transaction.chainSlug == CHAIN_SLUG_MANTLE) {
queueParams_.overrideParams.gasLimit = 8_000_000_000;
- } else if (queueParams_.transaction.chainSlug == 1329) {
- // Sei default gas limit
+ } else if (queueParams_.transaction.chainSlug == CHAIN_SLUG_SEI) {
queueParams_.overrideParams.gasLimit = 8_000_000;
- } else if (queueParams_.transaction.chainSlug == 999) {
- // HyperEVM default gas limit
+ } else if (queueParams_.transaction.chainSlug == CHAIN_SLUG_HYPEREVM) {
queueParams_.overrideParams.gasLimit = 1_500_000;
} else {
- queueParams_.overrideParams.gasLimit = 10_000_000; // other chains default gas limit
+ queueParams_.overrideParams.gasLimit = 10_000_000;
}
If constants don't exist, define them in Constants.sol.
Committable suggestion skipped: line range outside the PR's diff.
address watcherFeesPayer = r.requestFeesDetails.winningBid.transmitter == address(0) | ||
? r.requestFeesDetails.consumeFrom | ||
: r.requestFeesDetails.winningBid.transmitter; | ||
feesManager__().transferCredits(watcherFeesPayer, address(this), totalFees); | ||
IERC20(address(feesManager__())).transferFrom(watcherFeesPayer, address(this), totalFees); | ||
} |
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.
Fix credits transfer destination.
Line 327 currently pulls the ERC20 credits into RequestHandler
itself. Nothing ever moves those tokens back out, so watcherFeesPayer
loses balance, the credits remain stranded here, and settlement (unblockAndAssignCredits
) cannot pay the winning transmitter. This should move the amount into FeesManager
(or otherwise reinstate the previous accounting flow) instead of address(this)
.
- IERC20(address(feesManager__())).transferFrom(watcherFeesPayer, address(this), totalFees);
+ IERC20(address(feesManager__())).transferFrom(
+ watcherFeesPayer,
+ address(feesManager__()),
+ totalFees
+ );
📝 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.
address watcherFeesPayer = r.requestFeesDetails.winningBid.transmitter == address(0) | |
? r.requestFeesDetails.consumeFrom | |
: r.requestFeesDetails.winningBid.transmitter; | |
feesManager__().transferCredits(watcherFeesPayer, address(this), totalFees); | |
IERC20(address(feesManager__())).transferFrom(watcherFeesPayer, address(this), totalFees); | |
} | |
address watcherFeesPayer = r.requestFeesDetails.winningBid.transmitter == address(0) | |
? r.requestFeesDetails.consumeFrom | |
: r.requestFeesDetails.winningBid.transmitter; | |
IERC20(address(feesManager__())).transferFrom( | |
watcherFeesPayer, | |
address(feesManager__()), | |
totalFees | |
); | |
} |
🤖 Prompt for AI Agents
In contracts/evmx/watcher/RequestHandler.sol around lines 324 to 328, the
transferFrom currently moves ERC20 credits from watcherFeesPayer into the
RequestHandler (address(this)) which strands tokens and breaks later settlement;
change the transfer recipient to the FeesManager so credits are held/recorded
there (i.e., call transferFrom(watcherFeesPayer, address(feesManager__()),
totalFees) or otherwise route funds into FeesManager) and ensure the call uses
the same IERC20 instance and existing approvals so accounting and
unblockAndAssignCredits can later pay the winning transmitter.
function getLastTransmissionParams() external view returns (TransmissionParams memory) { | ||
return lastTransmissionParams; | ||
} | ||
|
||
function reset() external { | ||
payAndCheckFeesCalled = false; | ||
} |
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.
Reset should clear cached structs.
reset()
only flips payAndCheckFeesCalled
and leaves lastExecuteParams
/ lastTransmissionParams
intact. Tests relying on a clean slate will read stale params and mis-attribute calls. Reset both structs so state truly resets.
Apply:
function reset() external {
payAndCheckFeesCalled = false;
+ delete lastExecuteParams;
+ delete lastTransmissionParams;
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
function getLastTransmissionParams() external view returns (TransmissionParams memory) { | |
return lastTransmissionParams; | |
} | |
function reset() external { | |
payAndCheckFeesCalled = false; | |
} | |
function getLastTransmissionParams() external view returns (TransmissionParams memory) { | |
return lastTransmissionParams; | |
} | |
function reset() external { | |
payAndCheckFeesCalled = false; | |
delete lastExecuteParams; | |
delete lastTransmissionParams; | |
} |
🤖 Prompt for AI Agents
In test/mock/MockFeesManager.sol around lines 37 to 43, the reset() function
currently only clears payAndCheckFeesCalled but leaves lastExecuteParams and
lastTransmissionParams populated; update reset() to also clear those cached
structs so state is truly reset (use Solidity's delete keyword or reassign
default structs for lastExecuteParams and lastTransmissionParams in the reset
function, keeping payAndCheckFeesCalled reset behavior intact).
function createSignature( | ||
bytes32 digest_, | ||
uint256 privateKey_ | ||
) public pure returns (bytes memory sig) { | ||
bytes32 digest = keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", digest_)); | ||
(uint8 sigV, bytes32 sigR, bytes32 sigS) = vm.sign(privateKey_, digest); | ||
sig = new bytes(65); | ||
bytes1 v32 = bytes1(sigV); | ||
assembly { | ||
mstore(add(sig, 96), v32) | ||
mstore(add(sig, 32), sigR) | ||
mstore(add(sig, 64), sigS) | ||
} | ||
} |
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.
Fix createSignature
mutability
This should compile, but createSignature
is marked pure
while calling vm.sign
, which is neither pure nor view. The compiler rejects that. Change the function to public
(or public view
) so the cheatcode call is permitted.
🤖 Prompt for AI Agents
In test/protocol/Socket.t.sol around lines 141 to 154, the function
createSignature is declared as public pure but calls the vm.sign cheatcode which
is not a pure/view operation; update the function signature to remove pure
(e.g., make it just public or public view) so the compiler allows the cheatcode
call and keep the rest of the implementation unchanged.
function testConstructorWithValidParameters() public view { | ||
assertEq(socket.chainSlug(), TEST_CHAIN_SLUG, "Chain slug should match"); | ||
assertEq(socket.owner(), socketOwner, "Owner should match"); | ||
assertEq(socket.version(), keccak256(bytes(VERSION)), "Version should match"); | ||
assertEq(socket.gasLimitBuffer(), 105, "Gas limit buffer should be 105"); |
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.
Remove illegal view
specifier from tests
This should compile, but every public view
test (starting here) calls forge-std assertions, which are non-view. Solidity forbids invoking them from a view
function, so the suite fails to compile. Drop the view
(and in digest tests, any other view
qualifiers) so the compiler accepts the file.
🤖 Prompt for AI Agents
In test/protocol/Socket.t.sol around lines 162 to 166, the test function is
declared "public view" but calls non-view forge-std assertions, which is
illegal; remove the "view" specifier from this test (and any other test
functions in the file that are declared view, e.g., digest tests) so they are
plain public functions and the compiler will accept the assertions.
function testCreatePayloadId() public pure { | ||
uint160 payloadPointer = 12345; | ||
uint64 switchboardId = 67890; | ||
uint32 chainSlug = 1; | ||
|
||
bytes32 payloadId = createPayloadId(payloadPointer, switchboardId, chainSlug); | ||
|
||
// Verify the structure | ||
uint32 chainSlugFromId = uint32(uint256(payloadId) >> 224); | ||
uint64 switchboardIdFromId = uint64(uint256(payloadId) >> 160); | ||
uint160 payloadPointerFromId = uint160(uint256(payloadId)); | ||
|
||
assertEq(chainSlugFromId, chainSlug, "Chain slug should match"); | ||
assertEq(switchboardIdFromId, switchboardId, "Switchboard ID should match"); | ||
assertEq(payloadPointerFromId, payloadPointer, "Payload pointer should match"); | ||
} | ||
|
||
function testCreatePayloadIdWithZeroValues() public pure { | ||
bytes32 payloadId = createPayloadId(0, 0, 0); | ||
|
||
assertEq(payloadId, bytes32(0), "Payload ID should be zero for zero inputs"); | ||
} | ||
|
||
function testCreatePayloadIdWithMaxValues() public pure { | ||
uint160 maxPayloadPointer = type(uint160).max; | ||
uint64 maxSwitchboardId = type(uint64).max; | ||
uint32 maxChainSlug = type(uint32).max; | ||
|
||
bytes32 payloadId = createPayloadId(maxPayloadPointer, maxSwitchboardId, maxChainSlug); | ||
|
||
// Verify the structure | ||
uint32 chainSlugFromId = uint32(uint256(payloadId) >> 224); | ||
uint64 switchboardIdFromId = uint64(uint256(payloadId) >> 160); | ||
uint160 payloadPointerFromId = uint160(uint256(payloadId)); | ||
|
||
assertEq(chainSlugFromId, maxChainSlug, "Chain slug should match"); | ||
assertEq(switchboardIdFromId, maxSwitchboardId, "Switchboard ID should match"); | ||
assertEq(payloadPointerFromId, maxPayloadPointer, "Payload pointer should match"); | ||
} | ||
|
||
function testCreatePayloadIdFuzz( | ||
uint160 payloadPointer, | ||
uint64 switchboardId, | ||
uint32 chainSlug | ||
) public pure { | ||
bytes32 payloadId = createPayloadId(payloadPointer, switchboardId, chainSlug); | ||
|
||
// Verify the structure | ||
uint32 chainSlugFromId = uint32(uint256(payloadId) >> 224); | ||
uint64 switchboardIdFromId = uint64(uint256(payloadId) >> 160); | ||
uint160 payloadPointerFromId = uint160(uint256(payloadId)); | ||
|
||
assertEq(chainSlugFromId, chainSlug, "Chain slug should match"); | ||
assertEq(switchboardIdFromId, switchboardId, "Switchboard ID should match"); | ||
assertEq(payloadPointerFromId, payloadPointer, "Payload pointer should match"); | ||
} | ||
} | ||
|
||
/** | ||
* @title ConvertersTest | ||
* @dev Tests for Converters utility functions | ||
*/ | ||
contract ConvertersTest is Test { | ||
function testToBytes32Format() public pure { | ||
address testAddr = address(0x1234567890123456789012345678901234567890); | ||
bytes32 result = toBytes32Format(testAddr); | ||
|
||
assertEq(result, bytes32(uint256(uint160(testAddr))), "Conversion should be correct"); | ||
} | ||
|
||
function testToBytes32FormatWithZeroAddress() public pure { | ||
bytes32 result = toBytes32Format(address(0)); | ||
|
||
assertEq(result, bytes32(0), "Zero address should convert to zero bytes32"); | ||
} | ||
|
||
function testFromBytes32Format() public pure { | ||
address originalAddr = address(0x1234567890123456789012345678901234567890); | ||
bytes32 bytes32Format = toBytes32Format(originalAddr); | ||
|
||
address convertedAddr = fromBytes32Format(bytes32Format); | ||
|
||
assertEq(convertedAddr, originalAddr, "Conversion should be reversible"); | ||
} | ||
|
||
function testFromBytes32FormatWithZeroAddress() public pure { | ||
bytes32 zeroBytes32 = bytes32(0); | ||
address convertedAddr = fromBytes32Format(zeroBytes32); | ||
|
||
assertEq(convertedAddr, address(0), "Zero bytes32 should convert to zero address"); | ||
} | ||
|
||
function testFromBytes32FormatWithInvalidAddress() public { | ||
// Create a bytes32 with non-zero upper bits | ||
bytes32 invalidBytes32 = bytes32(uint256(1) << 160); | ||
|
||
try this.fromBytes32FormatWrapper(invalidBytes32) { | ||
fail(); | ||
} catch { | ||
// Expected to revert | ||
} | ||
} | ||
|
||
function fromBytes32FormatWrapper( | ||
bytes32 bytes32FormatAddress | ||
) external pure returns (address) { | ||
return fromBytes32Format(bytes32FormatAddress); | ||
} | ||
|
||
function testFromBytes32FormatWithMaxValidAddress() public pure { | ||
address maxAddr = address(0xFFfFfFffFFfffFFfFFfFFFFFffFFFffffFfFFFfF); | ||
bytes32 bytes32Format = toBytes32Format(maxAddr); | ||
|
||
address convertedAddr = fromBytes32Format(bytes32Format); | ||
|
||
assertEq(convertedAddr, maxAddr, "Max address should convert correctly"); | ||
} | ||
|
||
function testConvertersRoundTrip() public pure { | ||
address originalAddr = address(0xabCDEF1234567890ABcDEF1234567890aBCDeF12); | ||
|
||
bytes32 bytes32Format = toBytes32Format(originalAddr); | ||
address convertedAddr = fromBytes32Format(bytes32Format); | ||
|
||
assertEq(convertedAddr, originalAddr, "Round trip conversion should work"); | ||
} | ||
|
||
function testConvertersFuzz(address addr) public pure { | ||
// Skip addresses that would cause overflow | ||
vm.assume(uint256(uint160(addr)) <= type(uint160).max); | ||
|
||
bytes32 bytes32Format = toBytes32Format(addr); | ||
address convertedAddr = fromBytes32Format(bytes32Format); | ||
|
||
assertEq(convertedAddr, addr, "Fuzz test should pass"); | ||
} | ||
} |
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.
Remove the pure
modifiers. Every test method is declared pure
, yet they call assertEq
and even vm.assume
. The compiler rejects that because those helpers are not pure. This should drop the pure
modifiers (and any view
equivalents) so the test contract compiles.
🤖 Prompt for AI Agents
In test/Utils.t.sol around lines 13 to 149, many test functions are incorrectly
declared pure (and some may be view) while they call test helpers like assertEq,
vm.assume, and external wrappers; remove the pure (and any view) modifiers from
all test functions and any helper wrappers in this range so they are ordinary
non-pure functions, ensuring the tests can call the stateful/assertion helpers
and compile.
"alwaysStrict": false, | ||
"forceConsistentCasingInFileNames": true, | ||
"removeComments": true, | ||
"noImplicitAny": true, | ||
"strictNullChecks": true, | ||
"strictFunctionTypes": true, | ||
"noImplicitAny": false, | ||
"strictNullChecks": false, | ||
"strictFunctionTypes": false, | ||
"noImplicitThis": true, | ||
"noUnusedLocals": false, | ||
"noUnusedParameters": false, | ||
"noImplicitReturns": true, | ||
"strict": true, | ||
"strict": false, | ||
"noFallthroughCasesInSwitch": true, |
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.
Restore strict TypeScript checks.
Turning off alwaysStrict
, noImplicitAny
, strictNullChecks
, strictFunctionTypes
, and strict
collapses the safety net that previously blocked unsound code. This should stay enabled; otherwise the compiler will stop catching implicit any
s, null/undefined hazards, and contravariant function issues across the overhaul you just landed. Revert these to true
and instead fix or suppress the specific violations (e.g., targeted tsconfig
overrides, @ts-expect-error
, or better typings).
🤖 Prompt for AI Agents
In tsconfig.json around lines 15 to 26, several strict TypeScript checks were
disabled; restore "alwaysStrict", "noImplicitAny", "strictNullChecks",
"strictFunctionTypes", and "strict" to true and re-run the TypeScript build to
surface violations; then address failures by fixing types or narrowing code,
adding targeted overrides (e.g., project/tsconfig extends or per-folder
tsconfig), using precise types or @ts-expect-error only where truly necessary,
and improving typings rather than leaving the global strict flags off.
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores