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

Add v25.1, update minor releases, use bitcoincore.org, verify using bitcoin core verify script, fixup alpine builds #144

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

willcl-ark
Copy link

@willcl-ark willcl-ark commented Sep 5, 2023

This pull includes a number of changes:

  • fix broken older builds, and update some minor point releases
  • add v25.1
  • switch default binary repository to bitcoincore.org for better reliability
  • use the bitcoin core verify-binaries/verify.py script to simplify downloading and verifying versions >= v22.0 which are now signed by keys found in the guix.sigs repository
  • lock builds to debian:bookworm-slim

Each commit message contains additional detail on the rationale and implementation.

If I got this right, this should also:

close #140
close #139
close #138
close #137
close #136

I'd like to see this, or something like this, merged and have updated images available from this repo, so let me know if you'd like any changes from me to move this forward.

bitcoin.org often gives 404 for these URLs, I'm not sure why.
bitcoincore.org is better maintained, make it the default.

In a later commit the verify.py script will attempt to fetch from both
endpoints again for versions >= 22.0

URLS replaced using a scripted diff:

-BEGIN VERIFY SCRIPT-
find . -type f -name "Dockerfile" | while read -r dockerfile; do
    sed -i 's|https://bitcoin.org|https://bitcoincore.org|g' "$dockerfile"
done
-END VERIFY SCRIPT-
@willcl-ark willcl-ark changed the title Update bitcoin.org urls to bitcoincore.org Add v25.0, update minor releases, use bitcoincore.org, verify using bitcoin core verify script, fixup alpine builds Sep 5, 2023
@willcl-ark
Copy link
Author

Re the commit message in "Use bitcoin-core verify.py script": I opened a pull bitcoin/bitcoin#28418 to permit exact architecture-platform specifiers and reduce the amount downloaded by the verify.py script.

IMO even if it's unsuccessful there, it doesn't matter much here, as these images are not updated frequently. So an extra few MB downloaded once or twice a year is OK.

@kroese
Copy link

kroese commented Sep 6, 2023

@willcl-ark In my fork ( https://github.com/dobtc/docker-bitcoin/blob/master/Dockerfile ) I do the verification directly from the Dockerfile without the need for any Python runtime. Since it is only during the build-stage it does not matter much, but it seems a bit simpler.

Other differences that might be worth copying:

  • I updated it from Debian Buster to Debian Bookworm
  • I remove the test_bitcoin binary. This is 12 MB of space savings as it is not used at all.

@willcl-ark
Copy link
Author

@kroese updating to bookworm and removing test_bitcoin are good ideas, I missed those and will take them here.

Re the verify script, I was split trying to decide which was the better approach; do people prefer to "see" the bash script in the dockerfile (and do I want to reimplement the functionality in bash, which I now see you've done!) I opted to use the core script as it does a few useful things in one:

  • fetches the file, from multiple sources if possible
  • (hopefully) will change in a backwards-compatible way if the binary verification process changes in the future (lower maintenance)
  • allows specification of --min-good-sigs if that was wanted here

Would be happy to revert that commit and take your changes if that was preferable to this repo though? cc @ruimarinho

@kroese
Copy link

kroese commented Sep 6, 2023

I fully agree that your solution is better. The only reason I did it in bash is that in the original Dockerfile it was also done that way so I wanted to minimize the amount of changes.

@willcl-ark
Copy link
Author

I fully agree that your solution is better. The only reason I did it in bash is that in the original Dockerfile it was also done that way so I wanted to minimize the amount of changes.

Actually, I'm not 100% convinved on it mysel; I'm happy to go either way on it still. I was just detailing my thought process :)

As the tests are passing on my fork, I added a commit to lock builds to bookworm.

geekwho-eth pushed a commit to geekwho-eth/docker-bitcoin-core that referenced this pull request Sep 13, 2023
Add bitcoin-core@25
@andrewtoth
Copy link

Should this be updated to v25.1 now?

See: https://github.com/bitcoin/bitcoin/tree/master/contrib/verify-binaries

Currently, we due to the way the bitcoin core verify script parses the
version we must download multiple linux builds, then only use the the
target arch. Hopefully the verify script can be updated to handle single
platform (architecture:os) downloads, and if so in the future this can
be updated.

Switch to a two-stage build (like alpine), as we now download python
which bloats the image size.
It seems that alpine:latest updated gcc, which causes build errors on
old versions. Therefore use an old alpine image to build, and copy
needed runtime libraries across with the binaries.
These haven't been needed since v23.0
Fixing the version minimises these images breaking in the future, if
latest-stable included breaking changes.

-BEGIN VERIFY SCRIPT-
find . -type f -name "Dockerfile" | while read -r dockerfile; do
    sed -i 's|bullseye-slim|bookworm-slim|g' "$dockerfile"
    sed -i 's|stable-slim|bookworm-slim|g' "$dockerfile"
done
-END VERIFY SCRIPT-
@fanquake
Copy link
Contributor

fanquake commented Nov 6, 2023

@ruimarinho is there anything blocking this? Would be good to at least get approval for the CI to run.

@willcl-ark willcl-ark changed the title Add v25.0, update minor releases, use bitcoincore.org, verify using bitcoin core verify script, fixup alpine builds Add v25.1, update minor releases, use bitcoincore.org, verify using bitcoin core verify script, fixup alpine builds Nov 6, 2023
@willcl-ark
Copy link
Author

I'm running the CI against my own dockerhub repo over here: https://github.com/willcl-ark/docker-bitcoin-core/actions/runs/6769216413

@CharlieC3
Copy link

@ruimarinho Any attention this PR can get would be greatly appreciated. You're likely busy, but it seems like this would be an easy layup and having 25.1 available would help the Bitcoin community.

@kroese
Copy link

kroese commented Dec 11, 2023

@CharlieC3 Just 25.1? By now 26.0 is already available.

I forked this project a few months ago as it seems pretty abandoned. So if you are looking for an alternative, Im providing automated builds of every possible version (even release candidates) at: https://github.com/dobtc/bitcoin and https://hub.docker.com/r/dobtc/bitcoin/

@CharlieC3
Copy link

@kroese I wanted to keep the ask small :) And thanks! I'll check it out.

@zhelnov
Copy link

zhelnov commented Dec 13, 2023

#145 i added v26.0 based on this pull, does it makes sense?

@fanquake
Copy link
Contributor

fanquake commented Apr 3, 2024

26.1 is also now available.

@willcl-ark
Copy link
Author

26.1 is also now available.

Commit added to this branch with 26.1

@BogdanRS
Copy link

Do we have an ETA for these changes?

geekwho-eth added a commit to geekwho-eth/docker-bitcoin-core that referenced this pull request Jun 6, 2024
Add bitcoin-core@25
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.

7 participants