diff --git a/internal/blocksync/pool.go b/internal/blocksync/pool.go index c25b5787f..cf9a92109 100644 --- a/internal/blocksync/pool.go +++ b/internal/blocksync/pool.go @@ -238,7 +238,7 @@ func (pool *BlockPool) PopRequest() { // to smooth the block sync rate and the unit represents the number of blocks per second. // -1 because the start height is assumed to be 1 @jmalicevic ToDo, verify it is still OK when // starting height is not 1 - if (pool.height-pool.startHeight-1)%100 == 0 && pool.height != pool.startHeight { + if (pool.height-pool.startHeight+1)%100 == 0 { newSyncRate := 100 / time.Since(pool.lastHundredBlockTimeStamp).Seconds() if pool.lastSyncRate == 0 { pool.lastSyncRate = newSyncRate diff --git a/internal/blocksync/reactor.go b/internal/blocksync/reactor.go index ac0961971..f09422996 100644 --- a/internal/blocksync/reactor.go +++ b/internal/blocksync/reactor.go @@ -110,16 +110,17 @@ func NewReactor( eventBus *eventbus.EventBus, ) *Reactor { r := &Reactor{ - logger: logger, - stateStore: stateStore, - blockExec: blockExec, - store: *store, - consReactor: consReactor, - blockSync: newAtomicBool(blockSync), - chCreator: channelCreator, - peerEvents: peerEvents, - metrics: metrics, - eventBus: eventBus, + logger: logger, + stateStore: stateStore, + blockExec: blockExec, + store: *store, + consReactor: consReactor, + blockSync: newAtomicBool(blockSync), + chCreator: channelCreator, + peerEvents: peerEvents, + metrics: metrics, + eventBus: eventBus, + lastTrustedBlock: &TrustedBlockData{}, } r.BaseService = *service.NewBaseService(logger, "BlockSync", r) @@ -545,10 +546,10 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh ) // TODO(sergio, jmalicevic): Should we also validate against the extended commit? - if r.lastTrustedBlock == nil || r.lastTrustedBlock.block == nil { - if r.initialState.LastBlockHeight != 0 { - r.lastTrustedBlock = &TrustedBlockData{r.store.LoadBlock(r.initialState.LastBlockHeight), r.store.LoadSeenCommit()} - if r.lastTrustedBlock == nil { + if r.lastTrustedBlock.block == nil { + if state.LastBlockHeight != 0 { + r.lastTrustedBlock = &TrustedBlockData{r.store.LoadBlock(state.LastBlockHeight), r.store.LoadSeenCommit()} + if r.lastTrustedBlock.block == nil { panic("Failed to load last trusted block") } } @@ -556,7 +557,7 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh var err error // Check for nil on block to avoid the case where we failed the block validation before persistence // but passed the verification before and created a lastTrustedBlock object - if r.lastTrustedBlock != nil && r.lastTrustedBlock.block != nil { + if r.lastTrustedBlock.block != nil { err = VerifyNextBlock(newBlock, newBlockID, verifyBlock, r.lastTrustedBlock.block, r.lastTrustedBlock.commit, state.Validators) } else { oldHash := r.initialState.Validators.Hash() @@ -566,7 +567,7 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh "new hash ", newBlock.ValidatorsHash, ) - peerID := r.pool.RedoRequest(r.lastTrustedBlock.block.Height + 1) + peerID := r.pool.RedoRequest(state.LastBlockHeight) if serr := blockSyncCh.SendError(ctx, p2p.PeerError{ NodeID: peerID, Err: ErrValidationFailed{}, @@ -580,11 +581,11 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh // If we ran state sync before blocksync, we can trust the state we have in the store and we will not end up here // If we ran block sync or consensus, we will be able to load the last block from our store // We use the validator set provided in the gensis vile, to verify the new block, using the lastCommit of verifyBlock - if r.initialState.LastBlockHeight != 0 { + if state.LastBlockHeight != 0 { panic("Cannot be here unles we start from genesis") } err = state.Validators.VerifyCommitLight(newBlock.ChainID, newBlockID, newBlock.Height, verifyBlock.LastCommit) - r.lastTrustedBlock = &TrustedBlockData{} + } if err != nil { diff --git a/internal/blocksync/reactor_test.go b/internal/blocksync/reactor_test.go index 560d04661..1ec601a21 100644 --- a/internal/blocksync/reactor_test.go +++ b/internal/blocksync/reactor_test.go @@ -424,9 +424,11 @@ func TestReactor_NonGenesisSync(t *testing.T) { valSet, privVals := factory.ValidatorSet(ctx, t, 4, 30) genDoc := factory.GenesisDoc(cfg, time.Now(), valSet.Validators, factory.ConsensusParams()) - maxBlockHeight := int64(101) + // Has to be 100 + the max starting height because we calculate the sync rate based on every 100 blocks processed + // Otherwise the test will time out + maxBlockHeight := int64(151) - rts := setup(ctx, t, genDoc, privVals, []int64{maxBlockHeight, 2}) // 2, 0}) //50, 4, 0}) + rts := setup(ctx, t, genDoc, privVals, []int64{maxBlockHeight, 2, 50, 0}) require.Equal(t, maxBlockHeight, rts.reactors[rts.nodes[0]].store.Height()) rts.start(ctx, t) @@ -439,7 +441,7 @@ func TestReactor_NonGenesisSync(t *testing.T) { continue } matching = matching && rts.reactors[rts.nodes[idx]].GetRemainingSyncTime() > time.Nanosecond && - rts.reactors[rts.nodes[idx]].pool.getLastSyncRate() > 0.001 + rts.reactors[rts.nodes[idx]].pool.getLastSyncRate() > 0.0001 if !matching { height, _, _ := rts.reactors[rts.nodes[idx]].pool.GetStatus()