From 006ad30de704d3560e196329e3cf3b96a738eec4 Mon Sep 17 00:00:00 2001
From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com>
Date: Thu, 14 Mar 2024 12:00:52 +0800
Subject: [PATCH 1/3] fix: `Rollback`: wrong modification of
`state.LastHeightValidatorsChanged` while rollback at a special height
(backport #2136) (#2611)
closes: #2137 #1074
## Description
Rollback at a specific height causes the node to fail to start. This is
caused by the error code:
https://github.com/cometbft/cometbft/blob/635d0b596c7300c98caa57e7aab26a7a4579ab97/internal/state/rollback.go#L71
According to the definition of the `State` struct as below
https://github.com/cometbft/cometbft/blob/635d0b596c7300c98caa57e7aab26a7a4579ab97/internal/state/state.go#L63
`LastHeightValidatorsChanged` satisfy the following condition under any
circumstances
1 <= `state.LastHeightValidatorsChanged` <= `state.LastBlockHeight + 1 +
1`
this condition can also be confirmed by looking at the code in `func
(store dbStore) save(state State, key []byte) error`
https://github.com/cometbft/cometbft/blob/635d0b596c7300c98caa57e7aab26a7a4579ab97/internal/state/store.go#L229
`state.LastHeightValidatorsChanged` is with `nextHeight +
1`(`state.LastBlockHeight + 1 + 1`) corresponds.
When we rollback from block height `state.LastHeightValidatorsChanged -
1` to height `state.LastHeightValidatorsChanged - 2`,
`newState.LastHeightValidatorsChanged` would be wrong to modified into
the `state.LastHeightValidatorsChanged - 1`
## Issue
Let me give an example to describe the issue:
### example
> ... => [block 242 (5validators)] ==state1(5validators)==> [block 243
(5validators)] ==state2(4validators)==> [block 244 (4validators)] => ...
1. Assume that the current height of the block is 243, current state is
`state2` and `state2.LastHeightValidatorsChanged` == 244, validatorSet
changed to 4 validators in the block 244 and state2
2. Rollback to block 242 and state1, the target `rollbackHeight` == 242
3. Since the error code,
https://github.com/cometbft/cometbft/blob/635d0b596c7300c98caa57e7aab26a7a4579ab97/internal/state/rollback.go#L71
`state1.LastHeightValidatorsChanged` was changed to 243 incorrectly and
it should be 244
4. After rollbacked to block 242, node could not start properly
## Solution
See code for details.
#### PR checklist
- [x] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [x] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
This is an automatic backport of pull request #2136 done by
[Mergify](https://mergify.com).
Co-authored-by: Ethan
---
state/rollback.go | 5 +++--
state/rollback_test.go | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/state/rollback.go b/state/rollback.go
index 6420192cf7..57ea039682 100644
--- a/state/rollback.go
+++ b/state/rollback.go
@@ -65,10 +65,11 @@ func Rollback(bs BlockStore, ss Store, removeBlock bool) (int64, []byte, error)
return -1, nil, err
}
+ nextHeight := rollbackHeight + 1
valChangeHeight := invalidState.LastHeightValidatorsChanged
// this can only happen if the validator set changed since the last block
- if valChangeHeight > rollbackHeight {
- valChangeHeight = rollbackHeight + 1
+ if valChangeHeight > nextHeight+1 {
+ valChangeHeight = nextHeight + 1
}
paramsChangeHeight := invalidState.LastHeightConsensusParamsChanged
diff --git a/state/rollback_test.go b/state/rollback_test.go
index dd9a2be53e..61bce2a892 100644
--- a/state/rollback_test.go
+++ b/state/rollback_test.go
@@ -262,7 +262,7 @@ func setupStateStore(t *testing.T, height int64) state.Store {
LastValidators: valSet,
Validators: valSet.CopyIncrementProposerPriority(1),
NextValidators: valSet.CopyIncrementProposerPriority(2),
- LastHeightValidatorsChanged: height + 1,
+ LastHeightValidatorsChanged: height + 1 + 1,
ConsensusParams: *params,
LastHeightConsensusParamsChanged: height + 1,
}
From 795e54b32da5a30edc5cb42c986d615af6f89132 Mon Sep 17 00:00:00 2001
From: Adam Tucker
Date: Mon, 25 Mar 2024 17:19:28 -0600
Subject: [PATCH 2/3] mergify
---
.github/mergify.yml | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/.github/mergify.yml b/.github/mergify.yml
index 5d98052284..a9937d9aca 100644
--- a/.github/mergify.yml
+++ b/.github/mergify.yml
@@ -4,24 +4,7 @@ defaults:
assignees:
- "{{ author }}"
-queue_rules:
- - name: default
- conditions:
- - "#approved-reviews-by>=1"
-
pull_request_rules:
- - name: Automerge to main
- conditions:
- - "#approved-reviews-by>=1"
- - base=osmo/v0.37.4
- - label=S:automerge
- actions:
- queue:
- method: squash
- name: default
- commit_message_template: |
- {{ title }} (#{{ number }})
- {{ body }}
- name: backport patches to v23 branch
conditions:
- base=osmo/v0.37.4
@@ -30,3 +13,11 @@ pull_request_rules:
backport:
branches:
- osmo-v23/v0.37.4
+ - name: backport patches to v24 branch
+ conditions:
+ - base=osmo/v0.37.4
+ - label=S:backport/v24
+ actions:
+ backport:
+ branches:
+ - osmo-v24/v0.37.4
From e228879e1d1c2b0ae430e9105070b47151c8ce4b Mon Sep 17 00:00:00 2001
From: Adam Tucker
Date: Mon, 25 Mar 2024 18:35:01 -0600
Subject: [PATCH 3/3] changelog, make sure CI runs on our main
---
.github/workflows/build.yml | 2 +-
.github/workflows/check-generated.yml | 2 +-
.github/workflows/docs-toc.yml | 2 +-
.github/workflows/e2e.yml | 2 +-
.github/workflows/govulncheck.yml | 2 +-
.github/workflows/lint.yml | 4 ++--
.github/workflows/tests.yml | 2 +-
CHANGELOG.md | 6 ++++++
8 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 7e5657361f..9212668a2b 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -6,7 +6,7 @@ on:
pull_request:
push:
branches:
- - v0.37.x
+ - osmo/v0.37.4
jobs:
build:
diff --git a/.github/workflows/check-generated.yml b/.github/workflows/check-generated.yml
index 377f3378b7..d7b71028e1 100644
--- a/.github/workflows/check-generated.yml
+++ b/.github/workflows/check-generated.yml
@@ -7,7 +7,7 @@ name: Check generated code
on:
pull_request:
branches:
- - v0.37.x
+ - osmo/v0.37.4
permissions:
contents: read
diff --git a/.github/workflows/docs-toc.yml b/.github/workflows/docs-toc.yml
index 225b33bbdb..4a3fcbf27c 100644
--- a/.github/workflows/docs-toc.yml
+++ b/.github/workflows/docs-toc.yml
@@ -4,7 +4,7 @@ on:
pull_request:
push:
branches:
- - v0.37.x
+ - osmo/v0.37.4
jobs:
check:
diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml
index 72afabc8e7..9bd7dfd5a6 100644
--- a/.github/workflows/e2e.yml
+++ b/.github/workflows/e2e.yml
@@ -6,7 +6,7 @@ on:
pull_request:
push:
branches:
- - v0.37.x
+ - osmo/v0.37.4
jobs:
e2e-test:
diff --git a/.github/workflows/govulncheck.yml b/.github/workflows/govulncheck.yml
index 305170a50c..ceeef2c8b1 100644
--- a/.github/workflows/govulncheck.yml
+++ b/.github/workflows/govulncheck.yml
@@ -8,7 +8,7 @@ on:
pull_request:
push:
branches:
- - v0.37.x
+ - osmo/v0.37.4
jobs:
govulncheck:
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index 98d74a82b3..093a1f1d7b 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -12,7 +12,7 @@ on:
pull_request:
push:
branches:
- - v0.37.x
+ - osmo/v0.37.4
jobs:
golangci:
@@ -23,7 +23,7 @@ jobs:
- uses: actions/checkout@v4
- uses: actions/setup-go@v4
with:
- go-version: '1.21'
+ go-version: "1.21"
- uses: technote-space/get-diff-action@v6
with:
PATTERNS: |
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
index ada5619270..b8cff42ec2 100644
--- a/.github/workflows/tests.yml
+++ b/.github/workflows/tests.yml
@@ -5,7 +5,7 @@ on:
paths:
- "**.go"
branches:
- - v0.37.x
+ - osmo/v0.37.4
jobs:
tests:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3001c71b4c..89b65f9bef 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,12 @@
## Unreleased
+## v0.37.4-v24-osmo-2
+
+* [#20](https://github.com/osmosis-labs/cometbft/pull/20) Fix the rollback command
+
+## v0.37.4-v24-osmo-1
+
* [#5](https://github.com/osmosis-labs/cometbft/pull/5) Batch verification
* [#11](https://github.com/osmosis-labs/cometbft/pull/11) Skip verification of commit sigs
* [#13](https://github.com/osmosis-labs/cometbft/pull/13) Avoid double-saving ABCI responses