electrum: Should `validate_merkle_for_anchor` throw an error for missing or invalid proof?

[ValuedMammal]

Refer to this comment #1489 (comment)

This is a good question and it makes sense to have a runtime option or feature to throw an error if merkle proof validation fails. But I don't think this has to be done in this PR or for the beta release since it will only change the API for the bdk_electrum crate.

[ValuedMammal]

Similarly, should we propagate the error if the transaction_get_merkle API call fails? or are there cases where we expect or allow this to occur, like if given a height of 0?

[storopoli]

Yes, it should.
It is up to the user to handle the error properly.

@evanlinjin has a good point (see below), either we make the Electrum operation atomic or we cannot return an error.

[notmandatory]

I assigned this to the beta milestone since adding the new param and error for this should only affect the bdk_electrum crate, not the bdk_wallet API.

[evanlinjin]

With the electrum API, you can't fetch the header and proof atomically. For both these methods, you fetch header by block height. It's possible that the merkle proof will not match, and not due to a malicious server.

I don't think returning an error makes sense.

[tnull]

With the electrum API, you can't fetch the header and proof atomically. For both these methods, you fetch header by block height. It's possible that the merkle proof will not match, and not due to a malicious server.

You could use this to detect that the reorg happened and that you should restart syncing without applying any changes to avoid inconsistencies. FWIW, this is what we do in LDK's ElectrumSyncClient.

[ValuedMammal]

We use validate_merkle_for_anchor internally to determine whether it's ok to insert a ConfirmationBlockTime anchor into the tx graph. We don't expect every call to this function to result in a newly added anchor - it could be that the transaction isn't confirmed or we tried to validate it against the wrong block header but the assumption is that the right information is eventually reconciled. I verified that it works using example-crates/wallet_electrum. To echo tnull's point, we could potentially use the knowledge that a merkle proof failed to detect that a reorg happened.