Allow substore migrations upon multistore loading

[ethanfrey]

This is needed for the regen upgrade module (#4233), as there are cases (eg. 0.34 -> 0.36) where a substore is renamed (distr -> distribution). The current master panics when trying to read the store with an unknown key. This no longer panics, and allows the user to pass in a set of rename steps to actually rename the store upon loading (so other data migration steps of individual items can function)

Provides a few different store loaders for testing and demonstration purposes. This actual functionality probably doesn't make too much sense to an end-user until #4233 is merged, but it may be needed in cases where one does want to rename stores before doing some debugging.

• Targeted PR against correct branch (see CONTRIBUTING.md)

• Linked to github-issue with discussion and accepted design OR link to spec that describes this work.

• Wrote tests

• Updated relevant documentation (docs/)

• Added a relevant changelog entry: clog add [section] [stanza] [message]

• rereviewed Files changed in the github PR explorer

---

For Admin Use:

• Added appropriate labels to PR (ex. wip, ready-for-review, docs)
• Reviewers Assigned
• Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

[codecov]

Codecov Report

Merging #4724 into master will decrease coverage by 0.05%.
The diff coverage is 54.44%.

@@            Coverage Diff             @@
##           master    #4724      +/-   ##
==========================================
- Coverage   54.12%   54.06%   -0.06%     
==========================================
  Files         272      272              
  Lines       17358    17425      +67     
==========================================
+ Hits         9395     9421      +26     
- Misses       7280     7319      +39     
- Partials      683      685       +2

[codecov]

Codecov Report

Merging #4724 into master will increase coverage by 0.07%.
The diff coverage is 73.83%.

@@            Coverage Diff             @@
##           master    #4724      +/-   ##
==========================================
+ Coverage   54.18%   54.25%   +0.07%     
==========================================
  Files         269      269              
  Lines       17225    17302      +77     
==========================================
+ Hits         9334     9388      +54     
- Misses       7201     7216      +15     
- Partials      690      698       +8

[alessio]

Blocking on additional unit testing to avoid coverage falling even further below what is generally acceptable. Else ACK on both functionality design and implementation.

[ethanfrey]

Thanks for the feedback @alessio I also see some code written in baseapp, but only tested in simapp. I can add some smaller tests for that.

Is there an easy way to see what code is not covered, so I can add targeted tests?

Click on the details link of codecov/patch which is my expected approach, doesn't give any info.

(Maybe this happens as the code is in a fork?)

[alessio]

(Maybe this happens as the code is in a fork?)

Yes, I think that that's the reason. I'd suggest to go with go test -cover - I usually leverage VSCode/Goland integration for more convenient visualization

[ethanfrey]

Added lots more tests, let me know.

[ethanfrey]

And good tip to use built-in vscode test coverage. What I wrote seems all green, except for many error return lines, like:

		if err != nil {
			return fmt.Errorf("Cannot read upgrade file %s: %v", upgradeInfoPath, err)
		}

This seems to be the case with the existing code base (and most go projects as well). It should be fine for now, but I'd be curious for a longer-term approach to ensure good error handling.

[fedekunze]

@ethanfrey is this still WIP?

[alexanderbez]

Looks great @ethanfrey. Left a few points of feedback :)

[rigelrozanski]

ACK pending Bez comments addressed

[ethanfrey]

Thanks for the reviews, I will make requested changes.

Open question about delete behavior (I will update godoc in any case), especially as the function is meant for the opinionated upgrade module.

[ethanfrey]

@fedekunze You can remove the WIP label. All code comments have been resolved (I think).

[ethanfrey]

I think this is ready now, and I just polished it.
If there are any docs (besides godoc) that I should update, please let me know.

[fedekunze]

LGTM

[ethanfrey]

Okay @alexanderbez I made the requested changes and rebased on the current master.

[alexanderbez]

utACK ⚡️