Skip to content
This repository was archived by the owner on Feb 3, 2023. It is now read-only.
This repository was archived by the owner on Feb 3, 2023. It is now read-only.

Proposal for Context / State / Instance refactor #938

@maackle

Description

@maackle

Context refactor

By changing the way state, instance, and context get initialized, we can break the circular dependency between Context and State and move stateful stuff out of Context. Instance can own its State entirely, meaning the State can never be uninitialized since it is tightly coupled to an active, initialized Instance.

  • Remove the following fields from Context because they are already on Instance (but should be taken out of their Options)
    • state
    • action_channel
    • observer_channel
  • Remove these fields and add to Instance (still Optional)
    • container_api
    • signal_tx
  • Change chain_storage to chain, which is a ChainStore (see "State refactor" for rationale)

That leaves Context looking like this:

pub struct Context {
    pub agent_id: AgentId,
    pub logger: Arc<Mutex<Logger>>,
    pub persister: Arc<Mutex<Persister>>,
    pub chain: Arc<RwLock<ChainStore>>,
    pub dht_storage: Arc<RwLock<ContentAddressableStorage>>,
    pub eav_storage: Arc<RwLock<EntityAttributeValueStorage>>,
    pub network_config: JsonString,
}

Instance refactor

And instance now looks like this, with fewer Options:

pub struct Instance {
    state: Arc<RwLock<State>>,
    action_tx: SyncSender<ActionWrapper>,
    observer_tx: SyncSender<Observer>,
    signal_tx: Option<SyncSender<Signal>>,
    container_api: Option<Arc<RwLock<IoHandler>>>,
}

State refactor

Now, since the following fields are just aliases of each other, perhaps we can just remove them from State:
- State::agent::chain::content_storage -> Context::chain_storage
- State::dht::content_storage -> Context::dht_storage
- State::dht::meta_storage -> Context::eav_storage

It's proposed that Context just directly point to a ChainStore, which is necessary if we move it out of State. The rationale for this is that State is meant to be a true Redux state, meaning it is updated immutably through pure reducers. We are only pretending to do this if we are mutating a Arc<RwLock<_>> through a write lock, so we could just move these to the Context to reflect that, and keep State reserved for true local state.

With the other removals suggested above, State will look like this:

pub struct State {
    nucleus: NucleusState,
    agent: AgentState,
    dht: DhtStore,
    network: NetworkState,
    pub history: HashSet<ActionWrapper>,
}

pub struct AgentState {
	// Removed `chain`
    actions: HashMap<ActionWrapper, ActionResponse>,
    top_chain_header: Option<ChainHeader>,
}

pub struct DhtStore {
    // Removed `content_storage` and `meta_storage`
    actions: HashMap<ActionWrapper, Result<Address, HolochainError>>,
}

// No other changes

Moreover, we can represent this State using im-rs immutable data structures to get pure reducers -- which is the most compelling reason for moving the RwLocks out of State

Metadata

Metadata

Assignees

No one assigned

    Labels

    adrArchitecture Decision Records

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions