feat(stablecoin): add close_position instruction - #354
Open
gravityblast wants to merge 1 commit into
Open
Conversation
gravityblast
force-pushed
the
feat/stablecoin-close-position
branch
from
September 7, 2026 08:51
73787d4 to
eb56c62
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Several docs still claim the position PDA is released / cleared to Account::default(), but the implementation only clears account data (and one new test could better verify nonce preservation).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds the Stablecoin program’s close_position instruction (spec §10.9) to allow clearing a fully-settled position (no debt, no collateral) while leaving the vault unchanged, and wires it through the guest entrypoint and IDL.
Changes:
- Implement
close_positionhost logic with PDA/ownership validation, position/vault consistency checks, and data-clearing post-state. - Add guest
#[instruction]entrypoint forclose_positionand extend the stablecoin coreInstructionenum + IDL. - Add a focused test suite covering success and key failure cases (authorization, debt/collateral remaining, vault mismatch/nonzero balance, uninitialized globals).
File summaries
| File | Description |
|---|---|
| programs/stablecoin/src/close_position.rs | New host instruction implementation (validations + post-states, no chained calls). |
| programs/stablecoin/src/lib.rs | Exposes the new close_position module. |
| programs/stablecoin/methods/guest/src/bin/stablecoin.rs | Adds guest entrypoint for close_position. |
| programs/stablecoin/core/src/lib.rs | Adds Instruction::ClosePosition variant and documentation. |
| programs/stablecoin/src/tests.rs | Adds unit tests for the new instruction behavior and failure modes. |
| artifacts/stablecoin-idl.json | Regenerates IDL to include close_position accounts/signature. |
Review details
Suppressed comments (1)
programs/stablecoin/core/src/lib.rs:223
- These account docs say the position is cleared to
Account::default()(PDA released), but the host implementation in this PR only clears the position account’sdataand preservesprogram_owner/noncedue to runtime constraints. Updating this avoids misleading clients about whether the(owner, position_nonce)can be reused.
/// 2. `position` — initialized, owned by `self_program_id`, at its `(owner, position_nonce)`
/// PDA. Cleared to `Account::default()`.
/// 3. `vault` — initialized, read-only; must equal `Position.vault_account_id` and hold a zero
/// balance.
/// 4. `protocol_parameters` — initialized, read-only; at its canonical PDA.
- Files reviewed: 6/6 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+1334
to
+1347
| let (post_states, chained_calls) = close( | ||
| init_position_account(0, 0), | ||
| init_vault_account(), | ||
| protocol_parameters_account(false), | ||
| ); | ||
|
|
||
| assert_eq!(post_states.len(), 4); | ||
| assert!(chained_calls.is_empty()); | ||
| // Data is zeroed, but program_owner and nonce must survive — the runtime | ||
| // rejects a program changing either, so the PDA cannot be released. | ||
| let cleared = post_states[1].account(); | ||
| assert_eq!(cleared.data, Data::default()); | ||
| assert_eq!(cleared.program_owner, STABLECOIN_PROGRAM_ID); | ||
| assert_eq!(cleared.nonce, init_position_account(0, 0).account.nonce); |
Comment on lines
+212
to
+215
| /// Clear a fully-settled position, releasing its PDA. | ||
| /// | ||
| /// Allowed while frozen. The vault is **not** closed — the Token Program has | ||
| /// no `CloseHolding`, so it lingers at `balance = 0` (§12). |
Comment on lines
+334
to
+337
| /// Clear a fully-settled position, releasing its PDA (spec §10.9; host fn | ||
| /// `stablecoin_program::close_position`). | ||
| /// | ||
| /// Allowed while frozen. Emits no chained calls; the vault lingers empty. |
Comment on lines
+11
to
+12
| /// Clear a fully-settled position, releasing its PDA. | ||
| pub mod close_position; |
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
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.
Adds
close_position(spec §10.9): clears a fully-settled position.Four accounts, no chained calls. Allowed while frozen — closing a settled
position removes an obligation and cannot worsen protocol health, so
protocol_parametersis validated and PDA-pinned butis_frozenis never read.Deviates from §10.9 on one point, because the runtime forbids what the spec
describes. The spec says the position post-state is
Account::default(),"cleared; PDA released". LEE's
validate_executionrejects any post-state thatchanges an account's
program_owner(rule 4) ornonce(rule 3), andAccount::default()changes both — it fails withModifiedProgramOwner, whichis how this surfaced in the e2e run. The instruction therefore zeroes the data
and leaves ownership intact. The account lingers stablecoin-owned and empty, the
same way the vault does per §12.
Consequence worth knowing: an
(owner, position_nonce)pair cannot be reusedafterwards, since
open_positionrequires an uninitialized position account.Spec §10.9 needs a follow-up correction.
The vault-balance assertion is deliberately redundant with the position's own
collateral_amount == 0. If the two ever disagree, refusing beats strandingtokens behind a dead account.
9 tests: the clear itself plus the untouched vault, frozen, missing
authorization, outstanding debt, remaining collateral, a non-empty vault, the
wrong vault, an uninitialized position, and uninitialized
ProtocolParameters.Seventh of eight issues in Plan 3 (#173). Stacked on #353.
closes #180