Skip to content

fix: conserve staged-removal validator balance on deferred top-up#377

Merged
matthias-wright merged 2 commits into
audit-may-2026from
m/last-block-top-up
Jun 26, 2026
Merged

fix: conserve staged-removal validator balance on deferred top-up#377
matthias-wright merged 2 commits into
audit-may-2026from
m/last-block-top-up

Conversation

@matthias-wright

Copy link
Copy Markdown
Collaborator

Builds on #276 (which builds on #275).

Addresses #374.

Changes:

  • Guard the Inactive deposit branch on account.balance > 0 to distinguish a staged-removal validator (still holds a credited balance) from a new-validator placeholder (balance 0). For the former, withdraw the full credited balance (balance_deduction = balance), refund the top-up separately, and let the completion path remove the account. Placeholders and active top-ups are unaffected.
  • Add regression test test_last_block_topup_does_not_drop_staged_removal_balance.

@sebastian-osec

Copy link
Copy Markdown

Question: should the top-up refund in this staged-removal path be taxed as an invalid deposit? The top-up passed validation when accepted; it is refunded only because the earlier staged removal wins. Since this branch uses push_invalid_deposit_withdrawals, a nonzero invalid_deposit_tax sends part of the valid top-up to treasury. If that is intended policy, this is fine, but the regression test only covers tax = 0.

@matthias-wright

Copy link
Copy Markdown
Collaborator Author

Good question. I think a tax-free withdrawal is more appropriate here.

Update(ca07b67):

  • Refund staged-removal top-up untaxed

@sebastian-osec

Copy link
Copy Markdown

Thanks. If we are okay with max_withdrawals_per_epoch applied once to validator withdrawals and once to refund withdrawals, then my concern is resolved. #276 .

One small follow-up: the docs still describe max_withdrawals_per_epoch as limiting total withdrawals in the last block of each epoch, not as a per-type limit:

The `max_withdrawals_per_epoch` protocol parameter (range: 1–256) limits how many withdrawals can be included in the last block of each epoch. If more withdrawals are scheduled for an epoch than the cap allows, only the first `max_withdrawals_per_epoch` withdrawals (in queue order) are included. The remaining overflow withdrawals are rescheduled to the next epoch, placed at the **front** of that epoch's queue so they have priority over withdrawals originally scheduled for that epoch.

The latest commit addresses my original tax concern too. The staged-removal top-up refund now bypasses the invalid-deposit tax helper and refunds the full top-up amount, with a nonzero-tax regression test covering it.

@matthias-wright matthias-wright merged commit af03346 into audit-may-2026 Jun 26, 2026
4 checks passed
@matthias-wright matthias-wright deleted the m/last-block-top-up branch June 26, 2026 08:38
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