M-1: Vendored portfolio layout lacks runtime validation; false claims about LiteSVM test

[v1ktorrr0x]

Bug

The NFT program mirrors the converged v17 portfolio layout (PortfolioAccountV16Account, 9227 bytes) via bytemuck cast in decode_portfolio (slab_types_v16.rs:484). This is the only place portfolio bytes are interpreted, and every security decision downstream depends on correct field offsets.

False Claims

• cpi_v16.rs:14: "end-to-end path is verified by the A.5 LiteSVM suite"
• README.md:95: "Struct offsets are validated at compile-time with size and offset assertions"
• slab_types_v16.rs:43: "The LiteSVM integration test (cross-cut phase) is the runtime ground truth"

Reality: There is no tests/ directory, and CI runs only cargo test --lib --tests (self-referential unit tests).

The const_assert! macros measure the mirror struct against itself, proving internal consistency but proving nothing about correspondence to the live engine layout.

Risk

If the engine layout ever inserts/reorders/resizes a field before the read fields (legs, owner, locks), the provenance header checks still pass while owner/market_id/gate-flags are silently read from wrong bytes:

• wrong owner offset → mint against positions you don't own
• wrong market_id/slot-reuse anchor → transfer gate decisions on garbage
• wrong lock/stale flags → bypass transfer restrictions

This is the failure mode warned about in SCHEMA_DRIFT_v12.19.md: "mirror schema drift can mask itself as compile-pass."

Fix

Either:

1. Remove false claims from comments/docs to match reality, OR
2. Add the promised LiteSVM integration test (spin up real wrapper portfolio, assert NFT reads match)

Labels: security, documentation

[0x-SquidSol]

PR: #185.

The documentation half is already fixed. cf56ba5 ("M-1 docs") removed the LiteSVM claims from cpi_v16.rs and slab_types_v16.rs, and README.md:128-130 now states the honest position: "verified for internal consistency via compile-time const_assert! macros; alignment with the live engine layout requires runtime validation against real on-chain data (deferred, see #110H)". That is option 1, done.

On the substance, I did the correspondence check nobody had done. Your central point is that the assertions prove internal consistency and nothing about the live engine, so drift can mask itself as a compile pass. I compared the mirror to percolator/src/v16.rs field-for-field:

• PortfolioAccountV16Account: identical — 21 fields, same names, types, order
• every sub-struct matches: ProvenanceHeaderV16Account (5), PortfolioLegV16Account (15), PortfolioSourceDomainV16Account (14), HealthCertV16Account (11), CloseProgressLedgerV16Account (18), ResolvedPayoutReceiptV16Account (6)

So the mirror is currently correct — the risk is real but not presently realised.

Worth recording a near-miss from that check, because it is the kind of thing that produces a false CRITICAL: my first pass appeared to show PORTFOLIO_SOURCE_DOMAIN_CAP as engine=4 vs mirror=2. Wrong twice over — the engine's 4 is #[cfg(kani)]-only, and my grep had truncated 2 * V16_MAX_PORTFOLIO_ASSETS_N at the 2. Both sides use the identical expression.

But the check found one genuine instance of your failure mode. V16_ACTIVE_BITMAP_WORDS was the literal 1, with the formula only in its doc comment, while the engine derives (V16_MAX_PORTFOLIO_ASSETS_N + 63) / 64. Both are 1 today. Raise the asset count above 64 upstream and the engine's bitmap becomes two words while the literal stays at one — shifting legs and every field after it, including the stale_state / b_stale_state / liquidation_lock flags the transfer gate reads. That is your "wrong lock/stale flags → bypass transfer restrictions", one upstream constant away.

And as you argued, the size assertion would not catch it: whoever bumped the count would update EXPECTED_PORTFOLIO_ACCOUNT_SIZE to whatever the wrong layout computed, and everything would pass.

#185 derives it and asserts the derivation, as const assertions rather than tests so every build checks them — including build-sbf. That mattered practically: cargo test cannot run on my machine at all (openssl-sys finds no OpenSSL on Windows), while cargo build checks these fine. I verified the assertion bites by hardcoding the value back and watching the build fail on it.

This narrows the residual risk rather than replacing the runtime validation #110H tracks — a cross-crate assertion against the engine would be stronger still, but adding percolator as a dependency changes this crate's build graph and CI, which is your call rather than mine.

[dcccrypto]

CONFIRMED — including the documentation claims, which I checked individually. Left open.

This is the most uncomfortable kind of finding and it is correctly reported. The three claims:

• cpi_v16.rs:14 — "end-to-end path is verified by the A.5 LiteSVM suite"
• README.md:95 — "Struct offsets are validated at compile-time with size and offset assertions"
• slab_types_v16.rs:43 — "The LiteSVM integration test (cross-cut phase) is the runtime ground truth"

against: no tests/ directory, and CI running only cargo test --lib --tests.

Why the const_assert! point is the real finding

Your sentence is the one that matters:

The const_assert! macros measure the mirror struct against itself, proving internal consistency but proving nothing about correspondence to the live engine layout.

A vendored 9227-byte layout consumed by bytemuck cast has exactly one failure mode worth defending against — the mirror drifting from the engine — and a self-referential assertion cannot detect it. The assertions are not weak evidence of layout correctness; they are zero evidence of it, while reading as strong evidence to anyone who sees them in the file. decode_portfolio being the only place portfolio bytes are interpreted means every downstream security decision inherits that.

This is the same class as SDK #375, where a parity gate that never ran for three of four targets meant ABI drift in those programs had no automated detection, and #379, where a suite that builds its own fixtures could not notice a constant disagreeing with production. Three separate repos, one shape: a check that validates a thing against itself.

Why open rather than fixed

Two parts, and only one is cheap:

• Correcting the false claims is a doc change and should happen regardless. A comment asserting a test that does not exist is worse than silence, because it stops the next reader from writing the test.
• Actually validating the mirror against the live engine layout needs a LiteSVM (or on-chain fixture) test that decodes a real portfolio account produced by the deployed wrapper. That is the fix, and stubbing it would recreate the problem — a test that appears to prove correspondence while proving something weaker is exactly what is being reported here.

Left open for the real test. Filing this against the claims rather than only the missing coverage was the right framing.

[dcccrypto]

The three false claims are all FIXED. Keeping open for the substantive half — runtime validation — which is what this issue should now mean.

Verified on current main rather than assumed:

claim	status
cpi_v16.rs:14 "verified by the A.5 LiteSVM suite"	gone — grep -rln -i litesvm src/ tests/ Cargo.toml returns nothing
README.md:95 "validated at compile-time with size and offset assertions"	corrected — now says they verify internal consistency and not engine alignment
slab_types_v16.rs:43 "LiteSVM integration test is the runtime ground truth"	corrected — same wording
"There is no tests/ directory"	outdated — 5 PoC files now exist

And one thing your issue did not catch, found while checking

The README deferred the remaining work to "#110H". #110 is closed — a batch of LOW/INFO findings. So the deferral was documented as tracked somewhere that no longer tracks it: a reader checking the pointer would find a closed issue and reasonably conclude the work was done. Fixed in #187, now pointing here.

That is the same class as the original finding — documentation asserting a verification that does not exist — one level up.

What stays open, sharpened

Your central point is the one that survives all of this, and it deserves restating in its strongest form:

The const_assert! macros measure the mirror struct against itself, proving internal consistency but proving nothing about correspondence to the live engine layout.

Sharper still: under the exact change they exist to guard against — raising V16_MAX_PORTFOLIO_ASSETS_N past 64 — whoever made it would update EXPECTED_PORTFOLIO_ACCOUNT_SIZE to whatever the wrong struct computed, and the assertion would pass. It is not weak evidence of layout correctness; in that scenario it is guaranteed to pass.

#185 (merged today) removed one concrete instance of that vector by deriving V16_ACTIVE_BITMAP_WORDS from the asset count instead of hardcoding 1. The author also compared the mirror to the live engine field-for-field and found it currently correct — so this is a latent vector, not a live defect.

The general case still needs a real portfolio account, produced by the deployed wrapper, decoded at runtime. That is the fix, and stubbing it would recreate exactly the problem you reported — a test that appears to prove correspondence while proving something weaker.

nft suite: 11 + 6 + 5 + 5 + 4 passing, 0 failed.