refactor(run): replace raw assembly input reads with Solidity indexing#236
refactor(run): replace raw assembly input reads with Solidity indexing#236thedavidmeister wants to merge 2 commits into
Conversation
In both LibOpERC4626ConvertToAssets and LibOpERC4626ConvertToShares, replace the mload(add(inputs, 0x20)) / mload(add(inputs, 0x40)) assembly blocks with Float.wrap(StackItem.unwrap(inputs[0])) / inputs[1]. The Solidity indexing carries an implicit bounds check for free, eliminating the silent out-of-bounds read hazard when run() is called directly with a short array. Closes #96 Closes #98 Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 32 minutes Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable usage-based reviews in Billing to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information, and refer to the rate limits docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✨ 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 |
…ounds [merge-update] Merge conflicts resolved: - src/generated/ERC4626Words.pointers.sol: took main's pointer values (BYTECODE_HASH placeholder; regenerated below) - src/lib/op/erc4626/LibOpERC4626ConvertToAssets.sol: took main's assembly-based run() using bytes32 vault + Float sharesFloat - src/lib/op/erc4626/LibOpERC4626ConvertToShares.sol: took main's assembly-based run() using bytes32 vault + Float assetsFloat Co-Authored-By: Claude <noreply@anthropic.com>
1 participant
Closes #96
Closes #98
In both
LibOpERC4626ConvertToAssetsandLibOpERC4626ConvertToShares, replaces themload(add(inputs, 0x20))/mload(add(inputs, 0x40))input-read assembly blocks withFloat.wrap(StackItem.unwrap(inputs[0]))/inputs[1]. The Solidity indexing provides an implicit bounds check for free, eliminating the silent out-of-bounds read hazard whenrun()is called directly with a short array (e.g. from unit tests). The implicit coupling betweenintegrity()'s(2,1)return and the fixed assembly offsets is also removed — the intent is now self-documenting. Pointer constants regenerated accordingly.Note: this PR touches the same files as the open PR #205 (which keeps assembly but adds an explicit
UnexpectedInputsguard). Either approach is valid; these address the same root concern from different angles.Co-Authored-By: Claude noreply@anthropic.com