From 4536d850ea39b6b90a4a7f46aed5135f7329f08f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 14:32:14 +0000 Subject: [PATCH] docs(LibConvert): the toX convention marks cost, not consumption The library NatSpec claimed that "toX" is adopted from Rust "to imply the additional costs and consumption of the source". The second half misstates the convention it cites. In Rust `to_` takes `&self` and marks an expensive conversion; `into_` takes `self` and is the marker that consumes the source. Under the convention as actually stated in Rust, `unsafeTo16BitBytes` is the correctly named function: it reads its source and allocates a new target. `unsafeToBytes` is the one that consumes, and it already documents that on itself. So the convention text moves. It now marks cost only, and states that consumption and the meaning of the `unsafe` prefix are per-function properties that do not generalise across the library. `unsafeTo16BitBytes` gains an explicit non-consumption note that names the contrast with `unsafeToBytes`, closing the dangerous direction of the inference. The reference implementation test carried the same wrong inference in a comment, asserting that `us` "can no longer be used" after the 16 bit pack. Corrected. The behaviour itself is already pinned by testUnsafeTo16BitBytesLeavesTheSourceIntact and testUnsafeToBytesAliasesTheSourceAndRewritesItsLengthPrefix. Closes #22 Co-Authored-By: Claude Opus 5 (1M context) --- src/LibConvert.sol | 19 +++++++++++++++++-- test/LibConvert.t.sol | 5 +++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/LibConvert.sol b/src/LibConvert.sol index e24d514..4abc42f 100644 --- a/src/LibConvert.sol +++ b/src/LibConvert.sol @@ -6,8 +6,16 @@ pragma solidity ^0.8.25; /// @notice Type conversions that require additional structural changes to /// complete safely. These are NOT mere type casts and involve additional /// reads and writes to complete, such as recalculating the length of an array. -/// The convention "toX" is adopted from Rust to imply the additional costs and -/// consumption of the source to produce the target. +/// The convention "toX" is adopted from Rust, where `to_` marks a conversion +/// that costs more than a cast. It marks the cost only. It says nothing about +/// the fate of the source, because in Rust the marker for consuming the source +/// is `into_` rather than `to_`, and this library has no equivalent marker. +/// +/// Whether a conversion consumes its source, and what the `unsafe` prefix +/// refers to, are therefore per-function properties. Both are stated on the +/// function itself and neither can be read off the name. Neither generalises +/// from one function in this library to another, so a caller MUST read the +/// NatSpec of the specific function it is calling. library LibConvert { /// Convert an array of integers to `bytes` data. This requires modifying /// the length in situ as the integer array length is measured in 32 byte @@ -32,6 +40,13 @@ library LibConvert { /// values are not checked for overflow due to the truncation. The caller /// MUST ensure that all values fit in `type(uint16).max` or that silent /// overflow is safe. + /// + /// The truncation is the only unsafety. This does NOT consume `us`. The + /// result is a freshly allocated buffer and `us` is only read, so `us` is + /// still a valid `uint256[]` after the call and the caller may keep using + /// it. That does NOT carry over to `unsafeToBytes`, which hands back the + /// source buffer itself with a rewritten length prefix, and so must not be + /// given a `us` that the caller still needs. /// @param us The `uint256[]` to truncate and concatenate to 16 bit `bytes`. /// @return The concatenated 2-byte chunks. function unsafeTo16BitBytes(uint256[] memory us) internal pure returns (bytes memory) { diff --git a/test/LibConvert.t.sol b/test/LibConvert.t.sol index fc86f30..94693ae 100644 --- a/test/LibConvert.t.sol +++ b/test/LibConvert.t.sol @@ -18,8 +18,9 @@ contract LibConvertTest is Test { function testUnsafeTo16BitBytesReferenceImplementation(uint256[] memory us) public pure { assertEq( - // Note the order of these calls is important because the unsafe call - // is unsafe, i.e. the `us` can no longer be used. + // Unlike `unsafeToBytes` this one does not consume `us`, so both + // calls read the same intact source and the order between them + // carries no requirement. LibConvertSlow.to16BitBytesSlow(us), LibConvert.unsafeTo16BitBytes(us) );