Skip to content

Commit 96539ed

Browse files
mateuszjasiukmateuszjasiuk
authored
fix: use correct derivation path for masp signs using ledger AND sign all sapling inputs (#2163)
1 parent 5595dc2 commit 96539ed

File tree

10 files changed

+168
-59
lines changed

10 files changed

+168
-59
lines changed

‎apps/extension/src/Approvals/ConfirmSignLedgerTx.tsx‎

Lines changed: 62 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { getSdk } from "@namada/sdk/web";
2+
import sdkInit from "@namada/sdk/web-init";
13
import clsx from "clsx";
24
import { ReactNode, useCallback, useEffect, useState } from "react";
35

@@ -84,8 +86,9 @@ export const ConfirmSignLedgerTx: React.FC<Props> = ({ details }) => {
8486
const signMaspTx = async (
8587
ledger: Ledger,
8688
bytes: Uint8Array,
87-
path: string
88-
): Promise<{ sbar: Uint8Array; rbar: Uint8Array }> => {
89+
path: string,
90+
shieldedHash?: string
91+
): Promise<{ sbar: Uint8Array; rbar: Uint8Array }[]> => {
8992
const signMaspSpendsResponse = await ledger.namadaApp.signMaspSpends(
9093
path,
9194
Buffer.from(bytes)
@@ -97,14 +100,36 @@ export const ConfirmSignLedgerTx: React.FC<Props> = ({ details }) => {
97100
);
98101
}
99102

100-
const spendSignatureResponse = await ledger.namadaApp.getSpendSignature();
101-
if (spendSignatureResponse.returnCode !== LedgerError.NoErrors) {
102-
throw new Error(
103-
`Getting spends signature error encountered: ${signMaspSpendsResponse.errorMessage}`
104-
);
103+
if (!shieldedHash) {
104+
throw new Error("Shielded hash is required for MASP transactions");
105105
}
106106

107-
return spendSignatureResponse;
107+
const { cryptoMemory } = await sdkInit();
108+
// TODO: Find a better way to init the sdk, token has to be any valid token
109+
const sdk = getSdk(
110+
cryptoMemory,
111+
"",
112+
"",
113+
"",
114+
"tnam1q9gr66cvu4hrzm0sd5kmlnjje82gs3xlfg3v6nu7"
115+
);
116+
const descriptors = await sdk
117+
.getMasp()
118+
.getDescriptorMap(bytes, fromBase64(shieldedHash));
119+
120+
const responses = [];
121+
// TODO: this probably means that we do not really need descriptor map, but just to iterate over the sapling inputs
122+
for (const _ of descriptors) {
123+
const spendSignatureResponse = await ledger.namadaApp.getSpendSignature();
124+
if (spendSignatureResponse.returnCode !== LedgerError.NoErrors) {
125+
throw new Error(
126+
`Getting spends signature error encountered: ${signMaspSpendsResponse.errorMessage}`
127+
);
128+
}
129+
responses.push(spendSignatureResponse);
130+
}
131+
132+
return responses;
108133
};
109134

110135
const signLedgerTx = async (
@@ -130,15 +155,20 @@ export const ConfirmSignLedgerTx: React.FC<Props> = ({ details }) => {
130155
ledger: Ledger,
131156
tx: string,
132157
zip32Path: string,
133-
signatures: string[]
158+
signatures: string[],
159+
shieldedHash?: string
134160
) => {
135-
const { sbar, rbar } = await signMaspTx(
161+
const responses = await signMaspTx(
136162
ledger,
137163
fromBase64(tx),
138-
zip32Path
164+
zip32Path,
165+
shieldedHash
139166
);
140-
const signature = toBase64(new Uint8Array([...rbar, ...sbar]));
141-
signatures.push(signature);
167+
168+
responses.forEach(({ sbar, rbar }) => {
169+
const signature = toBase64(new Uint8Array([...rbar, ...sbar]));
170+
signatures.push(signature);
171+
});
142172
},
143173
[]
144174
);
@@ -197,10 +227,12 @@ export const ConfirmSignLedgerTx: React.FC<Props> = ({ details }) => {
197227
if (!accountDetails) {
198228
throw new Error(`Failed to query account details for ${signer}`);
199229
}
230+
const [transparentAccount, shieldedAccount] = accountDetails;
231+
200232
const path = {
201-
account: accountDetails.path.account,
202-
change: accountDetails.path.change || 0,
203-
index: accountDetails.path.index || 0,
233+
account: transparentAccount.path.account,
234+
change: transparentAccount.path.change || 0,
235+
index: transparentAccount.path.index || 0,
204236
};
205237

206238
const pendingTxs = await requester.sendMessage(
@@ -247,7 +279,7 @@ export const ConfirmSignLedgerTx: React.FC<Props> = ({ details }) => {
247279
transferTypes.includes("Unshielding") ||
248280
transferTypes.includes("IbcUnshieldTransfer");
249281

250-
for await (const tx of pendingTxs) {
282+
for await (const { tx, shieldedHash } of pendingTxs) {
251283
if (txCount > 1) {
252284
setStepTwoDescription(
253285
<p>
@@ -259,11 +291,22 @@ export const ConfirmSignLedgerTx: React.FC<Props> = ({ details }) => {
259291
}
260292

261293
if (fromMasp) {
294+
if (!shieldedAccount) {
295+
throw new Error(
296+
`Shielded account details for ${signer} not found!`
297+
);
298+
}
262299
const zip32Path = makeSaplingPath(chains.namada.bip44.coinType, {
263-
account: path.account,
300+
account: shieldedAccount.path.account,
264301
});
265302
// Adds new signature to the collection
266-
await handleMaspSignTx(ledger, tx, zip32Path, maspSignatures);
303+
await handleMaspSignTx(
304+
ledger,
305+
tx,
306+
zip32Path,
307+
maspSignatures,
308+
shieldedHash
309+
);
267310
} else {
268311
const bip44Path = makeBip44Path(chains.namada.bip44.coinType, path);
269312
// Adds new signature to the collection

‎apps/extension/src/background/approvals/messages.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,9 @@ export class QueryTxDetailsMsg extends Message<TxDetails[]> {
311311
}
312312
}
313313

314-
export class QueryPendingTxBytesMsg extends Message<string[] | undefined> {
314+
export class QueryPendingTxBytesMsg extends Message<
315+
{ tx: string; shieldedHash?: string }[] | undefined
316+
> {
315317
public static type(): MessageType {
316318
return MessageType.QueryPendingTxBytes;
317319
}

‎apps/extension/src/background/approvals/service.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ describe("approvals service", () => {
224224
// tx bytes expected to be base64-encoded
225225
const bytes = "dHhEYXRh"; // "txData"
226226

227-
(keyRingService.queryAccountDetails as any).mockResolvedValue(() => ({}));
227+
(keyRingService.queryAccountDetails as any).mockResolvedValue([{}, {}]);
228228

229229
const signaturePromise = service.approveSignTx(
230230
signer,

‎apps/extension/src/background/approvals/service.ts‎

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ export class ApprovalsService {
7979
if (!details) {
8080
throw new Error(ApprovalErrors.AccountNotFound(signer));
8181
}
82+
const [account] = details;
8283

8384
const pendingTx: PendingTx = {
8485
signer,
@@ -89,7 +90,7 @@ export class ApprovalsService {
8990
await this.txStore.set(msgId, pendingTx);
9091

9192
return this.launchApprovalPopup(
92-
`${TopLevelRoute.ApproveSignTx}/${msgId}/${encodeURIComponent(origin)}/${details.type}/${signer}`
93+
`${TopLevelRoute.ApproveSignTx}/${msgId}/${encodeURIComponent(origin)}/${account.type}/${signer}`
9394
);
9495
}
9596

@@ -214,21 +215,22 @@ export class ApprovalsService {
214215

215216
const { tx: sdkTx } = this.sdkService.getSdk();
216217

217-
const txsWithSignatures = signatures.map((signature, i) => {
218-
const tx = pendingTx.txs[i];
219-
const signingData = tx.signingData.map((signingData) =>
220-
new Message().encode(new SigningDataMsgValue(signingData))
221-
);
222-
const txBytes = sdkTx.appendMaspSignature(
223-
tx.bytes,
224-
signingData,
225-
fromBase64(signature)
226-
);
218+
const tx = pendingTx.txs[0];
219+
const signingData = tx.signingData.map((signingData) =>
220+
new Message().encode(new SigningDataMsgValue(signingData))
221+
);
222+
const bytes = sdkTx.appendMaspSignatures(
223+
tx.bytes,
224+
signingData,
225+
signatures.map(fromBase64)
226+
);
227227

228-
return { ...tx, bytes: txBytes };
229-
});
228+
const txWithSignatures = {
229+
...tx,
230+
bytes,
231+
};
230232

231-
await this.txStore.set(msgId, { ...pendingTx, txs: txsWithSignatures });
233+
await this.txStore.set(msgId, { ...pendingTx, txs: [txWithSignatures] });
232234
}
233235

234236
async submitSignArbitrary(
@@ -406,11 +408,15 @@ export class ApprovalsService {
406408
}
407409

408410
async approveUpdateDefaultAccount(address: string): Promise<void> {
409-
const account = await this.keyRingService.queryAccountDetails(address);
411+
const accounts = await this.keyRingService.queryAccountDetails(address);
412+
if (!accounts) {
413+
throw new Error(ApprovalErrors.AccountNotFound(address));
414+
}
415+
const [transactionAccount] = accounts;
410416

411417
return this.launchApprovalPopup(TopLevelRoute.ApproveUpdateDefaultAccount, {
412418
address,
413-
alias: account?.alias ?? "",
419+
alias: transactionAccount.alias,
414420
});
415421
}
416422

@@ -441,15 +447,23 @@ export class ApprovalsService {
441447
);
442448
}
443449

444-
async queryPendingTxBytes(msgId: string): Promise<string[] | undefined> {
450+
async queryPendingTxBytes(
451+
msgId: string
452+
): Promise<{ tx: string; shieldedHash?: string }[] | undefined> {
445453
const pendingTx = await this.txStore.get(msgId);
446454

447455
if (!pendingTx) {
448456
throw new Error(ApprovalErrors.TransactionDataNotFound(msgId));
449457
}
450458

451459
if (pendingTx.txs) {
452-
return pendingTx.txs.map(({ bytes }) => toBase64(bytes));
460+
return pendingTx.txs.map((tx) => ({
461+
tx: toBase64(tx.bytes),
462+
shieldedHash:
463+
tx.signingData[0].shieldedHash ?
464+
toBase64(tx.signingData[0].shieldedHash)
465+
: undefined,
466+
}));
453467
}
454468
}
455469

‎apps/extension/src/background/keyring/keyring.ts‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -659,11 +659,12 @@ export class KeyRing {
659659
}
660660

661661
public async updateDefaultAccount(address: string): Promise<void> {
662-
const account = await this.queryAccountDetails(address);
663-
if (!account) {
662+
const accounts = await this.queryAccountDetails(address);
663+
if (!accounts) {
664664
throw new Error(`Account with address ${address} not found.`);
665665
}
666-
const { id, type } = account;
666+
const [transactionAccount] = accounts;
667+
const { id, type } = transactionAccount;
667668
await this.setActiveAccount(id, type);
668669
}
669670

@@ -919,7 +920,7 @@ export class KeyRing {
919920

920921
async queryAccountDetails(
921922
address: string
922-
): Promise<DerivedAccount | undefined> {
923+
): Promise<[DerivedAccount, DerivedAccount | undefined] | undefined> {
923924
const disposableKey = await this.localStorage.getDisposableSigner(address);
924925

925926
const account = await this.vaultStorage.findOneOrFail(
@@ -931,7 +932,13 @@ export class KeyRing {
931932
if (!account) {
932933
return;
933934
}
934-
return account.public;
935+
const shieldedAccount = await this.vaultStorage.findOne(
936+
KeyStore,
937+
"parentId",
938+
account.public.id
939+
);
940+
941+
return [account.public, shieldedAccount?.public];
935942
}
936943

937944
async genDisposableSigner(): Promise<

‎apps/extension/src/background/keyring/messages.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ export class DeleteAccountMsg extends Message<
394394
}
395395

396396
export class QueryAccountDetailsMsg extends Message<
397-
DerivedAccount | undefined
397+
[DerivedAccount, DerivedAccount | undefined] | undefined
398398
> {
399399
public static type(): MessageType {
400400
return MessageType.QueryAccountDetails;

‎apps/extension/src/background/keyring/service.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ export class KeyRingService {
254254

255255
async queryAccountDetails(
256256
address: string
257-
): Promise<DerivedAccount | undefined> {
257+
): Promise<[DerivedAccount, DerivedAccount | undefined] | undefined> {
258258
if (await this.vaultService.isLocked()) {
259259
throw new Error(ApprovalErrors.KeychainLocked());
260260
}

‎packages/sdk/src/masp/masp.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,20 @@ export class Masp {
99
*/
1010
constructor(protected readonly sdk: SdkWasm) {}
1111

12+
/**
13+
* Get spend notes descriptor map
14+
* @async
15+
* @param tx - tx bytes
16+
* @param shieldedHash - bytes of the shielded hash
17+
* @returns - Promise resolving to an array of numbers representing the descriptor map
18+
*/
19+
async getDescriptorMap(
20+
tx: Uint8Array,
21+
shieldedHash: Uint8Array
22+
): Promise<number[]> {
23+
return await this.sdk.get_descriptor_map(tx, shieldedHash);
24+
}
25+
1226
/**
1327
* Check if SDK has MASP parameters loaded
1428
* @async

‎packages/sdk/src/tx/tx.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -370,15 +370,15 @@ export class Tx {
370370
* Append signature for transactions signed by Ledger Hardware Wallet
371371
* @param txBytes - bytes of the transaction
372372
* @param signingData - signing data
373-
* @param signature - masp signature
373+
* @param signatures - masp signature
374374
* @returns transaction bytes with signature appended
375375
*/
376-
appendMaspSignature(
376+
appendMaspSignatures(
377377
txBytes: Uint8Array,
378378
signingData: Uint8Array[],
379-
signature: Uint8Array
379+
signatures: Uint8Array[]
380380
): Uint8Array {
381-
return this.sdk.sign_masp_ledger(txBytes, signingData, signature);
381+
return this.sdk.sign_masp_ledger(txBytes, signingData, signatures);
382382
}
383383

384384
/**

0 commit comments

Comments
 (0)