From 0bcfc61b510939a4033769d28c2afb8b39026bab Mon Sep 17 00:00:00 2001 From: Zahin Mohammad Date: Thu, 30 Apr 2026 12:27:38 -0400 Subject: [PATCH] fix(sdk-coin-xtz): preserve txid when re-parsing signed origination MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous initFromSerializedTransaction relied on localForger.parse throwing on signed bytes (unsigned + 64-byte signature) to detect signed input — but the appended signature bytes are sometimes accidentally valid Michelson contents, so parse silently succeeds and the txid is left empty. wallet-platform then fails XTZ wallet creation with "unable to calculate the id of the deployment transaction" and "SendQueue validation failed: txid: Path \`txid\` is required" (COINFLP-116). Detect signed input by stripping the 64-byte suffix, parsing, and verifying the parsed result re-forges to those same bytes — a strict round-trip check rather than relying on parse to throw. Ticket: COINFLP-116 Co-Authored-By: Claude Opus 4.7 (1M context) --- commitlint.config.js | 1 + modules/sdk-coin-xtz/src/lib/transaction.ts | 56 ++++++++++++------- modules/sdk-coin-xtz/test/unit/transaction.ts | 28 ++++++++++ 3 files changed, 64 insertions(+), 21 deletions(-) diff --git a/commitlint.config.js b/commitlint.config.js index bc432bbf8f..c237343dd7 100644 --- a/commitlint.config.js +++ b/commitlint.config.js @@ -73,6 +73,7 @@ module.exports = { 'WCN-', 'WCI-', 'COIN-', + 'COINFLP-', 'FIAT-', 'ME-', 'ANT-', diff --git a/modules/sdk-coin-xtz/src/lib/transaction.ts b/modules/sdk-coin-xtz/src/lib/transaction.ts index d20e962739..6af3b7e67a 100644 --- a/modules/sdk-coin-xtz/src/lib/transaction.ts +++ b/modules/sdk-coin-xtz/src/lib/transaction.ts @@ -1,10 +1,4 @@ -import { - BaseKey, - BaseTransaction, - InvalidTransactionError, - ParseTransactionError, - TransactionType, -} from '@bitgo/sdk-core'; +import { BaseKey, BaseTransaction, InvalidTransactionError, TransactionType } from '@bitgo/sdk-core'; import { BaseCoin as CoinConfig } from '@bitgo/statics'; import { localForger } from '@taquito/local-forging'; import { OpKind } from '@taquito/rpc'; @@ -20,6 +14,32 @@ import { } from './multisigUtils'; import * as Utils from './utils'; +const SIGNATURE_HEX_LENGTH = 128; + +async function tryParseSigned( + serialized: string +): Promise<{ parsed: ParsedTransaction; transactionId: string } | undefined> { + if (serialized.length <= SIGNATURE_HEX_LENGTH) { + return undefined; + } + // If `serialized` really is `forge(ops) || signature`, stripping the trailing 64 + // bytes gives the exact forge bytes and forge(parse(...)) reproduces them. If + // `serialized` was unsigned, stripping cuts into the operation contents: parse + // either throws or recovers a truncated parse whose forge does not match. So a + // clean round-trip is what proves the trailing bytes were a signature. + const operationBytes = serialized.slice(0, -SIGNATURE_HEX_LENGTH); + try { + const parsed = await localForger.parse(operationBytes); + const roundTrip = await localForger.forge(parsed); + if (roundTrip !== operationBytes) { + return undefined; + } + return { parsed, transactionId: await Utils.calculateTransactionId(serialized) }; + } catch (_) { + return undefined; + } +} + /** * Tezos transaction model. */ @@ -49,22 +69,16 @@ export class Transaction extends BaseTransaction { */ async initFromSerializedTransaction(serializedTransaction: string): Promise { this._encodedTransaction = serializedTransaction; - try { - const parsedTransaction = await localForger.parse(serializedTransaction); - await this.initFromParsedTransaction(parsedTransaction); - } catch (e) { - // If it throws, it is possible the serialized transaction is signed, which is not supported - // by local-forging. Try extracting the last 64 bytes and parse it again. - const unsignedSerializedTransaction = serializedTransaction.slice(0, -128); - const signature = serializedTransaction.slice(-128); - if (Utils.isValidSignature(signature)) { - throw new ParseTransactionError('Invalid transaction'); - } + // Only signed inputs have a transaction id (it is the hash of the signed bytes); + // unsigned inputs leave it unset and let the caller populate it after signing. + const signed = await tryParseSigned(serializedTransaction); + if (signed) { // TODO: encode the signature and save it in _signature - const parsedTransaction = await localForger.parse(unsignedSerializedTransaction); - const transactionId = await Utils.calculateTransactionId(serializedTransaction); - await this.initFromParsedTransaction(parsedTransaction, transactionId); + await this.initFromParsedTransaction(signed.parsed, signed.transactionId); + return; } + const parsed = await localForger.parse(serializedTransaction); + await this.initFromParsedTransaction(parsed); } /** diff --git a/modules/sdk-coin-xtz/test/unit/transaction.ts b/modules/sdk-coin-xtz/test/unit/transaction.ts index 70e5b46384..85f20a0be9 100644 --- a/modules/sdk-coin-xtz/test/unit/transaction.ts +++ b/modules/sdk-coin-xtz/test/unit/transaction.ts @@ -9,6 +9,14 @@ import { import { OperationContents } from '@taquito/rpc'; import { XtzLib } from '../../src'; +// Signing the fixture origination with this seed produces a 64-byte signature +// whose bytes are coincidentally valid Michelson contents. +function signerSeedProducingMichelsonShapedSignature(): Buffer { + const seed = Buffer.alloc(16); + seed.writeUInt32BE(174, 0); + return seed; +} + describe('Tezos transaction', function () { describe('should parse', () => { it('unsigned transaction', async () => { @@ -42,6 +50,26 @@ describe('Tezos transaction', function () { JSON.stringify(tx.toJson()).should.equal(JSON.stringify(parsedTransaction)); tx.toBroadcastFormat().should.equal(signedSerializedOriginationTransaction); }); + + it('signed transaction whose signature suffix forges as valid Michelson', async () => { + const signerWithMichelsonShapedSignature = new XtzLib.KeyPair({ + seed: signerSeedProducingMichelsonShapedSignature(), + }); + + const signedTx = new XtzLib.Transaction(coins.get('txtz')); + await signedTx.initFromSerializedTransaction(unsignedSerializedOriginationTransaction); + await signedTx.sign(signerWithMichelsonShapedSignature); + const signedBytes = signedTx.toBroadcastFormat(); + const expectedTxId = signedTx.id; + expectedTxId.should.match(/^o[a-zA-Z0-9]+$/); + + const reparsed = new XtzLib.Transaction(coins.get('txtz')); + await reparsed.initFromSerializedTransaction(signedBytes); + + reparsed.id.should.equal(expectedTxId); + reparsed.outputs.length.should.equal(1); + reparsed.outputs[0].address.should.startWith('KT1'); + }); }); describe('should sign', () => {