fix: fix critical unwrap bug by pawel-gebal-reef · Pull Request #15 · bittensor-church/alpha-wrapper

pawel-gebal-reef · 2026-06-02T16:24:21Z

Summary

Users could get permanently locked out of withdrawing their alpha. This PR fixes that root cause and hardens every stake-moving path against the same class of failure.

The problem

The vault moves alpha stake on the user's behalf — on deposit, on withdrawal, and when it rebalances toward the active validator set. Every one of those moves has to clear the chain's minimum-stake threshold, which the chain measures in TAO.

The vault instead gated its moves on a fixed alpha minimum. Alpha and TAO aren't interchangeable: their ratio is the alpha price, which drifts over time. So the vault's own check and the chain's real check disagreed, and on the wrong side of that drift the vault would refuse a withdrawal the chain would have accepted. A position that landed there could not be unwrapped at all.

The fix

The vault now measures every move against the chain's real TAO floor — the minimum-stake threshold converted through the live alpha price — and tracks the chain's own acceptance to within one oracle price-step: the only requests it refuses that the chain might have taken sit in a sub-0.004-TAO rounding band, rejected up front instead of burning the transaction's gas on a doomed move. The single rigid pre-check is replaced with handling that fits each path:

Deposit — a deposit worth less than the floor is rejected immediately with a clear error, instead of being accepted and then stranded.
Withdraw (alpha) — the position is consolidated onto one hotkey and delivered in full; a position too small to move on-chain is pointed at the TAO exit. Delivery is exact to within a few RAO of chain-side share rounding — you receive what the preview quoted, or the call reverts.
Withdraw (TAO) — everything sellable is sold; only genuine sub-floor dust is left, and the caller's slippage limit guards the remainder.
Rebalance — moves too small to land are skipped rather than attempted, since a rejected move would burn the whole transaction's gas.
Validator rotation — stake left on a validator that dropped out of the set is swept back into the active position on the next call, so nothing is ever stranded.

The floor is a single owner-tunable value that follows the chain's minimum without a redeploy.

Supporting changes

Live stake accounting — share price now reads the position's real on-chain stake rather than a cached figure, so validator rewards accrue to holders automatically.
Alpha-price interface — a small interface over the on-chain alpha-price precompile, used for the floor conversion.
Tests & e2e — unit and fuzz coverage for the floor behavior, exact-delivery, and rotation/consolidation, plus a live-chain e2e suite that exercises the floor boundary against a real localnet.

fine135 · 2026-06-09T15:23:17Z

I think frontend ABI wasn't updated.

fine135 · 2026-06-09T15:24:13Z

I think previewWithdraw now can return different value from real withdraw (when it leaves the dust)

fine135 · 2026-06-09T15:27:15Z

I'm not sure but I think withdrawForTao can leave a position permanently stuck. It calls removeStake raw (no try/catch, unlike the other stake ops), and since the vault always unstakes a pro-rata assets that's strictly less than the on-chain balance once any rewards have accrued, it never hits subtensor's full-unstake floor bypass. So for a sub-floor position the partial removeStake reverts AmountTooLow, while the alpha-rail withdraw also reverts WithdrawTooSmall and points the user back to withdrawForTao. That seems to leave both exit rails reverting with no way out, though it's bounded to dust-sized. Could you please double-check this?

pawel-gebal-reef · 2026-06-10T07:16:09Z

I think frontend ABI wasn't updated.

on purpose. I am thinking about deleting it in M3. The client will have its own frontend anyway, it's only a burden for us to maintain it.

fine135

Overall the code looks good :) please just take a look at the comments. Thanks!

fine135 · 2026-06-09T14:59:11Z

+    }
+
+    function _requireSubFloorElseBubble(bytes memory err, uint256 alpha, uint16 netuid) private view {
+        if (_isBelowMinStake(alpha, netuid)) return;


shouldn't we check for error here? We can have below min stake amount but the error can be different. I think we should decode err and verify it agains AmountTooLow error signature

That's not really doable because there is no error ABI to much. It;s not part of any public API. We would need to write a funciton that parser bytes and then matches them with some implemantation internal detail error message produced by Subtensor.
I think the worst that can happen if we assume that it's this particular AmountTooLow error is that some dust is lost (the thing that we are traying to protect against, but I think it's very unlikely this will happen

fine135 · 2026-06-09T15:09:41Z

+        uint256 delivered = _drainAssets(hotkeys, balances, validatorCount, clone, netuid, userSubstrateColdkey, assets);
+        // Burning shares for a zero-delivery transfer would forfeit the whole position; the
+        // request is non-transferable on the alpha rail, so revert and leave withdrawForTao.
+        if (delivered == 0) revert WithdrawTooSmall();


I think we don't need this line when we check for minAlphaOut below

We need it. It's a protection to not burn funds and get completely nothing from it.

If we remove the guard and with minAlphaOut = 0 a zero-delivery withdrawal succeeds: all shares burn, nothing is delivered, and the entire position is forfeited to the remaining holders.

fine135 · 2026-06-10T10:36:26Z

@@ -279,10 +290,14 @@ contract AlphaVault is ERC1155, ERC1155Supply, Ownable, ReentrancyGuard {
        if (assets == 0) revert ZeroAmount();
        _burn(msg.sender, tokenId, shares);


I'm not sure but I think the live withdraw path can silently redistribute value from the withdrawer to the remaining holders. It burns the full shares up front (_burn before _drainAssets), but _drainAssets swallows any sub-floor slice and can deliver delivered < assets. The undelivered remainder physically stays in the clone, and since _alignToWeights resets totalStake to the actual on-chain sum, that leftover alpha ends up backing the remaining (now fewer) shares - so it accrues to everyone else's share price. With minAlphaOut defaulting to 0 there's no revert and no signal: the withdrawer just receives less than their pro-rata and eats the difference. It looks bounded to sub-floor dust (a full forfeit hits delivered == 0 → WithdrawTooSmall instead), but it's a real value transfer rather than just stuck funds, and the same mechanism looks exploitable if someone depresses the AMM price to force more slices sub-floor. Could you please double-check this?

I think it's fine, it's not a way to extrct value from holders + we have a slippage protection.

pawel-gebal-reef · 2026-07-07T08:41:35Z

+
+    /// @dev Forces getAlphaPrice to read 0, simulating a sub-1e-9 subnet the EVM oracle cannot
+    ///      represent; exercises the vault's oracle-soft fall-through to the chain's own floor.
+    function setAlphaPriceZero(uint16 netuid, bool isZero) external {


why a separate function instead of providing 0 tto setAlphaPrice?

Done — collapsed into setAlphaPrice(netuid, 0). The blocker was that a stored 0 was indistinguishable from "unset" (which defaults to 1e18 for the chain-side price). Added an _isSet sentinel so setAlphaPrice(netuid, 0) now models the zero oracle read cleanly. Removed setAlphaPriceZero and _priceIsZero, and dropped the now-redundant bool on the test helper.

pawel-gebal-reef · 2026-07-07T08:43:36Z

+# ============================================================================
+# alpha-wrapper - Localnet E2E: min-stake floor + roller consolidation
+# ============================================================================
+# Focused live-chain coverage for the unwrap-bug fix:


This is too technical and doessn't explain the gist of why this was needed. This has to beee high level explanation of the test sceeenarios.
this should not refer to any past facts about the ccode; like bugs.

Done — rewrote the header as a high-level, three-scenario description (boundary enforcement / rotated-dust consolidation / sub-floor move skipped). Removed the "unwrap-bug fix" reference and the implementation detail.

pawel-gebal-reef · 2026-07-07T08:45:02Z

    python3 -c "import json; print(json.load(open('$HOME/.bittensor/wallets/$1/hotkeys/$2')).get('ss58Address',''))"
 }

 # ─── Chain read/write wrappers (DRY helpers over cast) ────────────────────────


Skip me this block comments.

In general, leave only comments that explain non obvious facts that aree not conveyed by variable or function names.

Done — removed the section-divider block comments; kept only the few comments that state something the function/variable names do not.

pawel-gebal-reef · 2026-07-07T08:45:45Z

 # ─── Chain read/write wrappers (DRY helpers over cast) ────────────────────────

 # Alpha stake for a single (hotkey, coldkey, netuid) from the staking precompile.
 get_stake() { # <hotkey_b32> <coldkey_b32> <netuid>


<hotkey_b32> <coldkey_b32> , leave out this comment, same for the sum_stake and following functions

Done — stripped the # <hotkey_b32> <coldkey_b32> <netuid>-style signature comments from get_stake, sum_stake, and the other helpers.

pawel-gebal-reef · 2026-07-07T08:46:46Z

-e2e_bootstrap() {
-    # All bootstrap/helper paths are repo-root-relative; fail fast if invoked elsewhere.
-    [[ -f scripts/chain_ops.py && -d src ]] || fail "Run from the repo root (CWD must contain scripts/ and src/)."
+# --- Environment bring-up ---------------------------------------------------


get rid of those block comment

Done — removed the "Bootstrap: pre-flight + phases 0-5" block comment above the setup function.

pawel-gebal-reef · 2026-07-07T08:48:40Z

+FB_HK_SS58="${ALL_HK_SS58S[0]}"
+FB_PRICE=$(cast call "$ALPHA" "getAlphaPrice(uint16)(uint256)" "$FB_NET" --rpc-url "$RPC_URL" | awk '{print $1}')
+[[ "$FB_PRICE" != "0" ]] || fail "Phase 6: alpha price reads 0 (oracle unavailable)"
+# boundaryAlpha = ceil(minStakeTaoFloor * 1e18 / price); the default floor is 2e6 RAO.


delete this comment

Done — deleted the boundaryAlpha = ceil(...) inline comment; the formula now lives once in the floor_boundary helper.

pawel-gebal-reef · 2026-07-07T08:49:05Z

+# The classifier's fixed-point floor must agree with the chain at the exact boundary. Compute the
+# boundary alpha from the live price and prove wrap rejects boundary-1 and accepts boundary. This
+# is a live-chain-only check the unit mocks (fixed price) cannot provide.
+FB_NET="${NETUIDS[0]}"


Use more descriptive variable naming in this file. I don't understand what name means.

Done — renamed every cryptic FB_/DL_/GB_ variable to a role name: FLOOR_NETUID, BOUNDARY_ALPHA, BELOW_FLOOR_ALPHA, DUST_TOKEN_ID, DUST_HOTKEY/ROTATED_IN_HOTKEY, OVER_HOTKEY/UNDER_HOTKEY, SKIP_*, and so on.

pawel-gebal-reef · 2026-07-07T08:49:59Z

+[[ "$CONSERVED" == "yes" ]] || fail "Phase 7: backing did not fold in deposit + reclaimed dust ($DL_TOTAL_AFTER)"
+ok "Backing folded in the fresh deposit and the reclaimed dust; snapshot refreshed to the current set"
+
+log "Phase 8: sub-floor rebalance residue skipped pre-call within a fixed gas budget"


come up with a better phase names, those have to be more public, high level lables

Done — dropped the "Phase N" numbering for descriptive labels: "Wrap enforces the stake floor at the exact boundary", "Rotated-out dust is consolidated by the next wrap", "A sub-floor rebalance move is skipped, not attempted".

pawel-gebal-reef · 2026-07-07T08:51:03Z

+    function getAlphaPrice(uint16 netuid) external view returns (uint256);
+}
+
+/// @dev Alpha precompile address on Bittensor EVM.


this is self explantary var name. Delete the comment, it also repeats dev note. Tidy this file up

Done — deleted the redundant @dev on ALPHA_PRECOMPILE (it repeated the interface note); the name plus the literal address are self-documenting. File tidied.

pawel-gebal-reef · 2026-07-07T08:52:39Z

    uint16 private constant BPS_BASE = 10_000;
+    /// @dev 8x the chain's current 2e6 floor: headroom to track chain-side increases while keeping
+    ///      any misconfiguration's dust misclassification bounded.
+    uint256 private constant MAX_MIN_STAKE_TAO_FLOOR = 16e6;


WTF is this name? MAX_MIN_STAKE? that's an abomination of a good name.

Fair — renamed MAX_MIN_STAKE_TAO_FLOOR -> STAKE_FLOOR_CAP (the ceiling for minStakeTaoFloor), and INITIAL_MIN_STAKE_TAO_FLOOR -> INITIAL_STAKE_FLOOR.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Union-richest roll seeding kept load-bearing and documented; quantized MockAlpha with full-precision chain price; floor knob, boundary, escape-hatch, and preview-divergence coverage; property fuzz tests for the floor classifier, consolidation trichotomy, exact delivery, and TAO-rail dust bound; e2e floor leg redesigned around a raised vault floor with selector-pinned reverts; unwraps CSV column fixed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

pawel-gebal-reef force-pushed the pg/fix_unwrap_bug branch from abd377b to f8ea426 Compare June 3, 2026 09:02

pawel-gebal-reef marked this pull request as ready for review June 3, 2026 10:37

pawel-gebal-reef force-pushed the pg/fix_unwrap_bug branch 4 times, most recently from 1fb3e78 to 093b874 Compare June 3, 2026 15:18

fine135 requested changes Jun 10, 2026

View reviewed changes

pawel-gebal-reef force-pushed the pg/fix_unwrap_bug branch 3 times, most recently from 231e868 to bf441fa Compare June 22, 2026 10:34

pawel-gebal-reef force-pushed the pg/fix_unwrap_bug branch 5 times, most recently from 690c5be to 59bb467 Compare July 3, 2026 15:49

pawel-gebal-reef commented Jul 7, 2026

View reviewed changes

pawel-gebal-reef force-pushed the pg/fix_unwrap_bug branch from 59bb467 to 5e2d36a Compare July 7, 2026 09:38

fix: fix unwrap bug by floor-aware wrap/unwrap exit handling

93057e2

pawel-gebal-reef force-pushed the pg/fix_unwrap_bug branch from 5e2d36a to 93057e2 Compare July 7, 2026 12:31

pawel-gebal-reef and others added 2 commits July 10, 2026 08:13

Merge branch 'main' into pg/fix_unwrap_bug

711d272

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

		@@ -279,10 +290,14 @@ contract AlphaVault is ERC1155, ERC1155Supply, Ownable, ReentrancyGuard {
		if (assets == 0) revert ZeroAmount();
		_burn(msg.sender, tokenId, shares);

Uh oh!

Conversation

pawel-gebal-reef commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

The problem

The fix

Supporting changes

Uh oh!

fine135 commented Jun 9, 2026

Uh oh!

fine135 commented Jun 9, 2026

Uh oh!

fine135 commented Jun 9, 2026

Uh oh!

pawel-gebal-reef commented Jun 10, 2026

Uh oh!

fine135 left a comment

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!

Uh oh!

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!

pawel-gebal-reef Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

pawel-gebal-reef commented Jun 2, 2026 •

edited

Loading

pawel-gebal-reef Jun 12, 2026 •

edited

Loading