From de04f573cfaa837155912526666b70a72f8089b7 Mon Sep 17 00:00:00 2001
From: William Banfield <4561443+williambanfield@users.noreply.github.com>
Date: Wed, 2 Feb 2022 10:25:04 -0500
Subject: [PATCH] types: make timely predicate adaptive after 10 rounds (#7739)

This change adds logic to double the message delay bound after every 10 rounds. Alternatives to this somewhat magic number were discussed. Specifically, whether or not to make '10' modifiable as a parameter was discussed. Since this behavior only exists to ensure liveness in the case that these values were poorly chosen to begin with, a method to configure this value was not created. Chains that notice many 'untimely' rounds per the [relevant metric](https://github.com/tendermint/tendermint/pull/7709) are expected to take action to increase the configured message delay to more accurately match the conditions of the network.

closes: https://github.com/tendermint/spec/issues/371
---
 internal/consensus/state.go |  4 +-
 types/proposal.go           | 20 +++++++++-
 types/proposal_test.go      | 78 +++++++++++++++++++++++--------------
 3 files changed, 68 insertions(+), 34 deletions(-)

diff --git a/internal/consensus/state.go b/internal/consensus/state.go
index 7d31af906..2cee22972 100644
--- a/internal/consensus/state.go
+++ b/internal/consensus/state.go
@@ -1402,7 +1402,7 @@ func (cs *State) proposalIsTimely() bool {
 		MessageDelay: cs.state.ConsensusParams.Synchrony.MessageDelay,
 	}
 
-	return cs.Proposal.IsTimely(cs.ProposalReceiveTime, sp)
+	return cs.Proposal.IsTimely(cs.ProposalReceiveTime, sp, cs.Round)
 }
 
 func (cs *State) defaultDoPrevote(ctx context.Context, height int64, round int32) {
@@ -2590,7 +2590,7 @@ func (cs *State) calculateProposalTimestampDifferenceMetric() {
 			MessageDelay: cs.state.ConsensusParams.Synchrony.MessageDelay,
 		}
 
-		isTimely := cs.Proposal.IsTimely(cs.ProposalReceiveTime, tp)
+		isTimely := cs.Proposal.IsTimely(cs.ProposalReceiveTime, tp, cs.Round)
 		cs.metrics.ProposalTimestampDifference.With("is_timely", fmt.Sprintf("%t", isTimely)).
 			Observe(cs.ProposalReceiveTime.Sub(cs.Proposal.Timestamp).Seconds())
 	}
diff --git a/types/proposal.go b/types/proposal.go
index 87171ff46..a4009eea2 100644
--- a/types/proposal.go
+++ b/types/proposal.go
@@ -3,6 +3,7 @@ package types
 import (
 	"errors"
 	"fmt"
+	"math/bits"
 	"time"
 
 	"github.com/tendermint/tendermint/internal/libs/protoio"
@@ -89,11 +90,26 @@ func (p *Proposal) ValidateBasic() error {
 //
 // For more information on the meaning of 'timely', see the proposer-based timestamp specification:
 // https://github.com/tendermint/spec/tree/master/spec/consensus/proposer-based-timestamp
-func (p *Proposal) IsTimely(recvTime time.Time, sp SynchronyParams) bool {
+func (p *Proposal) IsTimely(recvTime time.Time, sp SynchronyParams, round int32) bool {
+	// The message delay values are scaled as rounds progress.
+	// Every 10 rounds, the message delay is doubled to allow consensus to
+	// proceed in the case that the chosen value was too small for the given network conditions.
+	// For more information and discussion on this mechanism, see the relevant github issue:
+	// https://github.com/tendermint/spec/issues/371
+	maxShift := bits.LeadingZeros64(uint64(sp.MessageDelay)) - 1
+	nShift := int((round / 10))
+
+	if nShift > maxShift {
+		// if the number of 'doublings' would would overflow the size of the int, use the
+		// maximum instead.
+		nShift = maxShift
+	}
+	msgDelay := sp.MessageDelay * time.Duration(1<<nShift)
+
 	// lhs is `proposedBlockTime - Precision` in the first inequality
 	lhs := p.Timestamp.Add(-sp.Precision)
 	// rhs is `proposedBlockTime + MsgDelay + Precision` in the second inequality
-	rhs := p.Timestamp.Add(sp.MessageDelay).Add(sp.Precision)
+	rhs := p.Timestamp.Add(msgDelay).Add(sp.Precision)
 
 	if recvTime.Before(lhs) || recvTime.After(rhs) {
 		return false
diff --git a/types/proposal_test.go b/types/proposal_test.go
index fc99a3884..b8b8b3a67 100644
--- a/types/proposal_test.go
+++ b/types/proposal_test.go
@@ -216,54 +216,72 @@ func TestIsTimely(t *testing.T) {
 	genesisTime, err := time.Parse(time.RFC3339, "2019-03-13T23:00:00Z")
 	require.NoError(t, err)
 	testCases := []struct {
-		name           string
-		proposalHeight int64
-		proposalTime   time.Time
-		recvTime       time.Time
-		precision      time.Duration
-		msgDelay       time.Duration
-		expectTimely   bool
+		name         string
+		proposalTime time.Time
+		recvTime     time.Time
+		precision    time.Duration
+		msgDelay     time.Duration
+		expectTimely bool
+		round        int32
 	}{
 		// proposalTime - precision <= localTime <= proposalTime + msgDelay + precision
 		{
 			// Checking that the following inequality evaluates to true:
 			// 0 - 2 <= 1 <= 0 + 1 + 2
-			name:           "basic timely",
-			proposalHeight: 2,
-			proposalTime:   genesisTime,
-			recvTime:       genesisTime.Add(1 * time.Nanosecond),
-			precision:      time.Nanosecond * 2,
-			msgDelay:       time.Nanosecond,
-			expectTimely:   true,
+			name:         "basic timely",
+			proposalTime: genesisTime,
+			recvTime:     genesisTime.Add(1 * time.Nanosecond),
+			precision:    time.Nanosecond * 2,
+			msgDelay:     time.Nanosecond,
+			expectTimely: true,
 		},
 		{
 			// Checking that the following inequality evaluates to false:
 			// 0 - 2 <= 4 <= 0 + 1 + 2
-			name:           "local time too large",
-			proposalHeight: 2,
-			proposalTime:   genesisTime,
-			recvTime:       genesisTime.Add(4 * time.Nanosecond),
-			precision:      time.Nanosecond * 2,
-			msgDelay:       time.Nanosecond,
-			expectTimely:   false,
+			name:         "local time too large",
+			proposalTime: genesisTime,
+			recvTime:     genesisTime.Add(4 * time.Nanosecond),
+			precision:    time.Nanosecond * 2,
+			msgDelay:     time.Nanosecond,
+			expectTimely: false,
 		},
 		{
 			// Checking that the following inequality evaluates to false:
 			// 4 - 2 <= 0 <= 4 + 2 + 1
-			name:           "proposal time too large",
-			proposalHeight: 2,
-			proposalTime:   genesisTime.Add(4 * time.Nanosecond),
-			recvTime:       genesisTime,
-			precision:      time.Nanosecond * 2,
-			msgDelay:       time.Nanosecond,
-			expectTimely:   false,
+			name:         "proposal time too large",
+			proposalTime: genesisTime.Add(4 * time.Nanosecond),
+			recvTime:     genesisTime,
+			precision:    time.Nanosecond * 2,
+			msgDelay:     time.Nanosecond,
+			expectTimely: false,
+		},
+		{
+			// Checking that the following inequality evaluates to true:
+			// 0 - (2 * 2)  <= 4 <= 0 + (1 * 2) + 2
+			name:         "message delay adapts after 10 rounds",
+			proposalTime: genesisTime,
+			recvTime:     genesisTime.Add(4 * time.Nanosecond),
+			precision:    time.Nanosecond * 2,
+			msgDelay:     time.Nanosecond,
+			expectTimely: true,
+			round:        10,
+		},
+		{
+			// check that values that overflow time.Duration still correctly register
+			// as timely when round relaxation applied.
+			name:         "message delay fixed to not overflow time.Duration",
+			proposalTime: genesisTime,
+			recvTime:     genesisTime.Add(4 * time.Nanosecond),
+			precision:    time.Nanosecond * 2,
+			msgDelay:     time.Nanosecond,
+			expectTimely: true,
+			round:        5000,
 		},
 	}
 
 	for _, testCase := range testCases {
 		t.Run(testCase.name, func(t *testing.T) {
 			p := Proposal{
-				Height:    testCase.proposalHeight,
 				Timestamp: testCase.proposalTime,
 			}
 
@@ -272,7 +290,7 @@ func TestIsTimely(t *testing.T) {
 				MessageDelay: testCase.msgDelay,
 			}
 
-			ti := p.IsTimely(testCase.recvTime, sp)
+			ti := p.IsTimely(testCase.recvTime, sp, testCase.round)
 			assert.Equal(t, testCase.expectTimely, ti)
 		})
 	}