feat(trade): make builder fee env-driven and off by default

[vipineth]

Summary

Removes the builder fee from the official deployment and makes it configurable via env instead of a hardcoded constant. No fee is charged unless a deployment opts in.

Previously apps/terminal/src/config/hyperliquid.ts hardcoded the builder address and a 0.01% fee, which was injected into every order action.

Env vars

Var	Example	Notes
VITE_BUILDER_ADDRESS	0x744e…A7E	0x + 40 hex
VITE_BUILDER_FEE_BPS	1	basis points, 1 = 0.01%, one decimal place max, max 100

Basis points are converted to the API's tenth-of-a-bp f unit internally, so the Hyperliquid quirk stays out of the config surface. One decimal place is allowed (0.5 bps) to keep the API's 0.1 bp granularity.

Both set and valid → fee on. Both unset → off. Half-configured or malformed → off, with a DEV-only console.warn. Parsing follows the existing config/env.ts pattern (plain function, default-param env injection) rather than introducing zod for two values.

DEFAULT_BUILDER_CONFIG keeps its name and BuilderConfig type, so no consumer changes were forced.

Fee-disabled path

• Orders carry no builder field — the SDK strips undefined keys before msgpack signing
• Registration drops from two signatures to one (approveAgent only)
• The maxBuilderFee info query never fires

Bug fixes included

• Stray 0 render — the three order summaries guarded on {DEFAULT_BUILDER_CONFIG?.f && …}. A fee of 0 (attribution-only, a legal config) made React render a literal 0. Now !!-guarded.
• Needless maxBuilderFee request — use-agent-status.ts called builderFeeQuery.refetch() unconditionally. TanStack Query v5 refetch() ignores enabled: false, so a request fired with zeroAddress on every registration even with no builder configured. Now conditional.

Verification

• pnpm check — 435 files clean
• pnpm typecheck — all packages, zero errors
• pnpm test — 333 passed, 1 skipped (includes 17 new builder-config.test.ts cases)
• pnpm build:all — both apps build
• Grepped .output/ and dist/ for the old builder address → 0 matches

Deploy notes

• Confirm no VITE_BUILDER_* vars exist in Vercel Production or Preview
• These are inlined at build time, so flipping the fee needs a redeploy — and env-only changes produce no commit, so it must be triggered manually
• Existing on-chain builder-fee approvals from users stay put but charge nothing, since orders no longer name a builder. Worth a release-note line.
• Website FAQ copy is static, not env-driven — turning the fee back on means editing apps/website/src/components/Faq.astro too

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
hypeterminal-main	Error		Aug 14, 2026 9:30am

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
hypeterminal-website	Ignored	Preview	Aug 14, 2026 9:30am

[claude]

Code Review

Overview

This PR removes the hardcoded builder fee and replaces it with an env-driven opt-in (VITE_BUILDER_ADDRESS + VITE_BUILDER_FEE_TENTH_BPS). It also fixes two pre-existing bugs: a stray 0 render when builder fee is numeric zero, and an unnecessary maxBuilderFee network request when no builder is configured. Documentation and tests are thorough.

---

What's Good

• Clean parser design — parseBuilderConfig follows the existing config/env.ts pattern, takes an injectable env for testability, and fails fast with a single warnDisabled() helper.
• Validation is correct and tight — address regex (/^0x[0-9a-fA-F]{40}$/), integer-only fee (/^\d+$/), max-cap check, and whitespace trimming all in one pass.
• Bug fixes are correct — the !! guard on DEFAULT_BUILDER_CONFIG?.f properly prevents React rendering a bare 0 for an attribution-only fee. The conditional builderFeeQuery.refetch() correctly avoids a wasted query to maxBuilderFee with zeroAddress.
• Test coverage — 17 cases cover the happy path, both half-configured states, every invalid address/fee shape, and the whitespace edge case.
• Documentation is complete — README, env.example, hl-react README, signing doc, and FAQ all updated consistently.

---

Issues

1. !!DEFAULT_BUILDER_CONFIG?.f hides fee=0 row (minor UX inconsistency)

parseBuilderConfig intentionally accepts f: 0 as valid (attribution-only). But !!0 === false, so the builder fee row is silently hidden for that config. If this is intended — i.e. "don't show the row when there's no fee to display" — it's fine, but it's surprising given the parser accepts 0 as valid. Consider either:

• Rejecting f: 0 in the parser (and adding a test for it), or
• Checking DEFAULT_BUILDER_CONFIG !== undefined in the UI render guard instead of using !!f

2. Return type annotation on parseBuilderConfig should be explicit

// current
export function parseBuilderConfig(env: PublicEnv = import.meta.env): BuilderConfig {

Since BuilderConfig must be { b: ..., f: ... } | undefined for the function body to typecheck (it returns undefined in three paths), the return type annotation should state that explicitly:

export function parseBuilderConfig(env: PublicEnv = import.meta.env): BuilderConfig | undefined {

The PR says typecheck passes, which means BuilderConfig is already a union type — but naming that at the call site makes the contract clear to readers and avoids needing to look up the type definition.

3. DEFAULT_BUILDER_CONFIG export type

Same point — the export is typed as BuilderConfig but assigned from a function that can return undefined. If BuilderConfig wraps undefined, this is fine; if it doesn't, this is a latent typing issue.

---

Nits

• warnDisabled() is named like an action but it's really disabledWithWarning() — the current name reads as if it warns that something was disabled rather than returns disabled state. Minor.
• The note added to docs/hyperliquid-signing.md is written in a > **Note:** blockquote, which is fine, but the existing doc uses plain text — consistency is low-stakes but worth flagging.

---

Summary

Solid PR. The env-driven approach is correct, the validation is tight, and the two bug fixes are well-targeted. Address the f: 0 render ambiguity and make the return type explicit — everything else is minor.

[claude]

Code Review

Overview

This PR converts the hardcoded builder fee into an env-driven opt-in and removes it from the official deployment. It also fixes two pre-existing bugs: a stray 0 render and an unnecessary maxBuilderFee network request. The description is accurate and the scope is clean.

---

What's Good

• Parser design — parseBuilderConfig follows the config/env.ts pattern, takes an injectable env for testability, and short-circuits cleanly with a dedicated warnDisabled() helper.
• Regex coverage — address pattern correctly enforces 0x prefix + exactly 40 hex chars. Fee pattern (/^\d+(\.\d)?$/) correctly rejects two-decimal values, trailing dots, negative numbers, and scientific notation, while accepting one decimal place.
• Bug fix: stray 0 render — !!DEFAULT_BUILDER_CONFIG?.f is the right guard for preventing React from rendering 0 as a node. Correct.
• Bug fix: needless query — conditionally skipping builderFeeQuery.refetch() when no builder is configured is the right fix. TanStack Query v5 ignores enabled: false on refetch(), so the previous code was silently firing with zeroAddress.
• Test coverage — 17 cases are thorough: both halves of a partial config, every invalid address/fee shape, whitespace trimming, and the f: 0 edge case.
• Documentation — README, env.example, hl-react README, signing doc, and FAQ are all updated consistently.

---

Issues

1. UI guard inconsistency with f: 0 (minor)

parseBuilderConfig accepts f: 0 as a valid attribution-only config (confirmed by the "accepts a fee of 0" test), but the render guard !!DEFAULT_BUILDER_CONFIG?.f evaluates !!0 === false, silently hiding the builder fee row for that case. This is a logical inconsistency: the parser allows it, the UI hides it.

The fix is one of:

• Change the render guard to DEFAULT_BUILDER_CONFIG !== undefined — shows a "0%" row for attribution configs, which is more consistent.
• Reject f: 0 in the parser with Number(bps) === 0 → return warnDisabled() — removes the ambiguity entirely. Add a corresponding test that f: 0 is rejected.

Since there's no concrete use-case for attribution-only builders right now, option 2 is simpler.

2. warnDisabled bypasses the injected env for the DEV check

function warnDisabled(): undefined {
  if (import.meta.env.DEV) { // reads global, not injected env
    console.warn(...)
  }
}

In tests, import.meta.env.DEV is likely false, so the warning never fires. This is fine in practice, but it means the warning path has zero test coverage. Not blocking, just worth noting if this is ever tested directly.

3. parseBuilderConfig empty-string case for single var (cosmetic)

The first early-return if (!address && !bps) return undefined; is correct. But if either var is set to "" (empty string, which .trim() reduces to ""), the falsy check handles it. This is fine — just confirming the behavior is intentional and not a gap.

---

Summary

The core logic is correct and well-tested. The one real issue worth addressing before merge is the f: 0 inconsistency between parser and render guard. Everything else is minor or advisory.

Suggested change for the render guards (all three order summaries):

// Before
{!!DEFAULT_BUILDER_CONFIG?.f && (

// After — guard on existence, not value
{DEFAULT_BUILDER_CONFIG !== undefined && (

Or equivalently, reject f: 0 in parseBuilderConfig and keep !!f.

🤖 Generated with Claude Code