From 50fd86e2878a6dcf1399610d8edc146466a5b492 Mon Sep 17 00:00:00 2001 From: Ori Newman Date: Thu, 13 May 2021 15:45:03 +0300 Subject: [PATCH] Fix getBlock and getBlocks RPC commands to return blocks and transactions properly (#1716) * Fix getBlock RPC command to return transactions * Fix getBlocks RPC command to return transactions and blocks * Add GetBlockEvenIfHeaderOnly and use it for getBlock and getBlocks * Implement GetBlockEvenIfHeaderOnly for fakeRelayInvsContext * Use less nested code --- .../flows/testing/handle_relay_invs_test.go | 4 ++++ app/rpc/rpccontext/verbosedata.go | 2 +- app/rpc/rpchandlers/get_block.go | 12 ++++++---- app/rpc/rpchandlers/get_blocks.go | 15 ++++++++---- domain/consensus/consensus.go | 24 +++++++++++++++++++ .../consensus/model/externalapi/consensus.go | 1 + 6 files changed, 48 insertions(+), 10 deletions(-) diff --git a/app/protocol/flows/testing/handle_relay_invs_test.go b/app/protocol/flows/testing/handle_relay_invs_test.go index ba6725039..3650b929a 100644 --- a/app/protocol/flows/testing/handle_relay_invs_test.go +++ b/app/protocol/flows/testing/handle_relay_invs_test.go @@ -130,6 +130,10 @@ type fakeRelayInvsContext struct { rwLock sync.RWMutex } +func (f *fakeRelayInvsContext) GetBlockEvenIfHeaderOnly(blockHash *externalapi.DomainHash) (*externalapi.DomainBlock, error) { + panic("implement me") +} + func (f *fakeRelayInvsContext) GetBlockRelations(blockHash *externalapi.DomainHash) ([]*externalapi.DomainHash, *externalapi.DomainHash, []*externalapi.DomainHash, error) { panic(errors.Errorf("called unimplemented function from test '%s'", f.testName)) } diff --git a/app/rpc/rpccontext/verbosedata.go b/app/rpc/rpccontext/verbosedata.go index 1380b7ed6..d790b52b6 100644 --- a/app/rpc/rpccontext/verbosedata.go +++ b/app/rpc/rpccontext/verbosedata.go @@ -80,7 +80,7 @@ func (ctx *Context) PopulateBlockWithVerboseData(block *appmessage.RPCBlock, dom // Get the block if we didn't receive it previously if domainBlock == nil { - domainBlock, err = ctx.Domain.Consensus().GetBlock(blockHash) + domainBlock, err = ctx.Domain.Consensus().GetBlockEvenIfHeaderOnly(blockHash) if err != nil { return err } diff --git a/app/rpc/rpchandlers/get_block.go b/app/rpc/rpchandlers/get_block.go index 62e734b48..22ad29b9f 100644 --- a/app/rpc/rpchandlers/get_block.go +++ b/app/rpc/rpchandlers/get_block.go @@ -20,18 +20,22 @@ func HandleGetBlock(context *rpccontext.Context, _ *router.Router, request appme return errorMessage, nil } - header, err := context.Domain.Consensus().GetBlockHeader(hash) + block, err := context.Domain.Consensus().GetBlockEvenIfHeaderOnly(hash) if err != nil { errorMessage := &appmessage.GetBlockResponseMessage{} errorMessage.Error = appmessage.RPCErrorf("Block %s not found", hash) return errorMessage, nil } - block := &externalapi.DomainBlock{Header: header} response := appmessage.NewGetBlockResponseMessage() - response.Block = appmessage.DomainBlockToRPCBlock(block) - err = context.PopulateBlockWithVerboseData(response.Block, header, nil, getBlockRequest.IncludeTransactionVerboseData) + if getBlockRequest.IncludeTransactionVerboseData { + response.Block = appmessage.DomainBlockToRPCBlock(block) + } else { + response.Block = appmessage.DomainBlockToRPCBlock(&externalapi.DomainBlock{Header: block.Header}) + } + + err = context.PopulateBlockWithVerboseData(response.Block, block.Header, block, getBlockRequest.IncludeTransactionVerboseData) if err != nil { if errors.Is(err, rpccontext.ErrBuildBlockVerboseDataInvalidBlock) { errorMessage := &appmessage.GetBlockResponseMessage{} diff --git a/app/rpc/rpchandlers/get_blocks.go b/app/rpc/rpchandlers/get_blocks.go index b5bafbc89..d76427021 100644 --- a/app/rpc/rpchandlers/get_blocks.go +++ b/app/rpc/rpchandlers/get_blocks.go @@ -84,19 +84,24 @@ func HandleGetBlocks(context *rpccontext.Context, _ *router.Router, request appm response := appmessage.NewGetBlocksResponseMessage() response.BlockHashes = hashes.ToStrings(blockHashes) if getBlocksRequest.IncludeBlocks { - blocks := make([]*appmessage.RPCBlock, len(blockHashes)) + rpcBlocks := make([]*appmessage.RPCBlock, len(blockHashes)) for i, blockHash := range blockHashes { - blockHeader, err := context.Domain.Consensus().GetBlockHeader(blockHash) + block, err := context.Domain.Consensus().GetBlockEvenIfHeaderOnly(blockHash) if err != nil { return nil, err } - block := &externalapi.DomainBlock{Header: blockHeader} - blocks[i] = appmessage.DomainBlockToRPCBlock(block) - err = context.PopulateBlockWithVerboseData(blocks[i], blockHeader, nil, getBlocksRequest.IncludeTransactionVerboseData) + + if getBlocksRequest.IncludeTransactionVerboseData { + rpcBlocks[i] = appmessage.DomainBlockToRPCBlock(block) + } else { + rpcBlocks[i] = appmessage.DomainBlockToRPCBlock(&externalapi.DomainBlock{Header: block.Header}) + } + err = context.PopulateBlockWithVerboseData(rpcBlocks[i], block.Header, nil, getBlocksRequest.IncludeTransactionVerboseData) if err != nil { return nil, err } } + response.Blocks = rpcBlocks } return response, nil diff --git a/domain/consensus/consensus.go b/domain/consensus/consensus.go index 69907e3f1..4ba5179cd 100644 --- a/domain/consensus/consensus.go +++ b/domain/consensus/consensus.go @@ -112,6 +112,30 @@ func (s *consensus) GetBlock(blockHash *externalapi.DomainHash) (*externalapi.Do return block, nil } +func (s *consensus) GetBlockEvenIfHeaderOnly(blockHash *externalapi.DomainHash) (*externalapi.DomainBlock, error) { + s.lock.Lock() + defer s.lock.Unlock() + + stagingArea := model.NewStagingArea() + + block, err := s.blockStore.Block(s.databaseContext, stagingArea, blockHash) + if err == nil { + return block, nil + } + if !errors.Is(err, database.ErrNotFound) { + return nil, err + } + + header, err := s.blockHeaderStore.BlockHeader(s.databaseContext, stagingArea, blockHash) + if err != nil { + if errors.Is(err, database.ErrNotFound) { + return nil, errors.Wrapf(err, "block %s does not exist", blockHash) + } + return nil, err + } + return &externalapi.DomainBlock{Header: header}, nil +} + func (s *consensus) GetBlockHeader(blockHash *externalapi.DomainHash) (externalapi.BlockHeader, error) { s.lock.Lock() defer s.lock.Unlock() diff --git a/domain/consensus/model/externalapi/consensus.go b/domain/consensus/model/externalapi/consensus.go index 46c0fafae..1848400ad 100644 --- a/domain/consensus/model/externalapi/consensus.go +++ b/domain/consensus/model/externalapi/consensus.go @@ -7,6 +7,7 @@ type Consensus interface { ValidateTransactionAndPopulateWithConsensusData(transaction *DomainTransaction) error GetBlock(blockHash *DomainHash) (*DomainBlock, error) + GetBlockEvenIfHeaderOnly(blockHash *DomainHash) (*DomainBlock, error) GetBlockHeader(blockHash *DomainHash) (BlockHeader, error) GetBlockInfo(blockHash *DomainHash) (*BlockInfo, error) GetBlockRelations(blockHash *DomainHash) (parents []*DomainHash, selectedParent *DomainHash, children []*DomainHash, err error)