Move blob publish to broadcast batch dispatch, ensure public refs on inbound

[peterbroadhurst]

Resolves #595

• No longer set CRORequirePublicBlobRefs when assembling the messages prior to dispatching a batch (timing issue with this seen in Blob retrieval issue in test with message cache hit #595)
• Move upload of blobs from API thread, to the batch dispatch thread
• Set CRORequirePublicBlobRefs when aggregating pins for broadcast messages

[codecov-commenter]

Codecov Report

Merging #596 (e22a27d) into main (b0bac23) will not change coverage.
The diff coverage is 100.00%.

❗ Current head e22a27d differs from pull request most recent head 57b3bde. Consider uploading reports for the commit 57b3bde to get more accurate results

@@            Coverage Diff            @@
##              main      #596   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          306       306           
  Lines        18023     18017    -6     
=========================================
- Hits         18023     18017    -6     

Impacted Files	Coverage Δ
internal/assets/manager.go	100.00% <100.00%> (ø)
internal/batch/batch_manager.go	100.00% <100.00%> (ø)
internal/batch/batch_processor.go	100.00% <100.00%> (ø)
internal/broadcast/definition.go	100.00% <100.00%> (ø)
internal/broadcast/manager.go	100.00% <100.00%> (ø)
internal/broadcast/message.go	100.00% <100.00%> (ø)
internal/broadcast/operations.go	100.00% <100.00%> (ø)
internal/config/config.go	100.00% <100.00%> (ø)
internal/contracts/manager.go	100.00% <100.00%> (ø)
internal/data/data_manager.go	100.00% <100.00%> (ø)
... and 25 more

---

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 b0bac23...57b3bde. Read the comment docs.

[awrichar]

This was the part where I got stuck before. How does this public ref 1) get stored in the database, and 2) not cause the data hash to change? Hasn't the batch been sealed and stored before we reach here?

[awrichar]

After refreshing myself on the other recent batching changes, it looks like the answer to (2) is that the hash calculation changed to not include the public ref as part of the hash. That's good.

Still can't figure out (1), ie where this data item is updated in the database with the public reference received here. It's definitely updated when it's received again in the event aggregator, but I'd expect it's updated prior to that as well... I'm likely just missing something.

[peterbroadhurst]

Great point. It's not currently at the point the batch is written to the comms layer. As you observe, it is when it comes back in the other end at the end of this PR - but I actually optimized that away in a later change.

I will add it back in, as it's only a performance hit for blobs. But if it's ok will do that as a separate PR.

[peterbroadhurst]

Added in 17b9f90 in #599

[awrichar]

Approved, but I left one big question above for the sake of my own understanding.