From 6f5978e906478b75f01102cd8a458d4c4600ea07 Mon Sep 17 00:00:00 2001 From: Jasmina Malicevic Date: Tue, 24 May 2022 17:13:05 +0200 Subject: [PATCH] blocksync: fixed bug if switching from statesync and no actual block in store --- internal/blocksync/reactor.go | 77 ++++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 32 deletions(-) diff --git a/internal/blocksync/reactor.go b/internal/blocksync/reactor.go index 36ba625ef..03eecdd77 100644 --- a/internal/blocksync/reactor.go +++ b/internal/blocksync/reactor.go @@ -373,7 +373,9 @@ func (r *Reactor) processPeerUpdates(ctx context.Context, peerUpdates *p2p.PeerU func (r *Reactor) SwitchToBlockSync(ctx context.Context, state sm.State) error { r.blockSync.Set() r.initialState = state - r.pool.height = state.LastBlockHeight + 1 + // We fetch the block at the same height whose header we have downloaded so that we can + // generate an initial trusted state + r.pool.height = state.LastBlockHeight // + 1 if err := r.pool.Start(ctx); err != nil { return err @@ -577,29 +579,11 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh ) // TODO(sergio, jmalicevic): Should we also validate against the extended commit? - if r.lastTrustedBlock.block == nil { - if state.LastBlockHeight != 0 { - seenCommit := r.store.LoadSeenCommit() - - if seenCommit.Height != state.LastBlockHeight || !seenCommit.BlockID.Equals(state.LastBlockID) { - panic(" The last commit is not corresponding to the last existing height") - } - - r.lastTrustedBlock = &TrustedBlockData{r.store.LoadBlock(state.LastBlockHeight), seenCommit} - - if r.lastTrustedBlock.block == nil { - panic("Failed to load last trusted block") - } - } - } 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.block != nil { - err = VerifyNextBlock(newBlock, newBlockID, verifyBlock, r.lastTrustedBlock.block, r.lastTrustedBlock.commit, state.Validators) - } else { - oldHash := r.initialState.Validators.Hash() + if state.LastBlockHeight == 0 { + // We are starting from genesis + oldHash := state.Validators.Hash() if !bytes.Equal(oldHash, newBlock.ValidatorsHash) { r.logger.Error("The validator set provided by the new block does not match the expected validator set", "initial hash ", r.initialState.Validators.Hash(), @@ -607,6 +591,7 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh ) peerID := r.pool.RedoRequest(state.LastBlockHeight) + r.logger.Info("Redo peer request 1") if serr := blockSyncCh.SendError(ctx, p2p.PeerError{ NodeID: peerID, Err: ErrValidationFailed{}, @@ -615,16 +600,41 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh } continue } - // This case happens only if we do not have any state in our store. - // THus we can assume we are starting from genesis (where we trust the validator set). - // 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 state.LastBlockHeight != 0 { - panic("Cannot be here unles we start from genesis") - } err = state.Validators.VerifyCommitLight(newBlock.ChainID, newBlockID, newBlock.Height, verifyBlock.LastCommit) + } else { + if r.lastTrustedBlock.block == nil { + seenCommit := r.store.LoadSeenCommit() + if state.LastBlockHeight != r.pool.height { + // we are not starting from gensis and not switching from state sync -we whould have a block we can trust in the store + r.lastTrustedBlock = &TrustedBlockData{r.store.LoadBlock(state.LastBlockHeight), seenCommit} + if r.lastTrustedBlock.block == nil { + panic("Failed to load last trusted block") + } + } else { + // We have just switched from state sync so we have no actual block in the store + // but need to use the commit from the store to validate the block we got from our peer + // We will still not set the trusted block to equal to newBlock + if !seenCommit.BlockID.Equals(newBlockID) || !bytes.Equal(state.LastValidators.Hash(), newBlock.ValidatorsHash) { + peerID := r.pool.RedoRequest(r.pool.height) + if serr := blockSyncCh.SendError(ctx, p2p.PeerError{ + NodeID: peerID, + Err: ErrValidationFailed{}, + }); serr != nil { + return + } + continue + } + // We light client verified the header and tryign to apply this to the store would conflict with the current state object + // so we just use newBlock to set r.lastTrustedBlock properly and move on + r.lastTrustedBlock.block = newBlock + r.lastTrustedBlock.commit = verifyBlock.LastCommit + r.pool.PopRequest() + continue + } + } else { // we have been previously in blocksync, the trusted block should be set and used for further validation + err = VerifyNextBlock(newBlock, newBlockID, verifyBlock, r.lastTrustedBlock.block, r.lastTrustedBlock.commit, state.Validators) + } } if err == nil && state.ConsensusParams.ABCI.VoteExtensionsEnabled(newBlock.Height) { @@ -657,17 +667,20 @@ func (r *Reactor) poolRoutine(ctx context.Context, stateSynced bool, blockSyncCh NodeID: peerID, Err: err, }); serr != nil { - return + return // Should this be return? If we disconnected from this peer from some other module or if it was faulty before + // the fact that we cannot send it an error might not be the reason to stop blocksyncing } continue } // validate the block before we persist it + err = r.blockExec.ValidateBlock(ctx, state, newBlock) if err != nil { - r.logger.Error("The validator set provided by the new block does not match the expected validator set", + r.logger.Error("Block validation has failed", "initial hash ", r.initialState.Validators.Hash(), "new hash ", newBlock.ValidatorsHash, + err.Error(), ) peerID := r.pool.RedoRequest(r.lastTrustedBlock.block.Height + 1)