From 86ea7e3875248b046995dc749c6b98cf9ba94d91 Mon Sep 17 00:00:00 2001 From: turbocrime <134443988+turbocrime@users.noreply.github.com> Date: Wed, 20 Mar 2024 16:26:02 -0700 Subject: [PATCH] generate tx view in slice (#804) --- apps/extension/src/approve-transaction.ts | 10 +- apps/extension/src/message/popup.ts | 3 - apps/extension/src/state/tx-approval.ts | 35 +-- packages/perspective/plan/index.test.ts | 83 ++++---- packages/perspective/plan/index.ts | 16 +- .../perspective/plan/view-action-plan.test.ts | 201 +++++++++--------- packages/perspective/plan/view-action-plan.ts | 63 +++--- packages/router/src/ctx/approver.ts | 2 - .../router/src/grpc/custody/authorize.test.ts | 17 -- packages/router/src/grpc/custody/authorize.ts | 45 +--- 10 files changed, 211 insertions(+), 264 deletions(-) diff --git a/apps/extension/src/approve-transaction.ts b/apps/extension/src/approve-transaction.ts index d99db14fcb..909fdebceb 100644 --- a/apps/extension/src/approve-transaction.ts +++ b/apps/extension/src/approve-transaction.ts @@ -1,4 +1,3 @@ -import { TransactionView } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/transaction/v1/transaction_pb'; import { AuthorizeRequest } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/custody/v1/custody_pb'; import { PartialMessage } from '@bufbuild/protobuf'; import type { Jsonified } from '@penumbra-zone/types/src/jsonified'; @@ -7,10 +6,8 @@ import { popup } from './popup'; export const approveTransaction = async ( partialAuthorizeRequest: PartialMessage, - partialTransactionView: PartialMessage, ) => { const authorizeRequest = new AuthorizeRequest(partialAuthorizeRequest); - const transactionView = new TransactionView(partialTransactionView); const popupResponse = await popup({ type: PopupType.TxApproval, @@ -18,18 +15,13 @@ export const approveTransaction = async ( authorizeRequest: new AuthorizeRequest( authorizeRequest, ).toJson() as Jsonified, - transactionView: new TransactionView(transactionView).toJson() as Jsonified, }, }); if (popupResponse) { const resAuthorizeRequest = AuthorizeRequest.fromJson(popupResponse.authorizeRequest); - const resTransactionView = TransactionView.fromJson(popupResponse.transactionView); - if ( - !authorizeRequest.equals(resAuthorizeRequest) || - !transactionView.equals(resTransactionView) - ) + if (!authorizeRequest.equals(resAuthorizeRequest)) throw new Error('Invalid response from popup'); } diff --git a/apps/extension/src/message/popup.ts b/apps/extension/src/message/popup.ts index 3a985dafe4..6ccd9fbc3f 100644 --- a/apps/extension/src/message/popup.ts +++ b/apps/extension/src/message/popup.ts @@ -1,4 +1,3 @@ -import type { TransactionView } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/transaction/v1/transaction_pb'; import type { AuthorizeRequest } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/custody/v1/custody_pb'; import type { InternalMessage, @@ -28,11 +27,9 @@ export type TxApproval = InternalMessage< PopupType.TxApproval, { authorizeRequest: Jsonified; - transactionView: Jsonified; }, null | { authorizeRequest: Jsonified; - transactionView: Jsonified; choice: UserChoice; } >; diff --git a/apps/extension/src/state/tx-approval.ts b/apps/extension/src/state/tx-approval.ts index a2c1db305c..8bd69e3430 100644 --- a/apps/extension/src/state/tx-approval.ts +++ b/apps/extension/src/state/tx-approval.ts @@ -1,7 +1,10 @@ import { AuthorizeRequest } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/custody/v1/custody_pb'; import { AllSlices, SliceCreator } from '.'; import { PopupType, TxApproval } from '../message/popup'; -import { TransactionView } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/transaction/v1/transaction_pb'; +import { + TransactionPlan, + TransactionView, +} from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/transaction/v1/transaction_pb'; import { viewClient } from '../clients'; import { ConnectError } from '@connectrpc/connect'; import { errorToJson } from '@connectrpc/connect/protocol-connect'; @@ -19,6 +22,12 @@ import { asPublicTransactionView, asReceiverTransactionView, } from '@penumbra-zone/perspective/translators'; +import { localExtStorage } from '@penumbra-zone/storage'; +import { + AssetId, + Metadata, +} from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/asset/v1/asset_pb'; +import { viewTransactionPlan } from '@penumbra-zone/perspective/plan'; export interface TxApprovalSlice { /** @@ -49,16 +58,22 @@ export interface TxApprovalSlice { } export const createTxApprovalSlice = (): SliceCreator => (set, get) => ({ - acceptRequest: async ( - { request: { authorizeRequest: authReqJson, transactionView: txViewJson } }, - responder, - ) => { + acceptRequest: async ({ request: { authorizeRequest: authReqJson } }, responder) => { const existing = get().txApproval; - if (existing.authorizeRequest ?? existing.transactionView ?? existing.responder) - throw new Error('Another request is still pending'); + if (existing.responder) throw new Error('Another request is still pending'); const authorizeRequest = AuthorizeRequest.fromJson(authReqJson); - const transactionView = TransactionView.fromJson(txViewJson); + + const getMetadata = async (assetId: AssetId) => { + const { denomMetadata } = await viewClient.assetMetadataById({ assetId }); + return denomMetadata ?? new Metadata(); + }; + + const transactionView = await viewTransactionPlan( + authorizeRequest.plan ?? new TransactionPlan(), + getMetadata, + (await localExtStorage.get('wallets'))[0]?.fullViewingKey ?? '', + ); // pregenerate views from various perspectives. // TODO: should this be done in the component? @@ -106,9 +121,6 @@ export const createTxApprovalSlice = (): SliceCreator => (set, throw new Error('Missing response data'); // zustand doesn't like jsonvalue so stringify - const transactionView = TransactionView.fromJsonString( - transactionViewString, - ).toJson() as Jsonified; const authorizeRequest = AuthorizeRequest.fromJsonString( authorizeRequestString, ).toJson() as Jsonified; @@ -117,7 +129,6 @@ export const createTxApprovalSlice = (): SliceCreator => (set, type: PopupType.TxApproval, data: { choice, - transactionView, authorizeRequest, }, }); diff --git a/packages/perspective/plan/index.test.ts b/packages/perspective/plan/index.test.ts index da2d4f60ab..e6919bec21 100644 --- a/packages/perspective/plan/index.test.ts +++ b/packages/perspective/plan/index.test.ts @@ -1,11 +1,12 @@ import { Address } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/keys/v1/keys_pb'; -import { describe, expect, test } from 'vitest'; +import { describe, expect, test, vi } from 'vitest'; import { viewTransactionPlan } from '.'; import { MemoView_Visible, TransactionPlan, } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/transaction/v1/transaction_pb'; import { bech32ToAddress } from '@penumbra-zone/bech32'; +import { Metadata } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/asset/v1/asset_pb'; describe('viewTransactionPlan()', () => { const returnAddressAsBech32 = @@ -13,6 +14,7 @@ describe('viewTransactionPlan()', () => { const returnAddress = new Address({ inner: bech32ToAddress(returnAddressAsBech32) }); const chainId = 'testnet'; const expiryHeight = 100n; + const metadataByAssetId = vi.fn(() => Promise.resolve(new Metadata())); const mockFvk = 'penumbrafullviewingkey1vzfytwlvq067g2kz095vn7sgcft47hga40atrg5zu2crskm6tyyjysm28qg5nth2fqmdf5n0q530jreumjlsrcxjwtfv6zdmfpe5kqsa5lg09'; @@ -30,8 +32,8 @@ describe('viewTransactionPlan()', () => { }, }); - test('includes the return address if it exists', () => { - const txnView = viewTransactionPlan(validTxnPlan, {}, mockFvk); + test('includes the return address if it exists', async () => { + const txnView = await viewTransactionPlan(validTxnPlan, metadataByAssetId, mockFvk); const memoViewValue = txnView.bodyView!.memoView!.memoView.value! as MemoView_Visible; expect( @@ -39,31 +41,34 @@ describe('viewTransactionPlan()', () => { ).toBe(true); }); - test('leaves out the return address when it does not exist', () => { - const txnPlan = new TransactionPlan({ - transactionParameters: { - fee: { - amount: { - hi: 1n, - lo: 0n, + test('leaves out the return address when it does not exist', async () => { + const view = viewTransactionPlan( + new TransactionPlan({ + transactionParameters: { + fee: { + amount: { + hi: 1n, + lo: 0n, + }, }, }, - }, - }); - const txnView = viewTransactionPlan(txnPlan, {}, mockFvk); - const memoViewValue = txnView.bodyView!.memoView!.memoView.value! as MemoView_Visible; - - expect(memoViewValue.plaintext?.returnAddress).toBeUndefined(); + }), + metadataByAssetId, + mockFvk, + ); + await expect(view).resolves.toHaveProperty('bodyView.memoView.memoView.value.plaintext.text'); + await expect(view).resolves.not.toHaveProperty( + 'bodyView.memoView.memoView.value.plaintext.returnAddress', + ); }); - test('includes the fee', () => { - expect( - viewTransactionPlan(validTxnPlan, {}, mockFvk).bodyView!.transactionParameters!.fee, - ).toBe(validTxnPlan.transactionParameters!.fee); - }); + test('includes the fee', async () => + expect(viewTransactionPlan(validTxnPlan, metadataByAssetId, mockFvk)).resolves.toMatchObject({ + bodyView: { transactionParameters: { fee: validTxnPlan.transactionParameters!.fee } }, + })); - test('throws when there is no fee', () => { - expect(() => + test('throws when there is no fee', () => + expect( viewTransactionPlan( new TransactionPlan({ memo: { @@ -77,24 +82,24 @@ describe('viewTransactionPlan()', () => { expiryHeight, }, }), - {}, + metadataByAssetId, mockFvk, ), - ).toThrow('No fee found in transaction plan'); - }); + ).rejects.toThrow('No fee found in transaction plan')); - test('includes the memo', () => { - const txnView = viewTransactionPlan(validTxnPlan, {}, mockFvk); - const memoViewValue = txnView.bodyView!.memoView!.memoView.value! as MemoView_Visible; - - expect(memoViewValue.plaintext!.text).toBe('Memo text here'); - }); + test('includes the memo', async () => + expect(viewTransactionPlan(validTxnPlan, metadataByAssetId, mockFvk)).resolves.toMatchObject({ + bodyView: { memoView: { memoView: { value: { plaintext: { text: 'Memo text here' } } } } }, + })); - test('includes the transaction parameters', () => { - expect(viewTransactionPlan(validTxnPlan, {}, mockFvk).bodyView!.transactionParameters).toEqual({ - fee: validTxnPlan.transactionParameters!.fee, - chainId, - expiryHeight, - }); - }); + test('includes the transaction parameters', () => + expect(viewTransactionPlan(validTxnPlan, metadataByAssetId, mockFvk)).resolves.toMatchObject({ + bodyView: { + transactionParameters: { + fee: validTxnPlan.transactionParameters!.fee, + chainId, + expiryHeight, + }, + }, + })); }); diff --git a/packages/perspective/plan/index.ts b/packages/perspective/plan/index.ts index e25906e271..eae8374be1 100644 --- a/packages/perspective/plan/index.ts +++ b/packages/perspective/plan/index.ts @@ -1,11 +1,13 @@ -import { Metadata } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/asset/v1/asset_pb'; +import { + AssetId, + Metadata, +} from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/asset/v1/asset_pb'; import { getAddressView } from './get-address-view'; import { TransactionPlan, TransactionView, } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/transaction/v1/transaction_pb'; import { viewActionPlan } from './view-action-plan'; -import type { Jsonified } from '@penumbra-zone/types/src/jsonified'; /** * Given a `TransactionPlan`, returns a `TransactionView` that can be passed to @@ -17,18 +19,20 @@ import type { Jsonified } from '@penumbra-zone/types/src/jsonified'; * properties. Its main purpose is to be able to render the transaction plan, * not to be exhaustive. */ -export const viewTransactionPlan = ( +export const viewTransactionPlan = async ( txPlan: TransactionPlan, - metadataByAssetId: Record>, + metadataByAssetId: (id: AssetId) => Promise, fullViewingKey: string, -): TransactionView => { +): Promise => { const returnAddress = txPlan.memo?.plaintext?.returnAddress; const transactionParameters = txPlan.transactionParameters; if (!transactionParameters?.fee) throw new Error('No fee found in transaction plan'); return new TransactionView({ bodyView: { - actionViews: txPlan.actions.map(viewActionPlan(metadataByAssetId, fullViewingKey)), + actionViews: await Promise.all( + txPlan.actions.map(viewActionPlan(metadataByAssetId, fullViewingKey)), + ), memoView: { memoView: { case: 'visible', diff --git a/packages/perspective/plan/view-action-plan.test.ts b/packages/perspective/plan/view-action-plan.test.ts index 0efcd097b9..96a9042f03 100644 --- a/packages/perspective/plan/view-action-plan.test.ts +++ b/packages/perspective/plan/view-action-plan.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, test } from 'vitest'; +import { describe, expect, test, vi } from 'vitest'; import { viewActionPlan } from './view-action-plan'; import { ActionPlan, @@ -13,31 +13,25 @@ import { import { AssetId, Metadata, - ValueView_KnownAssetId, } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/asset/v1/asset_pb'; import { Address } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/keys/v1/keys_pb'; import { - BatchSwapOutputData, SwapPlaintext, + BatchSwapOutputData, } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/component/dex/v1/dex_pb'; -import { JsonObject } from '@bufbuild/protobuf'; import { Delegate, Undelegate, } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/component/stake/v1/stake_pb'; -import { bech32AssetId, bech32ToAddress } from '@penumbra-zone/bech32'; -import type { Jsonified } from '@penumbra-zone/types/src/jsonified'; +import { bech32ToAddress } from '@penumbra-zone/bech32'; describe('viewActionPlan()', () => { const addressAsBech32 = 'penumbra147mfall0zr6am5r45qkwht7xqqrdsp50czde7empv7yq2nk3z8yyfh9k9520ddgswkmzar22vhz9dwtuem7uxw0qytfpv7lk3q9dp8ccaw2fn5c838rfackazmgf3ahh09cxmz'; const address = new Address({ inner: bech32ToAddress(addressAsBech32) }); const assetId = new AssetId({ inner: new Uint8Array() }); - const assetIdAsString = bech32AssetId(assetId); const metadata = new Metadata({ penumbraAssetId: assetId }); - const metadataByAssetId = { - [assetIdAsString]: metadata.toJson() as Jsonified, - }; + const metadataByAssetId = vi.fn(() => Promise.resolve(metadata)); const mockFvk = 'penumbrafullviewingkey1vzfytwlvq067g2kz095vn7sgcft47hga40atrg5zu2crskm6tyyjysm28qg5nth2fqmdf5n0q530jreumjlsrcxjwtfv6zdmfpe5kqsa5lg09'; @@ -57,7 +51,7 @@ describe('viewActionPlan()', () => { }, }); - test('throws if the address is missing', () => { + test('throws if the address is missing', async () => { const actionPlan = new ActionPlan({ action: { case: 'spend', @@ -67,11 +61,13 @@ describe('viewActionPlan()', () => { }, }); - expect(() => viewActionPlan({}, mockFvk)(actionPlan)).toThrow('No address in spend plan'); + await expect(viewActionPlan(metadataByAssetId, mockFvk)(actionPlan)).rejects.toThrow( + 'No address in spend plan', + ); }); - test('includes the amount', () => { - const actionView = viewActionPlan(metadataByAssetId, mockFvk)(validSpendActionPlan); + test('includes the amount', async () => { + const actionView = await viewActionPlan(metadataByAssetId, mockFvk)(validSpendActionPlan); const spendView = actionView.actionView.value as SpendView; const spendViewVisible = spendView.spendView.value as SpendView_Visible; @@ -81,7 +77,7 @@ describe('viewActionPlan()', () => { }); }); - test('throws if the amount is missing', () => { + test('throws if the amount is missing', async () => { const actionPlan = new ActionPlan({ action: { case: 'spend', @@ -93,19 +89,20 @@ describe('viewActionPlan()', () => { }, }); - expect(() => viewActionPlan({}, mockFvk)(actionPlan)).toThrow('No value in note'); + await expect(viewActionPlan(metadataByAssetId, mockFvk)(actionPlan)).rejects.toThrow( + 'No value in note', + ); }); - test('includes the denom metadata', () => { - const actionView = viewActionPlan(metadataByAssetId, mockFvk)(validSpendActionPlan); - const spendView = actionView.actionView.value as SpendView; - const spendViewVisible = spendView.spendView.value as SpendView_Visible; - const valueView = spendViewVisible.note!.value?.valueView.value as ValueView_KnownAssetId; - - expect(valueView.metadata?.toJson()).toEqual(metadata.toJson()); - }); + test('includes the denom metadata', () => + expect( + viewActionPlan(metadataByAssetId, mockFvk)(validSpendActionPlan), + ).resolves.toHaveProperty( + 'actionView.value.spendView.value.note.value.valueView.value.metadata', + metadata, + )); - test('throws if the asset ID is missing', () => { + test('throws if the asset ID is missing', async () => { const actionPlan = new ActionPlan({ action: { case: 'spend', @@ -118,7 +115,9 @@ describe('viewActionPlan()', () => { }, }); - expect(() => viewActionPlan({}, mockFvk)(actionPlan)).toThrow('No asset ID in value'); + await expect(viewActionPlan(metadataByAssetId, mockFvk)(actionPlan)).rejects.toThrow( + 'No asset ID in value', + ); }); }); @@ -139,15 +138,15 @@ describe('viewActionPlan()', () => { }, }); - test('includes the destAddress', () => { - const actionView = viewActionPlan(metadataByAssetId, mockFvk)(validOutputActionPlan); + test('includes the destAddress', async () => { + const actionView = await viewActionPlan(metadataByAssetId, mockFvk)(validOutputActionPlan); const outputView = actionView.actionView.value as OutputView; const outputViewVisible = outputView.outputView.value as OutputView_Visible; expect(outputViewVisible.note?.address?.addressView.value?.address).toEqual(destAddress); }); - test('throws if the destAddress is missing', () => { + test('throws if the destAddress is missing', async () => { const actionPlan = new ActionPlan({ action: { case: 'output', @@ -160,13 +159,13 @@ describe('viewActionPlan()', () => { }, }); - expect(() => viewActionPlan({}, mockFvk)(actionPlan)).toThrow( + await expect(viewActionPlan(metadataByAssetId, mockFvk)(actionPlan)).rejects.toThrow( 'No destAddress in output plan', ); }); - test('includes the amount', () => { - const actionView = viewActionPlan(metadataByAssetId, mockFvk)(validOutputActionPlan); + test('includes the amount', async () => { + const actionView = await viewActionPlan(metadataByAssetId, mockFvk)(validOutputActionPlan); const outputView = actionView.actionView.value as OutputView; const outputViewVisible = outputView.outputView.value as OutputView_Visible; @@ -176,7 +175,7 @@ describe('viewActionPlan()', () => { }); }); - test('throws if the amount is missing', () => { + test('throws if the amount is missing', async () => { const actionPlan = new ActionPlan({ action: { case: 'output', @@ -186,19 +185,20 @@ describe('viewActionPlan()', () => { }, }); - expect(() => viewActionPlan({}, mockFvk)(actionPlan)).toThrow('No value to view'); + await expect(viewActionPlan(metadataByAssetId, mockFvk)(actionPlan)).rejects.toThrow( + 'No value to view', + ); }); - test('includes the denom metadata', () => { - const actionView = viewActionPlan(metadataByAssetId, mockFvk)(validOutputActionPlan); - const outputView = actionView.actionView.value as OutputView; - const outputViewVisible = outputView.outputView.value as OutputView_Visible; - const valueView = outputViewVisible.note!.value?.valueView.value as ValueView_KnownAssetId; - - expect(valueView.metadata?.toJson()).toEqual(metadata.toJson()); - }); + test('includes the denom metadata', () => + expect( + viewActionPlan(metadataByAssetId, mockFvk)(validOutputActionPlan), + ).resolves.toHaveProperty( + 'actionView.value.outputView.value.note.value.valueView.value.metadata', + metadata, + )); - test('throws if the asset ID is missing', () => { + test('throws if the asset ID is missing', async () => { const actionPlan = new ActionPlan({ action: { case: 'output', @@ -211,12 +211,14 @@ describe('viewActionPlan()', () => { }, }); - expect(() => viewActionPlan({}, mockFvk)(actionPlan)).toThrow('No asset ID in value'); + await expect(viewActionPlan(metadataByAssetId, mockFvk)(actionPlan)).rejects.toThrow( + 'No asset ID in value', + ); }); }); describe('`swap` action', () => { - test('returns an action view with the `swap` case', () => { + test('returns an action view with the `swap` case', async () => { const swapPlaintext = new SwapPlaintext({ claimAddress: { inner: new Uint8Array([0, 1, 2, 3]), @@ -261,7 +263,7 @@ describe('viewActionPlan()', () => { }, }); - const actionView = viewActionPlan({}, mockFvk)(actionPlan); + const actionView = viewActionPlan(metadataByAssetId, mockFvk)(actionPlan); const expected = new ActionView({ actionView: { @@ -284,20 +286,17 @@ describe('viewActionPlan()', () => { }, }); - expect(actionView.equals(expected)).toBe(true); + await expect(actionView).resolves.toEqual(expected); }); }); describe('`swapClaim` action', () => { - test('returns an action view with the `swapClaim` case', () => { + test('returns an action view with the `swapClaim` case', async () => { const asset1Id = new AssetId({ inner: new Uint8Array([0, 1, 2, 3]) }); const asset2Id = new AssetId({ inner: new Uint8Array([4, 5, 6, 7]) }); - const asset1IdAsString = bech32AssetId(asset1Id); - const asset2IdAsString = bech32AssetId(asset2Id); - const metadataByAssetId = { - [asset1IdAsString]: new Metadata({ penumbraAssetId: asset1Id }).toJson() as JsonObject, - [asset2IdAsString]: new Metadata({ penumbraAssetId: asset2Id }).toJson() as JsonObject, - }; + const metadataByAssetId = vi.fn((id: AssetId) => + Promise.resolve(new Metadata({ penumbraAssetId: id })), + ); const swapPlaintext = new SwapPlaintext({ claimAddress: address, @@ -357,7 +356,7 @@ describe('viewActionPlan()', () => { }, }); - const actionView = viewActionPlan(metadataByAssetId, mockFvk)(actionPlan); + const actionView = await viewActionPlan(metadataByAssetId, mockFvk)(actionPlan); const expected = new ActionView({ actionView: { @@ -381,7 +380,7 @@ describe('viewActionPlan()', () => { case: 'knownAssetId', value: { amount: outputData.lambda1, - metadata: Metadata.fromJson(metadataByAssetId[asset1IdAsString]!), + metadata: await metadataByAssetId(asset1Id), }, }, }, @@ -401,7 +400,7 @@ describe('viewActionPlan()', () => { case: 'knownAssetId', value: { amount: outputData.lambda2, - metadata: Metadata.fromJson(metadataByAssetId[asset2IdAsString]!), + metadata: await metadataByAssetId(asset2Id), }, }, }, @@ -427,7 +426,7 @@ describe('viewActionPlan()', () => { }); describe('`withdrawal` action', () => { - test('returns an action view with the `ics20Withdrawal` case as-is', () => { + test('returns an action view with the `ics20Withdrawal` case as-is', async () => { const actionPlan = new ActionPlan({ action: { case: 'ics20Withdrawal', @@ -435,23 +434,21 @@ describe('viewActionPlan()', () => { }, }); - const actionView = viewActionPlan({}, mockFvk)(actionPlan); + const actionView = viewActionPlan(metadataByAssetId, mockFvk)(actionPlan); - expect( - actionView.equals( - new ActionView({ - actionView: { - case: 'ics20Withdrawal', - value: { amount: { hi: 1n, lo: 0n } }, - }, - }), - ), - ).toBe(true); + await expect(actionView).resolves.toEqual( + new ActionView({ + actionView: { + case: 'ics20Withdrawal', + value: { amount: { hi: 1n, lo: 0n } }, + }, + }), + ); }); }); describe('`delegate` action', () => { - test('returns an action view with the action as-is', () => { + test('returns an action view with the action as-is', async () => { const delegate = new Delegate({ epochIndex: 0n, delegationAmount: { hi: 123n, lo: 456n }, @@ -463,23 +460,21 @@ describe('viewActionPlan()', () => { }, }); - const actionView = viewActionPlan({}, mockFvk)(actionPlan); + const actionView = viewActionPlan(metadataByAssetId, mockFvk)(actionPlan); - expect( - actionView.equals( - new ActionView({ - actionView: { - case: 'delegate', - value: delegate, - }, - }), - ), - ).toBe(true); + await expect(actionView).resolves.toEqual( + new ActionView({ + actionView: { + case: 'delegate', + value: delegate, + }, + }), + ); }); }); describe('`undelegate` action', () => { - test('returns an action view with the action as-is', () => { + test('returns an action view with the action as-is', async () => { const undelegate = new Undelegate({ startEpochIndex: 0n, delegationAmount: { hi: 123n, lo: 456n }, @@ -491,23 +486,21 @@ describe('viewActionPlan()', () => { }, }); - const actionView = viewActionPlan({}, mockFvk)(actionPlan); + const actionView = viewActionPlan(metadataByAssetId, mockFvk)(actionPlan); - expect( - actionView.equals( - new ActionView({ - actionView: { - case: 'undelegate', - value: undelegate, - }, - }), - ), - ).toBe(true); + await expect(actionView).resolves.toEqual( + new ActionView({ + actionView: { + case: 'undelegate', + value: undelegate, + }, + }), + ); }); }); describe('all other action cases', () => { - test('returns an action view with the case but no value', () => { + test('returns an action view with the case but no value', async () => { const actionPlan = new ActionPlan({ action: { case: 'proposalSubmit', @@ -515,18 +508,16 @@ describe('viewActionPlan()', () => { }, }); - const actionView = viewActionPlan({}, mockFvk)(actionPlan); + const actionView = viewActionPlan(metadataByAssetId, mockFvk)(actionPlan); - expect( - actionView.equals( - new ActionView({ - actionView: { - case: 'proposalSubmit', - value: {}, - }, - }), - ), - ).toBe(true); + await expect(actionView).resolves.toEqual( + new ActionView({ + actionView: { + case: 'proposalSubmit', + value: {}, + }, + }), + ); }); }); }); diff --git a/packages/perspective/plan/view-action-plan.ts b/packages/perspective/plan/view-action-plan.ts index 2f30c71ed3..9cae2d7570 100644 --- a/packages/perspective/plan/view-action-plan.ts +++ b/packages/perspective/plan/view-action-plan.ts @@ -3,6 +3,7 @@ import { ActionView, } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/transaction/v1/transaction_pb'; import { + AssetId, Metadata, Value, ValueView, @@ -22,33 +23,29 @@ import { SwapPlan, SwapView, } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/component/dex/v1/dex_pb'; -import { bech32AssetId } from '@penumbra-zone/bech32'; -import type { Jsonified } from '@penumbra-zone/types/src/jsonified'; -const getValueView = ( +const getValueView = async ( value: Value | undefined, - denomMetadataByAssetId: Record>, -): ValueView => { + denomMetadataByAssetId: (id: AssetId) => Promise, +): Promise => { if (!value) throw new Error('No value to view'); if (!value.assetId) throw new Error('No asset ID in value'); if (!value.amount) throw new Error('No amount in value'); - const denomMetadata = denomMetadataByAssetId[bech32AssetId(value.assetId)]; - return new ValueView({ valueView: { case: 'knownAssetId', value: { amount: value.amount, - metadata: denomMetadata ? Metadata.fromJson(denomMetadata) : undefined, + metadata: await denomMetadataByAssetId(value.assetId), }, }, }); }; -const getNoteView = ( +const getNoteView = async ( note: Note | undefined, - denomMetadataByAssetId: Record>, + denomMetadataByAssetId: (id: AssetId) => Promise, fullViewingKey: string, ) => { if (!note) throw new Error('No note to view'); @@ -57,32 +54,32 @@ const getNoteView = ( return new NoteView({ address: getAddressView(note.address, fullViewingKey), - value: getValueView(note.value, denomMetadataByAssetId), + value: await getValueView(note.value, denomMetadataByAssetId), }); }; -const getSpendView = ( +const getSpendView = async ( spendPlan: SpendPlan, - denomMetadataByAssetId: Record>, + denomMetadataByAssetId: (id: AssetId) => Promise, fullViewingKey: string, -): SpendView => { +): Promise => { if (!spendPlan.note?.address) throw new Error('No address in spend plan'); return new SpendView({ spendView: { case: 'visible', value: { - note: getNoteView(spendPlan.note, denomMetadataByAssetId, fullViewingKey), + note: await getNoteView(spendPlan.note, denomMetadataByAssetId, fullViewingKey), }, }, }); }; -const getOutputView = ( +const getOutputView = async ( outputPlan: OutputPlan, - denomMetadataByAssetId: Record>, + denomMetadataByAssetId: (id: AssetId) => Promise, fullViewingKey: string, -): OutputView => { +): Promise => { if (!outputPlan.destAddress) throw new Error('No destAddress in output plan'); return new OutputView({ @@ -91,7 +88,7 @@ const getOutputView = ( value: { note: { - value: getValueView(outputPlan.value, denomMetadataByAssetId), + value: await getValueView(outputPlan.value, denomMetadataByAssetId), address: getAddressView(outputPlan.destAddress, fullViewingKey), }, }, @@ -117,11 +114,11 @@ const getSwapView = (swapPlan: SwapPlan): SwapView => { }); }; -const getSwapClaimView = ( +const getSwapClaimView = async ( swapClaimPlan: SwapClaimPlan, - denomMetadataByAssetId: Record>, + denomMetadataByAssetId: (id: AssetId) => Promise, fullViewingKey: string, -): SwapClaimView => { +): Promise => { return new SwapClaimView({ swapClaimView: { case: 'visible', @@ -131,7 +128,7 @@ const getSwapClaimView = ( ? getAddressView(swapClaimPlan.swapPlaintext.claimAddress, fullViewingKey) : undefined, value: swapClaimPlan.outputData?.lambda1 - ? getValueView( + ? await getValueView( new Value({ amount: swapClaimPlan.outputData.lambda1, assetId: swapClaimPlan.outputData.tradingPair?.asset1, @@ -145,7 +142,7 @@ const getSwapClaimView = ( ? getAddressView(swapClaimPlan.swapPlaintext.claimAddress, fullViewingKey) : undefined, value: swapClaimPlan.outputData?.lambda2 - ? getValueView( + ? await getValueView( new Value({ amount: swapClaimPlan.outputData.lambda2, assetId: swapClaimPlan.outputData.tradingPair?.asset2, @@ -167,21 +164,29 @@ const getSwapClaimView = ( }; export const viewActionPlan = - (denomMetadataByAssetId: Record>, fullViewingKey: string) => - (actionPlan: ActionPlan): ActionView => { + (denomMetadataByAssetId: (id: AssetId) => Promise, fullViewingKey: string) => + async (actionPlan: ActionPlan): Promise => { switch (actionPlan.action.case) { case 'spend': return new ActionView({ actionView: { case: 'spend', - value: getSpendView(actionPlan.action.value, denomMetadataByAssetId, fullViewingKey), + value: await getSpendView( + actionPlan.action.value, + denomMetadataByAssetId, + fullViewingKey, + ), }, }); case 'output': return new ActionView({ actionView: { case: 'output', - value: getOutputView(actionPlan.action.value, denomMetadataByAssetId, fullViewingKey), + value: await getOutputView( + actionPlan.action.value, + denomMetadataByAssetId, + fullViewingKey, + ), }, }); case 'swap': @@ -195,7 +200,7 @@ export const viewActionPlan = return new ActionView({ actionView: { case: 'swapClaim', - value: getSwapClaimView( + value: await getSwapClaimView( actionPlan.action.value, denomMetadataByAssetId, fullViewingKey, diff --git a/packages/router/src/ctx/approver.ts b/packages/router/src/ctx/approver.ts index 9d8890a907..c30ced0d20 100644 --- a/packages/router/src/ctx/approver.ts +++ b/packages/router/src/ctx/approver.ts @@ -1,12 +1,10 @@ import { AuthorizeRequest } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/custody/v1/custody_pb'; -import { TransactionView } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/transaction/v1/transaction_pb'; import { createContextKey } from '@connectrpc/connect'; import { PartialMessage } from '@bufbuild/protobuf'; import { UserChoice } from '@penumbra-zone/types/src/user-choice'; export type TxApprovalFn = ( authorizeRequest: PartialMessage, - transactionView: PartialMessage, ) => Promise; export const approverCtx = createContextKey(undefined); diff --git a/packages/router/src/grpc/custody/authorize.test.ts b/packages/router/src/grpc/custody/authorize.test.ts index c82ff9c471..267c15fc44 100644 --- a/packages/router/src/grpc/custody/authorize.test.ts +++ b/packages/router/src/grpc/custody/authorize.test.ts @@ -131,23 +131,6 @@ describe('Authorize request handler', () => { await expect(authorize(req, mockCtx)).rejects.toThrow('User must login to extension'); }); - test('should fail if local context not include FVK', async () => { - mockExtLocalCtx.get.mockImplementation(() => { - return Promise.resolve([ - { - custody: { - encryptedSeedPhrase: { - cipherText: '1MUyDW2GHSeZYVF4f=', - nonce: 'MUyDW2GHSeZYVF4f', - }, - }, - fullViewingKey: undefined, - }, - ]); - }); - await expect(authorize(req, mockCtx)).rejects.toThrow('Unable to get full viewing key'); - }); - test('should fail if incorrect password is used', async () => { mockExtSessionCtx.get.mockImplementation(() => { return Promise.resolve({ diff --git a/packages/router/src/grpc/custody/authorize.ts b/packages/router/src/grpc/custody/authorize.ts index 49b53685b7..a97828093b 100644 --- a/packages/router/src/grpc/custody/authorize.ts +++ b/packages/router/src/grpc/custody/authorize.ts @@ -1,39 +1,11 @@ import type { Impl } from '.'; -import { approverCtx, extLocalCtx, extSessionCtx, servicesCtx } from '../../ctx'; +import { approverCtx, extLocalCtx, extSessionCtx } from '../../ctx'; import { authorizePlan, generateSpendKey } from '@penumbra-zone/wasm'; import { Key } from '@penumbra-zone/crypto-web'; -import { Code, ConnectError, HandlerContext } from '@connectrpc/connect'; -import { Metadata } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/asset/v1/asset_pb'; -import { viewTransactionPlan } from '@penumbra-zone/perspective/plan'; -import { bech32AssetId } from '@penumbra-zone/bech32'; -import type { Jsonified } from '@penumbra-zone/types/src/jsonified'; +import { Code, ConnectError } from '@connectrpc/connect'; import { Box } from '@penumbra-zone/types/src/box'; import { UserChoice } from '@penumbra-zone/types/src/user-choice'; -/** - * @todo As more asset types get used, the amount of asset metadata we store - * will grow. Loading all the asset metadata into memory for the purpose of - * compiling a transaction view may not be sustainable in the long term. - * Eventually, we may want to scan through the transaction plan, extract all the - * asset IDs in it, and then query just those from IndexedDB instead of grabbing - * all of them. - */ -const getMetadataByAssetId = async (ctx: HandlerContext) => { - const services = ctx.values.get(servicesCtx); - const walletServices = await services.getWalletServices(); - - const assetsMetadata: Metadata[] = []; - for await (const metadata of walletServices.indexedDb.iterateAssetsMetadata()) { - assetsMetadata.push(metadata); - } - return assetsMetadata.reduce>>((prev, curr) => { - if (curr.penumbraAssetId) { - prev[bech32AssetId(curr.penumbraAssetId)] = curr.toJson() as Jsonified; - } - return prev; - }, {}); -}; - export const authorize: Impl['authorize'] = async (req, ctx) => { if (!req.plan) throw new ConnectError('No plan included in request', Code.InvalidArgument); @@ -49,26 +21,15 @@ export const authorize: Impl['authorize'] = async (req, ctx) => { const wallets = await local.get('wallets'); const { custody: { encryptedSeedPhrase }, - fullViewingKey, } = wallets[0]!; - if (!fullViewingKey) - throw new ConnectError('Unable to get full viewing key', Code.Unauthenticated); - const key = await Key.fromJson(passwordKey); const decryptedSeedPhrase = await key.unseal(Box.fromJson(encryptedSeedPhrase)); if (!decryptedSeedPhrase) throw new ConnectError('Unable to decrypt seed phrase with password', Code.Unauthenticated); - const denomMetadataByAssetId = await getMetadataByAssetId(ctx); - const transactionViewFromPlan = viewTransactionPlan( - req.plan, - denomMetadataByAssetId, - fullViewingKey, - ); - - const choice = await approveReq(req, transactionViewFromPlan); + const choice = await approveReq(req); if (choice !== UserChoice.Approved) throw new ConnectError('Transaction was not approved', Code.PermissionDenied);