refactor module_store method

[rnbguy]

basecoin-rs/basecoin/app/src/abci/v0_38/tendermint.rs

Lines 124 to 137 in 57f47f2

	for IdentifiedModule { id, module } in modules.iter_mut() {
	module
	.store_mut()
	.commit()
	.expect("failed to commit to state");
	let mut state = self.store.write_access();
	state
	.set(id.clone().into(), module.store().root_hash())
	.expect("failed to update sub-store commitment");
	}
	let mut state = self.store.write_access();
	let data = state.commit().expect("failed to commit to state");

Currently, the basecoin app uses a single tree for the whole app state. So, this essentially increases the chain height multiple times.

Also, we update the module identifier paths to their recent module storage hash - which may look like never saturating. As in, after each commit, the module storage hashes will change.

This is not wrong - as the height is still monotonically increasing, but it is inefficient. Ideally, there should be a top tree storage, and each module should have its sub-tree storage at its identifier. The proofs for such tree structures will be chained.

This required the MainStorage to be modified to look something like this.

pub struct MainStorage<S> {
  main: S,
  modules: Map<ModuleIdentifier, S>
}

---

I was wrong. The current implementation already does this.

My confusion came from the fact the module stores are initialized as follows,

basecoin-rs/basecoin/src/runner.rs

Lines 20 to 24 in 57f47f2

	// instantiate the application with a KV store implementation of choice
	let app_builder = Builder::new(GrowingStore::<InMemoryStore>::default());
	// instantiate modules and setup inter-module communication (if required)
	let auth = Auth::new(app_builder.module_store(&prefix::Auth {}.identifier()));

My thought was, why app_builder.module_store(&prefix::Auth {}.identifier()) is called here if it is not using the same store for the application and also the modules.

And it shouldn't. If the module is absent in app_builder, app_builder.module_store creates a new store and returns it. That's how the individual module stores are initialized.

basecoin-rs/basecoin/app/src/builder.rs

Lines 32 to 39 in 57f47f2

	pub fn module_store(&self, prefix: &Identifier) -> SharedStore<ModuleStore<S>> {
	let modules = self.modules.read_access();
	modules
	.iter()
	.find(|m| &m.id == prefix)
	.map(|IdentifiedModule { module, .. }| module.store().share())
	.unwrap_or_else(|| SharedStore::new(ModuleStore::new(S::default())))
	}

Rather, the modules should be initialized as follows to avoid confusion.

let auth = Auth::new(SharedStore::new(ModuleStore::new(S::default())));

And module_store should be refactored to return Option.

 pub fn module_store(&self, prefix: &Identifier) -> Option<SharedStore<ModuleStore<S>>> { 
     let modules = self.modules.read_access(); 
     modules 
         .iter() 
         .find(|m| &m.id == prefix) 
         .map(|IdentifiedModule { module, .. }| module.store().share())
 }