fix(dot/sync): cleanup logs; don't log case where we fail to get parent while processing

[noot]

Changes

• don't log case where we fail to get parent while processing (except as a trace log)
• this doesn't explicitly track the first block that fails (and its whole descendant chain), but I think it's ok as the processing of the block will fail almost immediately if we don't have the parent (ie line 214)

Tests

./bin/gossamer --chain polkadot

ensure that logs don't get spammed anymore

Issues

• closes Record failed block addition to prevent processing of child blocks in the blocktree #2154

Primary Reviewer

• @qdm12

[codecov]

Codecov Report

Merging #2188 (75ea8b6) into development (0cc4006) will increase coverage by 0.01%.
The diff coverage is 66.66%.

❗ Current head 75ea8b6 differs from pull request most recent head 3c0232e. Consider uploading reports for the commit 3c0232e to get more accurate results

@@               Coverage Diff               @@
##           development    #2188      +/-   ##
===============================================
+ Coverage        61.51%   61.52%   +0.01%     
===============================================
  Files              213      213              
  Lines            27497    27512      +15     
===============================================
+ Hits             16915    16927      +12     
- Misses            8711     8714       +3     
  Partials          1871     1871              

Flag	Coverage Δ
unit-tests	61.52% <66.66%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
dot/sync/chain_processor.go	49.42% <66.66%> (-0.29%)	⬇️
dot/config.go	26.17% <0.00%> (-0.02%)	⬇️
dot/digest/digest.go	64.31% <0.00%> (ø)
cmd/gossamer/flags.go	66.66% <0.00%> (ø)
pkg/scale/result.go	97.14% <0.00%> (+0.12%)	⬆️
cmd/gossamer/config.go	75.41% <0.00%> (+0.22%)	⬆️
cmd/gossamer/utils.go	74.68% <0.00%> (+0.32%)	⬆️
dot/network/connmgr.go	90.41% <0.00%> (+1.36%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0cc4006...3c0232e. Read the comment docs.

[qdm12]

nit Maybe swap around the condition for less indentation?

Suggested change

	if errors.Is(err, errFailedToGetParent) {
	logger.Tracef("block data processing for block with hash %s failed: %s", bd.Hash, err)
	if err := s.pendingBlocks.addBlock(&types.Block{
	Header: *bd.Header,
	Body: *bd.Body,
	}); err != nil {
	logger.Debugf("failed to re-add block to pending blocks: %s", err)
	}
	continue
	}
	logger.Errorf("block data processing for block with hash %s failed: %s", bd.Hash, err)
	if !errors.Is(err, errFailedToGetParent) {
	logger.Errorf("block data processing for block with hash %s failed: %s", bd.Hash, err)
	continue
	}
	logger.Tracef("block data processing for block with hash %s failed: %s", bd.Hash, err)
	if err := s.pendingBlocks.addBlock(&types.Block{
	Header: *bd.Header,
	Body: *bd.Body,
	}); err != nil {
	logger.Debugf("failed to re-add block to pending blocks: %s", err)
	}
	}

[noot]

done!

[qdm12]

Are you sure that's fine to just silence away? Is it fine to get this errFailedToGetParent in all cases? That's why recording blocks that failed to be added would be nice since we could separate the errFailedToGetParent that are due to a previous block error and some that might be due to something else 🤔 ? I'm not super familiar with this code so I might just be too paranoid

[noot]

@qdm12 yeah I think it's fine, as the sync algorithm will refetch the blocks if they're still needed. one case why this would happen is that the blocktree gets pruned as some block got finalized, and then any blocks on the pruned chain will result in this error when processing, in which case we don't want them anyways. we were also just ignoring it before anyways and it was fine haha