solana icon indicating copy to clipboard operation
solana copied to clipboard

SendTransactionService sends in random order

Open apfitzge opened this issue 4 months ago • 3 comments

Problem

  • SendTransactionService is the primary way user transactions get sent from RPC to leader
  • SendTransactionService does not consider transaction priority at all in the sending order or in the dropping order when the "queue" is full. It is simply done randomly.
  • Random order incentivizes spamming multiple RPCs in hopes to randomly get to the top of their retry map

Proposed Solution

  • SendTransactionService should have some sort of priority-queue that we use to drop lowest priority transactions, and send/retry transactions with higher priority first.
  • Because we want to consider priority, we should be reasonably sure txs can pay for fees. We already load and check nonce-accounts, it seems reasonable we could load and check fee-payer balance.

apfitzge avatar Feb 07 '24 20:02 apfitzge

@lijunwangs or @CriesofCarrots do you have thoughts on the proposed solution? It seems like the current behavior contributes to priority fees not working as well as they should, although there are certainly other factors at play.

apfitzge avatar Feb 07 '24 20:02 apfitzge

SendTransactionService should have some sort of priority-queue that we use to drop lowest priority transactions, and send/retry transactions with higher priority first.

This feels a little dogmatic to me. It should really be up to the RPC operators how they want to filter or prioritize transactions. We could think about ways we could expose controls or customization to them, however...

Because we want to consider priority, we should be reasonably sure txs can pay for fees. We already load and check nonce-accounts, it seems reasonable we could load and check fee-payer balance.

The SendTransactionService only checks nonce accounts for (a) nonce transaction and (b) after the initial send to determine when to stop retrying. So much more limited than a fee-payer check before send. Adding this account load to every transaction before send will add some latency to the service, we can bench to see how much. In assessing whether that's worth imposing, we should see whether RPC operators generally support skipPreflight, because such a fee-payer check is already done in simulation.

CriesofCarrots avatar Feb 08 '24 20:02 CriesofCarrots

This feels a little dogmatic to me. It should really be up to the RPC operators how they want to filter or prioritize transactions. We could think about ways we could expose controls or customization to them, however...

I'm all for letting things be modular and RPCs implementing their own logic, but we need to provide sane defaults, which the current one does not seem to be.

The SendTransactionService only checks nonce accounts for (a) nonce transaction and (b) after the initial send to determine when to stop retrying. So much more limited than a fee-payer check before send. Adding this account load to every transaction before send will add some latency to the service, we can bench to see how much. In assessing whether that's worth imposing, we should see whether RPC operators generally support skipPreflight, because such a fee-payer check is already done in simulation.

Happy to give RPCs ability to opt-out of fee-payer checks via some configuration similar to pre-flight. However, pre-flight checks do not seem to be sufficient since they are only checked before we send the transaction the first timess; balance may have changed and the RPC is spamming transactions to leader that can never be processed due to lack of funds.

Adding this account load to every transaction before send will add some latency to the service, we can bench to see how much.

Yeah, agree this needs to be benched to see the affect. I think you are right in calling out what I said, in that nonce'd checks are far more limited.

apfitzge avatar Feb 08 '24 22:02 apfitzge