Skip to content

WIP: update for sdk2 libs. need to fix kv test#1074

Closed
ebuchman wants to merge 26 commits intodevelopfrom
sdk2
Closed

WIP: update for sdk2 libs. need to fix kv test#1074
ebuchman wants to merge 26 commits intodevelopfrom
sdk2

Conversation

@ebuchman
Copy link
Contributor

@ebuchman ebuchman commented Jan 6, 2018

replaces #1070

Changed branch name for consistency

@ebuchman ebuchman requested a review from melekes as a code owner January 6, 2018 20:57
@ebuchman ebuchman changed the title update for sdk2 libs. need to fix kv test WIP: update for sdk2 libs. need to fix kv test Jan 6, 2018
version: v1.0.0
- package: github.com/tendermint/abci
version: v0.9.0
version: sdk2
Copy link
Contributor

Choose a reason for hiding this comment

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

Am assuming these will be updated later on?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes

ebuchman and others added 11 commits January 14, 2018 22:40
encoded as %s. not sure this will work with raw bytes
```
panic: Cannot encode unregistered concrete type crypto.PubKeyEd25519. [recovered]
        panic: Cannot encode unregistered concrete type crypto.PubKeyEd25519.

goroutine 20 [running]:
testing.tRunner.func1(0xc4200c0690)
        /usr/lib/go-1.9/src/testing/testing.go:711 +0x2d2
panic(0xa77be0, 0xc42048a8b0)
        /usr/lib/go-1.9/src/runtime/panic.go:491 +0x283
github.com/tendermint/tendermint/state.(*ValidatorsInfo).Bytes(0xc42004ced8, 0xc4204925e0, 0xf, 0x10)
        /home/ubuntu/go/src/github.com/tendermint/tendermint/state/store.go:166 +0xa3
github.com/tendermint/tendermint/state.saveValidatorsInfo(0x10338c0, 0xc42048a7b0, 0x1, 0x1, 0xc42015b920)
        /home/ubuntu/go/src/github.com/tendermint/tendermint/state/store.go:219 +0x86
github.com/tendermint/tendermint/state.saveState(0x10338c0, 0xc42048a7b0, 0xb59658, 0xf, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /home/ubuntu/go/src/github.com/tendermint/tendermint/state/store.go:90 +0x82
github.com/tendermint/tendermint/state.SaveState(0x10338c0, 0xc42048a7b0, 0xb59658, 0xf, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /home/ubuntu/go/src/github.com/tendermint/tendermint/state/store.go:85 +0xaa
github.com/tendermint/tendermint/state.LoadStateFromDBOrGenesisDoc(0x10338c0, 0xc42048a7b0, 0xc4200a8460, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /home/ubuntu/go/src/github.com/tendermint/tendermint/state/store.go:55 +0x260
github.com/tendermint/tendermint/consensus.randConsensusNet(0x4, 0xb60021, 0x18, 0xc420492550, 0xca1598, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /home/ubuntu/go/src/github.com/tendermint/tendermint/consensus/common_test.go:358 +0x4c8
github.com/tendermint/tendermint/consensus.TestByzantine(0xc4200c0690)
        /home/ubuntu/go/src/github.com/tendermint/tendermint/consensus/byzantine_test.go:32 +0xce
```
@melekes
Copy link
Contributor

melekes commented Jan 24, 2018

@ebuchman want me to rebase this against latest develop?

@ebuchman
Copy link
Contributor Author

ebuchman commented Jan 26, 2018

Probably need to try this again from scratch, not sure if its even worth rebasing. Let's get the new sdk2 branches in the dependencies merged in to their develops and then try again :(

Maybe we can salvage some of the commits. We'll see

@ebuchman ebuchman closed this Jan 26, 2018
@xla xla deleted the sdk2 branch September 18, 2018 20:28
cboh4 pushed a commit to scrtlabs/tendermint that referenced this pull request Apr 7, 2025
…nged` while rollback at a special height (backport tendermint#2136) (tendermint#2611)

closes: tendermint#2137 tendermint#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
<hr>This is an automatic backport of pull request tendermint#2136 done by
[Mergify](https://mergify.com).

Co-authored-by: Ethan <cosinlinker@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants