feat(hypercore): forward the delivery venue on token requests (RHI-5510) - #729
Merged
Conversation
`adaptTransaction` rebuilds each tokenRequest from named fields, so any field it does not list is silently dropped. `balance` — which selects between a HyperCore recipient's spot wallet and the default perp dex's margin account — was never listed and was not declarable on the input types either. The consequence was that no SDK consumer could deliver USDC to HyperCore spot, even deliberately: the field never reached the wire, so every caller took the orchestrator's 'perp' default. Verified on dev 2026-08-05, where two intents asking for spot completed, filled, and credited perp margin with nothing reporting a problem. The orchestrator now requires the venue on HyperCore destinations and rejects it everywhere else, so this is what makes a HyperCore intent expressible at all from the SDK. Read via `'balance' in request` rather than a property access because the same mapping serves non-EVM requests, whose types carry no `balance`: Solana and Tron have no venue concept and the orchestrator rejects the field for them. HyperCore is EVM-addressed, so only the EVM arm can set it. Omitted entirely rather than sent as `undefined` when unset, so ordinary intents don't trip that rejection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013LgDwSpK4GJW2Jka5PeJBD
![rhinestone-kevin[bot]](https://avatars.githubusercontent.com/in/3714871?s=60&v=4){kind=small}
| // non-EVM requests (Solana/Tron), whose types carry no `balance` because | ||
| // those chains have no venue and the orchestrator rejects the field | ||
| // there. HyperCore is EVM-addressed, so only the EVM arm can set it. | ||
| ...('balance' in request && request.balance !== undefined |
There was a problem hiding this comment.
blocker — This only adds balance to the facade-level IntentInput; buildIntentRequest still reconstructs tokenRequests as { tokenAddress, amount? } and the orchestrator request type has no balance, so the value is dropped before the network call. HyperCore spot deliveries will still go to the orchestrator's default venue.
…5510)
Kevin caught that the previous commit only got `balance` as far as the
facade-level `IntentInput`. `buildIntentRequest` (`transactions/intents/
request.ts`) rebuilds `tokenRequests` as `{tokenAddress, amount?}` a second
time, and that is the last hop before the network call — so the field was
dropped again and HyperCore spot deliveries still went to the default venue.
The fix was incomplete in exactly the way the bug it fixes is: a closed
constructor silently discarding a field.
Two reasons it slipped:
* the token-request shape is declared in **three** places — `IntentTokenRequest`
(intents/types.ts), `OrchestratorIntentRequest.tokenRequests`
(clients/orchestrator/types.ts) and the published `IntentInput.tokenRequests`
projection (clients/orchestrator/public.ts) — with nothing tying them
together, so a new field has to be added three times. All three now carry it.
* `tsc` could not catch it: the value was added via a conditional spread
(`...(cond ? {balance} : {})`), which is exempt from excess-property checking,
so the field was accepted by a type that did not declare it and then vanished.
The new test asserts at `buildIntentRequest` — the boundary that actually
reaches the orchestrator — rather than at `adaptTransaction`, which is why the
first round's test passed while the field was still being dropped.
Also replaces the `'balance' in request` narrowing with `balance?: never` on the
non-EVM token-request arms. Solana and Tron have no venue and the orchestrator
rejects the field for them, so declaring it unsettable states that invariant in
the type; the `in` form widened the value to `{}` and only surfaced once
`IntentTokenRequest` declared a real type for it. `mapIntentRequestToWire`
forwards `tokenRequests` wholesale, so no change was needed there.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013LgDwSpK4GJW2Jka5PeJBD
| interface NonEvmTokenRequestWithAmount { | ||
| address: NonEvmAddress | ||
| amount: bigint | ||
| balance?: never |
There was a problem hiding this comment.
blocker — hyperCoreMainnet is exported as a NonEvmChain, so a public Transaction with targetChain: hyperCoreMainnet uses CrossChainNonEvmTransaction.tokenRequests; this added balance?: never still makes the HyperCore venue a type error for SDK consumers. CLAUDE.md treats src/config as the public config surface, and the orchestrator will require the venue, so callers using the supported HyperCore descriptor still cannot compile the request.
…HI-5510) Kevin again, and this one was a regression I introduced rather than an omission: the previous commit's `balance?: never` on the non-EVM token-request arms made the delivery venue a **compile error** on the only descriptor that can target HyperCore. `hyperCoreMainnet` is a `NonEvmChain` (`kind: 'hypercore'`), so `targetChain: hyperCoreMainnet` — the supported, documented way to reach HyperCore — resolves to `CrossChainNonEvmTransaction.tokenRequests`, i.e. the non-EVM arm. My reasoning was that HyperCore is EVM-*addressed* so only the EVM arm could carry a venue. That conflated two different splits: the SDK divides EVM from non-EVM by **CAIP-2 namespace** (`hypercore:` is not `eip155:`), not by address shape. Same conflation the orchestrator has in `vmType`, which is worth knowing about because it will keep biting. Both non-EVM arms now declare `balance?: HyperCoreBalance`. Solana and Tron share that arm, so type-level per-chain exclusivity isn't expressible without splitting the descriptor; the orchestrator rejects the field on their destinations at runtime, which is where that rule already lives. Adds the guard to `test/types/public-boundary.ts`, asserting a `targetChain: hyperCoreMainnet` transaction with `balance: 'spot'` / `'perp'` satisfies the public `Transaction` type. It belongs there rather than in the unit suite because the failure mode is a compile error — every runtime test in the previous two rounds passed while consumers could not have compiled the call at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013LgDwSpK4GJW2Jka5PeJBD
| Sponsorship, | ||
| TokenRequest, | ||
| TokenRequests, | ||
| HyperCoreBalance, |
There was a problem hiding this comment.
design-smell — CLAUDE.md says the public API is only the explicit exports from src/index.ts plus declared subpaths in src/package.json; adding HyperCoreBalance only to this local export list does not publish it. Consumers still cannot import the new named venue type from @rhinestone/sdk, so the intended public surface is missing.
…510)
`HyperCoreBalance` was added to the local export list in `src/config/account.ts`,
but per CLAUDE.md the public API is the explicit exports from `src/index.ts` plus
the declared subpaths — so consumers could not
`import type { HyperCoreBalance } from '@rhinestone/sdk'`. A named type nobody
can import is a half-published surface, and typing their own helper around the
venue is the reason it is named rather than inlined.
Re-exported from `src/index.ts` alongside `TokenRequest` / `NonEvmTokenRequest`,
and the type test now imports it from the package root so the export cannot
silently regress.
Kevin flagged this as a design-smell rather than a blocker; the gate was already
green on the previous commit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013LgDwSpK4GJW2Jka5PeJBD
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.
Part of RHI-5510.
adaptTransactionrebuilds each token request from named fields, so any field it doesn't list is silently dropped.balance— which selects between a HyperCore recipient's spot wallet and the default perp dex's margin account — was never listed, and wasn't declarable on the input types either.So no SDK consumer could deliver USDC to HyperCore spot, even deliberately: the field never reached the wire and every caller took the orchestrator's
'perp'default. Verified on dev 2026-08-05, where two intents asking for spot completed, filled, and credited perp margin with nothing reporting a problem.What changed
balance?: 'spot' | 'perp'declared onTokenRequestWithAmount/TokenRequestWithoutAmountand exported asHyperCoreBalance.adaptTransaction.'balance' in requestrather than a property access, because the same mapping serves non-EVM requests whose types carry nobalance: Solana and Tron have no venue concept and the orchestrator rejects the field for them. HyperCore is EVM-addressed, so only the EVM arm can set it.undefinedwhen unset, so ordinary intents don't trip that rejection. One of the two new tests asserts exactly that (not.toHaveProperty('balance')) — a dropped field is what caused this bug, so both directions are pinned.Changeset included as a
minor.Companion / ordering
Pairs with rhinestonewtf/orchestrator#1825, which makes the venue required on HyperCore destinations. This should publish first: until it does, SDK callers cannot express the field, so a HyperCore intent would 400. Blast radius is only internal tooling (
bundle-generator);relayerandrebalancing-servicecontain no HyperCore code, and no external client has ever sent a HyperCore intent.Verification
Both
tsc --noEmitconfigs clean, biome clean,src/api46 tests passing, 550 unit tests passing.One pre-existing failure in
src/actions/recovery.test.ts("installs the social recovery validator for a single guardian") is not from this change — confirmed by stashing and reproducing it on cleanorigin/main.🤖 Generated with Claude Code
https://claude.ai/code/session_013LgDwSpK4GJW2Jka5PeJBD