fix: marinade multisig dapp requests by preserving original message bytes WPN-1035

packages/snap/CHANGELOG.md

@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Validate that account IDs passed to `keyring_setSelectedAccounts` belong to the snap, rejecting unknown IDs with `InvalidParamsError` ([#604](https://github.com/MetaMask/snap-solana-wallet/pull/604))
 
+### Fixed
+
+- **BREAKING:** Preserve dapp-origin `signTransaction` and `signAndSendTransaction` payloads by signing the decoded transaction directly ([#608](https://github.com/MetaMask/snap-solana-wallet/pull/608))
+
 ## [2.8.0]
 
 ### Added

packages/snap/snap.manifest.json

@@ -7,7 +7,7 @@
     "url": "https://github.com/MetaMask/snap-solana-wallet.git"
   },
   "source": {
-    "shasum": "AzXzPnYm37l2ijVy4ZM6GRu3DouI8hs75QHbfXJ/sBQ=",
+    "shasum": "dQYNwGUz41FQ0ODk5AFIcEgBsLkDgK2DS19eD2s8a0Q=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

packages/snap/src/core/sdk-extensions/codecs.test.ts

@@ -14,6 +14,7 @@ import {
   fromBytesToCompilableTransactionMessage,
   fromCompilableTransactionMessageToBase64String,
   fromTransactionToBase64String,
+  fromUnknowBase64StringToTransaction,
   fromUnknowBase64StringToTransactionOrTransactionMessage,
 } from './codecs';
 
@@ -93,6 +94,35 @@ describe('codecs', () => {
       });
     });
 
+    describe(`fromUnknowBase64StringToTransaction | Scenario: ${name}`, () => {
+      it('wraps bare compiled message bytes without changing the message bytes', async () => {
+        const transactionMessageBytes = pipe(
+          transactionMessage,
+          compileTransactionMessage,
+          getCompiledTransactionMessageEncoder().encode,
+        );
+
+        const result = await fromUnknowBase64StringToTransaction(
+          transactionMessageBase64Encoded,
+        );
+
+        expect(result.messageBytes).toStrictEqual(transactionMessageBytes);
+        expect(Object.values(result.signatures)).toStrictEqual(
+          Object.keys(result.signatures).map(() => null),
+        );
+      });
+
+      it('decodes a full transaction without changing the message bytes', async () => {
+        const result = await fromUnknowBase64StringToTransaction(
+          signedTransactionBase64Encoded,
+        );
+
+        const { lifetimeConstraint, ...rest } = signedTransaction;
+
+        expect(result).toStrictEqual(rest);
+      });
+    });
+
     describe('fromUnknowBase64StringToTransactionOrTransactionMessage', () => {
       it('decodes a base64 encoded transaction message to a transaction message', async () => {
         const result =

packages/snap/src/core/sdk-extensions/codecs.ts

@@ -124,6 +124,35 @@ export const fromBase64StringToTransaction = async (
 ): Promise<Transaction> =>
   pipe(base64String, getBase64Encoder().encode, getTransactionDecoder().decode);
 
+/**
+ * Decodes a base64 encoded compiled transaction message to a Transaction shape
+ * without changing the original message bytes.
+ *
+ * @param base64String - The base64 encoded compiled transaction message.
+ * @returns A transaction with null signature slots for all required signers.
+ */
+export const fromBase64StringCompiledMessageToTransaction = async (
+  base64String: Infer<typeof Base64Struct>,
+): Promise<Transaction> => {
+  const messageBytes = getBase64Encoder().encode(
+    base64String,
+  ) as TransactionMessageBytes;
+
+  const compiledTransactionMessage =
+    getCompiledTransactionMessageDecoder().decode(messageBytes);
+
+  const signerAccounts = compiledTransactionMessage.staticAccounts
+    .slice(0, compiledTransactionMessage.header.numSignerAccounts)
+    .map((address) => [address, null]);
+
+  const signatures = Object.fromEntries(signerAccounts);
+
+  return {
+    messageBytes,
+    signatures,
+  };
+};
+
 /**
  * Decodes a base64 string to a transaction or a compilable transaction message.
  *
@@ -141,3 +170,17 @@ export const fromUnknowBase64StringToTransactionOrTransactionMessage = async (
     fromBase64StringToTransaction(base64String),
     fromBase64StringToCompilableTransactionMessage(base64String, rpc, config),
   ]);
+
+/**
+ * Decodes a base64 string to a transaction.
+ *
+ * @param base64String - The base64 string to decode that represents either a transaction or a transaction message.
+ * @returns The decoded transaction.
+ */
+export const fromUnknowBase64StringToTransaction = async (
+  base64String: Infer<typeof Base64Struct>,
+): Promise<Transaction> =>
+  PromiseAny<Transaction>([
+    fromBase64StringToTransaction(base64String),
+    fromBase64StringCompiledMessageToTransaction(base64String),
+  ]);

packages/snap/src/core/services/signer/Signer.test.ts

@@ -3,6 +3,7 @@
 /* eslint-disable @typescript-eslint/no-var-requires */
 /* eslint-disable @typescript-eslint/no-require-imports */
 import {
+  getBase64Encoder,
   getSignatureFromTransaction,
   isTransactionMessageWithBlockhashLifetime,
 } from '@solana/kit';
@@ -121,6 +122,25 @@ describe('Signer', () => {
             ),
           ).toBe(true);
         });
+
+        it(`Scenario ${name}: preserves message bytes when requested`, async () => {
+          const originalMessageBytes = getBase64Encoder().encode(
+            transactionMessageBase64Encoded,
+          );
+
+          const result = await signer.partiallySignBase64String(
+            transactionMessageBase64Encoded,
+            fromAccount,
+            scope,
+            undefined,
+            true,
+          );
+
+          expect(result.messageBytes).toStrictEqual(originalMessageBytes);
+          expect(Object.values(result.signatures)).toStrictEqual([
+            expect.any(Uint8Array),
+          ]);
+        });
       });
     });
   });

packages/snap/src/core/services/signer/Signer.ts

@@ -19,6 +19,7 @@ import type { Network } from '../../constants/solana';
 import type { DecompileTransactionMessageFetchingLookupTablesConfig } from '../../sdk-extensions/codecs';
 import {
   fromBytesToCompilableTransactionMessage,
+  fromUnknowBase64StringToTransaction,
   fromUnknowBase64StringToTransactionOrTransactionMessage,
 } from '../../sdk-extensions/codecs';
 import {
@@ -60,6 +61,7 @@ export class Signer {
    * @param account - The account to sign the transaction or transaction message with.
    * @param network - The network on which the transaction is being sent.
    * @param config - The configuration for the request.
+   * @param preserveMessageBytes - Whether to preserve the original message bytes.
    * @returns The signed transaction.
    * @throws If the base64 string is not a valid transaction or transaction message.
    */
@@ -68,6 +70,7 @@ export class Signer {
     account: SolanaKeyringAccount,
     network: Network,
     config?: DecompileTransactionMessageFetchingLookupTablesConfig,
+    preserveMessageBytes = false,
   ): Promise<Transaction> {
     this.#logger.log('Partially sign base64 string', {
       base64String,
@@ -76,6 +79,13 @@ export class Signer {
       config,
     });
 
+    if (preserveMessageBytes) {
+      const transaction =
+        await fromUnknowBase64StringToTransaction(base64String);
+
+      return this.#partiallySignTransaction(transaction, account);
+    }
+
     const rpc = this.#connection.getRpc(network);
 
     // The received base64 string can either represent a transaction or a transaction message.

packages/snap/src/core/services/wallet/WalletService.test.ts

@@ -1,6 +1,6 @@
 import { SolMethod } from '@metamask/keyring-api';
 
-import { Network } from '../../constants/solana';
+import { METAMASK_ORIGIN, Network } from '../../constants/solana';
 import {
   MOCK_SOLANA_KEYRING_ACCOUNT_0,
   MOCK_SOLANA_KEYRING_ACCOUNT_1,
@@ -243,6 +243,40 @@ describe('WalletService', () => {
           });
         });
 
+        it('requests byte preservation for dapp origins', async () => {
+          await service.signTransaction(
+            fromAccount,
+            transactionMessageBase64Encoded,
+            scope,
+            origin,
+          );
+
+          expect(mockSigner.partiallySignBase64String).toHaveBeenCalledWith(
+            transactionMessageBase64Encoded,
+            fromAccount,
+            scope,
+            undefined,
+            true,
+          );
+        });
+
+        it('does not request byte preservation for MetaMask origin', async () => {
+          await service.signTransaction(
+            fromAccount,
+            transactionMessageBase64Encoded,
+            scope,
+            METAMASK_ORIGIN,
+          );
+
+          expect(mockSigner.partiallySignBase64String).toHaveBeenCalledWith(
+            transactionMessageBase64Encoded,
+            fromAccount,
+            scope,
+            undefined,
+            false,
+          );
+        });
+
         it('starts monitoring the transaction for commitment "confirmed"', async () => {
           await service.signTransaction(
             fromAccount,
@@ -275,6 +309,40 @@ describe('WalletService', () => {
           });
         });
 
+        it('requests byte preservation for dapp origins', async () => {
+          await service.signAndSendTransaction(
+            fromAccount,
+            transactionMessageBase64Encoded,
+            scope,
+            origin,
+          );
+
+          expect(mockSigner.partiallySignBase64String).toHaveBeenCalledWith(
+            transactionMessageBase64Encoded,
+            fromAccount,
+            scope,
+            undefined,
+            true,
+          );
+        });
+
+        it('does not request byte preservation for MetaMask origin', async () => {
+          await service.signAndSendTransaction(
+            fromAccount,
+            transactionMessageBase64Encoded,
+            scope,
+            METAMASK_ORIGIN,
+          );
+
+          expect(mockSigner.partiallySignBase64String).toHaveBeenCalledWith(
+            transactionMessageBase64Encoded,
+            fromAccount,
+            scope,
+            undefined,
+            false,
+          );
+        });
+
         it('starts monitoring the transaction for commitment "confirmed"', async () => {
           await service.signAndSendTransaction(
             fromAccount,

packages/snap/src/core/services/wallet/WalletService.ts

@@ -18,7 +18,11 @@ import {
 } from '@solana/kit';
 
 import type { SolanaKeyringAccount } from '../../../entities';
-import type { Caip10Address, Network } from '../../constants/solana';
+import {
+  METAMASK_ORIGIN,
+  type Caip10Address,
+  type Network,
+} from '../../constants/solana';
 import type { DecompileTransactionMessageFetchingLookupTablesConfig } from '../../sdk-extensions/codecs';
 import { fromTransactionToBase64String } from '../../sdk-extensions/codecs';
 import { addressToCaip10 } from '../../utils/addressToCaip10';
@@ -179,12 +183,17 @@ export class WalletService {
           }
         : undefined;
 
+    // For transactions coming from DApps, preserve the original message bytes.
+    // Mutating them would change the signing payload and break multisig flows.
+    const shouldPreserveMessageBytes = origin !== METAMASK_ORIGIN;
+
     const partiallySignedTransaction =
       await this.#signer.partiallySignBase64String(
         transaction,
         account,
         scope,
         config,
+        shouldPreserveMessageBytes,
       );
 
     const signedTransactionBase64 = fromTransactionToBase64String(
@@ -250,12 +259,17 @@ export class WalletService {
           }
         : undefined;
 
+    // For transactions coming from DApps, preserve the original message bytes.
+    // Mutating them would change the signing payload and break multisig flows.
+    const shouldPreserveMessageBytes = origin !== METAMASK_ORIGIN;
+
     const partiallySignedTransaction =
       await this.#signer.partiallySignBase64String(
         transactionMessageBase64Encoded,
         account,
         scope,
         signConfig,
+        shouldPreserveMessageBytes,
       );
 
     const signature = getSignatureFromTransaction(partiallySignedTransaction);