mina icon indicating copy to clipboard operation
mina copied to clipboard

Remove usages of Mina_ledger.Ledger.Db.foldi from daemon

Open nholland94 opened this issue 1 year ago • 3 comments

The Mina_ledger.Ledger.Db.foldi is implemented in a very slow manner. In general, there shouldn't ever be a use case for iterating over the entire ledger in such a fashion from inside of the daemon's normal operations. The only time we should be doing that is for processing ledgers external, or perhaps exporting ledgers. It would be nice to optimize Mina_ledger.Ledger.Db.foldi to batch reads properly, but in lieu of such a change, it is easier to just removing remaining references to it in the daemon code. In particular, we use this operation unnecessarily during bootstrap, when we synchronize the epoch ledgers https://github.com/MinaProtocol/mina/blob/develop/src/lib/consensus/proof_of_stake.ml#L2863. Part of fixing this will revolve completely removing the current implementation of Sparse_ledger.of_any_ledger, which utilizes this very slow operation, and is akin to loading the entire on-disk ledger to an in-memory ledger (an operation we shouldn't allow).

Listing from https://github.com/MinaProtocol/mina/issues/13783#issuecomment-1686674076

  • [ ] Computing genesis ledger total currency src/lib/consensus/proof_of_stake.ml:40
  • [x] Computing staking ledger total currency as part of the delegation uptime compliance message (https://github.com/MinaProtocol/mina/pull/14913)
  • [ ] Computing the full list of accounts in the genesis ledger (lazily)

nholland94 avatar Jul 28 '23 18:07 nholland94

I removed one more instance of this function from bootstrap in this PR https://github.com/MinaProtocol/mina/pull/13885.

These are the remaining usages:

  • Computing genesis ledger total currency src/lib/consensus/proof_of_stake.ml:40
    • We do this when evaluating the negative one to genesis block transition, and when producing genesis epoch ledger states (so a total of 3x times during bootup of a daemon)
  • Computing staking ledger total currency as part of the delegation uptime compliance message src/app/delegation_compliance/delegation_compliance.ml:177
    • This one is silly and very easy to fix, as we already know the total currency of the staking ledger (stored in local state)
  • Computing the full list of accounts in the genesis ledger (lazily) src/lib/genesis_ledger/genesis_ledger.ml
    • This is only used in tests.

nholland94 avatar Aug 21 '23 16:08 nholland94

The delegation_compliance app can be deleted.

Note: it's not the total currency that's being computed, it's the total stake delegated to a particular account.

psteckler avatar Aug 21 '23 18:08 psteckler

Computing genesis ledger total currency src/lib/consensus/proof_of_stake.ml:40

  • We do this when evaluating the negative one to genesis block transition, and when producing genesis epoch ledger states (so a total of 3x times during bootup of a daemon)

This one might be worth fixing

deepthiskumar avatar Aug 28 '23 20:08 deepthiskumar