LibConvert NatSpec says the toX convention implies consumption of the source, but unsafeTo16BitBytes does not consume its source

[thedavidmeister]

Unit

LibConvert library NatSpec vs LibConvert.unsafeTo16BitBytes —
src/LibConvert.sol:5-11 and src/LibConvert.sol:30-58.

Intent oracle

The library-level NatSpec states a naming convention that applies to every toX
function in the library:

The convention "toX" is adopted from Rust to imply the additional costs and
consumption of the source to produce the target.

Violated property

The convention holds for one of the two functions and not the other, and nothing
at the call site distinguishes them:

• unsafeToBytes does consume its source. It aliases us and rewrites the
  shared length prefix from a word count to a byte count, which its own NatSpec
  spells out ("there is now two pointers to the same mutable data structure AND
  the length prefix for the uint256[] version is corrupt").
• unsafeTo16BitBytes does not consume its source. It allocates its own
  result with new bytes(us.length * 2) and only reads us. The source is a
  fully valid uint256[] after the call.

Both are named unsafe... and both are toX, so the library-level convention
tells a reader that both consume their source. One of those two readings is
wrong whichever way the reader goes.

Verified repro

Pinned as a passing test in #19, which demonstrates the non-consumption
directly: testUnsafeTo16BitBytesLeavesTheSourceIntact snapshots us before the
call and asserts every element and the length are unchanged afterwards. It
passes on a4692fddbb95a0018cec42969ef6c7fa1b0cf86c.

Conversely testUnsafeToBytesAliasesTheSourceAndRewritesItsLengthPrefix, in the
same PR, asserts that unsafeToBytes does consume: the returned pointer equals
the source pointer and the source's length prefix reads back as the byte length.

Neutral triage framing

Why this may be intended. The dangerous direction is already covered where it
matters: unsafeToBytes, the one that really does consume, documents the
consumption in detail on the function itself. A caller who reads the function
NatSpec rather than the library NatSpec is correctly informed in both cases. The
library-level sentence may be intended as a loose gloss on the naming rather than
a per-function guarantee.

Why it may still be worth acting on. The failure mode of believing the
library NatSpec about unsafeTo16BitBytes is benign (a caller needlessly avoids
reusing us), but the failure mode of generalising in the other direction is
not. A reader who notices that unsafeTo16BitBytes leaves its source intact may
reasonably infer that unsafe in this library refers only to the truncation, and
carry that inference to unsafeToBytes, where reusing us reads a corrupt
length prefix.

Cheap options. (a) Note on unsafeTo16BitBytes that, unlike unsafeToBytes,
it does not consume its source. (b) Soften the library-level sentence so it does
not read as a per-function guarantee. (c) Close as by design.

Severity: low — documentation clarity, no incorrect behaviour.

Notes

Found during an adversarial mutation test run over the whole repo.