Skip to content

Add reusable account package scaffold#1074

Draft
0xaslan wants to merge 26 commits into
mainfrom
at/extract-bm
Draft

Add reusable account package scaffold#1074
0xaslan wants to merge 26 commits into
mainfrom
at/extract-bm

Conversation

@0xaslan

@0xaslan 0xaslan commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add a standalone account Move package with proof-bound custody, app-data slots, and passive accumulator settlement.
  • Wire Predict to depend on the account package and add a predict_account app-data adapter for positions, fee summaries, stake, and builder-code state.
  • Keep this as a scaffold for the later PredictManager-to-Account rewire; existing Predict flows still use PredictManager.

Key decisions

  • Account data mutations require an owner-minted Proof at the Account API boundary, so Predict passes proofs through instead of calling assert_proof directly.
  • Account coin reads include unsettled accumulator funds, and coin writes passively settle funds before deposit/withdraw.
  • No tests were added in this scaffold commit; existing Predict tests still exercise the current PredictManager paths.

Test plan

  • sui move build --path packages/account --warnings-are-errors
  • sui move build --path packages/predict --warnings-are-errors
  • sui move test --path packages/account --gas-limit 100000000000
  • sui move test --path packages/predict --gas-limit 100000000000 (passes with existing W04039 warning in tests/plp/lp_book_tests.move:305)

0xaslan and others added 23 commits June 16, 2026 18:17
Replace the per-object `allowed_versions` set (authoritative on Registry,
mirrored on ExpiryMarket/PoolVault with permissionless sync) with one monotonic
`version_watermark` on ProtocolConfig. A gated flow asserts
`current_version!() >= watermark`; ProtocolConfig is threaded into every gated
public entrypoint and `config.assert_version()` is its first line, nowhere else.

- Setter `bump_version_watermark` derives the floor from the running
  `current_version!()` (no arbitrary target, so admin can't set it above the
  deployed binary and brick the package); monotonic, admin-only, ungated.
- Removed: Registry.allowed_versions + the two mirrors + set_allowed_versions +
  sync_* + enable_version/disable_version/disable_version_pause_cap and their
  error consts. PauseCap keeps only trading/mint pause (no version disable).
- Gate extended to the 18 ProtocolConfig admin setters and the 4 registry
  creation entrypoints; per-user PredictManager/builder_code custody stays
  ungated so user exits survive a freeze. Ungated bypasses: the watermark
  setter, kill switches, and revocations.
- Tests updated (incl. a #[test_only] watermark seam); design docs
  (architecture/configuration/decisions) + move.md updated.

sui move build --warnings-are-errors green; sui move test 283/283.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The account rewire makes every Predict trade/PLP flow take
`&sui::accumulator::AccumulatorRoot`. The only constructor is the `#[test_only]`
`sui::accumulator::create_for_testing`, which the repo-standard testnet framework
does not expose and which a dependency's test code cannot reach (framework
`#[test_only]`s are stripped for transitive deps). Predict's own canonical
framework must therefore carry it.

Predict is a new-style package, so it cannot pin a system dependency in
`[dependencies]`/`[dep-replacements]`. Use `implicit-dependencies = false` plus
`sui`/`std` git deps with `override = true` at the same nightly main commit
`account` already pins. This makes nightly the canonical `sui`/`std` Predict (and
`account`) bind to; the other in-repo deps keep the repo-standard testnet
framework (`Sui_1`), and the two `0x2` revisions link by identical manifest
digests. Deliberate, isolated dual-pin — documented in ACCUMULATOR_TESTING_STATUS.md
as temporary until a stable Sui release ships an accumulator test constructor.

`Move.lock` regenerated: `token` (rev = "main") advances c44689d -> 5411ef3.

A smoke test validates Predict-side root construction + stored-balance deposit/
withdraw across the framework split.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the deleted owned `PredictManager` with a lightweight `Trader` handle
(canonical AccountWrapper id + owner). setup now constructs the shared
AccumulatorRoot, inits the account registry, and whitelists PredictApp.
create_funded_manager creates an account through the registry and funds its DUSDC
stored balance. mint/redeem/redeem_settled call the new account-based expiry_market
entrypoints (wrapper + auth + root); check_manager reads account state via
predict_account. Flow tests hold the wrapper + root across a trade tx (take_account
/ take_root / return_account), since shared objects cannot survive a next_tx.

Flow-test call sites are not yet updated — the suite does not compile until the
dependent tests are rewired in follow-up commits.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Update every flow test and the strike-exposure-config / strike-exposure helper
tests off the deleted PredictManager and onto the Trader handle: take the shared
AccountWrapper + AccumulatorRoot for a trade tx, thread them through mint/redeem,
and read account state via predict_account accessors added to flow_test_helpers
(account_balance / position_count / fees_paid / active_stake / inactive_stake /
has_position). redeem_settled drops the Block Scholes feed (the settled path does
not price). Stale comments and abort codes from the proof era updated:
- redeem_settled on a live/unexpired market now aborts EMarketNotSettled (was the
  deleted EProofRequiredForLiveRedeem / a fall-through to ELivePricingExpired);
- the version-gate test documents that mint/redeem share assert_version with the
  cheap set_mint_paused path it stands in for.

Local bootstrap_pool helpers drop their unused PredictManager param. Full Predict
suite: 230/230 passing, 0 warnings (predict_manager_tests / registry_create_tests /
plp/lp_book_tests still pending the standalone rewire).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Delete predict_manager_tests.move (tested the removed PredictManager: caps,
  proofs, owner-gated custody) and replace it with predict_account_tests.move,
  which covers the new app-data accounting surface — position add/remove/scoping
  with the duplicate/not-found aborts, per-expiry trading-fee accumulation, and the
  lazy DEEP stake roll across epochs — against a real registry-derived Account.
- Trim registry_create_tests.move's obsolete create_manager test (managers now come
  from the account registry); keep the register_underlying coverage.
- Rewrite plp/lp_book_tests.move against the account-agnostic lp_book API
  (request_supply/withdraw take account_id + recipient; cancel returns the escrowed
  refund Balance), dropping the Fixture/PredictManager scaffolding. All drain
  economics assertions are unchanged and hand-derived.

Full Predict suite: 265/265 passing, 0 warnings; build green --warnings-are-errors.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Rename predict_manager_id -> account_id across schema, handlers, server,
  migration, and API docs (28 files). The Move order/LP events renamed the field;
  BCS decode was already positional-correct, this aligns the column/API naming.
- Fix the BuilderCodeCreated / BuilderCodeSet / BuilderFeesClaimed decode structs:
  their MoveStruct MODULE moved from the deleted `account_events` to the new
  `builder_code_events` module (a real event-type-resolution fix), and the
  PREDICT_MODULES allowlist follows.
- Remove the handlers, decode structs, schema tables, migration tables, and the
  /managers server endpoint for the four events the rewire deleted
  (PredictManagerCreated + the trade/deposit/withdraw cap-minted events). Account
  creation/custody now lives in the `account` package and is served by its own
  indexer suite; get_manager_state drops the creation row and keeps builder-code
  state.

All three crates build clean (zero warnings); cargo test --no-run is green.
Full handler test runs need a Postgres TempDb (not run here).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…er test

The deleted predict_manager::EMaxCapsReached guard reference is removed, and the
stale 'AccumulatorRoot cannot be constructed' claim is corrected: an empty root is
now constructable via accumulator::create_for_testing, and claim_all_builder_fees
runs assert_owner before any fund access, so the builder_code::ENotOwner guard is
now unit-testable. The non-zero builder-fee claim still needs the settlement barrier.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

// === Structs ===
/// Shared account wrapper: owner-gated shell around the reusable account state.
public struct AccountWrapper has key {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this wrapper? I am not sure I follow how it helps looking at existing functionality it offers!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Account is a store-only struct embedded in the wrapper, not a standalone shared object. Sui transactions can only pass/borrow shared key objects, so we need a shared handle to load an account through — that is AccountWrapper. It is also the authority boundary: load_account_mut consumes an Auth hot potato and checks it against the account owner before handing out &mut Account. Without the wrapper there is no shared object to load the account through, and nowhere to gate mutable access.

Comment thread packages/account/sources/account_registry.move Outdated
Comment thread packages/account/sources/account_registry.move Outdated
}

/// Create the sender's canonical derived account wrapper.
public fun new(registry: &mut AccountRegistry, ctx: &mut TxContext): AccountWrapper {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if we should make this permission-less by design (anyone can create an account, and it just takes the owner: address which is 1:1 for objects / wallet addresses)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping the current design. The two creation paths mirror the two auth paths: new derives the owner from ctx.sender() (same proof as generate_auth), and new_self_owned takes the owning objects &mut UID(same proof asgenerate_auth_as_object). So the authority that creates an account is the authority that can load it. A permissionless new(registry, owner: address)` would decouple that — anyone could materialize an object-owned account without proving control of the object. Accounts are derived deterministically so there is no functional gap, but we would rather keep creation aligned with the auth boundary.

Comment on lines +105 to +107
public fun account_id(self: &Account): ID {
self.account_id.to_inner()
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really needed; can always run object::id(&account)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Account is store-only (embedded in the wrapper, not a key object), so object::id(&account) does not compile — object::id requires key. The canonical account identity is the Accounts own account_id: UID, which is distinct from the wrapper object id, and account_id() exposes its inner ID — the one used for accumulator delivery, app-data storage, and event attribution. Happy to rename if the name is the concern, but the getter itself is needed.

Comment thread packages/account/sources/account.move Outdated
}

/// Claim settled address-balance funds for `T` into account custody.
public fun settle<T>(self: &mut Account, root: &AccumulatorRoot): u64 {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This keeps the parallelization issues we talked about; flagging as I don't see a comment

Per PR #1074 review: address<->ID conversion is trivial via native `.to_id()` /
`.to_address()`, so `derived_id` / `derived_wrapper_id` duplicated
`derived_address` / `derived_wrapper_address`. Remove both getters and update every
call site (account tests + the predict flow-test helper/smoke test that derived the
wrapper id) to `derived_wrapper_address(...).to_id()`; README updated to match.

Account 10/10, Predict 266/266, both build --warnings-are-errors clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
0xaslan and others added 2 commits June 18, 2026 09:42
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AppAuthorized/AppDeauthorized on authorize_app/deauthorize_app; Deposited/Withdrawn
on deposit/withdraw; FundsSettled on a nonzero accumulator settlement (the >0 guard;
the nonzero path needs barrier integration coverage, the amount==0 no-emit path is
unit-tested). type_name uses with_defining_ids (get is deprecated).

Account 13/13, build --warnings-are-errors clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
EOF
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants