Update libzcash_script to include support for V5 transactions

[jvff]

The upstream repository added support for ZIP-225 and ZIP-244, resulting in support for V5 transactions. This PR is an attempt to close #21 by updating the upstream dependency. When I tried to update it, I ran into some issues. I'll leave this as a draft PR until these issues are solved.

The update procedure was the following:

1. Replace depend/zcash subtree with a submodule (see more info in the "Outstanding issues" section)
2. Include some new FFI modules required by upstream (transaction_ffi, orchard_ffi and streams_ffi)
3. Include zcash/src/support/cleanse.cpp and zcash/src/support/cleanse.h in the build and package
4. Replicate some upstream dependencies (halo2, orchard, rand_core, tracing and zcash_primitives)
5. Patch the dependency tree to use some unreleased crate versions (this must also be done on crates that use this crate, see more info in the "Outstanding issues" section)

Outstanding issues

• subtree commit not found: when running git subtree pull -P depend/zcash <zcash-upstream> <commit>, it failed with an error saying that a commit could not be found. I couldn't find the commit anywhere, but later @str4d found it and pointed out the commit was out-of-tree. I also found the likely rebased commit. I wasn't sure how to fix this using git subtree, so I ended up replacing the subtree with a submodule, so that the remote is properly reference to avoid similar issues in the future. It also simplified the experimentation process because I could jump to different upstream commits more easily. Long-term, this should reference an in-tree commit in zcash/zcash, independently if a subtree or a submodule is used. The submodule is currently referencing a fork because of some fixes discussed in the next item, but a reference to the fork should NOT be in the final PR to be merged.
  • Solution: Reference a master branch commit in the final PR.
• C++ compile errors: I had to fix a few C++ compiler errors that appeared when trying to compile libzcash_script. These fixes can be seen here. I'm not sure if they are correct and should be upstreamed, or if they are the consequence of a mismatched compiler version. I'm still trying to figure that out, but when discussing with @str4d, one possibility is because Zcash is compiling it as C99 and zcash_script is compiling as C++17.
  • Solution: Compile as C99 (Build the secp256k1 C files separately #23).
• patched dependencies: currently upstream depends on some crates that don't have any releases yet, so it includes a [patch.crates-io] in Cargo.toml. That section with the patched dependencies must also be included in this repository and in any crate that uses zcash_script. Ideally, these should be proper dependencies to avoid having to downstream the patches.
  • Solution: Wait until the patches are removed or they are more stable. Included as is for now. Downstream dependencies will have to apply the same patches to use this library.

[teor2345]

• one possibility is because Zcash is compiling it as C99 and zcash_script is compiling as C++17.

zcash_script was meant to be using the same language and standard version as zcashd. So we should fix this issue by using C99.

[jvff]

• one possibility is because Zcash is compiling it as C99 and zcash_script is compiling as C++17.

zcash_script was meant to be using the same language and standard version as zcashd. So we should fix this issue by using C99.

This specific issue was fixed by #23.

[jvff]

I ran into a weird issue. It seems that there's a file in the Zcash source code that uses a C++20 feature: Aggregate initialization.

https://github.com/zcash/zcash/blob/master/src/primitives/transaction.cpp#L440-L442

What's even weirder is that compiling this on Linux using C++17 works, but on Windows it doesn't. I've disabled Windows CI builds for now, but this should be fixed in the future.

For the error itself, see: https://github.com/ZcashFoundation/zcash_script/runs/3799118607#step:5:1095

[teor2345]

I ran into a weird issue. It seems that there's a file in the Zcash source code that uses a C++20 feature: Aggregate initialization.

https://github.com/zcash/zcash/blob/master/src/primitives/transaction.cpp#L440-L442

What's even weirder is that compiling this on Linux using C++17 works, but on Windows it doesn't.

I assume we're using different compilers or compiler versions in the Windows and Linux tests.

I've disabled Windows CI builds for now, but this should be fixed in the future.

This seems like a blocker for a Zebra release, because we'd have to disable Windows Zebra builds as well.

For the error itself, see: https://github.com/ZcashFoundation/zcash_script/runs/3799118607#step:5:1095

error C7555: use of designated initializers requires at least '/std:c++20'

Given our tight timeframes here, I've committed the suggested fix to this PR, and re-enabled Windows builds.

[teor2345]

I ran into a weird issue. It seems that there's a file in the Zcash source code that uses a C++20 feature: Aggregate initialization.
https://github.com/zcash/zcash/blob/master/src/primitives/transaction.cpp#L440-L442
What's even weirder is that compiling this on Linux using C++17 works, but on Windows it doesn't.

I assume we're using different compilers or compiler versions in the Windows and Linux tests.

We're using MSVC on Windows, and clang (or maybe gcc?) elsewhere.

I tried a few different C++ standards version flags, for all platforms, and specifically for Windows. But none of them worked.

I've just applied a change that always sets the compiler to clang.
This change doesn't add any extra dependencies, because Zebra already depends on clang via bindgen.
But it might need some PATH tweaks in CI, or for some users.

If this PR passes CI, let's merge, update Zebra's dependencies, and go ahead with the release.

[teor2345]

I've just applied a change that always sets the compiler to clang.

That didn't work, because the cargo target is x86_64-pc-windows-msvc, which assumes that the compiler is MSVC.
(And it will expect MSVC-format binaries during linking.)

Maybe we should edit the problematic C++ file, and submit a patch to the zcashd repository?
I'm happy to do that tomorrow. (In 14 hours time.)

[str4d]

I wasn't sure how to fix this using git subtree,

IIRC the problem could not be fixed without rolling back the master branch, because the mistake was made in #16 which was merged into master, and repairing a broken subtree is AFAICT impossible. I did test locally, and I was able to fix the subtree in this way.

[teor2345]

Using Foundation repositories isn't urgent, but it is a release blocker.
(It's caused us a bunch of problems in the past when users have deleted branches or forks.)

[teor2345]

This is good to merge, once we're sure it works for Zebra.

[daira]

Maybe we should edit the problematic C++ file, and submit a patch to the zcashd repository? I'm happy to do that tomorrow. (In 14 hours time.)

Did you do this? We're not intentionally using C++20 in zcashd; if we did so accidentally because clang accepts it even in C++17 mode, that's a bug.

The logs at https://github.com/ZcashFoundation/zcash_script/runs/3799118607 have been deleted so I can't see which code this affected from there.

[teor2345]

Maybe we should edit the problematic C++ file, and submit a patch to the zcashd repository? I'm happy to do that tomorrow. (In 14 hours time.)

Did you do this? We're not intentionally using C++20 in zcashd; if we did so accidentally because clang accepts it even in C++17 mode, that's a bug.

The logs at https://github.com/ZcashFoundation/zcash_script/runs/3799118607 have been deleted so I can't see which code this affected from there.

No, we split CurrentTxVersionInfo into another file instead, because the C++20 code was not actually used by zcash_script.

Here's our commit with the subtree fix, but it's hard to see the diff, because we needed to change to our fork as well:
2e4cd75

CurrentTxVersionInfo still uses aggregate initialization (C++20) in the current zcashd code:
https://github.com/zcash/zcash/blob/f8e99e7ba501e4a3b4bdc75622788181b15129e7/src/primitives/tx_version_info.cpp#L11-21