From 5cb6a26eb7fb15cf3d7faf5b5198a86651004aba Mon Sep 17 00:00:00 2001 From: Diogo Santos Date: Fri, 17 Jan 2025 10:02:50 +0000 Subject: [PATCH 1/3] change exponential timer for linear timer --- app/featureset/featureset.go | 6 ++-- app/featureset/featureset_internal_test.go | 2 +- core/consensus/utils/roundtimer.go | 33 +++++++++++----------- core/consensus/utils/roundtimer_test.go | 18 ++++++------ docs/consensus.md | 4 +-- 5 files changed, 31 insertions(+), 32 deletions(-) diff --git a/app/featureset/featureset.go b/app/featureset/featureset.go index 26ec52a3f..14cec27b3 100644 --- a/app/featureset/featureset.go +++ b/app/featureset/featureset.go @@ -46,9 +46,9 @@ const ( // unless the user disabled this feature explicitly. GnosisBlockHotfix Feature = "gnosis_block_hotfix" - // Exponential enables Exponential round timer for consensus rounds. + // Linear enables Linear round timer for consensus rounds. // When active has precedence over EagerDoubleLinear round timer. - Exponential Feature = "exponential" + Linear Feature = "linear" ) var ( @@ -60,7 +60,7 @@ var ( AggSigDBV2: statusAlpha, JSONRequests: statusAlpha, GnosisBlockHotfix: statusAlpha, - Exponential: statusAlpha, + Linear: statusAlpha, // Add all features and there status here. } diff --git a/app/featureset/featureset_internal_test.go b/app/featureset/featureset_internal_test.go index fbee90065..92ca2cf72 100644 --- a/app/featureset/featureset_internal_test.go +++ b/app/featureset/featureset_internal_test.go @@ -16,7 +16,7 @@ func TestAllFeatureStatus(t *testing.T) { ConsensusParticipate, JSONRequests, GnosisBlockHotfix, - Exponential, + Linear, } for _, feature := range features { diff --git a/core/consensus/utils/roundtimer.go b/core/consensus/utils/roundtimer.go index c752c4ed7..a15c3c3db 100644 --- a/core/consensus/utils/roundtimer.go +++ b/core/consensus/utils/roundtimer.go @@ -3,7 +3,6 @@ package utils import ( - "math" "strings" "sync" "time" @@ -25,9 +24,9 @@ type TimerFunc func(core.Duty) RoundTimer // GetTimerFunc returns a timer function based on the enabled features. func GetTimerFunc() TimerFunc { - if featureset.Enabled(featureset.Exponential) { + if featureset.Enabled(featureset.Linear) { return func(core.Duty) RoundTimer { - return NewExponentialRoundTimer() + return NewLinearRoundTimer() } } @@ -54,7 +53,7 @@ func (t TimerType) Eager() bool { const ( TimerIncreasing TimerType = "inc" TimerEagerDoubleLinear TimerType = "eager_dlinear" - TimerExponential TimerType = "exponential" + TimerLinear TimerType = "linear" ) // increasingRoundTimeout returns the duration for a round that starts at incRoundStart in round 1 @@ -159,40 +158,40 @@ func (t *doubleEagerLinearRoundTimer) Timer(round int64) (<-chan time.Time, func return timer.Chan(), func() { timer.Stop() } } -// exponentialRoundTimer implements a round timerType with the following properties: +// linearRoundTimer implements a round timerType with the following properties: // // The first round has one second to complete consensus // If this round fails then other peers already had time to fetch proposal and therefore // won't need as much time to reach a consensus. Therefore start timeout with lower value -// which will increase exponentially -type exponentialRoundTimer struct { +// which will increase linearly +type linearRoundTimer struct { clock clockwork.Clock } -func (*exponentialRoundTimer) Type() TimerType { - return TimerExponential +func (*linearRoundTimer) Type() TimerType { + return TimerLinear } -func (t *exponentialRoundTimer) Timer(round int64) (<-chan time.Time, func()) { +func (t *linearRoundTimer) Timer(round int64) (<-chan time.Time, func()) { var timer clockwork.Timer if round == 1 { // First round has 1 second timer = t.clock.NewTimer(time.Second) } else { - // Subsequent rounds have exponentially more time starting at 200 milliseconds - timer = t.clock.NewTimer(time.Millisecond * time.Duration(math.Pow(2, float64(round-1))*100)) + // Subsequent rounds have linearly more time starting at 200 milliseconds + timer = t.clock.NewTimer(time.Duration(200*(round-1) + 200)) } return timer.Chan(), func() { timer.Stop() } } -// NewExponentialRoundTimer returns a new exponential round timer type. -func NewExponentialRoundTimer() RoundTimer { - return NewExponentialRoundTimerWithClock(clockwork.NewRealClock()) +// NewLinearRoundTimer returns a new linear round timer type. +func NewLinearRoundTimer() RoundTimer { + return NewLinearRoundTimerWithClock(clockwork.NewRealClock()) } -func NewExponentialRoundTimerWithClock(clock clockwork.Clock) RoundTimer { - return &exponentialRoundTimer{ +func NewLinearRoundTimerWithClock(clock clockwork.Clock) RoundTimer { + return &linearRoundTimer{ clock: clock, } } diff --git a/core/consensus/utils/roundtimer_test.go b/core/consensus/utils/roundtimer_test.go index 418c233f8..d2c544615 100644 --- a/core/consensus/utils/roundtimer_test.go +++ b/core/consensus/utils/roundtimer_test.go @@ -111,7 +111,7 @@ func TestDoubleEagerLinearRoundTimer(t *testing.T) { stop() } -func TestExponentialRoundTimer(t *testing.T) { +func TestLinearRoundTimer(t *testing.T) { tests := []struct { name string round int64 @@ -125,12 +125,12 @@ func TestExponentialRoundTimer(t *testing.T) { { name: "round 2", round: 2, - want: 200 * time.Millisecond, + want: 400 * time.Millisecond, }, { name: "round 3", round: 3, - want: 400 * time.Millisecond, + want: 600 * time.Millisecond, }, { name: "round 4", @@ -141,7 +141,7 @@ func TestExponentialRoundTimer(t *testing.T) { for _, tt := range tests { fakeClock := clockwork.NewFakeClock() - timer := utils.NewExponentialRoundTimerWithClock(fakeClock) + timer := utils.NewLinearRoundTimerWithClock(fakeClock) t.Run(tt.name, func(t *testing.T) { // Start the timerType @@ -170,14 +170,14 @@ func TestGetTimerFunc(t *testing.T) { require.Equal(t, utils.TimerEagerDoubleLinear, timerFunc(core.NewAttesterDuty(2)).Type()) featureset.DisableForT(t, featureset.EagerDoubleLinear) - featureset.EnableForT(t, featureset.Exponential) + featureset.EnableForT(t, featureset.Linear) timerFunc = utils.GetTimerFunc() - require.Equal(t, utils.TimerExponential, timerFunc(core.NewAttesterDuty(0)).Type()) - require.Equal(t, utils.TimerExponential, timerFunc(core.NewAttesterDuty(1)).Type()) - require.Equal(t, utils.TimerExponential, timerFunc(core.NewAttesterDuty(2)).Type()) + require.Equal(t, utils.TimerLinear, timerFunc(core.NewAttesterDuty(0)).Type()) + require.Equal(t, utils.TimerLinear, timerFunc(core.NewAttesterDuty(1)).Type()) + require.Equal(t, utils.TimerLinear, timerFunc(core.NewAttesterDuty(2)).Type()) - featureset.DisableForT(t, featureset.Exponential) + featureset.DisableForT(t, featureset.Linear) timerFunc = utils.GetTimerFunc() require.Equal(t, utils.TimerIncreasing, timerFunc(core.NewAttesterDuty(0)).Type()) diff --git a/docs/consensus.md b/docs/consensus.md index 42c775634..1f75e5b86 100644 --- a/docs/consensus.md +++ b/docs/consensus.md @@ -67,9 +67,9 @@ The `IncreasingRoundTimer` uses a linear increment strategy for round durations. The `EagerDoubleLinearRoundTimer` aligns start times across participants by starting at an absolute time and doubling the round duration when the leader is active. The round duration increases linearly according to `LinearRoundInc`. This aims to fix an issue with the original solution where the leader resets the timer at the start of the round while others reset when they receive the justified pre-prepare which leads to leaders getting out of sync with the rest. This timer is enabled by default and doesn't require additional flags. -### Exponential Round Timer +### Linear Round Timer -The `ExponentialRoundTimer` increases round durations exponentially. It provides a sufficient timeout for the initial round and grows from a smaller base timeout for subsequent rounds. The idea behind this timer is, that the consensus timeout includes fetching the signing data. As all nodes do that at the start, irregardless if they are leader or not, after the first timeout, the remaining nodes already had time to fetch their signing data. Therefore they won't need as much time to reach consensus as the leader for the first round did. The shorter subsequent rounds allow us to more quickly skip underperforming leader when compared to both `IncreasingRoundTimer` and `EagerDoubleLinearRoundTimer`, giving more leaders a chance to advance the protocol before its too late. To enable this timer, use the flag `--feature-set-enable "exponential"`. Since this timer has precedence over the `EagerDoubleLinearRoundTimer` there is no need to disable the default timer. +The `LinearRoundTimer` increases round durations linearly. It provides a sufficient timeout for the initial round and grows from a smaller base timeout for subsequent rounds. The idea behind this timer is, that the consensus timeout includes fetching the signing data. As all nodes do that at the start, irregardless if they are leader or not, after the first timeout, the remaining nodes already had time to fetch their signing data. Therefore they won't need as much time to reach consensus as the leader for the first round did. The shorter subsequent rounds allow us to more quickly skip underperforming leader when compared to both `IncreasingRoundTimer` and `EagerDoubleLinearRoundTimer`, giving more leaders a chance to advance the protocol before its too late. To enable this timer, use the flag `--feature-set-enable "linear"`. Since this timer has precedence over the `EagerDoubleLinearRoundTimer` there is no need to disable the default timer. ## Observability From e4d463f525e3e85cf0d31b22c835ebef9d2a947f Mon Sep 17 00:00:00 2001 From: Diogo Santos Date: Fri, 17 Jan 2025 10:10:17 +0000 Subject: [PATCH 2/3] fix comment --- core/consensus/utils/roundtimer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/consensus/utils/roundtimer.go b/core/consensus/utils/roundtimer.go index a15c3c3db..e9c0658d1 100644 --- a/core/consensus/utils/roundtimer.go +++ b/core/consensus/utils/roundtimer.go @@ -178,7 +178,7 @@ func (t *linearRoundTimer) Timer(round int64) (<-chan time.Time, func()) { // First round has 1 second timer = t.clock.NewTimer(time.Second) } else { - // Subsequent rounds have linearly more time starting at 200 milliseconds + // Subsequent rounds have linearly more time starting at 400 milliseconds timer = t.clock.NewTimer(time.Duration(200*(round-1) + 200)) } From ba416b2bca1bd18e139752cb090f5c758cb28a67 Mon Sep 17 00:00:00 2001 From: Diogo Santos Date: Mon, 20 Jan 2025 10:47:43 +0000 Subject: [PATCH 3/3] update linear timer to only affect proposer duties --- core/consensus/utils/roundtimer.go | 11 +++++++++-- core/consensus/utils/roundtimer_test.go | 21 ++++++++++++++++----- docs/consensus.md | 2 +- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/core/consensus/utils/roundtimer.go b/core/consensus/utils/roundtimer.go index e9c0658d1..4364156b5 100644 --- a/core/consensus/utils/roundtimer.go +++ b/core/consensus/utils/roundtimer.go @@ -25,8 +25,15 @@ type TimerFunc func(core.Duty) RoundTimer // GetTimerFunc returns a timer function based on the enabled features. func GetTimerFunc() TimerFunc { if featureset.Enabled(featureset.Linear) { - return func(core.Duty) RoundTimer { - return NewLinearRoundTimer() + return func(duty core.Duty) RoundTimer { + // Linear timer only affects Proposer duty + if duty.Type == core.DutyProposer { + return NewLinearRoundTimer() + } else if featureset.Enabled(featureset.EagerDoubleLinear) { + return NewDoubleEagerLinearRoundTimer() + } + + return NewIncreasingRoundTimer() } } diff --git a/core/consensus/utils/roundtimer_test.go b/core/consensus/utils/roundtimer_test.go index d2c544615..daaaaaba4 100644 --- a/core/consensus/utils/roundtimer_test.go +++ b/core/consensus/utils/roundtimer_test.go @@ -170,17 +170,28 @@ func TestGetTimerFunc(t *testing.T) { require.Equal(t, utils.TimerEagerDoubleLinear, timerFunc(core.NewAttesterDuty(2)).Type()) featureset.DisableForT(t, featureset.EagerDoubleLinear) - featureset.EnableForT(t, featureset.Linear) timerFunc = utils.GetTimerFunc() - require.Equal(t, utils.TimerLinear, timerFunc(core.NewAttesterDuty(0)).Type()) - require.Equal(t, utils.TimerLinear, timerFunc(core.NewAttesterDuty(1)).Type()) - require.Equal(t, utils.TimerLinear, timerFunc(core.NewAttesterDuty(2)).Type()) + require.Equal(t, utils.TimerIncreasing, timerFunc(core.NewAttesterDuty(0)).Type()) + require.Equal(t, utils.TimerIncreasing, timerFunc(core.NewAttesterDuty(1)).Type()) + require.Equal(t, utils.TimerIncreasing, timerFunc(core.NewAttesterDuty(2)).Type()) - featureset.DisableForT(t, featureset.Linear) + featureset.EnableForT(t, featureset.Linear) timerFunc = utils.GetTimerFunc() + // non proposer duty, defaults to increasing require.Equal(t, utils.TimerIncreasing, timerFunc(core.NewAttesterDuty(0)).Type()) require.Equal(t, utils.TimerIncreasing, timerFunc(core.NewAttesterDuty(1)).Type()) require.Equal(t, utils.TimerIncreasing, timerFunc(core.NewAttesterDuty(2)).Type()) + + featureset.EnableForT(t, featureset.EagerDoubleLinear) + // non proposer duty, defaults to eager + require.Equal(t, utils.TimerEagerDoubleLinear, timerFunc(core.NewAttesterDuty(0)).Type()) + require.Equal(t, utils.TimerEagerDoubleLinear, timerFunc(core.NewAttesterDuty(1)).Type()) + require.Equal(t, utils.TimerEagerDoubleLinear, timerFunc(core.NewAttesterDuty(2)).Type()) + + // proposer duty, uses linear + require.Equal(t, utils.TimerLinear, timerFunc(core.NewProposerDuty(0)).Type()) + require.Equal(t, utils.TimerLinear, timerFunc(core.NewProposerDuty(1)).Type()) + require.Equal(t, utils.TimerLinear, timerFunc(core.NewProposerDuty(2)).Type()) } diff --git a/docs/consensus.md b/docs/consensus.md index 1f75e5b86..f9015c2c1 100644 --- a/docs/consensus.md +++ b/docs/consensus.md @@ -69,7 +69,7 @@ The `EagerDoubleLinearRoundTimer` aligns start times across participants by star ### Linear Round Timer -The `LinearRoundTimer` increases round durations linearly. It provides a sufficient timeout for the initial round and grows from a smaller base timeout for subsequent rounds. The idea behind this timer is, that the consensus timeout includes fetching the signing data. As all nodes do that at the start, irregardless if they are leader or not, after the first timeout, the remaining nodes already had time to fetch their signing data. Therefore they won't need as much time to reach consensus as the leader for the first round did. The shorter subsequent rounds allow us to more quickly skip underperforming leader when compared to both `IncreasingRoundTimer` and `EagerDoubleLinearRoundTimer`, giving more leaders a chance to advance the protocol before its too late. To enable this timer, use the flag `--feature-set-enable "linear"`. Since this timer has precedence over the `EagerDoubleLinearRoundTimer` there is no need to disable the default timer. +The `LinearRoundTimer` increases round durations linearly. It provides a sufficient timeout for the initial round and grows from a smaller base timeout for subsequent rounds. The idea behind this timer is, that the consensus timeout includes fetching the signing data. As all nodes do that at the start, irregardless if they are leader or not, after the first timeout, the remaining nodes already had time to fetch their signing data. Therefore they won't need as much time to reach consensus as the leader for the first round did. The shorter subsequent rounds allow us to more quickly skip underperforming leader when compared to both `IncreasingRoundTimer` and `EagerDoubleLinearRoundTimer`, giving more leaders a chance to advance the protocol before its too late. This timer only affects Proposer duties, for the remaning ones it fallbacks to either `EagerDoubleLinearRoundTimer` or `IncreasingRoundTimer` depending on feature set flags. To enable it, use the flag `--feature-set-enable "linear"`. Since this timer has precedence over the `EagerDoubleLinearRoundTimer` there is no need to disable the default timer. ## Observability