chore(lib/grandpa): make interval configurable (#1903)

* chore(lib/grandpa): make interval configurable

* Resolved teams concerns

* Fixed integration test

Co-authored-by: noot <36753753+noot@users.noreply.github.com>
Co-authored-by: Quentin McGaw <quentin.mcgaw@gmail.com>

chain/gssmr/config.toml

@@ -24,6 +24,7 @@ unlock = ""
 roles = 4
 babe-authority = true
 grandpa-authority = true
+grandpa-interval = 1
 
 [network]
 port = 7001

chain/gssmr/defaults.go

@@ -91,6 +91,9 @@ var (
 	// DefaultDiscoveryInterval is the default interval for searching for DHT peers
 	DefaultDiscoveryInterval = time.Second * 10
 
+	// DefaultGrandpaInterval is the default time for a grandpa sub-round
+	DefaultGrandpaInterval = time.Second
+
 	// RPCConfig
 
 	// DefaultRPCHTTPHost rpc host

cmd/gossamer/config.go

@@ -579,6 +579,7 @@ func setDotCoreConfig(ctx *cli.Context, tomlCfg ctoml.CoreConfig, cfg *dot.CoreC
 	cfg.Roles = tomlCfg.Roles
 	cfg.BabeAuthority = tomlCfg.Roles == types.AuthorityRole
 	cfg.GrandpaAuthority = tomlCfg.Roles == types.AuthorityRole
+	cfg.GrandpaInterval = time.Second * time.Duration(tomlCfg.GrandpaInterval)
 	cfg.BABELead = ctx.GlobalBool(BABELeadFlag.Name)
 
 	// check --roles flag and update node configuration
@@ -633,6 +634,7 @@ func setDotCoreConfig(ctx *cli.Context, tomlCfg ctoml.CoreConfig, cfg *dot.CoreC
 		"babe-authority", cfg.BabeAuthority,
 		"grandpa-authority", cfg.GrandpaAuthority,
 		"wasm-interpreter", cfg.WasmInterpreter,
+		"grandpa-interval", cfg.GrandpaInterval,
 	)
 }
 
@@ -646,12 +648,7 @@ func setDotNetworkConfig(ctx *cli.Context, tomlCfg ctoml.NetworkConfig, cfg *dot
 	cfg.MinPeers = tomlCfg.MinPeers
 	cfg.MaxPeers = tomlCfg.MaxPeers
 	cfg.PersistentPeers = tomlCfg.PersistentPeers
-
-	if tomlCfg.DiscoveryInterval > 0 {
-		cfg.DiscoveryInterval = time.Second * time.Duration(tomlCfg.DiscoveryInterval)
-	} else {
-		cfg.DiscoveryInterval = 0
-	}
+	cfg.DiscoveryInterval = time.Second * time.Duration(tomlCfg.DiscoveryInterval)
 
 	// check --port flag and update node configuration
 	if port := ctx.GlobalUint(PortFlag.Name); port != 0 {

cmd/gossamer/config_test.go

@@ -409,6 +409,7 @@ func TestCoreConfigFromFlags(t *testing.T) {
 				BabeAuthority:    true,
 				GrandpaAuthority: true,
 				WasmInterpreter:  gssmr.DefaultWasmInterpreter,
+				GrandpaInterval:  testCfg.Core.GrandpaInterval,
 			},
 		},
 		{
@@ -420,6 +421,7 @@ func TestCoreConfigFromFlags(t *testing.T) {
 				BabeAuthority:    false,
 				GrandpaAuthority: false,
 				WasmInterpreter:  gssmr.DefaultWasmInterpreter,
+				GrandpaInterval:  testCfg.Core.GrandpaInterval,
 			},
 		},
 	}

cmd/gossamer/export.go

@@ -113,6 +113,7 @@ func dotConfigToToml(dcfg *dot.Config) *ctoml.Config {
 		Roles:            dcfg.Core.Roles,
 		BabeAuthority:    dcfg.Core.BabeAuthority,
 		GrandpaAuthority: dcfg.Core.GrandpaAuthority,
+		GrandpaInterval:  uint32(dcfg.Core.GrandpaInterval / time.Second),
 	}
 
 	cfg.Network = ctoml.NetworkConfig{

dot/config.go

@@ -103,6 +103,7 @@ type CoreConfig struct {
 	BABELead         bool
 	GrandpaAuthority bool
 	WasmInterpreter  string
+	GrandpaInterval  time.Duration
 }
 
 // RPCConfig is to marshal/unmarshal toml RPC config vars
@@ -180,6 +181,7 @@ func GssmrConfig() *Config {
 			BabeAuthority:    gssmr.DefaultBabeAuthority,
 			GrandpaAuthority: gssmr.DefaultGrandpaAuthority,
 			WasmInterpreter:  gssmr.DefaultWasmInterpreter,
+			GrandpaInterval:  gssmr.DefaultGrandpaInterval,
 		},
 		Network: NetworkConfig{
 			Port:              gssmr.DefaultNetworkPort,

dot/config/toml/config.go

@@ -82,6 +82,7 @@ type CoreConfig struct {
 	SlotDuration     uint64 `toml:"slot-duration,omitempty"`
 	EpochLength      uint64 `toml:"epoch-length,omitempty"`
 	WasmInterpreter  string `toml:"wasm-interpreter,omitempty"`
+	GrandpaInterval  uint32 `toml:"grandpa-interval,omitempty"`
 	BABELead         bool   `toml:"babe-lead,omitempty"`
 }
 

dot/services.go

@@ -385,6 +385,7 @@ func createGRANDPAService(cfg *Config, st *state.Service, dh *digest.Handler, ks
 		Voters:        voters,
 		Authority:     cfg.Core.GrandpaAuthority,
 		Network:       net,
+		Interval:      cfg.Core.GrandpaInterval,
 	}
 
 	if cfg.Core.GrandpaAuthority {

lib/grandpa/errors.go

@@ -101,6 +101,9 @@ var (
 	// ErrAuthorityNotInSet is returned when a precommit within a justification is signed by a key not in the authority set
 	ErrAuthorityNotInSet = errors.New("authority is not in set")
 
+	// ErrZeroInterval is returned when the grandpa sub-round interval is set to 0
+	ErrZeroInterval = errors.New("cannot have zero second interval")
+
 	errVoteExists              = errors.New("already have vote")
 	errVoteToSignatureMismatch = errors.New("votes and authority count mismatch")
 	errInvalidVoteBlock        = errors.New("block in vote is not descendant of previously finalised block")

lib/grandpa/grandpa.go

@@ -41,9 +41,7 @@ const (
 )
 
 var (
-	// TODO: make this configurable; currently 1s is same as substrate; total round length is interval * 2 (#1869)
-	interval = time.Second
-	logger   = log.New("pkg", "grandpa")
+	logger = log.New("pkg", "grandpa")
 )
 
 // Service represents the current state of the grandpa protocol
@@ -63,6 +61,7 @@ type Service struct {
 	resumed        chan struct{} // this channel will be closed when the service resumes
 	messageHandler *MessageHandler
 	network        Network
+	interval       time.Duration
 
 	// current state information
 	state           *State                                   // current state
@@ -93,6 +92,7 @@ type Config struct {
 	Voters        []Voter
 	Keypair       *ed25519.Keypair
 	Authority     bool
+	Interval      time.Duration
 }
 
 // NewService returns a new GRANDPA Service instance.
@@ -146,6 +146,10 @@ func NewService(cfg *Config) (*Service, error) {
 		return nil, err
 	}
 
+	if cfg.Interval == 0 {
+		return nil, ErrZeroInterval
+	}
+
 	ctx, cancel := context.WithCancel(context.Background())
 	s := &Service{
 		ctx:                ctx,
@@ -167,6 +171,7 @@ func NewService(cfg *Config) (*Service, error) {
 		resumed:            make(chan struct{}),
 		network:            cfg.Network,
 		finalisedCh:        finalisedCh,
+		interval:           cfg.Interval,
 	}
 
 	s.messageHandler = NewMessageHandler(s, s.blockState)
@@ -452,7 +457,7 @@ func (s *Service) playGrandpaRound() error {
 
 	logger.Debug("receiving pre-vote messages...")
 	go s.receiveMessages(ctx)
-	time.Sleep(interval)
+	time.Sleep(s.interval)
 
 	if s.paused.Load().(bool) {
 		return ErrServicePaused
@@ -481,7 +486,7 @@ func (s *Service) playGrandpaRound() error {
 	go s.sendVoteMessage(prevote, vm, roundComplete)
 
 	logger.Debug("receiving pre-commit messages...")
-	time.Sleep(interval)
+	time.Sleep(s.interval)
 
 	if s.paused.Load().(bool) {
 		return ErrServicePaused
@@ -514,7 +519,7 @@ func (s *Service) playGrandpaRound() error {
 }
 
 func (s *Service) sendVoteMessage(stage Subround, msg *VoteMessage, roundComplete <-chan struct{}) {
-	ticker := time.NewTicker(interval * 4)
+	ticker := time.NewTicker(s.interval * 4)
 	defer ticker.Stop()
 
 	for {
@@ -537,7 +542,7 @@ func (s *Service) sendVoteMessage(stage Subround, msg *VoteMessage, roundComplet
 
 // attemptToFinalize loops until the round is finalisable
 func (s *Service) attemptToFinalize() error {
-	ticker := time.NewTicker(interval / 100)
+	ticker := time.NewTicker(s.interval / 100)
 
 	for {
 		select {

lib/grandpa/grandpa_test.go

@@ -117,6 +117,7 @@ func newTestService(t *testing.T) (*Service, *state.Service) {
 		Keypair:       kr.Alice().(*ed25519.Keypair),
 		Authority:     true,
 		Network:       net,
+		Interval:      time.Second,
 	}
 
 	gs, err := NewService(cfg)

lib/grandpa/round_test.go

@@ -111,6 +111,7 @@ func setupGrandpa(t *testing.T, kp *ed25519.Keypair) (*Service, chan *networkVot
 		LogLvl:        log.LvlInfo,
 		Authority:     true,
 		Network:       net,
+		Interval:      time.Second,
 	}
 
 	gs, err := NewService(cfg)

lib/grandpa/vote_message_test.go

@@ -19,6 +19,7 @@ package grandpa
 import (
 	"math/big"
 	"testing"
+	"time"
 
 	"github.com/ChainSafe/gossamer/dot/state"
 	"github.com/ChainSafe/gossamer/dot/types"
@@ -41,6 +42,7 @@ func TestCheckForEquivocation_NoEquivocation(t *testing.T) {
 		Voters:        voters,
 		Keypair:       kr.Bob().(*ed25519.Keypair),
 		Network:       net,
+		Interval:      time.Second,
 	}
 
 	gs, err := NewService(cfg)
@@ -75,6 +77,7 @@ func TestCheckForEquivocation_WithEquivocation(t *testing.T) {
 		Voters:        voters,
 		Keypair:       kr.Bob().(*ed25519.Keypair),
 		Network:       net,
+		Interval:      time.Second,
 	}
 
 	gs, err := NewService(cfg)
@@ -121,6 +124,7 @@ func TestCheckForEquivocation_WithExistingEquivocation(t *testing.T) {
 		Voters:        voters,
 		Keypair:       kr.Bob().(*ed25519.Keypair),
 		Network:       net,
+		Interval:      time.Second,
 	}
 
 	gs, err := NewService(cfg)
@@ -177,6 +181,7 @@ func TestValidateMessage_Valid(t *testing.T) {
 		Voters:        voters,
 		Keypair:       kr.Bob().(*ed25519.Keypair),
 		Network:       net,
+		Interval:      time.Second,
 	}
 
 	gs, err := NewService(cfg)
@@ -210,6 +215,7 @@ func TestValidateMessage_InvalidSignature(t *testing.T) {
 		Voters:        voters,
 		Keypair:       kr.Bob().(*ed25519.Keypair),
 		Network:       net,
+		Interval:      time.Second,
 	}
 
 	gs, err := NewService(cfg)
@@ -243,6 +249,7 @@ func TestValidateMessage_SetIDMismatch(t *testing.T) {
 		DigestHandler: NewMockDigestHandler(),
 		Keypair:       kr.Bob().(*ed25519.Keypair),
 		Network:       net,
+		Interval:      time.Second,
 	}
 
 	gs, err := NewService(cfg)
@@ -277,6 +284,7 @@ func TestValidateMessage_Equivocation(t *testing.T) {
 		Voters:        voters,
 		Keypair:       kr.Bob().(*ed25519.Keypair),
 		Network:       net,
+		Interval:      time.Second,
 	}
 
 	gs, err := NewService(cfg)
@@ -321,6 +329,7 @@ func TestValidateMessage_BlockDoesNotExist(t *testing.T) {
 		Voters:        voters,
 		Keypair:       kr.Bob().(*ed25519.Keypair),
 		Network:       net,
+		Interval:      time.Second,
 	}
 
 	gs, err := NewService(cfg)
@@ -355,6 +364,7 @@ func TestValidateMessage_IsNotDescendant(t *testing.T) {
 		Voters:        voters,
 		Keypair:       kr.Bob().(*ed25519.Keypair),
 		Network:       net,
+		Interval:      time.Second,
 	}
 
 	gs, err := NewService(cfg)

tests/utils/gossamer_utils.go

@@ -445,6 +445,7 @@ func generateDefaultConfig() *ctoml.Config {
 			Roles:            4,
 			BabeAuthority:    true,
 			GrandpaAuthority: true,
+			GrandpaInterval:  1,
 		},
 		Network: ctoml.NetworkConfig{
 			Bootnodes:   nil,
@@ -511,6 +512,7 @@ func generateConfigNoGrandpa() *ctoml.Config {
 	cfg := generateDefaultConfig()
 	cfg.Core.GrandpaAuthority = false
 	cfg.Core.BABELead = true
+	cfg.Core.GrandpaInterval = 1
 	return cfg
 }