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

…lot (#2827)

While authoring a block in a given slot S, it could happen that best block was also authored in the slot S by some other author. In this case, we can't create a new block as a child of best block, because then we would have a chain where slot numbers don't increase.
In such scenario, we author a sibling block to best block. That is we author a block as a child of best block's parent.
After authoring such a block, we would have two blocks with same parent, same block number and same slot. This would create a fork, but that would eventually get resolved.

dot/services.go

@@ -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) {

dot/types/authority.go

@@ -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
 }
 

lib/babe/babe.go

@@ -454,32 +454,65 @@ func (b *Service) handleEpoch(epoch uint64) (next uint64, err error) {
 	return next, nil
 }
 
-func (b *Service) handleSlot(epoch, slotNum uint64,
-	authorityIndex uint32,
-	preRuntimeDigest *types.PreRuntimeDigest,
-) error {
+func (b *Service) getParentForBlockAuthoring(slotNum uint64) (*types.Header, error) {
 	parentHeader, err := b.blockState.BestBlockHeader()
 	if err != nil {
-		return err
+		return nil, fmt.Errorf("could not get best block header: %w", err)
 	}
 
 	if parentHeader == nil {
-		return errNilParentHeader
+		return nil, errNilParentHeader
+	}
+
+	atGenesisBlock := b.blockState.GenesisHash().Equal(parentHeader.Hash())
+	if !atGenesisBlock {
+		bestBlockSlotNum, err := b.blockState.GetSlotForBlock(parentHeader.Hash())
+		if err != nil {
+			return nil, fmt.Errorf("could not get slot for block: %w", err)
+		}
+
+		if bestBlockSlotNum > slotNum {
+			return nil, fmt.Errorf("%w: best block slot number is %d and got slot number %d",
+				errLaggingSlot, bestBlockSlotNum, slotNum)
+		}
+
+		if bestBlockSlotNum == slotNum {
+			// pick parent of best block instead to handle slot
+			newParentHeader, err := b.blockState.GetHeader(parentHeader.ParentHash)
+			if err != nil {
+				return nil, fmt.Errorf("could not get header: %w", err)
+			}
+			if newParentHeader == nil {
+				return nil, fmt.Errorf("%w: for block hash %s", errNilParentHeader, parentHeader.ParentHash)
+			}
+			parentHeader = newParentHeader
+		}
 	}
 
 	// 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()
 	if err != nil {
-		return err
+		return nil, fmt.Errorf("could not create deep copy of parent header: %w", err)
 	}
 
+	return parent, nil
+}
+
+func (b *Service) handleSlot(epoch, slotNum uint64,
+	authorityIndex uint32,
+	preRuntimeDigest *types.PreRuntimeDigest,
+) error {
 	currentSlot := Slot{
 		start:    time.Now(),
 		duration: b.constants.slotDuration,
 		number:   slotNum,
 	}
 
+	parent, err := b.getParentForBlockAuthoring(slotNum)
+	if err != nil {
+		return fmt.Errorf("could not get parent for claiming slot %d: %w", slotNum, err)
+	}
 	b.storageState.Lock()
 	defer b.storageState.Unlock()
 

lib/babe/babe_integration_test.go

@@ -6,7 +6,6 @@
 package babe
 
 import (
-	"path/filepath"
 	"testing"
 	"time"
 
@@ -15,16 +14,16 @@ 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/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"
 )
 
@@ -79,37 +78,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
 
 	var rtCfg wasmer.Config
 	rtCfg.Storage = rtstorage.NewTrieState(&genTrie)
@@ -119,12 +113,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)
@@ -135,6 +124,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 := mocks.NewMockNetwork(ctrl)
+		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 = NewTestService(t, &coreConfig)
+	}
+
 	return babeService
 }
 
@@ -256,17 +265,15 @@ func TestService_GetAuthorityIndex(t *testing.T) {
 }
 
 func TestStartAndStop(t *testing.T) {
-	serviceConfig := ServiceConfig{}
-	bs := createTestService(t, serviceConfig)
+	bs := createTestService(t, ServiceConfig{})
 	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)
@@ -292,3 +299,159 @@ 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() {
+		err = babeService.Stop()
+		require.NoError(t, err)
+	}()
+
+	// 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)
+
+	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: time.Second,
+		number:   slotnum,
+	}
+	preRuntimeDigest, err := types.NewBabePrimaryPreDigest(
+		0, slot.number,
+		[sr25519.VRFOutputLength]byte{},
+		[sr25519.VRFProofLength]byte{},
+	).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,
+		AuthData: []types.Authority{
+			{
+				Key:    alice.Public().(*sr25519.PublicKey),
+				Weight: 1,
+			},
+			{
+				Key:    bob.Public().(*sr25519.PublicKey),
+				Weight: 1,
+			},
+		},
+	}
+
+	// Create babe service for bob
+	cfgBob := ServiceConfig{
+		Authority: true,
+		Lead:      true,
+		Keypair:   bob,
+		AuthData: []types.Authority{
+			{
+				Key:    alice.Public().(*sr25519.PublicKey),
+				Weight: 1,
+			},
+			{
+				Key:    bob.Public().(*sr25519.PublicKey),
+				Weight: 1,
+			},
+		},
+	}
+
+	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)
+	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)
+
+	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)
+
+	bestBlockHeader, err := babeServiceAlice.blockState.BestBlockHeader()
+	require.NoError(t, err)
+	require.Equal(t, block.Header.Hash(), bestBlockHeader.Hash())
+
+	// If the slot we are claiming is the same as the slot of the best block header, test that we can
+	// still claim the slot without error.
+	bestBlockSlotNum, err := babeServiceAlice.blockState.GetSlotForBlock(bestBlockHeader.Hash())
+	require.NoError(t, err)
+
+	slot := Slot{
+		start:    time.Now(),
+		duration: time.Second,
+		number:   bestBlockSlotNum,
+	}
+	preRuntimeDigest, err := types.NewBabePrimaryPreDigest(
+		0, slot.number,
+		[sr25519.VRFOutputLength]byte{},
+		[sr25519.VRFProofLength]byte{},
+	).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)
+
+}