Browse Source

Node: Always cut over to quic-v1 (#3715)

bruce-riley 1 year ago
parent
commit
a858d76ef5
4 changed files with 8 additions and 180 deletions
  1. 6 99
      node/pkg/p2p/cutover.go
  2. 0 74
      node/pkg/p2p/cutover_test.go
  3. 2 3
      node/pkg/p2p/p2p.go
  4. 0 4
      node/pkg/p2p/watermark_test.go

+ 6 - 99
node/pkg/p2p/cutover.go

@@ -1,112 +1,19 @@
 package p2p
 
 import (
-	"fmt"
 	"strings"
-	"time"
-
-	"go.uber.org/zap"
 )
 
-// The format of this time is very picky. Please use the exact format specified by cutOverFmtStr!
-const mainnetCutOverTimeStr = "2024-01-16T15:00:00-0000"
-const testnetCutOverTimeStr = "2023-11-07T14:00:00-0000"
-const devnetCutOverTimeStr = "2022-12-31T23:59:59-0000"
-const cutOverFmtStr = "2006-01-02T15:04:05-0700"
-
-// shouldCutOverPtr is a global variable used to determine if a cut over is in progress. It is initialized by the first call evaluateCutOver.
-var shouldCutOverPtr *bool
-
-// shouldCutOver uses the global variable to determine if a cut over is in progress. It assumes evaluateCutOver has already been called, so will panic if the pointer is nil.
-func shouldCutOver() bool {
-	if shouldCutOverPtr == nil {
-		panic("shouldCutOverPtr is nil")
-	}
-
-	return *shouldCutOverPtr
-}
-
-// evaluateCutOver determines if a cut over is in progress. The first time it is called, it sets the global variable shouldCutOverPtr. It may be called more than once.
-func evaluateCutOver(logger *zap.Logger, networkID string) error {
-	if shouldCutOverPtr != nil {
-		return nil
-	}
-
-	cutOverTimeStr := getCutOverTimeStr(networkID)
-
-	sco, delay, err := evaluateCutOverImpl(logger, cutOverTimeStr, time.Now())
-	if err != nil {
-		return err
-	}
-
-	shouldCutOverPtr = &sco
-
-	if delay != time.Duration(0) {
-		// Wait for the cut over time and then panic so we restart with the new quic-v1.
-		go func() {
-			time.Sleep(delay)
-			logger.Info("time to cut over to new quic-v1", zap.String("cutOverTime", cutOverTimeStr), zap.String("component", "p2pco"))
-			panic("p2pco: time to cut over to new quic-v1")
-		}()
-	}
-
-	return nil
-}
-
-// evaluateCutOverImpl performs the actual cut over check. It is a separate function for testing purposes.
-func evaluateCutOverImpl(logger *zap.Logger, cutOverTimeStr string, now time.Time) (bool, time.Duration, error) {
-	if cutOverTimeStr == "" {
-		return false, 0, nil
-	}
-
-	cutOverTime, err := time.Parse(cutOverFmtStr, cutOverTimeStr)
-	if err != nil {
-		return false, 0, fmt.Errorf(`failed to parse cut over time: %w`, err)
-	}
-
-	if cutOverTime.Before(now) {
-		logger.Info("cut over time has passed, should use new quic-v1", zap.String("cutOverTime", cutOverTime.Format(cutOverFmtStr)), zap.String("now", now.Format(cutOverFmtStr)), zap.String("component", "p2pco"))
-		return true, 0, nil
-	}
-
-	// If we get here, we need to wait for the cutover and then force a restart.
-	delay := cutOverTime.Sub(now)
-	logger.Info("still waiting for cut over time",
-		zap.Stringer("cutOverTime", cutOverTime),
-		zap.String("now", now.Format(cutOverFmtStr)),
-		zap.Stringer("delay", delay),
-		zap.String("component", "p2pco"))
-
-	return false, delay, nil
-}
-
-// getCutOverTimeStr returns the cut over time string based on the network ID passed in.
-func getCutOverTimeStr(networkID string) string {
-	if strings.Contains(networkID, "/mainnet/") {
-		return mainnetCutOverTimeStr
-	}
-	if strings.Contains(networkID, "/testnet/") {
-		return testnetCutOverTimeStr
-	}
-	return devnetCutOverTimeStr
-}
-
-// cutOverBootstrapPeers checks to see if we are supposed to cut over, and if so updates the bootstrap peers. It assumes that the string has previously been validated.
+// cutOverBootstrapPeers updates the bootstrap peers to reflect the new quic-v1. It assumes that the string has previously been validated.
 func cutOverBootstrapPeers(bootstrapPeers string) string {
-	if shouldCutOver() {
-		bootstrapPeers = strings.ReplaceAll(bootstrapPeers, "/quic/", "/quic-v1/")
-	}
-
-	return bootstrapPeers
+	return strings.ReplaceAll(bootstrapPeers, "/quic/", "/quic-v1/")
 }
 
-// cutOverAddressPattern checks to see if we are supposed to cut over, and if so updates the address patterns. It assumes that the string is valid.
+// cutOverAddressPattern updates the address patterns. It assumes that the string is valid.
 func cutOverAddressPattern(pattern string) string {
-	if shouldCutOver() {
-		if !strings.Contains(pattern, "/quic-v1") {
-			// These patterns are hardcoded so we are not worried about invalid values.
-			pattern = strings.ReplaceAll(pattern, "/quic", "/quic-v1")
-		}
+	if !strings.Contains(pattern, "/quic-v1") {
+		// These patterns are hardcoded so we are not worried about invalid values.
+		pattern = strings.ReplaceAll(pattern, "/quic", "/quic-v1")
 	}
 
 	return pattern

+ 0 - 74
node/pkg/p2p/cutover_test.go

@@ -4,7 +4,6 @@ import (
 	"os"
 	"strings"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -13,8 +12,6 @@ import (
 
 // We want to be able to test the cutover conversion stuff so force us into cutover mode.
 func TestMain(m *testing.M) {
-	sco := true
-	shouldCutOverPtr = &sco
 	os.Exit(m.Run())
 }
 
@@ -38,75 +35,4 @@ func TestCutOverListeningAddresses(t *testing.T) {
 	}
 }
 
-func TestVerifyCutOverTime(t *testing.T) {
-	if mainnetCutOverTimeStr != "" {
-		_, err := time.Parse(cutOverFmtStr, mainnetCutOverTimeStr)
-		require.NoError(t, err)
-	}
-	if testnetCutOverTimeStr != "" {
-		_, err := time.Parse(cutOverFmtStr, testnetCutOverTimeStr)
-		require.NoError(t, err)
-	}
-	if devnetCutOverTimeStr != "" {
-		_, err := time.Parse(cutOverFmtStr, devnetCutOverTimeStr)
-		require.NoError(t, err)
-	}
-}
-
 const oldBootstrapPeers = "/dns4/guardian-0.guardian/udp/8999/quic/p2p/12D3KooWL3XJ9EMCyZvmmGXL2LMiVBtrVa2BuESsJiXkSj7333Jw,/dns4/guardian-0.guardian/udp/8999/quic/p2p/12D3KooWL3XJ9EMCyZvmmGXL2LMiVBtrVa2BuESsJiXkSj7333Jx"
-
-func TestGetCutOverTimeStr(t *testing.T) {
-	assert.Equal(t, mainnetCutOverTimeStr, getCutOverTimeStr("blah/blah/mainnet/blah"))
-	assert.Equal(t, testnetCutOverTimeStr, getCutOverTimeStr("blah/blah/testnet/blah"))
-	assert.Equal(t, devnetCutOverTimeStr, getCutOverTimeStr("blah/blah/devnet/blah"))
-}
-
-func TestCutOverDisabled(t *testing.T) {
-	logger := zap.NewNop()
-
-	cutOverTimeStr := ""
-	now, err := time.Parse(cutOverFmtStr, "2023-10-06T18:19:00-0000")
-	require.NoError(t, err)
-
-	cuttingOver, delay, err := evaluateCutOverImpl(logger, cutOverTimeStr, now)
-	require.NoError(t, err)
-	assert.False(t, cuttingOver)
-	assert.Equal(t, time.Duration(0), delay)
-}
-
-func TestCutOverInvalidTime(t *testing.T) {
-	logger := zap.NewNop()
-
-	cutOverTimeStr := "Hello World"
-	now, err := time.Parse(cutOverFmtStr, "2023-10-06T18:19:00-0000")
-	require.NoError(t, err)
-
-	_, _, err = evaluateCutOverImpl(logger, cutOverTimeStr, now)
-	require.EqualError(t, err, `failed to parse cut over time: parsing time "Hello World" as "2006-01-02T15:04:05-0700": cannot parse "Hello World" as "2006"`)
-}
-
-func TestCutOverAlreadyHappened(t *testing.T) {
-	logger := zap.NewNop()
-
-	cutOverTimeStr := "2023-10-06T18:18:00-0000"
-	now, err := time.Parse(cutOverFmtStr, "2023-10-06T18:19:00-0000")
-	require.NoError(t, err)
-
-	cuttingOver, delay, err := evaluateCutOverImpl(logger, cutOverTimeStr, now)
-	require.NoError(t, err)
-	assert.True(t, cuttingOver)
-	assert.Equal(t, time.Duration(0), delay)
-}
-
-func TestCutOverDelayRequired(t *testing.T) {
-	logger := zap.NewNop()
-
-	cutOverTimeStr := "2023-10-06T18:18:00-0000"
-	now, err := time.Parse(cutOverFmtStr, "2023-10-06T17:18:00-0000")
-	require.NoError(t, err)
-
-	cuttingOver, delay, err := evaluateCutOverImpl(logger, cutOverTimeStr, now)
-	require.NoError(t, err)
-	assert.False(t, cuttingOver)
-	assert.Equal(t, time.Duration(60*time.Minute), delay)
-}

+ 2 - 3
node/pkg/p2p/p2p.go

@@ -196,9 +196,6 @@ func ConnectToPeers(ctx context.Context, logger *zap.Logger, h host.Host, peers
 }
 
 func NewHost(logger *zap.Logger, ctx context.Context, networkID string, bootstrapPeers string, components *Components, priv crypto.PrivKey) (host.Host, error) {
-	if err := evaluateCutOver(logger, networkID); err != nil {
-		return nil, err
-	}
 	h, err := libp2p.New(
 		// Use the keypair we generated
 		libp2p.Identity(priv),
@@ -220,6 +217,8 @@ func NewHost(logger *zap.Logger, ctx context.Context, networkID string, bootstra
 
 		// Let this host use the DHT to find other hosts
 		libp2p.Routing(func(h host.Host) (routing.PeerRouting, error) {
+			// Update the bootstrap peers string so we will log the updated value.
+			bootstrapPeers = cutOverBootstrapPeers(bootstrapPeers)
 			logger.Info("Connecting to bootstrap peers", zap.String("bootstrap_peers", bootstrapPeers))
 
 			bootstrappers, _ := BootstrapAddrs(logger, bootstrapPeers, h.ID())

+ 0 - 4
node/pkg/p2p/watermark_test.go

@@ -99,10 +99,6 @@ func NewG(t *testing.T, nodeName string) *G {
 // TestWatermark runs 4 different guardians one of which does not send its P2PID in the signed part of the heartbeat.
 // The expectation is that hosts that send this information will become "protected" by the Connection Manager.
 func TestWatermark(t *testing.T) {
-	logger := zap.NewNop()
-	err := evaluateCutOver(logger, "/wormhole/dev")
-	require.NoError(t, err)
-
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()