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.

Sign up for GitHub

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

Jump to bottom

fix(lib/blocktree): reimplement BestBlockHash to take into account primary blocks in fork choice rule #2254

Merged
merged 14 commits into from
Feb 1, 2022
Merged

fix(lib/blocktree): reimplement BestBlockHash to take into account primary blocks in fork choice rule #2254

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
acc43e6
initial impl of fork choice rule using primary/secondary slots
noot Jan 27, 2022
f63f073
add primaryAncestorCount unit test
noot Jan 27, 2022
ab2e531
add unit test for blocktree.best
noot Jan 27, 2022
32d0c23
update blocktree unit tests
noot Jan 28, 2022
70fcf27
add more test cases
noot Jan 28, 2022
12e1e17
fix dot/state tests
noot Jan 28, 2022
b76e211
Merge branch 'development' of github.com:ChainSafe/gossamer into noot…
noot Jan 31, 2022
16bc46f
fix sync tests
noot Jan 31, 2022
bf26f04
fix grandpa unit tests
noot Jan 31, 2022
a9e2c91
fix author rpc integration tests
noot Jan 31, 2022
787790f
address comments
noot Jan 31, 2022
a79d5df
fix integration tests
noot Jan 31, 2022
0ef9ea3
fix
noot Jan 31, 2022
6b841f4
update AddBlocksToStateWithFixedBranches
noot Jan 31, 2022
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
16 changes: 8 additions & 8 deletions dot/rpc/modules/chain_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,9 +387,15 @@ func newTestStateService(t *testing.T) *state.Service {
}

func loadTestBlocks(t *testing.T, gh common.Hash, bs *state.BlockState, rt runtime.Instance) {
digest := types.NewDigest()
prd, err := types.NewBabeSecondaryPlainPreDigest(0, 1).ToPreRuntimeDigest()
require.NoError(t, err)
err = digest.Add(*prd)
require.NoError(t, err)

header1 := &types.Header{
Number: big.NewInt(1),
Digest: types.NewDigest(),
Digest: digest,
ParentHash: gh,
StateRoot: trie.EmptyHash,
}
Expand All @@ -399,16 +405,10 @@ func loadTestBlocks(t *testing.T, gh common.Hash, bs *state.BlockState, rt runti
Body: sampleBodyBytes,
}

err := bs.AddBlock(block1)
err = bs.AddBlock(block1)
require.NoError(t, err)
bs.StoreRuntime(header1.Hash(), rt)

digest := types.NewDigest()
prd, err := types.NewBabeSecondaryPlainPreDigest(0, 1).ToPreRuntimeDigest()
require.NoError(t, err)
err = digest.Add(*prd)
require.NoError(t, err)

header2 := &types.Header{
Number: big.NewInt(2),
Digest: digest,
Expand Down
7 changes: 7 additions & 0 deletions dot/rpc/modules/childstate_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,11 +266,18 @@ func setupChildStateStorage(t *testing.T) (*ChildStateModule, common.Hash) {
err = st.Storage.StoreTrie(tr, nil)
require.NoError(t, err)

digest := types.NewDigest()
prd, err := types.NewBabeSecondaryPlainPreDigest(0, 1).ToPreRuntimeDigest()
require.NoError(t, err)
err = digest.Add(*prd)
require.NoError(t, err)

b := &types.Block{
Header: types.Header{
ParentHash: bb.Header.Hash(),
Number: big.NewInt(0).Add(big.NewInt(1), bb.Header.Number),
StateRoot: stateRoot,
Digest: digest,
},
Body: types.Body{},
}
Expand Down
7 changes: 7 additions & 0 deletions dot/rpc/modules/state_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -530,11 +530,18 @@ func setupStateModule(t *testing.T) (*StateModule, *common.Hash, *common.Hash) {
err = chain.Storage.StoreTrie(ts, nil)
require.NoError(t, err)

digest := types.NewDigest()
prd, err := types.NewBabeSecondaryPlainPreDigest(0, 1).ToPreRuntimeDigest()
require.NoError(t, err)
err = digest.Add(*prd)
require.NoError(t, err)

b := &types.Block{
Header: types.Header{
ParentHash: chain.Block.BestBlockHash(),
Number: big.NewInt(3),
StateRoot: sr1,
Digest: digest,
},
Body: *types.NewBody([]types.Extrinsic{[]byte{}}),
}
Expand Down
2 changes: 1 addition & 1 deletion dot/state/block.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ func (bs *BlockState) BestBlockHash() common.Hash {
return common.Hash{}
}

return bs.bt.DeepestBlockHash()
return bs.bt.BestBlockHash()
}

// BestBlockHeader returns the block header of the current head of the chain
Expand Down
21 changes: 11 additions & 10 deletions dot/state/block_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func TestGetBlockByNumber(t *testing.T) {
blockHeader := &types.Header{
ParentHash: testGenesisHeader.Hash(),
Number: big.NewInt(1),
Digest: types.NewDigest(),
Digest: createPrimaryBABEDigest(t),
}

block := &types.Block{
Expand All @@ -102,7 +102,7 @@ func TestAddBlock(t *testing.T) {
// Create header
header0 := &types.Header{
Number: big.NewInt(1),
Digest: types.NewDigest(),
Digest: createPrimaryBABEDigest(t),
ParentHash: testGenesisHeader.Hash(),
}
// Create blockHash
Expand All @@ -119,7 +119,7 @@ func TestAddBlock(t *testing.T) {
// Create header & blockData for block 2
header1 := &types.Header{
Number: big.NewInt(2),
Digest: types.NewDigest(),
Digest: createPrimaryBABEDigest(t),
ParentHash: blockHash0,
}
blockHash1 := header1.Hash()
Expand Down Expand Up @@ -244,6 +244,7 @@ func TestAddBlock_BlockNumberToHash(t *testing.T) {
Header: types.Header{
ParentHash: bestHash,
Number: big.NewInt(0).Add(bestHeader.Number, big.NewInt(1)),
Digest: createPrimaryBABEDigest(t),
},
Body: types.Body{},
}
Expand Down Expand Up @@ -324,7 +325,7 @@ func TestGetHashByNumber(t *testing.T) {

header := &types.Header{
Number: big.NewInt(1),
Digest: types.NewDigest(),
Digest: createPrimaryBABEDigest(t),
ParentHash: testGenesisHeader.Hash(),
}

Expand All @@ -347,7 +348,7 @@ func TestAddBlock_WithReOrg(t *testing.T) {

header1a := &types.Header{
Number: big.NewInt(1),
Digest: types.NewDigest(),
Digest: createPrimaryBABEDigest(t),
ParentHash: testGenesisHeader.Hash(),
}

Expand All @@ -366,7 +367,7 @@ func TestAddBlock_WithReOrg(t *testing.T) {

header1b := &types.Header{
Number: big.NewInt(1),
Digest: types.NewDigest(),
Digest: createPrimaryBABEDigest(t),
ParentHash: testGenesisHeader.Hash(),
ExtrinsicsRoot: common.Hash{99},
}
Expand All @@ -386,7 +387,7 @@ func TestAddBlock_WithReOrg(t *testing.T) {

header2b := &types.Header{
Number: big.NewInt(2),
Digest: types.NewDigest(),
Digest: createPrimaryBABEDigest(t),
ParentHash: header1b.Hash(),
ExtrinsicsRoot: common.Hash{99},
}
Expand All @@ -410,7 +411,7 @@ func TestAddBlock_WithReOrg(t *testing.T) {

header2a := &types.Header{
Number: big.NewInt(2),
Digest: types.NewDigest(),
Digest: createPrimaryBABEDigest(t),
ParentHash: header1a.Hash(),
}

Expand All @@ -424,7 +425,7 @@ func TestAddBlock_WithReOrg(t *testing.T) {

header3a := &types.Header{
Number: big.NewInt(3),
Digest: types.NewDigest(),
Digest: createPrimaryBABEDigest(t),
ParentHash: header2a.Hash(),
}

Expand Down Expand Up @@ -456,7 +457,7 @@ func TestAddBlockToBlockTree(t *testing.T) {

header := &types.Header{
Number: big.NewInt(1),
Digest: types.NewDigest(),
Digest: createPrimaryBABEDigest(t),
ParentHash: testGenesisHeader.Hash(),
}

Expand Down
1 change: 1 addition & 0 deletions dot/state/storage_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ func TestStorage_GetStorageByBlockHash(t *testing.T) {
ParentHash: testGenesisHeader.Hash(),
Number: big.NewInt(1),
StateRoot: root,
Digest: createPrimaryBABEDigest(t),
},
Body: *body,
}
Expand Down
28 changes: 24 additions & 4 deletions dot/state/test_helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
runtime "github.com/ChainSafe/gossamer/lib/runtime/storage"
"github.com/ChainSafe/gossamer/lib/trie"
"github.com/ChainSafe/gossamer/lib/utils"
"github.com/ChainSafe/gossamer/pkg/scale"

"github.com/stretchr/testify/require"
)
Expand All @@ -35,6 +36,23 @@ func NewInMemoryDB(t *testing.T) chaindb.Database {
return db
}

func createPrimaryBABEDigest(t *testing.T) scale.VaryingDataTypeSlice {
babeDigest := types.NewBabeDigest()
err := babeDigest.Set(types.BabePrimaryPreDigest{AuthorityIndex: 0})
require.NoError(t, err)

bdEnc, err := scale.Marshal(babeDigest)
require.NoError(t, err)

digest := types.NewDigest()
err = digest.Add(types.PreRuntimeDigest{
ConsensusEngineID: types.BabeEngineID,
Data: bdEnc,
})
require.NoError(t, err)
return digest
}

// branch tree randomly
type testBranch struct {
hash common.Hash
Expand Down Expand Up @@ -190,10 +208,11 @@ func AddBlocksToStateWithFixedBranches(t *testing.T, blockState *BlockState, dep
previousHash = branch.hash

for i := branch.depth; i < depth; i++ {
d, err := types.NewBabePrimaryPreDigest(0, uint64(i+j+99), [32]byte{}, [64]byte{}).ToPreRuntimeDigest()
require.NoError(t, err)
require.NotNil(t, d)
digest := types.NewDigest()
_ = digest.Add(types.PreRuntimeDigest{
Data: []byte{byte(i), byte(j), r},
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove r argument from function signature

})
_ = digest.Add(*d)

block := &types.Block{
Header: types.Header{
Expand All @@ -206,7 +225,7 @@ func AddBlocksToStateWithFixedBranches(t *testing.T, blockState *BlockState, dep
}

hash := block.Header.Hash()
err := blockState.AddBlockWithArrivalTime(block, arrivalTime)
err = blockState.AddBlockWithArrivalTime(block, arrivalTime)
require.Nil(t, err)

blockState.StoreRuntime(hash, rt)
Expand Down Expand Up @@ -244,6 +263,7 @@ func generateBlockWithRandomTrie(t *testing.T, serv *Service,
ParentHash: *parent,
Number: big.NewInt(bNum),
StateRoot: trieStateRoot,
Digest: createPrimaryBABEDigest(t),
},
Body: *body,
}
Expand Down
2 changes: 1 addition & 1 deletion dot/sync/chain_processor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func TestChainProcessor_HandleJustification(t *testing.T) {
d, err := types.NewBabeSecondaryPlainPreDigest(0, 1).ToPreRuntimeDigest()
require.NoError(t, err)
digest := types.NewDigest()
err = digest.Add(d)
err = digest.Add(*d)
require.NoError(t, err)

header := &types.Header{
Expand Down
8 changes: 7 additions & 1 deletion dot/sync/message_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,19 @@ func addTestBlocksToState(t *testing.T, depth int, blockState BlockState) {
previousNum, err := blockState.BestBlockNumber()
require.Nil(t, err)

digest := types.NewDigest()
prd, err := types.NewBabeSecondaryPlainPreDigest(0, 1).ToPreRuntimeDigest()
require.NoError(t, err)
err = digest.Add(*prd)
require.NoError(t, err)

for i := 1; i <= depth; i++ {
block := &types.Block{
Header: types.Header{
ParentHash: previousHash,
Number: big.NewInt(int64(i)).Add(previousNum, big.NewInt(int64(i))),
StateRoot: trie.EmptyHash,
Digest: types.NewDigest(),
Digest: digest,
},
Body: types.Body{},
}
Expand Down
28 changes: 28 additions & 0 deletions dot/types/babe.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,31 @@ func GetSlotFromHeader(header *Header) (uint64, error) {

return slotNumber, nil
}

// IsPrimary returns true if the block was authored in a primary slot, false otherwise.
func IsPrimary(header *Header) (bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can make this a receiver function on Header? Header.Primary() (bool, error)

if header == nil {
return false, fmt.Errorf("cannot have nil header")
}

if len(header.Digest.Types) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should there be a nil check for header here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I can add it, hopefully doesn't happen though lol

return false, fmt.Errorf("chain head missing digest")
}

preDigest, ok := header.Digest.Types[0].Value().(PreRuntimeDigest)
if !ok {
return false, fmt.Errorf("first digest item is not pre-digest: type=%T", header.Digest.Types[0].Value())
}

digest, err := DecodeBabePreDigest(preDigest.Data)
if err != nil {
return false, fmt.Errorf("cannot decode BabePreDigest from pre-digest: %s", err)
}

switch digest.(type) {
case BabePrimaryPreDigest:
return true, nil
default:
return false, nil
}
}
Loading