feat: New Staking workflow - #309
Conversation
Implement the SWIP-40 and SWIP-41 staking flow with delayed queue-based stake updates, withdrawals, and exits while keeping redistribution aligned with effective active stake.
Prevent queued stake withdrawals and exits from executing while a node is frozen or actively participating in the current redistribution round, and wire staking to the redistribution contract for the runtime check.
Prevent claims from finalizing when postage payout fails, and initialize staking with the expected redistribution contract so deployment catches linkage mismatches early.
Add direct effective stake coverage and make the two-reveal winner assertion resilient to deterministic state changes, while cleaning related test typing and lint issues.
Reconcile queued withdrawals after slashing and preview stake state at a specific round so upcoming-round eligibility uses the same round context as the anchor.
Prevent new stake updates from being enqueued after an exit is scheduled, and align withdrawal waits on real networks with the intended 28-day round window while keeping local settings fast.
Use overlay presence as the stake initialization check and remove the dead lastUpdatedBlockNumber field and related test assertions.
…king Allow effective withdrawals and exits to execute without current-round participation blocking them, and remove the admin-controlled redistribution hook from staking and deployment wiring.
Clarify that queued stake preview getters are forward-looking rather than historical by switching the staking and redistribution APIs from target-round naming to explicit round lookahead semantics.
- Revert FrozenWithdrawal on frozen withdrawal/exit in applyUpdates instead of silently skipping - Check _queueClosed before _previewStake so terminating queues revert QueueClosed instead of NotStaked - Enforce WAIT_OVERLAY_CHANGE >= WAIT_BASE and WAIT_WITHDRAWAL >= WAIT_BASE in constructor - Make UPDATE_QUEUE_MAX_LENGTH public
- Remove dead `Frozen` error (unused after FrozenWithdrawal was added) - Merge identical `StakeState` into `Stake`, remove `_toStakeView` - Add `_revertOnFrozen` param to `_applyReadyUpdates` so privileged callers (freezeDeposit, slashDeposit, migrateStake) break instead of reverting when a frozen withdrawal is encountered - Move `_queueClosed` check from `_enqueueUpdate` to all six public callers including `createDeposit` which previously lacked it - Update tests to expect FrozenWithdrawal revert on applyUpdates while frozen
e2e56e7 to
8bba0ee
Compare
Move FrozenWithdrawal revert into applyUpdates as a post-call check instead of threading a bool through _applyReadyUpdates. The internal function now always breaks on frozen entries.
Swap freeze and apply order in freezeDeposit so mature withdrawals settle while the node is still unfrozen, then the freeze takes effect for future rounds.
- Revert FrozenWithdrawal only when queue head is due and blocked - Transfer min(scheduled amount, balance) on withdrawal apply - Pause/unpause revert Unauthorized; zero deposit uses InvalidAmount - InvalidWithdrawalAmount(WithdrawalAmountIssue) for withdraw rejects BREAKING CHANGE: pause/unpause ABI error is Unauthorized not OnlyPauser; InvalidWithdrawalAmount now takes uint8 reason.
- Document all custom errors with @notice - BelowMinimumStake(have,need), InvalidWaitConfiguration(...), UpdateQueueFull(count,limit) BREAKING CHANGE: StakeRegistry error signatures changed.
- Expose ROUND_LENGTH; rename NetworkId to networkId - Internal _addressNotFrozen; lookahead delegates when lookahead is zero - Reconcile queue via _applyPreviewUpdate plus WithdrawTokens storage cap - Clear last-scheduled-round when queue empty; reconcile only after slash paths that keep stake - Emit StakeMigrated before payout transfer from migrateStake Tests derive round length from contract; trim redundant constants; add cases for invalid waits, queue full, frozen apply with mismatched overlay delay, migrate event. BREAKING CHANGE: networkId() replaces NetworkId(); ROUND_LENGTH on ABI.
nugaon
left a comment
There was a problem hiding this comment.
Bee should continue participating under the old effective balance and metadata while updates are still waiting in the queue. A newly requested overlay or height must not be used for commit or reveal until the delay has passed and
applyUpdates(owner)has succeeded.
The contract doesn't wait for applyUpdates to be called, the new overlay/height is the effective state, even if applyUpdates was never called <-> Bee node is still sampling under the old overlay (following the doc's advice). The commit would fail or produce an incorrect proof. Bee must handle the the changes in the queue the same way as the contract: activating the property from the return block number.
Slashing allows playing the redis game with dust. If it is considered as dead code -> remove it please.
Queue must applying actions without hook functions because head is advanced after updating the items (current BZZ ERC20 contract does not have hooks so no reentrance).
Corrected this in PR. Slashing was respected historically where we have dead code in place for slashing but never used it. We removed slashing from code as we agreed on so there is solely focus on freezing now. |
Aligns param naming with Staking: all function args use _ prefix.
Protocol penalties are freeze-only; slash was never wired from Redistribution and is no longer part of the staking surface.
All slashing parts were removed as its not used and just introduces confusion and dead code. |
|
@acud This has been reviewed and approved by the Research team. Could you please go through the details of this PR and assess whether it is ready for implementation? |
|
@0xCardiE i went through the PR description (that's about as far as i managed to get so far) but i must say i can't really understand the functional changes required in |
Would be useful to go through referred SWIPS 40 and 41 as this is source of info on what is changing. Basicaly the whole staking logic is different. Here is overview of what is changed. 1. Staking changes are delayed, not instantBeforeAfter a staking transaction, Bee could treat overlay, height, and balance as updated immediately. NowAfter Bee should
2. Commit/reveal must use effective state, not applied stateBeforeBee may have waited for NowOnce Bee should
3. applyUpdates is for settlement and sync, not for game eligibilityBee should
4. Handle frozen nodes blocking withdrawal/exit payoutWhenA queued withdrawal or exit is due, but the node is still frozen. Behavior
Bee should
5. Participation eligibility comes from Redistribution, not StakeRegistry aloneBee should
6. Effective stake is a single live valueBeforeBee may have used a committed vs potential stake split. NowUse Bee should
7. Frozen nodes can queue changes but cannot playBee should
ImportantFreeze is account-level and survives exit/restake on the same registry. After restaking on the same contract, the node stays excluded until 8. Exit closes the staking queueAfter exit() is queued
Once the exit effective round is reached
9. Plan for future state, not just current stateFor changes not yet effectiveStandard getters do not include them yet. Bee should
10. Respect queue capacityBee should
11. Recommended operator workflowFor any staking change, Bee should follow:
12. Claim payout behavior (Redistribution)Bee should
Suggested Bee implementation tasks
|
|
thanks. i'll try to look into the diff though i'm afraid it is wide enough to require a complete pass over the entire codebase to see how it all fits. regarding |
Add unit tests for AlreadyStaked, InvalidAmount, height minimum on increaseHeight, OnlyRedistributor, and AccountFreezeExtended. Assert StakeFrozen from Redistribution claim logs. Include PR test summary doc.
Move PR test summary into STAKING.md. Document missing errors, freeze behavior, and Redistribution penalty integration. Remove PR_TESTS doc.
Testing - new stakingQueue-based Hardhat177 tests · Unit (
Integration (
EchidnaHarness: Fuzzes real Properties: no unauthorized admin/freeze; migrate only when paused; registry balance ≥ staked; frozen ⇒ zero effective stake; empty overlay ⇒ zero balance. Redistribution Echidna harnesses use a mock stake registry - full queue semantics are in the staking harness + Hardhat. |
|
@0xCardiE, I've tried to review the branch entirely as I wasn't really familiar with the existing state of the contracts, so I reviewed essentially this branch against the last tag from July last year. This is effectively a new contract entirely with not much in common with the old one. While at least in theory I don't really have any good input about the contract code, I will pose the following questions about your requested changes from the Bee codebase:
|
Great if you can do some Code Review on contract but wasn't expecting that from bee team, so no worries about that part. We had some reviews from Research already. Also its true its completely new contract as most of the logic is changed. So lets go to the points.
|
Summary
balance, and a frozen node's effective stake is0lookaheadsemantics instead ofAtRoundnamingapplyUpdatesreverts on frozen withdrawals/exits, post-exit operations revertQueueClosedinstead ofNotStaked, constructor validates wait-parameter ordering_applyReadyUpdatesso privileged callers (freezeDeposit,slashDeposit,migrateStake) can never be blocked by a frozen withdrawal — they skip frozen entries instead of revertingfreezeUntilBlock; the penalty persists across exit and stake deletion on the same registry (unstaking does not clear it)freezeDepositis monotonic — a shorter freeze never reduces an existing deadlineAccountstruct (Stake,freezeUntilBlock,UpdateQueue) with partial clears (_clearStake/_clearQueue) sofreezeUntilBlockis never wiped when stake or queue is clearedBee Node Changes
createDeposit,addTokens,increaseHeight,changeOverlay,withdraw, andexitenqueue updates with aneffectiveFromRoundreturned by the call (also emitted in events). Until that round is reached, the queued change is not yet effective.WAIT_BASEfor deposit, top-up, and height increaseWAIT_OVERLAY_CHANGEfor overlay changesWAIT_WITHDRAWALfor withdraw and exitWAIT_WITHDRAWALis intended to represent roughly a 28-day delay in rounds. Local development configs may still use short values.currentRound() >= effectiveFromRound, the contract treats a queued update as effective for all view getters and for Redistribution commit/reveal — even ifapplyUpdates(owner)has not been called yet.nodeEffectiveStake(owner),overlayOfAddress(owner),heightOfAddress(owner), andstakes(owner)all simulate the ready prefix of the queue via_previewStake(same logic Redistribution uses). Bee must mirror this: switch overlay, height, and balance for commit/reveal as soon as the effective round is reached, not whenapplyUpdatessucceeds.applyUpdatesis still required, but for different reasons: it writes matured updates to storage and executes BZZ transfers for withdrawals and exits. Token payout only happens insideapplyUpdates; persistedaccount.stakemay lag behind the effective preview until it runs. Bee should call it after the wait period (and retry on schedule) so on-chain storage and balances stay in sync, but commit/reveal must use the previewed effective state immediately once the round threshold is met.applyUpdates(owner)reverts withFrozenWithdrawal(). Bee should catch this revert and retry after the freeze expires. The frozen update stays at the queue head and will execute on a subsequentapplyUpdatescall once the node is unfrozen.Redistribution.isParticipatingInUpcomingRound(owner, depth)as the source of truth for eligibility. Bee should not try to reconstruct that decision from plainStakeRegistrygetters alone, because redistribution combines staking state with round/anchor logic.effectiveFromRoundfrom the enqueue event/return value to know when they will activate:nodeEffectiveStakeLookahead(owner, lookahead)overlayOfAddressLookahead(owner, lookahead)heightOfAddressLookahead(owner, lookahead)lookahead = 0means "effective state for the current round context", andlookahead = 1means "effective state one round ahead". This replaces the previous absolute-round preview naming.nodeEffectiveStake(owner)as the live effective stake value. The previous committed/potential split is gone.nodeEffectiveStakereturns0), and queued withdrawals or exits will not execute until the freeze no longer blocks them.nodeEffectiveStake(owner)stays0untilblock.number > freezeUntilBlock(owner). Deploying a new registry starts with a clean slate; there is no on-chain freeze import between contracts.exit()is queued, the owner's staking queue is closed. Any subsequent call tocreateDeposit,addTokens,changeOverlay,increaseHeight,withdraw, orexitreverts withQueueClosed(). No further staking updates can be enqueued for that owner until the exit is applied and the queue is cleared. Bee should treat a queued exit as a terminal pending action for that stake position. Note: once the exit'seffectiveFromRoundis reached,nodeEffectiveStakebecomes0and commit/reveal will revert withNotStaked()even beforeapplyUpdatesruns.UPDATE_QUEUE_MAX_LENGTHon-chain to check queue capacity before enqueuing. The queue reverts withUpdateQueueFull()when the limit is reached.effectiveFromRound)currentRound() >= effectiveFromRoundapplyUpdates(owner)to persist state and settle token transfers (especially for withdrawals/exits)Deployment Notes
WAIT_OVERLAY_CHANGE >= WAIT_BASEandWAIT_WITHDRAWAL >= WAIT_BASE. Deployment scripts that pass wait parameters violating this invariant will fail withInvalidWaitConfiguration().migrateStake()to withdraw, deploy the successor registry, nodes restake there. Freeze penalties are not carried over automatically.Redistribution Notes
claim()now keeps payout and round finalization atomic. If the postage payout fails, the whole claim reverts and can be retried later once the underlying issue is resolved.Reference