Skip to content

Commit

Permalink
fix: deserialize partially signed multisig transaction
Browse files Browse the repository at this point in the history
  • Loading branch information
ahsan-javaid authored and janniks committed Apr 13, 2022
1 parent 7973100 commit 52d4045
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 8 deletions.
1 change: 1 addition & 0 deletions packages/transactions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ transaction must be appended to the signature.
```typescript
// deserialize and sign transaction
const bufferReader = new BufferReader(serializedTx);
// Partially signed or unsigned multi-sig tx can be deserialized to add the required signatures
const deserializedTx = deserializeTransaction(bufferReader);

const signer = new TransactionSigner(deserializedTx);
Expand Down
3 changes: 2 additions & 1 deletion packages/transactions/src/authorization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,8 @@ export function deserializeMultiSigSpendingCondition(
}
const signaturesRequired = bufferReader.readUInt16BE();

if (numSigs !== signaturesRequired) throw new VerificationError(`Incorrect number of signatures`);
// Partially signed multi-sig tx can be serialized and deserialized without exception (Incorrect number of signatures)
// No need to check numSigs !== signaturesRequired to throw Incorrect number of signatures error

if (haveUncompressed && hashMode === AddressHashMode.SerializeP2SH)
throw new VerificationError('Uncompressed keys are not allowed in this hash mode');
Expand Down
8 changes: 6 additions & 2 deletions packages/transactions/tests/authorization.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -526,13 +526,17 @@ test('Invalid spending conditions', () => {

const badPublicKeyCountBuffer = Buffer.from(badPublicKeyCountBytes);

// Partially signed multi-sig tx can be serialized and deserialized without exception (Incorrect number of signatures)
// Should be able to deserialize as number of signatures are less than signatures required
expect(() => deserializeSpendingCondition(new BufferReader(badPublicKeyCountBuffer)))
.toThrow('Incorrect number of signatures');
.not.toThrowError();

const badPublicKeyCount2Buffer = Buffer.from(badPublicKeyCountBytes2);

// Partially signed multi-sig tx can be serialized and deserialized without exception (Incorrect number of signatures)
// Should be able to deserialize as number of signatures are less than signatures required
expect(() => deserializeSpendingCondition(new BufferReader(badPublicKeyCount2Buffer)))
.toThrow('Incorrect number of signatures');
.not.toThrowError();

// hashing mode doesn't allow uncompressed keys
const signatureFF = createMessageSignature('ff'.repeat(65));
Expand Down
13 changes: 8 additions & 5 deletions packages/transactions/tests/builder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ test('Make Multi-Sig STX token transfer', async () => {
expect(serializedSignedTx.toString('hex')).toBe(signedTx);
});

test('Should not deserilize partially signed multi-Sig STX token transfer', async () => {
test('Should deserialize partially signed multi-Sig STX token transfer', async () => {
const recipient = standardPrincipalCV('SP3FGQ8Z7JY9BWYZ5WM53E0M9NK7WHJF0691NZ159');
const amount = 2500000;
const fee = 0;
Expand Down Expand Up @@ -373,8 +373,9 @@ test('Should not deserilize partially signed multi-Sig STX token transfer', asyn

const bufferReader = new BufferReader(serializedTx);

// Should not be able to deserializeTransaction due to missing signatures.
expect(() => deserializeTransaction(bufferReader)).toThrow('Incorrect number of signatures');
// Partially signed or unsigned multi-sig tx can be serialized and deserialized without exception (Incorrect number of signatures)
// Should be able to deserializeTransaction with missing signatures.
expect(() => deserializeTransaction(bufferReader)).not.toThrowError();

// Now add the required signatures in the original transactions
const signer = new TransactionSigner(transaction);
Expand All @@ -385,7 +386,7 @@ test('Should not deserilize partially signed multi-Sig STX token transfer', asyn
const fullySignedTransaction = transaction.serialize();
const bufferReaderSignedTx = new BufferReader(fullySignedTransaction);

// Should not throw any exception after adding required signatures.
// Should deserialize fully signed multi sig transaction
const deserializedTx = deserializeTransaction(bufferReaderSignedTx);

expect(deserializedTx.auth.authType).toBe(authType);
Expand Down Expand Up @@ -514,7 +515,9 @@ test('Make Multi-Sig STX token transfer with two transaction signers', async ()

// deserialize
const bufferReader2 = new BufferReader(partiallySignedSerialized);
expect(() => deserializeTransaction(bufferReader2)).toThrow('Incorrect number of signatures');
// Partially signed multi-sig tx can be serialized and deserialized without exception (Incorrect number of signatures)
// Should be able to deserialize as number of signatures are less than signatures required
expect(() => deserializeTransaction(bufferReader2)).not.toThrowError();

// finish signing with new TransactionSigner
const signer2 = new TransactionSigner(transaction);
Expand Down

1 comment on commit 52d4045

@vercel
Copy link

@vercel vercel bot commented on 52d4045 Apr 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.