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(lib/grandpa): Include equivocatory nodes while creating justification #1911

Merged
merged 46 commits into from
Nov 24, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
ef6546f
feat: include equivocatory nodes while creating just
EclesioMeloJunior Oct 20, 2021
35fea30
chore: remove unecessary error return
EclesioMeloJunior Oct 20, 2021
1ceb49d
Merge branch 'development' into eclesio/include-eqv-votes
EclesioMeloJunior Oct 22, 2021
64b96fc
chore: undo condition format
EclesioMeloJunior Oct 22, 2021
c2bf981
chore: use equivocatory votes on threshold count
EclesioMeloJunior Oct 26, 2021
5039268
chore: add unsupported subround error
EclesioMeloJunior Oct 26, 2021
0100140
Merge branch 'development' into eclesio/include-eqv-votes
EclesioMeloJunior Oct 26, 2021
e2bafd8
Merge branch 'development' into eclesio/include-eqv-votes
EclesioMeloJunior Nov 1, 2021
b627509
Merge branch 'development' into eclesio/include-eqv-votes
EclesioMeloJunior Nov 2, 2021
d626dae
chore: remove unecessary check
EclesioMeloJunior Nov 2, 2021
b0f3c27
Merge branch 'development' into eclesio/include-eqv-votes
EclesioMeloJunior Nov 3, 2021
7d21850
Merge branch 'development' into eclesio/include-eqv-votes
EclesioMeloJunior Nov 3, 2021
e6299b5
chore: remove eqv votes and use the eqv voters
EclesioMeloJunior Nov 3, 2021
bbe8ba7
Merge branch 'eclesio/include-eqv-votes' of github.com:ChainSafe/goss…
EclesioMeloJunior Nov 3, 2021
0ed4ce5
Merge branch 'development' into eclesio/include-eqv-votes
EclesioMeloJunior Nov 3, 2021
a5f4684
chore: remove unecessary convertion
EclesioMeloJunior Nov 3, 2021
c9c278c
Merge branch 'eclesio/include-eqv-votes' of github.com:ChainSafe/goss…
EclesioMeloJunior Nov 3, 2021
0274ef9
chore: export ErrUnsupportedSubround
EclesioMeloJunior Nov 3, 2021
0b0c524
chore: improve code reading
EclesioMeloJunior Nov 4, 2021
6e5dd0b
Merge branch 'development' into eclesio/include-eqv-votes
EclesioMeloJunior Nov 4, 2021
de8a4ed
Merge branch 'development' into eclesio/include-eqv-votes
EclesioMeloJunior Nov 11, 2021
f58afce
chore: add tests to grandpa.createJustification func
EclesioMeloJunior Nov 15, 2021
56f3d05
Merge branch 'development' into eclesio/include-eqv-votes
EclesioMeloJunior Nov 15, 2021
ad81602
chore: add tests to grandpa message handler files
EclesioMeloJunior Nov 15, 2021
c93a134
chore: fix ci warns
EclesioMeloJunior Nov 16, 2021
7abd69b
Merge branch 'development' into eclesio/include-eqv-votes
EclesioMeloJunior Nov 16, 2021
0301b49
chore: put back a condition to check enough precommits in the justifi…
EclesioMeloJunior Nov 17, 2021
ec61d8e
chore: check if the equivocatory votes are on justification
EclesioMeloJunior Nov 17, 2021
a7d9ecb
chore: fix the fakeAuthorities format
EclesioMeloJunior Nov 17, 2021
f1f4d04
chore: removing parallel and uncoment Err...
EclesioMeloJunior Nov 17, 2021
5730645
chore: adjusting the equivocatory and prevote testing
EclesioMeloJunior Nov 17, 2021
69ee68d
chore: fix lint warns
EclesioMeloJunior Nov 17, 2021
5091650
chore: address nit comments
EclesioMeloJunior Nov 19, 2021
349080b
chore: adjust test asserts
EclesioMeloJunior Nov 19, 2021
32856fa
chore: adjust naming
EclesioMeloJunior Nov 19, 2021
c10dc71
chore: mock time.Unix in tests
EclesioMeloJunior Nov 19, 2021
a0dd708
chore: add test to equivocatory voters on to VerifyBlockJustification
EclesioMeloJunior Nov 19, 2021
37b2ba3
Merge branch 'development' into eclesio/include-eqv-votes
EclesioMeloJunior Nov 22, 2021
add5820
chore: adjust testing
EclesioMeloJunior Nov 22, 2021
e3546d8
chore: check errors
EclesioMeloJunior Nov 23, 2021
9ac7015
chore: changing the style to checking ok variable
EclesioMeloJunior Nov 23, 2021
3a4ee28
Merge branch 'development' into eclesio/include-eqv-votes
EclesioMeloJunior Nov 23, 2021
c675cab
chore: fix conflicts and ignore testing lint warns
EclesioMeloJunior Nov 23, 2021
9490f66
chore: check err
EclesioMeloJunior Nov 23, 2021
f3abef8
chore: fix runtime not found problems
EclesioMeloJunior Nov 24, 2021
1bec0db
chore: removing unused imports
EclesioMeloJunior Nov 24, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 22 additions & 17 deletions lib/grandpa/grandpa.go
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,7 @@ func (s *Service) determinePreCommit() (*Vote, error) {
return &pvb, nil
}

// isFinalisable returns true is the round is finalisable, false otherwise.
// isFinalisable returns true if the round is finalisable, false otherwise.
func (s *Service) isFinalisable(round uint64) (bool, error) {
var pvb Vote
var err error
Expand Down Expand Up @@ -766,16 +766,10 @@ func (s *Service) finalise() error {
s.bestFinalCandidate[s.state.round] = bfc

// create prevote justification ie. list of all signed prevotes for the bfc
pvs, err := s.createJustification(bfc.Hash, prevote)
if err != nil {
return err
}
pvs := s.createJustification(bfc.Hash, prevote)

// create precommit justification ie. list of all signed precommits for the bfc
pcs, err := s.createJustification(bfc.Hash, precommit)
if err != nil {
return err
}
pcs := s.createJustification(bfc.Hash, precommit)

pcj, err := scale.Marshal(*newJustification(s.state.round, bfc.Hash, bfc.Number, pcs))
if err != nil {
Expand Down Expand Up @@ -810,43 +804,54 @@ func (s *Service) finalise() error {
// createJustification collects the signed precommits received for this round and turns them into
// a justification by adding all signed precommits that are for the best finalised candidate or
// a descendent of the bfc
func (s *Service) createJustification(bfc common.Hash, stage Subround) ([]SignedVote, error) {
func (s *Service) createJustification(bfc common.Hash, stage Subround) []SignedVote {
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the reasoning behind removing the error?

Copy link
Member Author

@EclesioMeloJunior EclesioMeloJunior Oct 26, 2021

Choose a reason for hiding this comment

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

Oh, my bad, I realized that the error assignment inside the .Range function has been used lines below. I'm adding it back

var (
spc *sync.Map
err error
just []SignedVote
eqv map[ed25519.PublicKeyBytes][]*SignedVote
)

switch stage {
Copy link
Contributor

Choose a reason for hiding this comment

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

should probably throw an error if you pass an unsupported Subround.

Copy link
Member Author

Choose a reason for hiding this comment

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

added

case prevote:
spc = s.prevotes
eqv = make(map[ed25519.PublicKeyBytes][]*SignedVote, len(s.pvEquivocations))
case precommit:
spc = s.precommits
eqv = make(map[ed25519.PublicKeyBytes][]*SignedVote, len(s.pcEquivocations))
}

// TODO: use equivacatory votes to create justification as well (#1667)
spc.Range(func(_, value interface{}) bool {
pc := value.(*SignedVote)
var isDescendant bool

isDescendant, err = s.blockState.IsDescendantOf(bfc, pc.Vote.Hash)
if err != nil {
return false
}

if !isDescendant {
} else if !isDescendant {
Copy link
Contributor

Choose a reason for hiding this comment

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

I kinda prefer the second if statement here.

return true
}

just = append(just, *pc)
return true
})

if err != nil {
return nil, err
for _, votes := range eqv {
for _, vote := range votes {
var signedVote SignedVote = *vote
qdm12 marked this conversation as resolved.
Show resolved Hide resolved
isDescendant, err := s.blockState.IsDescendantOf(bfc, signedVote.Vote.Hash)

if err != nil {
continue
} else if !isDescendant {
Copy link
Contributor

Choose a reason for hiding this comment

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

same here.

continue
}

just = append(just, signedVote)
}
}

return just, nil
return just
}

// derivePrimary returns the primary for the current round
Expand Down
1 change: 1 addition & 0 deletions lib/grandpa/message_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ func (h *MessageHandler) verifyJustification(just *SignedVote, round, setID uint

// verify authority in justification set
authFound := false

for _, auth := range h.grandpa.authorities() {
justKey, err := just.AuthorityID.Encode()
if err != nil {
Expand Down
8 changes: 4 additions & 4 deletions lib/grandpa/vote_message.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@ func (s *Service) receiveMessages(ctx context.Context) {
for {
select {
case msg, ok := <-s.in:
if msg == nil || msg.msg == nil {
continue
}

if !ok {
return
}

if msg == nil || msg.msg == nil {
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
continue
}

logger.Trace("received vote message", "msg", msg.msg, "from", msg.from)
vm := msg.msg

Expand Down