Prune iavl tree using the same version

[chengwenxi]

Summary

Prune iavl tree using the same version

Problem Definition

When committing CommitMultiStore, each store is committed (using a self-increasing version), and the version is used to prune the historical versions.

If a store is loaded at the same time as CommitMultiStore is initialized, store.version is always equal to MultiStore.version. However, if the store are loaded after the CommitMultiStore is committed, their versions will be different, then we can't load the CommitMultiStore to the reserved historical version(Pruning.keepEvery set).

The test shows this problem visually:
https://github.com/chengwenxi/cosmos-sdk/blob/multiStore-test/store/store_test.go#L12-L40

There's nothing wrong with that right now, but we should consider this situation: if we want to add a new module dynamically in our app.

Proposal

We can prune iavl tree using the CommitMultiStore.version.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[alexanderbez]

Thanks @chengwenxi! Indeed, the iavl.Store keeps track of the version and the rootmulti.Store keeps track of lastCommitID. You're supposed to load the version after you've initialized the rootmulti.Store (just like in your test case or any app) and mounted all the stores/keys.

I suppose we never anticipated the need to mount and load another iavl.Store after the application has been bootstrapped. Can you provide some real-world uses cases that would need to mount and load a store dynamically?

In any case, instead of modifying Commit() to accept a version to prune, we should rather handle this in the rootmulti.Store (assuming the use cases are justified). e.g. After the rootmulti.Store has been mounted and loaded, future subsequent calls will pass the correct version to the underyling iavl.Store.

[chengwenxi]

Can you provide some real-world uses cases that would need to mount and load a store dynamically?

@alexanderbez I see that the upgrade module is ready, it's possible to mount a new module(using a new store) through the upgrade process.

[alexanderbez]

@chengwenxi with the upgrade module, the binary still needs to be restarted. That being the case, the new software version will have any new modules that it needs -- so they won't be mounted "dynamically", they'll be mounted normally.

[chengwenxi]

with the upgrade module, the binary still needs to be restarted.

@alexanderbez yeah, it needs to be restarted, but the versions of the old modules and new modules are still different(restart don't change anything), that's my mean about "dynamically".

Edit:
for example, the upgrade block height is 90000, all genesis modules' version is 90000, but the new modules' version will start from 1.

[alexanderbez]

@chengwenxi I don't really understand what you mean. Any new modules introduced will be handled and loaded the same way old modules are and in your example will start processing blocks at 90001.

[chengwenxi]

@alexanderbez Maybe I have a misunderstanding about the upgrade module. I think we can mount a new module without having to unsafe-reset-all the app state, is that right?

your example will start processing blocks at 90001.

so when we restart the new binary, the blocks will start from 90001 rather than 1.

[alexanderbez]

Correct. But there is no "dynamic" module loading. The new module will be introduced in the new binary and handled like any other existing module.

[chengwenxi]

Correct. But there is no "dynamic" module loading. The new module will be introduced in the new binary and handled like any other existing module.

OK, so I wrote a new test to explain what I mean:
https://github.com/chengwenxi/cosmos-sdk/blob/module-add-test/store/store_test.go#L17-L56

I think the upgrade process will be a similar way to introduce new modules.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[chengwenxi]

Resolved by #7415.