Generalize add_recipient to accept Address

[rustaceanrob]

Description

I would imagine many users would be handling a Address<NetworkChecked> when building a transaction. They may pass this structure directly with this patch, or continue to use ScriptBuf.

Notes to the reviewers

To my knowledge this is non-breaking, but it is a change in the function signature so I am not sure.

Changelog notice

Accept any type that is convertible to a ScriptBuf in TxBuilder::add_recipient

Checklists

All Submissions:

• I've signed all my commits
• I followed the contribution guidelines
• I ran cargo fmt and cargo clippy before committing

New Features:

• I've added tests for the new feature
• I've added docs for the new feature

Bugfixes:

• This pull request breaks the existing API
• I've added tests to reproduce the issue which are now passing
• I'm linking the issue being fixed by this PR

[ValuedMammal]

Concept ACK, I guess the same could be done for set_recipients?

[rustaceanrob]

That change would require a Box for the dyn Into trait. I am not necessarily opposed to that, but that does feel a little bit strange. I don't think it's the end of the world if this doesn't get in.

For 2.0.0, I would consider adding a Recipient type with the script and amount, and having useful constructors on it that use the impl Into syntax

[ValuedMammal]

ACK 3bb94e4

[evanlinjin]

That change would require a Box for the dyn Into trait. I am not necessarily opposed to that, but that does feel a little bit strange. I don't think it's the end of the world if this doesn't get in.

Why would that change require a Box?

    pub fn set_recipients(
        &mut self,
        recipients: impl IntoIterator<Item = (impl Into<ScriptBuf>, Amount)>,
    ) -> &mut Self {
        self.params.recipients = recipients
            .into_iter()
            .map(|(spk, amount)| (spk.into(), amount))
            .collect();
        self
    }

The only problem with this is that it will be a breaking change because the compiler needs to know the explicit type of recipients. I.e. tx_builder.set_recipients(Vec::new()) will no longer work because of missing annotations.

[evanlinjin]

ACK 3bb94e4

[rustaceanrob]

Why would that change require a Box?

I was thinking dynamic dispatch would be required, but it is cool to see that is not the case. Good catch on the potentially missing annotations in set_recipients if the change were to be applied. Cheers

[oleonardolima]

utACK 3bb94e4

[ValuedMammal]

reACK de2c44f