node: Add a command-line flag to enable or disable flow-canceling on …

…restart Added a command-line flag to enable or disable flow-canceling when starting the node. This should allow Guardians to disable flow canceling in the case of future bugs or during a security incident. This should prevent the need to rollback to earlier Guardian versions. (@mdulin2 )
wormhole-foundation · Jul 10, 2024 · ecdb4b0 · ecdb4b0
1 parent 3f75467
commit ecdb4b0
Show file tree

Hide file tree

Showing 7 changed files with 28 additions and 11 deletions.
diff --git a/node/cmd/guardiand/node.go b/node/cmd/guardiand/node.go
@@ -229,7 +229,8 @@ var (
 	// Prometheus remote write URL
 	promRemoteURL *string
 
-	chainGovernorEnabled *bool
+	chainGovernorEnabled      *bool
+	governorFlowCancelEnabled *bool
 
 	ccqEnabled           *bool
 	ccqAllowedRequesters *string
@@ -431,6 +432,7 @@ func init() {
 	promRemoteURL = NodeCmd.Flags().String("promRemoteURL", "", "Prometheus remote write URL (Grafana)")
 
 	chainGovernorEnabled = NodeCmd.Flags().Bool("chainGovernorEnabled", false, "Run the chain governor")
+	governorFlowCancelEnabled = NodeCmd.Flags().Bool("governorFlowCancelEnabled", false, "Enable flow cancel on the governor")
 
 	ccqEnabled = NodeCmd.Flags().Bool("ccqEnabled", false, "Enable cross chain query support")
 	ccqAllowedRequesters = NodeCmd.Flags().String("ccqAllowedRequesters", "", "Comma separated list of signers allowed to submit cross chain queries")
@@ -537,6 +539,11 @@ func runNode(cmd *cobra.Command, args []string) {
 		os.Exit(1)
 	}
 
+	if !(*chainGovernorEnabled) && *governorFlowCancelEnabled {
+		fmt.Println("Flow cancel can only be enabled when the governor is enabled")
+		os.Exit(1)
+	}
+
 	logger := zap.New(zapcore.NewCore(
 		consoleEncoder{zapcore.NewConsoleEncoder(
 			zap.NewDevelopmentEncoderConfig())},
@@ -1559,7 +1566,7 @@ func runNode(cmd *cobra.Command, args []string) {
 		node.GuardianOptionDatabase(db),
 		node.GuardianOptionWatchers(watcherConfigs, ibcWatcherConfig),
 		node.GuardianOptionAccountant(*accountantWS, *accountantContract, *accountantCheckEnabled, accountantWormchainConn, *accountantNttContract, accountantNttWormchainConn),
-		node.GuardianOptionGovernor(*chainGovernorEnabled),
+		node.GuardianOptionGovernor(*chainGovernorEnabled, *governorFlowCancelEnabled),
 		node.GuardianOptionGatewayRelayer(*gatewayRelayerContract, gatewayRelayerWormchainConn),
 		node.GuardianOptionQueryHandler(*ccqEnabled, *ccqAllowedRequesters),
 		node.GuardianOptionAdminService(*adminSocketPath, ethRPC, ethContract, rpcMap),

diff --git a/node/pkg/governor/governor.go b/node/pkg/governor/governor.go
@@ -194,12 +194,14 @@ type ChainGovernor struct {
 	nextConfigPublishTime time.Time
 	statusPublishCounter  int64
 	configPublishCounter  int64
+	flowCancelEnabled     bool
 }
 
 func NewChainGovernor(
 	logger *zap.Logger,
 	db db.GovernorDB,
 	env common.Environment,
+	flowCancelEnabled bool,
 ) *ChainGovernor {
 	return &ChainGovernor{
 		db:                  db,
@@ -209,6 +211,7 @@ func NewChainGovernor(
 		chains:              make(map[vaa.ChainID]*chainEntry),
 		msgsSeen:            make(map[string]bool),
 		env:                 env,
+		flowCancelEnabled:   flowCancelEnabled,
 	}
 }
 
@@ -247,6 +250,9 @@ func (gov *ChainGovernor) initConfig() error {
 		configTokens, flowCancelTokens, configChains = gov.initTestnetConfig()
 	}
 
+	if !gov.flowCancelEnabled { // If flow cancel is disabled, then use an empty set of tokens. Easier to put here than have 5 checks in the various sections of code that use it.
+		flowCancelTokens = []tokenConfigEntry{}
+	}
 	for _, ct := range configTokens {
 		addr, err := vaa.StringToAddress(ct.addr)
 		if err != nil {

diff --git a/node/pkg/governor/governor_monitoring_test.go b/node/pkg/governor/governor_monitoring_test.go
@@ -11,7 +11,7 @@ import (
 
 func TestIsVAAEnqueuedNilMessageID(t *testing.T) {
 	logger, _ := zap.NewProduction()
-	gov := NewChainGovernor(logger, nil, common.GoTest)
+	gov := NewChainGovernor(logger, nil, common.GoTest, false)
 	enqueued, err := gov.IsVAAEnqueued(nil)
 	require.EqualError(t, err, "no message ID specified")
 	assert.Equal(t, false, enqueued)

diff --git a/node/pkg/governor/governor_prices.go b/node/pkg/governor/governor_prices.go
@@ -309,7 +309,7 @@ func CheckQuery(logger *zap.Logger) error {
 	logger.Info("Instantiating governor.")
 	ctx := context.Background()
 	var db db.MockGovernorDB
-	gov := NewChainGovernor(logger, &db, common.MainNet)
+	gov := NewChainGovernor(logger, &db, common.MainNet, false)
 
 	if err := gov.initConfig(); err != nil {
 		return err

diff --git a/node/pkg/governor/governor_test.go b/node/pkg/governor/governor_test.go
@@ -545,7 +545,7 @@ func newChainGovernorForTestWithLogger(ctx context.Context, logger *zap.Logger)
 	}
 
 	var db db.MockGovernorDB
-	gov := NewChainGovernor(logger, &db, common.GoTest)
+	gov := NewChainGovernor(logger, &db, common.GoTest, true)
 
 	err := gov.Run(ctx)
 	if err != nil {
@@ -1775,7 +1775,7 @@ func TestSmallerPendingTransfersAfterBigOneShouldGetReleased(t *testing.T) {
 func TestMainnetConfigIsValid(t *testing.T) {
 	logger := zap.NewNop()
 	var db db.MockGovernorDB
-	gov := NewChainGovernor(logger, &db, common.GoTest)
+	gov := NewChainGovernor(logger, &db, common.GoTest, true)
 
 	gov.env = common.TestNet
 	err := gov.initConfig()
@@ -1785,7 +1785,7 @@ func TestMainnetConfigIsValid(t *testing.T) {
 func TestTestnetConfigIsValid(t *testing.T) {
 	logger := zap.NewNop()
 	var db db.MockGovernorDB
-	gov := NewChainGovernor(logger, &db, common.GoTest)
+	gov := NewChainGovernor(logger, &db, common.GoTest, true)
 
 	gov.env = common.TestNet
 	err := gov.initConfig()

diff --git a/node/pkg/node/options.go b/node/pkg/node/options.go
@@ -189,14 +189,18 @@ func GuardianOptionAccountant(
 
 // GuardianOptionGovernor enables or disables the governor.
 // Dependencies: db
-func GuardianOptionGovernor(governorEnabled bool) *GuardianOption {
+func GuardianOptionGovernor(governorEnabled bool, flowCancelEnabled bool) *GuardianOption {
 	return &GuardianOption{
 		name:         "governor",
 		dependencies: []string{"db"},
 		f: func(ctx context.Context, logger *zap.Logger, g *G) error {
 			if governorEnabled {
-				logger.Info("chain governor is enabled")
-				g.gov = governor.NewChainGovernor(logger, g.db, g.env)
+				if flowCancelEnabled {
+					logger.Info("chain governor is enabled with flow cancel enabled")
+				} else {
+					logger.Info("chain governor is enabled without flow cancel")
+				}
+				g.gov = governor.NewChainGovernor(logger, g.db, g.env, flowCancelEnabled)
 			} else {
 				logger.Info("chain governor is disabled")
 			}

diff --git a/node/pkg/publicrpc/publicrpcserver_test.go b/node/pkg/publicrpc/publicrpcserver_test.go
@@ -69,7 +69,7 @@ func TestGetSignedVAABadAddress(t *testing.T) {
 func TestGovernorIsVAAEnqueuedNoMessage(t *testing.T) {
 	ctx := context.Background()
 	logger, _ := zap.NewProduction()
-	gov := governor.NewChainGovernor(logger, nil, common.GoTest)
+	gov := governor.NewChainGovernor(logger, nil, common.GoTest, false)
 	server := &PublicrpcServer{logger: logger, gov: gov}
 
 	// A message without the messageId set should not panic but return an error instead.