From e5344332cbf9e49bebd1734ff3693b34f505c455 Mon Sep 17 00:00:00 2001 From: Nicolas Schapeler Date: Fri, 5 Jan 2024 17:17:00 -0800 Subject: [PATCH 1/8] remove bailout behavior from missing sig func --- .../src/__tests__/signatures-test.ts | 29 +++++++++++++++---- packages/transactions/src/signatures.ts | 11 +++++-- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/packages/transactions/src/__tests__/signatures-test.ts b/packages/transactions/src/__tests__/signatures-test.ts index 00bebaf8735..741cf6647d7 100644 --- a/packages/transactions/src/__tests__/signatures-test.ts +++ b/packages/transactions/src/__tests__/signatures-test.ts @@ -279,7 +279,7 @@ describe('signTransaction', () => { expect.assertions(1); const signedTransactionPromise = signTransaction([mockKeyPairA], MOCK_TRANSACTION); await expect(signedTransactionPromise).rejects.toThrow( - `Transaction is missing signature for address \`${mockPublicKeyAddressB}\``, + `Transaction is missing signatures for addresses: '${mockPublicKeyAddressB}'`, ); }); it('returns a transaction object having multiple signatures', async () => { @@ -368,7 +368,26 @@ describe('assertTransactionIsFullySigned', () => { }; expect(() => assertTransactionIsFullySigned(transaction)).toThrow( - 'Transaction is missing signature for address `A`', + "Transaction is missing signatures for addresses: 'A'", + ); + }); + + it('throws all missing signers if the transaction has no signature for multiple signers', () => { + const transaction: SignedTransaction = { + feePayer: mockPublicKeyAddressA, + instructions: [ + { + accounts: [{ address: mockPublicKeyAddressB, role: AccountRole.READONLY_SIGNER }], + programAddress: '11111111111111111111111111111111' as Address<'11111111111111111111111111111111'>, + }, + ], + lifetimeConstraint: mockBlockhashConstraint, + signatures: {}, + version: 0, + }; + + expect(() => assertTransactionIsFullySigned(transaction)).toThrow( + "Transaction is missing signatures for addresses: 'A', 'B'", ); }); @@ -394,7 +413,7 @@ describe('assertTransactionIsFullySigned', () => { }; expect(() => assertTransactionIsFullySigned(transaction)).toThrow( - 'Transaction is missing signature for address `B`', + "Transaction is missing signatures for addresses: 'B'", ); }); @@ -420,7 +439,7 @@ describe('assertTransactionIsFullySigned', () => { }; expect(() => assertTransactionIsFullySigned(transaction)).toThrow( - 'Transaction is missing signature for address `B`', + "Transaction is missing signatures for addresses: 'B'", ); }); @@ -456,7 +475,7 @@ describe('assertTransactionIsFullySigned', () => { }; expect(() => assertTransactionIsFullySigned(transaction)).toThrow( - 'Transaction is missing signature for address `C`', + "Transaction is missing signatures for addresses: 'C'", ); }); diff --git a/packages/transactions/src/signatures.ts b/packages/transactions/src/signatures.ts index e34bbfcfbdf..dc3d8aff98d 100644 --- a/packages/transactions/src/signatures.ts +++ b/packages/transactions/src/signatures.ts @@ -77,10 +77,17 @@ export function assertTransactionIsFullySigned a.address); const requiredSigners = new Set([transaction.feePayer, ...signerAddressesFromInstructions]); + const missingSigs = []; requiredSigners.forEach(address => { if (!transaction.signatures[address]) { - // TODO coded error - throw new Error(`Transaction is missing signature for address \`${address}\``); + missingSigs.push(address); } }); + + if (missingSigs.length > 0) { + // TODO coded error + throw new Error( + `Transaction is missing signatures for addresses: ${missingSigs.map(a => `'${a}'`).join(', ')}`, + ); + } } From 1ff51695ab51af00e4e05e5f8b1815fb3f8df5a2 Mon Sep 17 00:00:00 2001 From: Nicolas Schapeler Date: Fri, 5 Jan 2024 17:31:11 -0800 Subject: [PATCH 2/8] format --- packages/transactions/src/__tests__/signatures-test.ts | 10 +++++----- packages/transactions/src/signatures.ts | 5 ++--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/transactions/src/__tests__/signatures-test.ts b/packages/transactions/src/__tests__/signatures-test.ts index 741cf6647d7..1a0757b4ac8 100644 --- a/packages/transactions/src/__tests__/signatures-test.ts +++ b/packages/transactions/src/__tests__/signatures-test.ts @@ -279,7 +279,7 @@ describe('signTransaction', () => { expect.assertions(1); const signedTransactionPromise = signTransaction([mockKeyPairA], MOCK_TRANSACTION); await expect(signedTransactionPromise).rejects.toThrow( - `Transaction is missing signatures for addresses: '${mockPublicKeyAddressB}'`, + `Transaction is missing signature for address: '${mockPublicKeyAddressB}'`, ); }); it('returns a transaction object having multiple signatures', async () => { @@ -368,7 +368,7 @@ describe('assertTransactionIsFullySigned', () => { }; expect(() => assertTransactionIsFullySigned(transaction)).toThrow( - "Transaction is missing signatures for addresses: 'A'", + "Transaction is missing signature for address: 'A'", ); }); @@ -413,7 +413,7 @@ describe('assertTransactionIsFullySigned', () => { }; expect(() => assertTransactionIsFullySigned(transaction)).toThrow( - "Transaction is missing signatures for addresses: 'B'", + "Transaction is missing signature for address: 'B'", ); }); @@ -439,7 +439,7 @@ describe('assertTransactionIsFullySigned', () => { }; expect(() => assertTransactionIsFullySigned(transaction)).toThrow( - "Transaction is missing signatures for addresses: 'B'", + "Transaction is missing signature for address: 'B'", ); }); @@ -475,7 +475,7 @@ describe('assertTransactionIsFullySigned', () => { }; expect(() => assertTransactionIsFullySigned(transaction)).toThrow( - "Transaction is missing signatures for addresses: 'C'", + "Transaction is missing signature for address: 'C'", ); }); diff --git a/packages/transactions/src/signatures.ts b/packages/transactions/src/signatures.ts index dc3d8aff98d..c73353369fd 100644 --- a/packages/transactions/src/signatures.ts +++ b/packages/transactions/src/signatures.ts @@ -85,9 +85,8 @@ export function assertTransactionIsFullySigned 0) { + const startErr = missingSigs.length === 1 ? 'signature for address' : 'signatures for addresses'; // TODO coded error - throw new Error( - `Transaction is missing signatures for addresses: ${missingSigs.map(a => `'${a}'`).join(', ')}`, - ); + throw new Error('Transaction is missing ' + startErr + ':' + `${missingSigs.map(a => `'${a}'`).join(', ')}`); } } From 8aa59a6cfce588490016596f0624061155f87c72 Mon Sep 17 00:00:00 2001 From: Nicolas Schapeler Date: Fri, 5 Jan 2024 17:42:07 -0800 Subject: [PATCH 3/8] add type dec --- packages/transactions/src/signatures.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/transactions/src/signatures.ts b/packages/transactions/src/signatures.ts index c73353369fd..ff54fa21912 100644 --- a/packages/transactions/src/signatures.ts +++ b/packages/transactions/src/signatures.ts @@ -28,7 +28,7 @@ export function getSignatureFromTransaction( // TODO: Coded error. throw new Error( "Could not determine this transaction's signature. Make sure that the transaction " + - 'has been signed by its fee payer.', + 'has been signed by its fee payer.', ); } const transactionSignature = base58Decoder.decode(signatureBytes); @@ -77,7 +77,7 @@ export function assertTransactionIsFullySigned a.address); const requiredSigners = new Set([transaction.feePayer, ...signerAddressesFromInstructions]); - const missingSigs = []; + const missingSigs: Address[] = []; requiredSigners.forEach(address => { if (!transaction.signatures[address]) { missingSigs.push(address); From 783bb58ca57431152f37a04c95b1acf2e897e1f2 Mon Sep 17 00:00:00 2001 From: Nicolas Schapeler Date: Fri, 5 Jan 2024 18:10:09 -0800 Subject: [PATCH 4/8] fix test --- packages/transactions/src/signatures.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/transactions/src/signatures.ts b/packages/transactions/src/signatures.ts index ff54fa21912..25586f51906 100644 --- a/packages/transactions/src/signatures.ts +++ b/packages/transactions/src/signatures.ts @@ -87,6 +87,6 @@ export function assertTransactionIsFullySigned 0) { const startErr = missingSigs.length === 1 ? 'signature for address' : 'signatures for addresses'; // TODO coded error - throw new Error('Transaction is missing ' + startErr + ':' + `${missingSigs.map(a => `'${a}'`).join(', ')}`); + throw new Error('Transaction is missing ' + startErr + ': ' + `${missingSigs.map(a => `'${a}'`).join(', ')}`); } } From b117159d5bc95b1b4f40c3623e093149dd694e2e Mon Sep 17 00:00:00 2001 From: Nico <38372048+cryptopapi997@users.noreply.github.com> Date: Wed, 10 Jan 2024 00:57:19 +0100 Subject: [PATCH 5/8] Update packages/transactions/src/signatures.ts --- packages/transactions/src/signatures.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/transactions/src/signatures.ts b/packages/transactions/src/signatures.ts index 25586f51906..11c5745d50b 100644 --- a/packages/transactions/src/signatures.ts +++ b/packages/transactions/src/signatures.ts @@ -85,8 +85,8 @@ export function assertTransactionIsFullySigned 0) { - const startErr = missingSigs.length === 1 ? 'signature for address' : 'signatures for addresses'; // TODO coded error - throw new Error('Transaction is missing ' + startErr + ': ' + `${missingSigs.map(a => `'${a}'`).join(', ')}`); + throw new Error('Transaction is missing signatures for addresses: ' + missingSigs.join(', ')); } + } From a90903b139b4a5551d6648326eb19c93353aa8a8 Mon Sep 17 00:00:00 2001 From: Nicolas Schapeler Date: Wed, 10 Jan 2024 01:01:55 +0100 Subject: [PATCH 6/8] adapt tests --- .../transactions/src/__tests__/signatures-test.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/transactions/src/__tests__/signatures-test.ts b/packages/transactions/src/__tests__/signatures-test.ts index 1a0757b4ac8..f2db7bdbb3b 100644 --- a/packages/transactions/src/__tests__/signatures-test.ts +++ b/packages/transactions/src/__tests__/signatures-test.ts @@ -49,7 +49,7 @@ describe('getSignatureFromTransaction', () => { getSignatureFromTransaction(transactionWithoutFeePayerSignature); }).toThrow( "Could not determine this transaction's signature. Make sure that the transaction " + - 'has been signed by its fee payer.', + 'has been signed by its fee payer.', ); }); }); @@ -279,7 +279,7 @@ describe('signTransaction', () => { expect.assertions(1); const signedTransactionPromise = signTransaction([mockKeyPairA], MOCK_TRANSACTION); await expect(signedTransactionPromise).rejects.toThrow( - `Transaction is missing signature for address: '${mockPublicKeyAddressB}'`, + `Transaction is missing signatures for addresses: ${mockPublicKeyAddressB}`, ); }); it('returns a transaction object having multiple signatures', async () => { @@ -368,7 +368,7 @@ describe('assertTransactionIsFullySigned', () => { }; expect(() => assertTransactionIsFullySigned(transaction)).toThrow( - "Transaction is missing signature for address: 'A'", + "Transaction is missing signatures for addresses: A", ); }); @@ -387,7 +387,7 @@ describe('assertTransactionIsFullySigned', () => { }; expect(() => assertTransactionIsFullySigned(transaction)).toThrow( - "Transaction is missing signatures for addresses: 'A', 'B'", + "Transaction is missing signatures for addresses: A, B", ); }); @@ -413,7 +413,7 @@ describe('assertTransactionIsFullySigned', () => { }; expect(() => assertTransactionIsFullySigned(transaction)).toThrow( - "Transaction is missing signature for address: 'B'", + "Transaction is missing signatures for addresses: B", ); }); @@ -439,7 +439,7 @@ describe('assertTransactionIsFullySigned', () => { }; expect(() => assertTransactionIsFullySigned(transaction)).toThrow( - "Transaction is missing signature for address: 'B'", + "Transaction is missing signatures for addresses: B", ); }); @@ -475,7 +475,7 @@ describe('assertTransactionIsFullySigned', () => { }; expect(() => assertTransactionIsFullySigned(transaction)).toThrow( - "Transaction is missing signature for address: 'C'", + "Transaction is missing signatures for addresses: C", ); }); From 12f75691471c8bbe056ff39d127bca9a75055f58 Mon Sep 17 00:00:00 2001 From: Nicolas Schapeler Date: Wed, 10 Jan 2024 01:03:36 +0100 Subject: [PATCH 7/8] style --- .../transactions/src/__tests__/signatures-test.ts | 12 ++++++------ packages/transactions/src/signatures.ts | 3 +-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/transactions/src/__tests__/signatures-test.ts b/packages/transactions/src/__tests__/signatures-test.ts index f2db7bdbb3b..4419202161d 100644 --- a/packages/transactions/src/__tests__/signatures-test.ts +++ b/packages/transactions/src/__tests__/signatures-test.ts @@ -49,7 +49,7 @@ describe('getSignatureFromTransaction', () => { getSignatureFromTransaction(transactionWithoutFeePayerSignature); }).toThrow( "Could not determine this transaction's signature. Make sure that the transaction " + - 'has been signed by its fee payer.', + 'has been signed by its fee payer.', ); }); }); @@ -368,7 +368,7 @@ describe('assertTransactionIsFullySigned', () => { }; expect(() => assertTransactionIsFullySigned(transaction)).toThrow( - "Transaction is missing signatures for addresses: A", + 'Transaction is missing signatures for addresses: A', ); }); @@ -387,7 +387,7 @@ describe('assertTransactionIsFullySigned', () => { }; expect(() => assertTransactionIsFullySigned(transaction)).toThrow( - "Transaction is missing signatures for addresses: A, B", + 'Transaction is missing signatures for addresses: A, B', ); }); @@ -413,7 +413,7 @@ describe('assertTransactionIsFullySigned', () => { }; expect(() => assertTransactionIsFullySigned(transaction)).toThrow( - "Transaction is missing signatures for addresses: B", + 'Transaction is missing signatures for addresses: B', ); }); @@ -439,7 +439,7 @@ describe('assertTransactionIsFullySigned', () => { }; expect(() => assertTransactionIsFullySigned(transaction)).toThrow( - "Transaction is missing signatures for addresses: B", + 'Transaction is missing signatures for addresses: B', ); }); @@ -475,7 +475,7 @@ describe('assertTransactionIsFullySigned', () => { }; expect(() => assertTransactionIsFullySigned(transaction)).toThrow( - "Transaction is missing signatures for addresses: C", + 'Transaction is missing signatures for addresses: C', ); }); diff --git a/packages/transactions/src/signatures.ts b/packages/transactions/src/signatures.ts index 11c5745d50b..ef6d00419df 100644 --- a/packages/transactions/src/signatures.ts +++ b/packages/transactions/src/signatures.ts @@ -28,7 +28,7 @@ export function getSignatureFromTransaction( // TODO: Coded error. throw new Error( "Could not determine this transaction's signature. Make sure that the transaction " + - 'has been signed by its fee payer.', + 'has been signed by its fee payer.', ); } const transactionSignature = base58Decoder.decode(signatureBytes); @@ -88,5 +88,4 @@ export function assertTransactionIsFullySigned Date: Wed, 10 Jan 2024 02:23:00 +0100 Subject: [PATCH 8/8] fix test in signers --- packages/signers/src/__tests__/sign-transaction-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/signers/src/__tests__/sign-transaction-test.ts b/packages/signers/src/__tests__/sign-transaction-test.ts index 30ee4cfcabc..c341baedadb 100644 --- a/packages/signers/src/__tests__/sign-transaction-test.ts +++ b/packages/signers/src/__tests__/sign-transaction-test.ts @@ -331,7 +331,7 @@ describe('signTransactionWithSigners', () => { // Then we expect an error letting us know the transaction is not fully signed. // This is because sending signers are ignored by signTransactionWithSigners. - await expect(promise).rejects.toThrow('Transaction is missing signature for address `2222`'); + await expect(promise).rejects.toThrow('Transaction is missing signatures for addresses: 2222'); }); it('can be cancelled using an AbortSignal', async () => {