Main patch tm-v0.34.11 (#359)

* statesync: improve e2e test outcomes (backport #6378) (#6380) (cherry picked from commit d36a5905a67db1ed7afb09f371b3ea3910afb6eb) Co-authored-by: Sam Kleinman <[email protected]> * Fix codecov: `statesync: improve e2e test outcomes (backport #6378) (#6380)` * evidence: fix bug with hashes (backport #6375) (#6381) * Fix codecov: `evidence: fix bug with hashes (backport #6375) (#6381)` * libs/os: avoid CopyFile truncating destination before checking if regular file (backport: #6428) (#6436) Co-authored-by: Callum Waters <[email protected]> * Fix lint: `libs/os: avoid CopyFile truncating destination before checking if regular file (backport: #6428) (#6436)` * p2p/conn: check for channel id overflow before processing receive msg (backport #6522) (#6528) * p2p/conn: check for channel id overflow before processing receive msg (#6522) Per tendermint spec, each Channel has a globally unique byte id, which is mapped to uint8 in Go. However, the proto PacketMsg.ChannelID field is declared as int32, and when receive the packet, we cast it to a byte without checking for possible overflow. That leads to a malform packet with invalid channel id is sent successfully. To fix it, we just add a check for possible overflow, and return invalid channel id error. Fixed #6521 (cherry picked from commit 1f46a4c90e268def505037a5d42627942f605ef4) * state sync: tune request timeout and chunkers (backport #6566) (#6581) * state sync: tune request timeout and chunkers (#6566) (cherry picked from commit 7d961b55b2132d53ccf7ee8d6c86b84fc7fc9ddc) * fix build * fix config * fix config Co-authored-by: Aleksandr Bezobchuk <[email protected]> Co-authored-by: Aleksandr Bezobchuk <[email protected]> * statesync: increase chunk priority and robustness (#6582) Co-authored-by: Callum Waters <[email protected]> Co-authored-by: Sam Kleinman <[email protected]> Co-authored-by: Callum Waters <[email protected]> Co-authored-by: Aleksandr Bezobchuk <[email protected]> Co-authored-by: Aleksandr Bezobchuk <[email protected]>
Finschia · Jan 20, 2022 · 7320ceb · 7320ceb
1 parent fb1b65d
commit 7320ceb
Show file tree

Hide file tree

Showing 30 changed files with 1,008 additions and 461 deletions.
diff --git a/config/config.go b/config/config.go
@@ -791,13 +791,15 @@ func (cfg *MempoolConfig) ValidateBasic() error {
 
 // StateSyncConfig defines the configuration for the Ostracon state sync service
 type StateSyncConfig struct {
-	Enable        bool          `mapstructure:"enable"`
-	TempDir       string        `mapstructure:"temp_dir"`
-	RPCServers    []string      `mapstructure:"rpc_servers"`
-	TrustPeriod   time.Duration `mapstructure:"trust_period"`
-	TrustHeight   int64         `mapstructure:"trust_height"`
-	TrustHash     string        `mapstructure:"trust_hash"`
-	DiscoveryTime time.Duration `mapstructure:"discovery_time"`
+	Enable              bool          `mapstructure:"enable"`
+	TempDir             string        `mapstructure:"temp_dir"`
+	RPCServers          []string      `mapstructure:"rpc_servers"`
+	TrustPeriod         time.Duration `mapstructure:"trust_period"`
+	TrustHeight         int64         `mapstructure:"trust_height"`
+	TrustHash           string        `mapstructure:"trust_hash"`
+	DiscoveryTime       time.Duration `mapstructure:"discovery_time"`
+	ChunkRequestTimeout time.Duration `mapstructure:"chunk_request_timeout"`
+	ChunkFetchers       int32         `mapstructure:"chunk_fetchers"`
 }
 
 func (cfg *StateSyncConfig) TrustHashBytes() []byte {
@@ -812,8 +814,10 @@ func (cfg *StateSyncConfig) TrustHashBytes() []byte {
 // DefaultStateSyncConfig returns a default configuration for the state sync service
 func DefaultStateSyncConfig() *StateSyncConfig {
 	return &StateSyncConfig{
-		TrustPeriod:   168 * time.Hour,
-		DiscoveryTime: 15 * time.Second,
+		TrustPeriod:         168 * time.Hour,
+		DiscoveryTime:       15 * time.Second,
+		ChunkRequestTimeout: 10 * time.Second,
+		ChunkFetchers:       4,
 	}
 }
 
@@ -828,28 +832,47 @@ func (cfg *StateSyncConfig) ValidateBasic() error {
 		if len(cfg.RPCServers) == 0 {
 			return errors.New("rpc_servers is required")
 		}
+
 		if len(cfg.RPCServers) < 2 {
 			return errors.New("at least two rpc_servers entries is required")
 		}
+
 		for _, server := range cfg.RPCServers {
 			if len(server) == 0 {
 				return errors.New("found empty rpc_servers entry")
 			}
 		}
+
+		if cfg.DiscoveryTime != 0 && cfg.DiscoveryTime < 5*time.Second {
+			return errors.New("discovery time must be 0s or greater than five seconds")
+		}
+
 		if cfg.TrustPeriod <= 0 {
 			return errors.New("trusted_period is required")
 		}
+
 		if cfg.TrustHeight <= 0 {
 			return errors.New("trusted_height is required")
 		}
+
 		if len(cfg.TrustHash) == 0 {
 			return errors.New("trusted_hash is required")
 		}
+
 		_, err := hex.DecodeString(cfg.TrustHash)
 		if err != nil {
 			return fmt.Errorf("invalid trusted_hash: %w", err)
 		}
+
+		if cfg.ChunkRequestTimeout < 5*time.Second {
+			return errors.New("chunk_request_timeout must be at least 5 seconds")
+		}
+
+		if cfg.ChunkFetchers <= 0 {
+			return errors.New("chunk_fetchers is required")
+		}
 	}
+
 	return nil
 }
 

diff --git a/config/config_test.go b/config/config_test.go
@@ -126,6 +126,35 @@ func TestMempoolConfigValidateBasic(t *testing.T) {
 func TestStateSyncConfigValidateBasic(t *testing.T) {
 	cfg := TestStateSyncConfig()
 	require.NoError(t, cfg.ValidateBasic())
+
+	testVerify := func(expectedError string) {
+		actual := cfg.ValidateBasic()
+		require.Error(t, actual)
+		require.Equal(t, expectedError, actual.Error())
+	}
+
+	// Enabled
+	cfg.Enable = true
+	testVerify("rpc_servers is required")
+	cfg.RPCServers = []string{""}
+	testVerify("at least two rpc_servers entries is required")
+	cfg.RPCServers = []string{"", ""}
+	testVerify("found empty rpc_servers entry")
+	cfg.RPCServers = []string{"a", "b"}
+	cfg.DiscoveryTime = 1 * time.Second
+	testVerify("discovery time must be 0s or greater than five seconds")
+	cfg.DiscoveryTime = 0
+	cfg.TrustPeriod = 0
+	testVerify("trusted_period is required")
+	cfg.TrustPeriod = 1
+	testVerify("trusted_height is required")
+	cfg.TrustHeight = 1
+	testVerify("trusted_hash is required")
+	cfg.TrustHash = "0"
+	testVerify("invalid trusted_hash: encoding/hex: odd length hex string")
+	cfg.TrustHash = "00"
+	// Success with Enabled
+	require.NoError(t, cfg.ValidateBasic())
 }
 
 func TestFastSyncConfigValidateBasic(t *testing.T) {

diff --git a/config/toml.go b/config/toml.go
@@ -406,6 +406,13 @@ discovery_time = "{{ .StateSync.DiscoveryTime }}"
 # Will create a new, randomly named directory within, and remove it when done.
 temp_dir = "{{ .StateSync.TempDir }}"
 
+# The timeout duration before re-requesting a chunk, possibly from a different
+# peer (default: 1 minute).
+chunk_request_timeout = "{{ .StateSync.ChunkRequestTimeout }}"
+
+# The number of concurrent chunk fetchers to run (default: 1).
+chunk_fetchers = "{{ .StateSync.ChunkFetchers }}"
+
 #######################################################
 ###       Fast Sync Configuration Connections       ###
 #######################################################

diff --git a/evidence/pool.go b/evidence/pool.go
@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
-	"sort"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -194,9 +193,11 @@ func (evpool *Pool) CheckEvidence(evList types.EvidenceList) error {
 	hashes := make([][]byte, len(evList))
 	for idx, ev := range evList {
 
-		ok := evpool.fastCheck(ev)
+		_, isLightEv := ev.(*types.LightClientAttackEvidence)
 
-		if !ok {
+		// We must verify light client attack evidence regardless because there could be a
+		// different conflicting block with the same hash.
+		if isLightEv || !evpool.isPending(ev) {
 			// check that the evidence isn't already committed
 			if evpool.isCommitted(ev) {
 				return &types.ErrInvalidEvidence{Evidence: ev, Reason: errors.New("evidence was already committed")}
@@ -213,7 +214,7 @@ func (evpool *Pool) CheckEvidence(evList types.EvidenceList) error {
 				evpool.logger.Error("Can't add evidence to pending list", "err", err, "ev", ev)
 			}
 
-			evpool.logger.Info("Verified new evidence of byzantine behavior", "evidence", ev)
+			evpool.logger.Info("Check evidence: verified evidence of byzantine behavior", "evidence", ev)
 		}
 
 		// check for duplicate evidence. We cache hashes so we don't have to work them out again.
@@ -255,68 +256,6 @@ func (evpool *Pool) State() sm.State {
 	return evpool.state
 }
 
-//--------------------------------------------------------------------------
-
-// fastCheck leverages the fact that the evidence pool may have already verified the evidence to see if it can
-// quickly conclude that the evidence is already valid.
-func (evpool *Pool) fastCheck(ev types.Evidence) bool {
-	if lcae, ok := ev.(*types.LightClientAttackEvidence); ok {
-		key := keyPending(ev)
-		evBytes, err := evpool.evidenceStore.Get(key)
-		if evBytes == nil { // the evidence is not in the nodes pending list
-			return false
-		}
-		if err != nil {
-			evpool.logger.Error("Failed to load light client attack evidence", "err", err, "key(height/hash)", key)
-			return false
-		}
-		var trustedPb tmproto.LightClientAttackEvidence
-		err = trustedPb.Unmarshal(evBytes)
-		if err != nil {
-			evpool.logger.Error("Failed to convert light client attack evidence from bytes",
-				"err", err, "key(height/hash)", key)
-			return false
-		}
-		trustedEv, err := types.LightClientAttackEvidenceFromProto(&trustedPb)
-		if err != nil {
-			evpool.logger.Error("Failed to convert light client attack evidence from protobuf",
-				"err", err, "key(height/hash)", key)
-			return false
-		}
-		// ensure that all the byzantine validators that the evidence pool has match the byzantine validators
-		// in this evidence
-		if trustedEv.ByzantineValidators == nil && lcae.ByzantineValidators != nil {
-			return false
-		}
-
-		if len(trustedEv.ByzantineValidators) != len(lcae.ByzantineValidators) {
-			return false
-		}
-
-		byzValsCopy := make([]*types.Validator, len(lcae.ByzantineValidators))
-		for i, v := range lcae.ByzantineValidators {
-			byzValsCopy[i] = v.Copy()
-		}
-
-		// ensure that both validator arrays are in the same order
-		sort.Sort(types.ValidatorsByVotingPower(byzValsCopy))
-
-		for idx, val := range trustedEv.ByzantineValidators {
-			if !bytes.Equal(byzValsCopy[idx].Address, val.Address) {
-				return false
-			}
-			if byzValsCopy[idx].StakingPower != val.StakingPower {
-				return false
-			}
-		}
-
-		return true
-	}
-
-	// for all other evidence the evidence pool just checks if it is already in the pending db
-	return evpool.isPending(ev)
-}
-
 // IsExpired checks whether evidence or a polc is expired by checking whether a height and time is older
 // than set by the evidence consensus parameters
 func (evpool *Pool) isExpired(height int64, time time.Time) bool {

diff --git a/evidence/pool_test.go b/evidence/pool_test.go
@@ -198,6 +198,9 @@ func TestReportConflictingVotes(t *testing.T) {
 	// should be able to retrieve evidence from pool
 	evList, _ = pool.PendingEvidence(defaultEvidenceMaxBytes)
 	require.Equal(t, []types.Evidence{ev}, evList)
+
+	next = pool.EvidenceFront()
+	require.NotNil(t, next)
 }
 
 func TestEvidencePoolUpdate(t *testing.T) {
@@ -258,64 +261,32 @@ func TestVerifyDuplicatedEvidenceFails(t *testing.T) {
 
 // check that valid light client evidence is correctly validated and stored in
 // evidence pool
-func TestCheckEvidenceWithLightClientAttack(t *testing.T) {
+func TestLightClientAttackEvidenceLifecycle(t *testing.T) {
 	var (
-		nValidators          = 5
-		validatorPower int64 = 10
-		height         int64 = 10
+		height       int64 = 100
+		commonHeight int64 = 90
 	)
-	conflictingVals, conflictingVoters, conflictingPrivVals := types.RandVoterSet(nValidators, validatorPower)
-	trustedHeader := makeHeaderRandom(height)
-	trustedHeader.Time = defaultEvidenceTime
-
-	conflictingHeader := makeHeaderRandom(height)
-	conflictingHeader.VotersHash = conflictingVoters.Hash()
-
-	trustedHeader.VotersHash = conflictingHeader.VotersHash
-	trustedHeader.NextValidatorsHash = conflictingHeader.NextValidatorsHash
-	trustedHeader.ConsensusHash = conflictingHeader.ConsensusHash
-	trustedHeader.AppHash = conflictingHeader.AppHash
-	trustedHeader.LastResultsHash = conflictingHeader.LastResultsHash
-
-	// for simplicity we are simulating a duplicate vote attack where all the validators in the
-	// conflictingVals set voted twice
-	blockID := makeBlockID(conflictingHeader.Hash(), 1000, []byte("partshash"))
-	voteSet := types.NewVoteSet(evidenceChainID, height, 1, tmproto.SignedMsgType(2), conflictingVoters)
-	commit, err := types.MakeCommit(blockID, height, 1, voteSet, conflictingPrivVals, defaultEvidenceTime)
-	require.NoError(t, err)
-	ev := &types.LightClientAttackEvidence{
-		ConflictingBlock: &types.LightBlock{
-			SignedHeader: &types.SignedHeader{
-				Header: conflictingHeader,
-				Commit: commit,
-			},
-			ValidatorSet: conflictingVals,
-			VoterSet:     conflictingVoters,
-		},
-		CommonHeight:        height,
-		TotalVotingPower:    int64(nValidators) * validatorPower,
-		ByzantineValidators: conflictingVals.Validators,
-		Timestamp:           defaultEvidenceTime,
-	}
-
-	trustedBlockID := makeBlockID(trustedHeader.Hash(), 1000, []byte("partshash"))
-	trustedVoteSet := types.NewVoteSet(evidenceChainID, height, 1, tmproto.SignedMsgType(2), conflictingVoters)
-	trustedCommit, err := types.MakeCommit(trustedBlockID, height, 1, trustedVoteSet, conflictingPrivVals,
-		defaultEvidenceTime)
-	require.NoError(t, err)
+	ev, trusted, common := makeLunaticEvidence(t, height, commonHeight,
+		10, 5, 5, defaultEvidenceTime, defaultEvidenceTime.Add(1*time.Hour))
 
 	state := sm.State{
-		LastBlockTime:   defaultEvidenceTime.Add(1 * time.Minute),
-		LastBlockHeight: 11,
+		LastBlockTime:   defaultEvidenceTime.Add(2 * time.Hour),
+		LastBlockHeight: 110,
 		ConsensusParams: *types.DefaultConsensusParams(),
 	}
 	stateStore := &smmocks.Store{}
-	stateStore.On("LoadValidators", height).Return(conflictingVals, nil)
-	stateStore.On("LoadVoters", height, mock.AnythingOfType("*types.VoterParams")).Return(conflictingVoters, nil)
+	stateStore.On("LoadValidators", height).Return(trusted.ValidatorSet, nil)
+	stateStore.On("LoadValidators", commonHeight).Return(common.ValidatorSet, nil)
+	stateStore.On("LoadVoters", height, mock.AnythingOfType("*types.VoterParams")).Return(
+		ev.ConflictingBlock.VoterSet, nil) // Should use correct VoterSet for bls.VerifyAggregatedSignature
+	stateStore.On("LoadVoters", commonHeight, mock.AnythingOfType("*types.VoterParams")).Return(
+		ev.ConflictingBlock.VoterSet, nil) // Should use correct VoterSet for bls.VerifyAggregatedSignature
 	stateStore.On("Load").Return(state, nil)
 	blockStore := &mocks.BlockStore{}
-	blockStore.On("LoadBlockMeta", height).Return(&types.BlockMeta{Header: *trustedHeader})
-	blockStore.On("LoadBlockCommit", height).Return(trustedCommit)
+	blockStore.On("LoadBlockMeta", height).Return(&types.BlockMeta{Header: *trusted.Header})
+	blockStore.On("LoadBlockMeta", commonHeight).Return(&types.BlockMeta{Header: *common.Header})
+	blockStore.On("LoadBlockCommit", height).Return(trusted.Commit)
+	blockStore.On("LoadBlockCommit", commonHeight).Return(common.Commit)
 
 	pool, err := evidence.NewPool(memdb.NewDB(), stateStore, blockStore)
 	require.NoError(t, err)
@@ -324,8 +295,32 @@ func TestCheckEvidenceWithLightClientAttack(t *testing.T) {
 	err = pool.AddEvidence(ev)
 	assert.NoError(t, err)
 
-	err = pool.CheckEvidence(types.EvidenceList{ev})
-	assert.NoError(t, err)
+	hash := ev.Hash()
+
+	require.NoError(t, pool.AddEvidence(ev))
+	require.NoError(t, pool.AddEvidence(ev))
+
+	pendingEv, _ := pool.PendingEvidence(state.ConsensusParams.Evidence.MaxBytes)
+	require.Equal(t, 1, len(pendingEv))
+	require.Equal(t, ev, pendingEv[0])
+
+	require.NoError(t, pool.CheckEvidence(pendingEv))
+	require.Equal(t, ev, pendingEv[0])
+
+	state.LastBlockHeight++
+	state.LastBlockTime = state.LastBlockTime.Add(1 * time.Minute)
+	pool.Update(state, pendingEv)
+	require.Equal(t, hash, pendingEv[0].Hash())
+
+	remaindingEv, _ := pool.PendingEvidence(state.ConsensusParams.Evidence.MaxBytes)
+	require.Empty(t, remaindingEv)
+
+	// evidence is already committed so it shouldn't pass
+	require.Error(t, pool.CheckEvidence(types.EvidenceList{ev}))
+	require.NoError(t, pool.AddEvidence(ev))
+
+	remaindingEv, _ = pool.PendingEvidence(state.ConsensusParams.Evidence.MaxBytes)
+	require.Empty(t, remaindingEv)
 }
 
 // Tests that restarting the evidence pool after a potential failure will recover the
@@ -365,7 +360,7 @@ func TestRecoverPendingEvidence(t *testing.T) {
 			Evidence: tmproto.EvidenceParams{
 				MaxAgeNumBlocks: 20,
 				MaxAgeDuration:  20 * time.Minute,
-				MaxBytes:        1000,
+				MaxBytes:        defaultEvidenceMaxBytes,
 			},
 		},
 	}, nil)