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

Author a sibling block in case best block's slot is same as current slot #2827

Merged
merged 62 commits into from
Sep 26, 2022
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
5cd4263
temp
kishansagathiya Jul 18, 2022
f57fa84
temp
kishansagathiya Jul 18, 2022
ca4724f
Merge branch 'development' into kishan/fix/slot-number-must-increase
kishansagathiya Aug 2, 2022
12fde0b
temp
kishansagathiya Aug 3, 2022
ae2fa69
While authoring a new block, if we find that best block was authored in
kishansagathiya Aug 4, 2022
cc2ffaf
clean up
kishansagathiya Aug 4, 2022
cbc44d2
add a check for current slot is greater than slot in best block
kishansagathiya Aug 5, 2022
3de132f
test for err slot lagging
kishansagathiya Aug 16, 2022
a63a073
test
kishansagathiya Aug 17, 2022
aca3b68
fix test for errLaggingSlot
kishansagathiya Aug 17, 2022
9c9f47b
test that we create a fork if best block is authored in the same slot
kishansagathiya Aug 17, 2022
527f515
temp
kishansagathiya Aug 22, 2022
cecc308
fixed tests
kishansagathiya Aug 29, 2022
d022f43
Merge branch 'development' into kishan/fix/slot-number-must-increase
kishansagathiya Aug 29, 2022
540c09a
Author a sibling block in case best block's slot is same as current s…
kishansagathiya Sep 9, 2022
1edb86b
chore(dot): remove unneeded error functions (#2790)
qdm12 Sep 9, 2022
7fa9196
chore(wasmer): add helpers.go file with helper functions (#2749)
qdm12 Sep 9, 2022
64e3c03
Merge branch 'development' of github.com:ChainSafe/gossamer into deve…
kishansagathiya Sep 13, 2022
db03cbb
Merge branch 'development' into kishan/fix/slot-number-must-increase
kishansagathiya Sep 13, 2022
95a7a35
merge fix
kishansagathiya Sep 13, 2022
220f5ae
Merge branch 'development' of github.com:ChainSafe/gossamer into deve…
kishansagathiya Sep 14, 2022
37fa232
Merge branch 'development' into kishan/fix/slot-number-must-increase
kishansagathiya Sep 14, 2022
11471e8
intermediate
kishansagathiya Sep 15, 2022
e0109dc
it probably fixes the test
kishansagathiya Sep 15, 2022
93b3057
Merge branch 'development' into kishan/fix/slot-number-must-increase
kishansagathiya Sep 15, 2022
fdb1fd2
Use a real block import handler
kishansagathiya Sep 15, 2022
388678a
more fix
kishansagathiya Sep 16, 2022
0862104
improved createTestService
kishansagathiya Sep 16, 2022
c99b776
Merge branch 'development' into kishan/fix/slot-number-must-increase
kishansagathiya Sep 16, 2022
26f8769
reuse network mocks
kishansagathiya Sep 16, 2022
22d9b2c
more fix
kishansagathiya Sep 16, 2022
3e2aa7f
Update lib/babe/babe_integration_test.go
kishansagathiya Sep 19, 2022
1589dc9
Update lib/babe/babe_integration_test.go
kishansagathiya Sep 19, 2022
8ec533c
Update lib/babe/babe_integration_test.go
kishansagathiya Sep 19, 2022
8448863
Update lib/babe/babe.go
kishansagathiya Sep 19, 2022
ce9e195
Update lib/babe/babe_integration_test.go
kishansagathiya Sep 19, 2022
bcdf2b5
Update lib/babe/babe_integration_test.go
kishansagathiya Sep 19, 2022
9a9f26d
Update lib/babe/babe_integration_test.go
kishansagathiya Sep 19, 2022
b065fb8
Update lib/babe/babe_integration_test.go
kishansagathiya Sep 19, 2022
264db41
Update lib/babe/babe_integration_test.go
kishansagathiya Sep 19, 2022
24e69e6
Update dot/state/block_notify.go
kishansagathiya Sep 21, 2022
2fd728a
Update lib/babe/babe_integration_test.go
kishansagathiya Sep 21, 2022
31fa786
Merge branch 'development' into kishan/fix/slot-number-must-increase
kishansagathiya Sep 21, 2022
881eee0
Merge branch 'kishan/fix/slot-number-must-increase' of github.com:Cha…
kishansagathiya Sep 21, 2022
34800ee
Update lib/babe/babe.go
kishansagathiya Sep 21, 2022
8155d43
Update lib/babe/babe.go
kishansagathiya Sep 21, 2022
d0b2a1b
Update lib/babe/babe.go
kishansagathiya Sep 21, 2022
03db54d
Update lib/babe/babe_integration_test.go
kishansagathiya Sep 21, 2022
45b5fd3
Update lib/babe/babe_integration_test.go
kishansagathiya Sep 21, 2022
a71de07
Update lib/babe/errors.go
kishansagathiya Sep 21, 2022
37b41ca
fix
kishansagathiya Sep 21, 2022
532c617
split handleSlot function
kishansagathiya Sep 21, 2022
f56e154
Update lib/babe/babe_integration_test.go
kishansagathiya Sep 21, 2022
849ad82
Update lib/babe/babe_integration_test.go
kishansagathiya Sep 21, 2022
9ba1b58
separate mock interface
kishansagathiya Sep 22, 2022
8dbc936
Merge branch 'kishan/fix/slot-number-must-increase' of github.com:Cha…
kishansagathiya Sep 22, 2022
db736ba
some clean up
kishansagathiya Sep 22, 2022
3366ecc
Update lib/babe/babe_integration_test.go
kishansagathiya Sep 22, 2022
65f649e
added mock generate file
kishansagathiya Sep 26, 2022
e667450
Merge branch 'kishan/fix/slot-number-must-increase' of github.com:Cha…
kishansagathiya Sep 26, 2022
06313f1
fixed the mock generation command
kishansagathiya Sep 26, 2022
bb1a02c
Merge branch 'development' into kishan/fix/slot-number-must-increase
kishansagathiya Sep 26, 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
File renamed without changes.
File renamed without changes.
6 changes: 6 additions & 0 deletions dot/core/mocks_generate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Copyright 2022 ChainSafe Systems (ON)
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
// SPDX-License-Identifier: LGPL-3.0-only

package core

//go:generate mockgen -destination=mocks.go -package $GOPACKAGE . BlockState,StorageState,TransactionState,Network,EpochState,CodeSubstitutedState,RuntimeInstance
6 changes: 0 additions & 6 deletions dot/core/mocks_generate_test.go

This file was deleted.

File renamed without changes.
File renamed without changes.
1 change: 1 addition & 0 deletions dot/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ func (nb nodeBuilder) createBABEService(cfg *Config, st *state.Service, ks keyst
cs *core.Service, telemetryMailer telemetry.Client) (babe.ServiceIFace, error) {
return nb.createBABEServiceWithBuilder(cfg, st, ks, cs, telemetryMailer, babe.Builder{})
}

func (nodeBuilder) createBABEServiceWithBuilder(cfg *Config, st *state.Service, ks keystore.Keystore,
cs *core.Service, telemetryMailer telemetry.Client, newBabeService ServiceBuilder) (babe.
ServiceIFace, error) {
Expand Down
1 change: 1 addition & 0 deletions dot/state/block_notify.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ func (bs *BlockState) notifyImported(block *types.Block) {
}
}(ch)
}

kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
}

func (bs *BlockState) notifyFinalized(hash common.Hash, round, setID uint64) {
Expand Down
4 changes: 3 additions & 1 deletion dot/types/authority.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ import (

// Authority struct to hold authority data
type Authority struct {
Key crypto.PublicKey
Key crypto.PublicKey
// Weight exists for potential improvements in the protocol and could
// have a use-case in the future. In polkadot all authorities have the weight = 1.
Weight uint64
}

Expand Down
24 changes: 24 additions & 0 deletions lib/babe/babe.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,30 @@ func (b *Service) handleSlot(epoch, slotNum uint64,
return errNilParentHeader
}

atGenesisBlock := b.blockState.GenesisHash().Equal(parentHeader.Hash())
if !atGenesisBlock {
bestBlockSlotNum, err := b.blockState.GetSlotForBlock(parentHeader.Hash())
Comment on lines +468 to +469
Copy link
Contributor

Choose a reason for hiding this comment

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

nit maybe move the content of that if block to a separate function?

if err != nil {
return fmt.Errorf("could not get slot for block %s: %w", parentHeader.Hash(), err)
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
}

if bestBlockSlotNum > slotNum {
return errLaggingSlot
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
}

if bestBlockSlotNum == slotNum {
// pick parent of best block instead to handle slot
newParentHeader, err := b.blockState.GetHeader(parentHeader.ParentHash)
if err != nil {
return fmt.Errorf("could not get header for hash %s: %w", parentHeader.ParentHash, err)
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
}
if newParentHeader == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit 🤔 can this ever happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It probably isn't supposed to happen, but I have seen it happen.

Copy link
Contributor

Choose a reason for hiding this comment

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

Uh should we create an issue for that? Sounds kinda strange we can't find the parent header right?

return errNilParentHeader
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
}
parentHeader = newParentHeader
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
}
}

// there is a chance that the best block header may change in the course of building the block,
// so let's copy it first.
parent, err := parentHeader.DeepCopy()
Expand Down
236 changes: 200 additions & 36 deletions lib/babe/babe_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
package babe

import (
"path/filepath"
"testing"
"time"

Expand All @@ -15,17 +14,17 @@ import (
"github.com/ChainSafe/gossamer/dot/types"
"github.com/ChainSafe/gossamer/internal/log"
"github.com/ChainSafe/gossamer/lib/babe/mocks"
"github.com/ChainSafe/gossamer/lib/common"
"github.com/ChainSafe/gossamer/lib/crypto/sr25519"
"github.com/ChainSafe/gossamer/lib/genesis"
"github.com/ChainSafe/gossamer/lib/runtime"
rtstorage "github.com/ChainSafe/gossamer/lib/runtime/storage"
"github.com/ChainSafe/gossamer/lib/runtime/wasmer"
"github.com/ChainSafe/gossamer/lib/trie"
"github.com/ChainSafe/gossamer/lib/utils"
"github.com/ChainSafe/gossamer/pkg/scale"
"github.com/golang/mock/gomock"

mock "github.com/stretchr/testify/mock"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -80,37 +79,32 @@ func createTestService(t *testing.T, cfg ServiceConfig) *Service {

cfg.Telemetry = telemetryMock

if cfg.TransactionState == nil {
cfg.TransactionState = state.NewTransactionState(telemetryMock)
}

testDatadirPath := t.TempDir()
require.NoError(t, err)

var dbSrv *state.Service
if cfg.BlockState == nil || cfg.StorageState == nil || cfg.EpochState == nil {
config := state.Config{
Path: testDatadirPath,
LogLevel: log.Info,
Telemetry: telemetryMock,
}
dbSrv = state.NewService(config)
dbSrv.UseMemDB()
config := state.Config{
Path: testDatadirPath,
LogLevel: log.Info,
Telemetry: telemetryMock,
}
dbSrv = state.NewService(config)
dbSrv.UseMemDB()

err = dbSrv.Initialise(gen, genHeader, genTrie)
require.NoError(t, err)
err = dbSrv.Initialise(gen, genHeader, genTrie)
require.NoError(t, err)

err = dbSrv.Start()
require.NoError(t, err)
err = dbSrv.Start()
require.NoError(t, err)

t.Cleanup(func() {
_ = dbSrv.Stop()
})
t.Cleanup(func() {
_ = dbSrv.Stop()
})

cfg.BlockState = dbSrv.Block
cfg.StorageState = dbSrv.Storage
cfg.EpochState = dbSrv.Epoch
}
cfg.BlockState = dbSrv.Block
cfg.StorageState = dbSrv.Storage
cfg.EpochState = dbSrv.Epoch
cfg.TransactionState = dbSrv.Transaction
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved

var rtCfg wasmer.Config
rtCfg.Storage = rtstorage.NewTrieState(genTrie)
Expand All @@ -120,12 +114,7 @@ func createTestService(t *testing.T, cfg ServiceConfig) *Service {
require.NoError(t, err)

nodeStorage := runtime.NodeStorage{}
if dbSrv != nil {
nodeStorage.BaseDB = dbSrv.Base
} else {
nodeStorage.BaseDB, err = utils.SetupDatabase(filepath.Join(testDatadirPath, "offline_storage"), false)
require.NoError(t, err)
}
nodeStorage.BaseDB = dbSrv.Base

rtCfg.NodeStorage = nodeStorage
rt, err := wasmer.NewRuntimeFromGenesis(rtCfg)
Expand All @@ -136,6 +125,26 @@ func createTestService(t *testing.T, cfg ServiceConfig) *Service {
cfg.LogLvl = defaultTestLogLvl
babeService, err := NewService(&cfg)
require.NoError(t, err)

if cfg.BlockImportHandler == nil {
mockNetwork := core.NewMockNetwork(ctrl)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to keep this injected from each test?

Test code duplication is better than dark magic test helping code, which makes it hard to debug tests, are hard to read and also suggests to use gomock.Any()/AnyTimes() which are generally bad testing-depth wise.

mockNetwork.EXPECT().GossipMessage(gomock.Any()).AnyTimes()

coreConfig := core.Config{
BlockState: dbSrv.Block,
EpochState: dbSrv.Epoch,
StorageState: storageState,
TransactionState: dbSrv.Transaction,
Runtime: rt,
Keystore: rtCfg.Keystore,
Network: mockNetwork,
CodeSubstitutedState: dbSrv.Base,
CodeSubstitutes: make(map[common.Hash]string),
}

babeService.blockImportHandler = core.NewTestService(t, &coreConfig)
}

return babeService
}

Expand Down Expand Up @@ -257,17 +266,15 @@ func TestService_GetAuthorityIndex(t *testing.T) {
}

func TestStartAndStop(t *testing.T) {
serviceConfig := ServiceConfig{}
bs := createTestService(t, serviceConfig)
bs := createTestService(t, ServiceConfig{})
qdm12 marked this conversation as resolved.
Show resolved Hide resolved
err := bs.Start()
require.NoError(t, err)
err = bs.Stop()
require.NoError(t, err)
}

func TestService_PauseAndResume(t *testing.T) {
serviceConfig := ServiceConfig{}
bs := createTestService(t, serviceConfig)
bs := createTestService(t, ServiceConfig{})
err := bs.Start()
require.NoError(t, err)
time.Sleep(time.Second)
Expand All @@ -293,3 +300,160 @@ func TestService_PauseAndResume(t *testing.T) {
err = bs.Stop()
require.NoError(t, err)
}

func TestService_HandleSlotWithLaggingSlot(t *testing.T) {
cfg := ServiceConfig{
Authority: true,
Lead: true,
}
babeService := createTestService(t, cfg)

err := babeService.Start()
require.NoError(t, err)
defer func() {
_ = babeService.Stop()
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
}()

// add a block
parentHash := babeService.blockState.GenesisHash()
rt, err := babeService.blockState.GetRuntime(nil)
require.NoError(t, err)

epochData, err := babeService.initiateEpoch(testEpochIndex)
require.NoError(t, err)

ext := runtime.NewTestExtrinsic(t, rt, parentHash, parentHash, 0, "System.remark", []byte{0xab, 0xcd})
block := createTestBlock(t, babeService, emptyHeader, [][]byte{common.MustHexToBytes(ext)},
1, testEpochIndex, epochData)

babeService.blockState.AddBlock(block)
time.Sleep(babeService.constants.slotDuration * 1)
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved

header, err := babeService.blockState.BestBlockHeader()
require.NoError(t, err)

bestBlockSlotNum, err := babeService.blockState.GetSlotForBlock(header.Hash())
require.NoError(t, err)

slotnum := uint64(1)
slot := Slot{
start: time.Now(),
duration: 1 * time.Second,
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
number: slotnum,
}
testVRFOutputAndProof := &VrfOutputAndProof{}
preRuntimeDigest, err := types.NewBabePrimaryPreDigest(
0, slot.number,
testVRFOutputAndProof.output,
testVRFOutputAndProof.proof,
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
).ToPreRuntimeDigest()

require.NoError(t, err)

err = babeService.handleSlot(
babeService.epochHandler.epochNumber,
bestBlockSlotNum-1,
babeService.epochHandler.epochData.authorityIndex,
preRuntimeDigest)

require.ErrorIs(t, err, errLaggingSlot)
}

func TestService_HandleSlotWithSameSlot(t *testing.T) {
alice := keyring.Alice().(*sr25519.Keypair)
bob := keyring.Bob().(*sr25519.Keypair)

// Create babe service for alice
cfgAlice := ServiceConfig{
Authority: true,
Lead: true,
Keypair: alice,
}
cfgAlice.AuthData = []types.Authority{
{
Key: alice.Public().(*sr25519.PublicKey),
Weight: 1,
},
{
Key: bob.Public().(*sr25519.PublicKey),
Weight: 1,
},
}
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved

// Create babe service for bob
cfgBob := ServiceConfig{
Authority: true,
Lead: true,
Keypair: bob,
}
cfgBob.AuthData = []types.Authority{
{
Key: alice.Public().(*sr25519.PublicKey),
Weight: 1,
},
{
Key: bob.Public().(*sr25519.PublicKey),
Weight: 1,
},
}
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved

babeServiceBob := createTestService(t, cfgBob)

err := babeServiceBob.Start()
require.NoError(t, err)
defer func() {
_ = babeServiceBob.Stop()
}()

// wait till bob creates a block
time.Sleep(babeServiceBob.constants.slotDuration * 1)
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
require.NoError(t, err)

block, err := babeServiceBob.blockState.GetBlockByNumber(1)
require.NoError(t, err)

err = babeServiceBob.Stop()
require.NoError(t, err)

time.Sleep(babeServiceBob.constants.slotDuration * 1)
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved

babeServiceAlice := createTestService(t, cfgAlice)

// Add block created by Bob to Alice
err = babeServiceAlice.blockState.AddBlock(block)
require.NoError(t, err)

time.Sleep(babeServiceBob.constants.slotDuration * 1)
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved

bestBlockHeader, err := babeServiceAlice.blockState.BestBlockHeader()
require.NoError(t, err)
require.Equal(t, block.Header.Hash().String(), bestBlockHeader.Hash().String())
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved

// If the slot we are claiming is same as slot in best header, test that we don't
// through any error and can claim slot.
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
bestBlockSlotNum, err := babeServiceAlice.blockState.GetSlotForBlock(bestBlockHeader.Hash())
require.NoError(t, err)

slot := Slot{
start: time.Now(),
duration: 1 * time.Second,
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
number: bestBlockSlotNum,
}
testVRFOutputAndProof := &VrfOutputAndProof{}
preRuntimeDigest, err := types.NewBabePrimaryPreDigest(
0, slot.number,
testVRFOutputAndProof.output,
testVRFOutputAndProof.proof,
kishansagathiya marked this conversation as resolved.
Show resolved Hide resolved
).ToPreRuntimeDigest()
require.NoError(t, err)

// slot gets occupied even if it has been occupied by a block
// authored by someone else
err = babeServiceAlice.handleSlot(
testEpochIndex,
bestBlockSlotNum,
0,
preRuntimeDigest)
require.NoError(t, err)

}
Loading