chainHead: Adjust unpinning logic

[lexnv]

Users need to submit a chainHead_unstable_unpin for each block when they are no longer interested in performing operations on that block.

ChainHead will generate the following events in order:

• NewBlock { hash: 0x1 }
• BestBlock { hash: 0x1 }
• Finalized { hashes: [0x1], pruned_hashes: [...] }

The RPC spec suggests that users are encouraged to call the unpin method either at Finalized event or when they are no longer interested in that block.

Currently, there is nothing stopping the user from calling unpin after the NewBlock and before the BestBlock arrives.

From the substrate perspective, the blocks are pinned on the received event if it wasn't pinned already.
This could lead to the block being unpinned after NewBlock and pin again at BestBlock.

However, if we allow users to unpin only block hashes reported by the finalized event (either finalized blocks or pruned blocks) we could avoid the pinning race; and it would be clear for users when to call the unpin.

Suggestion:

• chainHead_unstable_unpin - called only for hashes reported by the Finalized event
• chainHead_unstable_unpin - since the finalized event contains multiple hashes, extend this method to receive an array of hashes too

What do you think?

[bkchr]

From the substrate perspective, the blocks are pinned on the received event if it wasn't pinned already.
This could lead to the block being unpinned after NewBlock and pin again at BestBlock.

I don't get why we need just store which blocks we already had pinned and the user has unpinned?

pinned: HashMap<Block::Hash, Option<PinHandle>>

When pinned.get(block_hash).unwrap() == None it means the user already has unpinned it (or use an enum to be more precise). Nevertheless, I don't see the problem.

[lexnv]

Closing this for now, as we can address this issue inside substrate without enforcing the user to call unpin at a certain time, thanks for the input!