@liamsi I wasn't quite sure what to do with this block retrieving logic in the store package. I'm guessing that we're replacing a portion of this with RetrieveBlockData, but what about the rest?

I'm guessing that we're replacing a portion of this with RetrieveBlockData, but what about the rest?

Yeah, you are right. Most places like these and others will be replaced by RetrieveBlockData. That was the main reason @marbar3778's store PR #218 (#182) was blocked (as RetrieveBlockData didn't exist when he started it). After Marko finishes that PR the tendermint store won't even store the parts requested in the portion you've linked.
That said, for some nodes calling RetrieveBlock will be very similar to the portion you've linked: they will request shares (instead of Parts) from their local ipld store (because e.g. they already downloaded it during consensus), for other nodes it means requesting the block from the network. The latter might not always be feasible (as the latency might be too large) or not desired in all circumstances. There is no general answer to this: we will have to look and understand each such place and decide to either remove it, replace it with only returning the header (where possible), or, Header+state transition relevant Txs (not Messages), or, actually call RetrieveBlockData (and potentially request it from the network).

In some cases it might be worth to rethink the current logic, e.g. for some RPC endpoints (see #195).
To move forward quickly we can simply replace all such occurrences with RetrieveBlockData and revisit each place in a separate PR (or all places together in one PR).

In some cases it might be worth to rethink the current logic, e.g. for some RPC endpoints (see #195).
To move forward quickly we can simply replace all such occurrences with RetrieveBlockData and revisit each place in a separate PR (or all places together in one PR).

@liamsi I think I can split this into a separate PR, will we need #218 to do this properly?

will we need #218 to do this properly?

I don't think so. Without #218 it would just mean that we are storing the block data twice: once in the IPFS data store and another time in the tendermint store.

Closing as this is no longer required.