From a858d76ef5d10b76c2604c319ea59bcc6326d335 Mon Sep 17 00:00:00 2001 From: bruce-riley <96066700+bruce-riley@users.noreply.github.com> Date: Wed, 17 Jan 2024 10:12:00 -0600 Subject: [PATCH] Node: Always cut over to quic-v1 (#3715) --- node/pkg/p2p/cutover.go | 105 ++------------------------------- node/pkg/p2p/cutover_test.go | 74 ----------------------- node/pkg/p2p/p2p.go | 5 +- node/pkg/p2p/watermark_test.go | 4 -- 4 files changed, 8 insertions(+), 180 deletions(-) diff --git a/node/pkg/p2p/cutover.go b/node/pkg/p2p/cutover.go index 0c358d05e..0d7d4790f 100644 --- a/node/pkg/p2p/cutover.go +++ b/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 diff --git a/node/pkg/p2p/cutover_test.go b/node/pkg/p2p/cutover_test.go index 4b7ffaf28..3fc7d7991 100644 --- a/node/pkg/p2p/cutover_test.go +++ b/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) -} diff --git a/node/pkg/p2p/p2p.go b/node/pkg/p2p/p2p.go index 4dbbe207d..2642e3378 100644 --- a/node/pkg/p2p/p2p.go +++ b/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()) diff --git a/node/pkg/p2p/watermark_test.go b/node/pkg/p2p/watermark_test.go index 2dd101145..3e1373b28 100644 --- a/node/pkg/p2p/watermark_test.go +++ b/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()