Conversation
❌ This pull request cannot be evaluated by MergifyDetailsThe pull request reports more than 18400 files, reaching the limit of 10000 files. |
2 similar comments
❌ This pull request cannot be evaluated by MergifyDetailsThe pull request reports more than 18400 files, reaching the limit of 10000 files. |
❌ This pull request cannot be evaluated by MergifyDetailsThe pull request reports more than 18400 files, reaching the limit of 10000 files. |
❌ This pull request cannot be evaluated by MergifyDetailsThe pull request reports more than 18400 files, reaching the limit of 10000 files. |
![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
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| // We represent this as a bytes32 with zero-padding | ||
| bytes32 preimage = keccak256(abi.encodePacked(celestiaICAModule, salt)); | ||
| // Take last 20 bytes (like Cosmos address derivation) | ||
| return preimage; |
There was a problem hiding this comment.
Bug: Address derivation uses wrong hash and no truncation
The _deriveCelestiaAddress function has two issues that would cause cross-chain address mismatches. According to the documentation in IMPLEMENTATION_STATUS.md, the final address derivation should be sha256(addressPreimage)[:20], but the implementation uses keccak256 instead of sha256, and returns the full 32-byte hash without truncating to 20 bytes. The comment even acknowledges this with "Take last 20 bytes" but the code never performs this truncation. This would cause the Solidity-computed forwarding addresses to never match the Cosmos SDK addresses on Celestia, breaking the entire forwarding mechanism.
| bytes32 constant CELESTIA_ICA_MODULE = bytes32(uint256(0x696361000000000000000000000000000000000000000000000000000000)); | ||
|
|
||
| // Celestia ICA router address (Hyperlane ICA router identifier on Celestia) | ||
| bytes32 constant CELESTIA_ICA_ROUTER = bytes32(uint256(0x696361726f7574657200000000000000000000000000000000000000000000)); |
There was a problem hiding this comment.
Bug: Bytes32 conversion causes wrong byte alignment for module identifiers
The bytes32(uint256(...)) conversion pattern used for CELESTIA_ICA_MODULE and CELESTIA_ICA_ROUTER produces incorrectly aligned bytes. When a hex literal shorter than 64 characters is cast through uint256 then to bytes32, the bytes become right-aligned (padded with leading zeros). For example, bytes32(uint256(0x696361...)) with 60 hex chars produces 0x00696361... instead of the expected left-aligned 0x696361...0000. This contrasts with the direct literal assignment in EnrollCelestiaDomain.s.sol. Since these values are passed to CelestiaICAHelper and used in address derivation, this byte shift would cause forwarding addresses to mismatch the Cosmos SDK implementation.
❌ This pull request cannot be evaluated by MergifyDetailsThe pull request reports more than 18400 files, reaching the limit of 10000 files. |
1 similar comment
❌ This pull request cannot be evaluated by MergifyDetailsThe pull request reports more than 18400 files, reaching the limit of 10000 files. |
Manav-Aggarwal
force-pushed
the
manav/prototype-ica
branch
from
3c48d86 to
ba74c4f
Compare
Manav-Aggarwal
force-pushed
the
manav/prototype-ica
branch
from
ba74c4f to
8e126e3
Compare
1 participant
Overview
Note
Vendors noble-curves and noble-hashes into the SDK, adding elliptic curves, hashes, and KDFs for cryptographic operations.
@noble/curvessources (secp256k1, p256/p384/p521, ed25519/ed448, bls12-381, bn254, misc) and core abstractions (weierstrass/edwards, tower fields).@noble/hashessources (sha2/sha3, blake1/2/3, hmac, hkdf, pbkdf2, scrypt, argon2, eskdf) with utils and crypto adapters.Written by Cursor Bugbot for commit 36cc939. This will update automatically on new commits. Configure here.