Skip to content

Conversation

@maradini77
Copy link

Summary

Fixes memory pointer alignment issue in LowLevelCall.returnData() function.

Problem

The free memory pointer at 0x40 was not aligned to 32-byte boundary, which violates Solidity ABI expectations and can cause incorrect subsequent allocations.

Solution

  • Align free memory pointer to 32-byte boundary using bitwise operations

@maradini77 maradini77 requested a review from a team as a code owner October 21, 2025 18:57
@changeset-bot
Copy link

changeset-bot bot commented Oct 21, 2025

⚠️ No Changeset found

Latest commit: bcfd6cb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link

coderabbitai bot commented Oct 21, 2025

Walkthrough

The LowLevelCall.sol contract has been modified to change how the free-memory pointer is updated following a returnData memory write operation. Instead of incrementing the pointer by the raw returndatasize, the pointer now advances to the next 32-byte aligned address after the returned data. This modification maintains the existing data copying mechanism while ensuring proper 32-byte memory alignment for the resulting bytes memory object. No public API changes were introduced.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "Fix memory pointer alignment in LowLevelCall.returnData()" directly aligns with the changeset. The raw summary and PR description both confirm that the primary change is fixing memory pointer alignment in the LowLevelCall.returnData() function by ensuring the free-memory pointer is aligned to a 32-byte boundary. The title is concise, specific, and clearly identifies both the function being modified and the nature of the fix, making it easy for teammates scanning the history to understand the primary change.
Description Check ✅ Passed The pull request description is clearly related to the changeset and provides meaningful context about the fix. It uses a structured format explaining the problem (unaligned free memory pointer violating Solidity ABI expectations), the specific function affected (LowLevelCall.returnData()), and the solution (aligning the pointer to 32-byte boundary using bitwise operations). The description is neither vague nor generic, and directly corresponds to the changes summarized in the raw summary.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1cf1377 and bcfd6cb.

📒 Files selected for processing (1)
  • contracts/utils/LowLevelCall.sol (1 hunks)
⏰ 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). (6)
  • GitHub Check: tests
  • GitHub Check: coverage
  • GitHub Check: halmos
  • GitHub Check: slither
  • GitHub Check: tests-upgradeable
  • GitHub Check: tests-foundry
🔇 Additional comments (1)
contracts/utils/LowLevelCall.sol (1)

108-108: LGTM! Alignment fix is mathematically correct.

The alignment logic properly rounds up returndatasize() to the next 32-byte boundary using and(add(returndatasize(), 0x1f), not(0x1f)), maintaining Solidity's memory management requirements and the "memory-safe" annotation. The function is used across multiple critical components (Address.sol, Create2.sol, draft-AccountERC7579.sol, ERC4626.sol).

No dedicated unit tests were found for returnData(). Consider adding test coverage for edge cases (non-aligned sizes like 1, 17, 31, 33 and aligned sizes like 32, 64, 96) to verify proper memory pointer alignment across all return data sizes.


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