-
Notifications
You must be signed in to change notification settings - Fork 12.1k
Add Memory
utility library
#5189
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
Changes from 18 commits
9eb5f1c
2d397f4
2a0fb7e
a7e61c3
1aae8bb
1b2679a
d514606
14fa04e
d0d55fc
608e3cd
ac92bb4
6094bb7
6bb96d5
860e5a8
ecdb768
95907aa
124ccee
c3237df
27f0a9b
e67e8b4
3847050
4fd1947
c4e0375
340e94c
f03d149
e5e9103
05a2890
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'openzeppelin-solidity': minor | ||
--- | ||
|
||
`Memory`: Add library with utilities to manipulate memory |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
// SPDX-License-Identifier: MIT | ||
|
||
pragma solidity ^0.8.20; | ||
|
||
/** | ||
* @dev Utilities to manipulate memory. | ||
* | ||
* Memory is a contiguous and dynamic byte array in which Solidity stores non-primitive types. | ||
* This library provides functions to manipulate pointers to this dynamic array. | ||
* | ||
* WARNING: When manipulating memory, make sure to follow the Solidity documentation | ||
* guidelines for https://docs.soliditylang.org/en/v0.8.20/assembly.html#memory-safety[Memory Safety]. | ||
*/ | ||
library Memory { | ||
type Pointer is bytes32; | ||
|
||
/// @dev Returns a `Pointer` to the current free `Pointer`. | ||
function getFreePointer() internal pure returns (Pointer ptr) { | ||
assembly ("memory-safe") { | ||
ptr := mload(0x40) | ||
} | ||
} | ||
|
||
/// @dev Sets the free `Pointer` to a specific value. | ||
/// | ||
/// WARNING: Everything after the pointer may be overwritten. | ||
ernestognw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
function setFreePointer(Pointer ptr) internal pure { | ||
assembly ("memory-safe") { | ||
mstore(0x40, ptr) | ||
} | ||
} | ||
|
||
/// @dev Returns a `Pointer` to the content of a `bytes` buffer. Skips the length word. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Everything from here onward (line 33 to 103) was recently added with no discussion about the usecase. I was personally happy with having utils to "cleanup" memory manually, which AFAIK was the original idea behind this PR. Some of the things here ressemble `Bytes._unsafeReadBytesOffset, which was left private (and not internal) because we could not guarantee the memory safety. This library goes way furter, with assembly marked memory safe when its not something we know (depends on the user input). Using the copy function, its quite easy to break stuff by writting to the wrong locations (for example to 0x60). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The background for these changes is that they're used by the RLP library and we previously kept private: I agree with your take. However I think it's worth pursuing this pattern since it composes pretty well imo (as long as we add proper WARNINGS and docs). See how it looks in some places in RLP: function readRawBytes(Item memory item) internal pure returns (bytes memory) {
uint256 itemLength = item.length;
bytes memory result = new bytes(itemLength);
result.contentPointer().copy(item.ptr, itemLength);
return result;
} I would be fine removing these functions. Want to hear your thoughts on the pattern There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than exposing these in Would also consider constraining There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Amxx Worth noting the original proveth implementation does essentially the same, but manually, without /*
* @param src Pointer to source
* @param dest Pointer to destination
* @param len Amount of memory to copy from the source
*/
function copy(uint src, uint dest, uint len) private pure {
if (len == 0) return;
// copy as many word sizes as possible
for (; len >= WORD_SIZE; len -= WORD_SIZE) {
assembly {
mstore(dest, mload(src))
}
src += WORD_SIZE;
dest += WORD_SIZE;
}
// left over bytes. Mask is used to remove unwanted bytes from the word
uint mask = 256 ** (WORD_SIZE - len) - 1;
assembly {
let srcpart := and(mload(src), not(mask)) // zero out src
let destpart := and(mload(dest), mask) // retrieve the bytes
mstore(dest, or(destpart, srcpart))
}
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed all new controversial functions. Opened #5792 instead. Also updated the RLP PR (#5680) accordingly as suggested @0xClandestine |
||
function contentPointer(bytes memory buffer) internal pure returns (Pointer) { | ||
return addOffset(asPointer(buffer), 32); | ||
} | ||
|
||
/** | ||
* @dev Copies `length` bytes from `srcPtr` to `destPtr`. Equivalent to https://www.evm.codes/?fork=cancun#5e[`mcopy`]. | ||
* | ||
* WARNING: Reading or writing beyond the allocated memory bounds of either pointer | ||
* will result in undefined behavior and potential memory corruption. | ||
*/ | ||
function copy(Pointer destPtr, Pointer srcPtr, uint256 length) internal pure { | ||
assembly ("memory-safe") { | ||
mcopy(destPtr, srcPtr, length) | ||
} | ||
} | ||
|
||
/** | ||
* @dev Extracts a `bytes1` from a `Pointer`. `offset` starts from the most significant byte. | ||
* | ||
* NOTE: Will return `0x00` if `offset` is larger or equal to `32`. | ||
*/ | ||
function extractByte(Pointer ptr, uint256 offset) internal pure returns (bytes1 v) { | ||
bytes32 word = extractWord(ptr); | ||
assembly ("memory-safe") { | ||
v := byte(offset, word) | ||
} | ||
} | ||
|
||
/// @dev Extracts a `bytes32` from a `Pointer`. | ||
function extractWord(Pointer ptr) internal pure returns (bytes32 v) { | ||
assembly ("memory-safe") { | ||
v := mload(ptr) | ||
} | ||
} | ||
|
||
/// @dev Adds an offset to a `Pointer`. | ||
function addOffset(Pointer ptr, uint256 offset) internal pure returns (Pointer) { | ||
return asPointer(bytes32(asUint256(ptr) + offset)); | ||
} | ||
|
||
/// @dev `Pointer` to `bytes32`. Expects a pointer to a properly ABI-encoded `bytes` object. | ||
arr00 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
function asBytes32(Pointer ptr) internal pure returns (bytes32) { | ||
return Pointer.unwrap(ptr); | ||
} | ||
|
||
/// @dev `Pointer` to `uint256`. Expects a pointer to a properly ABI-encoded `bytes` object. | ||
function asUint256(Pointer ptr) internal pure returns (uint256) { | ||
return uint256(asBytes32(ptr)); | ||
} | ||
|
||
/// @dev `bytes32` to `Pointer`. Expects a pointer to a properly ABI-encoded `bytes` object. | ||
function asPointer(bytes32 value) internal pure returns (Pointer) { | ||
return Pointer.wrap(value); | ||
} | ||
|
||
/// @dev Returns a `Pointer` to the `value`'s header (i.e. includes the length word). | ||
function asPointer(bytes memory value) internal pure returns (Pointer) { | ||
bytes32 ptr; | ||
assembly ("memory-safe") { | ||
ptr := value | ||
} | ||
return asPointer(ptr); | ||
} | ||
|
||
/// @dev `Pointer` to `bytes`. Expects a pointer to a properly ABI-encoded `bytes` object. | ||
function asBytes(Pointer ptr) internal pure returns (bytes memory b) { | ||
assembly ("memory-safe") { | ||
b := ptr | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
// SPDX-License-Identifier: MIT | ||
|
||
pragma solidity ^0.8.20; | ||
|
||
import {Test} from "forge-std/Test.sol"; | ||
import {Memory} from "@openzeppelin/contracts/utils/Memory.sol"; | ||
|
||
contract MemoryTest is Test { | ||
using Memory for *; | ||
|
||
// - first 0x80 bytes are reserved (scratch + FMP + zero) | ||
uint256 constant START_PTR = 0x80; | ||
// - moving the free memory pointer to far causes OOG errors | ||
uint256 constant END_PTR = type(uint24).max; | ||
|
||
function testGetSetFreePointer(uint256 seed) public pure { | ||
bytes32 ptr = bytes32(bound(seed, START_PTR, END_PTR)); | ||
ptr.asPointer().setFreePointer(); | ||
assertEq(Memory.getFreePointer().asBytes32(), ptr); | ||
} | ||
|
||
function testSymbolicContentPointer(uint256 seed) public pure { | ||
Memory.Pointer ptr = bytes32(bound(seed, START_PTR, END_PTR)).asPointer(); | ||
assertEq(ptr.asBytes().contentPointer().asBytes32(), ptr.addOffset(32).asBytes32()); | ||
} | ||
|
||
function testCopy(bytes memory data, uint256 destSeed) public pure { | ||
uint256 minDestPtr = Memory.getFreePointer().asUint256(); | ||
Memory.Pointer destPtr = bytes32(bound(destSeed, minDestPtr, minDestPtr + END_PTR)).asPointer(); | ||
destPtr.addOffset(data.length + 32).setFreePointer(); | ||
destPtr.copy(data.asPointer(), data.length + 32); | ||
bytes memory copiedData = destPtr.asBytes(); | ||
assertEq(data.length, copiedData.length); | ||
for (uint256 i = 0; i < data.length; i++) { | ||
assertEq(data[i], copiedData[i]); | ||
} | ||
} | ||
|
||
function testExtractByte(uint256 seed, uint256 index, bytes32 value) public pure { | ||
index = bound(index, 0, 31); | ||
Memory.Pointer ptr = bytes32(bound(seed, START_PTR, END_PTR)).asPointer(); | ||
|
||
assembly ("memory-safe") { | ||
mstore(ptr, value) | ||
} | ||
|
||
bytes1 expected; | ||
assembly ("memory-safe") { | ||
expected := byte(index, value) | ||
} | ||
assertEq(ptr.extractByte(index), expected); | ||
} | ||
|
||
function testExtractWord(uint256 seed, bytes32 value) public pure { | ||
Memory.Pointer ptr = bytes32(bound(seed, START_PTR, END_PTR)).asPointer(); | ||
assembly ("memory-safe") { | ||
mstore(ptr, value) | ||
} | ||
assertEq(ptr.extractWord(), value); | ||
} | ||
|
||
function testSymbolicAddOffset(uint256 seed, uint256 offset) public pure { | ||
offset = bound(offset, 0, type(uint256).max - END_PTR); | ||
Memory.Pointer ptr = bytes32(bound(seed, START_PTR, END_PTR)).asPointer(); | ||
assertEq(ptr.addOffset(offset).asUint256(), ptr.asUint256() + offset); | ||
} | ||
} |
ernestognw marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
const { ethers } = require('hardhat'); | ||
const { expect } = require('chai'); | ||
const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); | ||
|
||
async function fixture() { | ||
const mock = await ethers.deployContract('$Memory'); | ||
|
||
return { mock }; | ||
} | ||
|
||
describe('Memory', function () { | ||
beforeEach(async function () { | ||
Object.assign(this, await loadFixture(fixture)); | ||
}); | ||
|
||
describe('free pointer', function () { | ||
it('sets free memory pointer', async function () { | ||
const ptr = ethers.toBeHex(0xa0, 32); | ||
await expect(this.mock.$setFreePointer(ptr)).to.not.be.reverted; | ||
}); | ||
|
||
it('gets free memory pointer', async function () { | ||
await expect(this.mock.$getFreePointer()).to.eventually.equal( | ||
ethers.toBeHex(0x80, 32), // Default pointer | ||
); | ||
}); | ||
}); | ||
|
||
it('extractWord extracts a word', async function () { | ||
const ptr = await this.mock.$getFreePointer(); | ||
await expect(this.mock.$extractWord(ptr)).to.eventually.equal(ethers.toBeHex(0, 32)); | ||
}); | ||
|
||
it('extractByte extracts a byte', async function () { | ||
const ptr = await this.mock.$getFreePointer(); | ||
await expect(this.mock.$extractByte(ptr, 0)).to.eventually.equal(ethers.toBeHex(0, 1)); | ||
}); | ||
|
||
it('contentPointer', async function () { | ||
const data = ethers.toUtf8Bytes('hello world'); | ||
const result = await this.mock.$contentPointer(data); | ||
expect(result).to.equal(ethers.toBeHex(0xa0, 32)); // 0x80 is the default free pointer (length) | ||
}); | ||
|
||
describe('addOffset', function () { | ||
it('addOffset', async function () { | ||
const basePtr = ethers.toBeHex(0x80, 32); | ||
const offset = 32; | ||
const expectedPtr = ethers.toBeHex(0xa0, 32); | ||
|
||
await expect(this.mock.$addOffset(basePtr, offset)).to.eventually.equal(expectedPtr); | ||
}); | ||
|
||
it('addOffsetwraps around', async function () { | ||
const basePtr = ethers.toBeHex(0x80, 32); | ||
const offset = 256; | ||
const expectedPtr = ethers.toBeHex(0x180, 32); | ||
await expect(this.mock.$addOffset(basePtr, offset)).to.eventually.equal(expectedPtr); | ||
}); | ||
}); | ||
|
||
describe('pointer conversions', function () { | ||
it('asBytes32 / asPointer', async function () { | ||
const ptr = ethers.toBeHex('0x1234', 32); | ||
await expect(this.mock.$asBytes32(ptr)).to.eventually.equal(ptr); | ||
await expect(this.mock.$asPointer(ethers.Typed.bytes32(ptr))).to.eventually.equal(ptr); | ||
}); | ||
|
||
it('asBytes / asPointer', async function () { | ||
const ptr = await this.mock.$asPointer(ethers.Typed.bytes(ethers.toUtf8Bytes('hello world'))); | ||
expect(ptr).to.equal(ethers.toBeHex(0x80, 32)); // Default free pointer | ||
await expect(this.mock.$asBytes(ptr)).to.eventually.equal(ethers.toBeHex(0x20, 32)); | ||
}); | ||
|
||
it('asUint256', async function () { | ||
const value = 0x1234; | ||
const ptr = ethers.toBeHex(value, 32); | ||
await expect(this.mock.$asUint256(ptr)).to.eventually.equal(value); | ||
}); | ||
}); | ||
|
||
describe('memory operations', function () { | ||
it('copy', async function () { | ||
await expect(this.mock.$copy(ethers.toBeHex(0x80, 32), ethers.toBeHex(0xc0, 32), 32)).to.not.be.reverted; | ||
}); | ||
|
||
it('copy with zero length', async function () { | ||
await expect(this.mock.$copy(ethers.toBeHex(0x80, 32), ethers.toBeHex(0xc0, 32), 0)).to.not.be.reverted; | ||
}); | ||
}); | ||
}); |
Uh oh!
There was an error while loading. Please reload this page.