Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/heavy-baboons-give.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

`Packing`: Added a new utility for packing and unpacking multiple values into a single bytes32.
40 changes: 40 additions & 0 deletions contracts/utils/Packing.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.20;

/**
* @dev Helper library packing and unpacking multiple values into bytes32
*/
library Packing {
type Uint128x2 is bytes32;
Copy link
Collaborator Author

@Amxx Amxx Apr 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now we only have (uint128, uint128) <> bytes32, but we can imagine having other types, such as uint64x4

Everything here is designed to scale to other types.


/// @dev Cast a bytes32 into a Uint128x2
function asUint128x2(bytes32 self) internal pure returns (Uint128x2) {
return Uint128x2.wrap(self);
}

/// @dev Cast a Uint128x2 into a bytes32
function asBytes32(Uint128x2 self) internal pure returns (bytes32) {
return Uint128x2.unwrap(self);
}

/// @dev Pack two uint128 into a Uint128x2
function pack(uint128 high128, uint128 low128) internal pure returns (Uint128x2) {
return Uint128x2.wrap(bytes32(bytes16(high128)) | bytes32(uint256(low128)));
}

/// @dev Split a Uint128x2 into two uint128
function split(Uint128x2 self) internal pure returns (uint128, uint128) {
return (high(self), low(self));
}

/// @dev Get the first element of a Uint128x2 (high part)
function high(Uint128x2 self) internal pure returns (uint128) {
return uint128(bytes16(Uint128x2.unwrap(self)));
}

/// @dev Get the second element of a Uint128x2 (low part)
function low(Uint128x2 self) internal pure returns (uint128) {
return uint128(uint256(Uint128x2.unwrap(self)));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high and low don't make much sense in the context of another type like Uint64x4, what do you think of only having a single split() function?:

Suggested change
/// @dev Split a Uint128x2 into two uint128
function split(Uint128x2 self) internal pure returns (uint128, uint128) {
return (high(self), low(self));
}
/// @dev Get the first element of a Uint128x2 (high part)
function high(Uint128x2 self) internal pure returns (uint128) {
return uint128(bytes16(Uint128x2.unwrap(self)));
}
/// @dev Get the second element of a Uint128x2 (low part)
function low(Uint128x2 self) internal pure returns (uint128) {
return uint128(uint256(Uint128x2.unwrap(self)));
}
/// @dev Split a Uint128x2 into two uint128
function split(Uint128x2 self) internal pure returns (uint128, uint128) {
return (
uint128(bytes16(Uint128x2.unwrap(self))),
uint128(uint256(Uint128x2.unwrap(self)))
)
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are places where doing a full split, and keeping only part of the result is a bit more expensive, and also less clean in terms of code.

I would keep these two function, but we can rename them.

I considered first() and second(), which may scale better

Copy link
Member

@ernestognw ernestognw Apr 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, also I realized it would be easy to reach a Stack too deep error if we pack too many values, so I'd be in favor of having single getters.

Agree with first and second, but the distinction with high and low is that the former doesn't express the order (i.e. first ltr or rtl?). Shall we make it explicit? Maybe by exposing first() and firstRtl() Edit: Nah, let's keep first and second and be explicit in the Natspec

}
3 changes: 3 additions & 0 deletions contracts/utils/README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Miscellaneous contracts and libraries containing utility functions you can use t
* {StorageSlot}: Methods for accessing specific storage slots formatted as common primitive types. Also include primitives for reading from and writing to transient storage (only value types are currently supported).
* {Multicall}: Abstract contract with an utility to allow batching together multiple calls in a single transaction. Useful for allowing EOAs to perform multiple operations at once.
* {Context}: An utility for abstracting the sender and calldata in the current execution context.
* {Packing}: A library for packing and unpacking multiple values into bytes32
* {Panic}: A library to revert with https://docs.soliditylang.org/en/v0.8.20/control-structures.html#panic-via-assert-and-error-via-require[Solidity panic codes].

[NOTE]
Expand Down Expand Up @@ -117,4 +118,6 @@ Ethereum contracts have no native concept of an interface, so applications must

{{Context}}

{{Packing}}

{{Panic}}
27 changes: 27 additions & 0 deletions test/utils/Packing.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.20;

import {Test} from "forge-std/Test.sol";
import {Packing} from "@openzeppelin/contracts/utils/Packing.sol";

contract Base64Test is Test {
using Packing for *;

// Pack a pair of arbitrary uint128, and check that split recovers the correct values
function testUint128x2(uint128 high, uint128 low) external {
Packing.Uint128x2 packed = Packing.pack(high, low);
assertEq(packed.high(), high);
assertEq(packed.low(), low);

(uint128 recoveredHigh, uint128 recoveredLow) = packed.split();
assertEq(recoveredHigh, high);
assertEq(recoveredLow, low);
}

// split an arbitrary bytes32 into a pair of uint128, and check that repack matches the input
function testUint128x2(bytes32 input) external {
(uint128 high, uint128 low) = input.asUint128x2().split();
assertEq(Packing.pack(high, low).asBytes32(), input);
}
}
27 changes: 27 additions & 0 deletions test/utils/Packing.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
const { ethers } = require('hardhat');
const { expect } = require('chai');
const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
const { generators } = require('../helpers/random');

async function fixture() {
return { mock: await ethers.deployContract('$Packing') };
}

describe('Packing', function () {
beforeEach(async function () {
Object.assign(this, await loadFixture(fixture));
});

it('Uint128x2', async function () {
const high = generators.uint256() % 2n ** 128n;
const low = generators.uint256() % 2n ** 128n;
const packed = ethers.hexlify(ethers.toBeArray((high << 128n) | low));

expect(await this.mock.$asUint128x2(packed)).to.equal(packed);
expect(await this.mock.$asBytes32(packed)).to.equal(packed);
expect(await this.mock.$pack(high, low)).to.equal(packed);
expect(await this.mock.$split(packed)).to.deep.equal([high, low]);
expect(await this.mock.$high(packed)).to.equal(high);
expect(await this.mock.$low(packed)).to.equal(low);
});
});