Skip to content

Commit

Permalink
fix: verify commitment level when confirming transactions with one-sh…
Browse files Browse the repository at this point in the history
…ot 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
  • Loading branch information
steveluscher authored Nov 29, 2022
1 parent 3b2014d commit 04789ca
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 5 deletions.
36 changes: 31 additions & 5 deletions web3.js/src/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -3410,7 +3410,7 @@ export class Connection {
done = true;
resolve({__type: TransactionStatus.PROCESSED, response});
},
subscriptionCommitment,
confirmationCommitment,
);
const subscriptionSetupPromise = new Promise<void>(
resolveSubscriptionSetup => {
Expand Down Expand Up @@ -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,
Expand All @@ -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':
Expand Down
36 changes: 36 additions & 0 deletions web3.js/test/connection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
});
});
}

Expand Down

0 comments on commit 04789ca

Please sign in to comment.