AccountLocks: validate_account_locks

[apfitzge]

Problem

• We want account lock validation should be possible for any SVMMessage:
  • Code currently lives in SanitizedTransaction

Summary of Changes

• Add account_locks::validate_account_locks that takes AccountKeys and limit
• Add custom has_duplicates:
  • for >= 32 accounts, use a pre-allocated AHashSet
  • for < 32 accounts, use brute-force O(n^2) algorithm

Fixes #

[apfitzge]

See the final benchmark results (modified) below.
Instead of normal bench-suite I ran for batch size of 1 for various numbers of accounts.
I did this to confirm that 32 accounts was the sweet spot for switching over to using the hash set instead of O(n^2)

Benchmark Results

bench_lock_accounts/batch_size_1_locks_count_2
                        time:   [249.89 µs 250.13 µs 250.36 µs]
                        thrpt:  [4.0901 Melem/s 4.0939 Melem/s 4.0978 Melem/s]
                 change:
                        time:   [+0.8693% +1.0815% +1.2943%] (p = 0.00 < 0.05)
                        thrpt:  [-1.2777% -1.0699% -0.8618%]
                        Change within noise threshold.
Found 5 outliers among 100 measurements (5.00%)
  2 (2.00%) low mild
  3 (3.00%) high mild
bench_lock_accounts/batch_size_1_locks_count_4
                        time:   [342.27 µs 342.59 µs 342.90 µs]
                        thrpt:  [2.9863 Melem/s 2.9890 Melem/s 2.9918 Melem/s]
                 change:
                        time:   [-1.9893% -1.7873% -1.5844%] (p = 0.00 < 0.05)
                        thrpt:  [+1.6099% +1.8198% +2.0297%]
                        Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe
bench_lock_accounts/batch_size_1_locks_count_8
                        time:   [546.80 µs 547.51 µs 548.25 µs]
                        thrpt:  [1.8677 Melem/s 1.8703 Melem/s 1.8727 Melem/s]
                 change:
                        time:   [-0.4993% -0.2809% -0.0710%] (p = 0.01 < 0.05)
                        thrpt:  [+0.0711% +0.2817% +0.5018%]
                        Change within noise threshold.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) low mild
  1 (1.00%) high mild
bench_lock_accounts/batch_size_1_locks_count_16
                        time:   [1.0024 ms 1.0032 ms 1.0040 ms]
                        thrpt:  [1.0199 Melem/s 1.0208 Melem/s 1.0216 Melem/s]
                 change:
                        time:   [-0.9296% -0.7228% -0.5230%] (p = 0.00 < 0.05)
                        thrpt:  [+0.5258% +0.7281% +0.9383%]
                        Change within noise threshold.
Found 5 outliers among 100 measurements (5.00%)
  4 (4.00%) low mild
  1 (1.00%) high mild
bench_lock_accounts/batch_size_1_locks_count_32
                        time:   [1.8623 ms 1.8636 ms 1.8649 ms]
                        thrpt:  [549.09 Kelem/s 549.46 Kelem/s 549.84 Kelem/s]
                 change:
                        time:   [-7.1087% -6.9359% -6.7658%] (p = 0.00 < 0.05)
                        thrpt:  [+7.2568% +7.4528% +7.6527%]
                        Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
  2 (2.00%) high mild
  1 (1.00%) high severe
bench_lock_accounts/batch_size_1_locks_count_48
                        time:   [2.6437 ms 2.6461 ms 2.6484 ms]
                        thrpt:  [386.65 Kelem/s 386.99 Kelem/s 387.33 Kelem/s]
                 change:
                        time:   [-18.093% -17.911% -17.729%] (p = 0.00 < 0.05)
                        thrpt:  [+21.550% +21.819% +22.090%]
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild
bench_lock_accounts/batch_size_1_locks_count_64
                        time:   [3.4304 ms 3.4333 ms 3.4364 ms]
                        thrpt:  [297.99 Kelem/s 298.25 Kelem/s 298.51 Kelem/s]
                 change:
                        time:   [-26.656% -26.489% -26.319%] (p = 0.00 < 0.05)
                        thrpt:  [+35.720% +36.034% +36.345%]
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

[tao-stones]

Not too sure about duplicating implementation, it might be necessary. These two functions can be in separate module, more like util of account_locks, so it's easier to change. Not strong about it, up to you or other reviewers. Also, those two functions need test?

[tao-stones]

now we have duplicated implementations from SDK, not sure what is the best way to keep them synced in the future (in case there's more else if to add, for example). Maybe add comments on both places?

[apfitzge]

Now that you mention it - I should just deprecate the SDK one.

I moved it there at some point from bank.
No reason it needs to be there any more, and I actually would like to guarantee we're not calling it elsewhere!

[apfitzge]

ah nvm I just seperated the validate fn from get_account_locks.

I think with the recent changes we don't actually use the get_account_locks anymore (nor should we as it allocates). May need to deprecate that.

[apfitzge]

@tao-stones

There are several uses of the get_account_locks and validate_account_locks from SanitizedTransaction.

With the code in this PR I can remove / transition those uses to this new code - but there are several uses across different modules, many requiring some changes to fn args - which complicates this PR

wdyt of this plan?

1. merge this PR with duplciate logic (after I address your comments)
2. create PRs to use new code where needed
3. deprecate the get_account_locks and related fns (unsure if this has downstream effects)

[tao-stones]

sounds good, maybe can deprecate sdk version sooner

[tao-stones]

lgtm - duplicated implementation in SDK will be deprecated.