Address balances: account for storage cost of accumulator objects #24690

aschran · 2025-12-18T16:08:51Z

Adds estimated cost of objects into the storage fund balance at end of epoch.

Release notes

Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.

For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.

vercel · 2025-12-18T16:08:56Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
sui-docs	Ready	Preview, Comment	Dec 22, 2025 10:05pm

2 Skipped Deployments

Project	Deployment	Review	Updated (UTC)
multisig-toolkit	Ignored	Preview	Dec 22, 2025 10:05pm
sui-kiosk	Ignored	Preview	Dec 22, 2025 10:05pm

Adds estimated cost of objects into the storage fund balance at end of epoch.

Copilot

Pull request overview

This PR adds support for accounting for the storage cost of accumulator objects in the storage fund balance at the end of each epoch. The changes introduce a new end-of-epoch transaction type WriteAccumulatorStorageCost that computes and stores the estimated storage cost based on the count of accumulator objects, which is then incorporated into the storage fund balance calculation during epoch advancement. The protocol version is bumped from 105 to 106.

Introduces WriteAccumulatorStorageCost end-of-epoch transaction kind to record accumulator storage costs
Adds tracking of net accumulator object count through dynamic fields on the accumulator root
Updates epoch advancement logic to include accumulator storage fund amount in balance calculations

Reviewed changes

Copilot reviewed 42 out of 44 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
crates/sui-types/src/transaction.rs	Adds `WriteAccumulatorStorageCost` struct and enum variant for the new transaction type
sui-execution/*/sui-adapter/src/execution_engine.rs	Adds panic handlers in v1/v2 engines and execution logic in latest engine
crates/sui-types/src/accumulator_metadata.rs	Adds functions to track and retrieve accumulator object counts
crates/sui-framework/packages/sui-system/sources/sui_system.move	Implements storage cost reading/writing via `AccumulatorStorageCostKey` in extra_fields
crates/sui-framework/packages/sui-system/sources/sui_system_state_inner.move	Updates `advance_epoch` to include accumulator storage fund amount in calculations
crates/sui-protocol-config/src/lib.rs	Bumps protocol version to 106 and adjusts feature flag enablement
crates/sui-core/src/authority.rs	Creates `WriteAccumulatorStorageCost` transaction at end of epoch with estimated costs
crates/sui-core/src/accumulators/mod.rs	Tracks object creation/deletion in barrier transactions
crates/sui-graphql/schema.graphql	Adds `WriteAccumulatorStorageCostTransaction` to GraphQL schemas
crates/sui-json-rpc-types/src/sui_transaction.rs	Adds enum variant for JSON-RPC representation
crates/sui-types/src/*_conversions.rs	Adds todo placeholders for SDK and proto conversions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mystenmark · 2025-12-19T21:43:31Z

crates/sui-protocol-config/src/lib.rs

                105 => {
+                    cfg.feature_flags.enable_multi_epoch_transaction_expiration = true;
+                }
+                106 => {


i don't think 105 is closed for additions yet?

it may or may not be - I have experienced wonkiness in the past when making framework changes without bumping the protocol version (sometimes it doesn't pick up the new framework if there's a snapshot for the current version already), so rather than waste time thinking about it I just bumped the version. I think there's no downside to making extra ones right?

crates/sui-core/src/authority.rs

mystenmark · 2025-12-19T21:47:31Z

Looks good overall! Could probably use a comment somewhere summarizing our discussions about why we decided to do it this way

aschran · 2025-12-21T01:17:38Z

Looks good overall! Could probably use a comment somewhere summarizing our discussions about why we decided to do it this way

done

lxfind · 2025-12-21T22:57:29Z

crates/sui-framework/packages/sui-system/sources/sui_system.move

 }

+/// Key for storing the storage cost for accumulator objects, computed at end of epoch.
+public struct AccumulatorStorageCostKey has copy, drop, store {}


This doesn't need to be public?

I don't think private structs are supported yet? if I remove public I get this error

error[E01003]: invalid modifier ┌─ /var/folders/hy/nr0t30bd76907v6q_0jfst380000gn/T/.tmp8P7tpN/sui-framework/sources/accumulator_metadata.move:147:1 │ 147 │ struct AccumulatorObjectCountKey has copy, drop, store {} │ ^^^^^^ Invalid struct declaration. Internal struct declarations are not yet supported │ = Visibility annotations are required on struct declarations from the Move 2024 edition onwards.

let me know if I'm being stupid and there is another way to declare it non-public

lxfind · 2025-12-21T22:57:58Z

crates/sui-framework/packages/sui-framework/sources/accumulator_metadata.move

 use fun accumulator_owner_destroy as Owner.destroy;
+
+/// Key for storing the net count of accumulator objects as a dynamic field on the accumulator root.
+public struct AccumulatorObjectCountKey has copy, drop, store {}


This doesn't need to be public

lxfind · 2025-12-21T23:06:02Z

crates/sui-framework/packages/sui-system/sources/sui_system.move

+/// Stores the computed storage cost for accumulator objects.
+/// This is called by an end-of-epoch transaction to record the storage cost
+/// that will be used by advance_epoch.
+fun write_accumulator_storage_cost(


What is the reason to use this to pass the accumulator storage cost instead of directly passing to advance_epoch?

This is documented on record_accumulator_object_changes

hmm I only see the explanation of what this function does. What I meant is that, we could add a new argument to advance_epoch function?

It would be fairly onerous to add a new argument to advance_epoch, IIUC

lxfind · 2025-12-21T23:17:48Z

crates/sui-framework/packages/sui-framework/sources/accumulator_metadata.move

+
+/// Returns the current count of accumulator objects stored as a dynamic field.
+#[allow(unused_function)]
+fun get_accumulator_object_count(accumulator_root: &AccumulatorRoot): u64 {


This function doesn't seem to be used

sorry for confusion, I'm going to add checks to the address_balance_tests that will use it but haven't finished that yet

TIL we deprecated internal structs.

ok added the test changes

lxfind · 2025-12-21T23:19:00Z

crates/sui-core/src/authority.rs

+                Ok(Some(count)) => count,
+                Ok(None) => return None,
+                Err(e) => {
+                    debug_fatal!("failed to read accumulator object count: {}", e);


I am not sure it is safe to not panic here if this errors

I think it should be okay because either all validators will fail for the same reason, and then this will be skipped by all (consequently leading to the same value as last epoch being used for the storage fund adjustment), or else the local validator will fork at end-of-epoch and crash anyway? @mystenmark what do you think?

What are the reasons this may fail?

the only possible failures would come from derive_dynamic_field_id. possibly I'm missing something but I think that can only fail if bcs::to_bytes fails, so... unlikely

lxfind · 2025-12-21T23:25:39Z

crates/sui-protocol-config/src/lib.rs

+                    cfg.feature_flags.enable_multi_epoch_transaction_expiration = true;
+                }
+                106 => {
+                    // est. 100 bytes per object * 76 (storage_gas_price)


What if storage_gas_price changes?

If storage_gas_price changes, then we could either change this or leave it the same - the numbers here are merely an approximation anyways

But we could also just use storage_gas_price directly instead of hardcoded 76?

Possibly, but if we are changing storage_gas_price we probably want to re-evaluate this again anyway (it'd be part of a much larger gas change I'd imagine?)

no existing protocol config parameters reference other parameters, so I was loath to create that dependency here

vercel bot deployed to Preview – sui-docs December 18, 2025 16:10 View deployment

aschran force-pushed the aschran/ab-storage-2 branch from b0f5f89 to b3c0d0a Compare December 19, 2025 03:41

aschran temporarily deployed to sui-typescript-aws-kms-test-env December 19, 2025 03:41 — with GitHub Actions Inactive

aschran force-pushed the aschran/ab-storage-2 branch from b3c0d0a to 2aeb654 Compare December 19, 2025 03:42

aschran temporarily deployed to sui-typescript-aws-kms-test-env December 19, 2025 03:42 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs December 19, 2025 03:45 View deployment

aschran force-pushed the aschran/ab-storage-2 branch from 2aeb654 to c4f36d5 Compare December 19, 2025 03:46

aschran temporarily deployed to sui-typescript-aws-kms-test-env December 19, 2025 03:46 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs December 19, 2025 03:48 View deployment

aschran force-pushed the aschran/ab-storage-2 branch from c4f36d5 to 158bc6a Compare December 19, 2025 16:48

aschran temporarily deployed to sui-typescript-aws-kms-test-env December 19, 2025 16:48 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs December 19, 2025 16:50 View deployment

aschran force-pushed the aschran/ab-storage-2 branch from 158bc6a to f991234 Compare December 19, 2025 18:00

aschran temporarily deployed to sui-typescript-aws-kms-test-env December 19, 2025 18:00 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs December 19, 2025 18:02 View deployment

Address balances: account for storage cost of accumulator objects

261c059

Adds estimated cost of objects into the storage fund balance at end of epoch.

aschran force-pushed the aschran/ab-storage-2 branch from f991234 to 261c059 Compare December 19, 2025 18:30

aschran temporarily deployed to sui-typescript-aws-kms-test-env December 19, 2025 18:30 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs December 19, 2025 18:31 View deployment

aschran marked this pull request as ready for review December 19, 2025 19:43

aschran requested review from a team and mystenmark as code owners December 19, 2025 19:44

aschran requested review from amnn, emmazzz, henryachen and nickvikeras and removed request for a team December 19, 2025 19:44

aschran requested review from Copilot, evan-wall-mysten, tpham-mysten and wlmyng and removed request for mystenmark December 19, 2025 19:44

aschran temporarily deployed to sui-typescript-aws-kms-test-env December 19, 2025 19:44 — with GitHub Actions Inactive

aschran requested a review from mystenmark December 19, 2025 19:44

Copilot started reviewing on behalf of aschran December 19, 2025 19:44 View session

Copilot AI reviewed Dec 19, 2025

View reviewed changes

mystenmark reviewed Dec 19, 2025

View reviewed changes

crates/sui-core/src/authority.rs Outdated Show resolved Hide resolved

mystenmark requested a review from lxfind December 19, 2025 21:46

aschran temporarily deployed to sui-typescript-aws-kms-test-env December 21, 2025 01:17 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs December 21, 2025 01:18 View deployment

address comments

734f868

aschran force-pushed the aschran/ab-storage-2 branch from b6b058b to 734f868 Compare December 21, 2025 02:39

aschran temporarily deployed to sui-typescript-aws-kms-test-env December 21, 2025 02:39 — with GitHub Actions Inactive

vercel bot deployed to Preview – sui-docs December 21, 2025 02:40 View deployment

lxfind reviewed Dec 21, 2025

View reviewed changes

add tests verifying accumulator object count

5f49304

aschran temporarily deployed to sui-typescript-aws-kms-test-env December 22, 2025 19:42 — with GitHub Actions Inactive

aschran requested review from lxfind and mystenmark December 22, 2025 19:42

vercel bot deployed to Preview – sui-docs December 22, 2025 19:43 View deployment

update mock sui system pkgs

007afaf

aschran had a problem deploying to sui-typescript-aws-kms-test-env December 22, 2025 22:03 — with GitHub Actions Failure

vercel bot deployed to Preview – sui-docs December 22, 2025 22:05 View deployment

Address balances: account for storage cost of accumulator objects #24690

Are you sure you want to change the base?

Address balances: account for storage cost of accumulator objects #24690

Conversation

aschran commented Dec 18, 2025

Release notes

Uh oh!

vercel bot commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mystenmark commented Dec 19, 2025

Uh oh!

aschran commented Dec 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vercel bot commented Dec 18, 2025 •

edited

Loading