feat: implement fractional scaling on TokenRouter#7659
feat: implement fractional scaling on TokenRouter#7659yorhodes merged 22 commits intoaudit-q1-2026from
Conversation
🦋 Changeset detectedLatest commit: 7492491 The changes in this PR will be included in the next version bump. This PR includes no changesetsWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
yorhodes
requested review from
Xaroz,
antigremlin,
larryob,
ltyu,
paulbalaji and
xeno097
as code owners
📝 WalkthroughWalkthroughTokenRouter's scaling mechanism shifts from a single scale value to fractional scaling using numerator and denominator parameters. This change cascades through token contracts, constructors, test suites, and SDK utilities for reading/deploying scaled tokens. Version bumped to 11.0.0. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant TokenRouter
participant Math Library
Client->>TokenRouter: OutboundTransfer(localAmount)
TokenRouter->>TokenRouter: Validate scaleNumerator & scaleDenominator > 0
TokenRouter->>Math Library: mulDiv(localAmount, scaleNumerator, scaleDenominator, rounding)
Math Library-->>TokenRouter: scaledOutboundAmount
TokenRouter-->>Client: Return scaledOutboundAmount
Client->>TokenRouter: InboundTransfer(messageAmount)
TokenRouter->>TokenRouter: Validate scaleNumerator & scaleDenominator > 0
TokenRouter->>Math Library: mulDiv(messageAmount, scaleDenominator, scaleNumerator, rounding)
Math Library-->>TokenRouter: scaledInboundAmount
TokenRouter-->>Client: Return scaledInboundAmount
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
solidity/test/token/HypERC4626Test.t.sol (1)
70-101: Test constructor calls updated correctly, but fractional scaling lacks test coverage.The
SCALE, SCALE(1:1 ratio) setup works fine for these tests—keeps things as they were. However, the code supports different numerator and denominator values for fractional scaling, and there are no test cases exercising those ratios. Add tests with non-equal scale values (e.g., 2:1, 1:2) to cover the actual scaling math and edge cases the implementation supports.
🧹 Nitpick comments (2)
typescript/sdk/src/token/types.ts (1)
41-49: Consider validating numerator as well.The denominator has a
.gt(0)constraint which is good for preventing division by zero. Should the numerator also be validated to be positive? A numerator of 0 or negative values would result in unusual scaling behavior.💡 Optional enhancement for numerator validation
scale: z .union([ z.number(), z.object({ - numerator: z.number(), + numerator: z.number().gt(0), denominator: z.number().gt(0), }), ]) .optional(),typescript/sdk/src/token/EvmERC20WarpRouteReader.ts (1)
1082-1132: Solid implementation of version-aware scale fetching.This is a right proper piece of work here. The branching logic based on contract version ensures backward compatibility with older deployments while supportin' the new fractional scaling. A few observations, if ye don't mind:
The return type includes
undefinedbut the function always returns a value. Consider removingundefinedfrom the return type unless there's a case where it should return undefined.Using
toNumber()on BigNumber could technically overflow for values >Number.MAX_SAFE_INTEGER, though scale values in practice should be small. If ye want to be extra careful, consider keeping them as strings or usingBigInt.🔎 Optional: Clarify return type
If
undefinedisn't a valid return value, consider updating the return type:async fetchScale( tokenRouterAddress: Address, - ): Promise<number | { numerator: number; denominator: number } | undefined> { + ): Promise<number | { numerator: number; denominator: number }> {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
.changeset/pre.json(1 hunks).changeset/spotty-adults-dress.md(1 hunks)solidity/CHANGELOG.md(1 hunks)solidity/contracts/PackageVersioned.sol(1 hunks)solidity/contracts/test/TestLpCollateralRouter.sol(1 hunks)solidity/contracts/test/TestLpTokenRouter.sol(1 hunks)solidity/contracts/token/HypERC20.sol(1 hunks)solidity/contracts/token/HypERC20Collateral.sol(1 hunks)solidity/contracts/token/HypERC721.sol(1 hunks)solidity/contracts/token/HypERC721Collateral.sol(1 hunks)solidity/contracts/token/HypNative.sol(1 hunks)solidity/contracts/token/TokenBridgeCctpBase.sol(1 hunks)solidity/contracts/token/bridge/EverclearTokenBridge.sol(3 hunks)solidity/contracts/token/extensions/HypERC4626.sol(1 hunks)solidity/contracts/token/extensions/HypERC4626Collateral.sol(1 hunks)solidity/contracts/token/extensions/HypERC4626OwnerCollateral.sol(1 hunks)solidity/contracts/token/extensions/HypFiatToken.sol(1 hunks)solidity/contracts/token/extensions/HypXERC20.sol(1 hunks)solidity/contracts/token/extensions/HypXERC20Lockbox.sol(1 hunks)solidity/contracts/token/extensions/OPL2ToL1TokenBridgeNative.sol(3 hunks)solidity/contracts/token/libs/TokenRouter.sol(3 hunks)solidity/core-utils/index.ts(1 hunks)solidity/package.json(1 hunks)solidity/script/EverclearTokenBridge.s.sol(1 hunks)solidity/script/xerc20/ApproveLockbox.s.sol(1 hunks)solidity/script/xerc20/ezETH.s.sol(2 hunks)solidity/test/AmountRouting.t.sol(2 hunks)solidity/test/hooks/RateLimitedHook.t.sol(2 hunks)solidity/test/token/EverclearTokenBridge.t.sol(4 hunks)solidity/test/token/HypERC20.t.sol(9 hunks)solidity/test/token/HypERC20CollateralVaultDeposit.t.sol(2 hunks)solidity/test/token/HypERC20MovableCollateral.t.sol(1 hunks)solidity/test/token/HypERC4626Test.t.sol(2 hunks)solidity/test/token/HypNativeLp.t.sol(1 hunks)solidity/test/token/HypnativeMovable.t.sol(1 hunks)solidity/test/token/LpCollateralRouter.t.sol(1 hunks)solidity/test/token/MovableCollateralRouter.t.sol(1 hunks)solidity/test/token/WHypERC4626.t.sol(1 hunks)typescript/sdk/src/token/EvmERC20WarpModule.hardhat-test.ts(2 hunks)typescript/sdk/src/token/EvmERC20WarpRouteReader.hardhat-test.ts(5 hunks)typescript/sdk/src/token/EvmERC20WarpRouteReader.ts(11 hunks)typescript/sdk/src/token/TokenMetadataMap.ts(1 hunks)typescript/sdk/src/token/deploy.ts(2 hunks)typescript/sdk/src/token/types.ts(1 hunks)typescript/sdk/src/utils/decimals.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
typescript/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
typescript/**/*.{ts,tsx}: UseChainMapfor per-chain configurations in TypeScript
Import types from@hyperlane-xyz/sdkwhen using TypeScript SDK types
Files:
typescript/sdk/src/token/EvmERC20WarpRouteReader.hardhat-test.tstypescript/sdk/src/token/EvmERC20WarpModule.hardhat-test.tstypescript/sdk/src/utils/decimals.tstypescript/sdk/src/token/types.tstypescript/sdk/src/token/TokenMetadataMap.tstypescript/sdk/src/token/deploy.tstypescript/sdk/src/token/EvmERC20WarpRouteReader.ts
solidity/**/*.sol
📄 CodeRabbit inference engine (CLAUDE.md)
solidity/**/*.sol: UseonlyOwneror appropriate access modifiers on privileged functions in Solidity
Validate all external inputs at system boundaries in Solidity contracts
Ensure backward compatibility for protocol upgrades in Solidity
Optimize for gas efficiency in Solidity by avoiding unnecessary storage writes
Check the contract inheritance hierarchy before implementing new Solidity contracts
Files:
solidity/contracts/PackageVersioned.solsolidity/contracts/token/extensions/OPL2ToL1TokenBridgeNative.solsolidity/script/xerc20/ApproveLockbox.s.solsolidity/contracts/token/extensions/HypXERC20.solsolidity/test/token/MovableCollateralRouter.t.solsolidity/contracts/token/HypERC20Collateral.solsolidity/contracts/token/HypNative.solsolidity/contracts/token/HypERC721Collateral.solsolidity/contracts/token/extensions/HypXERC20Lockbox.solsolidity/test/token/HypERC4626Test.t.solsolidity/test/token/HypNativeLp.t.solsolidity/contracts/test/TestLpTokenRouter.solsolidity/test/token/WHypERC4626.t.solsolidity/test/hooks/RateLimitedHook.t.solsolidity/contracts/token/libs/TokenRouter.solsolidity/script/xerc20/ezETH.s.solsolidity/contracts/token/extensions/HypERC4626OwnerCollateral.solsolidity/test/token/HypERC20CollateralVaultDeposit.t.solsolidity/contracts/token/extensions/HypERC4626.solsolidity/test/token/HypnativeMovable.t.solsolidity/contracts/token/bridge/EverclearTokenBridge.solsolidity/test/token/HypERC20.t.solsolidity/script/EverclearTokenBridge.s.solsolidity/contracts/token/extensions/HypFiatToken.solsolidity/contracts/token/HypERC20.solsolidity/test/AmountRouting.t.solsolidity/contracts/test/TestLpCollateralRouter.solsolidity/contracts/token/extensions/HypERC4626Collateral.solsolidity/contracts/token/HypERC721.solsolidity/contracts/token/TokenBridgeCctpBase.solsolidity/test/token/LpCollateralRouter.t.solsolidity/test/token/EverclearTokenBridge.t.solsolidity/test/token/HypERC20MovableCollateral.t.sol
🧠 Learnings (15)
📓 Common learnings
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7253
File: typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts:285-289
Timestamp: 2025-10-24T18:58:44.557Z
Learning: In the Hyperlane funding script (typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts), tokens with 0 decimals are not valid and should be rejected. The truthy check on decimals is intentional and correct for this use case.
📚 Learning: 2025-11-26T13:28:51.658Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7410
File: typescript/cli/package.json:20-20
Timestamp: 2025-11-26T13:28:51.658Z
Learning: In the hyperlane-xyz/hyperlane-monorepo repository, hyperlane-xyz/registry is maintained in a separate repository and published to npm, so it should use a pinned version (e.g., "23.6.0") rather than the workspace protocol ("workspace:*") that other internal Hyperlane packages use.
Applied to files:
solidity/package.json
📚 Learning: 2025-12-18T22:36:13.376Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-18T22:36:13.376Z
Learning: Chain metadata lives in external `hyperlane-xyz/registry` package, not in the main repository
Applied to files:
solidity/package.json
📚 Learning: 2025-09-05T15:13:08.394Z
Learnt from: ltyu
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6992
File: typescript/sdk/src/token/types.test.ts:188-197
Timestamp: 2025-09-05T15:13:08.394Z
Learning: WarpRouteDeployConfigSchema.safeParse automatically populates missing token fields during preprocessing, so manual specification of top-level token fields in RoutingFee test cases is not necessary.
Applied to files:
typescript/sdk/src/token/EvmERC20WarpRouteReader.hardhat-test.tstypescript/sdk/src/token/EvmERC20WarpRouteReader.tssolidity/test/AmountRouting.t.sol
📚 Learning: 2025-10-24T18:58:44.557Z
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7253
File: typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts:285-289
Timestamp: 2025-10-24T18:58:44.557Z
Learning: In the Hyperlane funding script (typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts), tokens with 0 decimals are not valid and should be rejected. The truthy check on decimals is intentional and correct for this use case.
Applied to files:
typescript/sdk/src/token/EvmERC20WarpRouteReader.hardhat-test.tstypescript/sdk/src/utils/decimals.tssolidity/test/token/HypERC4626Test.t.solsolidity/test/token/HypERC20CollateralVaultDeposit.t.solsolidity/test/token/HypERC20.t.soltypescript/sdk/src/token/deploy.tstypescript/sdk/src/token/EvmERC20WarpRouteReader.tssolidity/test/AmountRouting.t.sol
📚 Learning: 2025-12-18T22:36:13.376Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-18T22:36:13.376Z
Learning: Applies to solidity/**/*.sol : Ensure backward compatibility for protocol upgrades in Solidity
Applied to files:
solidity/contracts/PackageVersioned.soltypescript/sdk/src/token/EvmERC20WarpModule.hardhat-test.tssolidity/core-utils/index.tssolidity/CHANGELOG.md
📚 Learning: 2025-12-16T17:23:10.420Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7627
File: typescript/infra/config/environments/mainnet3/gasPrices.json:10-13
Timestamp: 2025-12-16T17:23:10.420Z
Learning: In typescript/infra/config/environments/mainnet3/gasPrices.json, the "decimals" field represents operational precision for gas price calculations, which may differ from the standard token decimal precision defined in token registries. These are different types of precisions serving different purposes.
Applied to files:
typescript/sdk/src/utils/decimals.ts
📚 Learning: 2025-12-18T22:36:13.376Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-18T22:36:13.376Z
Learning: Applies to typescript/**/*.{ts,tsx} : Use `ChainMap` for per-chain configurations in TypeScript
Applied to files:
typescript/sdk/src/utils/decimals.ts
📚 Learning: 2025-10-08T12:02:18.956Z
Learnt from: troykessler
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7141
File: typescript/sdk/src/token/altVMDeploy.ts:118-133
Timestamp: 2025-10-08T12:02:18.956Z
Learning: In AltVM deployer (typescript/sdk/src/token/altVMDeploy.ts), synthetic token creation via `createSyntheticToken` accepts empty strings for `name` and `denom`, and zero for `decimals` without causing validation failures. These placeholder values are valid across AltVM protocols, as different protocols have different metadata requirements.
Applied to files:
typescript/sdk/src/utils/decimals.tstypescript/sdk/src/token/deploy.tstypescript/sdk/src/token/EvmERC20WarpRouteReader.ts
📚 Learning: 2025-11-27T16:18:06.108Z
Learnt from: troykessler
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7481
File: typescript/aleo-sdk/src/clients/signer.ts:127-136
Timestamp: 2025-11-27T16:18:06.108Z
Learning: In typescript/aleo-sdk/src/clients/signer.ts, the createMailbox method calls deployProgram('dispatch_proxy', mailboxSalt) which is guaranteed to return at least 3 programs. Therefore, accessing programs[programs.length - 3] and programs[programs.length - 1] is safe and does not require additional length validation.
Applied to files:
solidity/script/xerc20/ApproveLockbox.s.sol
📚 Learning: 2025-10-28T15:45:08.081Z
Learnt from: yorhodes
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7262
File: solidity/contracts/token/HypNative.sol:76-77
Timestamp: 2025-10-28T15:45:08.081Z
Learning: In HypNative (solidity/contracts/token/HypNative.sol), the contract holds two separate pools of assets: (1) LP assets tracked in `lpAssets` which are explicitly deposited by LPs or donated by fee recipients and are withdrawable by vault share holders, and (2) user collateral deposits which are held for cross-chain bridging operations and are NOT owned by LPs or tracked in `lpAssets`. The contract balance will naturally be higher than `lpAssets` by design.
Applied to files:
solidity/test/token/HypERC4626Test.t.solsolidity/test/token/HypNativeLp.t.solsolidity/test/token/HypERC20CollateralVaultDeposit.t.sol
📚 Learning: 2025-10-28T15:45:08.081Z
Learnt from: yorhodes
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7262
File: solidity/contracts/token/HypNative.sol:76-77
Timestamp: 2025-10-28T15:45:08.081Z
Learning: In HypNative (solidity/contracts/token/HypNative.sol), when native tokens are received via the `receive()` function during rebalancing, they represent user collateral returning from cross-chain operations, not LP contributions. Therefore, `receive()` should NOT call `donate()` as that would incorrectly credit returning user collateral to the LPs and inflate their share value.
Applied to files:
solidity/test/token/HypNativeLp.t.sol
📚 Learning: 2025-09-15T17:15:41.014Z
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7034
File: typescript/infra/config/environments/mainnet3/rebalancer/USDC/pulsechain.yaml:0-0
Timestamp: 2025-09-15T17:15:41.014Z
Learning: Bridge addresses for USDC warp routes in pulsechain.yaml configurations should reference the canonical addresses from the hyperlane-registry repo's mainnet-cctp-config.yaml file rather than being generated independently.
Applied to files:
solidity/test/hooks/RateLimitedHook.t.sol
📚 Learning: 2025-09-18T15:49:20.478Z
Learnt from: yorhodes
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6750
File: solidity/contracts/token/bridge/EverclearTokenBridge.sol:212-212
Timestamp: 2025-09-18T15:49:20.478Z
Learning: EverclearTokenBridge sends funds directly to recipients for ERC20 transfers, while EverclearEthBridge sends funds to remote routers that handle WETH unwrapping and ETH delivery. The virtual _getReceiver and _getIntentCalldata functions enable this architectural difference between the two bridge implementations.
Applied to files:
solidity/contracts/token/bridge/EverclearTokenBridge.solsolidity/script/EverclearTokenBridge.s.solsolidity/test/token/EverclearTokenBridge.t.sol
📚 Learning: 2025-10-07T13:54:32.514Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7168
File: typescript/infra/scripts/helloworld/kathy.ts:410-414
Timestamp: 2025-10-07T13:54:32.514Z
Learning: In typescript/infra, the chainMetadataTypes.ts schema enforces that bech32Prefix is required for all Cosmos and CosmosNative protocol chains through a Zod refinement check, so app.metadata(origin).bech32Prefix will always be defined for these protocols.
Applied to files:
typescript/sdk/src/token/EvmERC20WarpRouteReader.ts
🧬 Code graph analysis (2)
typescript/sdk/src/token/EvmERC20WarpRouteReader.hardhat-test.ts (2)
typescript/sdk/src/token/types.ts (1)
WarpRouteDeployConfigMailboxRequired(566-568)typescript/sdk/src/token/config.ts (2)
TokenType(1-20)TokenType(22-22)
typescript/sdk/src/token/deploy.ts (1)
typescript/sdk/src/token/types.ts (7)
isCollateralTokenConfig(143-143)isXERC20TokenConfig(198-198)isEverclearCollateralTokenConfig(255-257)isOpL2TokenConfig(122-122)isOpL1TokenConfig(119-119)isSyntheticTokenConfig(232-232)isSyntheticRebaseTokenConfig(242-244)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (67)
- GitHub Check: cli-evm-e2e-matrix (core-init)
- GitHub Check: cli-evm-e2e-matrix (warp-send)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-1)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-rebalancing-config)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-hook-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-check-3)
- GitHub Check: cli-evm-e2e-matrix (warp-check-4)
- GitHub Check: cli-evm-e2e-matrix (warp-check-1)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-simple-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-check-2)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-2)
- GitHub Check: cli-evm-e2e-matrix (core-read)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
- GitHub Check: cli-evm-e2e-matrix (relay)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-basic)
- GitHub Check: cli-evm-e2e-matrix (core-deploy)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-evm-e2e-matrix (core-apply)
- GitHub Check: cli-evm-e2e-matrix (status)
- GitHub Check: cli-evm-e2e-matrix (warp-rebalancer)
- GitHub Check: cli-evm-e2e-matrix (warp-init)
- GitHub Check: cli-evm-e2e-matrix (warp-read)
- GitHub Check: cli-evm-e2e-matrix (core-check)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-config)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-submitters)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
- GitHub Check: cli-evm-e2e-matrix (warp-check-5)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ism-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-2)
- GitHub Check: cli-radix-e2e-matrix (core-apply)
- GitHub Check: env-test-matrix (mainnet3, optimism, core)
- GitHub Check: env-test-matrix (testnet4, sepolia, core)
- GitHub Check: cli-cosmos-e2e-matrix (warp-read)
- GitHub Check: env-test-matrix (mainnet3, ethereum, core)
- GitHub Check: cli-radix-e2e-matrix (warp-deploy)
- GitHub Check: env-test-matrix (mainnet3, optimism, igp)
- GitHub Check: env-test-matrix (mainnet3, ethereum, igp)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-route-extension)
- GitHub Check: cli-radix-e2e-matrix (core-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-install-test-run
- GitHub Check: cli-cosmos-e2e-matrix (core-read)
- GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
- GitHub Check: cli-cosmos-e2e-matrix (core-apply)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (core-check)
- GitHub Check: aleo-sdk-e2e-run
- GitHub Check: pnpm-test-run
- GitHub Check: cosmos-sdk-e2e-run
- GitHub Check: coverage-run
- GitHub Check: Cursor Bugbot
- GitHub Check: infra-test
- GitHub Check: build-and-push-to-gcr
- GitHub Check: e2e-matrix (evm)
- GitHub Check: agent-configs (testnet4)
- GitHub Check: fork-tests
- GitHub Check: agent-configs (mainnet3)
- GitHub Check: diff-check
- GitHub Check: diff-check
- GitHub Check: lander-coverage
- GitHub Check: lint-rs
- GitHub Check: test-rs
- GitHub Check: slither
🔇 Additional comments (65)
.changeset/spotty-adults-dress.md (1)
1-5: Changeset looks good.Properly documenting the major version bump for the fractional scaling feature. The description captures the essence of what's changing.
.changeset/pre.json (1)
1-11: Pre-release config is set up correctly.Beta tagging for the major version change is the right move here.
typescript/sdk/src/token/EvmERC20WarpModule.hardhat-test.ts (1)
1679-1695: Scale stub handles backward compatibility well.For contracts predating the fractional scale feature, defaulting to scale=1 makes sense. The stub is properly restored alongside the version stub, keeping things clean.
typescript/sdk/src/token/TokenMetadataMap.ts (1)
47-51: Return type properly extended for fractional scales.The method now accurately reflects that scale can be either a plain number or a structured fraction. Nice and straightforward.
typescript/sdk/src/token/EvmERC20WarpRouteReader.hardhat-test.ts (3)
254-293: Well-structured test for fractional scale derivation.The test properly validates the round-trip of scale configuration: deploy with numerator/denominator, then verify deriveWarpRouteConfig returns the same structured scale. The 10^12 denominator for 18→6 decimal conversion is spot on.
959-976: Stub properly handles legacy contract scenario.Returning scale=1 for contracts below the token fee version threshold prevents mismatches during config derivation. Good cleanup with the restore.
1094-1116: Backward compatibility stubs are consistent.The fetchScale stubs across these legacy contract tests ensure smooth config derivation without version-related issues.
solidity/contracts/token/extensions/HypFiatToken.sol (1)
19-27: Constructor signature updated for fractional scaling.The shift from a single
_scaleparameter to_scaleNumeratorand_scaleDenominatoraligns with the TokenRouter changes. This is a breaking change, but that's the point of the major version bump.typescript/sdk/src/token/deploy.ts (2)
154-156: Conversion logic handles both scale formats correctly.The ternary cleanly converts a numeric scale to
{ numerator: scale, denominator: 1 }while passing through structured scales as-is. This maintains backward compatibility while supporting the new fractional representation.
158-193: Constructor argument updates are consistent across token types.All relevant token types now receive numerator and denominator as separate parameters instead of a single scale. The ordering is consistent: for collateral types it's
[token, numerator, denominator, mailbox, ...], for native it's[numerator, denominator, mailbox], and for synthetic it's[decimals, numerator, denominator, mailbox, ...].typescript/sdk/src/utils/decimals.ts (3)
16-26: Type expansion for fractional scale looks good.The updated type signature properly handles the new fractional scale representation comin' from the broader PR changes. This keeps things consistent with how the Solidity side now represents scale as numerator/denominator.
56-60: Type signature update is consistent.The
areDecimalsUniformfunction's type update mirrors the changes in the main function - keeps everything tidy like a well-organized swamp.
36-49: Floating-point precision risk with fractional scale comparison.When
config.scaleis a fraction, line 44 divides the numerator by denominator to get a scalar value. This can produce inexact floating-point results—for instance,{numerator: 1, denominator: 3}becomes 0.333..., which will never equal an integer power of 10 from line 35. Line 47's strict equality check (!==) then risks false negatives.If fractional scales are a supported use case, consider either using approximate comparison (with epsilon tolerance) or keeping scales as pre-computed integers rather than fractions requiring division.
solidity/contracts/token/extensions/OPL2ToL1TokenBridgeNative.sol (3)
38-44: Constructor update for fractional scaling looks proper.Aye, this is exactly how it should be done. Passin'
SCALE, SCALE(which is1, 1) to the TokenRouter base means the effective scale stays at 1 - no fancy math tricks needed for native token transfers. The change maintains backward compatibility while alignin' with the new fractional scaling interface.
265-269: OpL1V1NativeTokenBridge constructor properly updated.Same story here - the base initialization with
(SCALE, SCALE, _mailbox)keeps things consistent across all the native bridge variants.
275-279: OpL1V2NativeTokenBridge constructor properly updated.All three native bridge constructors now follow the same pattern - nice and consistent, just how I like me swamp organized.
typescript/sdk/src/token/EvmERC20WarpRouteReader.ts (7)
690-702: Scale integration in deriveHypXERC20TokenConfig looks good.Nice and clean - fetchin' the scale alongside the other metadata and includin' it in the returned config. Keeps things organized.
713-727: Scale integration in deriveHypXERC20LockboxTokenConfig properly implemented.Same pattern applied consistently - that's the way to do it.
795-805: Scale integration in deriveHypCollateralTokenConfig looks correct.The collateral config now properly includes the scale field.
855-864: Scale integration in deriveHypSyntheticTokenConfig properly implemented.Synthetic tokens gettin' their scale too - all's right in the swamp.
867-888: deriveHypNativeTokenConfig updated correctly.Good job renamin' the parameter from
_addresstotokenRouterAddressfor clarity, and properly fetchin' and includin' the scale in the native token config.
939-953: Scale integration in deriveHypSyntheticRebaseConfig looks proper.Rebase configs now carry their scale - consistent with the rest.
1050-1068: Scale integration in deriveEverclearCollateralTokenBridgeConfig properly implemented.The Everclear bridge configs are gettin' their scale too. Everything's lined up nicely.
solidity/package.json (1)
4-4: Version bump to 11.0.0-beta.0 is appropriate.Aye, this major version bump makes sense. The TokenRouter constructor signature change is a breakin' change, so bumpin' the major version is the right call. The beta tag keeps everyone informed that this is still cookin' in the pot before the final release.
This version matches
SCALE_FRACTION_VERSIONinEvmERC20WarpRouteReader.ts, which is used to determine when to read the new fractional scale interface - nice coordination there.solidity/contracts/token/TokenBridgeCctpBase.sol (1)
99-116: Constructor update for fractional scaling is correct.This change is spot on. The CCTP bridge passes
(_SCALE, _SCALE, _mailbox)which gives an effective scale of 1 - exactly what's needed since CCTP handles its own token transfers without needin' any scaling magic. The comment down on line 176 even confirms this intention with "no scaling needed for CCTP."As per coding guidelines, backward compatibility is maintained since the effective behavior remains unchanged.
solidity/contracts/token/extensions/HypXERC20Lockbox.sol (1)
24-29: Input validation for scale parameters already handled in TokenRouter.The
TokenRouterbase constructor validates that_scaleDenominatorcannot be zero (line 69 ofsolidity/contracts/token/libs/TokenRouter.sol), preventing division issues. The scale parameters are safe as inherited.Likely an incorrect or invalid review comment.
solidity/test/token/HypNativeLp.t.sol (1)
32-32: LGTM - constructor update aligns with fractional scaling.The HypNative constructor call now properly passes both scale components (1, 1), which maintains the previous behavior of scale = 1. Test logic remains sound.
solidity/test/token/HypERC20MovableCollateral.t.sol (1)
24-29: Constructor update looks good.The scale parameters (1e18, 1) correctly maintain the previous scaling behavior. The refactored constructor signature aligns with the fractional scaling model.
solidity/contracts/token/extensions/HypERC4626Collateral.sol (1)
57-65: Fractional scaling integration looks solid.The constructor properly accepts and forwards both scale components to the TokenRouter base. The vault interaction logic remains unchanged, which is appropriate.
solidity/test/token/HypnativeMovable.t.sol (2)
72-72: Test setup updated correctly.The HypNative instances now use the two-parameter scale model (1, 1) which maintains the original scaling behavior while aligning with the new fractional scaling architecture.
79-79: Consistent constructor usage.Second HypNative instance properly initialized with the same scale parameters.
solidity/test/token/LpCollateralRouter.t.sol (1)
20-20: Test fixture initialization updated properly.The scale parameters (1, 1) keep the same scaling semantics as before. All downstream test assertions remain valid.
solidity/contracts/test/TestLpCollateralRouter.sol (1)
8-14: Test helper updated to match new TokenRouter signature.The constructor and base initialization correctly implement the two-parameter scale model. This aligns with the pattern used across the codebase.
solidity/contracts/token/HypERC20.sol (1)
18-25: Constructor signature updated correctly for fractional scaling.The new parameters enable more precise scaling calculations across chains. The base TokenRouter initialization properly forwards both scale components and validates
_scaleDenominator != 0to prevent division by zero in scaling calculations (line 69 of TokenRouter.sol).solidity/test/token/WHypERC4626.t.sol (1)
10-11: LGTM!The constructor call's been updated proper-like to match the new HypERC4626 signature with fractional scaling. Using 1/1 for the scale keeps things neutral, which is exactly what ye want for these test scenarios.
solidity/script/xerc20/ApproveLockbox.s.sol (1)
34-39: LGTM! Backward compatibility preserved nicely.The new logic uses 1/1 for scale parameters, meanin' existing deployments won't see any behavioral changes when upgraded. That's the way to do it - like layers on an onion, each upgrade should build on the last without breakin' what's already there.
solidity/test/AmountRouting.t.sol (2)
71-76: LGTM!Constructor call updated to pass both scale parameters. Usin' the same
SCALEconstant for numerator and denominator keeps things simple for this test - no need to complicate it when ye just need a neutral 1/1 scale.
85-90: Consistent with the local warp route setup.Same pattern applied here, keepin' the test setup nice and symmetrical. Both ends of the bridge use the same neutral scaling.
solidity/contracts/test/TestLpTokenRouter.sol (1)
7-14: LGTM!Test helper's been updated to match the new TokenRouter signature. All the pieces fit together like a good swamp stew - the parameters flow right through to the base class.
solidity/contracts/token/HypNative.sol (1)
22-26: Verify scaleDenominator validation in TokenRouter base contract.The constructor properly passes both numerator and denominator to TokenRouter. Like a good ogre with layers, the scaling logic needs proper foundation—make sure the base contract validates that
_scaleDenominator > 0to prevent any division by zero issues in the scaling calculations.solidity/contracts/token/extensions/HypXERC20.sol (1)
15-23: LGTM! Constructor properly passes the fractional scale to the base.The changes here are like layers of an onion, mate - each one wraps the next properly. The scale parameters flow nicely to the TokenRouter base where the actual validation happens.
solidity/contracts/token/HypERC20Collateral.sol (1)
47-55: LGTM! Constructor updated correctly for fractional scaling.Nothing fancy here - just passing those scale bits along to where they need to go. Keeps things simple, which is how I like it in me swamp.
solidity/contracts/token/extensions/HypERC4626.sol (1)
48-58: LGTM! Constructor changes properly propagate to base HypERC20.This one's got layers, you see. The internal exchange rate business does its own thing with shares and assets, while the base fractional scaling handles cross-chain normalization. Both work together like donkey and me - different but somehow it works.
solidity/contracts/token/extensions/HypERC4626OwnerCollateral.sol (1)
33-45: LGTM! Constructor correctly forwards fractional scale parameters.Clean and simple - just the way things should be done. The scale parameters make their way down to the base properly.
solidity/test/hooks/RateLimitedHook.t.sol (1)
55-79: LGTM! Test setup updated for new constructor signatures.Using
SCALE, SCALE(which is 1, 1) means no actual scaling happens here - just testing the rate limiting behavior without that complexity getting in the way. Smart approach for focused tests.solidity/test/token/HypERC20CollateralVaultDeposit.t.sol (2)
41-46: Test deployment updated correctly.The vault deposit tests keep working as before - just had to adjust for the new constructor signature. Nothing's getting scaled differently here.
400-406: HypERC4626 instantiation updated properly.Good stuff - the metadata test now creates the remote token with the right constructor shape. All five parameters present and accounted for.
solidity/contracts/token/libs/TokenRouter.sol (2)
393-420: Scaling logic is mathematically sound.The outbound and inbound calculations properly invert each other - what you multiply on the way out, you divide on the way in. The documentation's pretty clear about what happens in each case too. Just need that numerator check I mentioned earlier and this is golden.
54-55: Good use of immutable for gas efficiency.Making these immutable means they're baked into the bytecode - saves gas on every read. Following the coding guidelines about optimizing for gas efficiency. As per coding guidelines, this is the right approach.
solidity/contracts/token/bridge/EverclearTokenBridge.sol (3)
368-382: LGTM on the EverclearTokenBridge constructor.The two-scale parameter forwarding is wired up correctly. Clean and straightforward delegation to the base
EverclearBridgeconstructor.
454-472: Nice use of a constant for the 1:1 scale ratio.Using
SCALE = 1for both numerator and denominator is the right approach for ETH bridging where no decimal adjustment is needed. The constant makes the intent crystal clear - what you send is what you get, no funny business with the amounts.
105-122: All good—denominator validation is in place.The constructor properly threads both scale components to the
TokenRouterbase, and the NatSpec comments document what these parameters mean nicely. The base class validates that_scaleDenominatorisn't zero, so you're protected from division by zero if someone misconfigures it. Everything looks sound.solidity/test/token/EverclearTokenBridge.t.sol (4)
90-96: Constructor call updated correctly.The test deployment now passes both scale components. Using
1, 1here means no scaling happens, which keeps these tests focused on the bridge mechanics rather than scale math.
130-137: LGTM.Same pattern, same approval - the signature change is properly reflected.
529-544: MockEverclearTokenBridge constructor properly updated.The mock inherits the new signature and forwards both scale parameters to the parent. This'll keep your tests workin' just fine.
610-617: Fork test deployment updated.The
_deployBridgeoverride correctly uses the new constructor signature with1, 1scale values.solidity/test/token/HypERC20.t.sol (9)
105-110: Base test setup updated correctly.The
remoteTokenimplementation now gets both scale parameters. UsingSCALE, SCALE(both 1) makes sense for the base test fixture.
338-343: LGTM.HypERC20Test deployment follows the new pattern.
448-453: Collateral test updated properly.
473-476: Constructor revert test updated.The invalid token check test now uses the new two-parameter signature. Good to see this edge case is still covered.
526-531: HypXERC20 test setup looks good.
589-594: HypXERC20Lockbox test follows the pattern.
666-671: HypFiatToken test updated correctly.
727-727: HypNative constructor call updated.Using
SCALE, SCALEfor native token transfers is correct - no decimal adjustment needed for ETH.
812-816: This is where the real scalin' magic gets tested.Good to see
HypERC20ScaledTestactually exercises non-trivial scaling withEFFECTIVE_SCALE(100) as numerator andSCALE(1) as denominator. The tests at lines 847-878 verify both outbound multiplication (TRANSFER_AMT * EFFECTIVE_SCALE) and inbound division (TRANSFER_AMT / EFFECTIVE_SCALE). This gives proper coverage for the fractional scaling feature.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## audit-q1-2026 #7659 +/- ##
=================================================
+ Coverage 81.16% 81.18% +0.01%
=================================================
Files 120 120
Lines 2777 2779 +2
Branches 254 255 +1
=================================================
+ Hits 2254 2256 +2
Misses 475 475
Partials 48 48
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
solidity/contracts/token/libs/TokenRouter.sol (1)
66-77: Previous critical issue resolved - numerator validation added.Right, so the earlier concern about division by zero when
scaleNumeratoris 0 has been addressed. The validation at lines 71-74 now requires both numerator and denominator to be greater than zero before proceeding. This prevents the division-by-zero scenario that would've occurred in_inboundAmount.
🧹 Nitpick comments (2)
typescript/cli/src/tests/ethereum/warp/warp-check-2.e2e-test.ts (1)
182-187: Consider adding explicit scale verification and non-trivial test values.Right, so the deploy call now takes the two scale parameters (numerator and denominator). Using
1, 1gives you a clean 1:1 ratio—no scaling—which is fine for basic functionality. But since fractional scaling is the whole point of this PR, you might want to beef up the test coverage a bit:
- Verify the scale values actually got set on the deployed contract (e.g., read
scaleNumerator()andscaleDenominator()if those getters exist).- Consider a test case with a non-trivial scale like
10, 1or1, 10to properly exercise the fractional scaling math.The test still does what it says on the tin—checking non-proxy deployments—but validating the new feature more directly would give you better coverage.
solidity/test/token/HypERC20.t.sol (1)
921-934: Consider actually invoking the contract here.This test calculates the expected scaled amount locally but doesn't verify that the contract produces the same result. The comment on line 928 suggests calling a transfer, but that wasn't implemented. Additionally, the chosen values (999 * 100 / 1 = 99900) don't produce a fractional result, so rounding behavior isn't demonstrated.
While the critical rounding behavior is tested elsewhere (in
test_inboundAmount_roundsDownandHypERC20CollateralScaledDownRoundingTest), it'd strengthen this test to actually calltransferRemoteand verify the emitted event contains the expected scaled amount.🔎 Optional enhancement
function test_outboundAmount_roundsDown() public view { - // Test that outbound scaling rounds down - // With scaleNumerator=100, scaleDenominator=1 - // An amount of 1.5 (represented as 150 with 2 decimal places) should round down - uint256 amount = 999; // Amount that doesn't divide evenly - uint256 expected = (amount * EFFECTIVE_SCALE) / 1; // 99900 - - // Access via a transfer to verify the scaling - // The localToken uses EFFECTIVE_SCALE as numerator, 1 as denominator - assertEq(expected, 99900); - - // Verify actual rounding: 999 * 100 / 1 = 99900 (no rounding needed here) - // Test a case where rounding matters with different scale + // Test that outbound scaling rounds down + // With scaleNumerator=100, scaleDenominator=3 (hypothetical) + // amount=100 -> 100 * 100 / 3 = 3333 (rounds down from 3333.333...) + + vm.expectEmit(true, true, false, true); + emit SentTransferRemote( + DESTINATION, + BOB.addressToBytes32(), + TRANSFER_AMT * EFFECTIVE_SCALE + ); + + vm.prank(ALICE); + localToken.transferRemote{value: REQUIRED_VALUE}( + DESTINATION, + BOB.addressToBytes32(), + TRANSFER_AMT + ); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
solidity/contracts/token/libs/TokenRouter.solsolidity/test/token/HypERC20.t.soltypescript/cli/src/tests/ethereum/warp/warp-check-2.e2e-test.ts
🧰 Additional context used
📓 Path-based instructions (2)
typescript/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
typescript/**/*.{ts,tsx}: UseChainMapfor per-chain configurations in TypeScript
Import types from@hyperlane-xyz/sdkwhen using TypeScript SDK types
Files:
typescript/cli/src/tests/ethereum/warp/warp-check-2.e2e-test.ts
solidity/**/*.sol
📄 CodeRabbit inference engine (CLAUDE.md)
solidity/**/*.sol: UseonlyOwneror appropriate access modifiers on privileged functions in Solidity
Validate all external inputs at system boundaries in Solidity contracts
Ensure backward compatibility for protocol upgrades in Solidity
Optimize for gas efficiency in Solidity by avoiding unnecessary storage writes
Check the contract inheritance hierarchy before implementing new Solidity contracts
Files:
solidity/contracts/token/libs/TokenRouter.solsolidity/test/token/HypERC20.t.sol
🧠 Learnings (4)
📓 Common learnings
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7253
File: typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts:285-289
Timestamp: 2025-10-24T18:58:44.557Z
Learning: In the Hyperlane funding script (typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts), tokens with 0 decimals are not valid and should be rejected. The truthy check on decimals is intentional and correct for this use case.
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-18T22:36:13.445Z
Learning: Applies to solidity/**/*.sol : Ensure backward compatibility for protocol upgrades in Solidity
📚 Learning: 2025-10-24T18:58:44.557Z
Learnt from: xeno097
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7253
File: typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts:285-289
Timestamp: 2025-10-24T18:58:44.557Z
Learning: In the Hyperlane funding script (typescript/infra/scripts/funding/fund-wallet-from-deployer-key.ts), tokens with 0 decimals are not valid and should be rejected. The truthy check on decimals is intentional and correct for this use case.
Applied to files:
typescript/cli/src/tests/ethereum/warp/warp-check-2.e2e-test.tssolidity/contracts/token/libs/TokenRouter.solsolidity/test/token/HypERC20.t.sol
📚 Learning: 2025-11-27T16:18:06.108Z
Learnt from: troykessler
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7481
File: typescript/aleo-sdk/src/clients/signer.ts:127-136
Timestamp: 2025-11-27T16:18:06.108Z
Learning: In typescript/aleo-sdk/src/clients/signer.ts, the createMailbox method calls deployProgram('dispatch_proxy', mailboxSalt) which is guaranteed to return at least 3 programs. Therefore, accessing programs[programs.length - 3] and programs[programs.length - 1] is safe and does not require additional length validation.
Applied to files:
typescript/cli/src/tests/ethereum/warp/warp-check-2.e2e-test.ts
📚 Learning: 2025-10-28T15:45:08.081Z
Learnt from: yorhodes
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7262
File: solidity/contracts/token/HypNative.sol:76-77
Timestamp: 2025-10-28T15:45:08.081Z
Learning: In HypNative (solidity/contracts/token/HypNative.sol), when native tokens are received via the `receive()` function during rebalancing, they represent user collateral returning from cross-chain operations, not LP contributions. Therefore, `receive()` should NOT call `donate()` as that would incorrectly credit returning user collateral to the LPs and inflate their share value.
Applied to files:
solidity/test/token/HypERC20.t.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (67)
- GitHub Check: cli-evm-e2e-matrix (core-read)
- GitHub Check: cli-evm-e2e-matrix (warp-check-5)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-2)
- GitHub Check: cli-evm-e2e-matrix (warp-rebalancer)
- GitHub Check: cli-evm-e2e-matrix (warp-check-2)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-config)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-2)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-1)
- GitHub Check: cli-evm-e2e-matrix (warp-check-3)
- GitHub Check: cli-evm-e2e-matrix (warp-init)
- GitHub Check: cli-evm-e2e-matrix (warp-check-4)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-basic)
- GitHub Check: cli-evm-e2e-matrix (warp-check-1)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-submitters)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
- GitHub Check: cli-evm-e2e-matrix (warp-read)
- GitHub Check: cli-evm-e2e-matrix (warp-send)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-simple-updates)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-hook-updates)
- GitHub Check: cli-evm-e2e-matrix (core-init)
- GitHub Check: cli-evm-e2e-matrix (status)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-rebalancing-config)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-evm-e2e-matrix (core-apply)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ism-updates)
- GitHub Check: cli-evm-e2e-matrix (core-check)
- GitHub Check: cli-evm-e2e-matrix (relay)
- GitHub Check: cli-evm-e2e-matrix (core-deploy)
- GitHub Check: env-test-matrix (mainnet3, optimism, igp)
- GitHub Check: env-test-matrix (mainnet3, optimism, core)
- GitHub Check: env-test-matrix (mainnet3, ethereum, igp)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
- GitHub Check: env-test-matrix (mainnet3, ethereum, core)
- GitHub Check: env-test-matrix (testnet4, sepolia, core)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
- GitHub Check: cli-radix-e2e-matrix (warp-deploy)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-cosmos-e2e-matrix (core-apply)
- GitHub Check: cli-cosmos-e2e-matrix (core-check)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-route-extension)
- GitHub Check: cli-radix-e2e-matrix (core-apply)
- GitHub Check: cli-radix-e2e-matrix (core-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
- GitHub Check: cosmos-sdk-e2e-run
- GitHub Check: aleo-sdk-e2e-run
- GitHub Check: cli-cosmos-e2e-matrix (core-read)
- GitHub Check: cli-cosmos-e2e-matrix (warp-read)
- GitHub Check: cli-install-test-run
- GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-deploy)
- GitHub Check: pnpm-test-run
- GitHub Check: coverage-run
- GitHub Check: infra-test
- GitHub Check: build-and-push-to-gcr
- GitHub Check: Cursor Bugbot
- GitHub Check: lint-prettier
- GitHub Check: diff-check
- GitHub Check: agent-configs (testnet4)
- GitHub Check: agent-configs (mainnet3)
- GitHub Check: lander-coverage
- GitHub Check: lint-rs
- GitHub Check: test-rs
- GitHub Check: fork-tests
- GitHub Check: slither
- GitHub Check: diff-check
🔇 Additional comments (9)
solidity/contracts/token/libs/TokenRouter.sol (4)
11-11: LGTM on the Math library addition.Good call pulling in OpenZeppelin's Math library here. The
mulDivfunction handles overflow protection and precise division - exactly what you need for fractional scaling.Also applies to: 30-30
56-57: Clean variable declarations.Using immutable for both numerator and denominator makes sense - set once at deployment, stored in bytecode rather than storage. Public visibility lets anyone read the scaling parameters.
406-414: Outbound scaling looks solid.The fractional scaling calculation is straightforward - multiply by numerator, divide by denominator, round down. Using
mulDivkeeps things safe from overflow, and rounding down consistently protects against any funny business with dust amounts.
425-433: Inbound scaling correctly inverts the fraction.The inverse scaling calculation flips the numerator and denominator, which is exactly right. Both directions use rounding down consistently, so users might lose a tiny bit in round-trip conversions due to integer division, but that's better than letting the contract get exploited. The docs explain this well.
solidity/test/token/HypERC20.t.sol (5)
105-110: Constructor updates look solid.All the constructor invocations across test contracts have been properly updated to include both
scaleNumeratorandscaleDenominator. UsingSCALE(which equals 1) for both parameters maintains the legacy behavior of 1:1 scaling, ensuring backward compatibility.Also applies to: 338-343, 448-453, 526-531, 589-594, 666-671, 727-727
802-824: Excellent constructor validation coverage.These tests properly verify that the constructor rejects invalid scale parameters (zero numerator or denominator), which is critical for preventing division-by-zero errors and ensuring valid token scaling behavior.
826-919: Scaled test setup works as intended.The
HypERC20ScaledTestcorrectly implements a 100:1 scaling ratio (scaleNumerator=100, scaleDenominator=1). The tests verify that outbound transfers multiply by the effective scale and inbound transfers divide by it, which matches the expected fractional scaling behavior.
936-967: Good test for inbound rounding behavior.This test properly verifies that inbound scaling rounds down by using an amount (999) that doesn't divide evenly by the effective scale (100), then confirming the recipient receives 9 tokens (not 10). The test actually invokes the contract via
_handleLocalTransferand verifies both the balance change and event emissions.
970-1071: Comprehensive fractional scaling test coverage.The
HypERC20CollateralScaledDownRoundingTestprovides excellent coverage for fractional scaling scenarios using a 3/2 (1.5x) scale ratio:
- Outbound test (lines 1007-1033): Verifies that 101 tokens scale to 151 (not 152), confirming proper rounding down from 151.5
- Inbound test (lines 1035-1065): Verifies that a message amount of 100 produces 66 local tokens (not 67), confirming proper rounding down from 66.666...
Both tests actually invoke the contract and verify the results, which is spot-on. The rounding down behavior is critical for preventing users from receiving more tokens than they should during cross-chain transfers.
The override at lines 1067-1070 to skip
testRemoteTransfer_withFeemakes sense since the parent test doesn't account for the scaling logic in fee calculations.
larryob
left a comment
larryob
left a comment
There was a problem hiding this comment.
Nice and simple. Would be nice to add some context on why this is useful to the description (maybe a linear issue as well).
Releases: @hyperlane-xyz/cli@20.2.0-beta.0 @hyperlane-xyz/provider-sdk@0.8.0-beta.0 @hyperlane-xyz/deploy-sdk@0.8.0-beta.0 @hyperlane-xyz/infra@20.2.0-beta.0 @hyperlane-xyz/rebalancer@0.1.0-beta.0 @hyperlane-xyz/core@11.0.0-beta.0 @hyperlane-xyz/aleo-sdk@20.2.0-beta.0 @hyperlane-xyz/cosmos-sdk@20.2.0-beta.0 @hyperlane-xyz/radix-sdk@20.2.0-beta.0 @hyperlane-xyz/sdk@20.2.0-beta.0 @hyperlane-xyz/widgets@20.2.0-beta.0 @hyperlane-xyz/ccip-server@20.2.0-beta.0 @hyperlane-xyz/helloworld@20.2.0-beta.0 @hyperlane-xyz/http-registry-server@20.2.0-beta.0 @hyperlane-xyz/starknet-core@20.2.0-beta.0 @hyperlane-xyz/cosmos-types@20.2.0-beta.0 @hyperlane-xyz/eslint-config@20.2.0-beta.0 @hyperlane-xyz/github-proxy@20.2.0-beta.0 @hyperlane-xyz/tsconfig@20.2.0-beta.0 @hyperlane-xyz/utils@20.2.0-beta.0 [skip ci]
- Add .gt(0) validation for scale numerator in TypeScript schema (Cursor comment) - Support string representation for large scale values (> MAX_SAFE_INTEGER) - Update fetchScale to safely handle BigNumber overflow with try/catch - Update deploy.ts, decimals.ts, and TokenMetadataMap.ts to handle string scale values Fractional scaling tests already exist in HypERC20.t.sol (CodeRabbit flagged incorrectly)
yorhodes
force-pushed
the
evm-bidirectional-scaling
branch
from
9bf7c88 to
dd8488f
Compare
yorhodes
requested review from
Mo-Hussain,
nambrot,
troykessler and
yjamin
as code owners
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@solidity/test/token/HypERC20.t.sol`:
- Around line 921-934: The test test_outboundAmount_roundsDown currently only
does local arithmetic with denominator=1 and doesn't exercise router rounding;
change it to perform an actual transfer through the router/localToken logic
(make the test non-view) using a scaleDenominator > 1 so rounding can occur,
then assert the emitted/returned outbound amount equals floor(amount *
scaleNumerator / scaleDenominator). Specifically, in
test_outboundAmount_roundsDown invoke the contract transfer path that triggers
outbound scaling (e.g., call the router or localToken transfer function used
elsewhere in tests), use a concrete amount that yields a fractional result
(e.g., amount and set EFFECTIVE_SCALE numerator and scaleDenominator to force
remainder), compute expected = (amount * scaleNumerator) / scaleDenominator
(integer division) and assert the emitted event or return value matches
expected; alternatively, if you prefer minimal change, remove
test_outboundAmount_roundsDown to avoid misleading coverage.
- Around line 1067-1070: The test testRemoteTransfer_withFee was skipped and
leaves a coverage gap now that fractional scaling is applied; re-enable the test
(remove vm.skip) and update its assertions to account for scaling by computing
expected remote amounts using the same scaling logic used in the contract (apply
the scale factor/decimals and fee calculation before comparing). Locate
testRemoteTransfer_withFee in HypERC20.t.sol and replace the hard-coded expected
remote amount with a calculation that mirrors the contract's scale + fee math
(use the same helper/scale constants or functions used elsewhere in tests) so
the assertions validate the scaled transferred amount and fee.
| function test_outboundAmount_roundsDown() public view { | ||
| // Test that outbound scaling rounds down | ||
| // With scaleNumerator=100, scaleDenominator=1 | ||
| // An amount of 1.5 (represented as 150 with 2 decimal places) should round down | ||
| uint256 amount = 999; // Amount that doesn't divide evenly | ||
| uint256 expected = (amount * EFFECTIVE_SCALE) / 1; // 99900 | ||
|
|
||
| // Access via a transfer to verify the scaling | ||
| // The localToken uses EFFECTIVE_SCALE as numerator, 1 as denominator | ||
| assertEq(expected, 99900); | ||
|
|
||
| // Verify actual rounding: 999 * 100 / 1 = 99900 (no rounding needed here) | ||
| // Test a case where rounding matters with different scale | ||
| } |
There was a problem hiding this comment.
Outbound rounding test isn’t exercising router logic.
Right now it only checks arithmetic with denominator=1, so it won’t catch rounding behavior in the contract. I’d either remove it or rework it to send a transfer with a fractional denominator and assert on the emitted amount.
💡 Minimal fix (remove misleading test)
- function test_outboundAmount_roundsDown() public view {
- // Test that outbound scaling rounds down
- // With scaleNumerator=100, scaleDenominator=1
- // An amount of 1.5 (represented as 150 with 2 decimal places) should round down
- uint256 amount = 999; // Amount that doesn't divide evenly
- uint256 expected = (amount * EFFECTIVE_SCALE) / 1; // 99900
-
- // Access via a transfer to verify the scaling
- // The localToken uses EFFECTIVE_SCALE as numerator, 1 as denominator
- assertEq(expected, 99900);
-
- // Verify actual rounding: 999 * 100 / 1 = 99900 (no rounding needed here)
- // Test a case where rounding matters with different scale
- }📝 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 test_outboundAmount_roundsDown() public view { | |
| // Test that outbound scaling rounds down | |
| // With scaleNumerator=100, scaleDenominator=1 | |
| // An amount of 1.5 (represented as 150 with 2 decimal places) should round down | |
| uint256 amount = 999; // Amount that doesn't divide evenly | |
| uint256 expected = (amount * EFFECTIVE_SCALE) / 1; // 99900 | |
| // Access via a transfer to verify the scaling | |
| // The localToken uses EFFECTIVE_SCALE as numerator, 1 as denominator | |
| assertEq(expected, 99900); | |
| // Verify actual rounding: 999 * 100 / 1 = 99900 (no rounding needed here) | |
| // Test a case where rounding matters with different scale | |
| } |
🤖 Prompt for AI Agents
In `@solidity/test/token/HypERC20.t.sol` around lines 921 - 934, The test
test_outboundAmount_roundsDown currently only does local arithmetic with
denominator=1 and doesn't exercise router rounding; change it to perform an
actual transfer through the router/localToken logic (make the test non-view)
using a scaleDenominator > 1 so rounding can occur, then assert the
emitted/returned outbound amount equals floor(amount * scaleNumerator /
scaleDenominator). Specifically, in test_outboundAmount_roundsDown invoke the
contract transfer path that triggers outbound scaling (e.g., call the router or
localToken transfer function used elsewhere in tests), use a concrete amount
that yields a fractional result (e.g., amount and set EFFECTIVE_SCALE numerator
and scaleDenominator to force remainder), compute expected = (amount *
scaleNumerator) / scaleDenominator (integer division) and assert the emitted
event or return value matches expected; alternatively, if you prefer minimal
change, remove test_outboundAmount_roundsDown to avoid misleading coverage.
| function testRemoteTransfer_withFee() public override { | ||
| // test does not handle scaling logic | ||
| vm.skip(true); | ||
| } |
There was a problem hiding this comment.
Skipping the fee-path test leaves a coverage gap.
Since scaling is now in play, it’d be better to keep this test and adjust expected remote amounts for the fractional scale, rather than skipping entirely.
💡 Example adjustment for scaled expectation
- function testRemoteTransfer_withFee() public override {
- // test does not handle scaling logic
- vm.skip(true);
- }
+ function testRemoteTransfer_withFee() public override {
+ feeContract = new LinearFee(
+ address(primaryToken),
+ 1e18,
+ 100e18,
+ address(this)
+ );
+ localToken.setFeeRecipient(address(feeContract));
+ uint256 fee = feeContract
+ .quoteTransferRemote(DESTINATION, BOB.addressToBytes32(), TRANSFER_AMT)[
+ 0
+ ].amount;
+ uint256 total = TRANSFER_AMT + fee;
+ uint256 expectedRemote =
+ (TRANSFER_AMT * SCALE_NUMERATOR) / SCALE_DENOMINATOR;
+
+ uint256 nativeValue = REQUIRED_VALUE;
+ deal(address(primaryToken), ALICE, total);
+ vm.prank(ALICE);
+ primaryToken.approve(address(localToken), total);
+
+ (
+ uint256 senderBefore,
+ uint256 beneficiaryBefore,
+ uint256 recipientBefore
+ ) = _getBalances(ALICE, BOB);
+
+ vm.prank(ALICE);
+ localToken.transferRemote{value: nativeValue}(
+ DESTINATION,
+ BOB.addressToBytes32(),
+ TRANSFER_AMT
+ );
+
+ _processTransfers();
+ (
+ uint256 senderAfter,
+ uint256 beneficiaryAfter,
+ uint256 recipientAfter
+ ) = _getBalances(ALICE, BOB);
+
+ assertEq(senderAfter, senderBefore - total);
+ assertEq(beneficiaryAfter, beneficiaryBefore + fee);
+ assertEq(recipientAfter, recipientBefore + expectedRemote);
+ }🤖 Prompt for AI Agents
In `@solidity/test/token/HypERC20.t.sol` around lines 1067 - 1070, The test
testRemoteTransfer_withFee was skipped and leaves a coverage gap now that
fractional scaling is applied; re-enable the test (remove vm.skip) and update
its assertions to account for scaling by computing expected remote amounts using
the same scaling logic used in the contract (apply the scale factor/decimals and
fee calculation before comparing). Locate testRemoteTransfer_withFee in
HypERC20.t.sol and replace the hard-coded expected remote amount with a
calculation that mirrors the contract's scale + fee math (use the same
helper/scale constants or functions used elsewhere in tests) so the assertions
validate the scaled transferred amount and fee.
2 participants
Description
Backward compatibility
Yes, via implementation upgrade
Testing
Unit Tests
Note
Major: fractional scaling for token routing
scalewithscaleNumerator/scaleDenominatorinTokenRouter; implement bidirectional scaling viamulDivwith round-down in_outboundAmount/_inboundAmountand require non-zero valuesHypERC20,HypERC20Collateral,HypNative,HypERC721(1,1),HypXERC20*,HypFiatToken,HypERC4626*, CCTP/OP/Everclear bridges) to accept the new scale fractionSDK/CLI updates
fetchScalewith version gating (legacyscalevs new numerator/denominator); includescalein derived configs (native/collateral/synthetic/xERC20/everclear/4626)scaleas number or{ numerator, denominator }and pass to constructorsTokenMetadata.scaleschema to number or fraction; TokenMetadataMap/getScale supports fraction; decimals verification updated to handle fractionsRelease
11.0.0-beta.0with changelog entryWritten by Cursor Bugbot for commit 7492491. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Chores