relax the restriction of delete latest version #483
Conversation
Closes: cosmos#482 Update nodedb.go changelog
yihuang
force-pushed
the
delete-latest-version
branch
from
cd40d73
to
de3e0a4
Compare
| if !tree.VersionExists(version) { | ||
| return errors.Wrap(ErrVersionDoesNotExist, "") | ||
| } | ||
| if err := tree.ndb.DeleteVersion(version, true); err != nil { | ||
| if err := tree.ndb.DeleteVersion(version, false); err != nil { |
There was a problem hiding this comment.
Are you sure this works? You have to move every node in the merkle tree for the historical state at the prior height, to being in the live state section of node db
There was a problem hiding this comment.
I'm not sure, only did basic unit test in cosmos-sdk side here:cosmos/cosmos-sdk#11361, I need more understanding on internals of iavl I think.
Do you think there is an efficient way to achieve this rollback feature?
There was a problem hiding this comment.
I think we don't need to patch iavl at all, there's LoadVersionForOverwriting which is perfect for rollback state.
There was a problem hiding this comment.
We should also check if it will be pruned correctly.
There was a problem hiding this comment.
https://github.com/cosmos/cosmos-sdk/pull/11361/files#diff-54fe31c47e1d9d86bbaf2face4c464c1f5a74e3c81ca5ac9a27a25a157752592R56
it's checked in this unit test, that one can rollback and recreate new states.
Closes: #482
This is required for the rollback command to work in cosmos-sdk (cosmos/cosmos-sdk#11333 (comment)).
I'm not sure if there are other consequences of this, but my local test with rollback command seems fine using this change.