From 04789cab81cb8f4efb79fd86fe1cdf5a557e886e Mon Sep 17 00:00:00 2001 From: Steven Luscher Date: Mon, 28 Nov 2022 18:50:16 -0800 Subject: [PATCH] fix: verify commitment level when confirming transactions with one-shot fetch (#28969) * Rename `subscriptionCommitment` to `confirmationCommitment` * Reorganize status checking code to return early if `value` is `null` * Bail if the one-shot signature result does not meet the target commitment --- web3.js/src/connection.ts | 36 ++++++++++++++++++++++++++++----- web3.js/test/connection.test.ts | 36 +++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 5 deletions(-) diff --git a/web3.js/src/connection.ts b/web3.js/src/connection.ts index c89d03c98f..d6457567d8 100644 --- a/web3.js/src/connection.ts +++ b/web3.js/src/connection.ts @@ -3386,7 +3386,7 @@ export class Connection { assert(decodedSignature.length === 64, 'signature has invalid length'); - const subscriptionCommitment = commitment || this.commitment; + const confirmationCommitment = commitment || this.commitment; let timeoutId; let signatureSubscriptionId: number | undefined; let disposeSignatureSubscriptionStateChangeObserver: @@ -3410,7 +3410,7 @@ export class Connection { done = true; resolve({__type: TransactionStatus.PROCESSED, response}); }, - subscriptionCommitment, + confirmationCommitment, ); const subscriptionSetupPromise = new Promise( resolveSubscriptionSetup => { @@ -3438,10 +3438,36 @@ export class Connection { return; } const {context, value} = response; + if (value == null) { + return; + } if (value?.err) { reject(value.err); - } - if (value) { + } else { + switch (confirmationCommitment) { + case 'confirmed': + case 'single': + case 'singleGossip': { + if (value.confirmationStatus === 'processed') { + return; + } + break; + } + case 'finalized': + case 'max': + case 'root': { + if ( + value.confirmationStatus === 'processed' || + value.confirmationStatus === 'confirmed' + ) { + return; + } + break; + } + // exhaust enums to ensure full coverage + case 'processed': + case 'recent': + } done = true; resolve({ __type: TransactionStatus.PROCESSED, @@ -3463,7 +3489,7 @@ export class Connection { >(resolve => { if (typeof strategy === 'string') { let timeoutMs = this._confirmTransactionInitialTimeout || 60 * 1000; - switch (subscriptionCommitment) { + switch (confirmationCommitment) { case 'processed': case 'recent': case 'single': diff --git a/web3.js/test/connection.test.ts b/web3.js/test/connection.test.ts index e13260540e..dd333a778b 100644 --- a/web3.js/test/connection.test.ts +++ b/web3.js/test/connection.test.ts @@ -1214,6 +1214,42 @@ describe('Connection', function () { // value: {err: null}, // }); // }); + + it('confirm transaction - does not confirm the transaction when signature status check yields confirmation for a lower commitment before signature subscription confirms the transaction', async () => { + const mockSignature = + 'w2Zeq8YkpyB463DttvfzARD7k9ZxGEwbsEw4boEK7jDp3pfoxZbTdLFSsEPhzXhpCcjGi2kHtHFobgX49MMhbWt'; + + // Keep the subscription from ever returning data. + await mockRpcMessage({ + method: 'signatureSubscribe', + params: [mockSignature, {commitment: 'finalized'}], + result: new Promise(() => {}), // Never resolve. + }); + clock.runAllAsync(); + + const confirmationPromise = + connection.confirmTransaction(mockSignature); + clock.runAllAsync(); + + // Return a signature status with a lower finality through the RPC API. + await mockRpcResponse({ + method: 'getSignatureStatuses', + params: [[mockSignature]], + value: [ + { + slot: 0, + confirmations: null, + confirmationStatus: 'processed', // Lower than we expect + err: null, + }, + ], + }); + clock.runAllAsync(); + + await expect(confirmationPromise).to.be.rejectedWith( + TransactionExpiredTimeoutError, + ); + }); }); }