Skip to content

Commit

Permalink
node: Add a command-line flag to enable or disable flow-canceling on …
Browse files Browse the repository at this point in the history
…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 )
  • Loading branch information
Maxwell Dulin authored and johnsaigle committed Jul 22, 2024
1 parent 295e55f commit 9120d17
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 11 deletions.
11 changes: 9 additions & 2 deletions node/cmd/guardiand/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,8 @@ var (
// Prometheus remote write URL
promRemoteURL *string

chainGovernorEnabled *bool
chainGovernorEnabled *bool
governorFlowCancelEnabled *bool

ccqEnabled *bool
ccqAllowedRequesters *string
Expand Down Expand Up @@ -429,6 +430,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")
Expand Down Expand Up @@ -535,6 +537,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())},
Expand Down Expand Up @@ -1555,7 +1562,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),
Expand Down
6 changes: 6 additions & 0 deletions node/pkg/governor/governor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -209,6 +211,7 @@ func NewChainGovernor(
chains: make(map[vaa.ChainID]*chainEntry),
msgsSeen: make(map[string]bool),
env: env,
flowCancelEnabled: flowCancelEnabled,
}
}

Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion node/pkg/governor/governor_monitoring_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion node/pkg/governor/governor_prices.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions node/pkg/governor/governor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down
10 changes: 7 additions & 3 deletions node/pkg/node/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,14 +195,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")
}
Expand Down
2 changes: 1 addition & 1 deletion node/pkg/publicrpc/publicrpcserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 9120d17

Please sign in to comment.