Respect disableoutputsubstitution in payjoin-cli

[spacebear21]

Fix for #238

[DanGould]

This looks correct to me but I wonder if it makes sense to combine separate is_output_substitution_disabled() and substitute_output_address into a try_substitute_output[_address] that always errored if disableoutputsubstitution were true.

My concern with such a change is requiring one to go through all the trouble of getting a substitute address, or worse, a channel open contract address, and never using it. I'm inclined to approve what you have and also request the substitute_output_address function is fallable in case output substitution is in fact disabled.

The substitution in this implementation is mostly so that a static payjoin-cli server which only posted a single bip21 didn't receive a bunch of payments to one address, so I think this change is appropriate.

[spacebear21]

I wonder if it makes sense to combine separate is_output_substitution_disabled() and substitute_output_address into a try_substitute_output[_address] that always errored if disableoutputsubstitution were true.

I considered this approach as well but shared your concern about never using a generated address. How about if substitute_output_address takes a callable instead of an address? Something like this:

    pub fn substitute_output_address(
        &mut self,
        generate_address: impl Fn() -> Result<bitcoin::Address, Error>,
    ) -> Result<(), Error> {
        if self.params.disable_output_substitution {
            return Error();
        }
        let substitute_address = generate_address()?;
        self.payjoin_psbt.unsigned_tx.output[self.owned_vouts[0]].script_pubkey =
            substitute_address.script_pubkey();
        Ok(())
    }

[DanGould]

This looks like a fix if I've ever seem one 👍