Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Implement rollback command #11179

Merged
merged 1 commit into from
Mar 3, 2022
Merged

Conversation

yihuang
Copy link
Collaborator

@yihuang yihuang commented Feb 14, 2022

Description

Closes: #10281

Currently it has panic: StoreBlockHeight (6) > StateBlockHeight + 1 (5) error when start after rollback. fixed after update tendermint.


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

}
rs.pruneStores()

if i < target {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be <=?

Copy link
Collaborator Author

@yihuang yihuang Feb 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be <=?

no, it's to check if latest height is changed, I just fixed it.

height, hash, err := tmcmd.RollbackState(ctx.Config)
if err != nil {
return fmt.Errorf("failed to rollback tendermint state: %w", err)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, we need to be sure that both tendermint an SDK will rollback to the same version. In RollbackToVersion we can essentially rollback to an older version than the target.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RollbackToVersion only prune heights that are bigger than the height returned by tendermint's rollback.

@bradyjoestar
Copy link
Contributor

I run a 4-peers simd cluster, rollbacked the peer3 data and restarted it.
The rollback cmd is:

simd  rollback  --home ~/cosmos_testnet/validators/validator3/simd/

It reported the following errors:


3:57AM INF starting ABCI with Tendermint
3:57AM INF starting service impl=multiAppConn module=proxy service=multiAppConn
3:57AM INF starting service connection=query impl=localClient module=abci-client service=localClient
3:57AM INF starting service connection=snapshot impl=localClient module=abci-client service=localClient
3:57AM INF starting service connection=mempool impl=localClient module=abci-client service=localClient
3:57AM INF starting service connection=consensus impl=localClient module=abci-client service=localClient
3:57AM INF starting service impl=EventBus module=events service=EventBus
3:57AM INF starting service impl=PubSub module=pubsub service=PubSub
3:57AM INF starting service impl=IndexerService module=txindex service=IndexerService
3:57AM INF ABCI Handshake App Info hash="����\v���Ѫz�\bC�%�0\b��I�Y����b���" height=5 module=consensus protocol-version=0 software-version=0.43.0-beta1-777-g1408c6d21
3:57AM INF ABCI Replay Blocks appHeight=5 module=consensus stateHeight=4 storeHeight=5
3:57AM INF Replay last block using mock app module=consensus
Error: error during handshake: error on replay: wrong Block.Header.AppHash.  Expected 2DB43C7B3D3556C02EDACCBEB650717FD60819CF066AF07E06407638BA9651F3, got 1504D73ED2FC4FBFB759C2948670A2DC8A34A20201256BB3FB2BEE5FD1199E2E
Usage:
  simd start [flags]

Flags:
      --abci string                                     specify abci transport (socket | grpc) (default "socket")
      --address string                                  Listen address (default "tcp://0.0.0.0:26658")
      --blocksync.enable                                enable fast blockchain syncing (default true)
      --consensus.create-empty-blocks                   set this to false to only produce blocks when there are txs or when the AppHash changes (default true)
      --consensus.create-empty-blocks-interval string   the possible interval between empty blocks (default "0s")

@yihuang
Copy link
Collaborator Author

yihuang commented Feb 17, 2022

I run a 4-peers simd cluster, rollbacked the peer3 data and restarted it.

I just fixed a trivial issue, can you try again.

PS: can you share your script to setup devnet, my script don't work with tendermint 0.35 somehow, and I haven't figured it out.

I always get this error when I backport this patch to 0.45.x and test with that:

"StoreBlockHeight (9) > StateBlockHeight + 1 (8)"

@bradyjoestar
Copy link
Contributor

PS: can you share your script to setup devnet, my script don't work with tendermint 0.35 somehow, and I haven't figured it out.

yeah, I will try it later.
Here is the setup scripts.
https://github.com/bradyjoestar/cosmos_rollback

@yihuang
Copy link
Collaborator Author

yihuang commented Feb 17, 2022

PS: can you share your script to setup devnet, my script don't work with tendermint 0.35 somehow, and I haven't figured it out.

yeah, I will try it later. Here is the setup scripts. https://github.com/bradyjoestar/cosmos_rollback

after fixing the rollback in tendermint: tendermint/tendermint#7837, it works now.

@yihuang yihuang changed the title Implement rollback command feat: Implement rollback command Feb 17, 2022
go.mod Outdated Show resolved Hide resolved
@tac0turtle
Copy link
Member

@yihuang 0.35.2 was released with the patch, can you update then we merge this

@@ -905,6 +905,30 @@ func (rs *Store) buildCommitInfo(version int64) *types.CommitInfo {
}
}

// RollbackToVersion delete the versions after `target` and update the latest version.
func (rs *Store) RollbackToVersion(target int64) int64 {
if target < 0 {
Copy link
Contributor

@JayT106 JayT106 Mar 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the input target should be always greater than 0 right? the State.InitialHeight will be 1.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I think so, just for safety here.

Closes: cosmos#10281

fix tendermint rollback

changelog

update tendermint to recent v0.35.x branch
@yihuang
Copy link
Collaborator Author

yihuang commented Mar 3, 2022

@yihuang 0.35.2 was released with the patch, can you update then we merge this

done

@yihuang
Copy link
Collaborator Author

yihuang commented Mar 3, 2022

@yihuang 0.35.2 was released with the patch, can you update then we merge this

shall we backport this to 0.45?

@tac0turtle tac0turtle merged commit 8296ad9 into cosmos:master Mar 3, 2022
mergify bot pushed a commit that referenced this pull request Mar 3, 2022
Closes: #10281

fix tendermint rollback

changelog

update tendermint to recent v0.35.x branch

(cherry picked from commit 8296ad9)

# Conflicts:
#	CHANGELOG.md
#	go.mod
#	go.sum
@yihuang yihuang deleted the rollback-cmd branch March 3, 2022 15:00
yihuang added a commit that referenced this pull request Mar 3, 2022
Closes: #10281

fix tendermint rollback

changelog

update tendermint to recent v0.35.x branch

(cherry picked from commit 8296ad9)

# Conflicts:
#	CHANGELOG.md
#	go.mod
#	go.sum
@alexanderbez
Copy link
Contributor

So there aren't any tests for this. Was this tested to actually work?

Because in my attempt I came across IAVL issues: #11233 (comment). Did you not come across this?

This should not have been merged w/o unit tests for the multi-store IMO.

@yihuang
Copy link
Collaborator Author

yihuang commented Mar 4, 2022

So there aren't any tests for this. Was this tested to actually work?

Because in my attempt I came across IAVL issues: #11233 (comment). Did you not come across this?

This should not have been merged w/o unit tests for the multi-store IMO.

I did the basic test manually:

  • start a devnet
  • stop devnet at a certain height
  • call ./simd rollback --home path/to/node0, it show the height - 1.
  • start devnet and observe node0's log, there's:
executed block height=5
stopping...
starting...
ABCI Replay Blocks appHeight=4 module=consensus stateHeight=4 storeHeight=5
executed block height=5
...
executed block height=6
...

@alexanderbez
Copy link
Contributor

I don't see how that's possible. IAVL doesn't let you prune/delete the current version (height).

@tac0turtle
Copy link
Member

@alexanderbez I was able to test this successfully locally on a chain that I submitted txs to in order to change the app hash a few times.

tac0turtle pushed a commit that referenced this pull request Mar 7, 2022
Closes: #10281

fix tendermint rollback

changelog

update tendermint to recent v0.35.x branch

(cherry picked from commit 8296ad9)

# Conflicts:
#	CHANGELOG.md
#	go.mod
#	go.sum

Co-authored-by: yihuang <huang@crypto.com>
@alexanderbez
Copy link
Contributor

That doesn't this should've been merged w/o ms unit tests.

@yihuang
Copy link
Collaborator Author

yihuang commented Mar 8, 2022

That doesn't this should've been merged w/o ms unit tests.

#11333

yihuang added a commit to yihuang/cosmos-sdk that referenced this pull request Mar 10, 2022
Closes: cosmos#10281

fix tendermint rollback

changelog

update tendermint to recent v0.35.x branch

(cherry picked from commit 8296ad9)

Co-authored-by: yihuang <huang@crypto.com>
zakir-code pushed a commit to PundiAI/cosmos-sdk that referenced this pull request Apr 8, 2022
Closes: cosmos#10281

fix tendermint rollback

changelog

update tendermint to recent v0.35.x branch

(cherry picked from commit 8296ad9)

# Conflicts:
#	CHANGELOG.md
#	go.mod
#	go.sum

Co-authored-by: yihuang <huang@crypto.com>
JimLarson pushed a commit to agoric-labs/cosmos-sdk that referenced this pull request Jul 7, 2022
Closes: cosmos#10281

fix tendermint rollback

changelog

update tendermint to recent v0.35.x branch

(cherry picked from commit 8296ad9)

# Conflicts:
#	CHANGELOG.md
#	go.mod
#	go.sum

Co-authored-by: yihuang <huang@crypto.com>
randy75828 pushed a commit to Switcheo/cosmos-sdk that referenced this pull request Aug 10, 2022
Closes: cosmos#10281

fix tendermint rollback

changelog

update tendermint to recent v0.35.x branch

(cherry picked from commit 8296ad9)

# Conflicts:
#	CHANGELOG.md
#	go.mod
#	go.sum

Co-authored-by: yihuang <huang@crypto.com>
randy75828 pushed a commit to Switcheo/cosmos-sdk that referenced this pull request Aug 10, 2022
Closes: cosmos#10281

fix tendermint rollback

changelog

update tendermint to recent v0.35.x branch

(cherry picked from commit 8296ad9)

# Conflicts:
#	CHANGELOG.md
#	go.mod
#	go.sum

Co-authored-by: yihuang <huang@crypto.com>
tomtau pushed a commit to crypto-org-chain/cosmos-sdk that referenced this pull request Sep 2, 2022
Closes: cosmos#10281

fix tendermint rollback

changelog

update tendermint to recent v0.35.x branch

(cherry picked from commit 8296ad9)

Co-authored-by: yihuang <huang@crypto.com>
JeancarloBarrios pushed a commit to agoric-labs/cosmos-sdk that referenced this pull request Sep 28, 2024
Closes: cosmos#10281

fix tendermint rollback

changelog

update tendermint to recent v0.35.x branch

(cherry picked from commit 8296ad9)

# Conflicts:
#	CHANGELOG.md
#	go.mod
#	go.sum

Co-authored-by: yihuang <huang@crypto.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add rollback support in the event of an incorrect hash
6 participants