Test-coverage gaps and minor inconsistencies observed during a v0.7.0 read-through

[mscardellato]

Summary

I went through contracts/src/RAIL0.sol and the 98-test suite after the v0.7.0 deployment, focusing on test coverage, code cleanliness, and source/README consistency. The contract is in good shape overall (consistent custom errors, manual reentrancy guard, no dead code, README and source aligned on VERSION/addresses/signatures).

The six points below are the only ones I'd flag <80><94> none are correctness bugs in the deployed code. They split into:

#	Topic	Type	Suggested action
1	Non-bool-returning ERC-20 path is untested	Test gap	Add a MockNonReturning* and exercise it
2	Conservation invariants tested by table, not by fuzz	Test gap	Add testFuzz_Distribute_*
3	_safeTransfer failure path tested only via void	Test gap	Cover release and _distribute-via-capture
4	refund is gross <80><94> merchant absorbs fee on full refund	Doc + test	Note in README, add an explicit assertion
5	IEIP3009 / IERC20 have members RAIL0 never calls	Cleanup	Trim or document
6	Two tests use bare vm.expectRevert()	Test hardening	Assert the specific selector

---

1. Non-bool-returning ERC-20 compatibility is documented but untested

Where. README L235:

SafeERC20-style transfers. _safeTransfer / _safeTransferFrom accept both bool-returning and non-returning ERC-20 implementations, and revert with TransferFailed on any failure. Compatible with USDT-mainnet-style tokens that don't return a value.

The relevant branch in RAIL0.sol:435 / :442 is !success || (data.length > 0 && !abi.decode(data, (bool))) <80><94> the data.length == 0 case is the non-returning path that the README is referring to.

Test status. Every mock in RAIL0.t.sol (MockERC20, MockTransferFails, MockTransferFromFails, MockBlacklistERC20, MockReentrant) returns a bool. No test exercises the data.length == 0 branch.

Impact. This is the classical regression that bites contracts the moment they touch Ethereum mainnet USDT. Coverage today gives no signal if a future refactor breaks the empty-return path.

Suggested fix. Add MockNonReturningTransfer and MockNonReturningTransferFrom (same as MockERC20 but with function transfer(address , uint256) external returning nothing). Re-run the success and failure paths against them on void, release, refund, and a capture th
at goes through _distribute.

---

2. Conservation invariants are table-tested, not fuzz-tested

Where. RAIL0.t.sol:634 test_Capture_FeeMath_Conservation iterates a 5<97>5 table of (feeBps, amount) pairs.

Impact. The conservation invariant of _distribute (fee + payee == amount, no overflow, fee = (amount * feeBps) / 10_000) is arguabl
y the most important property of the contract, and 25 hardcoded points is a thin signal. Foundry supports fuzzing natively, and the test almo
st rewrites itself:

function testFuzz_Distribute_Conservation(uint120 amount, uint16 feeBps) public {
    vm.assume(amount > 0 && feeBps <= 10_000);
    // ... same charge + assert as today, but amount and feeBps come from the fuzzer
}

Suggested fix. Convert (or duplicate) the conservation test into a fuzz test. Worth a second testFuzz_Capture_PartialsConserveTotal tha
t splits amount into N random partials and asserts the sum hits the right total.

---

3. _safeTransfer / _safeTransferFrom failure paths are partially covered

Where. RAIL0.t.sol:1362 test_SafeTransfer_RevertsOnBoolFalseReturn exercises only void. The same helpers are also reached by:

• release <86><92> _safeTransfer
• capture and charge <86><92> _distribute <86><92> _safeTransfer
• refund <86><92> _safeTransferFrom (covered separately at L1385)

Impact. Three call sites share one test. A future change to one path (e.g., switching to an unchecked transfer for gas) would not be caug
ht by the current suite.

Suggested fix. Mirror test_SafeTransfer_RevertsOnBoolFalseReturn for release and for capture-with-fee (where _distribute triggers
the failing transfer to either feeReceiver or payee).

---

4. refund semantics under non-zero feeBps <80><94> implicit, not documented

When feeBps > 0:

• capture(amount) distributes the gross amount (fee to feeReceiver, remainder to payee), but refundableAmount += amount (gross).
• A subsequent refund(refundableAmount) pulls the full gross amount from payee via transferFrom.

Consequence. If a merchant refunds the entire capture, the merchant pays the fee out of pocket <80><94> the fee already left to feeR eceiver is not returned. The merchant net-loses the fee for that capture.

This is consistent with the protocol's design (refund is gross because refundableAmount is gross), and test_E2E_AuthorizeCaptureRefund_Wit hFee (L1510) demonstrates it numerically: payee ends at 95e6 after receiving 195e6 and refunding 100e6. But:

• the README never tells the merchant this can happen;
• no test explicitly asserts the property "merchant net loss on full refund equals fee amount."

Impact. Support-ticket bait. Easy to fix without changing behavior.

Suggested fix.

1. README note in the refund section (paragraph after L116) along the lines of: "Note: refundableAmount is tracked gross. Refunding the f
   ull captured amount returns the buyer their gross payment but leaves the merchant short by amount * feeBps / 10_000 <80><94> the fee al
   ready paid to feeReceiver is not recovered."
2. A small dedicated test asserting the property, separate from the E2E test.

---

5. Dead members in IEIP3009 / IERC20

Where. contracts/src/interfaces/IERC20.sol.

RAIL0 only calls transfer, transferFrom, and transferWithAuthorization. The interface additionally declares: balanceOf, allowance,
approve, event Transfer, event Approval, IEIP3009.authorizationState, event AuthorizationUsed.

These are never read by the contract. The contract compiles unchanged with a minimal interface (transfer, transferFrom, transferWithAuth orization).

Impact. No runtime impact, but new readers may infer that authorizationState is consulted by RAIL0 (it isn't), or that the interface gu
arantees approve exists on every accepted token (the allowlist isn't actually validated against the full interface).

Suggested fix. Either:

• trim to the minimum surface actually called, or
• add a short comment at the top of the file: "Complete EIP-20 + EIP-3009 surface, kept for clarity; not every member is consumed by RAIL0."

(I have no strong preference; closer to your call.)

---

6. Two tests use bare vm.expectRevert() where a selector would be safer

Where.

• test_Authorize_RevertsOnTamperedPayment (L308) <80><94> expects a revert, doesn't pin its source. The current expectation is that the
  token reverts on a bad signature. If a future RAIL0 change added an earlier validation that reverted too, the test would pass for the wrong
  reason.
• test_Reentrancy_GuardBlocksInnerCall (L1500) <80><94> asserts reenterSucceeded == false and reenterAttempted == true, but the inn
  er call could fail for any reason without being detected as such. Decoding the revert data from the low-level .call() and checking it again
  st Reentrancy.selector would be more stringent.

Impact. Two out of 98 tests don't fully constrain the failure mode. These are the two where the constraint matters most for audit posture
(signature-verification path and reentrancy-guard path).

Suggested fix. Where feasible, switch to vm.expectRevert(<Specific>.selector). For the reentrancy test, capture and decode the inner re
vert data to compare against RAIL0.Reentrancy.selector.

---

Out of scope (verified, not flagged)

• README <86><94> source parity post-v0.7.0 (VERSION="7", addresses, function signatures, struct, events).
• Test count claim in README (98 <80><94> matches).
• No TODO/FIXME/HACK strings in contracts/.
• No maxAmount references left over from v0.6.
• Domain separator chain-fork rebuild is tested (test_DomainSeparator_RebuildsOnChainFork).
• Fee-receiver DoS escape hatch is tested for void and release.