Skip to content

Commit fe54f8a

Browse files
piyalbasuclaude
andcommitted
Show all operation fields in the transaction signing details view
The signing-approval Details view dropped several operation fields whose values are falsy but meaningful, and rendered combined account-flag bitmasks as blank. Use presence checks instead of truthy guards, decode flag bitmasks bit-by-bit, surface data-entry deletions and home-domain clears, and add a warning when a setOptions op disables the master key. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 191f6cf commit fe54f8a

4 files changed

Lines changed: 309 additions & 28 deletions

File tree

Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,220 @@
1+
import React from "react";
2+
import { render, screen } from "@testing-library/react";
3+
import { Provider } from "react-redux";
4+
import {
5+
Account,
6+
Asset,
7+
BASE_FEE,
8+
Networks,
9+
Operation,
10+
StrKey,
11+
TransactionBuilder,
12+
xdr,
13+
} from "stellar-sdk";
14+
15+
import { makeDummyStore } from "popup/__testHelpers__";
16+
import { Operations } from "../index";
17+
18+
// setOptions never triggers the asset scanner, but mock it so the component's
19+
// effect can never reach the network in the test environment.
20+
jest.mock("popup/helpers/blockaid", () => ({
21+
scanAsset: jest.fn().mockResolvedValue(undefined),
22+
}));
23+
24+
// Build a setOptions transaction with the bundled SDK, serialize it to XDR and
25+
// decode it back — the exact path Freighter uses to obtain the operation object
26+
// it renders on the signing-approval screen. A present-but-zero Uint32 field
27+
// (e.g. masterWeight: 0) decodes to the JS number 0.
28+
// Valid ed25519 strkeys minted from fixed bytes — avoids curve math (and the
29+
// crypto RNG, which is unavailable in this test environment).
30+
const SOURCE_KEY = StrKey.encodeEd25519PublicKey(Buffer.alloc(32, 1));
31+
const ADDED_SIGNER = StrKey.encodeEd25519PublicKey(Buffer.alloc(32, 7));
32+
33+
type SetOptionsOptions = Parameters<typeof Operation.setOptions>[0];
34+
35+
const decodeOperation = (operation: xdr.Operation) => {
36+
const account = new Account(SOURCE_KEY, "0");
37+
const tx = new TransactionBuilder(account, {
38+
fee: BASE_FEE,
39+
networkPassphrase: Networks.TESTNET,
40+
})
41+
.addOperation(operation)
42+
.setTimeout(0)
43+
.build();
44+
45+
return TransactionBuilder.fromXDR(tx.toXDR(), Networks.TESTNET)
46+
.operations as Operation[];
47+
};
48+
49+
const decodeSetOptions = (options: SetOptionsOptions) =>
50+
decodeOperation(Operation.setOptions(options));
51+
52+
const renderOps = (operations: Operation[]) =>
53+
render(
54+
<Provider store={makeDummyStore({})}>
55+
<Operations
56+
flaggedKeys={{}}
57+
isMemoRequired={false}
58+
operations={operations}
59+
/>
60+
</Provider>,
61+
);
62+
63+
// Read the value rendered next to a given operation-detail label.
64+
const rowValue = (key: string) => {
65+
const keyEl = screen
66+
.getAllByTestId("OperationKeyVal__key")
67+
.find((el) => el.textContent === key);
68+
return keyEl?.parentElement
69+
?.querySelector('[data-testid="OperationKeyVal__value"]')
70+
?.textContent?.trim();
71+
};
72+
73+
const MASTER_KEY_WARNING = /disables your account's master key/i;
74+
75+
describe("Operations — setOptions field visibility", () => {
76+
it("decoder yields numeric 0 (falsy) for masterWeight/thresholds", () => {
77+
const [op] = decodeSetOptions({
78+
masterWeight: 0,
79+
lowThreshold: 0,
80+
medThreshold: 0,
81+
highThreshold: 0,
82+
}) as any[];
83+
84+
expect(op.masterWeight).toBe(0);
85+
expect(op.lowThreshold).toBe(0);
86+
expect(op.medThreshold).toBe(0);
87+
expect(op.highThreshold).toBe(0);
88+
});
89+
90+
it("renders masterWeight 0, zeroed thresholds, and a signer change, with a master-key warning", () => {
91+
renderOps(
92+
decodeSetOptions({
93+
masterWeight: 0,
94+
lowThreshold: 0,
95+
medThreshold: 0,
96+
highThreshold: 0,
97+
signer: { ed25519PublicKey: ADDED_SIGNER, weight: 1 },
98+
}),
99+
);
100+
101+
expect(screen.getByText("Set Options")).toBeInTheDocument();
102+
expect(screen.getByText("Signer")).toBeInTheDocument();
103+
expect(rowValue("Master Weight")).toBe("0");
104+
expect(rowValue("High Threshold")).toBe("0");
105+
expect(rowValue("Medium Threshold")).toBe("0");
106+
expect(rowValue("Low Threshold")).toBe("0");
107+
expect(screen.getByText(MASTER_KEY_WARNING)).toBeInTheDocument();
108+
});
109+
110+
it("renders masterWeight 0 on its own with the warning, never an empty operation", () => {
111+
renderOps(decodeSetOptions({ masterWeight: 0 }));
112+
113+
expect(rowValue("Master Weight")).toBe("0");
114+
expect(screen.getByText(MASTER_KEY_WARNING)).toBeInTheDocument();
115+
});
116+
117+
it("non-zero masterWeight/threshold render and do not warn", () => {
118+
renderOps(decodeSetOptions({ masterWeight: 2, highThreshold: 3 }));
119+
120+
expect(rowValue("Master Weight")).toBe("2");
121+
expect(rowValue("High Threshold")).toBe("3");
122+
expect(screen.queryByText(MASTER_KEY_WARNING)).not.toBeInTheDocument();
123+
});
124+
125+
it("HOME DOMAIN: clearing the home domain is surfaced, not hidden", () => {
126+
renderOps(decodeSetOptions({ homeDomain: "" }));
127+
128+
expect(rowValue("Home Domain")).toBe("(clearing home domain)");
129+
});
130+
131+
it("FLAGS: a single-bit setFlags decodes to its label", () => {
132+
renderOps(decodeSetOptions({ setFlags: 1 }));
133+
134+
expect(rowValue("Set Flags")).toBe("Authorization Required");
135+
});
136+
137+
it("FLAGS: a combined setFlags bitmask decodes every set bit, not a blank value", () => {
138+
// REVOCABLE (2) | CLAWBACK (8) = 10. The SDK types setFlags as a single
139+
// AuthFlag, but the wire format is a bitmask — cast to exercise that.
140+
renderOps(decodeSetOptions({ setFlags: 10 as any }));
141+
142+
expect(rowValue("Set Flags")).toBe(
143+
"Authorization Revocable, Authorization Clawback Enabled",
144+
);
145+
});
146+
147+
it("FLAGS: a combined clearFlags bitmask decodes every set bit", () => {
148+
// REQUIRED (1) | REVOCABLE (2) = 3
149+
renderOps(decodeSetOptions({ clearFlags: 3 as any }));
150+
151+
expect(rowValue("Clear Flags")).toBe(
152+
"Authorization Required, Authorization Revocable",
153+
);
154+
});
155+
});
156+
157+
describe("Operations — manageData value visibility", () => {
158+
it("renders a set value", () => {
159+
renderOps(
160+
decodeOperation(Operation.manageData({ name: "k", value: "hi" })),
161+
);
162+
163+
expect(rowValue("Value")).toBe("hi");
164+
});
165+
166+
it("renders the Value row for an empty value rather than hiding it", () => {
167+
renderOps(decodeOperation(Operation.manageData({ name: "k", value: "" })));
168+
169+
expect(screen.getByText("Value")).toBeInTheDocument();
170+
expect(rowValue("Value")).toBe("");
171+
});
172+
173+
it("surfaces a deletion when value is absent ", () => {
174+
renderOps(
175+
decodeOperation(Operation.manageData({ name: "k", value: null })),
176+
);
177+
178+
expect(rowValue("Value")).toBe("(deleting entry)");
179+
});
180+
});
181+
182+
describe("Operations — setTrustLineFlags visibility", () => {
183+
const TRUSTOR = StrKey.encodeEd25519PublicKey(Buffer.alloc(32, 3));
184+
const ASSET = new Asset(
185+
"USDC",
186+
StrKey.encodeEd25519PublicKey(Buffer.alloc(32, 9)),
187+
);
188+
189+
const decodeSetTrustLineFlags = (flags: {
190+
authorized?: boolean;
191+
authorizedToMaintainLiabilities?: boolean;
192+
clawbackEnabled?: boolean;
193+
}) =>
194+
decodeOperation(
195+
Operation.setTrustLineFlags({ trustor: TRUSTOR, asset: ASSET, flags }),
196+
);
197+
198+
it("renders a flag being enabled", () => {
199+
renderOps(decodeSetTrustLineFlags({ authorized: true }));
200+
201+
expect(rowValue("Authorized")).toBe("Enabled");
202+
});
203+
204+
it("renders a flag being cleared (set to false), not hidden", () => {
205+
renderOps(decodeSetTrustLineFlags({ authorized: false }));
206+
207+
expect(rowValue("Authorized")).toBe("Disabled");
208+
});
209+
210+
it("does not render a flag that is left unchanged", () => {
211+
renderOps(decodeSetTrustLineFlags({ clawbackEnabled: false }));
212+
213+
expect(
214+
screen
215+
.getAllByTestId("OperationKeyVal__key")
216+
.some((el) => el.textContent === "Authorized"),
217+
).toBe(false);
218+
expect(rowValue("Clawback Enabled")).toBe("Disabled");
219+
});
220+
});

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

Lines changed: 77 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,26 @@ const MemoRequiredWarning = ({
5252
) : null;
5353
};
5454

55+
const MasterKeyDisableWarning = () => {
56+
const { t } = useTranslation();
57+
58+
return (
59+
<KeyValueList
60+
operationKey=""
61+
operationValue={
62+
<IconButton
63+
label={t(
64+
"This transaction disables your account's master key. You may permanently lose access to this account unless another signer with sufficient weight is added.",
65+
)}
66+
altText="Warning"
67+
icon={<Icon.InfoCircle />}
68+
variant="error"
69+
/>
70+
}
71+
/>
72+
);
73+
};
74+
5575
const DestinationWarning = ({
5676
destination,
5777
flaggedKeys,
@@ -96,6 +116,18 @@ export const Operations = ({
96116
"8": "Authorization Clawback Enabled",
97117
};
98118

119+
// Account flags are a bitmask, so a combined value (e.g. REVOCABLE |
120+
// CLAWBACK = 10) is not a key in AuthorizationMapToDisplay. Decode each set
121+
// bit individually so combined flags are never rendered as a blank value.
122+
const decodeAuthorizationFlags = (bits: number) => {
123+
const labels = Object.entries(AuthorizationMapToDisplay)
124+
.filter(([bit]) => (bits & Number(bit)) !== 0)
125+
.map(([, label]) => label);
126+
return labels.length
127+
? labels.join(", ")
128+
: t("Unknown ({{bits}})", { bits });
129+
};
130+
99131
const RenderOpByType = ({ op }: { op: Operation }) => {
100132
const networkDetails = useSelector(settingsNetworkDetailsSelector);
101133

@@ -337,48 +369,49 @@ export const Operations = ({
337369
operationValue={inflationDest}
338370
/>
339371
)}
340-
{homeDomain && (
372+
{homeDomain !== undefined && (
341373
<KeyValueList
342374
operationKey={t("Home Domain")}
343-
operationValue={homeDomain}
375+
operationValue={
376+
homeDomain === "" ? t("(clearing home domain)") : homeDomain
377+
}
344378
/>
345379
)}
346-
{highThreshold && (
380+
{highThreshold !== undefined && (
347381
<KeyValueList
348382
operationKey={t("High Threshold")}
349-
operationValue={highThreshold?.toString()}
383+
operationValue={highThreshold.toString()}
350384
/>
351385
)}
352-
{medThreshold && (
386+
{medThreshold !== undefined && (
353387
<KeyValueList
354388
operationKey={t("Medium Threshold")}
355-
operationValue={medThreshold?.toString()}
389+
operationValue={medThreshold.toString()}
356390
/>
357391
)}
358-
{lowThreshold && (
392+
{lowThreshold !== undefined && (
359393
<KeyValueList
360394
operationKey={t("Low Threshold")}
361-
operationValue={lowThreshold?.toString()}
395+
operationValue={lowThreshold.toString()}
362396
/>
363397
)}
364-
{masterWeight && (
398+
{masterWeight !== undefined && (
365399
<KeyValueList
366400
operationKey={t("Master Weight")}
367-
operationValue={masterWeight?.toString()}
401+
operationValue={masterWeight.toString()}
368402
/>
369403
)}
370-
{setFlags && (
404+
{masterWeight === 0 && <MasterKeyDisableWarning />}
405+
{setFlags !== undefined && (
371406
<KeyValueList
372407
operationKey={t("Set Flags")}
373-
operationValue={AuthorizationMapToDisplay[setFlags?.toString()]}
408+
operationValue={decodeAuthorizationFlags(setFlags)}
374409
/>
375410
)}
376-
{clearFlags && (
411+
{clearFlags !== undefined && (
377412
<KeyValueList
378413
operationKey={t("Clear Flags")}
379-
operationValue={
380-
AuthorizationMapToDisplay[clearFlags.toString()]
381-
}
414+
operationValue={decodeAuthorizationFlags(clearFlags)}
382415
/>
383416
)}
384417
</>
@@ -435,15 +468,19 @@ export const Operations = ({
435468

436469
case "manageData": {
437470
const { name, value } = op;
471+
// A null/undefined value means the data entry is being deleted; an
472+
// empty value decodes to a zero-length buffer. Always render the row so
473+
// a deletion is never silently hidden from the approval screen.
474+
const isDeletingEntry = value === undefined || value === null;
438475
return (
439476
<>
440477
<KeyValueList operationKey={t("Name")} operationValue={name} />
441-
{value && (
442-
<KeyValueList
443-
operationKey={t("Value")}
444-
operationValue={value?.toString()}
445-
/>
446-
)}
478+
<KeyValueList
479+
operationKey={t("Value")}
480+
operationValue={
481+
isDeletingEntry ? t("(deleting entry)") : value.toString()
482+
}
483+
/>
447484
</>
448485
);
449486
}
@@ -537,22 +574,34 @@ export const Operations = ({
537574
operationKey={t("Asset Code")}
538575
operationValue={asset.code}
539576
/>
540-
{flags.authorized && (
577+
{/*
578+
A flag present in the decoded `flags` object is being changed:
579+
`true` enables it, `false` *clears* it. Use a presence check so a
580+
cleared flag is never hidden, and render the value explicitly — a
581+
raw boolean is not rendered by React.
582+
*/}
583+
{flags.authorized !== undefined && (
541584
<KeyValueList
542585
operationKey={t(FLAG_TYPES.authorized)}
543-
operationValue={flags.authorized}
586+
operationValue={flags.authorized ? t("Enabled") : t("Disabled")}
544587
/>
545588
)}
546-
{flags.authorizedToMaintainLiabilities && (
589+
{flags.authorizedToMaintainLiabilities !== undefined && (
547590
<KeyValueList
548591
operationKey={t(FLAG_TYPES.authorizedToMaintainLiabilities)}
549-
operationValue={flags.authorizedToMaintainLiabilities}
592+
operationValue={
593+
flags.authorizedToMaintainLiabilities
594+
? t("Enabled")
595+
: t("Disabled")
596+
}
550597
/>
551598
)}
552-
{flags.clawbackEnabled && (
599+
{flags.clawbackEnabled !== undefined && (
553600
<KeyValueList
554601
operationKey={t(FLAG_TYPES.clawbackEnabled)}
555-
operationValue={flags.clawbackEnabled}
602+
operationValue={
603+
flags.clawbackEnabled ? t("Enabled") : t("Disabled")
604+
}
556605
/>
557606
)}
558607
</>

0 commit comments

Comments
 (0)