-
Notifications
You must be signed in to change notification settings - Fork 249
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
Add P2PKH transparent input support to transaction::Builder #107
Add P2PKH transparent input support to transaction::Builder #107
Conversation
@@ -19,8 +19,13 @@ pairing = { path = "../pairing" } | |||
rand = "0.7" | |||
rand_core = "0.5" | |||
rand_os = "0.2" | |||
ripemd160 = { version = "0.8", optional = true } | |||
secp256k1 = { version = "=0.15.0", optional = true } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dependency is pinned until rust-bitcoin/rust-secp256k1#150 is resolved, because we don't want to pull rand
0.6 into our dependency tree (and we aren't using the rand
feature of secp256k1
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have they fixed this yet? (Issue isn't closed but they seem to have merged some things.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They merged a fix, and then immediately reverted it because they had more point releases to make, and the fix for this requires a minor version bump.
Thanks to @adityapk00 for testing this PR as part of his ZecWallet light client and fixing a bug! This should now work correctly. |
Hmm, interesting: draft PRs do not show merge conflicts with master in the GitHub UI (I assume because GitHub assumes you'll rebase on master before marking as ready to review). Lesson learned, I'll rebase 😅 |
This is the counterpart to legacy::TransparentAddress::script.
e25a2d4
to
b479981
Compare
Rebased on master to fix merge conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get the following when running cargo test --release
:
...
Compiling zcash_client_backend v0.1.0 (/home/eirik/development/rust_workspace/librustzcash/zcash_client_backend)
warning: use of deprecated item 'rand_os::OsRng': OsRng is now provided by rand_core and rand
--> zcash_client_backend/src/welding_rig.rs:190:9
|
190 | use rand_os::OsRng;
| ^^^^^^^^^^^^^^
|
= note: #[warn(deprecated)] on by default
warning: use of deprecated item 'rand_os::OsRng': OsRng is now provided by rand_core and rand
--> zcash_client_backend/src/welding_rig.rs:255:23
|
255 | let mut rng = OsRng;
| ^^^^^
warning: use of deprecated item 'rand_os::OsRng': OsRng is now provided by rand_core and rand
--> zcash_client_backend/src/welding_rig.rs:190:9
|
190 | use rand_os::OsRng;
| ^^^^^^^^^^^^^^
Finished release [optimized] target(s) in 2m 19s
Running target/release/deps/bellman-3e8a46c8cdf6e6ec
...
@@ -19,8 +19,13 @@ pairing = { path = "../pairing" } | |||
rand = "0.7" | |||
rand_core = "0.5" | |||
rand_os = "0.2" | |||
ripemd160 = { version = "0.8", optional = true } | |||
secp256k1 = { version = "=0.15.0", optional = true } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have they fixed this yet? (Issue isn't closed but they seem to have merged some things.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
Enabled by the
transparent-inputs
feature.