diff --git a/.changeset/afraid-chicken-attack.md b/.changeset/afraid-chicken-attack.md new file mode 100644 index 00000000000..9baefdc2ca7 --- /dev/null +++ b/.changeset/afraid-chicken-attack.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`Bytes`: Add `splice(bytes,uint256)` and `splice(bytes,uint256,uint256)` functions that move a specified range of bytes to the start of the buffer and truncate it in place, as an alternative to `slice`. diff --git a/contracts/utils/Bytes.sol b/contracts/utils/Bytes.sol index 1f38bce88e0..b76198e8032 100644 --- a/contracts/utils/Bytes.sol +++ b/contracts/utils/Bytes.sol @@ -80,7 +80,7 @@ library Bytes { /** * @dev Copies the content of `buffer`, from `start` (included) to `end` (excluded) into a new bytes object in - * memory. + * memory. The `end` argument is truncated to the length of the `buffer`. * * NOTE: replicates the behavior of https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/slice[Javascript's `Array.slice`] */ @@ -100,6 +100,36 @@ library Bytes { } /** + * @dev Moves the content of `buffer`, from `start` (included) to the end of `buffer` to the start of that buffer. + * + * NOTE: This function modifies the provided buffer in place. If you need to preserve the original buffer, use {slice} instead + */ + function splice(bytes memory buffer, uint256 start) internal pure returns (bytes memory) { + return splice(buffer, start, buffer.length); + } + + /** + * @dev Moves the content of `buffer`, from `start` (included) to end (excluded) to the start of that buffer. The + * `end` argument is truncated to the length of the `buffer`. + * + * NOTE: This function modifies the provided buffer in place. If you need to preserve the original buffer, use {slice} instead + */ + function splice(bytes memory buffer, uint256 start, uint256 end) internal pure returns (bytes memory) { + // sanitize + uint256 length = buffer.length; + end = Math.min(end, length); + start = Math.min(start, end); + + // allocate and copy + assembly ("memory-safe") { + mcopy(add(buffer, 0x20), add(add(buffer, 0x20), start), sub(end, start)) + mstore(buffer, sub(end, start)) + } + + return buffer; + } + + /* * @dev Reverses the byte order of a bytes32 value, converting between little-endian and big-endian. * Inspired in https://graphics.stanford.edu/~seander/bithacks.html#ReverseParallel[Reverse Parallel] */ diff --git a/test/utils/Bytes.t.sol b/test/utils/Bytes.t.sol index 20d5cb0014f..be0bf783e95 100644 --- a/test/utils/Bytes.t.sol +++ b/test/utils/Bytes.t.sol @@ -3,9 +3,84 @@ pragma solidity ^0.8.20; import {Test} from "forge-std/Test.sol"; +import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; import {Bytes} from "@openzeppelin/contracts/utils/Bytes.sol"; contract BytesTest is Test { + using Bytes for bytes; + + function testSliceWithStartOnly(bytes memory buffer, uint256 start) public pure { + bytes memory originalBuffer = bytes.concat(buffer); + bytes memory result = buffer.slice(start); + + // Original buffer was not modified + assertEq(buffer, originalBuffer); + + // Should return bytes from start to end + assertEq(result.length, Math.saturatingSub(buffer.length, start)); + + // Verify content matches + for (uint256 i = 0; i < result.length; ++i) { + assertEq(result[i], buffer[start + i]); + } + } + + function testSlice(bytes memory buffer, uint256 start, uint256 end) public pure { + bytes memory originalBuffer = bytes.concat(buffer); + bytes memory result = buffer.slice(start, end); + + // Original buffer was not modified + assertEq(buffer, originalBuffer); + + // Calculate expected bounds after sanitization + uint256 sanitizedEnd = Math.min(end, buffer.length); + uint256 sanitizedStart = Math.min(start, sanitizedEnd); + uint256 expectedLength = sanitizedEnd - sanitizedStart; + + assertEq(result.length, expectedLength); + + // Verify content matches when there's content to verify + for (uint256 i = 0; i < result.length; ++i) { + assertEq(result[i], buffer[sanitizedStart + i]); + } + } + + function testSpliceWithStartOnly(bytes memory buffer, uint256 start) public pure { + bytes memory originalBuffer = bytes.concat(buffer); + bytes memory result = buffer.splice(start); + + // Result should be the same object as input (modified in place) + assertEq(result, buffer); + + // Should contain bytes from start to end, moved to beginning + assertEq(result.length, Math.saturatingSub(originalBuffer.length, start)); + + // Verify content matches moved content + for (uint256 i = 0; i < result.length; ++i) { + assertEq(result[i], originalBuffer[start + i]); + } + } + + function testSplice(bytes memory buffer, uint256 start, uint256 end) public pure { + bytes memory originalBuffer = bytes.concat(buffer); + bytes memory result = buffer.splice(start, end); + + // Result should be the same object as input (modified in place) + assertEq(result, buffer); + + // Calculate expected bounds after sanitization + uint256 sanitizedEnd = Math.min(end, originalBuffer.length); + uint256 sanitizedStart = Math.min(start, sanitizedEnd); + uint256 expectedLength = sanitizedEnd - sanitizedStart; + + assertEq(result.length, expectedLength); + + // Verify content matches moved content + for (uint256 i = 0; i < result.length; ++i) { + assertEq(result[i], originalBuffer[sanitizedStart + i]); + } + } + // REVERSE BITS function testSymbolicReverseBytes32(bytes32 value) public pure { assertEq(Bytes.reverseBytes32(Bytes.reverseBytes32(value)), value); diff --git a/test/utils/Bytes.test.js b/test/utils/Bytes.test.js index def131e12b7..0357cac90d1 100644 --- a/test/utils/Bytes.test.js +++ b/test/utils/Bytes.test.js @@ -64,8 +64,8 @@ describe('Bytes', function () { }); }); - describe('slice', function () { - describe('slice(bytes, uint256)', function () { + describe('slice & splice', function () { + describe('slice(bytes, uint256) & splice(bytes, uint256)', function () { for (const [descr, start] of Object.entries({ 'start = 0': 0, 'start within bound': 10, @@ -74,11 +74,12 @@ describe('Bytes', function () { it(descr, async function () { const result = ethers.hexlify(lorem.slice(start)); expect(await this.mock.$slice(lorem, start)).to.equal(result); + expect(await this.mock.$splice(lorem, start)).to.equal(result); }); } }); - describe('slice(bytes, uint256, uint256)', function () { + describe('slice(bytes, uint256, uint256) & splice(bytes, uint256, uint256)', function () { for (const [descr, [start, end]] of Object.entries({ 'start = 0': [0, 42], 'start and end within bound': [17, 42], @@ -89,6 +90,7 @@ describe('Bytes', function () { it(descr, async function () { const result = ethers.hexlify(lorem.slice(start, end)); expect(await this.mock.$slice(lorem, start, ethers.Typed.uint256(end))).to.equal(result); + expect(await this.mock.$splice(lorem, start, ethers.Typed.uint256(end))).to.equal(result); }); } });