web3.js: add getSignatureStatuses to confirm transaction

[abrzezinski94]

Problem

If you call confirmTransaction for a signature that has already been processed, the signatureSubscribe subscription will never fire a notification. This will certainly mean that confirmTransaction() will throw an expiry error for a transaction that is, in actual fact, confirmed.

solana-labs/solana-web3.js#1107

Summary of Changes

add getSignatureStatuses

Fixes #1107.

[codecov]

Codecov Report

Merging #28290 (bed9ea7) into master (831ed96) will decrease coverage by 0.1%.
The diff coverage is 65.4%.

❗ Current head bed9ea7 differs from pull request most recent head 78b36ee. Consider uploading reports for the commit 78b36ee to get more accurate results

@@            Coverage Diff            @@
##           master   solana-labs/solana#28290     +/-   ##
=========================================
- Coverage    77.1%    77.0%   -0.2%     
=========================================
  Files          55       55             
  Lines        2934     2977     +43     
  Branches      408      422     +14     
=========================================
+ Hits         2264     2294     +30     
- Misses        529      535      +6     
- Partials      141      148      +7     

[steveluscher]

Hey! Thanks for this PR; I really appreciate the help to make transaction confirmation more robust.

I have ideas to make this way simpler.

You've implemented a polling strategy here, and have done so without removing the existing subscription strategy. This means that we now have two competing strategies, where what we could do instead is to make them compliment each other.

In the original issue, I wrote:

1. Before subscribing or honouring expiry signals, check the signature status using the getSignatureStatuses method.
2. If the signature has not been confirmed and you have not received an expiry signal, subscribe for status updates.
3. Race the expiry strategy and the confirmation strategy.

What I was getting at there is that – between the time you send the transaction and you subscribe for the signature – you can miss the subscription notification.

The solution for this should be simple:

1. Subscribe first.
2. Immediately after the subscription is live, send a single getSignatureStatuses to rule out the possibility of having missed a prior signature change.

This should make this PR almost literally one line, and should eliminate the inefficiency of polling!

Want to take a crack at that?

[abrzezinski94]

Hey! Thanks for this PR; I really appreciate the help to make transaction confirmation more robust.

I have ideas to make this way simpler.

You've implemented a polling strategy here, and have done so without removing the existing subscription strategy. This means that we now have two competing strategies, where what we could do instead is to make them compliment each other.

In the original issue, I wrote:

1. Before subscribing or honouring expiry signals, check the signature status using the getSignatureStatuses method.
2. If the signature has not been confirmed and you have not received an expiry signal, subscribe for status updates.
3. Race the expiry strategy and the confirmation strategy.

What I was getting at there is that – between the time you send the transaction and you subscribe for the signature – you can miss the subscription notification.

The solution for this should be simple:

1. Subscribe first.
2. Immediately after the subscription is live, send a single getSignatureStatuses to rule out the possibility of having missed a prior signature change.

This should make this PR almost literally one line, and should eliminate the inefficiency of polling!

Want to take a crack at that?

Hi we use pooling like that in mango/gov ui and its quite nice. Websockets can sometimes fail and its good to have a backup method to confirm tx.

What do you think about adding option to config it for example:

a. websockets only
b. pooling only
c. websockets + pooling (i would leave it as default method for reliability)

and change suggested by you ofc to shot first with getSignatureStatuses for websockets

[steveluscher]

I would rather resolve the architectural decision here amongst ourselves, rather than to push it out to library users as a choice.

Some reasons I disfavor polling:

• It is very low resolution. The maximum confirmation resolution that you can achieve is half the sampling frequency (as per Nyquist).
• It's wasteful of client connections, and RPC capacity; one subscription channel can report on n signatures. Polling requires n connections, m times per second.

I think I would need some evidence that websockets are so unreliable – other than the known problem of being able to ‘miss’ signatures that this PR aims to solve – as to warrant making every Solana application that uses web3.js start sending all extra network requests to RPCs. If there's evidence of the WS transport being that unreliable, we should fix that first.

Pull request has been modified.

[steveluscher]

After those couple fixes we'll need these tests, and then we're done!

1. Test that getSignatureStatuses does not get fired off until onSignature has resolved.
2. Test that getSignatureStatuses does not get fired at all if (somehow) onSignature resolves synchronously (who knows; that could get implemented in the future)
3. Test that transactions get confirmed if no subscription notifications get fired, but getSignatureStatuses returns a confirmation
4. Test that transactions still get confirmed if getSignatureStatuses returns nothing, but then you get a subscription notification.

You should be able to accomplish that without writing a ‘live’ test at all.

[steveluscher]

This doesn’t quite do what we set out to do yet, because it doesn’t wait for the subscription to be set up (ie. wait for the server to vend a clientSubscriptionId). We want to wait for the subscription to be live before firing off the patch-up call to getSignatureStatuses.

his also could use a test! Assert that the gss call does not get made until onSignature resolves.

[abrzezinski94]

isn't subscriptionId returned immediately from onSignature ?

[steveluscher]

Oh, right! Those are the client ids, not the server ids which we keep internal.

Ooh, nice. This just got way more complicated. Essentially, you need a way now of observing the state change of the subscription you just set up from 'pending' to 'subscribed' before you make the gSS call. Oftentimes you’ll be the first subscription for the whole app, in which case this could take considerable time (ie. the whole WebSocket connection has to be set up first).

This will be the first time that any code needs to observe the internal state of a subscription. Any ideas on how to achieve this with a minimum of changes?

[abrzezinski94]

at first glance maybe we could get hash like this.onSignature do it (i would need to look if for sure it will be the same).

Observe this._subscriptionsByHash (maybe event raise in _updateSubscriptions? Or just check changes in while loop) and when typeof this._subscriptionsByHash[hash] !== 'undefined' && state is 'subscribed' fire gss we can do it all in confirmation promise.

[abrzezinski94]

Ok i pushed some solution for you to look this is just working poc and we can probably make it better. still needs to write tests if it fulfills all needs. It dont have any impact on outside code.

Pull request has been modified.

[steveluscher]

Let me take a crack at this.

Pull request has been modified.

[steveluscher]

@jordansexton, other than the fact that commented-out test doesn't work, what do you think of this (see my three commits in the history here).

[brianlong]

The solution proposed here looks good, and I do not see an undue burden on RPC nodes. The server load could go down if clients stop polling in a loop. Thanks for working on this!