Merge pull request #11107 from smartcontractkit/BCF-2716-p2p-warn-bac…

…kport

BCF-2716: P2P.V1 default swap & deprecation warning - [v2.7.0]

core/cmd/shell_local.go

@@ -290,7 +290,7 @@ func (s *Shell) runNode(c *cli.Context) error {
 
 	s.Config.SetPasswords(pwd, vrfpwd)
 
-	s.Config.LogConfiguration(lggr.Debugf)
+	s.Config.LogConfiguration(lggr.Debugf, lggr.Warnf)
 
 	if err := s.Config.Validate(); err != nil {
 		return errors.Wrap(err, "config validation failed")
@@ -689,7 +689,8 @@ var errDBURLMissing = errors.New("You must set CL_DATABASE_URL env variable or p
 
 // ConfigValidate validate the client configuration and pretty-prints results
 func (s *Shell) ConfigFileValidate(_ *cli.Context) error {
-	s.Config.LogConfiguration(func(f string, params ...any) { fmt.Printf(f, params...) })
+	fn := func(f string, params ...any) { fmt.Printf(f, params...) }
+	s.Config.LogConfiguration(fn, fn)
 	if err := s.configExitErr(s.Config.Validate); err != nil {
 		return err
 	}

core/config/app_config.go

@@ -28,7 +28,7 @@ type AppConfig interface {
 
 	Validate() error
 	ValidateDB() error
-	LogConfiguration(log LogfFn)
+	LogConfiguration(log, warn LogfFn)
 	SetLogLevel(lvl zapcore.Level) error
 	SetLogSQL(logSQL bool)
 	SetPasswords(keystore, vrf *string)

core/config/docs/core.toml

@@ -359,6 +359,8 @@ TraceLogging = false # Default
 # automatically fall back to V1. If V2 starts working again later, it will automatically be preferred again. This is useful
 # for migrating networks without downtime. Note that the two networking stacks _must not_ be configured to bind to the same IP/port.
 #
+# Note: P2P.V1 is deprecated will be removed in the future.
+#
 # All nodes in the OCR network should share the same networking stack.
 [P2P]
 # IncomingMessageBufferSize is the per-remote number of incoming
@@ -377,9 +379,10 @@ PeerID = '12D3KooWMoejJznyDuEk5aX6GvbjaG12UzeornPCBNzMRqdwrFJw' # Example
 # TraceLogging enables trace level logging.
 TraceLogging = false # Default
 
+# P2P.V1 is deprecated and will be removed in a future version.
 [P2P.V1]
 # Enabled enables P2P V1.
-Enabled = true # Default
+Enabled = false # Default
 # AnnounceIP should be set as the externally reachable IP address of the Chainlink node.
 AnnounceIP = '1.2.3.4' # Example
 # AnnouncePort should be set as the externally reachable port of the Chainlink node.
@@ -423,7 +426,7 @@ PeerstoreWriteInterval = '5m' # Default
 [P2P.V2]
 # Enabled enables P2P V2.
 # Note: V1.Enabled is true by default, so it must be set false in order to run V2 only.
-Enabled = false # Default
+Enabled = true # Default
 # AnnounceAddresses is the addresses the peer will advertise on the network in `host:port` form as accepted by the TCP version of Go’s `net.Dial`. 
 # The addresses should be reachable by other nodes on the network. When attempting to connect to another node, 
 # a node will attempt to dial all of the other node’s AnnounceAddresses in round-robin fashion.

core/scripts/chaincli/handler/handler.go

@@ -64,9 +64,7 @@ HTTPSPort = 0
 LogPoller = true
 [OCR2]
 Enabled = true
-[P2P]
-[P2P.V2]
-Enabled = true
+
 [Keeper]
 TurnLookBack = 0
 [[EVM]]

core/scripts/common/vrf/docker/toml-config/base.toml

@@ -26,6 +26,5 @@ HTTPSPort = 0
 
 [P2P]
 [P2P.V2]
-Enabled = true
 AnnounceAddresses = ['0.0.0.0:6690']
 ListenAddresses = ['0.0.0.0:6690']

core/scripts/ocr2vrf/util.go

@@ -14,16 +14,17 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	gethtypes "github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/ethclient"
+	"github.com/urfave/cli"
+	"go.dedis.ch/kyber/v3"
+	"go.dedis.ch/kyber/v3/group/edwards25519"
+	"go.dedis.ch/kyber/v3/pairing"
+
 	"github.com/smartcontractkit/libocr/offchainreporting2plus/confighelper"
 	"github.com/smartcontractkit/libocr/offchainreporting2plus/types"
 	"github.com/smartcontractkit/ocr2vrf/altbn_128"
 	"github.com/smartcontractkit/ocr2vrf/dkg"
 	"github.com/smartcontractkit/ocr2vrf/ocr2vrf"
 	ocr2vrftypes "github.com/smartcontractkit/ocr2vrf/types"
-	"github.com/urfave/cli"
-	"go.dedis.ch/kyber/v3"
-	"go.dedis.ch/kyber/v3/group/edwards25519"
-	"go.dedis.ch/kyber/v3/pairing"
 
 	"github.com/smartcontractkit/chainlink/v2/core/cmd"
 	"github.com/smartcontractkit/chainlink/v2/core/gethwrappers/generated/authorized_forwarder"
@@ -43,11 +44,7 @@ var (
 	g1                               = suite.G1()
 	g2                               = suite.G2()
 	tomlConfigTemplate               = `
-	[P2P.V1]
-	Enabled = false
-
 	[P2P.V2]
-	Enabled = true
 	ListenAddresses = ["127.0.0.1:8000"]
 
 	[Feature]

core/services/chainlink/config.go

@@ -52,6 +52,50 @@ func (c *Config) TOMLString() (string, error) {
 	return string(b), nil
 }
 
+// deprecationWarnings returns an error if the Config contains deprecated fields.
+// This is typically used before defaults have been applied, with input from the user.
+func (c *Config) deprecationWarnings() (err error) {
+	if c.P2P.V1 != (toml.P2PV1{}) {
+		err = multierr.Append(err, config.ErrDeprecated{Name: "P2P.V1"})
+		var err2 error
+		if c.P2P.V1.AnnounceIP != nil {
+			err2 = multierr.Append(err2, config.ErrDeprecated{Name: "AnnounceIP"})
+		}
+		if c.P2P.V1.AnnouncePort != nil {
+			err2 = multierr.Append(err2, config.ErrDeprecated{Name: "AnnouncePort"})
+		}
+		if c.P2P.V1.BootstrapCheckInterval != nil {
+			err2 = multierr.Append(err2, config.ErrDeprecated{Name: "BootstrapCheckInterval"})
+		}
+		if c.P2P.V1.DefaultBootstrapPeers != nil {
+			err2 = multierr.Append(err2, config.ErrDeprecated{Name: "DefaultBootstrapPeers"})
+		}
+		if c.P2P.V1.DHTAnnouncementCounterUserPrefix != nil {
+			err2 = multierr.Append(err2, config.ErrDeprecated{Name: "DHTAnnouncementCounterUserPrefix"})
+		}
+		if c.P2P.V1.DHTLookupInterval != nil {
+			err2 = multierr.Append(err2, config.ErrDeprecated{Name: "DHTLookupInterval"})
+		}
+		if c.P2P.V1.ListenIP != nil {
+			err2 = multierr.Append(err2, config.ErrDeprecated{Name: "ListenIP"})
+		}
+		if c.P2P.V1.ListenPort != nil {
+			err2 = multierr.Append(err2, config.ErrDeprecated{Name: "ListenPort"})
+		}
+		if c.P2P.V1.NewStreamTimeout != nil {
+			err2 = multierr.Append(err2, config.ErrDeprecated{Name: "NewStreamTimeout"})
+		}
+		if c.P2P.V1.PeerstoreWriteInterval != nil {
+			err2 = multierr.Append(err2, config.ErrDeprecated{Name: "PeerstoreWriteInterval"})
+		}
+		err2 = config.NamedMultiErrorList(err2, "P2P.V1")
+		err = multierr.Append(err, err2)
+	}
+	return
+}
+
+// Validate returns an error if the Config is not valid for use, as-is.
+// This is typically used after defaults have been applied.
 func (c *Config) Validate() error {
 	if err := config.Validate(c); err != nil {
 		return fmt.Errorf("invalid configuration: %w", err)

core/services/chainlink/config_general.go

@@ -35,11 +35,13 @@ import (
 type generalConfig struct {
 	inputTOML     string // user input, normalized via de/re-serialization
 	effectiveTOML string // with default values included
-	secretsTOML   string // with env overdies includes, redacted
+	secretsTOML   string // with env overrides includes, redacted
 
 	c       *Config // all fields non-nil (unless the legacy method signature return a pointer)
 	secrets *Secrets
 
+	warning error // warnings about inputTOML, e.g. deprecated fields
+
 	logLevelDefault zapcore.Level
 
 	appIDOnce sync.Once
@@ -123,7 +125,7 @@ func (o *GeneralConfigOpts) parseSecrets(secrets string) error {
 	return nil
 }
 
-// New returns a coreconfig.GeneralConfig for the given options.
+// New returns a GeneralConfig for the given options.
 func (o GeneralConfigOpts) New() (GeneralConfig, error) {
 	err := o.parse()
 	if err != nil {
@@ -135,6 +137,8 @@ func (o GeneralConfigOpts) New() (GeneralConfig, error) {
 		return nil, err
 	}
 
+	_, warning := utils.MultiErrorList(o.Config.deprecationWarnings())
+
 	o.Config.setDefaults()
 	if !o.SkipEnv {
 		err = o.Secrets.setEnv()
@@ -163,6 +167,7 @@ func (o GeneralConfigOpts) New() (GeneralConfig, error) {
 		secretsTOML:   secrets,
 		c:             &o.Config,
 		secrets:       &o.Secrets,
+		warning:       warning,
 	}
 	if lvl := o.Config.Log.Level; lvl != nil {
 		cfg.logLevelDefault = zapcore.Level(*lvl)
@@ -253,10 +258,13 @@ func validateEnv() (err error) {
 	return
 }
 
-func (g *generalConfig) LogConfiguration(log coreconfig.LogfFn) {
+func (g *generalConfig) LogConfiguration(log, warn coreconfig.LogfFn) {
 	log("# Secrets:\n%s\n", g.secretsTOML)
 	log("# Input Configuration:\n%s\n", g.inputTOML)
 	log("# Effective Configuration, with defaults applied:\n%s\n", g.effectiveTOML)
+	if g.warning != nil {
+		warn("# Configuration warning:\n%s\n", g.warning)
+	}
 }
 
 // ConfigTOML implements chainlink.ConfigV2

core/services/chainlink/config_p2p_test.go

@@ -4,9 +4,10 @@ import (
 	"testing"
 	"time"
 
-	"github.com/smartcontractkit/libocr/commontypes"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/smartcontractkit/libocr/commontypes"
 )
 
 func TestP2PConfig(t *testing.T) {
@@ -23,7 +24,7 @@ func TestP2PConfig(t *testing.T) {
 	assert.True(t, p2p.TraceLogging())
 
 	v1 := p2p.V1()
-	assert.False(t, v1.Enabled())
+	assert.True(t, v1.Enabled())
 	assert.Equal(t, "1.2.3.4", v1.AnnounceIP().String())
 	assert.Equal(t, uint16(1234), v1.AnnouncePort())
 	assert.Equal(t, time.Minute, v1.BootstrapCheckInterval())
@@ -38,7 +39,7 @@ func TestP2PConfig(t *testing.T) {
 	assert.Equal(t, time.Minute, v1.PeerstoreWriteInterval())
 
 	v2 := p2p.V2()
-	assert.True(t, v2.Enabled())
+	assert.False(t, v2.Enabled())
 	assert.Equal(t, []string{"a", "b", "c"}, v2.AnnounceAddresses())
 	assert.ElementsMatch(
 		t,

core/services/chainlink/config_test.go

@@ -389,7 +389,7 @@ func TestConfig_Marshal(t *testing.T) {
 		PeerID:                    mustPeerID("12D3KooWMoejJznyDuEk5aX6GvbjaG12UzeornPCBNzMRqdwrFJw"),
 		TraceLogging:              ptr(true),
 		V1: toml.P2PV1{
-			Enabled:                          ptr(false),
+			Enabled:                          ptr(true),
 			AnnounceIP:                       mustIP("1.2.3.4"),
 			AnnouncePort:                     ptr[uint16](1234),
 			BootstrapCheckInterval:           models.MustNewDuration(time.Minute),
@@ -402,7 +402,7 @@ func TestConfig_Marshal(t *testing.T) {
 			PeerstoreWriteInterval:           models.MustNewDuration(time.Minute),
 		},
 		V2: toml.P2PV2{
-			Enabled:           ptr(true),
+			Enabled:           ptr(false),
 			AnnounceAddresses: &[]string{"a", "b", "c"},
 			DefaultBootstrappers: &[]ocrcommontypes.BootstrapperLocator{
 				{PeerID: "12D3KooWMoejJznyDuEk5aX6GvbjaG12UzeornPCBNzMRqdwrFJw", Addrs: []string{"foo:42", "bar:10"}},
@@ -820,7 +820,7 @@ PeerID = '12D3KooWMoejJznyDuEk5aX6GvbjaG12UzeornPCBNzMRqdwrFJw'
 TraceLogging = true
 
 [P2P.V1]
-Enabled = false
+Enabled = true
 AnnounceIP = '1.2.3.4'
 AnnouncePort = 1234
 BootstrapCheckInterval = '1m0s'
@@ -833,7 +833,7 @@ NewStreamTimeout = '1s'
 PeerstoreWriteInterval = '1m0s'
 
 [P2P.V2]
-Enabled = true
+Enabled = false
 AnnounceAddresses = ['a', 'b', 'c']
 DefaultBootstrappers = ['12D3KooWMoejJznyDuEk5aX6GvbjaG12UzeornPCBNzMRqdwrFJw@foo:42/bar:10', '12D3KooWMoejJznyDuEk5aX6GvbjaG12UzeornPCBNzMRqdwrFJw@test:99']
 DeltaDial = '1m0s'
@@ -1248,6 +1248,21 @@ func Test_generalConfig_LogConfiguration(t *testing.T) {
 		secrets   = "# Secrets:\n"
 		input     = "# Input Configuration:\n"
 		effective = "# Effective Configuration, with defaults applied:\n"
+		warning   = "# Configuration warning:\n"
+
+		deprecated = `2 errors:
+	- P2P.V1: is deprecated and will be removed in a future version
+	- P2P.V1: 10 errors:
+		- AnnounceIP: is deprecated and will be removed in a future version
+		- AnnouncePort: is deprecated and will be removed in a future version
+		- BootstrapCheckInterval: is deprecated and will be removed in a future version
+		- DefaultBootstrapPeers: is deprecated and will be removed in a future version
+		- DHTAnnouncementCounterUserPrefix: is deprecated and will be removed in a future version
+		- DHTLookupInterval: is deprecated and will be removed in a future version
+		- ListenIP: is deprecated and will be removed in a future version
+		- ListenPort: is deprecated and will be removed in a future version
+		- NewStreamTimeout: is deprecated and will be removed in a future version
+		- PeerstoreWriteInterval: is deprecated and will be removed in a future version`
 	)
 	tests := []struct {
 		name         string
@@ -1257,10 +1272,11 @@ func Test_generalConfig_LogConfiguration(t *testing.T) {
 		wantConfig    string
 		wantEffective string
 		wantSecrets   string
+		wantWarning   string
 	}{
 		{name: "empty", wantEffective: emptyEffectiveTOML, wantSecrets: emptyEffectiveSecretsTOML},
 		{name: "full", inputSecrets: secretsFullTOML, inputConfig: fullTOML,
-			wantConfig: fullTOML, wantEffective: fullTOML, wantSecrets: secretsFullRedactedTOML},
+			wantConfig: fullTOML, wantEffective: fullTOML, wantSecrets: secretsFullRedactedTOML, wantWarning: deprecated},
 		{name: "multi-chain", inputSecrets: secretsMultiTOML, inputConfig: multiChainTOML,
 			wantConfig: multiChainTOML, wantEffective: multiChainEffectiveTOML, wantSecrets: secretsMultiRedactedTOML},
 	}
@@ -1274,28 +1290,39 @@ func Test_generalConfig_LogConfiguration(t *testing.T) {
 			}
 			c, err := opts.New()
 			require.NoError(t, err)
-			c.LogConfiguration(lggr.Infof)
+			c.LogConfiguration(lggr.Infof, lggr.Warnf)
 
 			inputLogs := observed.FilterMessageSnippet(secrets).All()
 			if assert.Len(t, inputLogs, 1) {
+				assert.Equal(t, zapcore.InfoLevel, inputLogs[0].Level)
 				got := strings.TrimPrefix(inputLogs[0].Message, secrets)
 				got = strings.TrimSuffix(got, "\n")
 				assert.Equal(t, tt.wantSecrets, got)
 			}
 
 			inputLogs = observed.FilterMessageSnippet(input).All()
 			if assert.Len(t, inputLogs, 1) {
+				assert.Equal(t, zapcore.InfoLevel, inputLogs[0].Level)
 				got := strings.TrimPrefix(inputLogs[0].Message, input)
 				got = strings.TrimSuffix(got, "\n")
 				assert.Equal(t, tt.wantConfig, got)
 			}
 
 			inputLogs = observed.FilterMessageSnippet(effective).All()
 			if assert.Len(t, inputLogs, 1) {
+				assert.Equal(t, zapcore.InfoLevel, inputLogs[0].Level)
 				got := strings.TrimPrefix(inputLogs[0].Message, effective)
 				got = strings.TrimSuffix(got, "\n")
 				assert.Equal(t, tt.wantEffective, got)
 			}
+
+			inputLogs = observed.FilterMessageSnippet(warning).All()
+			if tt.wantWarning != "" && assert.Len(t, inputLogs, 1) {
+				assert.Equal(t, zapcore.WarnLevel, inputLogs[0].Level)
+				got := strings.TrimPrefix(inputLogs[0].Message, warning)
+				got = strings.TrimSuffix(got, "\n")
+				assert.Equal(t, tt.wantWarning, got)
+			}
 		})
 	}
 }