Validate account locks on buffering #34229
Conversation
apfitzge
force-pushed
the
bugfix/scheduler_verify_account_locks
branch
from
5830094
to
78a66d3
Compare
| { | ||
| saturating_add_assign!(self.count_metrics.num_dropped_on_validate_locks, 1); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
perhaps move this all the way up to ImmutableDeserializedPacket::new() or somewhere during packet receiving to drop bad packets before inserting into storage?
There was a problem hiding this comment.
This is done before we insert to storage.
We can't do it in ImmutableDeserializedPacket::new because we haven't resolved look up tables at that point.
This is immediately after we resolve look up tables, which I think is the earliest we can do this.
Potentially, it could/should be done in SanitizedTransaction::try_new but that's changing the definition of a SanitizedTransaction. Not sure we want to do that?
There was a problem hiding this comment.
I got confused by thread-local-MI and central-scheduler code. So this is somewhat parallel to unprocessed_transactino_storage::consume_scan_should_process_packet() where immmutable_deserialized_packet is sanitized then accounts locks checked, which should be OK then.
Leave SanitizedTransaction to solana-transaction-types crate :D, there maybe can add a type for "sanitized and account locks validated" type
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #34229 +/- ##
=======================================
Coverage 81.9% 81.9%
=======================================
Files 819 819
Lines 219304 219313 +9
=======================================
+ Hits 179700 179721 +21
+ Misses 39604 39592 -12 |
Problem
Prio-graph has an assumption that account locks are not duplicate - see crash #34221
prio-graph crate should be fixed, but these transactions are not valid and should be dropped as soon as reasonably possible.
Summary of Changes
Fixes #34221