fix(hive): render op summaries in the approval card

chrome-extension/src/background/chains/hiveHandler.ts

@@ -3,6 +3,7 @@ import { v4 as uuidv4 } from 'uuid';
 import * as wallet from '../wallet';
 import { createProviderRpcError, createTimeoutError } from '../utils';
 import { requireHiveFirmware } from '../firmware';
+import { SUPPORTED_OPS, opSummary } from './hiveOps';
 
 const TAG = ' | hiveHandler | ';
 
@@ -563,27 +564,6 @@ async function hiveSignBuffer(
   return { result: signature, publicKey: public_key };
 }
 
-// Firmware clear-sign op table — phase 1 + phase 2
-// (handoff-hive-sign-operations-phase2.md). The vault serializer and the
-// firmware both re-enforce this; the check here just fails fast with a
-// clear dApp-facing error.
-const SUPPORTED_OPS = new Set([
-  'vote',
-  'comment',
-  'custom_json',
-  'transfer_to_vesting',
-  'withdraw_vesting',
-  'convert',
-  'comment_options',
-  'transfer_to_savings',
-  'transfer_from_savings',
-  'claim_reward_balance',
-  'delegate_vesting_shares',
-  'account_update2',
-  'limit_order_create',
-  'limit_order_cancel',
-]);
-
 /** Strict "x.xxx" normalization — same no-parseFloat rule as hiveTransfer. */
 function normalizeAmount3(amount: any, what: string): string {
   if (typeof amount !== 'string' || !/^\d+(\.\d{1,3})?$/.test(amount)) {
@@ -624,50 +604,6 @@ async function hpToVests(hp3: string): Promise<string> {
 // conversions/withdrawals; per-account id tracking if a dApp ever collides.
 const epochRequestId = () => Math.floor(Date.now() / 1000);
 
-/** One-line device-preview summary per op for the side-panel approval. */
-function opSummary(name: string, p: Record<string, any>): string {
-  switch (name) {
-    case 'vote':
-      return `@${p.voter} → @${p.author}/${p.permlink} (${(Number(p.weight) / 100).toFixed(0)}%)`;
-    case 'comment':
-      return `@${p.author}: ${p.title || p.permlink}`;
-    case 'custom_json':
-      return `${p.id}: ${String(p.json).slice(0, 120)}`;
-    case 'transfer_to_vesting':
-      return `Power up ${p.amount} → @${p.to}`;
-    case 'withdraw_vesting':
-      return String(p.vesting_shares).startsWith('0.000000')
-        ? `Stop power down (@${p.account})`
-        : `Power down ${p.vesting_shares} from @${p.account}`;
-    case 'convert':
-      return `Convert ${p.amount} → HIVE (request ${p.requestid})`;
-    case 'comment_options':
-      return `Payout options for @${p.author}/${p.permlink}${
-        (p.extensions?.[0]?.[1]?.beneficiaries ?? [])
-          .map((b: any) => ` · ${(Number(b.weight) / 100).toFixed(1)}% → @${b.account}`)
-          .join('') || ''
-      }`;
-    case 'transfer_to_savings':
-      return `Savings deposit ${p.amount} → @${p.to}`;
-    case 'transfer_from_savings':
-      return `Savings withdraw ${p.amount} → @${p.to}`;
-    case 'claim_reward_balance':
-      return `Claim ${p.reward_hive}, ${p.reward_hbd}, ${p.reward_vests}`;
-    case 'delegate_vesting_shares':
-      return String(p.vesting_shares).startsWith('0.000000')
-        ? `Remove delegation from @${p.delegatee}`
-        : `Delegate ${p.vesting_shares} → @${p.delegatee}`;
-    case 'account_update2':
-      return `Update profile @${p.account}`;
-    case 'limit_order_create':
-      return `Sell ${p.amount_to_sell} for ${p.min_to_receive}${p.fill_or_kill ? ' (fill or kill)' : ''}`;
-    case 'limit_order_cancel':
-      return `Cancel order ${p.orderid} (@${p.owner})`;
-    default:
-      return name;
-  }
-}
-
 /**
  * Shared path for vote/post/custom_json/broadcast: validate ops against the
  * firmware's phase-1 clear-sign table, approve, sign via the vault
@@ -700,7 +636,11 @@ async function hiveSignAndBroadcastOps(
   const event = buildEvent(requestInfo, displayType, params);
   (event as any).unsignedTx = {
     from: from.name,
-    operations: operations.map(([name, p]) => ({ op: name, summary: opSummary(name, p) })),
+    // `params` verbatim so the Raw tab remains a complete record: the summary
+    // is a one-liner and necessarily elides (long custom_json, default payout
+    // controls), and a value no view can recover is a value the user cannot
+    // check against the device screen.
+    operations: operations.map(([name, p]) => ({ op: name, summary: opSummary(name, p), params: p })),
   };
   await requestUserApproval(event, requestInfo, displayType, params, requireApproval);
 

chrome-extension/src/background/chains/hiveOpSummary.test.ts

@@ -0,0 +1,159 @@
+/**
+ * Every clear-signable Hive op must render a readable summary.
+ *
+ * The side-panel approval card shows `unsignedTx.operations[].summary` and
+ * nothing else — a Hive tx has no single destination or amount to fall back
+ * on. opSummary()'s `default` arm returns the bare op name, so an op added to
+ * SUPPORTED_OPS without a matching `case` degrades the approval to
+ * "limit_order_create" with no values in it, silently. That is the exact
+ * unreadable-confirm this pairs with RequestDetailsCard to prevent.
+ */
+import { describe, it, expect } from 'vitest';
+import { SUPPORTED_OPS, opSummary } from './hiveOps';
+
+// One realistic payload per op, in the vault serializer's field names.
+const SAMPLES: Record<string, Record<string, any>> = {
+  vote: { voter: 'alice', author: 'bob', permlink: 'a-post', weight: 10000 },
+  comment: { author: 'alice', permlink: 'a-post', title: 'Hello', body: 'hi', json_metadata: '{}' },
+  custom_json: { required_auths: [], required_posting_auths: ['alice'], id: 'follow', json: '["follow",{}]' },
+  // to !== from on purpose: a self-power-up would hide a from/to swap.
+  transfer_to_vesting: { from: 'alice', to: 'bob', amount: '1.500 HIVE' },
+  withdraw_vesting: { account: 'alice', vesting_shares: '1000.000000 VESTS' },
+  limit_order_create: {
+    owner: 'alice',
+    orderid: 1,
+    amount_to_sell: '1.500 HIVE',
+    min_to_receive: '0.400 HBD',
+    fill_or_kill: false,
+    expiration: 1700003600,
+  },
+  limit_order_cancel: { owner: 'alice', orderid: 1 },
+  convert: { owner: 'alice', requestid: 1, amount: '0.400 HBD' },
+  comment_options: {
+    author: 'alice',
+    permlink: 'a-post',
+    max_accepted_payout: '1000000.000 HBD',
+    percent_hbd: 10000,
+    allow_votes: true,
+    allow_curation_rewards: true,
+    extensions: [],
+  },
+  transfer_to_savings: { from: 'alice', to: 'bob', amount: '1.500 HIVE', memo: '' },
+  transfer_from_savings: { from: 'alice', request_id: 1, to: 'bob', amount: '1.500 HIVE', memo: '' },
+  claim_reward_balance: {
+    account: 'alice',
+    reward_hive: '1.500 HIVE',
+    reward_hbd: '0.400 HBD',
+    reward_vests: '1000.000000 VESTS',
+  },
+  delegate_vesting_shares: { delegator: 'alice', delegatee: 'bob', vesting_shares: '1000.000000 VESTS' },
+  account_update2: { account: 'alice', json_metadata: '', posting_json_metadata: '{}' },
+};
+
+describe('Hive op summaries', () => {
+  it('has a sample payload for every supported op', () => {
+    const missing = [...SUPPORTED_OPS].filter(op => !(op in SAMPLES));
+    expect(missing, `No test payload for: ${missing.join(', ')}`).toEqual([]);
+  });
+
+  it('summarizes every supported op with more than its own name', () => {
+    for (const op of SUPPORTED_OPS) {
+      const summary = opSummary(op, SAMPLES[op]);
+      // The `default` arm returns `name` verbatim. Anything equal to the op
+      // name means no case matched and the approval would render unreadably.
+      expect(summary, `${op} falls through to the default arm — add a case to opSummary()`).not.toBe(op);
+      expect(summary.length, `${op} summary is empty`).toBeGreaterThan(0);
+    }
+  });
+
+  it('renders the values a user must check against the device screen', () => {
+    // Amount + counterparty are what the OLED shows; the panel has to show the
+    // same thing or the comparison the clear-sign table exists for is impossible.
+    expect(opSummary('limit_order_create', SAMPLES.limit_order_create)).toContain('1.500 HIVE');
+    expect(opSummary('limit_order_create', SAMPLES.limit_order_create)).toContain('0.400 HBD');
+    expect(opSummary('transfer_to_savings', SAMPLES.transfer_to_savings)).toContain('bob');
+    // Recipient, not sender — the vault serializes str(from), str(to) and a
+    // swapped pair would still render plausibly.
+    expect(opSummary('transfer_to_vesting', SAMPLES.transfer_to_vesting)).toContain('@bob');
+    expect(opSummary('transfer_to_vesting', SAMPLES.transfer_to_vesting)).toContain('1.500 HIVE');
+    expect(opSummary('claim_reward_balance', SAMPLES.claim_reward_balance)).toContain('1000.000000 VESTS');
+  });
+
+  it('distinguishes the zero-amount sentinels from real amounts', () => {
+    // '0.000000 VESTS' means stop/remove, not "send zero" — a user approving
+    // these must not see the same wording as an actual power-down.
+    expect(opSummary('withdraw_vesting', { account: 'alice', vesting_shares: '0.000000 VESTS' })).toMatch(/stop/i);
+    expect(
+      opSummary('delegate_vesting_shares', { delegator: 'alice', delegatee: 'bob', vesting_shares: '0.000000 VESTS' }),
+    ).toMatch(/remove/i);
+  });
+
+  it('shows the JSON a custom_json actually signs, object or string', () => {
+    // The vault serializes `typeof json === 'string' ? json : JSON.stringify(json)`
+    // (hive-ops.ts:151). String(obj) would render "[object Object]" while the
+    // object's real contents get signed — approval showing neither.
+    const asObject = opSummary('custom_json', { id: 'follow', json: { follow: 'bob' } });
+    expect(asObject).not.toContain('[object Object]');
+    expect(asObject).toContain('"follow":"bob"');
+
+    const asString = opSummary('custom_json', { id: 'follow', json: '["follow",{"a":1}]' });
+    expect(asString).toContain('["follow",{"a":1}]');
+  });
+
+  it('marks a truncated custom_json so two payloads cannot look identical', () => {
+    const prefix = 'x'.repeat(120);
+    const a = opSummary('custom_json', { id: 'test', json: prefix + 'AAAA' });
+    const b = opSummary('custom_json', { id: 'test', json: prefix + 'BBBBBBBB' });
+    expect(a).toContain('…');
+    expect(a).not.toBe(b);
+    expect(a).toContain('+4 more chars');
+    expect(b).toContain('+8 more chars');
+    // Short payloads must not be marked at all.
+    expect(opSummary('custom_json', { id: 'test', json: '{"a":1}' })).not.toContain('…');
+  });
+
+  it('surfaces every non-default comment_options payout control', () => {
+    // Defaults stay quiet — a default-everything comment_options is just the post.
+    expect(opSummary('comment_options', SAMPLES.comment_options)).toBe('Payout options for @alice/a-post');
+
+    const declined = opSummary('comment_options', { ...SAMPLES.comment_options, max_accepted_payout: '0.000 HBD' });
+    expect(declined).toContain('0.000 HBD');
+
+    const allHive = opSummary('comment_options', { ...SAMPLES.comment_options, percent_hbd: 0 });
+    expect(allHive).toContain('0.0% HBD');
+
+    const noVotes = opSummary('comment_options', { ...SAMPLES.comment_options, allow_votes: false });
+    expect(noVotes).toMatch(/votes disabled/i);
+
+    const noCuration = opSummary('comment_options', {
+      ...SAMPLES.comment_options,
+      allow_curation_rewards: false,
+    });
+    expect(noCuration).toMatch(/curation rewards disabled/i);
+
+    // Materially different payout behaviour must not render identically.
+    expect(declined).not.toBe(allHive);
+    expect(allHive).not.toBe(noVotes);
+  });
+
+  it('reads percent_steem_dollars, the legacy alias the vault also accepts', () => {
+    // hive-ops.ts:225 falls back to it; a summary that ignored it would show
+    // the default while a non-default value was signed.
+    const legacy = opSummary('comment_options', {
+      ...SAMPLES.comment_options,
+      percent_hbd: undefined,
+      percent_steem_dollars: 0,
+    });
+    expect(legacy).toContain('0.0% HBD');
+  });
+
+  it('names the beneficiaries a comment_options redirects payout to', () => {
+    const withBenes = {
+      ...SAMPLES.comment_options,
+      extensions: [[0, { beneficiaries: [{ account: 'carol', weight: 2500 }] }]],
+    };
+    const summary = opSummary('comment_options', withBenes);
+    expect(summary).toContain('carol');
+    expect(summary).toContain('25.0%');
+  });
+});

chrome-extension/src/background/chains/hiveOps.ts

@@ -0,0 +1,117 @@
+/**
+ * The Hive clear-sign op table and its approval-card summaries.
+ *
+ * A leaf module on purpose: hiveHandler.ts imports @extension/storage, which
+ * touches chrome.* at import time and so cannot load under vitest. Keeping the
+ * table and the pure formatter here is what lets hiveOpSummary.test.ts import
+ * them at all.
+ */
+
+// Firmware clear-sign op table — phase 1 + phase 2
+// (handoff-hive-sign-operations-phase2.md). The vault serializer and the
+// firmware both re-enforce this; the check here just fails fast with a
+// clear dApp-facing error.
+export const SUPPORTED_OPS = new Set([
+  'vote',
+  'comment',
+  'custom_json',
+  'transfer_to_vesting',
+  'withdraw_vesting',
+  'convert',
+  'comment_options',
+  'transfer_to_savings',
+  'transfer_from_savings',
+  'claim_reward_balance',
+  'delegate_vesting_shares',
+  'account_update2',
+  'limit_order_create',
+  'limit_order_cancel',
+]);
+
+/** Hive's own defaults for comment_options — anything else is worth showing. */
+const DEFAULT_MAX_PAYOUT = '1000000.000 HBD';
+const DEFAULT_PERCENT_HBD = 10000;
+
+/**
+ * Cap a value for one-line display, always marking the cut.
+ *
+ * Silent truncation renders two different payloads sharing a 120-char prefix
+ * as the same string, so the marker carries the dropped length. The full value
+ * is in the Raw tab (hiveHandler stashes `params` verbatim) — this is the
+ * one-line view, not the record.
+ */
+function truncate(s: string, max: number): string {
+  return s.length <= max ? s : `${s.slice(0, max)}… (+${s.length - max} more chars)`;
+}
+
+/**
+ * The exact JSON the vault will serialize — mirrors hive-ops.ts:151.
+ *
+ * A dApp may pass `json` as an object; `String(obj)` yields "[object Object]"
+ * while the object's actual contents are what gets signed. The approval must
+ * show the signed bytes, not a placeholder for them.
+ */
+function serializedJson(json: any): string {
+  return typeof json === 'string' ? json : JSON.stringify(json ?? {});
+}
+
+/** One-line device-preview summary per op for the side-panel approval. */
+export function opSummary(name: string, p: Record<string, any>): string {
+  switch (name) {
+    case 'vote':
+      return `@${p.voter} → @${p.author}/${p.permlink} (${(Number(p.weight) / 100).toFixed(0)}%)`;
+    case 'comment':
+      return `@${p.author}: ${p.title || p.permlink}`;
+    case 'custom_json':
+      return `${p.id}: ${truncate(serializedJson(p.json), 120)}`;
+    case 'transfer_to_vesting':
+      return `Power up ${p.amount} → @${p.to}`;
+    case 'withdraw_vesting':
+      return String(p.vesting_shares).startsWith('0.000000')
+        ? `Stop power down (@${p.account})`
+        : `Power down ${p.vesting_shares} from @${p.account}`;
+    case 'convert':
+      return `Convert ${p.amount} → HIVE (request ${p.requestid})`;
+    case 'comment_options': {
+      // Every payout control the device screen shows, whenever it differs from
+      // the Hive default. Showing only the post + beneficiaries made two
+      // comment_options with materially different payout behaviour — capped
+      // payout, all-HIVE split, votes or curation disabled — render
+      // identically in the browser.
+      const percentHbd = Number(p.percent_hbd ?? p.percent_steem_dollars);
+      const controls: string[] = [];
+      if (p.max_accepted_payout != null && p.max_accepted_payout !== DEFAULT_MAX_PAYOUT) {
+        controls.push(`max payout ${p.max_accepted_payout}`);
+      }
+      if (Number.isFinite(percentHbd) && percentHbd !== DEFAULT_PERCENT_HBD) {
+        controls.push(`${(percentHbd / 100).toFixed(1)}% HBD`);
+      }
+      if (p.allow_votes === false) controls.push('votes disabled');
+      if (p.allow_curation_rewards === false) controls.push('curation rewards disabled');
+      const beneficiaries = (p.extensions?.[0]?.[1]?.beneficiaries ?? [])
+        .map((b: any) => ` · ${(Number(b.weight) / 100).toFixed(1)}% → @${b.account}`)
+        .join('');
+      return `Payout options for @${p.author}/${p.permlink}${
+        controls.length ? ` · ${controls.join(' · ')}` : ''
+      }${beneficiaries}`;
+    }
+    case 'transfer_to_savings':
+      return `Savings deposit ${p.amount} → @${p.to}`;
+    case 'transfer_from_savings':
+      return `Savings withdraw ${p.amount} → @${p.to}`;
+    case 'claim_reward_balance':
+      return `Claim ${p.reward_hive}, ${p.reward_hbd}, ${p.reward_vests}`;
+    case 'delegate_vesting_shares':
+      return String(p.vesting_shares).startsWith('0.000000')
+        ? `Remove delegation from @${p.delegatee}`
+        : `Delegate ${p.vesting_shares} → @${p.delegatee}`;
+    case 'account_update2':
+      return `Update profile @${p.account}`;
+    case 'limit_order_create':
+      return `Sell ${p.amount_to_sell} for ${p.min_to_receive}${p.fill_or_kill ? ' (fill or kill)' : ''}`;
+    case 'limit_order_cancel':
+      return `Cancel order ${p.orderid} (@${p.owner})`;
+    default:
+      return name;
+  }
+}