diff --git a/catchup/catchpointService.go b/catchup/catchpointService.go index a5175aff4c..a0a22c5e33 100644 --- a/catchup/catchpointService.go +++ b/catchup/catchpointService.go @@ -373,8 +373,9 @@ func (cs *CatchpointCatchupService) processStageLatestBlockDownload() (err error var blk *bookkeeping.Block var cert *agreement.Certificate // check to see if the current ledger might have this block. If so, we should try this first instead of downloading anything. - if ledgerBlock, err := cs.ledger.Block(blockRound); err == nil { + if ledgerBlock, ledgerCert, err0 := cs.ledger.BlockCert(blockRound); err0 == nil { blk = &ledgerBlock + cert = &ledgerCert } var protoParams config.ConsensusParams var ok bool @@ -551,15 +552,17 @@ func (cs *CatchpointCatchupService) processStageBlocksDownload() (err error) { } blk = nil + cert = nil // check to see if the current ledger might have this block. If so, we should try this first instead of downloading anything. - if ledgerBlock, err := cs.ledger.Block(topBlock.Round() - basics.Round(blocksFetched)); err == nil { + if ledgerBlock, ledgerCert, err0 := cs.ledger.BlockCert(topBlock.Round() - basics.Round(blocksFetched)); err0 == nil { blk = &ledgerBlock + cert = &ledgerCert } else { - switch err.(type) { + switch err0.(type) { case ledgercore.ErrNoEntry: // this is expected, ignore this one. default: - cs.log.Warnf("processStageBlocksDownload encountered the following error when attempting to retrieve the block for round %d : %v", topBlock.Round()-basics.Round(blocksFetched), err) + cs.log.Warnf("processStageBlocksDownload encountered the following error when attempting to retrieve the block for round %d : %v", topBlock.Round()-basics.Round(blocksFetched), err0) } } diff --git a/catchup/catchpointService_test.go b/catchup/catchpointService_test.go index 48cea110d7..34f1adf0fd 100644 --- a/catchup/catchpointService_test.go +++ b/catchup/catchpointService_test.go @@ -22,12 +22,14 @@ import ( "github.com/stretchr/testify/require" + "github.com/algorand/go-algorand/agreement" "github.com/algorand/go-algorand/components/mocks" "github.com/algorand/go-algorand/crypto" "github.com/algorand/go-algorand/data/basics" "github.com/algorand/go-algorand/data/bookkeeping" "github.com/algorand/go-algorand/ledger" "github.com/algorand/go-algorand/ledger/ledgercore" + "github.com/algorand/go-algorand/logging" "github.com/algorand/go-algorand/protocol" "github.com/algorand/go-algorand/test/partitiontest" ) @@ -35,7 +37,7 @@ import ( type catchpointCatchupLedger struct { } -func (l *catchpointCatchupLedger) Block(rnd basics.Round) (blk bookkeeping.Block, err error) { +func (l *catchpointCatchupLedger) BlockCert(rnd basics.Round) (blk bookkeeping.Block, cert agreement.Certificate, err error) { blk = bookkeeping.Block{ BlockHeader: bookkeeping.BlockHeader{ UpgradeState: bookkeeping.UpgradeState{ @@ -43,13 +45,14 @@ func (l *catchpointCatchupLedger) Block(rnd basics.Round) (blk bookkeeping.Block }, }, } + cert = agreement.Certificate{} commitments, err := blk.PaysetCommit() if err != nil { - return blk, err + return blk, cert, err } blk.TxnCommitments = commitments - return blk, nil + return blk, cert, nil } func (l *catchpointCatchupLedger) GenesisHash() (d crypto.Digest) { @@ -95,3 +98,64 @@ func TestCatchpointServicePeerRank(t *testing.T) { err := cs.processStageLatestBlockDownload() require.NoError(t, err) } + +type catchpointAccessorMock struct { + mocks.MockCatchpointCatchupAccessor + t *testing.T + topBlk bookkeeping.Block +} + +func (m *catchpointAccessorMock) EnsureFirstBlock(ctx context.Context) (blk bookkeeping.Block, err error) { + return m.topBlk, nil +} + +func (m *catchpointAccessorMock) StoreBlock(ctx context.Context, blk *bookkeeping.Block, cert *agreement.Certificate) (err error) { + require.NotNil(m.t, blk) + require.NotNil(m.t, cert) + return nil +} + +type catchpointCatchupLedger2 struct { + catchpointCatchupLedger + blk bookkeeping.Block +} + +func (l *catchpointCatchupLedger2) BlockCert(rnd basics.Round) (blk bookkeeping.Block, cert agreement.Certificate, err error) { + return l.blk, agreement.Certificate{}, nil +} + +// TestProcessStageBlocksDownloadNilCert ensures StoreBlock does not receive a nil certificate when ledger has already had a block. +// It uses two mocks catchpointAccessorMock and catchpointCatchupLedger2 and pre-crafted blocks to make a single iteration of processStageBlocksDownload. +func TestProcessStageBlocksDownloadNilCert(t *testing.T) { + partitiontest.PartitionTest(t) + + var err error + blk1 := bookkeeping.Block{ + BlockHeader: bookkeeping.BlockHeader{ + Round: 1, + UpgradeState: bookkeeping.UpgradeState{ + CurrentProtocol: protocol.ConsensusCurrentVersion, + }, + }, + } + blk1.TxnCommitments, err = blk1.PaysetCommit() + require.NoError(t, err) + + blk2 := blk1 + blk2.BlockHeader.Round = 2 + blk2.BlockHeader.Branch = blk1.Hash() + blk2.TxnCommitments, err = blk2.PaysetCommit() + require.NoError(t, err) + + ctx, cf := context.WithCancel(context.Background()) + cs := CatchpointCatchupService{ + ctx: ctx, + cancelCtxFunc: cf, + ledgerAccessor: &catchpointAccessorMock{topBlk: blk2, t: t}, + ledger: &catchpointCatchupLedger2{blk: blk1}, + log: logging.TestingLog(t), + } + + err = cs.processStageBlocksDownload() + require.NoError(t, err) +} diff --git a/ledger/catchupaccessor.go b/ledger/catchupaccessor.go index 64ada07a08..3661c60057 100644 --- a/ledger/catchupaccessor.go +++ b/ledger/catchupaccessor.go @@ -238,7 +238,7 @@ const ( // CatchupAccessorClientLedger represents ledger interface needed for catchpoint accessor clients type CatchupAccessorClientLedger interface { - Block(rnd basics.Round) (blk bookkeeping.Block, err error) + BlockCert(rnd basics.Round) (blk bookkeeping.Block, cert agreement.Certificate, err error) GenesisHash() crypto.Digest BlockHdr(rnd basics.Round) (blk bookkeeping.BlockHeader, err error) Latest() (rnd basics.Round)