feat: cip30 signTx should send CBOR in sign transactionWitnesserReq

Instead of the only the transaction body.
The witnesser request was missing the auxiliaryData and witness fields.
Plus, it's more reliable to use the CBOR instead of reconstructing it.

Author: mirceahasegan

packages/core/src/Cardano/types/Transaction.ts

@@ -11,6 +11,7 @@ import { PlutusData } from './PlutusData';
 import { ProposalProcedure, VotingProcedures } from './Governance';
 import { RewardAccount } from '../Address';
 import { Script } from './Script';
+import { Serialization } from '../..';
 
 /** transaction hash as hex string */
 export type TransactionId = OpaqueString<'TransactionId'>;
@@ -164,3 +165,6 @@ export type TxBodyWithHash = {
   hash: TransactionId;
   body: TxBody;
 };
+
+export const isTxBodyWithHash = (tx: Serialization.TxCBOR | TxBodyWithHash): tx is TxBodyWithHash =>
+  typeof tx === 'object' && 'hash' in tx && 'body' in tx;

packages/wallet/src/Wallets/BaseWallet.ts

@@ -619,26 +619,29 @@ export class BaseWallet implements ObservableWallet {
   }: FinalizeTxProps): Promise<Cardano.Tx> {
     const knownAddresses = await firstValueFrom(this.addresses$);
     const dRepPublicKey = await this.governance.getPubDRepKey();
+    const emptyWitness = { signatures: new Map() };
+
+    let transaction: Serialization.Transaction;
+    if (Cardano.isTxBodyWithHash(tx)) {
+      // Reconstruct transaction from parts
+      transaction = new Serialization.Transaction(
+        bodyCbor ? Serialization.TransactionBody.fromCbor(bodyCbor) : Serialization.TransactionBody.fromCore(tx.body),
+        Serialization.TransactionWitnessSet.fromCore({ ...emptyWitness, ...witness }),
+        auxiliaryData ? Serialization.AuxiliaryData.fromCore(auxiliaryData) : undefined
+      );
+      if (isValid !== undefined) transaction.setIsValid(isValid);
+    } else {
+      // Transaction CBOR is available. Use as is.
+      transaction = Serialization.Transaction.fromCbor(tx);
+    }
 
     const context = {
       ...signingContext,
       dRepPublicKey,
       knownAddresses,
-      txInKeyPathMap: await util.createTxInKeyPathMap(tx.body, knownAddresses, this.util)
+      txInKeyPathMap: await util.createTxInKeyPathMap(transaction.body().toCore(), knownAddresses, this.util)
     };
 
-    const emptyWitness = { signatures: new Map() };
-
-    // The Witnesser takes a serializable transaction. We cant build that from the hash alone, if
-    // the bodyCbor is available, use that instead of the coreTx type to build the transaction.
-    const transaction = new Serialization.Transaction(
-      bodyCbor ? Serialization.TransactionBody.fromCbor(bodyCbor) : Serialization.TransactionBody.fromCore(tx.body),
-      Serialization.TransactionWitnessSet.fromCore({ ...emptyWitness, ...witness }),
-      auxiliaryData ? Serialization.AuxiliaryData.fromCore(auxiliaryData) : undefined
-    );
-
-    if (isValid !== undefined) transaction.setIsValid(isValid);
-
     const result = await this.witnesser.witness(transaction, context, signingOptions);
 
     this.#newTransactions.signed$.next(result);

packages/wallet/src/cip30.ts

@@ -465,10 +465,10 @@ const baseCip30WalletApi = (
   },
   signTx: async ({ sender }: SenderContext, tx: Cbor, partialSign?: Boolean): Promise<Cbor> => {
     const scope = new ManagedFreeableScope();
-    logger.debug('signTx');
-    const txDecoded = Serialization.Transaction.fromCbor(Serialization.TxCBOR(tx));
+    logger.debug('signTx', tx);
+    const txCbor = Serialization.TxCBOR(tx);
+    const txDecoded = Serialization.Transaction.fromCbor(txCbor);
     const wallet = await firstValueFrom(wallet$);
-    const hash = txDecoded.getId();
     const coreTx = txDecoded.toCore();
 
     const needsForeignSignature = await requiresForeignSignatures(coreTx, wallet);
@@ -493,9 +493,8 @@ const baseCip30WalletApi = (
           witness: { signatures }
         } = await signOrCancel(
           wallet.finalizeTx({
-            bodyCbor: txDecoded.body().toCbor(),
             signingContext: { sender },
-            tx: { ...coreTx, hash }
+            tx: txCbor
           }),
           confirmationResult,
           () => new TxSignError(TxSignErrorCode.UserDeclined, 'user declined signing tx')

packages/wallet/src/types.ts

@@ -49,8 +49,13 @@ export interface SyncStatus extends Shutdown {
   isSettled$: Observable<boolean>;
 }
 
+/**
+ * If tx is the transaction CBOR, the auxiliary data, witness and isValid properties are ignored.
+ * If tx is `Cardano.TxBodyWithHash`, the transaction is reconstructed from along with the other
+ * provided properties.
+ */
 export type FinalizeTxProps = Omit<TxContext, 'signingContext'> & {
-  tx: Cardano.TxBodyWithHash;
+  tx: Cardano.TxBodyWithHash | Serialization.TxCBOR;
   bodyCbor?: HexBlob;
   signingContext?: Partial<SignTransactionContext>;
 };

packages/wallet/test/PersonalWallet/methods.test.ts

@@ -229,6 +229,18 @@ describe('BaseWallet methods', () => {
     });
 
     describe('finalizeTx', () => {
+      let witnessSpy: jest.SpyInstance;
+
+      beforeEach(() => {
+        witnessSpy = jest.spyOn(witnesser, 'witness');
+      });
+
+      afterEach(() => {
+        witnessSpy.mockClear();
+        witnessSpy.mockReset();
+        witnessSpy.mockRestore();
+      });
+
       it('resolves with TransactionWitnessSet', async () => {
         const txInternals = await wallet.initializeTx(props);
         const unhydratedTxBody = Serialization.TransactionBody.fromCore(txInternals.body).toCore();
@@ -241,24 +253,50 @@ describe('BaseWallet methods', () => {
 
       it('passes through sender to witnesser', async () => {
         const sender = { url: 'https://lace.io' };
-        const witnessSpy = jest.spyOn(witnesser, 'witness');
         const txInternals = await wallet.initializeTx(props);
-        await wallet.finalizeTx({ signingContext: { sender }, tx: txInternals });
 
+        // Reset witness calls from wallet.initializeTx
+        witnessSpy.mockClear();
+
+        await wallet.finalizeTx({ signingContext: { sender }, tx: txInternals });
+        expect(witnessSpy).toHaveBeenCalledTimes(1);
         expect(witnessSpy).toBeCalledWith(expect.anything(), expect.objectContaining({ sender }), void 0);
       });
 
       it('uses the original CBOR to create the serializable transaction if given', async () => {
         const sender = { url: 'https://lace.io' };
-        const witnessSpy = jest.spyOn(witnesser, 'witness');
         const txInternals = await wallet.initializeTx(props);
+
+        // Reset witness calls from wallet.initializeTx
+        witnessSpy.mockClear();
+
         await wallet.finalizeTx({
+          auxiliaryData: geniusYieldTx.auxiliaryData()?.toCore(),
           bodyCbor: geniusYieldTx.body().toCbor(),
           signingContext: { sender },
-          tx: txInternals
+          tx: txInternals,
+          witness: geniusYieldTx.witnessSet()?.toCore()
+        });
+
+        expect(witnessSpy).toHaveBeenCalledTimes(1);
+        const tx: Serialization.Transaction = witnessSpy.mock.calls[0][0];
+        expect(tx.body().toCbor()).toEqual(geniusYieldTx.body().toCbor());
+        // The transaction CBOR will not match due to reencoding witnessSet and auxiliaryData
+        // expect(tx.toCbor()).toEqual(geniusYieldTx.toCbor());
+      });
+
+      it('uses the complete transaction CBOR, ignoring auxiliaryData, witness and isValid', async () => {
+        const sender = { url: 'https://lace.io' };
+        await wallet.finalizeTx({
+          auxiliaryData: 'ignored auxiliary data' as Cardano.AuxiliaryData,
+          signingContext: { sender },
+          tx: geniusYieldTx.toCbor(),
+          witness: 'ignored witness set' as Partial<Cardano.Witness>
         });
 
-        expect(witnessSpy).toBeCalledWith(geniusYieldTx, expect.objectContaining({ sender }), void 0);
+        expect(witnessSpy).toHaveBeenCalledTimes(1);
+        const tx: Serialization.Transaction = witnessSpy.mock.calls[0][0];
+        expect(tx.toCbor()).toEqual(geniusYieldTx.toCbor());
       });
     });