feat(stablecoin): rebuild open_position on the fee-aware layout - #341
feat(stablecoin): rebuild open_position on the fee-aware layout#341gravityblast wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR upgrades the Stablecoin program’s open_position instruction from the earlier scaffold to the fee-aware/spec §10.4 behavior: it now reads global ProtocolParameters (for the single collateral definition id + freeze flag) and stamps Position.opened_at from the system CLOCK_01 account. It also introduces a breaking ABI change by renaming the instruction field and expanding the account list (and regenerates the IDL accordingly).
Changes:
- Rebuild
open_positionhost logic to gate onProtocolParameters.is_frozen, bind the collateral definition viaProtocolParameters.collateral_definition_id, and setopened_atfromCLOCK_01. - Update the instruction ABI:
collateral_amount→initial_collateral_amount, and extend the required account list to includeprotocol_parametersandclock. - Update unit + integration tests and regenerate
artifacts/stablecoin-idl.jsonto match the new ABI.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| programs/stablecoin/src/open_position.rs | Implements spec §10.4 flow: reads ProtocolParameters, reads CLOCK_01, updates Position.opened_at, and echoes new read-only accounts. |
| programs/stablecoin/methods/guest/src/bin/stablecoin.rs | Updates guest entrypoint signature/account list to match the new OpenPosition ABI and forward clock + protocol_parameters. |
| programs/stablecoin/core/src/lib.rs | Updates the Instruction::OpenPosition variant field name and documents the expanded 7-account contract. |
| programs/stablecoin/src/tests.rs | Extends host-function unit tests/fixtures for frozen/uninitialized params, collateral binding, opened_at, and echoed accounts. |
| programs/integration_tests/tests/stablecoin.rs | Seeds ProtocolParameters + clock in the fixture state and updates the open/withdraw e2e test to pass the new accounts/field name. |
| artifacts/stablecoin-idl.json | Regenerates the IDL to reflect renamed params and the 7-account layout for OpenPosition. |
Suppressed comments (1)
programs/stablecoin/src/open_position.rs:86
- This panic message refers to a "token definition" but the parameter is
collateral_definition. Aligning terminology makes it clearer which account was mismatched.
"User collateral holding does not match the provided token definition"
);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
64bee78 to
f0c2e9e
Compare
37b5082 to
ffccb26
Compare
There was a problem hiding this comment.
🟡 Changes recommended
open_position’s rustdoc panic/precondition list is now out of sync with the new ProtocolParameters/clock validation behavior and should be updated to match the actual failure modes.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
Replaces the Plan 1
open_positionscaffold with the full spec §10.4 version.The scaffold took the collateral definition as a caller-supplied argument, never
read
is_frozen, and wroteopened_at = 0. It now readsProtocolParametersfor the single global
collateral_definition_idand the freeze flag, and stampsopened_atfrom the clock.Breaking ABI change.
Instruction::OpenPositionrenamescollateral_amountto
initial_collateral_amount, and the account list goes from 5 to 7 — addingprotocol_parametersandclock, both read-only. IDL regenerated. Nothing inmodules/stablecoinreferences this instruction, so the FFI module is unaffected.Note the issue text specifies
opened_atfromctx.nowand a 6-post-stateresult.
ProgramContextexposes no clock — time comes from the systemCLOCK_01account, same as the three pokes — so the host function takes a
clockaccount,decodes it through the existing
read_clock, and echoes it back. That makes 7post-states, not 6.
5 new unit tests on top of the 9 rebuilt ones: frozen protocol, uninitialized
ProtocolParameters, a collateral definition not bound at init,opened_atmatching the clock, and both new accounts echoed unchanged. The open/withdraw
e2e test seeds
ProtocolParametersand the clock directly rather than runningthe bootstrap; the real bootstrap path lands with the lifecycle test in #181.
Second of eight issues in Plan 3 (#173). Stacked on #336 — review that first;
this diff assumes
checks.rsis already inlib.rs.closes #175