Clear Transient Store in EndBlock instead of Commit

[rigelrozanski]

Simultaneous to this we should also turn off commit's during simulation, which really slows things down and needed to be turned on in #2310

CC @mossid

Some convo's around this:

@rigelrozanski
I think organizationally speaking it makes more sense to exist in Commit as that’s the point for the store operations (this is the current implementation of Transient Store). EndBlock I see more for application logic closing operations. It would be feasible for it to exist in either I believe though
@ValarDragon:
I kind of feel like it would make sense to clear the Transient Store in EndBlock, as its really just there to provide a performance increase for things the app may want to query at various times intra-block. Commit is for persisting what must needs to be written disk. (Though I do see your point, as its still very much in the realm of things Commit handles)
Given that theres a motivation for clearing the transient store in EndBlock though (faster simulations, which I think is valuable), I think it we should move it there.
(This shouldn't block your PR being merged post-review though!)

[mossid]

I'm currently working on #2309 so after then transientStore will be exposed. EndBlock can read a KVStore from the Context then typeassert it to TransientStore, being able to manually Wipe() on it.

One problem here is that we are going to wrap the store with GasKVStore, PrefixStore, etc, making it hard to typeassert. Maybe we can add a method like KVStore.Base which returns the innermost store.

[alexanderbez]

Can all the wrappers just delegate the Type method to their "parent"?

[mossid]

By Type you mean Store.GetStoreType()?

[alexanderbez]

Ahh yes @mossid 👍

[alexanderbez]

@rigelrozanski any updates on you share on this?

[rigelrozanski]

I think this can be postlaunch potentially even - the only reason this was ever a priority is because we needed TendermintUpdates (in staking) to be cleared from the transient store for the simulations - but because this is no longer relevant after the staking refactor, this is not priority - HOWEVER what is still priority is removing commits from the simulation - this should be prelaunch.

[ValarDragon]

Don't we still need this to remove commits from the simulation, since there is a transient store key used for params?

[rigelrozanski]

ah maybe - we probably should update the sim to be as close to real execution as possible

[jackzampolin]

Is this still something we want to do? I'm going to move this to if have time...

[rigelrozanski]

👍 this is def. if-have-time

[rigelrozanski]

CC @mossid - is it now possible to easily implement this?

[jackzampolin]

I don't know if this is still needed. Going to go ahead and close this issue. Please reopen with actionable steps if needed.