Skip to content

WIP: Delaying a validator set update by 1 block.#1638

Closed
jaekwon wants to merge 4 commits intodevelopfrom
jae/nnvalset
Closed

WIP: Delaying a validator set update by 1 block.#1638
jaekwon wants to merge 4 commits intodevelopfrom
jae/nnvalset

Conversation

@jaekwon
Copy link
Contributor

@jaekwon jaekwon commented May 29, 2018

No description provided.

@jaekwon jaekwon requested review from ebuchman and melekes as code owners May 29, 2018 08:03
privValidator types.PrivValidator // for signing votes

// services for creating and executing blocks
// TODO: encapsulate all of this in one "BlockManager"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no managers plz.

state/state.go Outdated
// Note that if s.LastBlockHeight causes a valset change,
// we set s.LastHeightValidatorsChanged = s.LastBlockHeight + 1
Validators *types.ValidatorSet
NextNextValidators *types.ValidatorSet
Copy link
Contributor

Choose a reason for hiding this comment

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

why not Validators and NextValidators ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, it should be Validators, esp since State is a field of ConsensusState & the way it's used there.

@codecov-io
Copy link

Codecov Report

Merging #1638 into develop will decrease coverage by 0.06%.
The diff coverage is 82.35%.

@@             Coverage Diff             @@
##           develop    #1638      +/-   ##
===========================================
- Coverage    61.51%   61.44%   -0.07%     
===========================================
  Files          121      121              
  Lines        11095    11101       +6     
===========================================
- Hits          6825     6821       -4     
- Misses        3649     3656       +7     
- Partials       621      624       +3
Impacted Files Coverage Δ
consensus/state.go 75.9% <ø> (ø) ⬆️
state/validation.go 35.08% <0%> (-1.28%) ⬇️
rpc/core/status.go 0% <0%> (ø) ⬆️
state/state.go 88.73% <100%> (+3.01%) ⬆️
consensus/replay.go 54.12% <100%> (ø) ⬆️
state/execution.go 70.85% <100%> (ø) ⬆️
state/store.go 70.4% <100%> (+0.72%) ⬆️
consensus/reactor.go 71.88% <0%> (-1.08%) ⬇️
blockchain/pool.go 68.05% <0%> (-0.7%) ⬇️

@ebuchman ebuchman mentioned this pull request Jun 8, 2018
19 tasks
@kansi
Copy link

kansi commented Jun 12, 2018

It would be helpful if you could point to the rationale behind this change?

@jaekwon
Copy link
Contributor Author

jaekwon commented Jun 16, 2018

Hi @kansi, thank you for bringing this up.

The transactions in the LastBlock are sufficient for validators to know what the next validator should be, because it can update the validator set based on the validator updates returned by EndBlock. But light clients don't know that, so if the application wants to change the validator set completely 100%, a general purpose tendermint light client can't validate it because it doesn't know how to interpret the txs which caused the validator changes.

Delaying validator set updates by 1 block allows the next pending validator set to be included in the block header as "NextValidators", so light clients can know how the validator set changed for a blockchain, even with arbitrary validator set changes.

See #1756 which includes the changes to the "lite" packages which can now handle arbitrary validator set changes.

@jaekwon
Copy link
Contributor Author

jaekwon commented Jun 23, 2018

Closing, replaced by #1756

@jaekwon jaekwon closed this Jun 23, 2018
@xla xla deleted the jae/nnvalset branch June 28, 2018 09:14
@xla xla restored the jae/nnvalset branch June 28, 2018 09:14
@xla xla deleted the jae/nnvalset branch September 18, 2018 20:27
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.

4 participants