Skip to content

Enforce validators can only use the correct pubkey type#2739

Merged
ebuchman merged 12 commits intodevelopfrom
dev/enforce_validator_key_type
Nov 28, 2018
Merged

Enforce validators can only use the correct pubkey type#2739
ebuchman merged 12 commits intodevelopfrom
dev/enforce_validator_key_type

Conversation

@ValarDragon
Copy link
Contributor

@ValarDragon ValarDragon commented Nov 1, 2018

The first commit on this branch mirrors #2714 before the commit chaos happened.

What remains to be done:

  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Updated CHANGELOG_PENDING.md

@codecov-io
Copy link

codecov-io commented Nov 1, 2018

Codecov Report

Merging #2739 into develop will increase coverage by <.01%.
The diff coverage is 79.16%.

@@             Coverage Diff             @@
##           develop    #2739      +/-   ##
===========================================
+ Coverage    62.68%   62.68%   +<.01%     
===========================================
  Files          212      212              
  Lines        17305    17327      +22     
===========================================
+ Hits         10847    10862      +15     
- Misses        5548     5552       +4     
- Partials       910      913       +3
Impacted Files Coverage Δ
libs/common/string.go 81.08% <100%> (+4.41%) ⬆️
state/execution.go 74.89% <70.58%> (-1.42%) ⬇️
blockchain/pool.go 78.83% <0%> (-0.69%) ⬇️
consensus/reactor.go 65.76% <0%> (-0.12%) ⬇️
p2p/pex/pex_reactor.go 72.15% <0%> (+0.31%) ⬆️
p2p/pex/addrbook.go 69.39% <0%> (+0.48%) ⬆️

@ValarDragon ValarDragon force-pushed the dev/enforce_validator_key_type branch from dced7f8 to 4c62d24 Compare November 1, 2018 05:02
@ValarDragon
Copy link
Contributor Author

ValarDragon commented Nov 1, 2018

I'm kind of confused about the pre-existing situation about how errors are handled in execution.go. If one validator update fails, won't all subsequent validator updates fail? Really we just want that one update to fail. I think that issue is distinct from this one (i.e. shouldn't block this PR), though I am still confused by it.

@ValarDragon ValarDragon requested a review from zramsay as a code owner November 1, 2018 05:11
Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

🍃 🌴 🌤

@@ -85,6 +85,7 @@ type ConsensusParams struct {
TxSize
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh damn, this needs to be updated - TxSize and BlockGossip were removed!

Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

💎

@melekes melekes added the T:breaking Type: Breaking Change label Nov 27, 2018
Copy link
Contributor

@ebuchman ebuchman left a comment

Choose a reason for hiding this comment

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

Thanks Dev!

@ebuchman ebuchman merged commit 4571f0f into develop Nov 28, 2018
@ebuchman ebuchman deleted the dev/enforce_validator_key_type branch November 28, 2018 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T:breaking Type: Breaking Change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants