Conversation
WalkthroughReplaced container-based in-memory deployment logic with an internal contractCreationCode(bytes) helper that assembles raw creation bytecode from input data; tests updated to deploy that creation code via inline assembly and validate stored data on-chain. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test
participant Lib as LibDataContract
participant EVM as EVM (create)
participant Deployed as DeployedContract
Test->>Lib: contractCreationCode(data)
Lib-->>Test: creationCode (bytes)
Test->>EVM: create(creationCode) via inline assembly
EVM-->>Deployed: deployed contract at addr
Test->>Deployed: call read(start, length)
Deployed-->>Test: return data / slice
Test->>Test: assert keccak/length/slice equality
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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: 2
🤖 Fix all issues with AI agents
In `@src/lib/LibDataContract.sol`:
- Around line 90-102: The prefix computation uses add(mload(data), 1) without
limiting length to 2 bytes, risking silent overflow and inconsistent behavior vs
newContainer; update the prefix calculation to mask the length into 2 bytes the
same way as newContainer (use and(0xFFFF, add(mload(data), 1))) before the shl,
so the computed prefix (built from basePrefix and shifted length) always encodes
a 2-byte length; modify the code around the prefix variable calculation (the
or/shl/add calls) to apply this 2-byte mask.
- Line 107: The mcopy call is using swapped source/destination args causing the
uninitialized creation code buffer to be read and the input data to be
overwritten; update the mcopy invocation so it copies mload(data) bytes from the
input data pointer (dataPointer) into the creation code buffer pointer
(creationCodeDataPointer) by swapping the first two arguments in the mcopy call
(reference symbols: mcopy, dataPointer, creationCodeDataPointer, mload(data)).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@test/lib/LibDataContract.t.sol`:
- Around line 46-48: After the assembly block that assigns dataContract via
create(...) add an explicit zero-address check (e.g., assert or require) to fail
fast when create returns address(0); reference the local variable dataContract
and ensure this check runs before calling read so a deployment failure is
reported directly instead of being masked by ReadError from read.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test/lib/LibDataContract.t.sol (1)
5-17: 🛠️ Refactor suggestion | 🟠 MajorRemove unused imports.
console2is imported but never used. Additionally,DataContractMemoryContainer,WriteError, andZOLTU_PROXY_ADDRESSare imported but no longer referenced after the write-function removal.♻️ Proposed fix
-import {Test, console2} from "forge-std/Test.sol"; +import {Test} from "forge-std/Test.sol"; import {LibMemCpy} from "rain.solmem/lib/LibMemCpy.sol"; import {LibBytes} from "rain.solmem/lib/LibBytes.sol"; import { LibPointer, Pointer, - DataContractMemoryContainer, LibDataContract, - ReadError, - WriteError, - ZOLTU_PROXY_ADDRESS + ReadError } from "src/lib/LibDataContract.sol";src/lib/LibDataContract.sol (1)
7-7:⚠️ Potential issue | 🟡 MinorRemove unused
WriteErrorimport.WriteError is imported but never used in this file—there are no write functions that would throw it. The three exported functions are
contractCreationCode,read, andreadSlice.Proposed fix
-import {WriteError, ReadError} from "../error/ErrDataContract.sol"; +import {ReadError} from "../error/ErrDataContract.sol";
🤖 Fix all issues with AI agents
In `@test/lib/LibDataContract.t.sol`:
- Around line 46-50: Extract the repeated inline assembly deployment block into
a single internal test helper (e.g., _deployDataContract(bytes memory data)
internal returns (address)) that builds creationCode via
LibDataContract.contractCreationCode(data), performs the assembly create call,
and requires the returned address != address(0); then replace the four
duplicated 5-line assembly blocks in the tests with calls to this helper to
centralize the address(0) check and remove duplication.
- Around line 108-110: Remove the leftover commented-out old write flow in the
test by deleting the three commented lines referencing
LibDataContract.newContainer, LibMemCpy.unsafeCopyBytesTo, and
LibDataContract.write so the test no longer contains dead/commented code; ensure
no other references to this old flow remain in LibDataContract.t.sol and run
tests to confirm nothing else depends on those lines.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@test/lib/LibDataContract.t.sol`:
- Around line 51-67: Fuzz can generate data longer than EIP-170 max deployed
code (24,576 bytes) so create returns address(0) and later reads revert; in the
tests (testRoundCreationCodeFuzz, testRoundSliceError, testSameReads,
testZeroPrefix) guard the create/deploy path by constraining or skipping such
inputs — add an assumption like vm.assume(uint256(data.length) + 1 <= 24576)
before building/creating the contract (or check data.length and skip/assert on
address(0) before calling LibDataContract.read) so you never attempt to deploy
data exceeding the EVM code-size limit when using
LibDataContract.contractCreationCode and assembly create.
Comment on lines
+51
to
+67
| function testRoundCreationCodeFuzz(bytes memory data, bytes memory garbage, uint16 start, uint16 sliceLength) | ||
| external | ||
| { | ||
| bytes32 dataHash = keccak256(data); | ||
| vm.assume(uint256(start) + uint256(sliceLength) <= data.length); | ||
|
|
||
| bytes memory expectedSlice = new bytes(sliceLength); | ||
| LibMemCpy.unsafeCopyBytesTo(data.dataPointer().unsafeAddBytes(start), expectedSlice.dataPointer(), sliceLength); | ||
|
|
||
| /// Writing any data to a contract then reading it back without corrupting | ||
| /// memory or the data itself. | ||
| function testRoundFuzz(bytes memory data, bytes memory garbage) public { | ||
| // Put some garbage in unallocated memory. | ||
| LibMemCpy.unsafeCopyBytesTo(garbage.dataPointer(), LibPointer.allocatedMemoryPointer(), garbage.length); | ||
|
|
||
| (DataContractMemoryContainer container, Pointer pointer) = LibDataContract.newContainer(data.length); | ||
|
|
||
| LibMemCpy.unsafeCopyBytesTo(data.dataPointer(), pointer, data.length); | ||
|
|
||
| address datacontract = LibDataContract.write(container); | ||
|
|
||
| bytes memory round = LibDataContract.read(datacontract); | ||
| bytes memory creationCode = LibDataContract.contractCreationCode(data); | ||
| address dataContract; | ||
| assembly ("memory-safe") { | ||
| dataContract := create(0, add(creationCode, 0x20), mload(creationCode)) | ||
| } |
There was a problem hiding this comment.
Fuzz test can fail when data.length exceeds EVM max contract code size (24 576 bytes).
contractCreationCode allows up to 65 534 bytes, but EVM's EIP-170 limits deployed code to 24 576 bytes. When data.length + 1 > 24576, create silently returns address(0) and the subsequent read reverts with ReadError, causing a spurious test failure.
The same concern applies to testRoundSliceError, testSameReads, and testZeroPrefix.
Proposed fix
function testRoundCreationCodeFuzz(bytes memory data, bytes memory garbage, uint16 start, uint16 sliceLength)
external
{
+ vm.assume(data.length <= 24575);
bytes32 dataHash = keccak256(data);
vm.assume(uint256(start) + uint256(sliceLength) <= data.length);🤖 Prompt for AI Agents
In `@test/lib/LibDataContract.t.sol` around lines 51 - 67, Fuzz can generate data
longer than EIP-170 max deployed code (24,576 bytes) so create returns
address(0) and later reads revert; in the tests (testRoundCreationCodeFuzz,
testRoundSliceError, testSameReads, testZeroPrefix) guard the create/deploy path
by constraining or skipping such inputs — add an assumption like
vm.assume(uint256(data.length) + 1 <= 24576) before building/creating the
contract (or check data.length and skip/assert on address(0) before calling
LibDataContract.read) so you never attempt to deploy data exceeding the EVM
code-size limit when using LibDataContract.contractCreationCode and assembly
create.
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.
Motivation
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Tests