bitcoindevkit / bdk

A modern, lightweight, descriptor-based wallet library written in Rust!
Other
871 stars 312 forks source link

ci: workflows failing due to "Connection timed out" errors #1162

Open notmandatory opened 1 year ago

notmandatory commented 1 year ago

Describe the bug

CI workflows are randomly failing due to "Connection timed out" errors when downloading the bitcoind binary from "https://bitcoincore.org/bin".

error: failed to run custom build command for `bitcoind v0.30.0`

Caused by:
  process didn't exit successfully: `/home/runner/work/bdk/bdk/target/debug/build/bitcoind-99a74cf3b9077890/build-script-build` (exit status: 101)
  --- stdout
  filename:bitcoin-22.0-x86_64-linux-gnu.tar.gz version:22.0 hash:59ebd25dd82a51638b7a6bb914586201e67db67b919b2a1ff08925a7936d1b16
  url:https://bitcoincore.org/bin//bitcoin-core-22.0/bitcoin-22.0-x86_64-linux-gnu.tar.gz

  --- stderr
  thread 'main' panicked at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bitcoind-0.30.0/build.rs:98:49:
  called `Result::unwrap()` on an `Err` value: IoError(Os { code: 110, kind: TimedOut, message: "Connection timed out" })
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Error: Process completed with exit code 101.

For example: https://github.com/bitcoindevkit/bdk/actions/runs/6459716661/job/17538503354?pr=1160

To Reproduce

Push a new PR for master or release/0.29 branch.

Expected behavior

There should be no random failures.

Build environment

Additional context

https://github.com/RCasatta/bitcoind/pull/144

realeinherjar commented 1 year ago

Assign me to me

realeinherjar commented 1 year ago

This is really strange. I am trying to reproduce locally with both clearnet and behind several VPNs with no avail. I think this was a problem in that probably was fixed now.

What do you guys think on using multiple hosts in

https://github.com/RCasatta/bitcoind/blob/602b86e0c0ffbafac6a83ef114c87185805db3f1/build.rs#L90-L98

as how bitcoin/bitcoin does?

https://github.com/bitcoin/bitcoin/blob/04265ba9378efbd4c35b33390b1e5cf246d420a9/contrib/verify-binaries/verify.py#L49-L50

This would be a PR in RCasatta/bitcoind with a procedure that would catch an error using the primary link and then try again with the secondary link.

Cc @evanlinjin and @notmandatory

EDIT: Don't know why the permalinks are not rendering... EDIT2: Don't know what is the threat model on the whole CW vs Kobra (https://www.coindesk.com/policy/2023/09/19/cobra-cant-fight-700k-craig-wright-legal-fees-as-an-anon-uk-judge-rules-again/) in using the domain

notmandatory commented 1 year ago

I've also not been able to reproduce this locally. I suspect it is something with github and bitcoincore.org. Could be that bitcoincore.org is rate limiting by ip subnet, and all the projects on github that are using bitcoind and electrsd crates are exceeding that limit. Could we create our own repo on github to store the binaries? Then we can use the env variable settings to point bitcoind / electrsd to that local URL.

realeinherjar commented 1 year ago

Yes, I see your argument now... We can create our own repo under bitcoindevkit/bitcoind (or something else). I am able to change the Test step to use the binaries in the repo in:

https://github.com/bitcoindevkit/bdk/blob/6ebdd195e29ffc23f21820f04128f7a56db4f93d/.github/workflows/cont_integration.yml#L50-L53

What do you think about that approach? We would need to have extra opsec with this repo, maybe hardcoding the SHA256 of the binary in the cont_integration.yml workflow and checking if the SHA matches. This would imply that we update both the binary repo but also the cont_integration.yml file for every new bitcoin core release.

LLFourn commented 1 year ago

I think the question should be why isn't our CI cache saving it. We should be caching our dependency compilation artifacts I would have thought.

RCasatta commented 1 year ago

Yeah, caching in CI would be the best course of action. Bitcoind crates also allows to specify BITCOIND_TARBALL_FILE env var and it will use that instead of downloading and it will also verify the hashes.

RCasatta commented 1 year ago

Or even better use nix (and cachix) like the MR I saw only now

realeinherjar commented 1 year ago

Or even better use nix (and cachix) like the MR I saw only now

Yes, I am working on that. Probably will be ready next week. I will ask you to review some things like the env variables for bitcoind and electrsd

notmandatory commented 1 year ago

As discussed on discord this isn't needed to complete the alpha.2 milestone. I'm moving this to the beta.0 release milestone for now.