Add new nibbles and clz functions to Bytes.sol
#5725
Conversation
🦋 Changeset detectedLatest commit: 8116913 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
ernestognw
changed the title
equal, nibbles and clz functions to Bytes.solnibbles and clz functions to Bytes.sol
contracts/utils/Bytes.sol
Outdated
| function clz(uint256 x) internal pure returns (uint256) { | ||
| if (x == 0) return 32; // All 32 bytes are zero | ||
| uint256 r = 0; | ||
| if (x > 0xffffffffffffffffffffffffffffffff) r = 128; // Upper 128 bits | ||
| if ((x >> r) > 0xffffffffffffffff) r |= 64; // Next 64 bits | ||
| if ((x >> r) > 0xffffffff) r |= 32; // Next 32 bits | ||
| if ((x >> r) > 0xffff) r |= 16; // Next 16 bits | ||
| if ((x >> r) > 0xff) r |= 8; // Next 8 bits | ||
| return 31 ^ (r >> 3); // Convert to leading zero bytes count | ||
| } |
There was a problem hiding this comment.
We could reuse the Math.log256 function here
| function clz(uint256 x) internal pure returns (uint256) { | |
| if (x == 0) return 32; // All 32 bytes are zero | |
| uint256 r = 0; | |
| if (x > 0xffffffffffffffffffffffffffffffff) r = 128; // Upper 128 bits | |
| if ((x >> r) > 0xffffffffffffffff) r |= 64; // Next 64 bits | |
| if ((x >> r) > 0xffffffff) r |= 32; // Next 32 bits | |
| if ((x >> r) > 0xffff) r |= 16; // Next 16 bits | |
| if ((x >> r) > 0xff) r |= 8; // Next 8 bits | |
| return 31 ^ (r >> 3); // Convert to leading zero bytes count | |
| } | |
| function clz(uint256 x) internal pure returns (uint256) { | |
| return Math.ternary(x == 0, 32, 31 - Math.log256(x)); | |
| } |
Also, echoing on the reverseByte PR, but I think clz and reverseBytes should be in the same file (possibly a new one)
There was a problem hiding this comment.
Updated. Agree that the reverseBit functions and clz should be in the same library. imo it's fine that we put them in Bytes.sol. Any strong reason not to do so?
There was a problem hiding this comment.
IMO we should distinguish between bytes as in "arbitrary length buffer", and Bytes32 as in "a type of fixed length that has math properties".
I would argue that clz is one of these "local" math property and is closer to the Math library then it is to things like indexOf or slice that are more "global". By that argument, the reverseBytesXX are also more of a "local" operation (similar to clz)
There was a problem hiding this comment.
If CLZ operates on uint256, I think there is a strong case for putting it in Math.
We could have a variant of CLZ in Bytes that does something like
function clz(bytes memory buffer) internal pure returns (uint256) {
uint256 i = 0;
while (i < buffer.length && buffer[i] != bytes1(0)) ++i;
return 8 * i + (i == buffer.length ? 0 : Math.clz(buffer[i]));
}(should be optimized for reading in blocks of 32 instead of byte by byte
Requires #5726
PR Checklist
npx changeset add)