Skip to content

STR-3252: add bookkeeping to update tracked_balance field, tests#1845

Closed
delbonis wants to merge 1 commit into
mainfrom
STR-3252-tracked-balance-decr
Closed

STR-3252: add bookkeeping to update tracked_balance field, tests#1845
delbonis wants to merge 1 commit into
mainfrom
STR-3252-tracked-balance-decr

Conversation

@delbonis
Copy link
Copy Markdown
Contributor

Description

This PR adds bookkeeping to properly decrement the tracked_balance field of EeAccountState when we're processing an update.

This requires adding a new extradata field. This is a narrow implementation of the recommendations of the ticket. This is really unfortunate, so I'm creating another PR that actually just removes this and the associated bookkeeping since it's only used as a sanity check and is not actually required for security.

Claude was used for a lot of the legwork.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature/Enhancement (non-breaking change which adds functionality or enhances an existing one)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactor
  • New or updated tests
  • Dependency Update

Notes to Reviewers

Is this PR addressing any specification, design doc or external reference document?

  • Yes
  • No

If yes, please add relevant links:

Checklist

  • I have performed a self-review of my code.
  • I have commented my code where necessary.
  • I have updated the documentation if needed.
  • My changes do not introduce new warnings.
  • I have added (where necessary) tests that prove my changes are effective or that my feature works.
  • New and existing tests pass with my changes.
  • I have disclosed my use of AI in the body of this PR.

Related Issues

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: df75333e10

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

// accumulates this value and checks it against `extra_data.value_sent`
// in `process_chunks_on_acct`, so this subtraction stays bound to what
// the chunks actually emitted.
state.try_subtract_tracked_balance(*extra_data.value_sent())?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep tracked-balance subtraction consistent with stored block state

Subtracting extra_data.value_sent here changes the state root produced by update processing, but the sequencer’s block assembly path still persists ExecBlockRecord.account_state without any corresponding output deduction (it only updates tip/queues in crates/alpen-ee/block-assembly/src/block.rs). bin/alpen-client/src/prover/spec_acct.rs then uses that persisted account_state().compute_state_root() as new_state, while this program now computes a lower post-state whenever chunk outputs carry value; for batches containing withdrawals/output messages, this creates a deterministic post-state root mismatch and breaks proof/update validation.

Useful? React with 👍 / 👎.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2026

Codecov Report

❌ Patch coverage is 79.03226% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.67%. Comparing base (d1d88bd) to head (df75333).

Files with missing lines Patch % Lines
bin/alpen-client/src/prover/spec_acct.rs 0.00% 5 Missing ⚠️
...e/sequencer/src/update_submitter/update_builder.rs 0.00% 4 Missing ⚠️
crates/ee-acct-runtime/src/verification_state.rs 33.33% 4 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (d1d88bd) and HEAD (df75333). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (d1d88bd) HEAD (df75333)
functional 1 0
@@             Coverage Diff             @@
##             main    #1845       +/-   ##
===========================================
- Coverage   79.76%   65.67%   -14.09%     
===========================================
  Files         674      668        -6     
  Lines       74805    74445      -360     
===========================================
- Hits        59665    48891    -10774     
- Misses      15140    25554    +10414     
Flag Coverage Δ
functional ?
unit 65.67% <79.03%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
bin/prover-perf/src/programs/alpen_acct.rs 89.70% <100.00%> (+0.31%) ⬆️
crates/alpen-ee/block-assembly/src/payload.rs 66.23% <ø> (-33.77%) ⬇️
crates/ee-acct-runtime/src/ee_program.rs 91.96% <100.00%> (+0.07%) ⬆️
crates/ee-acct-runtime/src/update_builder.rs 86.71% <100.00%> (+1.44%) ⬆️
crates/ee-acct-types/src/state.rs 82.17% <100.00%> (+2.63%) ⬆️
crates/proof-impl/alpen-acct/src/program.rs 88.76% <100.00%> (-7.83%) ⬇️
...e/sequencer/src/update_submitter/update_builder.rs 0.00% <0.00%> (-97.65%) ⬇️
crates/ee-acct-runtime/src/verification_state.rs 91.46% <33.33%> (-2.21%) ⬇️
bin/alpen-client/src/prover/spec_acct.rs 0.00% <0.00%> (-52.67%) ⬇️

... and 292 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link
Copy Markdown
Contributor

Commit: 04b7265

SP1 Execution Results

program cycles gas
EVM EE Chunk 565,505 771,876
EVM EE Account 422,067 527,791
Checkpoint 2,241,397 2,582,981

@delbonis delbonis closed this May 22, 2026
@storopoli storopoli deleted the STR-3252-tracked-balance-decr branch May 22, 2026 17:10
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.

1 participant