From cd27ae4b13327ef3c9f7af26424587a94dd4896d Mon Sep 17 00:00:00 2001 From: Edward Mack Date: Thu, 4 Nov 2021 11:20:57 -0400 Subject: [PATCH 1/2] feat(lib/babe): add check of types.ConfigData.SecondarySlots for disabling secondary verification (#1910) * add check for types.ConfigData.SecondarySlots * Update lib/babe/verify.go Co-authored-by: Quentin McGaw * update ok = false if !b.secondarySlots * stub test for secondary verification * add test to verify secondary digest * address PR comments * add err check * update pointers to fix tests * add tests for decoding * my wip * fix VRF encoding * update linter tags Co-authored-by: Quentin McGaw Co-authored-by: Tim Wu --- dot/types/babe.go | 2 +- lib/babe/verify.go | 53 ++++++++++++++++++++----------- lib/babe/verify_test.go | 69 ++++++++++++++++++++++++++++++++++++++--- 3 files changed, 101 insertions(+), 23 deletions(-) diff --git a/dot/types/babe.go b/dot/types/babe.go index 41403aab8d..b707c6b589 100644 --- a/dot/types/babe.go +++ b/dot/types/babe.go @@ -94,7 +94,7 @@ func (d *EpochDataRaw) ToEpochData() (*EpochData, error) { type ConfigData struct { C1 uint64 C2 uint64 - SecondarySlots byte // TODO: this is unused, will need to update BABE verifier to use this (#1863) + SecondarySlots byte } // GetSlotFromHeader returns the BABE slot from the given header diff --git a/lib/babe/verify.go b/lib/babe/verify.go index e60b5cf5eb..a7c4b759d8 100644 --- a/lib/babe/verify.go +++ b/lib/babe/verify.go @@ -31,9 +31,10 @@ import ( // verifierInfo contains the information needed to verify blocks // it remains the same for an epoch type verifierInfo struct { - authorities []types.Authority - randomness Randomness - threshold *common.Uint128 + authorities []types.Authority + randomness Randomness + threshold *common.Uint128 + secondarySlots bool } // onDisabledInfo contains information about an authority that's been disabled at a certain @@ -224,9 +225,10 @@ func (v *VerificationManager) getVerifierInfo(epoch uint64) (*verifierInfo, erro } return &verifierInfo{ - authorities: epochData.Authorities, - randomness: epochData.Randomness, - threshold: threshold, + authorities: epochData.Authorities, + randomness: epochData.Randomness, + threshold: threshold, + secondarySlots: configData.SecondarySlots > 0, }, nil } @@ -247,11 +249,12 @@ func (v *VerificationManager) getConfigData(epoch uint64) (*types.ConfigData, er // verifier is a BABE verifier for a specific authority set, randomness, and threshold type verifier struct { - blockState BlockState - epoch uint64 - authorities []types.Authority - randomness Randomness - threshold *common.Uint128 + blockState BlockState + epoch uint64 + authorities []types.Authority + randomness Randomness + threshold *common.Uint128 + secondarySlots bool } // newVerifier returns a Verifier for the epoch described by the given descriptor @@ -261,11 +264,12 @@ func newVerifier(blockState BlockState, epoch uint64, info *verifierInfo) (*veri } return &verifier{ - blockState: blockState, - epoch: epoch, - authorities: info.authorities, - randomness: info.randomness, - threshold: info.threshold, + blockState: blockState, + epoch: epoch, + authorities: info.authorities, + randomness: info.randomness, + threshold: info.threshold, + secondarySlots: info.secondarySlots, }, nil } @@ -409,15 +413,28 @@ func (b *verifier) verifyPreRuntimeDigest(digest *types.PreRuntimeDigest) (scale case types.BabePrimaryPreDigest: ok, err = b.verifyPrimarySlotWinner(d.AuthorityIndex, d.SlotNumber, d.VRFOutput, d.VRFProof) case types.BabeSecondaryVRFPreDigest: + if !b.secondarySlots { + ok = false + break + } pub := b.authorities[d.AuthorityIndex].Key - var pk *sr25519.PublicKey - pk, err = sr25519.NewPublicKey(pub.Encode()) + + pk, err := sr25519.NewPublicKey(pub.Encode()) if err != nil { return nil, err } ok, err = verifySecondarySlotVRF(&d, pk, b.epoch, len(b.authorities), b.randomness) + if err != nil { + return nil, err + } + case types.BabeSecondaryPlainPreDigest: + if !b.secondarySlots { + ok = false + break + } + ok = true err = verifySecondarySlotPlain(d.AuthorityIndex, d.SlotNumber, len(b.authorities), b.randomness) } diff --git a/lib/babe/verify_test.go b/lib/babe/verify_test.go index 83e065c348..3e1e49a167 100644 --- a/lib/babe/verify_test.go +++ b/lib/babe/verify_test.go @@ -19,18 +19,19 @@ package babe import ( "errors" "io/ioutil" + "math/big" "os" "testing" "time" - "github.com/ChainSafe/gossamer/lib/genesis" - "github.com/stretchr/testify/require" - "github.com/ChainSafe/gossamer/dot/state" "github.com/ChainSafe/gossamer/dot/types" + "github.com/ChainSafe/gossamer/lib/common" "github.com/ChainSafe/gossamer/lib/crypto/sr25519" - + "github.com/ChainSafe/gossamer/lib/genesis" + "github.com/ChainSafe/gossamer/pkg/scale" log "github.com/ChainSafe/log15" + "github.com/stretchr/testify/require" ) func newTestVerificationManager(t *testing.T, genCfg *types.BabeConfiguration) *VerificationManager { @@ -162,6 +163,66 @@ func TestVerificationManager_VerifyBlock_Ok(t *testing.T) { require.NoError(t, err) } +func TestVerificationManager_VerifyBlock_Secondary(t *testing.T) { + babeService := createTestService(t, nil) + rt, err := babeService.blockState.GetRuntime(nil) + require.NoError(t, err) + + cfg, err := rt.BabeConfiguration() + require.NoError(t, err) + + cfg.GenesisAuthorities = types.AuthoritiesToRaw(babeService.epochData.authorities) + cfg.C1 = 1 + cfg.C2 = 1 + cfg.SecondarySlots = 0 + + vm := newTestVerificationManager(t, cfg) + + kp, err := sr25519.GenerateKeypair() + require.NoError(t, err) + + dig := createSecondaryVRFPreDigest(t, kp, 0, uint64(0), uint64(0), Randomness{}) + + bd := types.NewBabeDigest() + err = bd.Set(dig) + require.NoError(t, err) + + bdEnc, err := scale.Marshal(bd) + require.NoError(t, err) + + // create pre-digest + preDigest := &types.PreRuntimeDigest{ + ConsensusEngineID: types.BabeEngineID, + Data: bdEnc, + } + + // create new block header + number := big.NewInt(1) + digest := types.NewDigest() + err = digest.Add(*preDigest) + require.NoError(t, err) + + // create seal and add to digest + seal := &types.SealDigest{ + ConsensusEngineID: types.BabeEngineID, + Data: []byte{0}, + } + require.NoError(t, err) + + err = digest.Add(*seal) + require.NoError(t, err) + + header, err := types.NewHeader(common.Hash{}, common.Hash{}, common.Hash{}, number, digest) + require.NoError(t, err) + + block := types.Block{ + Header: *header, + Body: nil, + } + err = vm.VerifyBlock(&block.Header) + require.EqualError(t, err, "failed to verify pre-runtime digest: could not verify slot claim VRF proof") +} + func TestVerificationManager_VerifyBlock_MultipleEpochs(t *testing.T) { babeService := createTestService(t, nil) rt, err := babeService.blockState.GetRuntime(nil) From dc89892b7983e62fec21cca8ef2933af39cd8eff Mon Sep 17 00:00:00 2001 From: Zach <44234405+zjhuber@users.noreply.github.com> Date: Thu, 4 Nov 2021 09:35:58 -0700 Subject: [PATCH 2/2] 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 --- chain/gssmr/config.toml | 1 + chain/gssmr/defaults.go | 3 +++ cmd/gossamer/config.go | 9 +++------ cmd/gossamer/config_test.go | 2 ++ cmd/gossamer/export.go | 1 + dot/config.go | 2 ++ dot/config/toml/config.go | 1 + dot/services.go | 1 + lib/grandpa/errors.go | 3 +++ lib/grandpa/grandpa.go | 19 ++++++++++++------- lib/grandpa/grandpa_test.go | 1 + lib/grandpa/round_test.go | 1 + lib/grandpa/vote_message_test.go | 10 ++++++++++ tests/utils/gossamer_utils.go | 2 ++ 14 files changed, 43 insertions(+), 13 deletions(-) diff --git a/chain/gssmr/config.toml b/chain/gssmr/config.toml index 8f15cf520a..c335b5003e 100644 --- a/chain/gssmr/config.toml +++ b/chain/gssmr/config.toml @@ -24,6 +24,7 @@ unlock = "" roles = 4 babe-authority = true grandpa-authority = true +grandpa-interval = 1 [network] port = 7001 diff --git a/chain/gssmr/defaults.go b/chain/gssmr/defaults.go index c929a3cd2f..f672ecf754 100644 --- a/chain/gssmr/defaults.go +++ b/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 diff --git a/cmd/gossamer/config.go b/cmd/gossamer/config.go index 5b12251d82..9b8bee0506 100644 --- a/cmd/gossamer/config.go +++ b/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 { diff --git a/cmd/gossamer/config_test.go b/cmd/gossamer/config_test.go index cd7ea10689..64ef0ee06c 100644 --- a/cmd/gossamer/config_test.go +++ b/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, }, }, } diff --git a/cmd/gossamer/export.go b/cmd/gossamer/export.go index 9724bd6c37..8097bfe4f3 100644 --- a/cmd/gossamer/export.go +++ b/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{ diff --git a/dot/config.go b/dot/config.go index 2757eaac5c..07c5258e52 100644 --- a/dot/config.go +++ b/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, diff --git a/dot/config/toml/config.go b/dot/config/toml/config.go index ed4656ec61..b6c090fa2b 100644 --- a/dot/config/toml/config.go +++ b/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"` } diff --git a/dot/services.go b/dot/services.go index 377d87200b..423c00807c 100644 --- a/dot/services.go +++ b/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 { diff --git a/lib/grandpa/errors.go b/lib/grandpa/errors.go index 1c0eb36f42..505b6e8556 100644 --- a/lib/grandpa/errors.go +++ b/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") diff --git a/lib/grandpa/grandpa.go b/lib/grandpa/grandpa.go index 5ba38b1fa8..cb4122e9f2 100644 --- a/lib/grandpa/grandpa.go +++ b/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 { diff --git a/lib/grandpa/grandpa_test.go b/lib/grandpa/grandpa_test.go index b854b12f0a..9c24bb95ac 100644 --- a/lib/grandpa/grandpa_test.go +++ b/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) diff --git a/lib/grandpa/round_test.go b/lib/grandpa/round_test.go index 6e712731f4..eab925eb44 100644 --- a/lib/grandpa/round_test.go +++ b/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) diff --git a/lib/grandpa/vote_message_test.go b/lib/grandpa/vote_message_test.go index 69cb2f5b16..e1f279a7fb 100644 --- a/lib/grandpa/vote_message_test.go +++ b/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) diff --git a/tests/utils/gossamer_utils.go b/tests/utils/gossamer_utils.go index 44adcb1b19..9f653b45aa 100644 --- a/tests/utils/gossamer_utils.go +++ b/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 }