Wave 6a: phantom-dust 4-field schema (KL-PHANTOM-DUST-SCHEMA-1 REVOKED) - #95
Merged
Merged
Conversation
…SCHEMA-1 REVOKED)
Adopts toly's 4-field phantom-dust schema (certified + potential per
side, toly:783-786). Closes the long-standing single-bound vs split
divergence flagged in Phase 2.
Schema:
- `phantom_dust_bound_<side>_q` → `phantom_dust_potential_<side>_q`
(renamed; semantically identical upper-bound tracking)
- Added `phantom_dust_certified_<side>_q` per side — lower-bound dust
certified by B-tracking-aware liquidation step 7 (toly:5030-5118).
Always 0 on this branch: B-tracking subsystem isn't ported, so the
certified fields exist purely for wire-format alignment with toly +
NFT vendored-bytes mirroring (Wave 6c). When B-tracking lands, the
certified field starts receiving non-zero values from the new
liquidation logic with no schema break.
- Net RiskEngine growth: +32 bytes (2 × u128 → 4 × u128).
Helpers:
- `inc_phantom_dust_bound` → `inc_phantom_dust_potential` (rename only)
- `inc_phantom_dust_bound_by` → `inc_phantom_dust_potential_by`
(fork-only liquidation step-10 helper, kept; toly inlines the
equivalent at toly:5099 but uses a different post-state formula)
- New toly-aligned: `get/set_phantom_dust_certified`,
`get/set_phantom_dust_potential` (toly:3353-3378). Setters are
grounded in resolve_market zeroing; getters in the new
`assert_public_postconditions` invariant.
Callsites updated:
- `sync_account_fee_to_slot_not_atomic` precision-boundary increment
- `try_close_position_at_oracle` step-10 dust bump (uses _by helper)
- `begin_full_drain_reset` full-drain side zeroing (now clears both
certified + potential per side, matching toly's
`begin_terminal_epoch_exhaustion_reset` semantics)
- `schedule_end_of_instruction_resets` §5.7 A/B/C — OI-cap uses
`phantom_dust_potential_<side>_q` (renamed bound)
- `resolve_market_not_atomic` clears all 4 fields per side on
`pre_stored_<side> == 0` (matches toly:9637-9644)
New invariant:
- `assert_public_postconditions` now asserts `certified <= potential`
per side as a forward-looking defense-in-depth gate. Trivially holds
on this branch (certified always 0); guards against off-by-one /
step-ordering bugs once B-tracking lands.
Kani:
- 2 new harnesses in proofs_invariants.rs:
- `proof_phantom_dust_certified_le_potential_at_genesis`
- `proof_phantom_dust_certified_gt_potential_rejects`
- Total engine Kani proof count: 311 → 313.
KEEP_LIST:
- KL-PHANTOM-DUST-SCHEMA-1 marked REVOKED.
Verify scripts:
- verify-engine-preservation.sh: §A.1/§A.2 entries removed (fields
renamed); §F.4 updated to track inc_phantom_dust_potential; §F.5
updated to track inc_phantom_dust_potential_by (still fork-only).
- verify-engine-deferred-absent.sh: phantom_dust_{certified,potential}
EXPECT_ABSENT entries removed (fields now legitimately present).
Migration:
- Path B per Wave 6 pre-decision: force-resolve + rebuild J51cB2
during deployment prep, no on-chain migration instruction in code.
Only 1 mainnet market live + closed beta. Operationally trivial.
Tests: 335 passing, 0 regressions.
Build: cargo build --release clean.
Verify: both engine verify scripts silent.
Follow-ups (Wave 6 sub-PRs):
- Wave 6b: dynamic-fee API rename + per-call threading
- Wave 6c: percolator-nft slab_types mirror update for 4-field schema
- Wave 6d: wrapper integration + SLAB_LEN bump (includes the 2-line
wrapper offset-print rename — already coupled to this engine PR via
path = "../percolator")
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request migrates the ChangesWave 6a Phantom-Dust 4-Field Schema
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/percolator.rs (1)
668-688: Coordinate this#[repr(C)]layout bump with the wrapper/schema consumers.Adding four
u128fields here shifts every trailingRiskEngineoffset. Please make sure the matching wrapper/SLAB_LEN/schema-version change ships atomically with this, otherwise older zero-copy readers will deserialize garbage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/percolator.rs` around lines 668 - 688, Adding four u128 fields (phantom_dust_certified_long_q, phantom_dust_certified_short_q, phantom_dust_potential_long_q, phantom_dust_potential_short_q) changes the #[repr(C)] layout and shifts trailing RiskEngine offsets; update all consumers atomically: bump the schema version constant and SLAB_LEN used by the zero-copy wrapper, adjust any wrapper/readers that compute field offsets for RiskEngine, and ensure the wrapper serialization/deserialization schemas and tests are updated to expect the new field order/size so older readers cannot misparse; coordinate these changes together in the same commit/release.examples/detailed_offsets.rs (1)
65-66: ⚡ Quick winInclude certified phantom-dust offsets in this schema dump.
With the 4-field schema, this diagnostic output should also print
phantom_dust_certified_*so layout checks cover all migrated fields, not only potential fields.Suggested diff
+ println!( + "phantom_dust_certified_long_q:{}", + offset_of!(RiskEngine, phantom_dust_certified_long_q) + ); + println!( + "phantom_dust_certified_short_q:{}", + offset_of!(RiskEngine, phantom_dust_certified_short_q) + ); println!("phantom_dust_potential_long_q:{}", offset_of!(RiskEngine, phantom_dust_potential_long_q)); println!("phantom_dust_potential_short_q:{}", offset_of!(RiskEngine, phantom_dust_potential_short_q));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/detailed_offsets.rs` around lines 65 - 66, The schema dump is missing the certified phantom-dust fields; add prints for the offsets of RiskEngine::phantom_dust_certified_long_q and RiskEngine::phantom_dust_certified_short_q using the same offset_of! pattern as the existing phantom_dust_potential_* lines so the layout check covers all four migrated fields.tests/proofs_safety.rs (1)
2364-2365: ⚡ Quick winExtend canonicalization proof to include certified phantom-dust fields.
Given the 4-field migration, this harness should also dirty and assert-reset
phantom_dust_certified_long_qandphantom_dust_certified_short_q; otherwiseinit_in_placeregressions on certified fields won’t be caught.Suggested diff
+ engine.phantom_dust_certified_long_q = 70; + engine.phantom_dust_certified_short_q = 80; engine.phantom_dust_potential_long_q = 50; engine.phantom_dust_potential_short_q = 60; @@ + assert!(engine.phantom_dust_certified_long_q == 0); + assert!(engine.phantom_dust_certified_short_q == 0); assert!(engine.phantom_dust_potential_long_q == 0); assert!(engine.phantom_dust_potential_short_q == 0);Also applies to: 2408-2409
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/proofs_safety.rs` around lines 2364 - 2365, The test harness currently dirties and resets only phantom_dust_potential_long_q and phantom_dust_potential_short_q; extend the canonicalization proof to also touch and assert-reset the certified fields by setting engine.phantom_dust_certified_long_q and engine.phantom_dust_certified_short_q to nonzero values before canonicalization and adding corresponding assertions after init_in_place to ensure they were reset; update the same pattern at the other occurrence (the block around the similar assignments at the later lines) so both locations include phantom_dust_certified_long_q and phantom_dust_certified_short_q alongside phantom_dust_potential_*.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/percolator.rs`:
- Around line 4439-4451: The current invariant only checks certified <=
potential; strengthen it to fail-fast if any phantom_dust_certified value is
non-zero on this branch: after or instead of the existing comparison, call
get_phantom_dust_certified(Side::Long) and
get_phantom_dust_certified(Side::Short) and return Err(RiskError::CorruptState)
if either is != 0 (enforcing the documented phantom_dust_certified_*_q == 0
contract), so migrations/wrapper-layout bugs cannot seed a surviving non-zero
certified value that schedule_end_of_instruction_resets() or the OI-cap logic
would ignore. Ensure the check references get_phantom_dust_certified and the
same RiskError used elsewhere.
---
Nitpick comments:
In `@examples/detailed_offsets.rs`:
- Around line 65-66: The schema dump is missing the certified phantom-dust
fields; add prints for the offsets of RiskEngine::phantom_dust_certified_long_q
and RiskEngine::phantom_dust_certified_short_q using the same offset_of! pattern
as the existing phantom_dust_potential_* lines so the layout check covers all
four migrated fields.
In `@src/percolator.rs`:
- Around line 668-688: Adding four u128 fields (phantom_dust_certified_long_q,
phantom_dust_certified_short_q, phantom_dust_potential_long_q,
phantom_dust_potential_short_q) changes the #[repr(C)] layout and shifts
trailing RiskEngine offsets; update all consumers atomically: bump the schema
version constant and SLAB_LEN used by the zero-copy wrapper, adjust any
wrapper/readers that compute field offsets for RiskEngine, and ensure the
wrapper serialization/deserialization schemas and tests are updated to expect
the new field order/size so older readers cannot misparse; coordinate these
changes together in the same commit/release.
In `@tests/proofs_safety.rs`:
- Around line 2364-2365: The test harness currently dirties and resets only
phantom_dust_potential_long_q and phantom_dust_potential_short_q; extend the
canonicalization proof to also touch and assert-reset the certified fields by
setting engine.phantom_dust_certified_long_q and
engine.phantom_dust_certified_short_q to nonzero values before canonicalization
and adding corresponding assertions after init_in_place to ensure they were
reset; update the same pattern at the other occurrence (the block around the
similar assignments at the later lines) so both locations include
phantom_dust_certified_long_q and phantom_dust_certified_short_q alongside
phantom_dust_potential_*.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 13658c5c-0b00-4dd2-8d0e-e00582e0dc78
📒 Files selected for processing (6)
examples/detailed_offsets.rssrc/percolator.rstests/proofs_instructions.rstests/proofs_invariants.rstests/proofs_liveness.rstests/proofs_safety.rs
Comment on lines
+4439
to
+4451
| // Wave 6a / KL-PHANTOM-DUST-SCHEMA-1 (REVOKED): certified is a lower | ||
| // bound, potential is the upper bound; certified must never exceed | ||
| // potential on either side. On this branch certified is always 0 | ||
| // (see field doc), so the invariant is `0 <= potential` — trivially | ||
| // true — but the explicit check grounds the get helpers and matches | ||
| // toly's defense-in-depth posture once B-tracking lands. | ||
| if self.get_phantom_dust_certified(Side::Long) | ||
| > self.get_phantom_dust_potential(Side::Long) | ||
| || self.get_phantom_dust_certified(Side::Short) | ||
| > self.get_phantom_dust_potential(Side::Short) | ||
| { | ||
| return Err(RiskError::CorruptState); | ||
| } |
There was a problem hiding this comment.
Enforce the branch’s certified == 0 contract here.
This check only guarantees certified <= potential, but the surrounding docs say phantom_dust_certified_*_q is always zero until B-tracking is ported. If a migration or wrapper-layout mistake seeds a non-zero certified value, schedule_end_of_instruction_resets() and the OI-cap logic ignore it, so the bad state can survive as a “valid” market. Fail fast on any non-zero certified dust on this branch, or gate the relaxed invariant behind the future B-tracking port.
Suggested tightening for the current branch
- if self.get_phantom_dust_certified(Side::Long)
- > self.get_phantom_dust_potential(Side::Long)
- || self.get_phantom_dust_certified(Side::Short)
- > self.get_phantom_dust_potential(Side::Short)
+ if self.get_phantom_dust_certified(Side::Long) != 0
+ || self.get_phantom_dust_certified(Side::Short) != 0
+ || self.get_phantom_dust_certified(Side::Long)
+ > self.get_phantom_dust_potential(Side::Long)
+ || self.get_phantom_dust_certified(Side::Short)
+ > self.get_phantom_dust_potential(Side::Short)
{
return Err(RiskError::CorruptState);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Wave 6a / KL-PHANTOM-DUST-SCHEMA-1 (REVOKED): certified is a lower | |
| // bound, potential is the upper bound; certified must never exceed | |
| // potential on either side. On this branch certified is always 0 | |
| // (see field doc), so the invariant is `0 <= potential` — trivially | |
| // true — but the explicit check grounds the get helpers and matches | |
| // toly's defense-in-depth posture once B-tracking lands. | |
| if self.get_phantom_dust_certified(Side::Long) | |
| > self.get_phantom_dust_potential(Side::Long) | |
| || self.get_phantom_dust_certified(Side::Short) | |
| > self.get_phantom_dust_potential(Side::Short) | |
| { | |
| return Err(RiskError::CorruptState); | |
| } | |
| // Wave 6a / KL-PHANTOM-DUST-SCHEMA-1 (REVOKED): certified is a lower | |
| // bound, potential is the upper bound; certified must never exceed | |
| // potential on either side. On this branch certified is always 0 | |
| // (see field doc), so the invariant is `0 <= potential` — trivially | |
| // true — but the explicit check grounds the get helpers and matches | |
| // toly's defense-in-depth posture once B-tracking lands. | |
| if self.get_phantom_dust_certified(Side::Long) != 0 | |
| || self.get_phantom_dust_certified(Side::Short) != 0 | |
| || self.get_phantom_dust_certified(Side::Long) | |
| > self.get_phantom_dust_potential(Side::Long) | |
| || self.get_phantom_dust_certified(Side::Short) | |
| > self.get_phantom_dust_potential(Side::Short) | |
| { | |
| return Err(RiskError::CorruptState); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/percolator.rs` around lines 4439 - 4451, The current invariant only
checks certified <= potential; strengthen it to fail-fast if any
phantom_dust_certified value is non-zero on this branch: after or instead of the
existing comparison, call get_phantom_dust_certified(Side::Long) and
get_phantom_dust_certified(Side::Short) and return Err(RiskError::CorruptState)
if either is != 0 (enforcing the documented phantom_dust_certified_*_q == 0
contract), so migrations/wrapper-layout bugs cannot seed a surviving non-zero
certified value that schedule_end_of_instruction_resets() or the OI-cap logic
would ignore. Ensure the check references get_phantom_dust_certified and the
same RiskError used elsewhere.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the long-standing phantom-dust schema divergence between fork and toly (Phase 2 KL-PHANTOM-DUST-SCHEMA-1). Adopts toly's 4-field schema (
certified+potentialper side, toly:783-786). Net RiskEngine growth: +32 bytes (2 × u128 → 4 × u128).phantom_dust_bound_<side>_q→phantom_dust_potential_<side>_q(rename only, semantically identical upper bound)phantom_dust_certified_<side>_qper side — always 0 on this branch (no B-tracking-aware liquidation logic); exists for wire-format alignment with toly + NFT vendored-bytes mirror (Wave 6c)get/set_phantom_dust_certified,get/set_phantom_dust_potentialassert_public_postconditions:certified <= potentialper sideMigration
Path B per pre-decision in FULL_SYNC_PLAN.md: force-resolve + rebuild J51cB2 during deployment prep. No on-chain migration instruction. Only 1 mainnet market live + closed beta. Operationally trivial.
Verification
Test plan
Follow-ups (Wave 6 sub-PRs)
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests