feat: dynamic per-subnet validator sets with an admin-adjustable cap#26
feat: dynamic per-subnet validator sets with an admin-adjustable cap#26pawel-gebal-reef wants to merge 8 commits into
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e tests Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pawel-gebal-reef
changed the title
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pawel-gebal-reef
force-pushed
the
pg/valis-num---bump
branch
from
0ffae47 to
a81a961
Compare
| return _generateHotkeys(count, "generated-hotkey"); | ||
| } | ||
|
|
||
| function _generateHotkeys(uint256 count, string memory tag) internal pure returns (bytes32[] memory hotkeys) { |
There was a problem hiding this comment.
is it used except for _generarateHotkeys(uint256 count) call?
| assertEq(_totalVaultStakeAcrossHotkeys(NETUID1), 90 ether); | ||
| } | ||
|
|
||
| uint256 private constant FRESH_NETUID = 93; |
There was a problem hiding this comment.
This and the folowing pricate constants should be defined at the top of the file, together with other vars/constants
| _setValidators(FRESH_NETUID, hotkeys, weights); | ||
| _setRegBlock(FRESH_NETUID, 93); | ||
| _simulateAlphaDepositHotkey(alice, FRESH_NETUID, deposit, hotkeys[count - 1]); | ||
| _wrapHotkey(alice, FRESH_NETUID, hotkeys[count - 1]); |
There was a problem hiding this comment.
This name doesn't tell much. I would prefer a function name that better explain what the functions does.
| } | ||
| } | ||
|
|
||
| /// @dev Mirrors the documented weight-to-target rule: BPS share per slot, remainder on |
There was a problem hiding this comment.
You refer to some kind of docs that are not part of this repo/project. Drop this comment, or explain better what the function does.
What I don't like in general is that the maths and the function is not obvious. IF it's used for expected values in assertions. That's bad. Those expected values should rather be hardcode than computed this way. Expected values have to be easily verifiable by a software dev "in-memory"
| (bytes32[] memory hotkeys, uint256[] memory weights, uint256 tokenId) = _wrapFreshSubnet(20, FRESH_DEPOSIT); | ||
|
|
||
| assertEq(vault.totalStake(tokenId), FRESH_DEPOSIT); | ||
| uint256[] memory targets = _expectedTargets(FRESH_DEPOSIT, weights); |
There was a problem hiding this comment.
That's what I was talking about in prev. comment. This shouldn't be computed here in some wierd way (likely duplicating what the code does). It's another surface for a potential error.
…cap changes - restore the lost add_stake dispatch in chain_ops.py; all three localnet e2e jobs died at the first stake with zero RAO landed - drop the hardcoded 500k gas on updateValidators in favor of eth_estimateGas (a 20-validator commit needs ~673k) - pin threads = 4 in [profile.ci] so FOUNDRY_PROFILE=ci forge snapshot reproduces CI without a CLI flag - cache the last-seen set length in _sweepRotatedStake and regenerate gas snapshots - size get_vault_state columns by the attested set and tighten test names and assertions Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
forge 1.7.x silently ignores `threads` in foundry.toml, so the previous commit's [profile.ci] pin regenerated .gas-snapshot at the local core count (2) while CI ran at 4 threads, diverging the fuzz input streams. Restore the --threads 4 CLI flag and regenerate at 4 threads with the CI-pinned forge v1.7.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Summary
Replaces the fixed 3-validator cap in
ValidatorRegistrywith an admin-adjustablemaxValidators(default 20, hard limit 64) and makes per-subnet validator sets fully dynamic: attestations carry 1..maxValidatorsvalidators and the stored set, thegetValidatorsABI, and all ofAlphaVault's consuming plumbing are sized by the actual attested count instead of fixedbytes32[3]/uint16[3]slots with a trailing-zero sentinel.The EIP-712
WeightAttestationpayload and typehash are unchanged (they were already dynamic), so attester tooling keeps signing the same struct.Changes
ValidatorRegistry
maxValidators(public, default 20) withsetMaxValidatorsgated onDEFAULT_ADMIN_ROLE, zero rejected, hard-capped byMAX_VALIDATORS_LIMIT = 64(mirrors theMAX_SIGNERScareless-admin gas bound); lowering the cap does not shrink already-committed setsValidatorSetstores dynamic arrays;_commitis a plain delete+push (drops the old diff-write; readability over the write optimization, per the locked design in the cap research doc)getValidatorsreturns exact-sizebytes32[]/uint16[]; empty means "not configured"AlphaVault
[3]/[6]plumbing generalized:_lastSeenHotkeysisbytes32[], rebalance targets/loops are count-sized,_totalStakemerges current + last-seen sets of any sizes with_contains-based dedup_resolveValidatorsderives the count from array length and additionally enforceshotkeys.length == weights.length— the structural guarantee the fixed-size ABI used to give for freegetBestValidators/lastSeenHotkeysABI changed to dynamic arraysunwrapForTaoextracted to_sellAlphaForTao(also resolves a stack-too-deep under coverage's--ir-minimum)Tooling
scripts/get_vault_state.pyderives the validator count from array length and pre-fills 20 CSV columnsGas
Count-3 hot paths pay for the dynamic arrays and the simpler commit/snapshot writes:
The cap is a bound, not a target — worst-case cost scales with the attested count (up to 19
moveStakeper rebalance; up to 2N merged entries onunwrapForTaoafter a full rotation). Real-precompile cost at count 20 is only exercised by mocks here; worth a localnet e2e pass before attesting large sets.🤖 Generated with Claude Code