ElementsProject / rust-elements

Rust support for Elements transaction/block deserialization
Creative Commons Zero v1.0 Universal
52 stars 33 forks source link

bitcoin: bump 0.28.0 -> 0.29.1 #147

Closed RCasatta closed 2 years ago

RCasatta commented 2 years ago

bitcoin_hases: bump 0.10.1 -> 0.11.0 rand: bump 0.6.5 -> 0.8

Code changes relate to:

RCasatta commented 2 years ago

I've got a test failure which I am not sure how to tackle (to check the CI the test is temporarily ignored):

---- confidential::tests::nonce_serde stdout ----
thread 'confidential::tests::nonce_serde' panicked at 'expected Token::Bytes([2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]) but serialized as Tuple { len: 33, }', /home/casatta/.cargo/registry/src/github.com-1ecc6299db9ec823/serde_test-1.0.136/src/ser.rs:240:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
RCasatta commented 2 years ago

Rebased over master, I still have to figure out https://github.com/ElementsProject/rust-elements/pull/147#issuecomment-1259561107

RCasatta commented 2 years ago

The serde error is due to https://github.com/rust-bitcoin/rust-secp256k1/pull/406

So I think it's expected and I have update the test vector and mentioned in the CHANGELOG

apoelstra commented 2 years ago

Can you add a commit which renames serde-feature to serde similar to how rust-bitcoin does it now? (Will also need a CHANGELOG entry)

RCasatta commented 2 years ago

Do not merge yet, I've got issues in downstream lib on serde that wasn't there before the rename (suggestion appreciated, not sure what's happening)

``` error[E0277]: the trait bound `secp256k1_zkp::SurjectionProof: serde::Deserialize<'_>` is not satisfied --> /home/casatta/.cargo/git/checkouts/rust-elements-29d0ae2a32add2d1/7a53252/src/pset/map/output.rs:130:5 | 130 | /// The blind asset surjection proof | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `serde::Deserialize<'_>` is not implemented for `secp256k1_zkp::SurjectionProof` | = help: the following other types implement trait `serde::Deserialize<'de>`: &'a Path &'a [u8] &'a serde_json::value::RawValue &'a str () (T0, T1) (T0, T1, T2) (T0, T1, T2, T3) and 276 others = note: required because of the requirements on the impl of `serde::Deserialize<'_>` for `Box` = note: 1 redundant requirement hidden = note: required because of the requirements on the impl of `serde::Deserialize<'_>` for `std::option::Option>` note: required by a bound in `next_value` --> /home/casatta/.cargo/registry/src/github.com-1ecc6299db9ec823/serde-1.0.145/src/de/mod.rs:1869:12 | 1869 | V: Deserialize<'de>, | ^^^^^^^^^^^^^^^^ required by this bound in `next_value` For more information about this error, try `rustc --explain E0277`. ```
apoelstra commented 2 years ago

Is there a serde feature on secp256k1-zkp that we need to enable?

RCasatta commented 2 years ago

Yes, but it's there like it was before the rename.

apoelstra commented 2 years ago

Maybe your downstream crate is using a different version of secp-zkp now, and use-serde isn't enabled on the downstream version.

BTW we missed an opportunity to rename secp-zkp/use-serde to secp-zkp/serde when we did the release last week :/

RCasatta commented 2 years ago

yeah sorry, there is no issue I was just missing a cargo update ( suggested by @noib3 ) so it's mergeable to me, I am going to merge it tomorrow unless @sanket1729 has some feedback

BTW we missed an opportunity to rename secp-zkp/use-serde to secp-zkp/serde when we did the release last week :/

yes, we should have done that, opening an issue -> https://github.com/ElementsProject/rust-secp256k1-zkp/issues/61