solana-labs / solana

Web-Scale Blockchain for fast, secure, scalable, decentralized apps and marketplaces.
https://solanalabs.com
Apache License 2.0
13.11k stars 4.24k forks source link

SendTransactionService sends in random order #35134

Open apfitzge opened 8 months ago

apfitzge commented 8 months ago

Problem

Proposed Solution

apfitzge commented 8 months ago

@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.

CriesofCarrots commented 8 months ago

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.

apfitzge commented 8 months ago

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.