fix(lib): use lossy decimal conversions to prevent revert on sub-decimal inputs

[thedavidmeister]

Summary

• Replaces toFixedDecimalLossless/fromFixedDecimalLosslessPacked in LibERC4626.convertToAssets and convertToShares with the non-reverting lossy variants
• Sub-decimal share/asset amounts are now truncated toward zero (floor) before being forwarded to the vault instead of reverting, matching ERC-4626's floor-rounding convention
• Adds ERC4626Words.rounding.t.sol with two fuzz tests sweeping arbitrary significands with one extra decimal digit beyond vault precision, asserting neither word reverts (256 runs each, all pass locally)
• Regenerates BYTECODE_HASH in ERC4626Words.pointers.sol to match the updated bytecode

Refs #70

Test plan

• testFuzzConvertToAssetsSubDecimalNeverReverts — passes 256 fuzz runs locally
• testFuzzConvertToSharesSubDecimalNeverReverts — passes 256 fuzz runs locally
• forge build succeeds with updated pointers
• copy-artifacts CI (BYTECODE_HASH match) — pending CI run; may need hash fix if local nix env diverges from CI

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

Summary by CodeRabbit

• Tests
  • Added coverage verifying that sub-decimal asset and share conversions correctly revert when they would cause lossy precision conversion.
  • Confirmed reverts include the expected error details across a range of fuzzed inputs.

QA

• Discriminating tests: testFuzzConvertToAssetsSubDecimalReverts, testFuzzConvertToSharesSubDecimalReverts (256 runs each, test/src/concrete/ERC4626Words.subDecimalRevert.t.sol) — n/a, neither fails on base, and that is the point after the Lossless decimals conversion assumes vault rate is exactly representable; reverts (DoS) for the common non-1:1 vault #70 REVERT ruling: base already reverts, so these are regression pins on correct behaviour, not cover for a fix. What was verified instead is that they are not duplicates: git grep -B3 '\.extern(' -- test/src/concrete test/src/abstract | grep expectRevert returns these two assertions and nothing else, i.e. they are the only place in the suite where a revert is asserted to propagate out of ERC4626Words.extern. Full suite 147 passed / 0 failed on the pinned CI toolchain (nix develop github:rainlanguage/rainix/53e96a7d#sol-shell -c forge test).
• Mutations applied: src/abstract/ERC4626Extern.sol buildOpcodeFunctionPointers → swap fs[OPCODE_ERC4626_CONVERT_TO_ASSETS]/fs[OPCODE_ERC4626_CONVERT_TO_SHARES], pointers regenerated via forge script script/Build.sol → killed by testFuzzConvertToSharesSubDecimalReverts ("next call did not revert as expected", counterexample 258) BUT ALSO killed independently by four pre-existing tests (testExternConvertToAssetsRouting, testExternConvertToSharesRouting, testParseEvalConvertToAssets, testParseEvalConvertToShares). Reported straight rather than claimed as a unique kill: no mutation of this repo's own source is killed by these two tests ALONE. Their residual value is a pin on the BaseRainlangExtern dispatch boundary — a dependency this repo consumes but does not own — which no in-repo mutant can express. The NatSpec was corrected in 00b5536 to say exactly this, because the earlier revision claimed the pointer-table cover that this mutation run disproved.
• Oracle: ERC-4626 token precision plus the Lossless decimals conversion assumes vault rate is exactly representable; reverts (DoS) for the common non-1:1 vault #70 ruling (REVERT, not floor), independent of the implementation — a 6-decimal asset given an exponent of -7, and 18-decimal shares given -19, cannot round-trip losslessly, so the expected LossyConversionFromFloat(significand, exponent) arguments are derived from the mock vault's declared decimals and the input exponent, never read back off LibERC4626. The significand % 10 != 0 assumption is likewise derived: significand*10^(-7+6) is an integer exactly when 10 divides it, which would legitimately not revert.
• Category check: Lossless decimals conversion assumes vault rate is exactly representable; reverts (DoS) for the common non-1:1 vault #70 asked for floor-truncation; the human ruling REJECTED that premise and closed Lossless decimals conversion assumes vault rate is exactly representable; reverts (DoS) for the common non-1:1 vault #70 as premise-corrected, so this PR closes nothing — Closes #70 was weakened to Refs #70 and the closing set is now empty. Covered: extern-level revert propagation (both opcodes). Deliberately NOT covered, deduped away to test(erc4626): cover lossy-precision revert paths in convertToAssets/Shares #175 which is on main already: lib-level revert assertions (testRunRevertsOnLossy*Input) and the floor/rounding tests. Not covered and out of scope: the lossy src/lib/erc4626 change the PR title and body still describe — it was dropped in the earlier conflict resolution and the ruling means it must stay dropped; the stale title/body prose is flagged in the run report as needing a retitle no pipeline tool exposes.

[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: 5d96be1e-1fd2-434e-a4bb-be2af8a8eabe

📥 Commits

Reviewing files that changed from the base of the PR and between 4a75e9b and ed130d5.

📒 Files selected for processing (1)

• test/src/concrete/ERC4626Words.subDecimalRevert.t.sol

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

---

Walkthrough

Adds fuzz tests for ERC4626Words.extern conversion paths. The tests verify that sub-decimal asset and share inputs revert with LossyConversionFromFloat and the expected conversion values.

Changes

Sub-decimal conversion reverts

Layer / File(s)	Summary
Extern conversion revert tests test/src/concrete/ERC4626Words.subDecimalRevert.t.sol	Adds mock-vault setup, dispatch helpers, and fuzz tests for lossy asset-to-share and share-to-asset conversions. The tests assert exact LossyConversionFromFloat revert arguments.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to ed130

The change makes sub-decimal conversions truncate instead of reverting; no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

• rainlanguage/rain.erc4626.words#170: Adds related ERC4626 conversion revert tests for oversized vault outputs.
• rainlanguage/rain.erc4626.words#244: Tests the same ERC4626Words.extern conversion paths for sub-decimal reverts.
• rainlanguage/rain.erc4626.words#212: Tests ERC4626 conversion behavior for decimal precision and lossy reversion.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title claims the PR implements lossy conversions, but the changes add extern-level tests that verify reverts for sub-decimal inputs.	Retitle the PR to describe extern-level revert tests for sub-decimal ERC4626 inputs, such as “test: cover ERC4626 extern reverts on sub-decimal inputs”.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The tests align with the documented REVERT ruling for #70 and add extern-dispatch coverage beyond the lib-level tests.
Out of Scope Changes check	✅ Passed	The changes are limited to extern-level ERC4626 conversion revert tests and support the linked issue's adopted design.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 2026-06-17-issue-70-lossless-dos

---

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.}

[thedavidmeister]

Parked pending the design ruling on #70 (see #70 (comment)). Current state note: the conflict resolution kept main's LOSSLESS implementation, so this PR now contains only test/src/concrete/ERC4626Words.rounding.t.sol asserting the REVERT — the opposite of what the title/body describe — and Closes #70 would wrongly auto-close a HIGH issue with no behavioral change. Do not merge as-is. If the ruling is FLOOR: restore the original lossy src change + rounding fuzz tests. If the ruling is REVERT: relink off #70 and reconcile with #175's overlapping tests.

[thedavidmeister]

Ruling landed — REVERT (see #70 (comment), issue now closed premise-corrected). Updated rework for this PR: its title/body still describe the lossy change, but the conflict-resolved content is revert-pinning tests at the words.extern level. Under the ruling: relink off closed #70, retitle to match the actual content, DEDUPE against #175's lib-level revert tests (merging separately — keep only coverage #175 doesn't provide, e.g. the extern-level path if genuinely additive), and add the QA-GUIDE.md §8 evidence block. If nothing additive remains after dedupe, this closes as superseded.

[thedavidmeister]

🤖 ai:vetter
vet-protocol 4
lens no source read — the ai:design label's provenance decides this routing, not the diff
Reviewed bb1a381: needs-work — ROUTING FIX, not a code defect (design doctor, rainlanguage/issue-pr-cron#241): this PR carried ai:design with NO trusted comment raising a design question at its head, which is a state no actor consumes — the human's queue withholds it (there is no claim to present) and every AI actor skips it. It is routed here so it has an owner again. Two moves, both yours: if the PR still needs work, push it — the question was superseded or never recorded, and the push IS the transition. If a genuine design question remains, re-raise it with pr-review-report flag-design <owner/repo> <pr> "<the question>", which puts the row back in the human's queue with a claim behind it. Do not push a no-op commit to clear this.

[thedavidmeister]

🤖 ai:producer
Linkage repair: weakened Closes #70 to Refs #70 in the PR body via pr-review-report weaken-closes. Every byte outside that keyword is unchanged, and the ## QA section was not touched.

[thedavidmeister]

🤖 ai:producer
QA-block repair: appended QA-GUIDE section 8's evidence block in the PR body via pr-review-report repair-qa-block. Every byte outside the ## QA section is unchanged.

[thedavidmeister]

🤖 ai:vetter
vet-protocol 4
lens source@00b5536eebf5cdb8a5ae08388911302724f4479f + audit skill invoked at pr:166
Reviewed 00b5536: needs-work — refs #70 (closed premise-corrected) — in-diff defect: bare first-party imports at ERC4626Words.subDecimalRevert.t.sol lines 6-12 ("src/concrete/ERC4626Words.sol", "test/utils/MockERC4626.sol" etc.) have no remapping and resolve only via foundry root-fallback — switch to relative paths per the sibling tests; also the title/body Summary still claim a lossy LibERC4626 conversion change and BYTECODE_HASH regen the diff does not contain (it is a test-only revert pin) — correct the prose so the merged record matches the code; the two fuzz pins themselves are sound (non-circular oracle, %10 bound exact for both words)
cost 218 — fuzz revert-pin bound arithmetic check