Skip to content

feat: migrate governance positions + getPositions, harden fee-payer rent accounting - #1206

Merged
bryzettler merged 23 commits into
developfrom
feat/migrate-governance-positions
Jul 24, 2026
Merged

feat: migrate governance positions + getPositions, harden fee-payer rent accounting#1206
bryzettler merged 23 commits into
developfrom
feat/migrate-governance-positions

Conversation

@bryzettler

Copy link
Copy Markdown
Contributor

Summary

Extends /migrate to migrate VSR governance positions (vehnt/veiot/vemobile) alongside tokens and hotspots, adds governance.getPositions for the wallet UI, and hardens fee-payer rent accounting. Positions are discovered server-side (registrar-agnostic, no new input) and transferred with transferPositionV0. Migration stays rent-neutral: the fee payer fronts destination-ATA rent and recovers it by closing the source ATA.

Changes

  • feat: migrate governance positions — enumerate every VSR position the source owns (freeze-authority classification) and transfer each to the destination; re-enumerated per paginated call, so migrated positions drop out naturally.
  • feat: governance.getPositionsGET /governance/positions/wallet/{wallet} returns mint, position, registrar, amountDeposited (TokenAmountOutput shape), numActiveVotes, lockup.
  • fix: skip stale-enumerated positions — verify the source ATA still holds the NFT before building each transfer; avoids TransferPositionV0AccountNotInitialized (3012) when a lagged RPC index reports a closed ATA.
  • fix: group create/close instructions — token ([createAta, transfer, close]) and position ([transferPositionV0, close]) are built as grouped TransactionInstruction[][] so each pair stays in one tx. Closes a rent-drain where a flat list could split a fee-payer-funded create from its offsetting close, letting a caller land the create alone and strand the rent. Enforced at build layer, independent of client submission. Mirrors the split-hotspot pattern.

Testing

  • migration.test.ts: 10/10 (incl. VSR position migration).
  • governance.test.ts: 33/33 (incl. getPositions amountDeposited assertions).

Security review — known & accepted (both pre-existing, /migrate is public so client trust isn't a safeguard)

  1. Mini-fanout subsidy (~0.01 SOL) — intended cost of recreating the reward-split config at the destination; bounded by existing rate limiting. No fix that doesn't break legitimate fanout migrations.
  2. Isolated Jito tip (~10k lamports) — after the grouping fix, the only remaining strandable cost, in the same bucket as the irreducible base tx fee. Rate limiting is the right control; a full server-side atomic-submission architecture would only shave the tip, not base fees. Revisit if a new non-self-neutralizing fee-payer-funded instruction is added.

@bryzettler
bryzettler requested review from Copilot, jthiller and madninja and removed request for Copilot July 13, 2026 20:04

@madninja madninja left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the substantive changes (the grouping/atomicity fix, VSR position migration, getPositions, and the rate-limiter swap). Core mechanics are correct: I verified the batcher pushes each group all-or-nothing (slice(0, prevLen) rollback), and transferPositionV0 creates the dest ATA on the fee payer + leaves PositionV0/numActiveVotes untouched, so the paired close keeps it rent-neutral and mid-vote positions migrate fine.

Leaving inline notes, ranked. #1 is the only one that can hard-fail a real user with no workaround; the rest are cleanup/perf/hardening. The accepted-risk section is fair on the on-chain economics — the main caveat is that the rate limiter isn't an abuse boundary (keys are caller-controlled) and the unbounded surface is server RPC cost, not fee-payer drain.

Comment thread packages/blockchain-api/src/server/api/routers/migration/procedures/migrate.ts Outdated
Comment thread packages/blockchain-api/src/server/api/routers/migration/procedures/migrate.ts Outdated
- Cap first batch (tokens + positions) at the Jito bundle limit; spill
  overflow tokens into nextParams and let positions re-enumerate on the
  next call, so oversized wallets paginate instead of hard-failing
- Key IP rate limiting off the right-most x-forwarded-for hop, throttle
  getPositions, and document the limiter as a courtesy throttle
- Batch getPositionsForOwner RPC lookups with getMultipleAccountsInfo
- Remove dead password field from MigrateInputSchema
- Gate on client amount only for native SOL; SPL amounts are advisory
- Fix list-positions comment to describe the actual classification
- Require a position mint's freeze authority to equal positionKey(mint)
  and that PDA to be an initialized VSR account, so a permissionlessly
  dusted fake position NFT can no longer brick a wallet's migration
- Reject source==destination and off-curve destination wallets
- Dedupe duplicate mints in tokens[]; skip unsupported (e.g. token-2022)
  mints with a warning instead of 500ing
- Warn when migrated positions carry live proxy assignments or
  delegations, which silently survive the transfer
@bryzettler
bryzettler requested a review from madninja July 22, 2026 14:45
…tion

- decode positions inside getPositionsForOwner so callers stop re-fetching
  the same accounts; drop redundant positionV0.fetchMultiple in both callers
- batch the per-position source-ATA re-check in migrate via one chunked
  getMultipleAccountsInfo instead of N serial getAccount calls
- extract shared fetchRegistrarsByKey helper for the duplicated registrar
  dedupe/fetch/Map block in getPositions and migrate
- register GET_POSITIONS_RATE_LIMIT_PER_IP in env.ts alongside migration vars
- narrow RateLimiterOptions.max to a thunk; all call sites already pass one
- use chunks from @helium/spl-utils in getMultipleAccountsChunked
Each of the 17 e2e matrix jobs ran a full build-anchor. When the anchor
build cache was evicted (repo sits at the 10GB Actions cache cap), every
job did a cold anchor build that can't finish inside the 20-minute job
timeout, so entire runs died as 'cancelled'. The matrix jobs only need
target/types to build @helium/idls; build once in a dedicated job with a
60-minute budget and hand the generated types/IDLs to the matrix via an
artifact.
…hange

Build the changed programs' IDLs at the PR base and head with
anchor idl build and require an @helium/idls changeset only when the
generated IDL actually differs (ignoring metadata.version). Internal
logic changes like the SIMD-553 CU optimizations no longer demand a
no-op package bump. Verified locally: all four programs this branch
touches produce byte-identical IDLs vs develop.
…tion

The websocket path can reject with the raw confirmation result
({ err: { InstructionError } }) rather than the InstructionError
itself, which #1213's fix didn't cover.
Resolve tests.yaml conflict: keep the IDL-diff-gated idls changeset
check from this branch (supersedes develop's any-source-change gate)
and carry over develop's tests/hpl-crons.ts e2e matrix entry.
On the surfpool mainnet fork, closed source position ATAs resurrect
from remote state on subsequent reads, so migrate rebuilt
transferPositionV0 for already-migrated positions and failed on the
frozen destination ATA. Guard server-side by checking the destination
ATA in the same batched fetch, and give the e2e tests that submit
payer migrations their own destination keypairs instead of sharing
one.
- Require source-wallet signature on the dedicated Jito tip tx so it
  can't be submitted standalone to drain the fee payer
- Validate migrate tokens input (BAD_REQUEST instead of 500); hotspots
  accept Helium entity keys via HeliumPublicKeySchema
- Guard out-of-range votingMintConfigIdx in getPositions
- Harden rate limiter LRU eviction (touch on access)
- Replace hand-rolled hotspot batching with fitGroupsWithinLimit;
  binary-search the overflow fit; add addDrafts/tryUnpack helpers
- Inline chunked getMultipleAccountsInfo, drop the shared helper
- Remove unused rate-limit env vars from env.ts
- Dedupe e2e test helpers into tests/e2e/helpers
- CI: detect program Cargo.toml changes in idls changeset gate
@bryzettler bryzettler removed the hold label Jul 22, 2026
…nance-positions

# Conflicts:
#	.github/workflows/tests.yaml
#	packages/blockchain-api-client/src/schemas/governance.ts
…nance-positions

# Conflicts:
#	packages/blockchain-api-client/src/contracts/governance.ts
#	packages/blockchain-api-client/src/schemas/governance.ts
#	packages/blockchain-api/tests/e2e/governance.test.ts
@bryzettler
bryzettler merged commit 77df26b into develop Jul 24, 2026
110 of 111 checks passed
@bryzettler
bryzettler deleted the feat/migrate-governance-positions branch July 24, 2026 20:53
@github-actions github-actions Bot mentioned this pull request Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants