Backport v0.38.5: IAVL Bump (RC) & Pruning Refactor

[alexanderbez]

Backport:

• Pruning Refactor #6475

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

• 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.
• Code follows the module structure standards.
• Wrote unit and integration tests
• Updated relevant documentation (docs/) or specification (x/<module>/spec/)
• Added relevant godoc comments.
• Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
• Re-reviewed Files changed in the Github PR explorer

[fedekunze]

utACK. Pending tests and lint fixes

[zmanian]

So my mental model here is

This is breaking from a config api point of view but a node from this branch should be to sync from genesis on existing 0.38.5 network like Kava.

[alexanderbez]

@zmanian that is correct, yes. A few (go) client-facing breaking API changes, but the state machine is obviously intact. @erikgrinaker plans to write something up about how to make sure an upgrade goes smoothly without any data loss or corruption.

[codecov]

Codecov Report

Merging #6505 into backport/release/v0.38.x will increase coverage by 0.09%.
The diff coverage is 63.57%.

@@                     Coverage Diff                      @@
##           backport/release/v0.38.x    #6505      +/-   ##
============================================================
+ Coverage                     51.50%   51.60%   +0.09%     
============================================================
  Files                           336      337       +1     
  Lines                         20577    20624      +47     
============================================================
+ Hits                          10599    10642      +43     
- Misses                         9175     9176       +1     
- Partials                        803      806       +3     

[colin-axner]

LGTM, left an small observance - feel free to resolve if you don't think it needs to be addressed

[erikgrinaker]

I've written a function that can repair faulty orphans in databases written with IAVL 0.13.x: cosmos/iavl#282

It must be run after the database is closed with 0.13.x and before it is opened with 0.14 or later.

We can consider running this automatically, but it's potentially scary to automatically delete a bunch of stuff on IAVL load. It also cannot be run directly on a bare SDK database since it uses a PrefixDB on top. Maybe we should add a script or something that node operators can run?

[alexanderbez]

@erikgrinaker that's excellent! AFAIU, this patching would be voluntary and not necessary for a fully functional upgrade, it's just to be safe, right?

[erikgrinaker]

@erikgrinaker that's excellent! AFAIU, this patching would be voluntary and not necessary for a fully functional upgrade, it's just to be safe, right?

It depends on what the user does after the upgrade.

With KeepEvery: 10000, let's say the user stops at height 37000, upgrades, and resumes from height 30000 - if the user at any point deletes version 30000, then versions between 30000 and 37000 are very likely to experience data loss and corruption. If the values written in version 30000-37000 are different from what they were when they were processed before the upgrade then all versions after 30000 are vulnerable to data corruption.

If the user is able to stop at 30000, without ever having saved versions above that, upgrading is safe.

[alexanderbez]

Ok I think we definitely need very clear communication and documentation on this. We should encourage the use of this patch script OR have operators run archive mode for a bit before upgrading...or both?

[erikgrinaker]

I'll need to read up on the new pruning mechanism and defaults, and doublecheck a couple of things. There are configurations that would eventually flush out the invalid data on its own, without any corruption, as long as the user doesn't change anything. Will get back to you.

[erikgrinaker]

I've tested this with Kava, all the upgrade instructions are accurate and the failure modes are as expected. No failures were found after any successful upgrade following upgrade instructions.

Let's cut an RC!