fix(sign-transaction): render hex signer keys instead of throwing

  SDK 17's revokeSponsorship signer arm returns sha256Hash and preAuthTx as
  already-hex strings (convertXdrSignerKeyToObject calls uint8ArrayToHex),
  while setOptions returns real Uint8Arrays. formattedBuffer asserted bytes,
  so reviewing a revokeSignerSponsorship op with a hash-based signer threw
  `TypeError: Expected Uint8Array, got string` from a render body and put the
  ErrorBoundary screen where the sign/reject decision belongs.

  formattedBuffer now accepts Uint8Array | string, passing a string through
  rather than re-encoding it -- encoding it again hexes the ASCII of the hex,
  which is what the old Buffer.from(<hex string>) path did. That silent
  corruption is the wrong-hex display reported in #2838, so this closes it
  too: the row now reads DEAD…BEEF where it used to read 6465…6566.

  The types were not the problem; the casts were. RevokeSignerOpts already
  declares these fields as `Uint8Array | string` -- correctly -- and it was
  `signer as unknown as Signer` at the call site that discarded the string,
  because setOptions' Signer type declares bare Uint8Array. Both signer
  components now derive their prop type from the operation unions:

    type RevokeSignerKey = Operation.RevokeSignerSponsorship["signer"]
    type SetOptionsSignerKey =
      NonNullable<Extract<OperationRecord, { type: "setOptions" }>["signer"]>

  so the compiler tracks reality and both `as unknown as Signer` casts are
  gone. Verified that dropping the cast without this change is a typecheck
  error, i.e. the compiler would have caught the original bug. The `in`
  checks became discriminant truthiness checks, which is how the SDK's
  `?: never` unions are meant to be narrowed.

  The setOptions path is fixed alongside the reported one. It returns real
  bytes today so it was never broken, but it carried the identical cast on
  the adjacent line and would have hidden the same defect on the next SDK
  shape change.

  Tests cover formattedBuffer for both input shapes (it had none) and assert
  the rendered hex for decoded revokeSignerSponsorship ops, driving the real
  TransactionBuilder decode path rather than a hand-built signer object.

Author: aristidesstaffieri

extension/src/popup/components/__tests__/OperationsKeyVal.test.tsx

@@ -1,9 +1,21 @@
 import React from "react";
 import { render, waitFor, screen, cleanup } from "@testing-library/react";
-import { Address, Keypair, Operation, StrKey, xdr } from "stellar-sdk";
+import {
+  Account,
+  Address,
+  Keypair,
+  Networks,
+  Operation,
+  StrKey,
+  TransactionBuilder,
+  xdr,
+} from "stellar-sdk";
 
 import { mockAccounts, TEST_PUBLIC_KEY, Wrapper } from "popup/__testHelpers__";
-import { KeyValueInvokeHostFn } from "../signTransaction/Operations/KeyVal";
+import {
+  KeyValueInvokeHostFn,
+  KeyValueSignerKeyOptions,
+} from "../signTransaction/Operations/KeyVal";
 import * as internalApi from "@shared/api/internal";
 import { APPLICATION_STATE } from "@shared/constants/applicationState";
 import {
@@ -307,4 +319,59 @@ describe("Operations KeyVal", () => {
       });
     });
   });
+
+  describe("KeyValueSignerKeyOptions", () => {
+    // SDK 17's revokeSignerSponsorship arm returns sha256Hash / preAuthTx as
+    // hex strings rather than bytes, so decode a real op instead of hand-rolling
+    // the shape.
+    const decodeRevokeSignerOp = (signer: {
+      sha256Hash?: any;
+      preAuthTx?: any;
+    }) => {
+      const tx = new TransactionBuilder(new Account(TEST_PUBLIC_KEY, "0"), {
+        fee: "100",
+        networkPassphrase: Networks.TESTNET,
+      })
+        .addOperation(
+          Operation.revokeSignerSponsorship({
+            account: TEST_PUBLIC_KEY,
+            signer: signer as any,
+          }),
+        )
+        .setTimeout(0)
+        .build();
+
+      return TransactionBuilder.fromXdr(
+        tx.toEnvelope().toXdr("base64"),
+        Networks.TESTNET,
+      ).operations[0] as Operation.RevokeSignerSponsorship;
+    };
+
+    const readValue = (label: string) =>
+      screen
+        .getByText(label)
+        .parentNode?.querySelector("[data-testid='OperationKeyVal__value']");
+
+    it("renders a sha256Hash signer key as its true hex value", async () => {
+      const op = decodeRevokeSignerOp({
+        sha256Hash: new Uint8Array(32).fill(0xab),
+      });
+
+      render(<KeyValueSignerKeyOptions signer={op.signer} />);
+      await waitFor(() => screen.getAllByTestId("OperationKeyVal"));
+
+      expect(readValue("Signer Sha256 Hash")).toHaveTextContent("ABAB…ABAB");
+    });
+
+    it("renders a preAuthTx signer key as its true hex value", async () => {
+      const op = decodeRevokeSignerOp({
+        preAuthTx: new Uint8Array(32).fill(0xcd),
+      });
+
+      render(<KeyValueSignerKeyOptions signer={op.signer} />);
+      await waitFor(() => screen.getAllByTestId("OperationKeyVal"));
+
+      expect(readValue("Pre Auth Transaction")).toHaveTextContent("CDCD…CDCD");
+    });
+  });
 });

extension/src/popup/components/signTransaction/Operations/KeyVal/index.tsx

@@ -8,7 +8,7 @@ import {
   LiquidityPoolAsset,
   nativeToScVal,
   Operation,
-  Signer,
+  OperationRecord,
   StrKey,
   xdr,
 } from "stellar-sdk";
@@ -148,11 +148,20 @@ export const KeyValueInvocation = ({
   );
 };
 
-export const KeyValueSigner = ({ signer }: { signer: Signer }) => {
+/**
+ * The signer shape a `setOptions` operation carries. Like {@link RevokeSignerKey}
+ * its hash fields are `Uint8Array | string`, so it must not be narrowed to
+ * `Signer` either -- even though this path returns real bytes today.
+ */
+type SetOptionsSignerKey = NonNullable<
+  Extract<OperationRecord, { type: "setOptions" }>["signer"]
+>;
+
+export const KeyValueSigner = ({ signer }: { signer: SetOptionsSignerKey }) => {
   const { t } = useTranslation();
 
   function renderSignerType() {
-    if ("ed25519PublicKey" in signer) {
+    if (signer.ed25519PublicKey) {
       return (
         <KeyValueWithPublicKey
           operationKey={t("Signer")}
@@ -161,7 +170,7 @@ export const KeyValueSigner = ({ signer }: { signer: Signer }) => {
       );
     }
 
-    if ("sha256Hash" in signer) {
+    if (signer.sha256Hash) {
       return (
         <KeyValueList
           operationKey={t("Signer")}
@@ -170,7 +179,7 @@ export const KeyValueSigner = ({ signer }: { signer: Signer }) => {
       );
     }
 
-    if ("preAuthTx" in signer) {
+    if (signer.preAuthTx) {
       return (
         <KeyValueList
           operationKey={t("Signer")}
@@ -179,7 +188,7 @@ export const KeyValueSigner = ({ signer }: { signer: Signer }) => {
       );
     }
 
-    if ("ed25519SignedPayload" in signer) {
+    if (signer.ed25519SignedPayload) {
       return (
         <KeyValueList
           operationKey={t("Signer")}
@@ -345,10 +354,21 @@ export const KeyValueClaimants = ({ claimants }: { claimants: Claimant[] }) => {
   );
 };
 
-export const KeyValueSignerKeyOptions = ({ signer }: { signer: Signer }) => {
+/**
+ * The signer shape a `revokeSignerSponsorship` operation carries. Unlike
+ * `setOptions`' `Signer`, its hash fields are `Uint8Array | string` -- which is
+ * the truth at runtime, so this must not be narrowed to `Signer`.
+ */
+type RevokeSignerKey = Operation.RevokeSignerSponsorship["signer"];
+
+export const KeyValueSignerKeyOptions = ({
+  signer,
+}: {
+  signer: RevokeSignerKey;
+}) => {
   const { t } = useTranslation();
 
-  if ("ed25519PublicKey" in signer) {
+  if (signer.ed25519PublicKey) {
     return (
       <KeyValueWithPublicKey
         operationKey={t("Signer Key")}
@@ -357,7 +377,7 @@ export const KeyValueSignerKeyOptions = ({ signer }: { signer: Signer }) => {
     );
   }
 
-  if ("sha256Hash" in signer) {
+  if (signer.sha256Hash) {
     return (
       <KeyValueList
         operationKey={t("Signer Sha256 Hash")}
@@ -366,7 +386,7 @@ export const KeyValueSignerKeyOptions = ({ signer }: { signer: Signer }) => {
     );
   }
 
-  if ("preAuthTx" in signer) {
+  if (signer.preAuthTx) {
     return (
       <KeyValueList
         operationKey={t("Pre Auth Transaction")}
@@ -375,7 +395,7 @@ export const KeyValueSignerKeyOptions = ({ signer }: { signer: Signer }) => {
     );
   }
 
-  if ("ed25519SignedPayload" in signer) {
+  if (signer.ed25519SignedPayload) {
     return (
       <KeyValueList
         operationKey={t("Signed Payload")}

extension/src/popup/components/signTransaction/Operations/index.tsx

@@ -2,7 +2,7 @@ import React, { useEffect } from "react";
 import { Badge, Icon, IconButton } from "@stellar/design-system";
 import { useSelector } from "react-redux";
 import { useTranslation } from "react-i18next";
-import { OperationRecord, Signer, xdr } from "stellar-sdk";
+import { OperationRecord, xdr } from "stellar-sdk";
 
 import {
   FLAG_TYPES,
@@ -417,11 +417,7 @@ export const Operations = ({
         } = op;
         return (
           <>
-            {signer && (
-              // v16 types the parsed setOptions signer as the builder opts
-              // type; at runtime it is a parsed Signer (Buffer-backed fields).
-              <KeyValueSigner signer={signer as unknown as Signer} />
-            )}
+            {signer && <KeyValueSigner signer={signer} />}
             {inflationDest && (
               <KeyValueWithPublicKey
                 operationKey={t("Inflation Destination")}
@@ -819,7 +815,7 @@ export const Operations = ({
           const { account, signer } = op;
           return (
             <>
-              <KeyValueSignerKeyOptions signer={signer as unknown as Signer} />
+              <KeyValueSignerKeyOptions signer={signer} />
               <KeyValueWithPublicKey
                 operationKey={t("Account")}
                 operationValue={account}

extension/src/popup/helpers/__tests__/formatters.test.ts

@@ -2,6 +2,7 @@ import BigNumber from "bignumber.js";
 import {
   formatAmountPreserveCursor,
   formatFiatAmount,
+  formattedBuffer,
   getValidBigNumber,
   isValidPositiveAmount,
   normalizeNumericString,
@@ -175,3 +176,22 @@ describe("formatFiatAmount", () => {
     expect(formatFiatAmount("not a number")).toBe("$0.00");
   });
 });
+
+describe("formattedBuffer", () => {
+  const bytes = new Uint8Array(32).fill(0xab);
+  const hex = "ab".repeat(32);
+
+  it("renders raw bytes as truncated uppercase hex", () => {
+    expect(formattedBuffer(bytes)).toBe("ABAB\u2026ABAB");
+  });
+
+  it("passes through a string that is already hex", () => {
+    // SDK 17's revokeSignerSponsorship arm hands us hex strings, not bytes,
+    // for sha256Hash / preAuthTx signer keys.
+    expect(formattedBuffer(hex)).toBe("ABAB\u2026ABAB");
+  });
+
+  it("renders bytes and their hex string identically", () => {
+    expect(formattedBuffer(hex)).toBe(formattedBuffer(bytes));
+  });
+});

extension/src/popup/helpers/formatters.ts

@@ -144,8 +144,22 @@ export const formatAmount = (val: string) => {
   return formattedWholeVal;
 };
 
-export const formattedBuffer = (data: Uint8Array) =>
-  truncatedPublicKey(xdr.encodeBytes(data, "hex").toUpperCase());
+/**
+ * Renders a hash for display as truncated uppercase hex.
+ *
+ * Accepts a string as well as bytes because the SDK is inconsistent: the
+ * `revokeSponsorship` signer arm returns `sha256Hash` / `preAuthTx` as
+ * already-hex strings (`convertXdrSignerKeyToObject`), while `setOptions`
+ * returns real `Uint8Array`s. A string is passed through rather than re-encoded
+ * -- encoding it again would hex the ASCII of the hex.
+ */
+export const formattedBuffer = (data: Uint8Array | string) =>
+  truncatedPublicKey(
+    (typeof data === "string"
+      ? data
+      : xdr.encodeBytes(data, "hex")
+    ).toUpperCase(),
+  );
 
 export const scrubPathGkey = (route: string, url: string) => {
   try {