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

fix(lib/grandpa): on verifying block justification, compare given block hash with finalised hash #3081

Merged
merged 42 commits into from
Feb 10, 2023
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
a86dcb3
Fixing descendant search
kishansagathiya Jan 23, 2023
b20a8e2
fix lint
kishansagathiya Jan 23, 2023
9e7be4d
parent -> hash
kishansagathiya Jan 23, 2023
d0aa5f8
different approach for getting descendants
kishansagathiya Jan 24, 2023
32db55a
different approach for getting descendants
kishansagathiya Jan 25, 2023
532db0e
Merge branch 'development' into kishan/fix/syncing
kishansagathiya Jan 25, 2023
0f76155
fixed the use of bs.GetAllDescendants
kishansagathiya Jan 26, 2023
b7f0a8a
Merge branch 'development' into kishan/fix/syncing
kishansagathiya Jan 26, 2023
03f1ecb
temp
kishansagathiya Jan 30, 2023
3529a36
temp
kishansagathiya Jan 30, 2023
66fb9d1
Merge branch 'kishan/fix/syncing' into kishan/fix/getting-runtime-ins…
kishansagathiya Jan 30, 2023
57f5eb8
temo
kishansagathiya Jan 30, 2023
1aa8c74
Merge branch 'kishan/fix/syncing' into kishan/fix/getting-runtime-ins…
kishansagathiya Jan 30, 2023
90a2992
check hash on verifying block justification
kishansagathiya Jan 30, 2023
a8d2af7
some cleanup
kishansagathiya Jan 30, 2023
7d775fa
more clean up
kishansagathiya Jan 30, 2023
0f625fd
more cleanup
kishansagathiya Jan 30, 2023
d0ce09a
more cleanup
kishansagathiya Jan 30, 2023
533a1cf
Merge branch 'development' into kishan/fix/getting-runtime-instance
kishansagathiya Jan 30, 2023
75fc709
clean up
kishansagathiya Jan 31, 2023
b7490a9
Merge branch 'kishan/fix/getting-runtime-instance' of github.com:Chai…
kishansagathiya Jan 31, 2023
664136c
use get finalised hash
kishansagathiya Feb 2, 2023
94af433
added test
kishansagathiya Feb 2, 2023
919b1ce
Merge branch 'development' into kishan/fix/getting-runtime-instance
kishansagathiya Feb 2, 2023
1c9d9e2
fix lint
kishansagathiya Feb 2, 2023
6017fdd
Merge branch 'development' into kishan/fix/getting-runtime-instance
kishansagathiya Feb 2, 2023
fb0b5b3
remove returning justification from VerifyJustification
kishansagathiya Feb 3, 2023
5577798
Merge branch 'development' into kishan/fix/getting-runtime-instance
kishansagathiya Feb 3, 2023
fb5c8b7
fixing a few more things
kishansagathiya Feb 3, 2023
99c9d4b
Merge branch 'kishan/fix/getting-runtime-instance' of github.com:Chai…
kishansagathiya Feb 3, 2023
df39d57
Merge branch 'development' into kishan/fix/getting-runtime-instance
kishansagathiya Feb 6, 2023
25134e5
fixing a test
kishansagathiya Feb 6, 2023
df0f311
no need to continue in case of storedFinalisedHash == hash
kishansagathiya Feb 7, 2023
9df1a6b
Merge branch 'development' into kishan/fix/getting-runtime-instance
kishansagathiya Feb 7, 2023
feb0d43
Merge branch 'development' into kishan/fix/getting-runtime-instance
kishansagathiya Feb 8, 2023
94befd7
Merge branch 'development' into kishan/fix/getting-runtime-instance
kishansagathiya Feb 8, 2023
3c10fe3
fix tests, removed some t.Parallel
kishansagathiya Feb 9, 2023
c1daf15
Merge branch 'kishan/fix/getting-runtime-instance' of github.com:Chai…
kishansagathiya Feb 9, 2023
ca43b2a
Merge branch 'development' into kishan/fix/getting-runtime-instance
kishansagathiya Feb 9, 2023
c4e9108
fix lint`
kishansagathiya Feb 9, 2023
e11ba97
try without t.Parallel
kishansagathiya Feb 9, 2023
aad958b
Merge branch 'development' into kishan/fix/getting-runtime-instance
kishansagathiya Feb 10, 2023
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
3 changes: 2 additions & 1 deletion lib/blocktree/blocktree.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"time"

"github.com/ChainSafe/gossamer/dot/types"

"github.com/ChainSafe/gossamer/lib/common"
"github.com/disiqueira/gotree"
"github.com/prometheus/client_golang/prometheus"
Expand Down Expand Up @@ -513,7 +514,7 @@ func (bt *BlockTree) StoreRuntime(hash common.Hash, in Runtime) {
func (bt *BlockTree) GetBlockRuntime(hash common.Hash) (Runtime, error) {
ins := bt.runtimes.get(hash)
if ins == nil {
return nil, ErrFailedToGetRuntime
return nil, fmt.Errorf("%w for hash %s", ErrFailedToGetRuntime, hash)
}
return ins, nil
}
3 changes: 3 additions & 0 deletions lib/grandpa/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ var (
// ErrAuthorityNotInSet is returned when a precommit within a justification is signed by a key not in the authority set
ErrAuthorityNotInSet = errors.New("authority is not in set")

// errFinalisedBlocksMismatch is returned when we find another block finalised in the same set id and round
errFinalisedBlocksMismatch = errors.New("already have finalised block with the same setID and round")

errVoteToSignatureMismatch = errors.New("votes and authority count mismatch")
errVoteBlockMismatch = errors.New("block in vote is not descendant of previously finalised block")
errVoteFromSelf = errors.New("got vote from ourselves")
Expand Down
8 changes: 7 additions & 1 deletion lib/grandpa/message_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,13 @@ func (s *Service) VerifyBlockJustification(hash common.Hash, justification []byt
}

if has {
return nil, fmt.Errorf("already have finalised block with setID=%d and round=%d", setID, fj.Round)
storedFinalisedHash, err := s.blockState.GetFinalisedHash(fj.Round, setID)
if err != nil {
return nil, fmt.Errorf("getting finalised hash: %w", err)
}
if storedFinalisedHash != hash {
return nil, fmt.Errorf("%w, setID=%d and round=%d", errFinalisedBlocksMismatch, setID, fj.Round)
}
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
}

isDescendant, err := isDescendantOfHighestFinalisedBlock(s.blockState, fj.Commit.Hash)
Expand Down
64 changes: 64 additions & 0 deletions lib/grandpa/message_handler_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -960,6 +960,70 @@ func TestMessageHandler_VerifyBlockJustification_invalid(t *testing.T) {
require.EqualError(t, err, expectedErr)
}

func TestMessageHandler_VerifyBlockJustification_ErrFinalisedBlockMismatch(t *testing.T) {
t.Parallel()

kr, err := keystore.NewEd25519Keyring()
require.NoError(t, err)
aliceKeyPair := kr.Alice().(*ed25519.Keypair)

auths := []types.GrandpaVoter{
{
Key: *kr.Alice().Public().(*ed25519.PublicKey),
},
{
Key: *kr.Bob().Public().(*ed25519.PublicKey),
},
{
Key: *kr.Charlie().Public().(*ed25519.PublicKey),
},
}

gs, st := newTestService(t, aliceKeyPair)
err = st.Grandpa.SetNextChange(auths, 1)
require.NoError(t, err)

body, err := types.NewBodyFromBytes([]byte{0})
require.NoError(t, err)

block := &types.Block{
Header: *testHeader,
Body: *body,
}

err = st.Block.AddBlock(block)
require.NoError(t, err)

setID := uint64(0)
round := uint64(1)
number := uint32(1)

err = st.Block.SetFinalisedHash(block.Header.Hash(), round, setID)
require.NoError(t, err)

var testHeader2 = &types.Header{
ParentHash: testHeader.Hash(),
Number: 2,
Digest: newTestDigest(),
}

var testHash = testHeader2.Hash()
block2 := &types.Block{
Header: *testHeader2,
Body: *body,
}
err = st.Block.AddBlock(block2)
require.NoError(t, err)

// justification fails since there is already a block finalised in this round and set id
precommits := buildTestJustification(t, 18, round, setID, kr, precommit)
just := newJustification(round, testHash, number, precommits)
data, err := scale.Marshal(*just)
require.NoError(t, err)
_, err = gs.VerifyBlockJustification(testHash, data)
require.ErrorIs(t, err, errFinalisedBlocksMismatch)
}

func Test_getEquivocatoryVoters(t *testing.T) {
t.Parallel()

Expand Down
15 changes: 15 additions & 0 deletions lib/grandpa/mocks_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion lib/grandpa/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ type BlockState interface {
IsDescendantOf(parent, child common.Hash) (bool, error)
LowestCommonAncestor(a, b common.Hash) (common.Hash, error)
HasFinalisedBlock(round, setID uint64) (bool, error)
GetFinalisedHeader(uint64, uint64) (*types.Header, error)
GetFinalisedHeader(round, setID uint64) (*types.Header, error)
GetFinalisedHash(round, setID uint64) (common.Hash, error)
SetFinalisedHash(common.Hash, uint64, uint64) error
BestBlockHeader() (*types.Header, error)
GetHighestFinalisedHeader() (*types.Header, error)
Expand Down