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

Update rust-bitcoin #2356

Closed
jkitman opened this issue Apr 29, 2023 · 12 comments
Closed

Update rust-bitcoin #2356

jkitman opened this issue Apr 29, 2023 · 12 comments
Assignees
Milestone

Comments

@jkitman
Copy link
Contributor

jkitman commented Apr 29, 2023

It's one of our main crypto libs we are a few versions behind.

edit (justinmoon): edited this to be "update rust-bitcoin" from "update secp256k1"

@jkitman jkitman added this to the Alpha Production Ready milestone Apr 29, 2023
@elsirion
Copy link
Contributor

elsirion commented Apr 29, 2023

This will have to happen in lock step with rust-bitcoin, miniscript, all the BTC clients using rust-bitcoin structs (bitcoind, esplora, electrum) and possibly some other crates. We need to make sure that all required ones support the same secp version.

EDIT: I don't consider this necessary for alpha btw, it's probably mostly API changes, I doubt any serialization changed. One thing that migth be relevant for alpha: MuSig2 with normal compressed instead of x-only public keys #1458.

@jkitman jkitman removed this from the Alpha Production Ready milestone Apr 29, 2023
@elsirion
Copy link
Contributor

elsirion commented Jul 3, 2023

This will likely be part of #2667.

@m1sterc001guy
Copy link
Contributor

m1sterc001guy commented Sep 27, 2023

This is also blocked on the lightning and lightning-invoice crates. They do not support bitcoin 0.30.1 or secp256k1 0.27.0 yet.

https://crates.io/crates/lightning/0.0.116/dependencies
https://crates.io/crates/lightning-invoice/0.24.0/dependencies

@justinmoon justinmoon changed the title Update secp256k1 Update rust-bitcoin Nov 2, 2023
@justinmoon justinmoon added this to the 0.2.1 milestone Nov 2, 2023
@benthecarman
Copy link
Contributor

Latest LDK release is now on 0.30

https://github.com/lightningdevkit/rust-lightning/releases/tag/v0.0.119

@EthnTuttle
Copy link
Contributor

@justinmoon
Copy link
Contributor

lightningdevkit/rust-lightning#2799

If this is the only regression, I think we can still integrate because the only part of LDK that currently runs in WASM is lightning_invoice and the regression this PR fixes is elsewhere.

I propose we update on master but don't backport it.

@tvolk131
Copy link
Member

If no one's currently working on this, I can give it a shot!

@m1sterc001guy
Copy link
Contributor

I have a broken branch attempting to do this here: https://github.com/m1sterc001guy/fedimint/tree/update_rust_bitcoin

These updates touch a lot of files and some structs in rust-bitcoin removed their Encodable and Decodable traits.

@justinmoon
Copy link
Contributor

Relevant TODO

// TODO: remove this as soon as we bump bitcoin_hashes in fedimint_core to
// 0.12.0
pub fn consensus_hash_sha256<E: Encodable>(encodable: &E) -> sha256::Hash {
let mut engine = sha256::HashEngine::default();
encodable
.consensus_encode(&mut engine)
.expect("Writing to HashEngine cannot fail");
sha256::Hash::from_engine(engine)
}

@tvolk131
Copy link
Member

tvolk131 commented May 3, 2024

Unless anyone knows of more work to do for this issue, I think we can mark this as done!

@m1sterc001guy
Copy link
Contributor

@tvolk131 is going to make a separate issue for removing bitcoin 29 from the codebase.

@benthecarman
Copy link
Contributor

congrats!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

7 participants