unified_scheduler_logic: replace get_account_locks_unchecked

[apfitzge]

Problem

• validate_account_locks function got pulled into account_locks from sdk
• following that change, get_account_locks and get_account_locks_unchecked will be deprecated
• unified-scheduler-logic crate uses get_account_locks_unchecked

Summary of Changes

• Replace use of get_account_locks_unchecked with more direct access

Fixes #

[apfitzge]

get rid of 2 allocations here

[apfitzge]

iteration + call to is_writable is what get_account_locks_unchecked was doing anyway.
So would expect performance to be slightly better since we are removing allocation and an iteration without adding additional work.

[ryoqun]

nit: method chain can be extended further:

let lock_contexts = transaction
    .message()
    .account_keys()
    ...

[apfitzge]

3f5297a

[ryoqun]

in fact, checking locks must be done prior to enqueuing tasks. that's because SchedulingStateMachine will cause dead locks on the existence of non-unique UsageQeueues inside a given Task.

So, the original comment was wrong and misleading... Nevertheless, this comment has been serving as a reminder for me. how about retaining it while being updated like this?:

// It's crucial for tasks to be validated with
// `SanitizedTransaction::validate_account_locks()` prior to the creation.
// That's because it's part of protocol consensus regarding the rejection of
// blocks containing malformed transactions (`AccountLoadedTwice` and
// `TooManyAccountLocks`). Even more,`SchedulingStateMachine` can't properly
// handle transactions with duplicate addresses (those falling under
// `AccountLoadedTwice`).
//
// However, it's okay for now not to call `::validate_account_locks()` here. 
//
// Currently the replaying stage is always calling `get_account_locks()`
// regardless unified scheduler is enabled or not at the blockstore 
// (`Bank::prepare_sanitized_batch()` is called in `process_entries()`).
// This verification will be hoisted for optimization
// when removing `--block-verificatio-method=blockstore-processor`.
//
// As for banking stage with unified scheduler, it will need to run
// `.validate_account_locks()` at least once somewhere in the code path.
// In the distant future, this function (`create_task()`) should be adjusted
// so that both stages do the checks before calling this (say, with some ad-hoc
// type like `SanitizedTransactionWithCheckedAccountLocks`) or do the checks
// here, to simplify the two code paths regarding the essential
// `validate_account_locks` validation.
//
// Lastly, `validate_account_locks()` is currently called in
// `DefaultTransactionHandler::handle()` via
// `Bank::prepare_unlocked_batch_from_single_tx()` as well.
// This redundancy is known. It was just left as-is out of abundance of caution.

(please reword and/or reformat as needed...)

[apfitzge]

108c35b

[ryoqun]

lgtm. thanks for updating the comment.

(ci is failing partly due to test_scheduler_drop_short_circuiting.... sorry about that. I'll work on it soon.)