Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to filter transactions pre-sigverify when accounts hit write-compute limit #24657

Closed
carllin opened this issue Apr 25, 2022 · 4 comments
Assignees
Labels
stale [bot only] Added to stale content; results in auto-close after a week.

Comments

@carllin
Copy link
Contributor

carllin commented Apr 25, 2022

Problem

Once an account hits the write-compute limit, we need to throttle the number of transactions that touch that account.

Proposed Solution

  1. Create shared map account_tx_throttle: HashMap<Pubkey, (Atomic<usize>, timestamp)> between banking threads and sigverify
  2. Banking thread that hits the account-write compute limit for a particular account check if an Atomic<usize> exists for that account already. If not, add it to the map
  3. The banking thread that hits the account-write compute limit sets atomic in account_tx_throttle to be the fee-per-cu of the transaction that hit the compute limit. Call this fee-per_cu value the throttle_minimum.
  4. Sigverify drops all transactions that write to that account if the fee-per-cu is less than the throttle_minimum
  5. Clear the throttles every two seconds

cc @aeyakovenko

@buffalu
Copy link
Contributor

buffalu commented May 15, 2022

Wrote this in discord but this could encourage more spam bc it’d drop transactions that could have fit in the next block but didn’t fit in the current one. If I saw this happening to my arb bot I’d spam a little more

@buffalu
Copy link
Contributor

buffalu commented May 15, 2022

If done for all 4 slots like @sakridge mentions in discord that’d make more sense

@aeyakovenko
Copy link
Member

Use a fixed sized vec So we don’t need a write lock. Collisions there are fine as long as they are machine diversified.

@tao-stones
Copy link
Contributor

If done for all 4 slots like @sakridge mentions in discord that’d make more sense

Tracking accounts write CU limits cross all leader slots is a good solution to discourage spamming, but also changes the scope of this task.

Instead of leader reports saturated account while building block for current slot, it now needs to predict if account(s) will be saturated by packets in banking buffer. This requires to adjust 4 slots' cu limit utilization as packets inserted and dropped (eg being out-prioritized) from banking buffer; which in turn requires to sanitize transactions and loaded account addresses at point of receive_and_buffer_packets.

On the other side in sigverify stage, packets are also required to be deserialized/sanitized/load-address in order to determine if packet writes to saturated accounts.

So the prerequisites would be:

  1. Deserialize packet, sanitize transaction and load account addresses pre-sigverify Deserialize packet, sanitize transaction and load account addresses pre-sigverify #27782
  2. Sigverify stage pushes sanitized transactions to downstream stages Sigverify stage pushes sanitized transactions to downstream stages #27783

@github-actions github-actions bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Sep 15, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale [bot only] Added to stale content; results in auto-close after a week.
Development

No branches or pull requests

4 participants