Bitcoin Core Github
44 subscribers
121K links
Download Telegram
⚠️ ismaelsadeeq opened an issue: "25.0 RC Testing Guide Feedback"
(https://github.com/bitcoin/bitcoin/issues/27736)
This issue is to discuss the [25.0 Release Candidate Testing Guide](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/25.0-Release-Candidate-Testing-Guide). If you have any issues with or feedback on the document, please leave a comment here.

Note: this is for feedback on the document, not on Bitcoin Core or on the 23.0 changes. Please see the [#27621](https://github.com/bitcoin/bitcoin/issues/27621) for instructions on how to report bug/results.

Thank you for your feedback
💬 ismaelsadeeq commented on issue "v25.0 testing":
(https://github.com/bitcoin/bitcoin/issues/27621#issuecomment-1560749074)
@chippsmith Thanks for your testing effort.
@schjonhaug Thank You for your feedback, I will update the document.
For all feedback on the testing guide document please drop it here [#27736](https://github.com/bitcoin/bitcoin/issues/27736)
💬 Sjors commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#discussion_r1203781715)
Done
💬 fanquake commented on pull request "ci: standardize custom libc++ usage in MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27485#issuecomment-1560813759)
Going to open a new PR with more extensive changes.
fanquake closed a pull request: "ci: standardize custom libc++ usage in MSAN jobs"
(https://github.com/bitcoin/bitcoin/pull/27485)
💬 willcl-ark commented on pull request "Prevent file descriptor exhaustion from too many RPC calls":
(https://github.com/bitcoin/bitcoin/pull/27731#discussion_r1203840774)
I am not sure this actually limits us within the file descriptor limit. For example if you run:

```bash
$ ulimit -n 175
$ src/bitcoind
Warning: Reducing -maxconnections from 125 to 15, because of system limitations.
Bitcoin Core starting
```

I think (but have not tested) that if I then recieve 15 inbound connections I would could still run out of file descriptors for the http server?

Perhaps if we also modify this line:

https://github.com/bitcoin/bitcoin/blob/51c050787fd6bcd0169
...
📝 fanquake opened a pull request: "ci: compile Clang and compiler-rt in msan jobs"
(https://github.com/bitcoin/bitcoin/pull/27737)
This reworks the MSAN CIs, to first compile Clang and compiler-rt (using GCC 12), and then, compile an MSAN instrumented libc++ using the just-built Clang 16. This fixes the `native_fuzz_with_msan` job, working around https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1005341, by not using the Debian provided Clang/LLVM.

Also included are changes to streamline how we use our "custom libc++", according to upstream: https://releases.llvm.org/16.0.0/projects/libcxx/docs/UsingLibcxx.html#using-a-c
...
💬 fanquake commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1560854361)
> Again, this is unrelated, because the issue happens in master. See also https://cirrus-ci.com/task/4517884630663168?logs=ci#L4910

Should be fixed in #27737.
💬 russeree commented on pull request "rpc: Fix invalid bech32 address handling":
(https://github.com/bitcoin/bitcoin/pull/27727#issuecomment-1560857791)
Very cool! I am not in a position to ack,nack. I have noticed a potential grammatical issue. The term 'byte' is correctly used only when the size of the program/data is 1. In all other cases, including 0, 2, 3, 4, ..., the correct term would be 'bytes'.

For instance, in the error string 'Invalid Bech32 v0 address program size (16 byte), per BIP141', 'byte' should be replaced with 'bytes' as the size is 16."
💬 fanquake commented on pull request "25.0 Final Changes":
(https://github.com/bitcoin/bitcoin/pull/27686#issuecomment-1560861380)
> Release notes for the new scanblocks RPC (https://github.com/bitcoin/bitcoin/pull/23549) are currently missing,
💬 fanquake commented on pull request "25.0 Final Changes":
(https://github.com/bitcoin/bitcoin/pull/27686#issuecomment-1560862446)
> Release notes for the new scanblocks RPC (https://github.com/bitcoin/bitcoin/pull/23549) are currently missing,

Pulled them in, and deleted the wiki page.
💬 theStack commented on issue "25.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/27736#issuecomment-1560893497)
Thanks for creating this testing guide! I didn't have a chance to look at any of the detailled steps yet, but have a suggestion to add two other features to test. v25.0 will be the first release where Bitcoin Core takes internal use of the compact block filters (BIP158, enabled via `-blockfilterindex=1`), rather than only serving them to peers. There is on one hand a new RPC call `scanblocks` [1] which looks for blocks containing a certain set of passed descriptors by using block filters, and on
...
💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#issuecomment-1560904235)
The first two commits are wonderful refactors. But cce96182ba2457335868c65dc16b081c3dee32ee breaks `p2p_compactblocks.py --valgrind` for me:

```
023-05-24T10:54:53.143000Z TestFramework (INFO): Testing reconstructing compact blocks from all peers...
2023-05-24T10:54:53.304000Z TestFramework (ERROR): Assertion failed
...
File "/home/sjors/.pyenv/versions/3.8.16/lib/python3.8/http/client.py", line 972, in send
self.sock.sendall(data)
BrokenPipeError: [Errno 32] Broken pipe
```

O
...
💬 MarcoFalke commented on pull request "ci: compile Clang and compiler-rt in msan jobs":
(https://github.com/bitcoin/bitcoin/pull/27737#issuecomment-1560912072)
No idea about the `bash` error. Maybe `set -ex` is missing in the file?
💬 fanquake commented on pull request "ci: compile Clang and compiler-rt in msan jobs":
(https://github.com/bitcoin/bitcoin/pull/27737#issuecomment-1560914159)
> No idea about the bash error. Maybe set -ex is missing in the file?

Have a change to just remove `cd` usage entirely, that should make the linter happy
🤔 MarcoFalke reviewed a pull request: "ci: compile Clang and compiler-rt in msan jobs"
(https://github.com/bitcoin/bitcoin/pull/27737#pullrequestreview-1441557570)
Do you happen to know what the difference to the debian package is? Maybe it can be fixed upstream instead?
💬 MarcoFalke commented on pull request "ci: compile Clang and compiler-rt in msan jobs":
(https://github.com/bitcoin/bitcoin/pull/27737#discussion_r1203908972)
```suggestion
export CI_IMAGE_NAME_TAG="ubuntu:22.04"
```

nit: Can use LTS if you don't need clang-16?
💬 MarcoFalke commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#issuecomment-1560936457)
I can't reproduce. Can you share the combined log or steps to reproduce starting on a fresh install of the operating system? Otherwise there is nothing we can do.
💬 MarcoFalke commented on pull request "rpc: Fix invalid bech32 address handling":
(https://github.com/bitcoin/bitcoin/pull/27727#issuecomment-1560940532)
@russeree Thx, I think it is fine if you open a follow-up after merge.