Skip to content

fix(blockchain-api): allow an off-curve owner when deriving associated token addresses - #1232

Merged
madninja merged 2 commits into
developfrom
fix/offcurve-ata-and-squads-packing
Aug 3, 2026
Merged

fix(blockchain-api): allow an off-curve owner when deriving associated token addresses#1232
madninja merged 2 commits into
developfrom
fix/offcurve-ata-and-squads-packing

Conversation

@madninja

@madninja madninja commented Aug 3, 2026

Copy link
Copy Markdown
Member

Problem

A Squads vault owns its assets through a program address, which is off the ed25519 curve. Eleven call sites derived associated token addresses without allowOwnerOffCurve, so getAssociatedTokenAddressSync threw TokenOwnerOffCurveError and took down the whole request rather than one derivation. Reproduced against a mainnet vault-held position by invoking the real helper:

RESULT: THREW -> TokenOwnerOffCurveError:

buildClaimInstructions is one of those call sites, and claim, undelegate and delegate all call it first, so all three were unavailable to any vault-held veHNT position with an unclaimed epoch. delegate.ts derived the reward account the same way when enabling autoclaim, so a vault could not turn automation back on either.

The derived address is identical either way and the programs re-validate the account they receive (claim_rewards_v1 constrains delegator_ata by associated_token::authority and position_token_account by token::mint plus a positive balance), so this decides whether a multisig can use an endpoint at all, not who is allowed to do what.

Scope

This started larger and was cut back deliberately. It also changed transaction packing so a vault's batches would survive being re-wrapped as a Squads proposal, and skipped the Jito tip for a payer that cannot sign. Both are dropped, because the benefit turned out to be blocked downstream:

  • getJitoTipTransaction builds a SystemProgram.transfer from the fee payer, so a vault can never sign a tip.
  • The submit path picks bundle-vs-single from transaction count alone and throws JitoMissingTipError when no tip is present.

So a multi-transaction batch for a vault fails at submission with or without the packing change, and a smaller size budget buys nothing until that is fixed. Everything else in this PR is a plain correctness fix, so it ships on its own.

Known follow-ups, not in this PR

  • The invariant is unenforced. Nothing stops a new call site omitting the flag. The right fix is an ata(mint, owner) wrapper that always passes it, plus an ESLint restriction on importing getAssociatedTokenAddressSync directly, which fixes the class rather than scanning for instances. That means migrating all 30 existing call sites, which does not belong in a fix this size.
  • positions/create.ts and positions/split.ts attach an ephemeral mint keypair. A wrapping wallet keeps the message and drops that signature, so a vault gets a proposal whose execution can only fail. These endpoints should reject an off-curve fee payer with a typed error.
  • Fee gates overstate the requirement for a vault, since jitoTipCost is added whenever there is more than one transaction on mainnet.
  • Squads propose mode does not cover governance. squadsProposeFields is spread into the token, data-credit and hotspot schemas only; no governance procedure reads input.multisig or calls buildActionProposal. buildActionProposal is also single-proposal, so a delegation recovery spanning many transactions would need a multi-proposal variant.
  • pnpm test:unit is referenced by no workflow, so the four existing unit tests in this package never run in CI.

@bryzettler bryzettler left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review of the off-curve/Squads changes. Line comments below cover what's anchorable in the diff; two issues live in files this PR doesn't touch, so they go here:

1. splitPosition bypasses the new ephemeral-signer guard (governance/procedures/positions/split.ts ~L160)

split.ts builds directly via buildVersionedTransaction({ ..., signers: [newMintKeypair] }) with no canSign check and no packGroups, so an off-curve (Squads-vault) wallet gets a mint-keypair-signed transaction back. Re-wrapping it into vault_transaction_create (ephemeralSigners: 0) keeps the message but drops the mint signature — exactly the failure mode the guard in pack-groups.ts refuses: members approve, execution fails on-chain after quorum and rent are spent. The invariant ("refuse rather than hand back a proposal whose execution can only fail") currently only covers the batched path; split should hit the same refusal.

2. Fee gates still charge jitoTipCost for payers that no longer get a tip

The estimated-fee/INSUFFICIENT_FUNDS gates add jitoTipCost whenever versionedTransactions.length > 1 on mainnet, but with this PR an off-curve payer never receives a tip transaction. A vault whose balance sits between the true requirement and requirement + tip gets a spurious INSUFFICIENT_FUNDS, and every off-curve caller sees an overstated estimate. The condition should mirror the tip decision (canSign(feePayer) && shouldUseJitoBundle(...)). Affected: delegation/claim-rewards.ts (~L126), delegation/delegate.ts (~L272/L622), delegation/undelegate.ts, positions/create.ts (~L373), proxy/assign.ts (~L360), relinquish-position-votes.ts.

Comment thread packages/blockchain-api/tests/unit/squads-wrap-budget.test.ts Outdated
Comment thread packages/blockchain-api/tests/unit/ata-off-curve-owner.test.ts Outdated
…d token addresses

A Squads vault owns its assets through a program address, which is off the
ed25519 curve. Eleven call sites derived associated token addresses without
`allowOwnerOffCurve`, so `getAssociatedTokenAddressSync` threw
`TokenOwnerOffCurveError` and failed the whole request rather than one
derivation.

`buildClaimInstructions` is one of them, and the claim, undelegate and delegate
procedures all call it first, so claiming delegation rewards, undelegating and
re-delegating an expired position were unavailable to every vault-held veHNT
position with an unclaimed epoch. `delegate.ts` derived the reward account the
same way when enabling autoclaim, so a vault could not turn automation back on
either.

The derived address is identical either way and the programs re-validate the
account they receive against their own `associated_token::authority` and
`token::authority` constraints, so this decides whether a multisig can use an
endpoint at all, not who is allowed to do what.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@madninja
madninja force-pushed the fix/offcurve-ata-and-squads-packing branch from b60fc17 to 0a305f2 Compare August 3, 2026 16:41
@madninja madninja changed the title fix(blockchain-api): let a multisig-held wallet use the governance endpoints fix(blockchain-api): allow an off-curve owner when deriving associated token addresses Aug 3, 2026
@madninja

madninja commented Aug 3, 2026

Copy link
Copy Markdown
Member Author

Cut this back to the associated-token fix only; the packing and Jito changes are gone (git diff origin/develop is now 8 files, +29/-7).

The packing half was dropped because its benefit is blocked downstream: getJitoTipTransaction builds the transfer from the fee payer, so a vault can never sign a tip, and the submit path picks bundle-vs-single from transaction count alone and throws JitoMissingTipError with no tip present. A multi-transaction batch for a vault therefore fails at submission with or without a corrected size budget.

That resolves the review comments on pack-groups.ts (order-dependent oversize check, untyped refusal, untested signer accumulation), effectiveMaxTxs, the Jito submit-path contract, and the wrap-fixture drift, by removing the code they applied to.

Two comments survive as follow-ups recorded in the description: the ata() wrapper plus an ESLint ban is the right shape for enforcing this (agreed, and it needs all 30 call sites, which is too much for a fix this size), and split.ts/create.ts should reject an off-curve fee payer with a typed error since they attach an ephemeral mint keypair. The fee-gate jitoTipCost point no longer applies here since the tip behaviour is unchanged.

…at cannot sign

Both build a transaction signed by an ephemeral mint keypair. A wallet that
cannot sign is served by re-wrapping the message into a multisig proposal, which
keeps the message and discards that signature, so the proposal can only fail on
execution once quorum and rent are spent.

`createPosition` was previously stopped by accident: its unconditional deposit-balance
check derived an associated token address that threw for an off-curve owner, so the
request failed before returning anything. Allowing an off-curve owner there removes
that accidental stop, which makes the unexecutable path reachable, so the refusal is
now explicit and typed. `splitPosition` never had the accidental stop and was
reachable already.

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

madninja commented Aug 3, 2026

Copy link
Copy Markdown
Member Author

Status of each review comment, after cutting the packing half and adding the typed refusal.

Addressed

  • positions/split.ts / positions/create.ts ephemeral mint keypair — both now refuse a fee payer that cannot sign with a typed BAD_REQUEST. Worth flagging that you were more right than the comment stated: create.ts's unconditional deposit-balance check derived an ATA that threw for an off-curve owner, so createPosition was stopped by accident before this PR. Fixing that derivation removes the accidental stop and makes the unexecutable path reachable, so this was a defect the reduced diff created rather than a pre-existing one. split.ts had no such accidental stop and was reachable already.
  • pack-groups.ts:54 untyped refusal causing a 500 plus a Sentry capture — the refusal is now a typed error at the procedure boundary instead.

Resolved by removing the code they applied to (verified: pack-groups.ts and both test files are gone, and build-batched-transactions.ts / constants.ts are byte-identical to develop)

  • pack-groups.ts:130 order-dependent oversize check. Note this structure still exists in the inline loop on develop; your comment said the tighter off-curve budget made it newly reachable, and that budget is gone, so it reverts to latent rather than fixed.
  • build-batched-transactions.ts:156 effectiveMaxTxs reserving a tip slot.
  • build-batched-transactions.ts:181 the submit-path contract living only in a comment.
  • squads-wrap-budget.test.ts:80 validating against a private copy of the wrap instead of wrapAsVaultProposal.
  • Fee gates charging jitoTipCost — the tip behaviour is unchanged now, so the gates match it again.

Not addressed, recorded in the description

  • ata-off-curve-owner.test.ts:83 — you are right that scanning for instances is the wrong altitude, and I deleted that test rather than keep a guard that passes on allowOwnerOffCurve: false. I attempted the ata() wrapper plus an ESLint ban and backed out: it needs all 30 call sites migrated with import surgery, which is too much churn for a fix this size and too easy to get subtly wrong mechanically. So the invariant is currently unenforced, and that is stated in the description rather than papered over.

Coverage honesty: the only new test is on canSign itself. Mutating the predicate turns the suite red; mutating the guard away at the create.ts call site does not, because nothing reaches those handlers without env and a connection. The call sites rest on review, not tests.

@madninja
madninja merged commit adb63c2 into develop Aug 3, 2026
67 checks passed
@madninja
madninja deleted the fix/offcurve-ata-and-squads-packing branch August 3, 2026 19:32
@github-actions github-actions Bot mentioned this pull request Aug 3, 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