BlockId should not have a default and passing a chain tip should be an Option

[LLFourn]

It does not make sense for a BlockId to have a default. We use the Default to semantically mean a null block. e.g.

self.indexed_graph
            .graph()
            .filter_chain_unspents(
                &self.chain,
                self.chain.tip().map(|cp| cp.block_id()).unwrap_or_default(),
                self.indexed_graph.index.outpoints().iter().cloned(),
            )

I think these should just take an Option for the tip since that's the rusty way of communicating null. When it's None it should just be assumed that nothing is in the chain.
Maybe the tip should take an argument that is Option<B> where B: Into<BlockId> to avoid the map.

On a related note ChainOracle has get_chain_tip despite this not being used anywhere. I'd say we should remove it. Perhaps we will want this later but we can just make a new trait for that like GetChainTip.

[SpaghettiSats]

I agree with you, BlockId should not have a Default and chain_tip should definitely be an Option. But I don't think get_chain_tip should be removed. I don't have such a broad view of the project, but I think it can be useful from the perspective of wallet developers, for example as used in example crates.
I would love to work on this if others also find it necessary.

[LLFourn]

Hey @SpaghettiSats. Getting the chain tip is clearly very useful, it just doesn't do anything by being in the trait at the moment. In the examples or your app you will almost always be dealing with a concrete impl of ChainOracle so you can just call its concrete method.

There is a way the library could make use of get_chain_tip and actually justify its existence and that's to have a more convenient version of filter_chain_unspents that doesn't take the tip argument but just fetches it automatically from chain (the first argument). We could consider making filter_chain_unspents the name of that convenient version and have filter_subchain_unspents or something have the API of the current method. Of course we'd need to do this for the fallible version as well. We should also do it for all the other methods that allow passing a chain and a chain tip e.g. balance. So it's quite a bit of typing but it would make the API nicer!

That's the direction I'd take if we want to keep it. Please have a go at that if you feel so inspired.

[SpaghettiSats]

Thanks for the explanation. Now I understand. I will try to do so!

[LLFourn]

See discussion: #1116 (comment)
This should be fixed with: #1079