From 7266f6fd26209f4b0e1387f1d186fae17194c9d0 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Fri, 24 Feb 2023 08:06:51 -0400 Subject: [PATCH 01/18] feat: remove slot timers and use a similar substrate approach --- chain/westend-dev/config.toml | 5 +- lib/babe/babe.go | 3 + lib/babe/epoch_handler.go | 129 +++++++++++++--------------------- lib/babe/slot.go | 67 ++++++++++++++++++ 4 files changed, 121 insertions(+), 83 deletions(-) create mode 100644 lib/babe/slot.go diff --git a/chain/westend-dev/config.toml b/chain/westend-dev/config.toml index db6416e5c9..c13860882a 100644 --- a/chain/westend-dev/config.toml +++ b/chain/westend-dev/config.toml @@ -1,5 +1,5 @@ [global] -basepath = "~/.gossamer/westend-dev" +basepath = "./tmp/westend-dev" log = "info" metrics-address = ":9876" @@ -22,9 +22,10 @@ key = "alice" unlock = "" [core] +babe-lead = true roles = 4 babe-authority = true -grandpa-authority = true +grandpa-authority = false grandpa-interval = 1 [network] diff --git a/lib/babe/babe.go b/lib/babe/babe.go index 3410029c37..cd95034f52 100644 --- a/lib/babe/babe.go +++ b/lib/babe/babe.go @@ -405,6 +405,7 @@ func (b *Service) handleEpoch(epoch uint64) (next uint64, err error) { epochTimer := time.NewTimer(time.Until(nextEpochStartTime)) errCh := make(chan error) + go b.epochHandler.run(ctx, errCh) select { @@ -532,9 +533,11 @@ func (b *Service) handleSlot(epoch uint64, slot Slot, } func getCurrentSlot(slotDuration time.Duration) uint64 { + // TODO: should we use milliseconds here? return uint64(time.Now().UnixNano()) / uint64(slotDuration.Nanoseconds()) } func getSlotStartTime(slot uint64, slotDuration time.Duration) time.Time { + // TODO: should we use milliseconds here? return time.Unix(0, int64(slot)*slotDuration.Nanoseconds()) } diff --git a/lib/babe/epoch_handler.go b/lib/babe/epoch_handler.go index 0bb6dc657e..9bdaab3b5d 100644 --- a/lib/babe/epoch_handler.go +++ b/lib/babe/epoch_handler.go @@ -7,12 +7,9 @@ import ( "context" "errors" "fmt" - "sort" - "time" "github.com/ChainSafe/gossamer/dot/types" "github.com/ChainSafe/gossamer/lib/crypto/sr25519" - "golang.org/x/exp/maps" ) type handleSlotFunc = func(epoch uint64, slot Slot, authorityIndex uint32, @@ -23,6 +20,7 @@ var ( ) type epochHandler struct { + slotHandler *slotHandler epochNumber uint64 firstSlot uint64 @@ -34,14 +32,20 @@ type epochHandler struct { handleSlot handleSlotFunc } -func newEpochHandler(epochNumber, firstSlot uint64, epochData *epochData, constants constants, - handleSlot handleSlotFunc, keypair *sr25519.Keypair) (*epochHandler, error) { - // determine which slots we'll be authoring in by pre-calculating VRF output - slotToPreRuntimeDigest := make(map[uint64]*types.PreRuntimeDigest, constants.epochLength) - for i := firstSlot; i < firstSlot+constants.epochLength; i++ { - preRuntimeDigest, err := claimSlot(epochNumber, i, epochData, keypair) +// preRuntimeDigestMap maps the slot number to its respective pre runtime digest +type preRuntimeDigestMap map[uint64]*types.PreRuntimeDigest + +// determine which slots we'll be authoring in by pre-calculating VRF output +func determineAuthoringSlotsInEpoch(epochNumber, startSlot, epochLength uint64, + epochData *epochData, keypair *sr25519.Keypair) (preRuntimeDigestMap, error) { + + preRuntimeDigestMap := make(preRuntimeDigestMap, epochLength) + finalSlot := startSlot + epochLength + + for slotNumber := startSlot; slotNumber < finalSlot; slotNumber++ { + preRuntimeDigest, err := claimSlot(epochNumber, slotNumber, epochData, keypair) if err == nil { - slotToPreRuntimeDigest[i] = preRuntimeDigest + preRuntimeDigestMap[slotNumber] = preRuntimeDigest continue } @@ -52,13 +56,26 @@ func newEpochHandler(epochNumber, firstSlot uint64, epochData *epochData, consta return nil, fmt.Errorf("failed to create new epoch handler: %w", err) } + return preRuntimeDigestMap, nil +} + +func newEpochHandler(epochNumber, firstSlot uint64, epochData *epochData, constants constants, + handleSlot handleSlotFunc, keypair *sr25519.Keypair) (*epochHandler, error) { + + preRuntimeDigestMap, err := determineAuthoringSlotsInEpoch(epochNumber, firstSlot, + constants.epochLength, epochData, keypair) + if err != nil { + return nil, fmt.Errorf("determining authoring slots: %w", err) + } + return &epochHandler{ + slotHandler: newSlotHandler(constants.slotDuration), epochNumber: epochNumber, firstSlot: firstSlot, constants: constants, epochData: epochData, handleSlot: handleSlot, - slotToPreRuntimeDigest: slotToPreRuntimeDigest, + slotToPreRuntimeDigest: preRuntimeDigestMap, }, nil } @@ -75,83 +92,33 @@ func (h *epochHandler) run(ctx context.Context, errCh chan<- error) { return } - // for each slot we're handling, create a timer that will fire when it starts - // we create timers only for slots where we're authoring - authoringSlots := getAuthoringSlots(h.slotToPreRuntimeDigest) + logger.Debugf("authoring in %d slots in epoch %d", len(h.slotToPreRuntimeDigest), h.epochNumber) - type slotWithTimer struct { - startTime time.Time - timer *time.Timer - slotNum uint64 - } + for { + select { + case <-ctx.Done(): + return + default: + } - slotTimeTimers := make([]*slotWithTimer, 0, len(authoringSlots)) - for _, authoringSlot := range authoringSlots { - if authoringSlot < currSlot { - // ignore slots already passed + currentSlot, err := h.slotHandler.waitForNextSlot() + if err != nil { continue } - startTime := getSlotStartTime(authoringSlot, h.constants.slotDuration) - waitTime := time.Until(startTime) - timer := time.NewTimer(waitTime) - - slotTimeTimers = append(slotTimeTimers, &slotWithTimer{ - timer: timer, - slotNum: authoringSlot, - startTime: startTime, - }) - - logger.Debugf("start time of slot %d: %v", authoringSlot, startTime) - } - - logger.Debugf("authoring in %d slots in epoch %d", len(slotTimeTimers), h.epochNumber) + fmt.Printf("processing slot time %d\n", currentSlot.start.UnixMilli()) + fmt.Printf("processing slot number %d\n", currentSlot.number) - for _, swt := range slotTimeTimers { - logger.Debugf("waiting for next authoring slot %d", swt.slotNum) + // check if the slot is an authoring slot otherwise wait for the next slot + digest, has := h.slotToPreRuntimeDigest[currentSlot.number] + if !has { + continue + } - select { - case <-ctx.Done(): - for _, swt := range slotTimeTimers { - swt.timer.Stop() - } - return - case <-swt.timer.C: - // we must do a time correction as the slot timer sometimes is triggered - // before the time defined in the constructor due to an inconsistency - // of the language -> https://github.com/golang/go/issues/17696 - - diff := time.Since(swt.startTime) - if diff < 0 { - time.Sleep(-diff) - } - - if _, has := h.slotToPreRuntimeDigest[swt.slotNum]; !has { - // this should never happen - panic(fmt.Sprintf("no VRF proof for authoring slot! slot=%d", swt.slotNum)) - } - - currentSlot := Slot{ - start: swt.startTime, - duration: h.constants.slotDuration, - number: swt.slotNum, - } - err := h.handleSlot(h.epochNumber, currentSlot, h.epochData.authorityIndex, h.slotToPreRuntimeDigest[swt.slotNum]) - if err != nil { - logger.Warnf("failed to handle slot %d: %s", swt.slotNum, err) - continue - } + err = h.handleSlot(h.epochNumber, currentSlot, h.epochData.authorityIndex, digest) + if err != nil { + logger.Warnf("failed to handle slot %d: %s", currentSlot.number, err) + continue } } } - -// getAuthoringSlots returns an ordered slice of slot numbers where we can author blocks, -// based on the given VRF output and proof map. -func getAuthoringSlots(slotToPreRuntimeDigest map[uint64]*types.PreRuntimeDigest) []uint64 { - authoringSlots := maps.Keys(slotToPreRuntimeDigest) - sort.Slice(authoringSlots, func(i, j int) bool { - return authoringSlots[i] < authoringSlots[j] - }) - - return authoringSlots -} diff --git a/lib/babe/slot.go b/lib/babe/slot.go new file mode 100644 index 0000000000..41f7fc1d2a --- /dev/null +++ b/lib/babe/slot.go @@ -0,0 +1,67 @@ +package babe + +import ( + "errors" + "fmt" + "time" +) + +func timeUntilNextSlot(slotDuration time.Duration) time.Duration { + fmt.Printf("slot duration: %v\nslot duration in milli: %v\n", slotDuration, slotDuration.Milliseconds()) + + nowInMillis := time.Now().UnixMilli() + slotDurationInMillis := slotDuration.Milliseconds() + + nextSlot := (nowInMillis + slotDurationInMillis) / slotDurationInMillis + + remainingMillis := nextSlot*slotDurationInMillis - nowInMillis + return time.Duration(remainingMillis) +} + +type slotHandler struct { + slotDuration time.Duration + untilNextSlot *time.Duration + lastSlot *Slot +} + +func newSlotHandler(slotDuration time.Duration) *slotHandler { + return &slotHandler{ + slotDuration: slotDuration, + } +} + +func (s *slotHandler) waitForNextSlot() (Slot, error) { + if s.untilNextSlot == nil { + dur := timeUntilNextSlot(s.slotDuration) + fmt.Printf("waiting %d mill\n", dur) + + time.Sleep(dur) + } else { + fmt.Printf("waiting %d milli\n", s.untilNextSlot.Milliseconds()) + time.Sleep(*s.untilNextSlot) + } + + waitDuration := timeUntilNextSlot(s.slotDuration) + s.untilNextSlot = &waitDuration + + currentSystemTime := time.Now() + currentSlotNumber := uint64(currentSystemTime.UnixNano()) / uint64(s.slotDuration.Nanoseconds()) + currentSlot := Slot{ + start: currentSystemTime, + duration: s.slotDuration, + number: currentSlotNumber, + } + + if s.lastSlot == nil { + s.lastSlot = ¤tSlot + return currentSlot, nil + } + + // Never yield the same slot twice. + if currentSlot.number <= s.lastSlot.number { + return Slot{}, errors.New("issue a slot equal or lower than the latest one") + } + + s.lastSlot = ¤tSlot + return currentSlot, nil +} From 15361a44e50dbb4f5c443a8ce0b14bffd5eec992 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Fri, 24 Feb 2023 22:40:28 -0400 Subject: [PATCH 02/18] chore: create integration test --- lib/babe/babe.go | 5 +- lib/babe/epoch_handler.go | 2 + lib/babe/epoch_handler_integration_test.go | 69 ++++++++++++++++++++ lib/babe/epoch_handler_test.go | 52 --------------- lib/babe/slot.go | 75 ++++++++++------------ lib/babe/slot_test.go | 30 +++++++++ 6 files changed, 138 insertions(+), 95 deletions(-) create mode 100644 lib/babe/epoch_handler_integration_test.go create mode 100644 lib/babe/slot_test.go diff --git a/lib/babe/babe.go b/lib/babe/babe.go index cd95034f52..0832949800 100644 --- a/lib/babe/babe.go +++ b/lib/babe/babe.go @@ -405,7 +405,6 @@ func (b *Service) handleEpoch(epoch uint64) (next uint64, err error) { epochTimer := time.NewTimer(time.Until(nextEpochStartTime)) errCh := make(chan error) - go b.epochHandler.run(ctx, errCh) select { @@ -421,7 +420,9 @@ func (b *Service) handleEpoch(epoch uint64) (next uint64, err error) { case err := <-errCh: // TODO: errEpochPast is sent on this channel, but it doesnot get logged here epochTimer.Stop() - logger.Errorf("error from epochHandler: %s", err) + if err != nil { + logger.Errorf("error from epochHandler: %s", err) + } } // setup next epoch, re-invoke block authoring diff --git a/lib/babe/epoch_handler.go b/lib/babe/epoch_handler.go index 9bdaab3b5d..b48025e6cc 100644 --- a/lib/babe/epoch_handler.go +++ b/lib/babe/epoch_handler.go @@ -80,6 +80,7 @@ func newEpochHandler(epochNumber, firstSlot uint64, epochData *epochData, consta } func (h *epochHandler) run(ctx context.Context, errCh chan<- error) { + defer close(errCh) currSlot := getCurrentSlot(h.constants.slotDuration) // if currSlot < h.firstSlot, it means we're at genesis and waiting for the first slot to arrive. @@ -97,6 +98,7 @@ func (h *epochHandler) run(ctx context.Context, errCh chan<- error) { for { select { case <-ctx.Done(): + errCh <- nil return default: } diff --git a/lib/babe/epoch_handler_integration_test.go b/lib/babe/epoch_handler_integration_test.go new file mode 100644 index 0000000000..91721419d0 --- /dev/null +++ b/lib/babe/epoch_handler_integration_test.go @@ -0,0 +1,69 @@ +package babe + +import ( + "context" + "testing" + "time" + + "github.com/ChainSafe/gossamer/dot/types" + "github.com/ChainSafe/gossamer/lib/crypto/sr25519" + "github.com/ChainSafe/gossamer/pkg/scale" + "github.com/stretchr/testify/require" +) + +func TestEpochHandler_run(t *testing.T) { + const authorityIndex uint32 = 0 + aliceKeyPair := keyring.Alice().(*sr25519.Keypair) + epochData := &epochData{ + threshold: scale.MaxUint128, + authorityIndex: authorityIndex, + authorities: []types.Authority{ + *types.NewAuthority(aliceKeyPair.Public(), 1), + }, + } + + const slotDuration = 6 * time.Second + const epochLength uint64 = 100 + + constants := constants{ //nolint:govet + slotDuration: slotDuration, + epochLength: epochLength, + } + + const expectedEpoch = 1 + startSlot := getCurrentSlot(slotDuration) + handleSlotFunc := testHandleSlotFunc(t, authorityIndex, expectedEpoch, startSlot) + + epochHandler, err := newEpochHandler(1, startSlot, epochData, constants, handleSlotFunc, aliceKeyPair) + require.NoError(t, err) + require.Equal(t, epochLength, uint64(len(epochHandler.slotToPreRuntimeDigest))) + + timeoutCtx, cancel := context.WithTimeout(context.Background(), slotDuration*10) + defer cancel() + + errCh := make(chan error) + go epochHandler.run(timeoutCtx, errCh) + + err = <-errCh + require.NoError(t, err) +} + +func testHandleSlotFunc(t *testing.T, expectedAuthorityIndex uint32, + expectedEpoch, startSlot uint64) handleSlotFunc { + currentSlot := startSlot + 1 + + return func(epoch uint64, slot Slot, authorityIndex uint32, + preRuntimeDigest *types.PreRuntimeDigest) error { + + require.NotNil(t, preRuntimeDigest) + require.Equal(t, expectedEpoch, epoch) + require.Equal(t, expectedAuthorityIndex, authorityIndex) + + require.Equal(t, slot.number, currentSlot, "%d != %d", slot.number, currentSlot) + + // increase the slot by one so we expect the next call + // to be exactly 1 slot greater than the previous call + currentSlot += 1 + return nil + } +} diff --git a/lib/babe/epoch_handler_test.go b/lib/babe/epoch_handler_test.go index 8918bea666..8f3a2b7da3 100644 --- a/lib/babe/epoch_handler_test.go +++ b/lib/babe/epoch_handler_test.go @@ -4,7 +4,6 @@ package babe import ( - "context" "testing" "time" @@ -45,54 +44,3 @@ func TestNewEpochHandler(t *testing.T) { require.Equal(t, epochData, epochHandler.epochData) require.NotNil(t, epochHandler.handleSlot) } - -func TestEpochHandler_run(t *testing.T) { - sd, err := time.ParseDuration("10ms") - require.NoError(t, err) - startSlot := getCurrentSlot(sd) - - var callsToHandleSlot, firstExecutedSlot uint64 - testHandleSlotFunc := func(epoch uint64, slot Slot, authorityIndex uint32, - preRuntimeDigest *types.PreRuntimeDigest, - ) error { - require.Equal(t, uint64(1), epoch) - if callsToHandleSlot == 0 { - firstExecutedSlot = slot.number - } else { - require.Equal(t, firstExecutedSlot+callsToHandleSlot, slot.number) - } - require.Equal(t, uint32(0), authorityIndex) - require.NotNil(t, preRuntimeDigest) - callsToHandleSlot++ - return nil - } - - epochData := &epochData{ - threshold: scale.MaxUint128, - } - - const epochLength uint64 = 100 - constants := constants{ //nolint:govet - slotDuration: sd, - epochLength: epochLength, - } - - keypair := keyring.Alice().(*sr25519.Keypair) - - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - epochHandler, err := newEpochHandler(1, startSlot, epochData, constants, testHandleSlotFunc, keypair) - require.NoError(t, err) - require.Equal(t, epochLength, uint64(len(epochHandler.slotToPreRuntimeDigest))) - - errCh := make(chan error) - go epochHandler.run(ctx, errCh) - timer := time.NewTimer(sd * time.Duration(epochLength)) - select { - case <-timer.C: - require.Equal(t, epochLength-(firstExecutedSlot-startSlot), callsToHandleSlot) - case err := <-errCh: - timer.Stop() - require.NoError(t, err) - } -} diff --git a/lib/babe/slot.go b/lib/babe/slot.go index 41f7fc1d2a..b6e36ff3e0 100644 --- a/lib/babe/slot.go +++ b/lib/babe/slot.go @@ -1,21 +1,20 @@ package babe import ( - "errors" "fmt" "time" ) -func timeUntilNextSlot(slotDuration time.Duration) time.Duration { - fmt.Printf("slot duration: %v\nslot duration in milli: %v\n", slotDuration, slotDuration.Milliseconds()) +// timeUntilNextSlotInNanos calculates, based on the current system time, the remainng +// time to the next slot +func timeUntilNextSlotInMilli(slotDuration time.Duration) time.Duration { + now := time.Now().UnixNano() + slotDurationInMilli := slotDuration.Nanoseconds() - nowInMillis := time.Now().UnixMilli() - slotDurationInMillis := slotDuration.Milliseconds() + nextSlot := (now + slotDurationInMilli) / slotDurationInMilli - nextSlot := (nowInMillis + slotDurationInMillis) / slotDurationInMillis - - remainingMillis := nextSlot*slotDurationInMillis - nowInMillis - return time.Duration(remainingMillis) + remaining := nextSlot*slotDurationInMilli - now + return time.Duration(remaining) } type slotHandler struct { @@ -31,37 +30,31 @@ func newSlotHandler(slotDuration time.Duration) *slotHandler { } func (s *slotHandler) waitForNextSlot() (Slot, error) { - if s.untilNextSlot == nil { - dur := timeUntilNextSlot(s.slotDuration) - fmt.Printf("waiting %d mill\n", dur) - - time.Sleep(dur) - } else { - fmt.Printf("waiting %d milli\n", s.untilNextSlot.Milliseconds()) - time.Sleep(*s.untilNextSlot) - } - - waitDuration := timeUntilNextSlot(s.slotDuration) - s.untilNextSlot = &waitDuration - - currentSystemTime := time.Now() - currentSlotNumber := uint64(currentSystemTime.UnixNano()) / uint64(s.slotDuration.Nanoseconds()) - currentSlot := Slot{ - start: currentSystemTime, - duration: s.slotDuration, - number: currentSlotNumber, + for { + if s.untilNextSlot != nil { + time.Sleep(*s.untilNextSlot) + } else { + // first timeout + waitDuration := timeUntilNextSlotInMilli(s.slotDuration) + time.Sleep(waitDuration) + } + + waitDuration := timeUntilNextSlotInMilli(s.slotDuration) + fmt.Printf("time until next slot: %d\n", waitDuration) + s.untilNextSlot = &waitDuration + + currentSystemTime := time.Now() + currentSlotNumber := uint64(currentSystemTime.UnixNano()) / uint64(s.slotDuration.Nanoseconds()) + currentSlot := Slot{ + start: currentSystemTime, + duration: s.slotDuration, + number: currentSlotNumber, + } + + // Never yield the same slot twice + if s.lastSlot == nil || currentSlot.number > s.lastSlot.number { + s.lastSlot = ¤tSlot + return currentSlot, nil + } } - - if s.lastSlot == nil { - s.lastSlot = ¤tSlot - return currentSlot, nil - } - - // Never yield the same slot twice. - if currentSlot.number <= s.lastSlot.number { - return Slot{}, errors.New("issue a slot equal or lower than the latest one") - } - - s.lastSlot = ¤tSlot - return currentSlot, nil } diff --git a/lib/babe/slot_test.go b/lib/babe/slot_test.go new file mode 100644 index 0000000000..cf1fa2e5fd --- /dev/null +++ b/lib/babe/slot_test.go @@ -0,0 +1,30 @@ +package babe + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestSlotHandeConstructor(t *testing.T) { + expected := &slotHandler{ + slotDuration: time.Duration(6000), + } + + handler := newSlotHandler(time.Duration(6000)) + require.Equal(t, expected, handler) +} + +func TestSlotHandlerNextSlot(t *testing.T) { + slotDuration := 2 * time.Second + handler := newSlotHandler(slotDuration) + + firstIteration, err := handler.waitForNextSlot() + require.NoError(t, err) + + secondIteration, err := handler.waitForNextSlot() + require.NoError(t, err) + + require.Greater(t, secondIteration.number, firstIteration.number) +} From 15ff45dd70aebb000b12ea8653c840e292ca6d93 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Fri, 24 Feb 2023 22:41:47 -0400 Subject: [PATCH 03/18] chore: remove unneeded print lines --- lib/babe/epoch_handler.go | 3 --- lib/babe/slot.go | 2 -- 2 files changed, 5 deletions(-) diff --git a/lib/babe/epoch_handler.go b/lib/babe/epoch_handler.go index b48025e6cc..2f9b3a53a4 100644 --- a/lib/babe/epoch_handler.go +++ b/lib/babe/epoch_handler.go @@ -108,9 +108,6 @@ func (h *epochHandler) run(ctx context.Context, errCh chan<- error) { continue } - fmt.Printf("processing slot time %d\n", currentSlot.start.UnixMilli()) - fmt.Printf("processing slot number %d\n", currentSlot.number) - // check if the slot is an authoring slot otherwise wait for the next slot digest, has := h.slotToPreRuntimeDigest[currentSlot.number] if !has { diff --git a/lib/babe/slot.go b/lib/babe/slot.go index b6e36ff3e0..30ab4fd808 100644 --- a/lib/babe/slot.go +++ b/lib/babe/slot.go @@ -1,7 +1,6 @@ package babe import ( - "fmt" "time" ) @@ -40,7 +39,6 @@ func (s *slotHandler) waitForNextSlot() (Slot, error) { } waitDuration := timeUntilNextSlotInMilli(s.slotDuration) - fmt.Printf("time until next slot: %d\n", waitDuration) s.untilNextSlot = &waitDuration currentSystemTime := time.Now() From ac5433feb05942dcc49f1c64b4faa88e6895dbdf Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 27 Feb 2023 14:24:33 -0400 Subject: [PATCH 04/18] chore: make sure we wait just the remaining time and not a full slot duration time --- lib/babe/epoch_handler.go | 7 ++---- lib/babe/epoch_handler_integration_test.go | 2 +- lib/babe/slot.go | 27 +++++++++++----------- lib/babe/slot_test.go | 7 ++---- 4 files changed, 18 insertions(+), 25 deletions(-) diff --git a/lib/babe/epoch_handler.go b/lib/babe/epoch_handler.go index 2f9b3a53a4..c41eb753b6 100644 --- a/lib/babe/epoch_handler.go +++ b/lib/babe/epoch_handler.go @@ -103,10 +103,7 @@ func (h *epochHandler) run(ctx context.Context, errCh chan<- error) { default: } - currentSlot, err := h.slotHandler.waitForNextSlot() - if err != nil { - continue - } + currentSlot := h.slotHandler.waitForNextSlot() // check if the slot is an authoring slot otherwise wait for the next slot digest, has := h.slotToPreRuntimeDigest[currentSlot.number] @@ -114,7 +111,7 @@ func (h *epochHandler) run(ctx context.Context, errCh chan<- error) { continue } - err = h.handleSlot(h.epochNumber, currentSlot, h.epochData.authorityIndex, digest) + err := h.handleSlot(h.epochNumber, currentSlot, h.epochData.authorityIndex, digest) if err != nil { logger.Warnf("failed to handle slot %d: %s", currentSlot.number, err) continue diff --git a/lib/babe/epoch_handler_integration_test.go b/lib/babe/epoch_handler_integration_test.go index 91721419d0..755a7397a6 100644 --- a/lib/babe/epoch_handler_integration_test.go +++ b/lib/babe/epoch_handler_integration_test.go @@ -50,7 +50,7 @@ func TestEpochHandler_run(t *testing.T) { func testHandleSlotFunc(t *testing.T, expectedAuthorityIndex uint32, expectedEpoch, startSlot uint64) handleSlotFunc { - currentSlot := startSlot + 1 + currentSlot := startSlot return func(epoch uint64, slot Slot, authorityIndex uint32, preRuntimeDigest *types.PreRuntimeDigest) error { diff --git a/lib/babe/slot.go b/lib/babe/slot.go index 30ab4fd808..827eb1775f 100644 --- a/lib/babe/slot.go +++ b/lib/babe/slot.go @@ -17,9 +17,9 @@ func timeUntilNextSlotInMilli(slotDuration time.Duration) time.Duration { } type slotHandler struct { - slotDuration time.Duration - untilNextSlot *time.Duration - lastSlot *Slot + slotDuration time.Duration + untilNextSlotTimer *time.Timer + lastSlot *Slot } func newSlotHandler(slotDuration time.Duration) *slotHandler { @@ -28,19 +28,15 @@ func newSlotHandler(slotDuration time.Duration) *slotHandler { } } -func (s *slotHandler) waitForNextSlot() (Slot, error) { +func (s *slotHandler) waitForNextSlot() Slot { for { - if s.untilNextSlot != nil { - time.Sleep(*s.untilNextSlot) - } else { - // first timeout - waitDuration := timeUntilNextSlotInMilli(s.slotDuration) - time.Sleep(waitDuration) + // check if there is enough time to collaaborate + untilNextSlot := timeUntilNextSlotInMilli(s.slotDuration) + oneThird := s.slotDuration / 3 + if untilNextSlot <= oneThird { + time.Sleep(untilNextSlot) } - waitDuration := timeUntilNextSlotInMilli(s.slotDuration) - s.untilNextSlot = &waitDuration - currentSystemTime := time.Now() currentSlotNumber := uint64(currentSystemTime.UnixNano()) / uint64(s.slotDuration.Nanoseconds()) currentSlot := Slot{ @@ -52,7 +48,10 @@ func (s *slotHandler) waitForNextSlot() (Slot, error) { // Never yield the same slot twice if s.lastSlot == nil || currentSlot.number > s.lastSlot.number { s.lastSlot = ¤tSlot - return currentSlot, nil + return currentSlot + } else { + untilNextSlot := timeUntilNextSlotInMilli(s.slotDuration) + time.Sleep(untilNextSlot) } } } diff --git a/lib/babe/slot_test.go b/lib/babe/slot_test.go index cf1fa2e5fd..a64bc03e26 100644 --- a/lib/babe/slot_test.go +++ b/lib/babe/slot_test.go @@ -20,11 +20,8 @@ func TestSlotHandlerNextSlot(t *testing.T) { slotDuration := 2 * time.Second handler := newSlotHandler(slotDuration) - firstIteration, err := handler.waitForNextSlot() - require.NoError(t, err) - - secondIteration, err := handler.waitForNextSlot() - require.NoError(t, err) + firstIteration := handler.waitForNextSlot() + secondIteration := handler.waitForNextSlot() require.Greater(t, secondIteration.number, firstIteration.number) } From c8ced584dac879ae782015c6c1de4843a500dd42 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 27 Feb 2023 14:26:55 -0400 Subject: [PATCH 05/18] chore: remove unneeded `untilNextSlot` prop --- lib/babe/slot.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/babe/slot.go b/lib/babe/slot.go index 827eb1775f..f7010e522a 100644 --- a/lib/babe/slot.go +++ b/lib/babe/slot.go @@ -17,9 +17,8 @@ func timeUntilNextSlotInMilli(slotDuration time.Duration) time.Duration { } type slotHandler struct { - slotDuration time.Duration - untilNextSlotTimer *time.Timer - lastSlot *Slot + slotDuration time.Duration + lastSlot *Slot } func newSlotHandler(slotDuration time.Duration) *slotHandler { From 0e57ec596e1ff0735d5835b5845a5f84773e5aa5 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 27 Feb 2023 14:51:31 -0400 Subject: [PATCH 06/18] chore: remove unneeded diffs --- chain/westend-dev/config.toml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/chain/westend-dev/config.toml b/chain/westend-dev/config.toml index c13860882a..db6416e5c9 100644 --- a/chain/westend-dev/config.toml +++ b/chain/westend-dev/config.toml @@ -1,5 +1,5 @@ [global] -basepath = "./tmp/westend-dev" +basepath = "~/.gossamer/westend-dev" log = "info" metrics-address = ":9876" @@ -22,10 +22,9 @@ key = "alice" unlock = "" [core] -babe-lead = true roles = 4 babe-authority = true -grandpa-authority = false +grandpa-authority = true grandpa-interval = 1 [network] From a038c6200a44355bcba1ed92bcc09f60b5949aa0 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 27 Feb 2023 15:02:25 -0400 Subject: [PATCH 07/18] chore: remove unneeded else branch --- lib/babe/slot.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/babe/slot.go b/lib/babe/slot.go index f7010e522a..0968b83376 100644 --- a/lib/babe/slot.go +++ b/lib/babe/slot.go @@ -48,9 +48,8 @@ func (s *slotHandler) waitForNextSlot() Slot { if s.lastSlot == nil || currentSlot.number > s.lastSlot.number { s.lastSlot = ¤tSlot return currentSlot - } else { - untilNextSlot := timeUntilNextSlotInMilli(s.slotDuration) - time.Sleep(untilNextSlot) } + + time.Sleep(timeUntilNextSlotInMilli(s.slotDuration)) } } From 65fa6d0753e9e34e62e0e6718184d8e075299cf0 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 27 Feb 2023 15:06:53 -0400 Subject: [PATCH 08/18] chore: remove unneeded diffs --- lib/babe/epoch_handler.go | 34 ++++++++-------------------------- 1 file changed, 8 insertions(+), 26 deletions(-) diff --git a/lib/babe/epoch_handler.go b/lib/babe/epoch_handler.go index c41eb753b6..5a7a0f5675 100644 --- a/lib/babe/epoch_handler.go +++ b/lib/babe/epoch_handler.go @@ -32,20 +32,14 @@ type epochHandler struct { handleSlot handleSlotFunc } -// preRuntimeDigestMap maps the slot number to its respective pre runtime digest -type preRuntimeDigestMap map[uint64]*types.PreRuntimeDigest - -// determine which slots we'll be authoring in by pre-calculating VRF output -func determineAuthoringSlotsInEpoch(epochNumber, startSlot, epochLength uint64, - epochData *epochData, keypair *sr25519.Keypair) (preRuntimeDigestMap, error) { - - preRuntimeDigestMap := make(preRuntimeDigestMap, epochLength) - finalSlot := startSlot + epochLength - - for slotNumber := startSlot; slotNumber < finalSlot; slotNumber++ { - preRuntimeDigest, err := claimSlot(epochNumber, slotNumber, epochData, keypair) +func newEpochHandler(epochNumber, firstSlot uint64, epochData *epochData, constants constants, + handleSlot handleSlotFunc, keypair *sr25519.Keypair) (*epochHandler, error) { + // determine which slots we'll be authoring in by pre-calculating VRF output + slotToPreRuntimeDigest := make(map[uint64]*types.PreRuntimeDigest, constants.epochLength) + for i := firstSlot; i < firstSlot+constants.epochLength; i++ { + preRuntimeDigest, err := claimSlot(epochNumber, i, epochData, keypair) if err == nil { - preRuntimeDigestMap[slotNumber] = preRuntimeDigest + slotToPreRuntimeDigest[i] = preRuntimeDigest continue } @@ -56,18 +50,6 @@ func determineAuthoringSlotsInEpoch(epochNumber, startSlot, epochLength uint64, return nil, fmt.Errorf("failed to create new epoch handler: %w", err) } - return preRuntimeDigestMap, nil -} - -func newEpochHandler(epochNumber, firstSlot uint64, epochData *epochData, constants constants, - handleSlot handleSlotFunc, keypair *sr25519.Keypair) (*epochHandler, error) { - - preRuntimeDigestMap, err := determineAuthoringSlotsInEpoch(epochNumber, firstSlot, - constants.epochLength, epochData, keypair) - if err != nil { - return nil, fmt.Errorf("determining authoring slots: %w", err) - } - return &epochHandler{ slotHandler: newSlotHandler(constants.slotDuration), epochNumber: epochNumber, @@ -75,7 +57,7 @@ func newEpochHandler(epochNumber, firstSlot uint64, epochData *epochData, consta constants: constants, epochData: epochData, handleSlot: handleSlot, - slotToPreRuntimeDigest: preRuntimeDigestMap, + slotToPreRuntimeDigest: slotToPreRuntimeDigest, }, nil } From 8cca3feecc974969a30537e396c8bb52a33bf17b Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 27 Feb 2023 15:11:24 -0400 Subject: [PATCH 09/18] chore: remove unneeded `continue` keyword --- lib/babe/epoch_handler.go | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/babe/epoch_handler.go b/lib/babe/epoch_handler.go index 5a7a0f5675..3f373f2240 100644 --- a/lib/babe/epoch_handler.go +++ b/lib/babe/epoch_handler.go @@ -96,7 +96,6 @@ func (h *epochHandler) run(ctx context.Context, errCh chan<- error) { err := h.handleSlot(h.epochNumber, currentSlot, h.epochData.authorityIndex, digest) if err != nil { logger.Warnf("failed to handle slot %d: %s", currentSlot.number, err) - continue } } } From a3a17e4ff93463675b2a1ca53f061c209bc44fd5 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Tue, 28 Feb 2023 15:51:14 -0400 Subject: [PATCH 10/18] chore: include substrate permalink + rename variables --- lib/babe/epoch_handler.go | 4 ++-- lib/babe/slot.go | 21 ++++++++++++--------- lib/babe/slot_test.go | 2 +- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/lib/babe/epoch_handler.go b/lib/babe/epoch_handler.go index 3f373f2240..5baa65016c 100644 --- a/lib/babe/epoch_handler.go +++ b/lib/babe/epoch_handler.go @@ -88,12 +88,12 @@ func (h *epochHandler) run(ctx context.Context, errCh chan<- error) { currentSlot := h.slotHandler.waitForNextSlot() // check if the slot is an authoring slot otherwise wait for the next slot - digest, has := h.slotToPreRuntimeDigest[currentSlot.number] + preRuntimeDigest, has := h.slotToPreRuntimeDigest[currentSlot.number] if !has { continue } - err := h.handleSlot(h.epochNumber, currentSlot, h.epochData.authorityIndex, digest) + err := h.handleSlot(h.epochNumber, currentSlot, h.epochData.authorityIndex, preRuntimeDigest) if err != nil { logger.Warnf("failed to handle slot %d: %s", currentSlot.number, err) } diff --git a/lib/babe/slot.go b/lib/babe/slot.go index 0968b83376..dad4c66484 100644 --- a/lib/babe/slot.go +++ b/lib/babe/slot.go @@ -4,15 +4,15 @@ import ( "time" ) -// timeUntilNextSlotInNanos calculates, based on the current system time, the remainng +// timeUntilNextSlot calculates, based on the current system time, the remainng // time to the next slot -func timeUntilNextSlotInMilli(slotDuration time.Duration) time.Duration { +func timeUntilNextSlot(slotDuration time.Duration) time.Duration { now := time.Now().UnixNano() - slotDurationInMilli := slotDuration.Nanoseconds() + slotDurationInNano := slotDuration.Nanoseconds() - nextSlot := (now + slotDurationInMilli) / slotDurationInMilli + nextSlot := (now + slotDurationInNano) / slotDurationInNano - remaining := nextSlot*slotDurationInMilli - now + remaining := nextSlot*slotDurationInNano - now return time.Duration(remaining) } @@ -27,12 +27,15 @@ func newSlotHandler(slotDuration time.Duration) *slotHandler { } } +// waitForNextSlot returns a new Slot greater than the last one when a new slot starts +// based on the current system time similar to: +// https://github.com/paritytech/substrate/blob/fbddfbd76c60c6fda0024e8a44e82ad776033e4b/client/consensus/slots/src/slots.rs#L125 func (s *slotHandler) waitForNextSlot() Slot { for { // check if there is enough time to collaaborate - untilNextSlot := timeUntilNextSlotInMilli(s.slotDuration) - oneThird := s.slotDuration / 3 - if untilNextSlot <= oneThird { + untilNextSlot := timeUntilNextSlot(s.slotDuration) + oneThirdSlotDuration := s.slotDuration / 3 + if untilNextSlot <= oneThirdSlotDuration { time.Sleep(untilNextSlot) } @@ -50,6 +53,6 @@ func (s *slotHandler) waitForNextSlot() Slot { return currentSlot } - time.Sleep(timeUntilNextSlotInMilli(s.slotDuration)) + time.Sleep(timeUntilNextSlot(s.slotDuration)) } } diff --git a/lib/babe/slot_test.go b/lib/babe/slot_test.go index a64bc03e26..1d20e8d35c 100644 --- a/lib/babe/slot_test.go +++ b/lib/babe/slot_test.go @@ -7,7 +7,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestSlotHandeConstructor(t *testing.T) { +func TestSlotHandlerConstructor(t *testing.T) { expected := &slotHandler{ slotDuration: time.Duration(6000), } From 8ec67bc9a454fdec99b6facd30f4871bad48ef2b Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Wed, 1 Mar 2023 15:38:53 -0400 Subject: [PATCH 11/18] chore: fix check lint CI --- lib/babe/epoch_handler_integration_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/babe/epoch_handler_integration_test.go b/lib/babe/epoch_handler_integration_test.go index 755a7397a6..e3b0559a3e 100644 --- a/lib/babe/epoch_handler_integration_test.go +++ b/lib/babe/epoch_handler_integration_test.go @@ -32,9 +32,9 @@ func TestEpochHandler_run(t *testing.T) { const expectedEpoch = 1 startSlot := getCurrentSlot(slotDuration) - handleSlotFunc := testHandleSlotFunc(t, authorityIndex, expectedEpoch, startSlot) + handler := testHandleSlotFunc(t, authorityIndex, expectedEpoch, startSlot) - epochHandler, err := newEpochHandler(1, startSlot, epochData, constants, handleSlotFunc, aliceKeyPair) + epochHandler, err := newEpochHandler(1, startSlot, epochData, constants, handler, aliceKeyPair) require.NoError(t, err) require.Equal(t, epochLength, uint64(len(epochHandler.slotToPreRuntimeDigest))) @@ -63,7 +63,7 @@ func testHandleSlotFunc(t *testing.T, expectedAuthorityIndex uint32, // increase the slot by one so we expect the next call // to be exactly 1 slot greater than the previous call - currentSlot += 1 + currentSlot++ return nil } } From a711c522797d53d16dce3ade401222fc757fb9b3 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Wed, 1 Mar 2023 16:01:48 -0400 Subject: [PATCH 12/18] chore: add license header to new files --- lib/babe/epoch_handler_integration_test.go | 3 +++ lib/babe/slot.go | 3 +++ lib/babe/slot_test.go | 3 +++ 3 files changed, 9 insertions(+) diff --git a/lib/babe/epoch_handler_integration_test.go b/lib/babe/epoch_handler_integration_test.go index e3b0559a3e..97da7c1b30 100644 --- a/lib/babe/epoch_handler_integration_test.go +++ b/lib/babe/epoch_handler_integration_test.go @@ -1,3 +1,6 @@ +// Copyright 2023 ChainSafe Systems (ON) +// SPDX-License-Identifier: LGPL-3.0-only + package babe import ( diff --git a/lib/babe/slot.go b/lib/babe/slot.go index dad4c66484..47b118d8fb 100644 --- a/lib/babe/slot.go +++ b/lib/babe/slot.go @@ -1,3 +1,6 @@ +// Copyright 2023 ChainSafe Systems (ON) +// SPDX-License-Identifier: LGPL-3.0-only + package babe import ( diff --git a/lib/babe/slot_test.go b/lib/babe/slot_test.go index 1d20e8d35c..b36cf9fcda 100644 --- a/lib/babe/slot_test.go +++ b/lib/babe/slot_test.go @@ -1,3 +1,6 @@ +// Copyright 2023 ChainSafe Systems (ON) +// SPDX-License-Identifier: LGPL-3.0-only + package babe import ( From 6c22c94e28124077e86e816e663fe041efa90127 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Fri, 10 Mar 2023 16:00:56 -0400 Subject: [PATCH 13/18] chore: address comments --- lib/babe/epoch_handler.go | 2 +- lib/babe/epoch_handler_integration_test.go | 11 ++++++----- lib/babe/epoch_handler_test.go | 6 +++--- lib/babe/slot.go | 6 +++--- lib/babe/slot_test.go | 6 +++++- 5 files changed, 18 insertions(+), 13 deletions(-) diff --git a/lib/babe/epoch_handler.go b/lib/babe/epoch_handler.go index 5baa65016c..81e9ffe8cc 100644 --- a/lib/babe/epoch_handler.go +++ b/lib/babe/epoch_handler.go @@ -20,7 +20,7 @@ var ( ) type epochHandler struct { - slotHandler *slotHandler + slotHandler slotHandler epochNumber uint64 firstSlot uint64 diff --git a/lib/babe/epoch_handler_integration_test.go b/lib/babe/epoch_handler_integration_test.go index 97da7c1b30..0460bf278c 100644 --- a/lib/babe/epoch_handler_integration_test.go +++ b/lib/babe/epoch_handler_integration_test.go @@ -15,6 +15,8 @@ import ( ) func TestEpochHandler_run(t *testing.T) { + t.Parallel() + const authorityIndex uint32 = 0 aliceKeyPair := keyring.Alice().(*sr25519.Keypair) epochData := &epochData{ @@ -28,7 +30,7 @@ func TestEpochHandler_run(t *testing.T) { const slotDuration = 6 * time.Second const epochLength uint64 = 100 - constants := constants{ //nolint:govet + testConstants := constants{ slotDuration: slotDuration, epochLength: epochLength, } @@ -37,11 +39,11 @@ func TestEpochHandler_run(t *testing.T) { startSlot := getCurrentSlot(slotDuration) handler := testHandleSlotFunc(t, authorityIndex, expectedEpoch, startSlot) - epochHandler, err := newEpochHandler(1, startSlot, epochData, constants, handler, aliceKeyPair) + epochHandler, err := newEpochHandler(1, startSlot, epochData, testConstants, handler, aliceKeyPair) require.NoError(t, err) require.Equal(t, epochLength, uint64(len(epochHandler.slotToPreRuntimeDigest))) - timeoutCtx, cancel := context.WithTimeout(context.Background(), slotDuration*10) + timeoutCtx, cancel := context.WithTimeout(context.Background(), 10*slotDuration) defer cancel() errCh := make(chan error) @@ -57,12 +59,11 @@ func testHandleSlotFunc(t *testing.T, expectedAuthorityIndex uint32, return func(epoch uint64, slot Slot, authorityIndex uint32, preRuntimeDigest *types.PreRuntimeDigest) error { - require.NotNil(t, preRuntimeDigest) require.Equal(t, expectedEpoch, epoch) require.Equal(t, expectedAuthorityIndex, authorityIndex) - require.Equal(t, slot.number, currentSlot, "%d != %d", slot.number, currentSlot) + require.Equal(t, slot.number, currentSlot) // increase the slot by one so we expect the next call // to be exactly 1 slot greater than the previous call diff --git a/lib/babe/epoch_handler_test.go b/lib/babe/epoch_handler_test.go index 8f3a2b7da3..11d62cf112 100644 --- a/lib/babe/epoch_handler_test.go +++ b/lib/babe/epoch_handler_test.go @@ -28,19 +28,19 @@ func TestNewEpochHandler(t *testing.T) { sd, err := time.ParseDuration("6s") require.NoError(t, err) - constants := constants{ //nolint:govet + testConstants := constants{ slotDuration: sd, epochLength: 200, } keypair := keyring.Alice().(*sr25519.Keypair) - epochHandler, err := newEpochHandler(1, 9999, epochData, constants, testHandleSlotFunc, keypair) + epochHandler, err := newEpochHandler(1, 9999, epochData, testConstants, testHandleSlotFunc, keypair) require.NoError(t, err) require.Equal(t, 200, len(epochHandler.slotToPreRuntimeDigest)) require.Equal(t, uint64(1), epochHandler.epochNumber) require.Equal(t, uint64(9999), epochHandler.firstSlot) - require.Equal(t, constants, epochHandler.constants) + require.Equal(t, testConstants, epochHandler.constants) require.Equal(t, epochData, epochHandler.epochData) require.NotNil(t, epochHandler.handleSlot) } diff --git a/lib/babe/slot.go b/lib/babe/slot.go index 47b118d8fb..fe15984a2f 100644 --- a/lib/babe/slot.go +++ b/lib/babe/slot.go @@ -24,8 +24,8 @@ type slotHandler struct { lastSlot *Slot } -func newSlotHandler(slotDuration time.Duration) *slotHandler { - return &slotHandler{ +func newSlotHandler(slotDuration time.Duration) slotHandler { + return slotHandler{ slotDuration: slotDuration, } } @@ -35,7 +35,7 @@ func newSlotHandler(slotDuration time.Duration) *slotHandler { // https://github.com/paritytech/substrate/blob/fbddfbd76c60c6fda0024e8a44e82ad776033e4b/client/consensus/slots/src/slots.rs#L125 func (s *slotHandler) waitForNextSlot() Slot { for { - // check if there is enough time to collaaborate + // check if there is enough time to collaborate untilNextSlot := timeUntilNextSlot(s.slotDuration) oneThirdSlotDuration := s.slotDuration / 3 if untilNextSlot <= oneThirdSlotDuration { diff --git a/lib/babe/slot_test.go b/lib/babe/slot_test.go index b36cf9fcda..49d80f385a 100644 --- a/lib/babe/slot_test.go +++ b/lib/babe/slot_test.go @@ -11,6 +11,8 @@ import ( ) func TestSlotHandlerConstructor(t *testing.T) { + t.Parallel() + expected := &slotHandler{ slotDuration: time.Duration(6000), } @@ -20,7 +22,9 @@ func TestSlotHandlerConstructor(t *testing.T) { } func TestSlotHandlerNextSlot(t *testing.T) { - slotDuration := 2 * time.Second + t.Parallel() + + const slotDuration = 2 * time.Second handler := newSlotHandler(slotDuration) firstIteration := handler.waitForNextSlot() From 1feaf8979fcb5acf89c2f0bf1b895b5e3ebd5ee3 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Fri, 10 Mar 2023 16:45:32 -0400 Subject: [PATCH 14/18] chore: make `waitForNextSlot` context aware --- lib/babe/epoch_handler.go | 12 ++++++------ lib/babe/slot.go | 32 ++++++++++++++++++++++++++++---- lib/babe/slot_test.go | 28 ++++++++++++++++++++++++++-- 3 files changed, 60 insertions(+), 12 deletions(-) diff --git a/lib/babe/epoch_handler.go b/lib/babe/epoch_handler.go index 81e9ffe8cc..d1e196670d 100644 --- a/lib/babe/epoch_handler.go +++ b/lib/babe/epoch_handler.go @@ -78,22 +78,22 @@ func (h *epochHandler) run(ctx context.Context, errCh chan<- error) { logger.Debugf("authoring in %d slots in epoch %d", len(h.slotToPreRuntimeDigest), h.epochNumber) for { - select { - case <-ctx.Done(): + currentSlot, err := h.slotHandler.waitForNextSlot(ctx) + if errors.Is(err, context.Canceled) { errCh <- nil return - default: + } else if err != nil { + errCh <- err + return } - currentSlot := h.slotHandler.waitForNextSlot() - // check if the slot is an authoring slot otherwise wait for the next slot preRuntimeDigest, has := h.slotToPreRuntimeDigest[currentSlot.number] if !has { continue } - err := h.handleSlot(h.epochNumber, currentSlot, h.epochData.authorityIndex, preRuntimeDigest) + err = h.handleSlot(h.epochNumber, currentSlot, h.epochData.authorityIndex, preRuntimeDigest) if err != nil { logger.Warnf("failed to handle slot %d: %s", currentSlot.number, err) } diff --git a/lib/babe/slot.go b/lib/babe/slot.go index fe15984a2f..558bda9caf 100644 --- a/lib/babe/slot.go +++ b/lib/babe/slot.go @@ -4,6 +4,8 @@ package babe import ( + "context" + "fmt" "time" ) @@ -33,13 +35,16 @@ func newSlotHandler(slotDuration time.Duration) slotHandler { // waitForNextSlot returns a new Slot greater than the last one when a new slot starts // based on the current system time similar to: // https://github.com/paritytech/substrate/blob/fbddfbd76c60c6fda0024e8a44e82ad776033e4b/client/consensus/slots/src/slots.rs#L125 -func (s *slotHandler) waitForNextSlot() Slot { +func (s *slotHandler) waitForNextSlot(ctx context.Context) (Slot, error) { for { // check if there is enough time to collaborate untilNextSlot := timeUntilNextSlot(s.slotDuration) oneThirdSlotDuration := s.slotDuration / 3 if untilNextSlot <= oneThirdSlotDuration { - time.Sleep(untilNextSlot) + err := waitUntilNextSlot(ctx, untilNextSlot) + if err != nil { + return Slot{}, fmt.Errorf("waiting next slot: %w", err) + } } currentSystemTime := time.Now() @@ -53,9 +58,28 @@ func (s *slotHandler) waitForNextSlot() Slot { // Never yield the same slot twice if s.lastSlot == nil || currentSlot.number > s.lastSlot.number { s.lastSlot = ¤tSlot - return currentSlot + return currentSlot, nil } - time.Sleep(timeUntilNextSlot(s.slotDuration)) + err := waitUntilNextSlot(ctx, untilNextSlot) + if err != nil { + return Slot{}, fmt.Errorf("waiting next slot: %w", err) + } + } +} + +// waitUntilNextSlot is a blocking function that uses context.WithTimeout +// to "sleep", however if the parent context is canceled it releases with +// context.Canceled error +func waitUntilNextSlot(ctx context.Context, untilNextSlot time.Duration) error { + withTimeout, cancelWithTimeout := context.WithTimeout(ctx, untilNextSlot) + defer cancelWithTimeout() + + <-withTimeout.Done() + err := withTimeout.Err() + if err != context.DeadlineExceeded { + return err } + + return nil } diff --git a/lib/babe/slot_test.go b/lib/babe/slot_test.go index 49d80f385a..76ac8e9b58 100644 --- a/lib/babe/slot_test.go +++ b/lib/babe/slot_test.go @@ -4,6 +4,7 @@ package babe import ( + "context" "testing" "time" @@ -27,8 +28,31 @@ func TestSlotHandlerNextSlot(t *testing.T) { const slotDuration = 2 * time.Second handler := newSlotHandler(slotDuration) - firstIteration := handler.waitForNextSlot() - secondIteration := handler.waitForNextSlot() + firstIteration, err := handler.waitForNextSlot(context.Background()) + require.NoError(t, err) + + secondIteration, err := handler.waitForNextSlot(context.Background()) + require.NoError(t, err) require.Greater(t, secondIteration.number, firstIteration.number) } + +func TestSlotHandlerNextSlot_ContextCanceled(t *testing.T) { + t.Parallel() + + const slotDuration = 2 * time.Second + handler := newSlotHandler(slotDuration) + + ctx, cancel := context.WithCancel(context.Background()) + + firstIteration, err := handler.waitForNextSlot(ctx) + require.NoError(t, err) + require.NotEqual(t, Slot{}, firstIteration) + + cancel() + + secondIteration, err := handler.waitForNextSlot(ctx) + require.Equal(t, Slot{}, secondIteration) + require.ErrorIs(t, err, context.Canceled) + require.EqualError(t, err, "waiting next slot: context canceled") +} From 610f25502c9bc76ddd8033a3d6c561d46d449e5b Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Fri, 10 Mar 2023 19:03:42 -0400 Subject: [PATCH 15/18] chore: fix `TestSlotHandlerConstructor` test --- lib/babe/slot_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/babe/slot_test.go b/lib/babe/slot_test.go index 76ac8e9b58..a3decc9aef 100644 --- a/lib/babe/slot_test.go +++ b/lib/babe/slot_test.go @@ -14,7 +14,7 @@ import ( func TestSlotHandlerConstructor(t *testing.T) { t.Parallel() - expected := &slotHandler{ + expected := slotHandler{ slotDuration: time.Duration(6000), } From 311a3da29d43dd33fb20ef04e1d585d66be13cb0 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Fri, 10 Mar 2023 19:05:37 -0400 Subject: [PATCH 16/18] chore: add integration build flag --- lib/babe/epoch_handler_integration_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/babe/epoch_handler_integration_test.go b/lib/babe/epoch_handler_integration_test.go index 0460bf278c..f334528cfe 100644 --- a/lib/babe/epoch_handler_integration_test.go +++ b/lib/babe/epoch_handler_integration_test.go @@ -1,6 +1,8 @@ // Copyright 2023 ChainSafe Systems (ON) // SPDX-License-Identifier: LGPL-3.0-only +//go:build integration + package babe import ( From f3d38da834075e0e5c8ebfd9c195f222e0267b35 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 13 Mar 2023 16:31:51 -0400 Subject: [PATCH 17/18] chore: solve context dependency + resolve slot mismatch error --- lib/babe/epoch_handler.go | 2 +- lib/babe/epoch_handler_integration_test.go | 5 ++++- lib/babe/slot.go | 7 ++++--- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/babe/epoch_handler.go b/lib/babe/epoch_handler.go index d1e196670d..a2b6d1f555 100644 --- a/lib/babe/epoch_handler.go +++ b/lib/babe/epoch_handler.go @@ -79,7 +79,7 @@ func (h *epochHandler) run(ctx context.Context, errCh chan<- error) { for { currentSlot, err := h.slotHandler.waitForNextSlot(ctx) - if errors.Is(err, context.Canceled) { + if errors.Is(err, context.DeadlineExceeded) { errCh <- nil return } else if err != nil { diff --git a/lib/babe/epoch_handler_integration_test.go b/lib/babe/epoch_handler_integration_test.go index f334528cfe..7a26e6ab69 100644 --- a/lib/babe/epoch_handler_integration_test.go +++ b/lib/babe/epoch_handler_integration_test.go @@ -7,6 +7,7 @@ package babe import ( "context" + "fmt" "testing" "time" @@ -65,7 +66,9 @@ func testHandleSlotFunc(t *testing.T, expectedAuthorityIndex uint32, require.Equal(t, expectedEpoch, epoch) require.Equal(t, expectedAuthorityIndex, authorityIndex) - require.Equal(t, slot.number, currentSlot) + fmt.Printf("slot.number = %d\ncurrentSlot = %d\n", slot.number, currentSlot) + + require.GreaterOrEqual(t, slot.number, currentSlot) // increase the slot by one so we expect the next call // to be exactly 1 slot greater than the previous call diff --git a/lib/babe/slot.go b/lib/babe/slot.go index 558bda9caf..0a1d65b479 100644 --- a/lib/babe/slot.go +++ b/lib/babe/slot.go @@ -76,9 +76,10 @@ func waitUntilNextSlot(ctx context.Context, untilNextSlot time.Duration) error { defer cancelWithTimeout() <-withTimeout.Done() - err := withTimeout.Err() - if err != context.DeadlineExceeded { - return err + + parentCtxErr := ctx.Err() + if parentCtxErr != nil { + return parentCtxErr } return nil From e417fe465f3e7abc2d625fbcc9a973f5e43cba10 Mon Sep 17 00:00:00 2001 From: EclesioMeloJunior Date: Mon, 20 Mar 2023 16:12:39 -0400 Subject: [PATCH 18/18] chore: return any error --- lib/babe/epoch_handler.go | 7 ++-- lib/babe/epoch_handler_integration_test.go | 48 ++++++++++++++++++++-- 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/lib/babe/epoch_handler.go b/lib/babe/epoch_handler.go index a2b6d1f555..52dc8c9132 100644 --- a/lib/babe/epoch_handler.go +++ b/lib/babe/epoch_handler.go @@ -61,6 +61,8 @@ func newEpochHandler(epochNumber, firstSlot uint64, epochData *epochData, consta }, nil } +// run executes the block production for each available successfully claimed slot +// it is important to note that any error will be transmitted through errCh func (h *epochHandler) run(ctx context.Context, errCh chan<- error) { defer close(errCh) currSlot := getCurrentSlot(h.constants.slotDuration) @@ -79,10 +81,7 @@ func (h *epochHandler) run(ctx context.Context, errCh chan<- error) { for { currentSlot, err := h.slotHandler.waitForNextSlot(ctx) - if errors.Is(err, context.DeadlineExceeded) { - errCh <- nil - return - } else if err != nil { + if err != nil { errCh <- err return } diff --git a/lib/babe/epoch_handler_integration_test.go b/lib/babe/epoch_handler_integration_test.go index 7a26e6ab69..bb8eac33a5 100644 --- a/lib/babe/epoch_handler_integration_test.go +++ b/lib/babe/epoch_handler_integration_test.go @@ -7,7 +7,6 @@ package babe import ( "context" - "fmt" "testing" "time" @@ -17,6 +16,48 @@ import ( "github.com/stretchr/testify/require" ) +func TestEpochHandler_run_shouldReturnAfterContextCancel(t *testing.T) { + t.Parallel() + + const authorityIndex uint32 = 0 + aliceKeyPair := keyring.Alice().(*sr25519.Keypair) + epochData := &epochData{ + threshold: scale.MaxUint128, + authorityIndex: authorityIndex, + authorities: []types.Authority{ + *types.NewAuthority(aliceKeyPair.Public(), 1), + }, + } + + const slotDuration = 6 * time.Second + const epochLength uint64 = 100 + + testConstants := constants{ + slotDuration: slotDuration, + epochLength: epochLength, + } + + const expectedEpoch = 1 + startSlot := getCurrentSlot(slotDuration) + handler := testHandleSlotFunc(t, authorityIndex, expectedEpoch, startSlot) + + epochHandler, err := newEpochHandler(1, startSlot, epochData, testConstants, handler, aliceKeyPair) + require.NoError(t, err) + require.Equal(t, epochLength, uint64(len(epochHandler.slotToPreRuntimeDigest))) + + timeoutCtx, cancel := context.WithCancel(context.Background()) + go func() { + time.Sleep(7 * time.Second) + cancel() + }() + + errCh := make(chan error) + go epochHandler.run(timeoutCtx, errCh) + + err = <-errCh + require.ErrorIs(t, err, context.Canceled) +} + func TestEpochHandler_run(t *testing.T) { t.Parallel() @@ -53,7 +94,8 @@ func TestEpochHandler_run(t *testing.T) { go epochHandler.run(timeoutCtx, errCh) err = <-errCh - require.NoError(t, err) + require.ErrorIs(t, err, context.DeadlineExceeded) + } func testHandleSlotFunc(t *testing.T, expectedAuthorityIndex uint32, @@ -66,8 +108,6 @@ func testHandleSlotFunc(t *testing.T, expectedAuthorityIndex uint32, require.Equal(t, expectedEpoch, epoch) require.Equal(t, expectedAuthorityIndex, authorityIndex) - fmt.Printf("slot.number = %d\ncurrentSlot = %d\n", slot.number, currentSlot) - require.GreaterOrEqual(t, slot.number, currentSlot) // increase the slot by one so we expect the next call