👍 maflcko approved a pull request: "ci: add markdown link check job"
(https://github.com/bitcoin/bitcoin/pull/30034#pullrequestreview-2045256787)
utACK

💬 maflcko commented on pull request "ci: add markdown link check job":
(https://github.com/bitcoin/bitcoin/pull/30034#discussion_r1593795459)
nit: Any reason to modify the default here, or specify the default value?

💬 maflcko commented on pull request "ci: add markdown link check job":
(https://github.com/bitcoin/bitcoin/pull/30034#discussion_r1593798620)
nit: forgot to put this in args?

💬 willcl-ark commented on pull request "ci: add markdown link check job":
(https://github.com/bitcoin/bitcoin/pull/30034#discussion_r1593798691)
Just making it clear(er) that we are piping stdout to null vs printing stderr. But not really.

👋 willcl-ark's pull request is ready for review: "ci: add markdown link check job"
(https://github.com/bitcoin/bitcoin/pull/30034)

💬 maflcko commented on pull request "ci: Roll clang in test-each-commit task":
(https://github.com/bitcoin/bitcoin/pull/30060#issuecomment-2100256768)
This fails, because the image does not exist yet.

💬 maflcko commented on pull request "ci: add markdown link check job":
(https://github.com/bitcoin/bitcoin/pull/30034#discussion_r1593801135)
I'd say to keep stdout, unless there is a reason to suppress it?

💬 fanquake commented on pull request "ci: Roll clang in test-each-commit task":
(https://github.com/bitcoin/bitcoin/pull/30060#issuecomment-2100260070)
Yea, don't have to merge, but can do as soon as we have green CI. The image should be available very shortly?

💬 fanquake commented on pull request "build: swap cctools otool for llvm-objdump":
(https://github.com/bitcoin/bitcoin/pull/29739#issuecomment-2100263642)
Dropped the last commit just to make this simpler to review. The swap to objdump is the most complex of the cctools -> llvm swaps, because of the associated Python changes, all the of the rest are easier, and can be done in 1 go in a followup.

💬 maflcko commented on issue "ci: Enable bpfcc-tools":
(https://github.com/bitcoin/bitcoin/issues/29804#issuecomment-2100266431)
Not sure, the image is still missing, see also https://github.com/bitcoin/bitcoin/pull/30060#issuecomment-2100256768

💬 willcl-ark commented on pull request "ci: add markdown link check job":
(https://github.com/bitcoin/bitcoin/pull/30034#discussion_r1593808482)
Before I force push again, here are the two outputs, are you sure we want all that info?:

#### No suppressions (ignore my fail on stdout for my `.venv`):

<details>
<summary>Details</summary>

```log
₿ cargo run
Finished dev [unoptimized + debuginfo] target(s) in 0.00s
Running `target/debug/test_runner`
src/crc32c in HEAD currently refers to tree 454691a9b89ee8b9e1f71a48a7398edba49c3805
src/crc32c in HEAD was last updated in commit 5d45552fd4303f8d668ffbc50cce1053485aeead
...

💬 hebasto commented on issue "ci: Enable bpfcc-tools":
(https://github.com/bitcoin/bitcoin/issues/29804#issuecomment-2100269000)
> Not sure, the image is still missing, see also [#30060 (comment)](https://github.com/bitcoin/bitcoin/pull/30060#issuecomment-2100256768)

Right (

https://github.com/actions/runner-images/issues/9691#issuecomment-2099267078

💬 maflcko commented on pull request "ci: add markdown link check job":
(https://github.com/bitcoin/bitcoin/pull/30034#discussion_r1593823364)
Thanks for providing the context. (I didn't run the check locally, so I wasn't aware). I presume the reason for the suppression is that the output is too verbose.

The reason could be put in the code:

```suggestion
.stdout(Stdio::null()) // suppress verbose output
;
```

(up to you if you want to keep stderr as explicit inherit, or use the default inherit)

💬 maflcko commented on pull request "ci: add markdown link check job":
(https://github.com/bitcoin/bitcoin/pull/30034#issuecomment-2100286650)
utACK 9d2a8a8b1d67af14e4a5fc8276a1069b6ef9fe4a

with or without the nits

💬 willcl-ark commented on pull request "ci: add markdown link check job":
(https://github.com/bitcoin/bitcoin/pull/30034#issuecomment-2100291908)
Thanks @maflcko , did a final force-push for that silly nit: `git range-diff 9d2a8a8...82c5f95`

Sorry to invalidate the ACK

💬 maflcko commented on pull request "ci: add markdown link check job":
(https://github.com/bitcoin/bitcoin/pull/30034#issuecomment-2100296942)
utACK 82c5f9531021c76180fa34fa9cbd243ad1c994f3

Thanks!

💬 josibake commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#issuecomment-2100302186)
@theuni updated to remove `NO_LIMIT`. I also looked at a few alternatives to using an `Enum` but I think given the relatively simple usecase, an `Enum` works here with the benefit of not requiring an expansive refactor.

I'd be open to a more expansive refactor if there was a) conceptual buy in and b) additional use cases beyond supporting silent payments addresses (e.g. parsing bolt11/12 bech32 strings in Bitcoin Core, mercury layer bech32 addresses, etc).

⚠️ nimrare opened an issue: "libxcb-xinerama0 Library required by bitcoin-qt"
(https://github.com/bitcoin/bitcoin/issues/30061)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

I installed bitcoin core 26.1 on a Raspberry 5 (ARM Architecture) running Ubuntu 23.1. Upon running bitcoin-qt, I got the error that the library libxcb-xinerama0 is required but not installed.

It's easily fixed by installing it from the repositories (apt install libxcb-xinerama0), however, for a security conscious person this is a bit unsatisfying. Is there a reason this library is not
...

💬 hebasto commented on issue "libxcb-xinerama0 Library required by bitcoin-qt":
(https://github.com/bitcoin/bitcoin/issues/30061#issuecomment-2100340076)
> It's easily fixed by installing it from the repositories (apt install libxcb-xinerama0), however, for a security conscious person this is a bit unsatisfying.

It is expected: https://github.com/bitcoin/bitcoin/blob/43a66c55ec8770cf7c21112aac9b997f3f2fb704/contrib/devtools/symbol-check.py#L125

🤔 rkrux reviewed a pull request: "Test/rpc whitelistdefault test"
(https://github.com/bitcoin/bitcoin/pull/29858#pullrequestreview-2045275412)
Thanks for adding these tests.

Seems like this is in progress because there is test failure and a debugger is added.
Added few suggestions for now, will review again when the tests pass.