near / nearcore

Reference client for NEAR Protocol
https://near.org
GNU General Public License v3.0
2.31k stars 618 forks source link

Figure out what to do with EpochManager medium-term #6910

Open matklad opened 2 years ago

matklad commented 2 years ago

Today, we store EpochManager in NightshadeRuntie and access it via RuntimeAdapter.

This is awkward – EpochManager is very stateful, while runtime ideally shouldn't be. It also logically doesn't feel right – runtime should not care about evolution of epoch, it should exist within scope of one chunk.

The historical context is that we had plans to move EpochManager on chain (https://github.com/near/nearcore/issues/2745).

I think we should just move EpochManager to chain:

trait RuntimeAdapter {
 fn run_builtin_contract(contract: AccountId, input: &[u8]) -> Result<Vec<u8>>;

that is, even if epoch manager lives on chain, runtime shouldn't know specifics of it, it should expose just a general hook to run arbitrary contract. It would be up to Chain to run that and interpret inputs/outputs as borsh-encoded epoch data.


There's additional, deep reason to move epoch manager to Chain – testing. Today, Chain is separated from Runtime via RuntimeAdapter. For testing purposes, it uses KeyValueRuntime, a simplistic runtime which can only process transfer transactions. However, as KeyValueRuntime lacks a true EpochManager, epoch-dependent tests have to be integration and have to use NightshadeRuntime.


Current status:

We now have EpochManagerAdapter which contains almost all of the stuff that EpcohManager does. At the moment, we still have RuntimeAdapter: EpochManagerAdapter. The next step is to remove that super-trait bound! that means that initially Chain would get a

struct Chain {
  runtime_adapter: Arc<dyn RuntimeAdapter>
  epoch_manager: Arc<dyn EpochManager>
}

which initially would point at the same object in memory.

matklad commented 2 years ago

cc https://docs.google.com/document/d/1YoudktXIG2C6XPKT0ZaR6PvUIbZlOLpyQQ3DnTPLlh4/edit#heading=h.7q5fm4mwl3p2

matklad commented 2 years ago

Note: while we probably want

struct Chain {
  em: EpochManager
}

this probably won't work right of the bat, as today we use separte chains but shared em for view client and client. So we should start with

struct Chain {
  em: Arc<Mutex<EpochManager>>,
}

Getting to end-state would require redesigning our client/view-client split.

matklad commented 2 years ago

More specific plan:

  1. Move SafeEpochManager type to some publicly visible place:

    https://github.com/near/nearcore/blob/161bf79c2fc636bba72f58263a8ac04af093368c/nearcore/src/runtime/mod.rs#L80-L91

  2. Specifically:

    • keep read/write methods, so that call-sites know that they are locking
    • keep it Arc/clone
    • make the .0 private
    • rename to something more reasonable. EpochManagerHandle?
  3. Add epoch_manager field to stuff which holds dyn RuntimeAdapter and initialize it to be the same object as the one in RuntimeAdapter. Notaly, Chain should be treated like this.

    At this stage, we should reach the situation where runtime, view client and clinet all have separate epoch_manager field which point to the same actual object.

  4. Slowly&Incrementally replace calls to RuntimeAdapter with calls to epoch_manager. Probably best done by removing one trait's method at a time.

  5. Remove EpochManager from NightShadeRuntime.

    At this point, we are done. Runtime (as in NighthsadeRuntime & RuntimeAdapter) doesn't have access to EpochManager. Clinet and ViewCLient share the same EpcohManager object, just like they used to before refactor ?. Perhaps at some point we need to introduce trait EpochManagerAdapter to break dependencies between crates. Perhaps not


Future steps:

matklad commented 2 years ago

TODOs for potential follow up: