feat(evm-module): Profile.recreateProfile — testnet recovery for profiles orphaned by ProfileStorage redeploy#574
Open
zsculac wants to merge 8 commits into
Open
feat(evm-module): Profile.recreateProfile — testnet recovery for profiles orphaned by ProfileStorage redeploy#574zsculac wants to merge 8 commits into
zsculac wants to merge 8 commits into
Conversation
Re-attach a Profile to an existing Identity that survived a ProfileStorage redeploy. Admin-only and reuses the supplied identityId so id-keyed staking/conviction/sharding state stays addressable. Adds ProfileLib.ProfileAlreadyExists and bumps Profile _VERSION 1.2.0 -> 1.3.0. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Admin/operational/unrelated caller gating, ProfileAlreadyExists, empty/duplicate nodeName and nodeId, operator-fee boundary, whitelist gating, and a createProfile regression. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…tion Pre-seeds StakingStorage.setNodeStake and ConvictionStakingStorage.setOperatorFeeBalance under a bricked identityId, deletes the profile, and asserts recreateProfile restores the profile while the id-keyed state stays addressable and unchanged (no new identity minted). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
…llet Take address operationalWallet instead of uint72 identityId. The contract resolves identityId via IdentityStorage.getIdentityId and enforces _checkAdmin(identityId) in-body. Admin-only authorization is unchanged (ADR 0001) — the operational wallet is only an identifier; the admin key is still required. Operators no longer need to know the internal numeric identityId (often unknown) to recover a bricked node. A zero/unknown wallet resolves to id 0 (no admin) and reverts. Migrates all unit + integration call sites; adds no-identity and cross-identity admin safety tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…eateProfile Addresses CI review feedback on PR #574: - #3 (sharding): recreateProfile now reverts NodeIdShardingMismatch if the node is still in ShardingTableStorage (survives a ProfileStorage-only redeploy) and the supplied nodeId differs from the surviving ring entry. Read-only against sharding (no ring rewrite). Wires ShardingTableStorage into Profile + deploy deps; adds ProfileLib.NodeIdShardingMismatch. - #1 (ABI): regenerate abi/Profile.json so downstream consumers see recreateProfile, ProfileAlreadyExists and NodeIdShardingMismatch. - #2 (fee history): documented as an accepted known testnet limitation in ADR 0001 (data unrecoverable on-chain; mainnet would use a migration). Profile unit+integration: 48 passing / 0 failing (1 pre-existing skip). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI review (#574, comment B): the test hardcoded ProfileStorage storage slot 2 for isNameTaken via hardhat_setStorageAt — brittle to layout changes. Root cause: isNameTaken is never written by any contract path, so the name-uniqueness guard (copied verbatim from createProfile) is unreachable in production; the test only passed by faking the slot. Removed it and left a NOTE explaining why. The dead isNameTaken mapping is tracked as separate pre-existing cleanup (out of scope here). Profile unit+integration: 47 passing / 0 failing (1 pre-existing skip). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses CI review (#574, round 3): - #2 (Ask staleness): recreateProfile now calls askContract.recalculateActiveSet() when the recovered node is already in ShardingTableStorage (nodeExists). A ProfileStorage-only redeploy leaves Ask's active-set/pricing aggregates stale for a still-sharded, stake-bearing node; genesis createProfile has no ring entry so never needs it. Mirrors updateAsk's existing recalc pattern. No ABI change (internal call). - #4 (test fidelity): the id-keyed-state integration test no longer seeds/ asserts V8 StakingStorage.getNodeStake (unmaintained post-migration). It asserts the canonical V10 path — ConvictionStakingStorage getNodeStakeV10 invariance across recreate + a non-zero operator-fee balance. Added Ask-recompute integration tests (in-ring + not-in-ring). Not actioned: #1 fee-history (standing accepted known limitation, ADR 0001); #3-style dead isNameTaken guard (separate pre-existing cleanup task). Profile unit+integration: 49 passing / 0 failing (1 pre-existing skip). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
After a testnet ProfileStorage redeploy (IdentityStorage kept), every existing node has an on-chain Identity but no Profile — silently bricked. No on-chain path attaches a Profile to an existing
identityId. TS-only fix impossible.Root cause
identityIdis the cross-contract foreign key (IdentityStorage ↔ ProfileStorage ↔ Staking ↔ ConvictionStaking ↔ ShardingTable).lastIdentityIdlives in IdentityStorage, not ProfileStorage — a ProfileStorage-only redeploy wipesprofiles[]but leaves identity + id counter + sharding/staking state intact.Solution (testnet, Option A)
Profile.recreateProfile(address operationalWallet, string nodeName, bytes nodeId, uint16 initialOperatorFee):identityId = IdentityStorage.getIdentityId(operationalWallet), then_checkAdmin(identityId)(admin-only perdocs/adr/0001; zero/unknown wallet → id 0 → reverts, also proves identity exists).identityId; never callsIdentity.createIdentity(would orphan id-keyed staking/conviction/sharding state).ShardingTableStorage(nodeExists), suppliednodeIdmust equal the surviving ring entry, else revertsNodeIdShardingMismatch(read-only — no ring writes).askContract.recalculateActiveSet()so the recovered node's stale pre-redeploy active-set/pricing contribution is recomputed (mirrorsupdateAsk; genesiscreateProfilehas no ring entry so never needs it).ProfileAlreadyExists,NodeIdShardingMismatch; createProfile validations preserved; parity (ask=0, relayCapable=false, reusesProfileCreated)._VERSION1.2.0 → 1.3.0. Logic-redeploy only;ShardingTableStorageadded to deploy deps (read-only).CI Codex review — status
Round 1: 🔴 ABI not regenerated → Fixed. 🔴 Sharding divergence → Fixed (
NodeIdShardingMismatch). 🔴 Operator-fee retroactive → Accepted known limitation (ADR 0001).Round 2 (after close/reopen — the “30,979-line / over cap” skip was a stale GitHub merge-ref vs. a 119-commit-ahead
main, not the real ~700-line diff): 🔴 fee re-flag → standing decision. 🟡 brittleisNameTakenslot test → Removed (asserted unreachable behavior;isNameTakenis written by no contract path — dead mapping tracked as separate cleanup).Round 3: 🔴 fee re-flag (3rd) → standing decision; “carry forward last fee” is impossible (schedule unrecoverable on-chain). 🔴 Ask active-set stale after recovery → Fixed (guarded
recalculateActiveSet). 🟡 deadisNameTakenguard in recreate → deferred to the separate holistic cleanup (removing only here would diverge fromcreateProfile). 🟡 integration test seeded V8StakingStoragenot V10 → Fixed (now asserts canonicalConvictionStakingStorage.getNodeStakeV10invariance + non-zero operator-fee balance; V8 seed/asserts dropped).Verification (this branch)
hardhat compileclean.packages/evm-module(turbo lintskips it) — stated, not claimed.Profile.recreateProfile).Operator preconditions (not code — verify before testnet recovery)
WhitelistStorage.whitelistingEnabled()is true, whitelist the bricked identities' admin wallets first (default disabled).updateAskto set a real ask post-recovery (recreate seeds ask=0; the recompute only clears the stale ring contribution).Out of scope
ProfileStorage batch-import/snapshot migration (mainnet Option C); IdentityStorage/staking/sharding-table writes; mainnet migration tooling; node-name → admin anti-squatting; the dead
isNameTakenmapping cleanup (tracked separately).🤖 Generated with Claude Code