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.
0xCardiE
changed the title
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
0xCardiE
force-pushed
the
feat/new_staking
branch
from
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.
Test references errors.general.queueClosed for the post-exit createDeposit revert assertion; missing key was failing TypeScript compile in CI.
Explain that break exits the loop and leaves the blocked item at head unapplied so FIFO is preserved for retry on a later call.
Label the three execution branches so queue application logic is easier to follow when reading _applyStoredUpdate.
Clarifies the helper simulates queue effects in memory without writing storage, distinct from _applyStoredUpdate.
Group stake, freezeUntilBlock, and update queue in one struct. Use _clearStake/_clearQueue so penalties survive exit and migrate. Refs #309
Replace the legacy manageStake fuzz target with EchidnaStakingHarness covering deposits, queue apply, freeze/slash, and migration. Update system/claim mocks so the echidna suite compiles against the new staking API.
Group queue items, head, and closed flag in UpdateQueue. Remove importFreezeUntilBlocks and its tests. Refs #309
|
Some observations, some of them might be considered features, but still worth identifying. Slashing associated issuesCurrent
Other possible issues
|
Not addressing any of the highlighted issues directly, but just to provide context I'll point out that the SWIP makes no attempt to describe how slashing would work with the update schedule because slashing is currently dead code. |
nugaon
left a comment
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).
| uint256 public constant MIN_STAKE = 100000000000000000; | ||
| uint256 public constant UPDATE_QUEUE_MAX_LENGTH = 10; | ||
| /// @notice Maximum staking height; prevents `2**height` overflow in `MIN_STAKE * (2 ** height)`. | ||
| uint8 public constant MAX_STAKING_HEIGHT = 128; |
There was a problem hiding this comment.
should be something like 16 since that is the max bucketDepth now.
There was a problem hiding this comment.
Is max bucketDepth possible to change in near future, should we give it some buffer anyway?
| enum WithdrawalAmountIssue { | ||
| /// Amount is zero; `withdraw` only accepts positive pulls (see `exit()` for full unwind). | ||
| Zero, | ||
| /// Amount is greater than the previewed stake balance. |
There was a problem hiding this comment.
why not dynamic? one would like to withdraw all the tokens
There was a problem hiding this comment.
not sure what was ment, please rephrase. this enum is used for more precise error output
Corrected this in PR. Slashing was respected historically where we have dead code in place for slashing but never used it. I dont have problem with changing that and that we remove slashing part in code and in staking if we all agree on that. |
Aligns param naming with Staking: all function args use _ prefix.
4 participants
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