Skip to content

[I-4] Calls struct has different order than CALLS_TYPE_HASH#7

Open
reednaa wants to merge 1 commit intomainfrom
calls-order
Open

[I-4] Calls struct has different order than CALLS_TYPE_HASH#7
reednaa wants to merge 1 commit intomainfrom
calls-order

Conversation

@reednaa
Copy link
Member

@reednaa reednaa commented Nov 17, 2025

Reorder the Calls structure to match the typehash. New struct:

struct Calls {
  uint256 nonce;
  bytes32 mode;
  ERC7821.Call[] calls;
}

Summary by CodeRabbit

  • Refactor
    • Reorganized data structure field ordering for improved code organization without altering functionality.

@coderabbitai
Copy link

coderabbitai bot commented Nov 17, 2025

Walkthrough

The Calls struct in LibCalls.sol has been reordered to move the bytes32 mode field after the uint256 nonce field. No fields were added or removed; only their declaration order changed.

Changes

Cohort / File(s) Change Summary
Struct field reordering
src/libs/LibCalls.sol
Reordered Calls struct fields: bytes32 mode relocated from before nonce to after it; no fields added or removed

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Careful attention needed to verify that storage layout changes do not affect existing contract state or deployments
  • Check if any code relies on implicit field ordering assumptions or direct memory access patterns
  • Verify ABI compatibility requirements are met if this struct is exposed in contract interfaces

Poem

A rabbit hops through struct fields with glee, 🐰
Rearranging mode and nonce so they align just right,
Order matters in the solidity night,
Where storage slots dance in harmony!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: reordering the Calls struct fields to align with CALLS_TYPE_HASH, moving mode before nonce.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch calls-order

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 19e16e4 and a15a2ea.

📒 Files selected for processing (1)
  • src/libs/LibCalls.sol (1 hunks)
🔇 Additional comments (2)
src/libs/LibCalls.sol (2)

24-42: LGTM! The typehash function correctly passes parameters in the expected order.

Line 39 passes nonce, then mode, then the calls hash, which matches the CALLS_TYPE_HASH definition. Since this function takes individual parameters rather than a Calls struct, the struct field reordering doesn't affect its implementation.


7-11: Struct field order correctly matches CALLS_TYPE_HASH and is safe to deploy.

The reordering from (mode, nonce, calls) to (nonce, mode, calls) aligns the struct with its type hash definition at line 20. Verification shows the Calls struct is used exclusively for type hashing in the typehash() function (line 39), which already passes parameters in the new order. No code accesses struct fields directly, eliminating any risk of breaking changes.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • JIRA integration encountered authorization issues. Please disconnect and reconnect the integration in the CodeRabbit UI.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant