Stop assuming Mainnet in Address From impls

[teor2345]

Motivation

Some From<...> for Address impls assume all addresses are on mainnet. But we also need to support testnet addresses.

Solution

Modify the From<...> for Address impls to take a tuple containing a Network enum value.

Replace the From<...> for Address impls with methods that take a Network enum value.

Add tests for testnet addresses.

Related Issues

Closes #1187.

[teor2345]

@yaahc rust style question:

Should we pass a tuple to the From impl:

impl From<(Script, Network)> for Address {
    fn from((script, network): (Script, Network)) -> Self {

https://github.com/ZcashFoundation/zebra/pull/1191/files#diff-49798c4b198412d5cae6a56e92cd70440e3f437f4daafbada4e12db00a3dbce2R95-R96

Or define a custom struct:

/// Custom type that holds a script and the network it belongs to.
pub struct ScriptForNetwork {
    pub script: Script,
    pub network: Network,
}

https://github.com/ZcashFoundation/zebra/pull/1170/files#diff-862e9002ed8031a07a7a9968aa734ea16dc490d152b7da97e4ed160843403494R105-R109

[dconnolly]

🎉

[dconnolly]

@yaahc rust style question:

Should we pass a tuple to the From impl:

impl From<(Script, Network)> for Address {
    fn from((script, network): (Script, Network)) -> Self {

https://github.com/ZcashFoundation/zebra/pull/1191/files#diff-49798c4b198412d5cae6a56e92cd70440e3f437f4daafbada4e12db00a3dbce2R95-R96

Or define a custom struct:

/// Custom type that holds a script and the network it belongs to.
pub struct ScriptForNetwork {
    pub script: Script,
    pub network: Network,
}

https://github.com/ZcashFoundation/zebra/pull/1170/files#diff-862e9002ed8031a07a7a9968aa734ea16dc490d152b7da97e4ed160843403494R105-R109

I am also interested in this

[yaahc]

@yaahc rust style question:
Should we pass a tuple to the From impl:

impl From<(Script, Network)> for Address {
    fn from((script, network): (Script, Network)) -> Self {

https://github.com/ZcashFoundation/zebra/pull/1191/files#diff-49798c4b198412d5cae6a56e92cd70440e3f437f4daafbada4e12db00a3dbce2R95-R96
Or define a custom struct:

/// Custom type that holds a script and the network it belongs to.
pub struct ScriptForNetwork {
    pub script: Script,
    pub network: Network,
}

https://github.com/ZcashFoundation/zebra/pull/1170/files#diff-862e9002ed8031a07a7a9968aa734ea16dc490d152b7da97e4ed160843403494R105-R109

I am also interested in this

I don't know of any convention for this, I don't think either of these options sounds ideal off of the top of my head tho the second one I might need to think about more. My instinct would be to have it be converted with a method on script that takes an Network argument. maybe something like let address = script.addr(network);

[teor2345]

Ok, I'll switch to a method on Script - that's what we do for the other conversions and checks that take a Network argument.

[teor2345]

See c01f804 for the method refactor - I needed to use a trait to implement methods on a foreign type.

[oxarbitrage]

Looks good. By what we know from #1170 the to_address will probably need changes but we can handle that in a separated PR(can be #1170 itself or other)

[teor2345]

Looks good. By what we know from #1170 the to_address will probably need changes but we can handle that in a separated PR(can be #1170 itself or other)

Let's add the extra script conversions in #1170, because they are needed to make the block subsidy checks work.
And we should add tests for them as well. Let's use zcashd test vectors, if we can.

[oxarbitrage]

Let's add the extra script conversions in #1170, because they are needed to make the block subsidy checks work.
And we should add tests for them as well. Let's use zcashd test vectors, if we can.

I added the extra conversions into #1170 after a rebase. This is being tested with zcashd test vectors in subsidy_is_valid_for_historical_blocks test case.