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 bitcoin to 0.31 #74

Closed
wants to merge 1 commit into from
Closed

Conversation

getong
Copy link

@getong getong commented Jan 27, 2024

This is a fix of #60 , which update rust to 1.67.
It fix the ci error.

Upgrade dependencies to use the latest `rust-bitcoin v0.31`.

While we are at it, bump the crate version ready for release.
@getong getong mentioned this pull request Jan 27, 2024
@tnull
Copy link
Contributor

tnull commented Jan 31, 2024

I don't think we want to bump the MSRV on this crate to 1.67.0. We will however need to find another way forward, e.g., by carefully pinning the respective offending dependency in CI (I think in this case it would be safe as it's just a downstream dependency of our electrsd test dependency I believe), or by introducing a script to download and run bitcoind/electrs as we did in LDK.

@getong getong force-pushed the fix-update branch 4 times, most recently from f87a198 to 8289ff2 Compare January 31, 2024 19:10
@getong
Copy link
Author

getong commented Jan 31, 2024

I don't think we want to bump the MSRV on this crate to 1.67.0. We will however need to find another way forward, e.g., by carefully pinning the respective offending dependency in CI (I think in this case it would be safe as it's just a downstream dependency of our electrsd test dependency I believe), or by introducing a script to download and run bitcoind/electrs as we did in LDK.

I am afraid not.

electrum-client use bitcoin 0.31 from version 0.19.
electrsd use electrum-client 0.19 from 0.27, so electrsd cannot be downstream here.

@tnull
Copy link
Contributor

tnull commented Feb 1, 2024

I don't think we want to bump the MSRV on this crate to 1.67.0. We will however need to find another way forward, e.g., by carefully pinning the respective offending dependency in CI (I think in this case it would be safe as it's just a downstream dependency of our electrsd test dependency I believe), or by introducing a script to download and run bitcoind/electrs as we did in LDK.

I am afraid not.

electrum-client use bitcoin 0.31 from version 0.19. electrsd use electrum-client 0.19 from 0.27, so electrsd cannot be downstream here.

I think there is a misunderstanding here: it's true we can't downgrade the electrsd dependency, but we can pin the dependencies downstream from it that do not meet the MSRV.

@getong
Copy link
Author

getong commented Feb 1, 2024

I revert the code, but this error seems that it might be jobserver, I make a issue of it.
rust-lang/jobserver-rs#69

@tcharding
Copy link
Contributor

Hey mate, I pushed up #79 to do the upgrade which is getting past CI now.

@getong getong closed this Feb 7, 2024
notmandatory added a commit that referenced this pull request Feb 7, 2024
ac2fd86 ci: limit test threads to 1 (Steve Myers)
19bdeb1 replace temporary dependency on bitcoin-internals with hex-conservative (Steve Myers)
c99118e bump electrsd to 0.26 + bitcoind_25_0 and fix msrv errors (Steve Myers)

Pull request description:

  This PR bumps electrsd to 0.26 and replaces temporary dependency on `bitcoin-internals` with `hex-conservative` as done in #75. Fixed CI MSRV issues with dev dependencies blocking #75 and #74.

ACKs for top commit:
  evanlinjin:
    ACK ac2fd86
  tcharding:
    ACK ac2fd86

Tree-SHA512: 37d825aae78d1ca89806870455b97a48e5a69bbdd78adadb16f506fb4df0fe98dc3a262b8fb74b68f5c44031e76c8fa0b5304c8592bf332356a0215843a52c33
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.

3 participants