Account package + Predict-uses-Account (stable testnet framework)#1077
Merged
Conversation
Clean port off the nightly-framework dev branch (at/extract-bm): adds the new `account` package and the Predict account-rewire, on the repo-standard testnet framework — no nightly `implicit-dependencies`/override pins. Indexer, sims, and Rust crates are intentionally excluded; this is a Move-only verification branch. The only nightly-only symbol, `sui::accumulator::create_for_testing` (the sole `AccumulatorRoot` constructor), is absent from testnet, so the AccumulatorRoot- dependent tests are disabled behind the single `accumulator_support` seam (no-op `create_shared_root`): 16 root-only test files renamed `*.move.disabled` (all `flows/*`, the accumulator smoke test, the two pure-root strike files) plus the custody/mint/builder-fee tests block-commented in their mixed files. Re-enable when a stable Sui ships the constructor: restore the seam body + the disabled files. Green on testnet sui 1.73.1: account 10/10, predict 198/198 (68 predict + 3 account root tests disabled); both source builds clean under --warnings-are-errors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Asserting emitted-event fields in Move was an outlier — no other event module in the repo (predict's 4, deepbook core) ships #[test_only] field getters, and field-level event coverage is the indexer's Rust decode/map tests (per predict-indexer.md). Removes the 11 #[test_only] getters from account_events.move and the event-assertion tests: - new_emits_account_created deleted (creation is covered by registry_creates_one_canonical_account_per_owner); - authorize_and_deauthorize_emit_events -> authorize_then_deauthorize_app, a behavior test asserting is_app_authorized true->false (keeps the deauthorize happy path); - the disabled custody_emits block deleted (its balance behavior is covered by the AccumulatorRoot-gated owner_auth test). Testnet sui 1.73.1: account 9/9, predict 198/198, both clean under --warnings-are-errors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Predict's source has never referenced the `deepbook` package (0 `deepbook::` uses on
main and here), and none of its other deps pull it transitively, so `deepbook =
{ local = "../deepbook" }` was a dead manifest declaration. Removing it takes deepbook
out of the dependency/lock graph entirely. DEEP staking + burning use the `token`
package directly, which stays.
Testnet sui 1.73.1: predict 198/198, clean under --warnings-are-errors.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The account rewire removed predict_manager::deposit (the old user funding path) and left no PTB-callable way to fund an account: account::deposit takes &mut Account, only reachable via load_account_mut, whose &mut Account result can't cross PTB commands. deposit_funds(wrapper, auth, coin, root, clock) folds authorize -> load -> deposit into one entrypoint (same shape as predict's mint/redeem), restoring transaction-level account funding. Testnet sui 1.73.1: account builds clean (--warnings-are-errors), 9/9. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Symmetric to deposit_funds: folds authorize -> load -> withdraw into one PTB-friendly call so a PTB doesn't need to carry &mut Account across commands. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tonylee08
reviewed
Jun 18, 2026
tonylee08
reviewed
Jun 18, 2026
tonylee08
approved these changes
Jun 18, 2026
tonylee08
left a comment
Collaborator
There was a problem hiding this comment.
lgtm overall, just small nits. Also in follow up PR we should update docs based on new architecture
The deleted predict_manager+cap model is replaced by the `account` package (deterministic shared AccountWrapper + per-call owner `Auth` hot potato): - run.sh: extract the transitively-published account package id (by the `account_registry` module it declares, not published[-1]=predict), the shared AccountRegistry, and the AccountAdminCap; authorize PredictApp on the registry (mirrors flow_test_helpers); write the account ids into .env.localnet. - env.ts: export ACCOUNT_PACKAGE_ID / ACCOUNT_REGISTRY_ID. - runtime.ts: add accountTarget() + a generateAuth() helper; createAccountTx (account_registry::new + account::share), depositToAccountTx + supply funding via account::deposit_funds, deriveAccountWrapperId (derived AccountWrapperKey), and the new mint/redeem/request_supply/request_withdraw signatures (Auth + root reordered after the trade primitives). Manager caps and proof minting deleted. - sim.ts / shared.ts: SimState carries the account wrapper id (caps removed); setup creates+funds the account; orchestration comments updated to "account". Economic-parity labels (manager_balance/manager_seed) are kept unchanged since they are account-agnostic and shared with the Python mirror. Also fix three pre-existing setup-call signature drifts the sim never caught up to (the predict contract threaded ProtocolConfig in): registry::mint_lifecycle_cap, registry::register_underlying, plp::lock_capital. Verified: tsc --noEmit, bash -n run.sh, and a localnet smoke (SUI 1.73.1, bash run.sh --sim_max_rows=2) — setup + mint + redeem + supply + bootstrap flush all execute, and localnet/Python parity is OK. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The account package (and Predict-on-account) requires the 1.73.x framework + runtime: sui::derived_object, sui::accumulator::AccumulatorRoot (the 0xacc settlement barrier), balance::settled_funds_value/withdraw_funds_from_object/ redeem_funds, and std::internal::Permit. On the old 1.69.1 image the package fails to build/publish (missing framework modules) and there is no accumulator root at genesis for the deposit/flush/settle flow. NOTE: the deployed predict-sim bench image must be rebuilt + redeployed for this to take effect; the Dockerfile ARG alone does nothing until the bench service pulls a new image. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
pei-mysten
approved these changes
Jun 18, 2026
The Sui funds accumulator authorizes a withdrawal from address X only if X is a transaction-input object. The account delivered/settled funds at the nested `account_id` UID, which is `derived_object::claim`'d but never instantiated as a standalone object — so it is never a tx input, and on a real node the settle aborts with "Unauthenticated funds-accumulator Split" (it panicked the validator in the localnet sim's first withdraw). Only the shared `AccountWrapper` is a real input object; the deep `&mut Account` predict threads everywhere can never reach its UID. (`builder_code` already settles correctly from its own shared-object id.) Fix — anchor the accumulator on `wrapper.id` and settle at the flow boundary: - account: `Account` stores `receive_address` (= wrapper address), set in `new_derived`. `receive_address()`/`balance()`/`unsettled_balance()` read it. New permissionless `settle<T>(wrapper, root, clock)` withdraws via `&mut wrapper.id` (authenticates) into stored balance. `deposit<T>`/`withdraw<T>` are now pure stored-balance (no `root`/`clock`); `deposit_funds`/`withdraw_funds` settle first. `account_id` stays the identity + app-data root. - predict: each coin-touching entrypoint (mint/redeem/redeem_settled, request_supply /request_withdraw, stake/unstake, cancels) calls `account::settle<T>(wrapper, ...)` before `load_account_mut`; `root` removed from the now forward-only internals; deep `account.deposit/withdraw` drop `root`/`clock`. Public entrypoint signatures are unchanged (they keep `root` for the boundary settle), so the simulation is untouched. Permissionless settle only consolidates the account's own delivered funds into its own stored balance; pulling funds out still requires `load_account_mut(auth)`. Verified: account 9/9, predict 198/198 (1.73.1, --warnings-are-errors); sim tsc/bash -n unchanged; and a 100-row localnet run settles 4 withdraw rows against flush-delivered PLP with no accumulator panic (the exact failure that previously aborted at the first withdraw). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The full localnet run completes for the first time (post accumulator fix) and surfaced a localnet/Python parity mismatch: open/liquidated order counts drift by ±1 from ~step 508 while every money field stays bit-identical. Root cause: the Python replay skipped the bounded liquidation pass when redeeming an already-liquidated order. Move `redeem_internal` runs `liquidate_live_orders` unconditionally in the live-market path — before the is-liquidated check — so a redeem of a liquidated order still advances the passive-scan watermark. Skipping it in Python silently drifted the watermark (first such redeem at step 285), later flipping which borderline orders the bounded scan selects (surfacing at step 508). The liquidation predicate, pricing, floor index, LTV, candidate ordering, and watermark mechanics were already bit-exact mirrors; settlement is stubbed in the model (market never settles), so the pass is unconditional on every redeem. Verified: re-running python_replay over the prior run's exact generated scenario now matches that run's localnet local_data.json bit-for-bit across all 1000 rows. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Summary
accountpackage (reusable on-chain identity/custody primitive) and rewires Predict to use it, ported off the nightly-framework dev branch (at/extract-bm) onto the repo-standard testnet framework.sui::accumulator::create_for_testing(the soleAccumulatorRootconstructor), is absent on testnet, so theAccumulatorRoot-dependent tests are disabled behind a single test seam.Key decisions
implicit-dependencies = false+sui/stdoverridepins frompredict/Move.tomland the nightlySuipin fromaccount/Move.toml; lockfiles regenerated against testnet. The non-test source builds cleanly on testnetsui 1.73.1— only the test constructor was ever nightly-only.create_for_testingis quarantined behindaccumulator_support::create_shared_root(a#[test_only]module per package). On this branch it is a no-op; root-dependent tests are disabled: 16 root-only test files renamed*.move.disabled(allflows/*, the accumulator smoke test, the two pure-root strike files) and the custody/mint/builder-fee tests block-commented in their mixed files. The same seam exists on the dev branch (nightly, full suite green), so the two branches don't diverge — only the toggle is branch-local.create_for_testing: restore the twoaccumulator_support.movebodies,git mvthe*.move.disabledfiles back, and uncomment the markedDISABLED(testnet-fw)blocks.Test plan
accountbuilds + tests on testnetsui 1.73.1: 10/10predictbuilds + tests on testnetsui 1.73.1: 198/198 (68 predict + 3 accountAccumulatorRoottests disabled)sui move build --warnings-are-errorscreate_for_testing🤖 Generated with Claude Code