-
Notifications
You must be signed in to change notification settings - Fork 28
Compression libraries: FastLZ, LZ4 and Snappy #218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughAdds three new Solidity decompression libraries (FastLZ, LZ4, Snappy) with memory and calldata variants, comprehensive JS/Foundry tests including gas benchmarking, a shared test suite, and devDependencies for compression tooling. Implementations are written in memory-safe assembly, include basic validation, and return bytes output or revert on decoding inconsistencies. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor T as Test/Caller
participant F as FastLZ
participant M as Memory
participant C as Calldata
rect rgb(240,248,255)
note over T,F: FastLZ decompression (level-1)
T->>F: decompress(input bytes)
F->>M: allocate output buffer
loop Parse chunks
F-->>F: read token (literal or back-ref)
alt Literal
F->>M: copy literal bytes
else Back-reference
F-->>F: compute length, offset
F->>M: copy from output window
end
end
F-->>T: output bytes (revert if misaligned)
end
rect rgb(245,255,240)
note over T,F: Calldata variant
T->>F: decompressCalldata(input calldata)
F-->>C: calldataload reads
F->>M: write output
F-->>T: output bytes
end
sequenceDiagram
autonumber
actor T as Test/Caller
participant L as LZ4
participant M as Memory
rect rgb(248,248,255)
note over T,L: LZ4 framed decompression
T->>L: decompress(input)
L-->>L: validate magic, version, flags
loop For each block
alt Uncompressed block
L->>M: copy block bytes
else Compressed block
loop Tokens
L-->>L: parse token (litLen/matchLen)
L->>M: copy literals
L-->>L: read offset
L->>M: copy match via sliding window
end
end
end
L-->>T: output (revert on invalid frame/EOF)
end
sequenceDiagram
autonumber
actor T as Test/Caller
participant S as Snappy
participant M as Memory
rect rgb(255,248,240)
note over T,S: Snappy uncompress
T->>S: uncompress(input)
S->>M: precompute output length, allocate
loop Tokens
alt Literal
S->>M: copy literal bytes
else Copy (1-3)
S-->>S: decode length+offset
S->>M: windowed copy
end
end
S-->>T: output (revert on mismatch)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (3)
contracts/utils/compression/Snappy.sol (3)
166-169
: Duplicate assignment to len in case 3 (calldata)Redundant line; remove the duplicate.
- len := add(shr(2, c), 1) - len := add(shr(2, c), 1) + len := add(shr(2, c), 1)
97-99
: Use the custom error DecodingFailure() instead of bare revertYou declared error DecodingFailure() but revert(0, 0) loses debuggability and consistency. Emit the custom error selector.
Example Yul pattern:
- mstore(0x00, DecodingFailure.selector) and revert(0x1c, 0x04)
- if iszero(and(eq(inputPtr, inputEnd), eq(outputPtr, mload(0x40)))) { - revert(0, 0) - } + if iszero(and(eq(inputPtr, inputEnd), eq(outputPtr, mload(0x40)))) { + // revert DecodingFailure() + mstore(0x00, /* DecodingFailure.selector */ 0x00000000) + revert(0x1c, 0x04) + }Replace 0x00000000 with the actual selector of DecodingFailure(). If you prefer, I can push a patch computing and inlining the selector.
Also applies to: 185-187
38-41
: Optional: align free memory pointer and decouple the final check from FMPBest practice is to 32-byte align the free memory pointer. If you align FMP, compare outputPtr to a saved outputEnd rather than mload(0x40).
Sketch:
- After computing outputEnd, set mstore(0x40, and(add(outputEnd, 31), not(31))).
- Keep a local let expectedEnd := outputEnd; at the end check eq(outputPtr, expectedEnd) instead of eq(outputPtr, mload(0x40)).
Also applies to: 124-127, 97-99, 185-187
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (10)
contracts/utils/compression/FastLZ.sol
(1 hunks)contracts/utils/compression/LZ4.sol
(1 hunks)contracts/utils/compression/Snappy.sol
(1 hunks)package.json
(1 hunks)test/utils/compression/FastLZ.t.sol
(1 hunks)test/utils/compression/FastLZ.test.js
(1 hunks)test/utils/compression/LZ4.test.js
(1 hunks)test/utils/compression/Snappy.test.js
(1 hunks)test/utils/compression/gas.test.js
(1 hunks)test/utils/compression/testsuite.js
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
test/utils/compression/testsuite.js (4)
test/utils/compression/FastLZ.test.js (4)
require
(1-1)require
(2-2)require
(3-3)require
(5-5)test/utils/compression/LZ4.test.js (3)
require
(1-1)require
(2-2)require
(3-3)test/utils/compression/Snappy.test.js (3)
require
(1-1)require
(2-2)require
(3-3)test/utils/compression/gas.test.js (4)
require
(1-1)require
(2-2)require
(3-3)require
(7-7)
test/utils/compression/FastLZ.test.js (3)
test/utils/compression/LZ4.test.js (7)
require
(1-1)require
(2-2)require
(3-3)mock
(10-10)raw
(22-22)hex
(23-23)compressed
(24-24)test/utils/compression/gas.test.js (6)
require
(1-1)require
(2-2)require
(3-3)require
(7-7)raw
(27-27)hex
(28-28)test/utils/compression/testsuite.js (1)
require
(1-1)
test/utils/compression/gas.test.js (1)
test/utils/compression/testsuite.js (1)
require
(1-1)
test/utils/compression/Snappy.test.js (4)
test/utils/compression/FastLZ.test.js (9)
require
(1-1)require
(2-2)require
(3-3)require
(5-5)unittests
(7-7)mock
(10-10)raw
(22-22)hex
(23-23)compressed
(24-24)test/utils/compression/LZ4.test.js (8)
require
(1-1)require
(2-2)require
(3-3)unittests
(7-7)mock
(10-10)raw
(22-22)hex
(23-23)compressed
(24-24)test/utils/compression/gas.test.js (9)
require
(1-1)require
(2-2)require
(3-3)require
(7-7)snappy
(6-6)snappy
(14-14)unittests
(9-9)raw
(27-27)hex
(28-28)test/utils/compression/testsuite.js (1)
require
(1-1)
test/utils/compression/LZ4.test.js (4)
test/utils/compression/FastLZ.test.js (9)
require
(1-1)require
(2-2)require
(3-3)require
(5-5)unittests
(7-7)mock
(10-10)raw
(22-22)hex
(23-23)compressed
(24-24)test/utils/compression/Snappy.test.js (8)
require
(1-1)require
(2-2)require
(3-3)unittests
(7-7)mock
(10-10)raw
(22-22)hex
(23-23)compressed
(24-24)test/utils/compression/gas.test.js (8)
require
(1-1)require
(2-2)require
(3-3)require
(7-7)lz4js
(5-5)unittests
(9-9)raw
(27-27)hex
(28-28)test/utils/compression/testsuite.js (1)
require
(1-1)
🪛 GitHub Check: codespell
test/utils/compression/testsuite.js
[failure] 28-28:
varius ==> various
[failure] 27-27:
Nam ==> Name
[failure] 25-25:
varius ==> various
⏰ 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). (3)
- GitHub Check: slither
- GitHub Check: tests
- GitHub Check: coverage
case 7 { | ||
let len := add(9, byte(1, chunk)) | ||
for { | ||
let i := 0 | ||
let ofs := add(add(shl(8, and(first, 31)), byte(2, chunk)), 1) | ||
let ref := sub(outputPtr, ofs) | ||
let step := xor(len, mul(lt(ofs, len), xor(ofs, len))) | ||
} lt(i, len) { | ||
i := add(i, step) | ||
} { | ||
mcopy(add(outputPtr, i), add(ref, i), step) | ||
} | ||
inputPtr := add(inputPtr, 3) | ||
outputPtr := add(outputPtr, len) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix FastLZ type‑7 byte ordering.
For type‑7 matches the format is ctrl | offsetLow | lengthExtra
. The code currently reads byte(1, chunk)
as the length byte and byte(2, chunk)
as the offset byte, then advances inputPtr
by 3. This swaps the order: valid streams produced by the reference encoder/LibZip place the offset immediately after ctrl
, so we subtract the wrong offset (using the length byte) and read the real offset as the next chunk’s first byte. Result: long matches (>=9 bytes) decode garbage or revert when the pointer check at Line 68 trips.
Reorder the reads so we pull the offset from byte(1, chunk)
, the length extension from byte(2, chunk)
, and only consume the extra length byte when we actually use it. One possible fix:
- case 7 {
- let len := add(9, byte(1, chunk))
- for {
- let i := 0
- let ofs := add(add(shl(8, and(first, 31)), byte(2, chunk)), 1)
+ case 7 {
+ let extra := byte(2, chunk)
+ let ofs := add(add(shl(8, and(first, 31)), byte(1, chunk)), 1)
+ let len := add(9, extra)
+ for {
+ let i := 0
let ref := sub(outputPtr, ofs)
let step := xor(len, mul(lt(ofs, len), xor(ofs, len)))
} lt(i, len) {
i := add(i, step)
} {
mcopy(add(outputPtr, i), add(ref, i), step)
}
- inputPtr := add(inputPtr, 3)
+ inputPtr := add(inputPtr, 3)
(and mirror the change in the calldata loop).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
case 7 { | |
let len := add(9, byte(1, chunk)) | |
for { | |
let i := 0 | |
let ofs := add(add(shl(8, and(first, 31)), byte(2, chunk)), 1) | |
let ref := sub(outputPtr, ofs) | |
let step := xor(len, mul(lt(ofs, len), xor(ofs, len))) | |
} lt(i, len) { | |
i := add(i, step) | |
} { | |
mcopy(add(outputPtr, i), add(ref, i), step) | |
} | |
inputPtr := add(inputPtr, 3) | |
outputPtr := add(outputPtr, len) | |
} | |
case 7 { | |
// pull offset low first, then length extension | |
let extra := byte(2, chunk) | |
let ofs := add(add(shl(8, and(first, 31)), byte(1, chunk)), 1) | |
let len := add(9, extra) | |
for { | |
let i := 0 | |
let ref := sub(outputPtr, ofs) | |
let step:= xor(len, mul(lt(ofs, len), xor(ofs, len))) | |
} lt(i, len) { | |
i := add(i, step) | |
} { | |
mcopy(add(outputPtr, i), add(ref, i), step) | |
} | |
inputPtr := add(inputPtr, 3) | |
outputPtr := add(outputPtr, len) | |
} |
🤖 Prompt for AI Agents
In contracts/utils/compression/FastLZ.sol around lines 37 to 51, the type-7
branch reads the bytes in the wrong order (using byte(1, chunk) as the length
and byte(2, chunk) as the offset) and always advances inputPtr by 3; update it
to read the offset from byte(1, chunk) and the length-extension from byte(2,
chunk), compute ofs using the correct offset byte, only advance inputPtr by 2
normally and by 3 when the extra length byte is actually consumed, and mirror
the same byte-order and pointer-advance fix in the calldata decoding loop so
offsets and lengths match the reference encoder/LibZip format.
output := mload(0x40) | ||
let outputPtr := add(output, 0x20) | ||
mstore(output, outputLength) | ||
mstore(0x40, add(outputPtr, outputLength)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard FMP computation against overflow
add(outputPtr, outputLength) can wrap on malicious varint; guard before storing to 0x40.
- mstore(0x40, add(outputPtr, outputLength))
+ let outputEnd := add(outputPtr, outputLength)
+ // overflow guard: outputEnd must be >= outputPtr
+ if lt(outputEnd, outputPtr) { revert(0, 0) }
+ mstore(0x40, outputEnd)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
output := mload(0x40) | |
let outputPtr := add(output, 0x20) | |
mstore(output, outputLength) | |
mstore(0x40, add(outputPtr, outputLength)) | |
output := mload(0x40) | |
let outputPtr := add(output, 0x20) | |
mstore(output, outputLength) | |
let outputEnd := add(outputPtr, outputLength) | |
// overflow guard: outputEnd must be >= outputPtr | |
if lt(outputEnd, outputPtr) { revert(0, 0) } | |
mstore(0x40, outputEnd) |
🤖 Prompt for AI Agents
In contracts/utils/compression/Snappy.sol around lines 38 to 41, the
mstore(0x40, add(outputPtr, outputLength)) writes the new free-memory pointer
without checking for overflow from add(outputPtr, outputLength); compute the end
pointer into a temp variable, validate that end >= outputPtr (and optionally
that end does not exceed a reasonable bound), and revert/invalid on failure
before calling mstore(0x40, end). Implement the check in assembly: compute let
end := add(outputPtr, outputLength), if lt(end, outputPtr) { revert(0,0) } (or
use invalid) else mstore(0x40, end).
mcopy(outputPtr, inputPtr, len) | ||
inputPtr := add(inputPtr, len) | ||
outputPtr := add(outputPtr, len) | ||
// continue to skip the offset copy logic that is shared by the other 3 cases |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bounds-check literal copy in memory variant
In the memory path, mcopy can read past the input slice without an immediate revert. Add a check to fail early.
- mcopy(outputPtr, inputPtr, len)
+ // bounds check: inputPtr + len must not exceed inputEnd
+ if gt(add(inputPtr, len), inputEnd) { revert(0, 0) }
+ mcopy(outputPtr, inputPtr, len)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
mcopy(outputPtr, inputPtr, len) | |
inputPtr := add(inputPtr, len) | |
outputPtr := add(outputPtr, len) | |
// continue to skip the offset copy logic that is shared by the other 3 cases | |
// bounds check: inputPtr + len must not exceed inputEnd | |
if gt(add(inputPtr, len), inputEnd) { revert(0, 0) } | |
mcopy(outputPtr, inputPtr, len) | |
inputPtr := add(inputPtr, len) | |
outputPtr := add(outputPtr, len) | |
// continue to skip the offset copy logic that is shared by the other 3 cases |
// copying in will not work if the offset is larger than the len being copied, so we compute | ||
// `step = Math.min(len, offset)` and use it for the memory copy in chunks | ||
for { | ||
let ptr := outputPtr | ||
let end := add(outputPtr, len) | ||
let step := xor(offset, mul(lt(len, offset), xor(len, offset))) // min(len, offset) | ||
} lt(ptr, end) { | ||
ptr := add(ptr, step) | ||
} { | ||
mcopy(ptr, sub(ptr, offset), step) | ||
} | ||
outputPtr := add(outputPtr, len) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard invalid offsets to avoid infinite loop and OOB back-references
- offset == 0 makes step == 0, causing an infinite loop.
- offset > bytes already produced reads before the start of the output buffer (memory disclosure/corruption).
Add hard checks before the back-reference copy.
// copying in will not work if the offset is larger than the len being copied, so we compute
// `step = Math.min(len, offset)` and use it for the memory copy in chunks
+ // Reject invalid offsets: zero would stall the loop; too-large offsets read before output start.
+ if or(iszero(offset), gt(offset, sub(outputPtr, add(output, 0x20)))) { revert(0, 0) }
for {
let ptr := outputPtr
let end := add(outputPtr, len)
let step := xor(offset, mul(lt(len, offset), xor(len, offset))) // min(len, offset)
} lt(ptr, end) {
ptr := add(ptr, step)
} {
mcopy(ptr, sub(ptr, offset), step)
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// copying in will not work if the offset is larger than the len being copied, so we compute | |
// `step = Math.min(len, offset)` and use it for the memory copy in chunks | |
for { | |
let ptr := outputPtr | |
let end := add(outputPtr, len) | |
let step := xor(offset, mul(lt(len, offset), xor(len, offset))) // min(len, offset) | |
} lt(ptr, end) { | |
ptr := add(ptr, step) | |
} { | |
mcopy(ptr, sub(ptr, offset), step) | |
} | |
outputPtr := add(outputPtr, len) | |
} | |
// copying in will not work if the offset is larger than the len being copied, so we compute | |
// `step = Math.min(len, offset)` and use it for the memory copy in chunks | |
// Reject invalid offsets: zero would stall the loop; too-large offsets read before output start. | |
if or(iszero(offset), gt(offset, sub(outputPtr, add(output, 0x20)))) { revert(0, 0) } | |
for { | |
let ptr := outputPtr | |
let end := add(outputPtr, len) | |
let step := xor(offset, mul(lt(len, offset), xor(len, offset))) // min(len, offset) | |
} lt(ptr, end) { | |
ptr := add(ptr, step) | |
} { | |
mcopy(ptr, sub(ptr, offset), step) | |
} | |
outputPtr := add(outputPtr, len) | |
} |
🤖 Prompt for AI Agents
In contracts/utils/compression/Snappy.sol around lines 83 to 95, the
back-reference copy loop fails to guard invalid offsets (offset == 0 yields step
== 0 causing an infinite loop, and offset > bytes already produced reads memory
before the output buffer). Before computing step and entering the loop, add
explicit assembly checks that revert on invalid back-references: if offset is
zero revert, and if offset is greater than the number of bytes already produced
(computed as sub(outputPtr, destPtr) or equivalent) revert; only then compute
step = min(len, offset) and proceed with the loop. Ensure the revert uses a safe
mechanism (e.g. revert(0,0) or a proper error selector) and that produced-length
uses the pointers available in the surrounding code.
output := mload(0x40) | ||
let outputPtr := add(output, 0x20) | ||
mstore(output, outputLength) | ||
mstore(0x40, add(outputPtr, outputLength)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard FMP computation against overflow (calldata variant)
Mirror the overflow guard in the calldata path.
- mstore(output, outputLength)
- mstore(0x40, add(outputPtr, outputLength))
+ mstore(output, outputLength)
+ let outputEnd := add(outputPtr, outputLength)
+ if lt(outputEnd, outputPtr) { revert(0, 0) }
+ mstore(0x40, outputEnd)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
output := mload(0x40) | |
let outputPtr := add(output, 0x20) | |
mstore(output, outputLength) | |
mstore(0x40, add(outputPtr, outputLength)) | |
output := mload(0x40) | |
let outputPtr := add(output, 0x20) | |
mstore(output, outputLength) | |
let outputEnd := add(outputPtr, outputLength) | |
if lt(outputEnd, outputPtr) { revert(0, 0) } | |
mstore(0x40, outputEnd) |
🤖 Prompt for AI Agents
In contracts/utils/compression/Snappy.sol around lines 124 to 127, add the same
overflow check used in the calldata path when advancing the free memory pointer:
compute the new pointer (end = outputPtr + outputLength), validate that end is
not less than outputPtr (indicating overflow) and revert/invalid if it is, then
store end into 0x40; this mirrors the calldata-path guard and prevents FMP
wraparound.
// copy len bytes from input to output in chunks of 32 bytes | ||
calldatacopy(outputPtr, inputPtr, len) | ||
inputPtr := add(inputPtr, len) | ||
outputPtr := add(outputPtr, len) | ||
// continue to skip the offset copy logic that is shared by the other 3 cases |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bounds-check literal copy in calldata variant
Pre-check avoids accidental OOB calldatacopy reverts and clarifies intent.
- // copy len bytes from input to output in chunks of 32 bytes
- calldatacopy(outputPtr, inputPtr, len)
+ // bounds check: inputPtr + len must not exceed inputEnd
+ if gt(add(inputPtr, len), inputEnd) { revert(0, 0) }
+ calldatacopy(outputPtr, inputPtr, len)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// copy len bytes from input to output in chunks of 32 bytes | |
calldatacopy(outputPtr, inputPtr, len) | |
inputPtr := add(inputPtr, len) | |
outputPtr := add(outputPtr, len) | |
// continue to skip the offset copy logic that is shared by the other 3 cases | |
// bounds check: inputPtr + len must not exceed inputEnd | |
if gt(add(inputPtr, len), inputEnd) { revert(0, 0) } | |
calldatacopy(outputPtr, inputPtr, len) | |
inputPtr := add(inputPtr, len) | |
outputPtr := add(outputPtr, len) | |
// continue to skip the offset copy logic that is shared by the other 3 cases |
// copying in will not work if the offset is larger than the len being copied, so we compute | ||
// `step = Math.min(len, offset)` and use it for the memory copy in chunks | ||
for { | ||
let ptr := outputPtr | ||
let end := add(outputPtr, len) | ||
let step := xor(offset, mul(lt(len, offset), xor(len, offset))) // min(len, offset) | ||
} lt(ptr, end) { | ||
ptr := add(ptr, step) | ||
} { | ||
mcopy(ptr, sub(ptr, offset), step) | ||
} | ||
outputPtr := add(outputPtr, len) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard invalid offsets in calldata variant
Same offset checks as memory variant to prevent infinite loops and OOB reads.
// `step = Math.min(len, offset)` and use it for the memory copy in chunks
for {
let ptr := outputPtr
let end := add(outputPtr, len)
let step := xor(offset, mul(lt(len, offset), xor(len, offset))) // min(len, offset)
} lt(ptr, end) {
ptr := add(ptr, step)
} {
- mcopy(ptr, sub(ptr, offset), step)
+ // Reject invalid offsets: zero would stall the loop; too-large offsets read before output start.
+ if or(iszero(offset), gt(offset, sub(outputPtr, add(output, 0x20)))) { revert(0, 0) }
+ mcopy(ptr, sub(ptr, offset), step)
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// copying in will not work if the offset is larger than the len being copied, so we compute | |
// `step = Math.min(len, offset)` and use it for the memory copy in chunks | |
for { | |
let ptr := outputPtr | |
let end := add(outputPtr, len) | |
let step := xor(offset, mul(lt(len, offset), xor(len, offset))) // min(len, offset) | |
} lt(ptr, end) { | |
ptr := add(ptr, step) | |
} { | |
mcopy(ptr, sub(ptr, offset), step) | |
} | |
outputPtr := add(outputPtr, len) | |
} | |
// copying in will not work if the offset is larger than the len being copied, so we compute | |
// `step = Math.min(len, offset)` and use it for the memory copy in chunks | |
for { | |
let ptr := outputPtr | |
let end := add(outputPtr, len) | |
let step := xor(offset, mul(lt(len, offset), xor(len, offset))) // min(len, offset) | |
} lt(ptr, end) { | |
ptr := add(ptr, step) | |
} { | |
// Reject invalid offsets: zero would stall the loop; too-large offsets read before output start. | |
if or(iszero(offset), gt(offset, sub(outputPtr, add(output, 0x20)))) { revert(0, 0) } | |
mcopy(ptr, sub(ptr, offset), step) | |
} | |
outputPtr := add(outputPtr, len) |
🤖 Prompt for AI Agents
contracts/utils/compression/Snappy.sol lines 171-183: the calldata copy loop
currently assumes the provided offset is valid, which can cause infinite loops
or out-of-bounds reads; mirror the same guards used in the memory variant by
validating the offset before entering the loop (ensure offset > 0 and cap offset
to len or revert if offset > len), compute step as min(len, offset) safely, and
if step is zero revert or handle as no-op so the loop cannot become infinite or
perform OOB mcopy.
See: OpenZeppelin/openzeppelin-contracts#5820
Summary by CodeRabbit
New Features
Tests
Chores