feat: switch all cross-chain address fields to bytes32#108
Merged
Conversation
… to cater for stellar byte32 length against addresses byte20 limit
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR migrates many cross-chain "address-like" fields to bytes32 across EVM contracts, JS/ABI helpers, tests, and EIP‑712 hashes; simplifies Stellar ad-manager admin auth to require_auth(); and adds a new TypeScript + Bash end-to-end cross-chain test framework (Stellar ↔ EVM) with ZK proof generation. Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as E2E Runner
participant Stellar as Stellar Localnet
participant EVM as EVM (Anvil)
participant ZK as ZK Proof System
participant AdMgr as Stellar AdManager
participant OrderP as EVM OrderPortal
Runner->>Stellar: deploy verifier, merkleManager, adManager
Runner->>EVM: deploy verifier, merkleManager, orderPortal
Runner->>Stellar: set_chain / set_token_route (signed)
Runner->>EVM: orderPortal.setChain / setTokenRoute
Runner->>Stellar: create_ad (signed)
Runner->>EVM: createOrder (mint/approve + signed request)
Runner->>Stellar: lock_for_order (signed)
Runner->>ZK: generateProofs(order, circuit)
ZK-->>Runner: proofs, nullifiers, targetRoot
Runner->>Stellar: unlock (signed + proof)
Stellar->>AdMgr: validate proof, transfer tokens
Runner->>EVM: unlock (signed + proof)
EVM->>OrderP: validate proof, transfer tokens -> order Filled
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
contracts/stellar/contracts/ad-manager/src/lib.rs (2)
91-109:⚠️ Potential issue | 🟡 MinorFix formatting to pass CI.
The pipeline failure indicates
cargo fmtcheck failed on theset_managerfunction signature. Runcargo fmt --allto apply the expected formatting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/stellar/contracts/ad-manager/src/lib.rs` around lines 91 - 109, The function signature for set_manager (pub fn set_manager(env: Env, manager: Address, status: bool) -> Result<(), AdManagerError>) is not formatted per rustfmt; run cargo fmt --all (or apply rustfmt) to reformat the declaration and body so CI passes, ensuring the signature and parameter alignment for set_manager, and keep surrounding symbols (Env, Address, AdManagerError, storage::set_manager, events::ManagerUpdated) intact.
193-216:⚠️ Potential issue | 🟠 MajorUpdate e2e scripts to use the new admin API without signature/auth_token parameters.
The
set_chainandset_token_routecalls inscripts/cross-chain-e2e/run.ts(lines 201-209 and 224-232) still pass--signature,--public_key,--auth_token, and--time_to_expireparameters. These functions now use Soroban's nativerequire_auth()and no longer accept these parameters. Remove these parameters and adjust the calls to match the new signatures:set_chain(order_chain_id, order_portal, supported)andset_token_route(ad_token, order_token, order_chain_id).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/stellar/contracts/ad-manager/src/lib.rs` around lines 193 - 216, E2E script calls still pass legacy auth flags to the admin functions; update the script invocations that call set_chain and set_token_route to remove the obsolete CLI flags (--signature, --public_key, --auth_token, --time_to_expire) and invoke the new signatures instead: call set_chain(order_chain_id, order_portal, supported) and call set_token_route(ad_token, order_token, order_chain_id); ensure no manual signature/auth_token handling remains and rely on the contract's require_auth() enforcement (referenced in the contract functions requiring admin auth).contracts/evm/src/AdManager.sol (1)
1154-1188:⚠️ Potential issue | 🔴 CriticalReject non-EVM
orderRecipientvalues duringlockForOrder().
unlock()assumesparams.orderRecipientis a local EVM address and converts it withtoAddressChecked(), butvalidateOrder()only checks for non-zero. That means liquidity can be locked for an order that is structurally unfillable on this chain.Suggested fix
function validateOrder(Ad memory ad, OrderParams calldata params) internal view returns (bytes32 orderHash) { if (!ad.open) revert AdManager__AdClosed(); if (params.amount == 0) revert AdManager__ZeroAmount(); if (params.bridger == bytes32(0)) revert AdManager__BridgerZero(); if (params.orderRecipient == bytes32(0)) revert AdManager__RecipientZero(); + toAddressChecked(params.orderRecipient); // Source chain must be supported and portal must match (if configured). ChainInfo memory ci = chains[params.orderChainId];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/evm/src/AdManager.sol` around lines 1154 - 1188, validateOrder currently only checks params.orderRecipient != bytes32(0) but unlock() uses toAddressChecked() assuming an on-chain EVM address; add a stricter validation in validateOrder to reject non-EVM recipients by ensuring params.orderRecipient encodes an EVM address (upper 12 bytes zero in the bytes32 representation) and revert if not (introduce a clear error like AdManager__RecipientNotEVM). Update validateOrder (referencing validateOrder, unlock, lockForOrder, and toAddressChecked) to perform this check so you never lock liquidity for structurally unfillable orders.
🧹 Nitpick comments (2)
contracts/evm/test/OrderPortal.t.sol (1)
573-590: Avoid unchecked truncation when asserting recipient balances.These casts bypass the new checked-local-address behavior by truncating
bytes32directly to 20 bytes. That makes the tests less strict than production and can hide regressions where a non-EVMadRecipientshould fail instead of being silently truncated.Suggested helper and replacement
+ function _toAddrChecked(bytes32 value) internal pure returns (address) { + require(uint256(value) >> 160 == 0, "non-EVM address"); + return address(uint160(uint256(value))); + } + - uint256 balRecipientBefore = orderToken.balanceOf(address(uint160(uint256(p.adRecipient)))); + uint256 balRecipientBefore = orderToken.balanceOf(_toAddrChecked(p.adRecipient)); ... - assertEq(orderToken.balanceOf(address(uint160(uint256(p.adRecipient)))), balRecipientBefore, "recipient balance changed"); + assertEq(orderToken.balanceOf(_toAddrChecked(p.adRecipient)), balRecipientBefore, "recipient balance changed"); ... - uint256 balRecipientBefore = orderToken.balanceOf(address(uint160(uint256(p.adRecipient)))); + uint256 balRecipientBefore = orderToken.balanceOf(_toAddrChecked(p.adRecipient)); ... - assertEq(orderToken.balanceOf(address(uint160(uint256(p.adRecipient)))), balRecipientBefore + p.amount, "recipient not credited"); + assertEq(orderToken.balanceOf(_toAddrChecked(p.adRecipient)), balRecipientBefore + p.amount, "recipient not credited"); ... - uint256 balRecipientBefore = address(uint160(uint256(p.adRecipient))).balance; + uint256 balRecipientBefore = _toAddrChecked(p.adRecipient).balance; ... - assertEq(address(uint160(uint256(p.adRecipient))).balance, balRecipientBefore + p.amount, "recipient not credited"); + assertEq(_toAddrChecked(p.adRecipient).balance, balRecipientBefore + p.amount, "recipient not credited");Also applies to: 602-623, 652-672
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/evm/test/OrderPortal.t.sol` around lines 573 - 590, The test is silently truncating p.adRecipient via address(uint160(uint256(...))) which bypasses checked-local-address rules; replace those casts with a small helper like toCheckedAddress(bytes32 x) used in the tests that verifies the upper 12 bytes are zero (e.g. require(uint96(uint256(x >> 160)) == 0) or check (bytes12(x) == 0)) and then returns address(uint160(uint256(x))); update all occurrences (e.g. the balanceOf arguments and any address(...) uses in unlock-related asserts) to call toCheckedAddress(p.adRecipient) so the test fails if a non-EVM recipient is provided instead of silently truncating.contracts/evm/test/Admanager.t.sol (1)
1138-1138: Use a checkedbytes32→addresshelper in these assertions.
address(uint160(uint256(...)))silently truncates the upper 96 bits, so this test would still pass for a non-EVMorderRecipienteven though production now rejects that case viatoAddressChecked. Mirror the checked conversion here, or assert the upper 96 bits are zero before casting, so the suite validates the same invariant asunlock.Suggested test helper
+ function _toAddrChecked(bytes32 value) internal pure returns (address) { + require(uint256(value) >> 160 == 0, "non-EVM address"); + return address(uint160(uint256(value))); + } + - uint256 balBefore = adToken.balanceOf(address(uint160(uint256(p.orderRecipient)))); + uint256 balBefore = adToken.balanceOf(_toAddrChecked(p.orderRecipient)); ... - uint256 balAfter = adToken.balanceOf(address(uint160(uint256(p.orderRecipient)))); + uint256 balAfter = adToken.balanceOf(_toAddrChecked(p.orderRecipient));Also applies to: 1161-1161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/evm/test/Admanager.t.sol` at line 1138, Replace the unchecked cast address(uint160(uint256(p.orderRecipient))) used when reading balances with the same checked conversion used in production: either call the test helper toAddressChecked(p.orderRecipient) (or implement one that reverts/asserts if uint256(p.orderRecipient) >> 160 != 0) and use that result for adToken.balanceOf, or explicitly assert the upper 96 bits are zero before the cast (e.g., require(uint256(p.orderRecipient) >> 160 == 0)) so that p.orderRecipient, balBefore, and the adToken.balanceOf check mirror the production toAddressChecked invariant.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/cross-chain-e2e.yml:
- Around line 1-71: The workflow file "Cross-Chain E2E" is entirely commented
out; either delete the file, re-enable the workflow by uncommenting the job
block that defines the cross-chain-e2e job (including steps like
actions/checkout, foundry setup, and the Run cross-chain E2E test step that runs
scripts/run_cross_chain_e2e.sh), or replace the content with a minimal
placeholder job that succeeds (e.g., a job named cross-chain-e2e that runs a
simple echo or true step) so actionlint no longer flags an empty workflow.
In `@contracts/evm/src/OrderPortal.sol`:
- Around line 591-607: validateOrder currently allows any non-zero bytes32
adRecipient even though unlock() later calls
toAddressChecked(params.adRecipient) and a non-EVM address would be unpayable;
call toAddressChecked(params.adRecipient) inside validateOrder (use the existing
helper toAddressChecked) and require the resulting address != address(0) (or
revert with a new/appropriate error) so adRecipient is a valid 20-byte EVM
address before proceeding; update any error messages to reference adRecipient
and keep the rest of validateOrder logic unchanged.
In `@scripts/cross-chain-e2e/lib/proof.ts`:
- Around line 51-55: The ORDER_TYPEHASH constant is still built from the old
address-based type string, so update the type string passed to keccak256 in
ORDER_TYPEHASH to the new bytes32-based schema (replace each address and string
fields with bytes32) — e.g., "Order(bytes32 orderChainToken,bytes32
adChainToken,uint256 amount,bytes32 bridger,uint256 orderChainId,bytes32
orderPortal,bytes32 orderRecipient,uint256 adChainId,bytes32 adManager,bytes32
adId,bytes32 adCreator,bytes32 adRecipient,uint256 salt)"; update the
Buffer.from(...) argument that builds ORDER_TYPEHASH and ensure
computeOrderHash() and any signature/proof code still reference the same
ORDER_TYPEHASH symbol so all off-chain hashes match the on-chain bytes32 schema.
In `@scripts/cross-chain-e2e/run.ts`:
- Around line 325-339: The evmOrderParams object is using 20-byte address
strings for fields that the Solidity OrderParams now expects as bytes32; convert
each address-shaped value (orderChainToken, adChainToken, bridger,
orderRecipient, adManager, adCreator, adRecipient) to 32-byte values before
assigning to evmOrderParams (e.g., use the existing toBytes32 helper or
ethers.utils.hexZeroPad/hexZeroFill to left-pad each hex address to 32 bytes) so
the ABI/hash/signature matches the contract; keep SALT and other non-address
fields unchanged and ensure the converted bytes32 values are what you pass into
evmOrderParams.
- Around line 235-255: The Stellar IDs are being truncated to 20 bytes before
calling OrderPortal setters which expect exact bytes32 values; stop slicing and
pass full 32-byte IDs (or convert via the correct helper) to
evm.orderPortal.getFunction("setChain") and getFunction("setTokenRoute"):
replace the manual 20-byte truncation of stellarAdTokenHex (and any similar
slicing) with a conversion that returns a full bytes32 hex (e.g., use
stellarIdToEvmAddress or ensure the hex is zero‑padded to 32 bytes) so
STELLAR_CHAIN_ID, stellarAdManagerEvm, and the Stellar token ID are stored and
encoded as full bytes32 values.
In `@scripts/run_cross_chain_e2e.sh`:
- Line 89: The anvil process is currently bound to all interfaces using "--host
0.0.0.0", which exposes the dev RPC; update the anvil invocation in the
run_cross_chain_e2e.sh script (the line that starts the anvil background
process) to bind only to loopback (e.g., replace "--host 0.0.0.0" with "--host
127.0.0.1" or remove the host flag if default is loopback) so the local E2E
chain is not reachable from other hosts.
---
Outside diff comments:
In `@contracts/evm/src/AdManager.sol`:
- Around line 1154-1188: validateOrder currently only checks
params.orderRecipient != bytes32(0) but unlock() uses toAddressChecked()
assuming an on-chain EVM address; add a stricter validation in validateOrder to
reject non-EVM recipients by ensuring params.orderRecipient encodes an EVM
address (upper 12 bytes zero in the bytes32 representation) and revert if not
(introduce a clear error like AdManager__RecipientNotEVM). Update validateOrder
(referencing validateOrder, unlock, lockForOrder, and toAddressChecked) to
perform this check so you never lock liquidity for structurally unfillable
orders.
In `@contracts/stellar/contracts/ad-manager/src/lib.rs`:
- Around line 91-109: The function signature for set_manager (pub fn
set_manager(env: Env, manager: Address, status: bool) -> Result<(),
AdManagerError>) is not formatted per rustfmt; run cargo fmt --all (or apply
rustfmt) to reformat the declaration and body so CI passes, ensuring the
signature and parameter alignment for set_manager, and keep surrounding symbols
(Env, Address, AdManagerError, storage::set_manager, events::ManagerUpdated)
intact.
- Around line 193-216: E2E script calls still pass legacy auth flags to the
admin functions; update the script invocations that call set_chain and
set_token_route to remove the obsolete CLI flags (--signature, --public_key,
--auth_token, --time_to_expire) and invoke the new signatures instead: call
set_chain(order_chain_id, order_portal, supported) and call
set_token_route(ad_token, order_token, order_chain_id); ensure no manual
signature/auth_token handling remains and rely on the contract's require_auth()
enforcement (referenced in the contract functions requiring admin auth).
---
Nitpick comments:
In `@contracts/evm/test/Admanager.t.sol`:
- Line 1138: Replace the unchecked cast
address(uint160(uint256(p.orderRecipient))) used when reading balances with the
same checked conversion used in production: either call the test helper
toAddressChecked(p.orderRecipient) (or implement one that reverts/asserts if
uint256(p.orderRecipient) >> 160 != 0) and use that result for
adToken.balanceOf, or explicitly assert the upper 96 bits are zero before the
cast (e.g., require(uint256(p.orderRecipient) >> 160 == 0)) so that
p.orderRecipient, balBefore, and the adToken.balanceOf check mirror the
production toAddressChecked invariant.
In `@contracts/evm/test/OrderPortal.t.sol`:
- Around line 573-590: The test is silently truncating p.adRecipient via
address(uint160(uint256(...))) which bypasses checked-local-address rules;
replace those casts with a small helper like toCheckedAddress(bytes32 x) used in
the tests that verifies the upper 12 bytes are zero (e.g.
require(uint96(uint256(x >> 160)) == 0) or check (bytes12(x) == 0)) and then
returns address(uint160(uint256(x))); update all occurrences (e.g. the balanceOf
arguments and any address(...) uses in unlock-related asserts) to call
toCheckedAddress(p.adRecipient) so the test fails if a non-EVM recipient is
provided instead of silently truncating.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 928b9800-2848-4901-85de-3bbc7850db8b
📒 Files selected for processing (28)
.github/workflows/cross-chain-e2e.ymlcontracts/evm/js-scripts/deploy/core/setup.tscontracts/evm/js-scripts/deploy/core/utils.tscontracts/evm/js-scripts/utils.tscontracts/evm/src/AdManager.solcontracts/evm/src/OrderPortal.solcontracts/evm/test/Admanager.t.solcontracts/evm/test/OrderPortal.t.solcontracts/evm/test/ProofBridge.t.solcontracts/stellar/contracts/ad-manager/src/lib.rscontracts/stellar/lib/proofbridge-core/src/eip712.rscontracts/stellar/lib/proofbridge-core/src/test.rscontracts/stellar/test_snapshots/test_ad_manager_lock_for_order.1.jsoncontracts/stellar/test_snapshots/test_ad_manager_unlock_with_bridger_proof.1.jsoncontracts/stellar/test_snapshots/test_full_cross_chain_flow.1.jsoncontracts/stellar/test_snapshots/test_nullifier_prevents_double_unlock.1.jsoncontracts/stellar/test_snapshots/test_order_portal_create_and_unlock.1.jsoncontracts/stellar/tests/fixtures/generate_fixtures.tscontracts/stellar/tests/integration_test.rspnpm-workspace.yamlscripts/cross-chain-e2e/lib/evm.tsscripts/cross-chain-e2e/lib/proof.tsscripts/cross-chain-e2e/lib/signing.tsscripts/cross-chain-e2e/lib/stellar.tsscripts/cross-chain-e2e/package.jsonscripts/cross-chain-e2e/run.tsscripts/cross-chain-e2e/tsconfig.jsonscripts/run_cross_chain_e2e.sh
There was a problem hiding this comment.
🧹 Nitpick comments (2)
contracts/evm/test/ProofBridge.t.sol (1)
97-99: Add at least one non-addressbytes32fixture to the parity path.These helpers still derive every cross-chain identifier from
_b32(address), so the upper 12 bytes are always zero in the updated hash-parity tests. That means a regression that accidentally truncatesbytes32values back to 20 bytes would still pass here, even though the PR’s main goal is lossless 32-byte Stellar round-tripping.Also applies to: 204-213, 222-231
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/evm/test/ProofBridge.t.sol` around lines 97 - 99, Tests currently derive every bytes32 via _b32(address) which zeros the top 12 bytes, so add at least one non-address bytes32 fixture (e.g., a bytes32 literal with non-zero high 12 bytes) into the parity test vectors used in the parity path and hash-parity tests to ensure 32-byte values aren't being truncated; update the arrays/fixtures referenced alongside _b32 (including the blocks around the existing _b32 helper and the test vector groups around the ranges noted) to include that non-address bytes32 value and ensure the parity assertions exercise it.contracts/evm/test/Admanager.t.sol (1)
1049-1054: Please cover the checked local-recipient decode path as well.
_openOrderalways feedsorderRecipientas_b32(address), and the balance assertions then truncate it back withaddress(uint160(uint256(...))). That keeps these tests green for EVM-shaped values, but it does not protect the newtoAddressChecked(bytes32)behavior onunlock. A negative case with a high-bit-setbytes32recipient would close that gap.Also applies to: 1144-1168
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/evm/test/Admanager.t.sol` around lines 1049 - 1054, Add a test exercising the checked local-recipient decode path by calling _openOrder with an orderRecipient that is a bytes32 value with high bits set (not a plain _b32(address)) so that unlock will invoke toAddressChecked(bytes32) and fail/revert as expected; modify the existing test that uses _defaultParams/adId/_b32(_recipient) to instead create a maliciousRecipient = bytes32(uint256(1) << 255 | uint256(uint160(validRecipient))) (or another high-bit-set value) and set p.orderRecipient = maliciousRecipient, then assert the unlock (or the balance assertions around unlock) reverts or behaves per the new toAddressChecked semantics; reference functions/vars: _openOrder, _defaultParams, p.orderRecipient, _b32, _openOrder, unlock, toAddressChecked to locate where to inject this negative-case check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@contracts/evm/test/Admanager.t.sol`:
- Around line 1049-1054: Add a test exercising the checked local-recipient
decode path by calling _openOrder with an orderRecipient that is a bytes32 value
with high bits set (not a plain _b32(address)) so that unlock will invoke
toAddressChecked(bytes32) and fail/revert as expected; modify the existing test
that uses _defaultParams/adId/_b32(_recipient) to instead create a
maliciousRecipient = bytes32(uint256(1) << 255 |
uint256(uint160(validRecipient))) (or another high-bit-set value) and set
p.orderRecipient = maliciousRecipient, then assert the unlock (or the balance
assertions around unlock) reverts or behaves per the new toAddressChecked
semantics; reference functions/vars: _openOrder, _defaultParams,
p.orderRecipient, _b32, _openOrder, unlock, toAddressChecked to locate where to
inject this negative-case check.
In `@contracts/evm/test/ProofBridge.t.sol`:
- Around line 97-99: Tests currently derive every bytes32 via _b32(address)
which zeros the top 12 bytes, so add at least one non-address bytes32 fixture
(e.g., a bytes32 literal with non-zero high 12 bytes) into the parity test
vectors used in the parity path and hash-parity tests to ensure 32-byte values
aren't being truncated; update the arrays/fixtures referenced alongside _b32
(including the blocks around the existing _b32 helper and the test vector groups
around the ranges noted) to include that non-address bytes32 value and ensure
the parity assertions exercise it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 78e91fbd-e988-4500-a9f7-8583267dae12
📒 Files selected for processing (4)
contracts/evm/test/Admanager.t.solcontracts/evm/test/OrderPortal.t.solcontracts/evm/test/ProofBridge.t.solcontracts/stellar/contracts/ad-manager/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- contracts/evm/test/OrderPortal.t.sol
This was referenced Apr 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
EVM: switch all cross-chain
addressfields tobytes32Why
Stellar contract IDs and account IDs are 32 bytes wide. EVM
addressis 20. Until now the EVM contracts stored cross-chain identifiers asaddress, which forced Stellar's 32-byte IDs to be truncated to their low 20 bytes whenever they round-tripped through EVM. That truncation made it impossible to recover the original Stellar ID from anything an EVM contract had seen, and it forced the EIP-712 order hash to diverge between chains — the EVM side was hashing truncated values while the Stellar side was hashing the full 32 bytes.This PR widens every cross-chain identifier on the EVM side to
bytes32so the same value round-trips losslessly in either direction and both chains compute byte-identical EIP-712 hashes.What changed
ORDER_TYPEHASHAll address-typed members of
Orderare nowbytes32. The type string is:New typehash:
0x1ca8761ed9ca27de2766ec2a5767edf8231ab90bcd988058bc64b2966494fe9b. The same constant is now used on both EVM and Stellar (contracts/stellar/lib/proofbridge-core/src/eip712.rs).AdManager.solandOrderPortal.solOrderParams: every cross-chain address field (orderChainToken,bridger,orderPortal/srcOrderPortal,orderRecipient,adCreator,adRecipient,adManager,adChainToken) is nowbytes32.ChainInfo.orderPortal/ChainInfo.adManager→bytes32.Ad.adRecipient→bytes32(EVM contracts still useaddress makerfor the ad creator's local identity, which is checked againstparams.adCreatorviatoBytes32).tokenRoute:mapping(address => mapping(uint256 => bytes32))— the remote token is now stored as full 32 bytes.setChain(uint256, bytes32, bool)andsetTokenRoute(address, bytes32, uint256)/setTokenRoute(address, uint256, bytes32)takebytes32for the remote side.ChainSet,TokenRouteSet,TokenRouteRemoved,OrderLocked,OrderUnlocked) emitbytes32for remote / cross-chain user fields.createAd(..., bytes32 adRecipient)andcreateAdRequestHash(..., bytes32 adRecipient, ...).Local-address extraction
Some fields inside
OrderParamsrefer to local EVM participants (e.g.orderRecipienton the order-chain side,adCreator/adRecipienton the ad-chain side). For those we still need a 20-byteaddressat call time — totransferERC-20s, to checkmsg.sender, etc.New helpers:
toBytes32(address) internal pure returns (bytes32)— left-pads with zeros.toAddressChecked(bytes32) internal pure returns (address)— extracts the low 20 bytes and reverts withAdManager__NotEvmAddress(bytes32)/OrderPortal__NotEvmAddress(bytes32)if the upper 12 bytes are non-zero. This guarantees we never silently truncate a Stellar-shaped ID into a (wrong) EVM address.unlockon both contracts usestoAddressCheckedon the local recipient before transferring tokens.Validation changes in
validateOrderWhere the contract previously compared
addresstoaddress, it now comparesbytes32tobytes32directly. Ad-side identity checks become, e.g.:params.adCreator != toBytes32(ad.maker)— localmakeris widened tobytes32for comparison.params.adChainToken != toBytes32(ad.token)— same.params.adRecipient != ad.adRecipient— both are alreadybytes32.Solidity tests
All three test suites (
AdManager.t.sol,OrderPortal.t.sol,ProofBridge.t.sol) updated:_b32(address)helper for buildingbytes32values.OrderParamsliterals updated to usebytes32fields throughout.chains(...),ads(...),tokenRoute(...).balanceOflookups cast a storedbytes32back toaddressviaaddress(uint160(uint256(b32)))where needed.All 84 Foundry tests pass, including the cross-chain EIP-712 parity tests that hash the same
OrderParamson both sides and assert bytewise equality.Off-chain / scripts
contracts/evm/js-scripts/utils.ts— ethersorderTypesusesbytes32for every address-typed field sosignTypedDataproduces the same struct hash the contract computes.contracts/evm/js-scripts/deploy/core/utils.ts— updatedAD_MANAGER_ABI/ORDER_PORTAL_ABIfragments (setChain,setTokenRoute,chains,tokenRoutereturn types).contracts/evm/js-scripts/deploy/core/setup.ts—addrToBytes32/b32EqAddrhelpers; everysetChain/setTokenRoutecall now passes a left-paddedbytes32for the remote counterpart.scripts/cross-chain-e2e/run.ts— passes full-width 32-byte values foradManager,adToken,adCreator,adRecipient,bridger,orderRecipient,orderPortal.stellarIdToEvmAddressis gone — no more truncation.scripts/cross-chain-e2e/lib/proof.tsandcontracts/stellar/tests/fixtures/generate_fixtures.ts—ORDER_TYPEHASHstring updated to the unifiedbytes32form.Stellar side
Mostly no-ops, because Stellar already used
BytesN<32>for every cross-chain field. Two concrete updates:proofbridge-core/src/eip712.rs: newORDER_TYPEHASHconstant matching the new keccak.proofbridge-core/src/test.rs:test_order_typehashasserts the new type string.0x06e3db29ab3f869edb083ac922864cdaed9cf70a0d8e2205881a2ce5f49cbbde.Test status
proofbridge-core: 15/15 unit tests passing (includingtest_order_typehash).lock_for_order,unlock_with_bridger_proof,full_cross_chain_flow,nullifier_prevents_double_unlock,order_portal_create_and_unlock).Migration notes
This is a breaking ABI change. Any off-chain client (relayer, indexer, frontend) that encodes
OrderParams, readschains(chainId)/tokenRoute(...), or subscribes toChainSet/TokenRouteSet/OrderLocked/OrderUnlockedmust:Orderto the newbytes32shape.bridger,orderRecipient,adCreator,adRecipient,orderPortal,adManager, …) with 12 zero bytes before passing it into a contract call. On the way out, fields whose value is a local EVM address can be recovered viaaddress(uint160(uint256(b32))).Summary by CodeRabbit
New Features
Refactor