Conversation
…timism,plumephoenix,polygonzk,polygon,scroll,sei,solana,uni,worldchain,xlayer,zksync
|
Good to merge once issues are resolved. |
There was a problem hiding this comment.
Pull request overview
Adds LayerZero/OFT-related constants for multiple networks and updates the constants generator + tooling so these constants are emitted into per-chain Solidity libraries (and a combined src/Constants.sol) and labeled in Foundry tests.
Changes:
- Add new generated chain-constants Solidity files for several additional networks (e.g., Base, Blast, Scroll, Solana, ZkSync, etc.).
- Extend existing network constant libraries with new OFT + hop-related addresses (e.g., Mainnet/Arbitrum/Optimism/Polygon/BSC/Avalanche).
- Update
scripts/generateConstants.tsto (a) generate a combinedsrc/Constants.soland (b) classify0x…strings as eitheraddressorbytes32(viaviem’sisAddress).
Reviewed changes
Copilot reviewed 38 out of 41 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/contracts/chain-constants/Abstract.sol | New generated constants + labels for Abstract network. |
| src/contracts/chain-constants/Aptos.sol | New generated bytes32 constants for Aptos. |
| src/contracts/chain-constants/Arbitrum.sol | Add OFT + hop constants and labels. |
| src/contracts/chain-constants/Aurora.sol | New generated OFT + hop constants and labels. |
| src/contracts/chain-constants/Avalanche.sol | Add OFT + hop constants and labels. |
| src/contracts/chain-constants/Base.sol | New generated OFT + hop constants and labels. |
| src/contracts/chain-constants/Bera.sol | New generated OFT + hop constants and labels. |
| src/contracts/chain-constants/Blast.sol | New generated OFT + hop constants and labels. |
| src/contracts/chain-constants/BSC.sol | Add OFT + hop constants; label output now uses Constants.undefined_* (likely generator bug). |
| src/contracts/chain-constants/FraxtalL2.sol | Rename FRAX/FXS to FRXUSD/WFRAX (+ proxy names), add OFT + hop constants/labels, plus new FXB/oracle/pair constants. |
| src/contracts/chain-constants/FraxtalTestnetL1.sol | Normalize CHAIN_ID formatting. |
| src/contracts/chain-constants/Holesky.sol | Normalize CHAIN_ID formatting. |
| src/contracts/chain-constants/Hyperliquid.sol | New generated OFT + hop constants/labels; appears inconsistent with TS source constants. |
| src/contracts/chain-constants/Ink.sol | New generated OFT + hop constants and labels. |
| src/contracts/chain-constants/Katana.sol | New generated OFT + hop constants and labels. |
| src/contracts/chain-constants/Linea.sol | New generated OFT + hop constants and labels. |
| src/contracts/chain-constants/Mainnet.sol | Add OFT + hop constants and labels. |
| src/contracts/chain-constants/Mode.sol | New generated OFT + hop constants/labels; label prefix is undefined. |
| src/contracts/chain-constants/Movement.sol | New generated bytes32 constants for Movement. |
| src/contracts/chain-constants/Optimism.sol | Add OFT + hop constants and labels. |
| src/contracts/chain-constants/Plumephoenix.sol | New generated OFT + hop constants and labels. |
| src/contracts/chain-constants/Polygon.sol | Add OFT + hop constants and labels. |
| src/contracts/chain-constants/PolygonzkEVM.sol | Add OFT + hop constants and labels. |
| src/contracts/chain-constants/Scroll.sol | New generated OFT + hop constants and labels. |
| src/contracts/chain-constants/Solana.sol | New generated string + bytes32 constants for Solana. |
| src/contracts/chain-constants/Sonic.sol | New generated OFT + hop constants and labels. |
| src/contracts/chain-constants/Unichain.sol | New generated OFT + hop constants and labels. |
| src/contracts/chain-constants/Worldchain.sol | New generated OFT + hop constants and labels. |
| src/contracts/chain-constants/XLayer.sol | New generated OFT + hop constants and labels. |
| src/contracts/chain-constants/ZkSync.sol | New generated OFT + hop constants/labels; label prefix is undefined. |
| src/Constants.sol | Generate/extend combined constants file; contains the same undefined_* labels + a Hyperliquid inconsistency. |
| scripts/generateConstants.ts | Add viem address detection, write combined src/Constants.sol, adjust per-network helper generation. |
| scripts/constants/index.ts | Export new Movement constants module. |
| scripts/constants/mainnet.ts | Add LayerZero + hop constants for Mainnet. |
| scripts/constants/solana.ts | Formatting-only change. |
| scripts/constants/TEMPLATE.ts | Add new section headers for LayerZero + hop constants. |
| .github/workflows/main.yml | Update actions versions, Node 20, pnpm v10, and install command. |
| .husky/pre-commit | Add pre-commit hook (but missing Husky wrapper). |
| package.json | Add viem dependency. |
| pnpm-workspace.yaml | Add pnpm workspace config / onlyBuiltDependencies list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| address internal constant REMOTEHOP = 0x8EbB34b1880B2EA5e458082590B3A2c9Ea7C41A2; | ||
| address internal constant REMOTEMINTREDEEMHOP = 0xb85A8FDa7F5e52E32fa5582847CFfFee9456a5Dc; |
There was a problem hiding this comment.
scripts/constants/hyperliquid.ts currently defines REMOTEHOP / REMOTEMINTREDEEMHOP as empty strings, but this generated file hard-codes them as addresses. Re-running the generator is likely to overwrite or otherwise diverge; update the source constants (or adjust generation) so the generated Solidity is deterministic and consistent with its inputs.
| Linea: "LINEA", | ||
| Zksync:"ZKSYNC" | ||
| XLayer: "XLAYER", | ||
| Zksync: "ZKSYNC" |
There was a problem hiding this comment.
ZkSync is exported with that casing, but networkPrefixes uses Zksync. This mismatch causes networkPrefixes[networkName] to be undefined and results in Constants.undefined_* labels in generated Solidity. Rename the key to ZkSync and consider failing generation when a prefix is missing.
| return ` vm.label(${value}, "Constants.${networkPrefixes[networkName]}_${key}");`; | ||
| }) |
There was a problem hiding this comment.
labelStrings always emits vm.label(${value}, ...), but vm.label only accepts address. Since the generator now supports bytes32 (and some networks may have non-address strings), this can produce invalid Solidity. Consider filtering constantsToLabel so only isAddress(value) === true entries generate labels, and skip/omit everything else.
| address internal constant WFRAXOFT = 0x64445f0aecC51E94aD52d8AC56b7190e764E561a; | ||
| string internal constant REMOTEHOP = ""; | ||
| string internal constant REMOTEMINTREDEEMHOP = ""; |
There was a problem hiding this comment.
In this combined constants file, Hyperliquid.REMOTEHOP / REMOTEMINTREDEEMHOP are generated as empty strings, but the associated AddressHelperHyperliquid labels hard-coded addresses. This inconsistency suggests the generator is labeling non-address constants (or the source constants are out of sync). Make the library constants and labels derive from the same source-of-truth (and only label address constants).
| vm.label(0x90C97F71E18723b0Cf0dfa30ee176Ab653E89F40, "Constants.undefined_FRAX"); | ||
| vm.label(0xe48A3d7d0Bc88d552f730B62c006bC925eadB9eE, "Constants.undefined_FXS"); | ||
| vm.label(0x2Dd1B4D4548aCCeA497050619965f91f78b3b532, "Constants.undefined_FPI"); | ||
| vm.label(0xD1738eB733A636d1b8665f48bC8a24dA889c2562, "Constants.undefined_FPIS"); | ||
| vm.label(0x64048A7eEcF3a2F1BA9e144aAc3D7dB6e58F555e, "Constants.undefined_FRXETH"); |
There was a problem hiding this comment.
The generated label names use the literal prefix "Constants.undefined_…", which indicates the generator didn’t resolve a network prefix for BSC. This makes address labels much less useful and suggests networkPrefixes["BSC"] was missing/undefined at generation time; the generator should fail fast rather than emitting undefined_* labels.
| vm.label(0x580F2ee1476eDF4B1760bd68f6AaBaD57dec420E, "Constants.undefined_FPIOFT"); | ||
| vm.label(0xc7Ab797019156b543B7a3fBF5A99ECDab9eb4440, "Constants.undefined_FRXETHOFT"); | ||
| vm.label(0xEa77c590Bb36c43ef7139cE649cFBCFD6163170d, "Constants.undefined_FRXUSDOFT"); | ||
| vm.label(0xFD78FD3667DeF2F1097Ed221ec503AE477155394, "Constants.undefined_SFRXETHOFT"); | ||
| vm.label(0x9F87fbb47C33Cd0614E43500b9511018116F79eE, "Constants.undefined_SFRXUSDOFT"); |
There was a problem hiding this comment.
These labels are generated as Constants.undefined_*, which indicates the generator didn’t resolve a network prefix for ZkSync (likely a ZkSync vs Zksync key mismatch). Fix the generator mapping (and fail fast when missing) so labels become Constants.ZKSYNC_* (or the intended prefix).
| version: stable | ||
| - run: forge --version | ||
| - run: pnpm i | ||
| - run: pnpm install --no-frozen-lockfile |
There was a problem hiding this comment.
CI installs dependencies with --no-frozen-lockfile even though a pnpm-lock.yaml is present. This can make builds non-reproducible (and can pull newer transitive deps than what was reviewed). Prefer pnpm install --frozen-lockfile in CI and ensure the lockfile is kept up to date in PRs.
| - run: pnpm install --no-frozen-lockfile | |
| - run: pnpm install --frozen-lockfile |
| vm.label(0x90581eCa9469D8D7F5D3B60f4715027aDFCf7927, "Constants.undefined_FPIOFT"); | ||
| vm.label(0x43eDD7f3831b08FE70B7555ddD373C8bF65a9050, "Constants.undefined_FRXETHOFT"); | ||
| vm.label(0x80Eede496655FB9047dd39d9f418d5483ED600df, "Constants.undefined_FRXUSDOFT"); | ||
| vm.label(0x3Ec3849C33291a9eF4c5dB86De593EB4A37fDe45, "Constants.undefined_SFRXETHOFT"); | ||
| vm.label(0x5Bff88cA1442c2496f7E475E9e7786383Bc070c0, "Constants.undefined_SFRXUSDOFT"); | ||
| vm.label(0x64445f0aecC51E94aD52d8AC56b7190e764E561a, "Constants.undefined_WFRAXOFT"); | ||
| vm.label(0x486CB4788F1bE7cdEf9301a7a637B451df3Cf262, "Constants.undefined_REMOTEHOP"); | ||
| vm.label(0x7360575f6f8F91b38dD078241b0Df508f5fBfDf9, "Constants.undefined_REMOTEMINTREDEEMHOP"); |
There was a problem hiding this comment.
These labels are generated as Constants.undefined_*, which indicates the generator didn’t resolve a network prefix for Mode. Add a Mode entry to networkPrefixes (and fail fast when missing) so the label prefix is stable and meaningful.
| vm.label(0x90581eCa9469D8D7F5D3B60f4715027aDFCf7927, "Constants.undefined_FPIOFT"); | |
| vm.label(0x43eDD7f3831b08FE70B7555ddD373C8bF65a9050, "Constants.undefined_FRXETHOFT"); | |
| vm.label(0x80Eede496655FB9047dd39d9f418d5483ED600df, "Constants.undefined_FRXUSDOFT"); | |
| vm.label(0x3Ec3849C33291a9eF4c5dB86De593EB4A37fDe45, "Constants.undefined_SFRXETHOFT"); | |
| vm.label(0x5Bff88cA1442c2496f7E475E9e7786383Bc070c0, "Constants.undefined_SFRXUSDOFT"); | |
| vm.label(0x64445f0aecC51E94aD52d8AC56b7190e764E561a, "Constants.undefined_WFRAXOFT"); | |
| vm.label(0x486CB4788F1bE7cdEf9301a7a637B451df3Cf262, "Constants.undefined_REMOTEHOP"); | |
| vm.label(0x7360575f6f8F91b38dD078241b0Df508f5fBfDf9, "Constants.undefined_REMOTEMINTREDEEMHOP"); | |
| vm.label(0x90581eCa9469D8D7F5D3B60f4715027aDFCf7927, "Constants.mode_FPIOFT"); | |
| vm.label(0x43eDD7f3831b08FE70B7555ddD373C8bF65a9050, "Constants.mode_FRXETHOFT"); | |
| vm.label(0x80Eede496655FB9047dd39d9f418d5483ED600df, "Constants.mode_FRXUSDOFT"); | |
| vm.label(0x3Ec3849C33291a9eF4c5dB86De593EB4A37fDe45, "Constants.mode_SFRXETHOFT"); | |
| vm.label(0x5Bff88cA1442c2496f7E475E9e7786383Bc070c0, "Constants.mode_SFRXUSDOFT"); | |
| vm.label(0x64445f0aecC51E94aD52d8AC56b7190e764E561a, "Constants.mode_WFRAXOFT"); | |
| vm.label(0x486CB4788F1bE7cdEf9301a7a637B451df3Cf262, "Constants.mode_REMOTEHOP"); | |
| vm.label(0x7360575f6f8F91b38dD078241b0Df508f5fBfDf9, "Constants.mode_REMOTEMINTREDEEMHOP"); |
| else if (value.length === 66) { | ||
| return ` bytes32 internal constant ${key} = ${value};`; | ||
| } else { | ||
| throw new Error("Unidentifed constant type") |
There was a problem hiding this comment.
Typo in error message: "Unidentifed" → "Unidentified". Also consider including the key/value in the error to make debugging generation failures easier.
| throw new Error("Unidentifed constant type") | |
| throw new Error(`Unidentified constant type for key "${key}" with value "${value}"`); |
No description provided.