feat: migrate governance positions + getPositions, harden fee-payer rent accounting - #1206
Conversation
…nt leak, replace allowlist with rate limiting
madninja
left a comment
There was a problem hiding this comment.
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.
- 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
…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
…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
Summary
Extends
/migrateto migrate VSR governance positions (vehnt/veiot/vemobile) alongside tokens and hotspots, addsgovernance.getPositionsfor the wallet UI, and hardens fee-payer rent accounting. Positions are discovered server-side (registrar-agnostic, no new input) and transferred withtransferPositionV0. Migration stays rent-neutral: the fee payer fronts destination-ATA rent and recovers it by closing the source ATA.Changes
governance.getPositions—GET /governance/positions/wallet/{wallet}returns mint, position, registrar,amountDeposited(TokenAmountOutputshape),numActiveVotes, lockup.TransferPositionV0→AccountNotInitialized (3012)when a lagged RPC index reports a closed ATA.[createAta, transfer, close]) and position ([transferPositionV0, close]) are built as groupedTransactionInstruction[][]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. getPositionsamountDepositedassertions).Security review — known & accepted (both pre-existing,
/migrateis public so client trust isn't a safeguard)