-
Notifications
You must be signed in to change notification settings - Fork 5
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
Bump bitcoin v0.29.1 #36
Conversation
08cbf8f
to
27bd38d
Compare
27bd38d
to
a4e0dc2
Compare
I'm not sure why this is failing CI on edition key. All the updates were to projects with MSRV 1.41.1? @Kixunil may you help me out? |
The binary client has higher MSRV than 1.41.1 and it depends on other things that were updated. Just revert those updates and we can decide later if we need to do something about them. |
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.
Apart from build failures this looks OK.
if witness.is_empty() { | ||
return Weight::ZERO; | ||
} | ||
let mut size = varint_size(witness.len() as u64); | ||
|
||
for item in witness { | ||
for item in witness.iter() { |
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.
Oh, we forgot IntoIterator for &'_ Witness
? Damn!
Made a PR right away: rust-bitcoin/rust-bitcoin#1354
a4e0dc2
to
db5addb
Compare
I think there's something deeper wrong with the build here. Even if I revert, there's an old dependency on Do you know exactly what dependency is msrv noncompliant? I looked at all of their docs and they seem to be. The tests failed at "once_cell" which is a build dependency of bitcoind v0.27.1. v0.27.* claim to be MSRV 0.41. Am I reading the output of cargo tree wrong? |
I think it's MSRV 1.41.1 but only if you use older |
I filed an issue in bitcoind which accidentally breaks its 1.41.1 MSRV promise because of a downstream package. Is there a good reason we're supporting all the way back to 1.41 on this crate? This seems like more of an application-layer crate that would sit at roughly the same level of software as bdk, which is 1.56 MSRV. |
I'd like to move this into the rust-bitcoin org eventually which requires 1.41.1 and for my projects I require whatever is in Debian stable - currently 1.48. But we're considering bumping to 1.48 in rust-bitcoin too so that wouldn't be too bad. The failures are for some tests only though so I wouldn't mind part of the tests being ran on higher MSRV. (Not sure what the people at rust-bitcoin would think about that.) |
depends on Kixunil/bip21#8