Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade zcash_script to zcashd v5.5.0 #84

Merged
merged 21 commits into from
May 5, 2023
Merged

Upgrade zcash_script to zcashd v5.5.0 #84

merged 21 commits into from
May 5, 2023

Conversation

teor2345
Copy link
Collaborator

@teor2345 teor2345 commented Apr 17, 2023

Motivation

We want to upgrade to the same dependencies as zcashd 5.5.0, to avoid duplicate dependencies.

Part of this upgrade has already happened in Zebra:
ZcashFoundation/zebra#6536

Changes

Minor fixes:

Review

This PR contains a copy of the zcashd changes since the last zcash_script update.

You can review the non-zcashd changes here:

@teor2345
Copy link
Collaborator Author

This currently works for builds and releases, but not for tests.

When I run cargo test, I get linker errors like:

"-Wl,-Bdynamic" "-lstdc++" "-lgcc_s" "-lutil" "-lrt" "-lpthread" "-lm" "-ldl" "-lc" "-Wl,--eh-frame-hdr" "-Wl,-z,noexecstack" "-L" "/home/dev/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-o" "/home/dev/zcash_script/target/debug/deps/zcash_script-2f81ce53335cc20b" "-Wl,--gc-sections" "-pie" "-Wl,-z,relro,-z,now" "-nodefaultlibs"                                                       
  = note: /nix/store/8qsl7gmnjsg7n460ll32flz5fzlk2kd7-binutils-2.38/bin/ld: /home/dev/zcash_script/target/debug/build/zcash_script-c13e79cb66fd76cc/out/libzcash_script.a(zcash_script.o): in function `rust::cxxbridge1::Box<orchard_bundle::Bundle>::~Box()':                                                                                                                                                       
          /home/dev/zcash_script/target/debug/build/zcash_script-c13e79cb66fd76cc/out/include/rust/cxx.h:760: undefined reference to `rust::cxxbridge1::Box<orchard_bundle::Bundle>::drop()'               
          /nix/store/8qsl7gmnjsg7n460ll32flz5fzlk2kd7-binutils-2.38/bin/ld: /home/dev/zcash_script/target/debug/build/zcash_script-c13e79cb66fd76cc/out/libzcash_script.a(zcash_script.o): in function `OrchardBundle::OrchardBundle(OrchardBundle const&)':                                                                                                                                                          
          /home/dev/zcash_script/depend/zcash/src/primitives/orchard.h:42: undefined reference to `orchard_bundle::Bundle::box_clone() const'                                                              
          /nix/store/8qsl7gmnjsg7n460ll32flz5fzlk2kd7-binutils-2.38/bin/ld: /home/dev/zcash_script/target/debug/build/zcash_script-c13e79cb66fd76cc/out/libzcash_script.a(zcash_script.o): in function `rust::cxxbridge1::Box<stream::CppStream>::~Box()':                                                                                                                                                            
          /home/dev/zcash_script/target/debug/build/zcash_script-c13e79cb66fd76cc/out/include/rust/cxx.h:760: undefined reference to `rust::cxxbridge1::Box<stream::CppStream>::drop()'                    
          /nix/store/8qsl7gmnjsg7n460ll32flz5fzlk2kd7-binutils-2.38/bin/ld: /home/dev/zcash_script/target/debug/build/zcash_script-c13e79cb66fd76cc/out/libzcash_script.a(zcash_script.o): in function `void OrchardBundle::Serialize<CSizeComputer>(CSizeComputer&) const':                                                                                                                                          
          /home/dev/zcash_script/depend/zcash/src/primitives/orchard.h:71: undefined reference to `orchard_bundle::Bundle::serialize(stream::CppStream&) const'                                            
          /nix/store/8qsl7gmnjsg7n460ll32flz5fzlk2kd7-binutils-2.38/bin/ld: /home/dev/zcash_script/depend/zcash/src/primitives/orchard.h:71: undefined reference to `ToRustStream(CSizeComputer&)'         
          /nix/store/8qsl7gmnjsg7n460ll32flz5fzlk2kd7-binutils-2.38/bin/ld: /home/dev/zcash_script/target/debug/build/zcash_script-c13e79cb66fd76cc/out/libzcash_script.a(zcash_script.o): in function `void OrchardBundle::Unserialize<CBaseDataStream<std::vector<char, zero_after_free_allocator<char> > > >(CBaseDataStream<std::vector<char, zero_after_free_allocator<char> > >&)':                             
          /home/dev/zcash_script/depend/zcash/src/primitives/orchard.h:80: undefined reference to `ToRustStream(CBaseDataStream<std::vector<char, zero_after_free_allocator<char> > >&)'                   
          /nix/store/8qsl7gmnjsg7n460ll32flz5fzlk2kd7-binutils-2.38/bin/ld: /home/dev/zcash_script/depend/zcash/src/primitives/orchard.h:80: undefined reference to `orchard_bundle::parse(stream::CppStream&)'                                                                                                                                                                                                       
          /nix/store/8qsl7gmnjsg7n460ll32flz5fzlk2kd7-binutils-2.38/bin/ld: /home/dev/zcash_script/target/debug/build/zcash_script-c13e79cb66fd76cc/out/libzcash_script.a(zcash_script.o): in function `rust::cxxbridge1::Box<orchard_bundle::Bundle>::operator=(rust::cxxbridge1::Box<orchard_bundle::Bundle>&&) &':                                                                                                 
          /home/dev/zcash_script/target/debug/build/zcash_script-c13e79cb66fd76cc/out/include/rust/cxx.h:767: undefined reference to `rust::cxxbridge1::Box<orchard_bundle::Bundle>::drop()'               
          /nix/store/8qsl7gmnjsg7n460ll32flz5fzlk2kd7-binutils-2.38/bin/ld: /home/dev/zcash_script/target/debug/build/zcash_script-c13e79cb66fd76cc/out/libzcash_script.a(transaction.o): in function `OrchardBundle::OrchardBundle()':                                                                                                                                                                               
          /home/dev/zcash_script/depend/zcash/src/primitives/orchard.h:37: undefined reference to `orchard_bundle::none()'                                                                                 
          /nix/store/8qsl7gmnjsg7n460ll32flz5fzlk2kd7-binutils-2.38/bin/ld: /home/dev/zcash_script/target/debug/build/zcash_script-c13e79cb66fd76cc/out/libzcash_script.a(transaction.o): in function `void OrchardBundle::Serialize<CBaseDataStream<std::vector<char, zero_after_free_allocator<char> > > >(CBaseDataStream<std::vector<char, zero_after_free_allocator<char> > >&) const':                          
          /home/dev/zcash_script/depend/zcash/src/primitives/orchard.h:71: undefined reference to `ToRustStream(CBaseDataStream<std::vector<char, zero_after_free_allocator<char> > >&)'                   
          /nix/store/8qsl7gmnjsg7n460ll32flz5fzlk2kd7-binutils-2.38/bin/ld: /home/dev/zcash_script/depend/zcash/src/primitives/orchard.h:71: undefined reference to `orchard_bundle::Bundle::serialize(stream::CppStream&) const'                                                                                                                                                                                     
          collect2: error: ld returned 1 exit status                                                                                                                          

But the missing functions are bridged here:
https://github.com/zcash/zcash/blob/035e21a61051a5d3b909e6d3d0606bc28d9857ef/src/rust/src/bridge.rs#L216

And defined here:
https://github.com/zcash/zcash/blob/035e21a61051a5d3b909e6d3d0606bc28d9857ef/src/rust/src/orchard_bundle.rs#L80-L82

I tried adding the generated rust/bridge.c to the list of files (like rust/blake2b.c), but it wouldn't compile. It seemed to be missing some important C++ headers.

@teor2345
Copy link
Collaborator Author

@conradoplg can you help fix these zcash_script linker errors?

@conradoplg
Copy link
Contributor

@conradoplg can you help fix these zcash_script linker errors?

Fixed, assuming CI passes. Indeed the linking error was from not including bridge.c. The include error after fixing that was caused by our build.rs generating the .c and .h in the same folder, which was making bridge.c pick up the wrong header with #include "streams.h" (it was picking up the generated one, instead of zcash/src/streams.h). I fixed it by generating them in different folders, like zcashd does (can't remember why I didn't do that in the first place)

@teor2345
Copy link
Collaborator Author

Thank you so much for this!

@teor2345 teor2345 marked this pull request as draft April 18, 2023 04:27
@teor2345
Copy link
Collaborator Author

This is waiting on the zcashd 5.5.0 release:
https://github.com/zcash/zcash/tags

@teor2345
Copy link
Collaborator Author

This is now updated for zcashd v5.5.0-rc1. We're waiting on the stable release before we do our release.

@daira
Copy link
Contributor

daira commented Apr 26, 2023

zcashd v5.5.0 has been tagged, and includes zcash/zcash#6597 and zcash/zcash#6596 (as part of zcash/zcash#6606).

@daira
Copy link
Contributor

daira commented Apr 30, 2023

I notice you're linking with -lstdc++, i.e. libstdc++, whereas zcashd uses libc++. I'm not sure that matters if you're recompiling the script code, just thought I'd point it out.

@teor2345
Copy link
Collaborator Author

teor2345 commented May 2, 2023

I notice you're linking with -lstdc++, i.e. libstdc++, whereas zcashd uses libc++. I'm not sure that matters if you're recompiling the script code, just thought I'd point it out.

Thanks, I opened ticket #85 for this.

@teor2345 teor2345 requested a review from conradoplg May 3, 2023 01:23
@conradoplg conradoplg merged commit 5cfaef7 into master May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade zcash_script after the zcashd 5.5.0 release
3 participants