Replaces stable sort with unstable sort

[brooksprumo]

Problem

The validator_stakes used to distribute rent to validators is sorted by stake and then pubkey. The sorting is done currently with a stable sort which is unneeded for two reasons:

1. If a validator_stake matches another one, then that means they are both for the same validator and thus their relative order does not matter.
2. Since validator_stakes is created by collecting from a HashMap, the pre-sorted vector is not deterministic across the cluster, which means the stable sort itself is not actually stable (when viewed across the cluster).

Since a stable sort is (1) not needed, and (2) not possible, we can use a faster, unstable sort instead.

Summary of Changes

Unstable sort validator_stakes.

[behzadnouri]

couldn't this be:

use std::cmp::Reverse;
validator_stakes.sort_unstable_by_key(|&(pubkey, stake)| Reverse((stake, pubkey)));

[brooksprumo]

Neat, I wasn't aware of Reverse before. Yes this will work.

While implementing, I noticed that this version will require us to copy pubkey and stake for each a and b in every comparison of the sort. If I remove the & in the closure, it shows the issue:

error: lifetime may not live long enough
    --> runtime/src/bank.rs:4996:65
     |
4996 |         validator_stakes.sort_unstable_by_key(|(pubkey, stake)| Reverse((stake, pubkey)));
     |                                                ---------------- ^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2`
     |                                                |              |
     |                                                |              return type of closure is Reverse<(&'2 u64, &solana_sdk::pubkey::Pubkey)>
     |                                                has type `&'1 (solana_sdk::pubkey::Pubkey, u64)`

While hunting for a way to work around this, I found a rust-lang issue about it. Here's a link to one of its comments with the same error:
rust-lang/rust#34162 (comment)

The work-around currently seems to use sort_by_cached_key() instead, so we'd only have to pay for the copy once. However this has space-complexity drawbacks over the non-cached version, since we'd still need to copy all the pubkeys (only once, which is nice) and save them off on the side. The PR that merged sort_by_cached_key() was linked by the previous issue, which is how I found this work-around.

Do you happen to know of another work-around for sort_unstable_by_key() in this case? If not, I was thinking that the extra copies of the pubkeys might not be worth it, and then would leave the current implementation. Wdyt?

[behzadnouri]

If you want to avoid copying, then maybe:

validator_stakes.sort_by(|(pubkey1, staked1), (pubkey2, staked2)| {
    ((staked1, pubkey1)).cmp(&(staked2, pubkey2)).reverse()
});

would be more readable.

[behzadnouri]

If you want to avoid copying, then maybe:

validator_stakes.sort_by(|(pubkey1, staked1), (pubkey2, staked2)| {
    ((staked1, pubkey1)).cmp(&(staked2, pubkey2)).reverse()
});

would be more readable.

[brooksprumo]

Merging the PR as is, and will address cleanup in a subsequent PR. Thanks!