consensus: axe the intermediate accumulation pathway for OC

[AshwinSekar]

Problem

This optimization doesn't actually get us anything and is complex to reason about

Summary of Changes

Remove it and bank hash cache to ibrl

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.0%. Comparing base (2ba28f5) to head (f0cda73).

Additional details and impacted files

@@           Coverage Diff            @@
##           master   #10594    +/-   ##
========================================
  Coverage    83.0%    83.0%            
========================================
  Files         844      843     -1     
  Lines      316049   315942   -107     
========================================
- Hits       262470   262399    -71     
+ Misses      53579    53543    -36     

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[bw-solana]

I think we could save a layer of nesting here

let Some(epoch_stakes) = root_bank.epoch_stakes(epoch) else {
    break;
}

[AshwinSekar]

we're not actually looping here, instead of looping through all the slots we now split into 2:

• Lookup the last slot and track for OC
• loop through the remaining slots to update tracking related to the propagated check

the previous versions of the code would still loop through remaining slots if the last slot was in an epoch without stake info.

Technically this means it's a malicious vote, e.g. the final slot is > 2 epochs than the root bank, so I could be convinced to return early here.

[bw-solana]

brain short circuited.

I like the idea of sub-functioning this out so we can just do some early returns. Something like:

fn track_last_vote_slot(...) {
    if last_vote_slot <= root {
        return;
    }

    let Some(epoch_stakes) = root_bank.epoch_stakes(epoch) else {
        return;
    }

    ...
}

but given we likely want to backport this improvement, punt it to a follow-up

[bw-solana]

latest LGTM.

waiting to click Approve to give @t-nelson a chance to rip things apart for backport possibility

[mergify]

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

[t-nelson]

wfm

[t-nelson]

try to minimize renames like these for backport candidates in the future. they bloat the diff unnecessarily

[AshwinSekar]

noted

[t-nelson]

nit: now that we aren't looping, this could be returned directly from the block instead of setting a mutable temporary

[AshwinSekar]

agreed will clean this up on master in a follow up