docs(LibConvert): the toX convention marks cost, not consumption

[thedavidmeister]

Closes #22.

Which side moves, and why

The issue frames this as a mismatch between the library NatSpec and one of
its two functions, and offers (a) note the non-consumption, (b) soften the
library sentence, or (c) close as by design. Before picking, it is worth
checking the sentence against the authority it names, because that changes
which function is the odd one out.

The library NatSpec says:

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

The Rust convention (API guidelines C-CONV) splits exactly this way:

prefix	cost	receiver	source
as_	free	&self	survives
to_	expensive	&self	survives
into_	variable	self	consumed

to_ marks the cost. into_ is the marker for consuming the source. So the
first half of the sentence is right and the second half is not — it attributes
into_'s meaning to to_.

That inverts the issue's implicit reading. Measured against the convention as
it actually exists, unsafeTo16BitBytes is the correctly named function:
it reads its source and allocates a new target, which is precisely to_.
unsafeToBytes is the one whose behaviour is into_ while its name says
to_ — and it is already the one that documents its consumption in full on
itself.

Decision: the convention text moves. The names and the behaviour stay.

Why not the behaviour

Making unsafeTo16BitBytes consume its source would be a silent runtime
break at live call sites, which is the worst failure mode available here. The
in-org call sites pass an aliased uint256[] view of a function-pointer array
that is still live in the caller's scope, e.g. rain.pyth
src/abstract/PythSubParser.sol:

uint256[] memory pointers;
assembly ("memory-safe") {
    pointers := fs
}
return LibConvert.unsafeTo16BitBytes(pointers);

pointers and fs are the same buffer. A consuming implementation would
corrupt fs. There is also nothing to gain: the result is half the size of
the source, so "consuming" would mean overwriting a still-needed input for no
saving.

Why not the name

Weighed explicitly rather than assumed away, in both directions.

Renaming unsafeTo16BitBytes. This is a published soldeer package
(rain-lib-typecast, autopublished on every push to main), so a rename is a
compile-time break for every consumer with no deprecation path. It is also
wide: a code search across the org finds 21 call sites in 8 repos —
rainlang, raindex, rain.pyth, rain.merkle, rain.flare,
rain.erc4626.words. And it is the wrong direction anyway, since under the
real convention this function's name is already correct.

Renaming unsafeToBytes to unsafeIntoBytes. This one is genuinely
cheap — the same search finds zero call sites outside this repo — so cost is
not the argument against it. Two things are:

1. It does not fix the bug. The sentence "the convention toX ... implies
   consumption" would still be false about Rust after the rename, and would
   still have to be corrected. The text has to move either way; once it has,
   the rename buys nothing the corrected text has not already delivered.
2. The package is public. Zero in-org call sites is the best evidence
   available, not proof of zero consumers, and it would put a Rust into_
   idiom on exactly one function in the entire org — trading a documented
   mismatch for an undocumented novelty.

What changed

1. Library NatSpec. toX now marks cost only, and says why: into_ is
   Rust's consuming marker and this library has no equivalent. It then states
   that consumption and the referent of the unsafe prefix are per-function
   properties, stated on the function, that do not generalise across the
   library. This is option (b), and the added sentence about unsafe closes
   the specific hazard the issue calls out — a reader inferring from
   unsafeTo16BitBytes that unsafe in this library means truncation only,
   and carrying that to unsafeToBytes.

2. unsafeTo16BitBytes NatSpec. Option (a): explicit non-consumption,
   naming the contrast with unsafeToBytes so the reader is pushed toward the
   dangerous function rather than away from it.

3. testUnsafeTo16BitBytesReferenceImplementation comment. The suite
   itself already carried the wrong inference:

   Note the order of these calls is important because the unsafe call is
   unsafe, i.e. the us can no longer be used.

   That is true of unsafeToBytes and false of unsafeTo16BitBytes — it is
   the issue's failure mode having already happened, in this repo. Corrected.
   The identical comment on the unsafeToBytes test is correct and is left
   alone.

unsafeToBytes itself is untouched: its NatSpec already spells out the
consumption, which is the half the issue agrees was never wrong.

QA

• Discriminating tests: n/a — no new test, and none is available to add. Both
  directions of the property are already pinned on main by
  testUnsafeTo16BitBytesLeavesTheSourceIntact (non-consumption) and
  testUnsafeToBytesAliasesTheSourceAndRewritesItsLengthPrefix (consumption),
  merged in test(LibConvert): pin aliasing, source lifetime and the write bounds #19, and the issue names those two as its verified repro. This diff
  changes no executable line, so there is no behaviour a new test could
  discriminate and the instruction was explicitly not to duplicate them.
• Mutations applied: n/a — the diff is NatSpec and code comments only. No
  executable line changes, so mutation-probe has nothing to mutate and there
  is no mutants.toml scope to declare. Verification is the existing suite
  continuing to pass unchanged under rainix-sol.
• Oracle: the Rust API guidelines as_/to_/into_ convention that the
  library NatSpec itself cites as its source, which is external to this
  codebase; plus the issue's own oracle quote of the library NatSpec sentence.
  Every behavioural claim in the new text (unsafeTo16BitBytes allocates and
  only reads; unsafeToBytes aliases and rewrites the length prefix) is one
  the two tests above already assert, not a restatement of the implementation.
• Category check: issue asks (a) note the non-consumption on
  unsafeTo16BitBytes, (b) soften the library-level sentence, (c) close as by
  design; covered (a) and (b), with (c) rejected and the reason given in "Which
  side moves, and why". The two further options the issue does not list —
  moving the name and moving the behaviour — are weighed and rejected in their
  own sections above.

Note for the reviewer

unsafeToBytes uses a named return (returns (bytes memory bs)), which the
no-named-returns convention disallows. Left alone deliberately: it is a code
change unrelated to #22, it would churn the gas snapshot, and this PR is
otherwise comment-only. Worth its own issue.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Documentation
  • Clarified conversion cost and function-specific unsafe behavior.
  • Documented that unsafeTo16BitBytes preserves the source array, while unsafeToBytes may reuse and mutate its source buffer.
• Tests
  • Updated reference guidance to reflect that unsafeTo16BitBytes does not alter the source array.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 435c1399-8bc9-4e59-bea0-d8012141b718

📥 Commits

Reviewing files that changed from the base of the PR and between 001817f and 4536d85.

📒 Files selected for processing (2)

• src/LibConvert.sol
• test/LibConvert.t.sol

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

Walkthrough

The change clarifies that toX describes conversion cost, not source consumption. It documents that unsafeTo16BitBytes preserves its source, unlike unsafeToBytes, and updates the related test comment.

Changes

LibConvert documentation clarification

Layer / File(s)	Summary
Conversion semantics and reference comment src/LibConvert.sol, test/LibConvert.t.sol	The NatSpec now defines source consumption and unsafe behavior per function. It documents that unsafeTo16BitBytes allocates a new result and preserves us, while unsafeToBytes reuses and mutates the source buffer. The test comment reflects the preserved source and valid call order.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 4536d

This documentation-only change does not alter runtime behavior, and no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the documentation change to the LibConvert toX convention, which is the main pull request objective.
Linked Issues check	✅ Passed	The changes address issue #22 by clarifying conversion cost, source consumption, and the differing behavior of unsafeTo16BitBytes and unsafeToBytes.
Out of Scope Changes check	✅ Passed	The changes are limited to related NatSpec documentation and a test comment, with no unrelated code or behavior changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch docs/libconvert-tox-convention-vs-consumption

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}