💬 Ashflower13 commented on pull request "test: fix `keys_to_multisig_script` (P2MS) helper for n/k > 16":
(https://github.com/bitcoin/bitcoin/pull/28312#issuecomment-2044199439)
Ok help need guidance Sent from my iPhoneOn Apr 8, 2024, at 18:17, tdb3 ***@***.***> wrote:
@tdb3 commented on this pull request.

Great work catching the bug and making testing more robust.
ACK for d113709.
One inline recommendation to cover an additional related edge case.

In test/functional/rpc_createmultisig.py:
> @@ -118,6 +119,16 @@ def run_test(self):
# Check that bech32m is currently not allowed
assert_raises_rpc_error(-5, "createmultisig cannot create bech3
...

💬 naiyoma commented on pull request "net: update comment for service bit support info for seed.bitcoin.sipa.be":
(https://github.com/bitcoin/bitcoin/pull/29809#issuecomment-2044216151)
@Sjors and @laanwj Thank you for the review. I'll look into how https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/check-dnsseeds.py can be modified . I'm pretty new to this codebase, and I was trying to figure out who supports which service bits.

📝 naiyoma converted_to_draft a pull request: "net: update comment for service bit support info for seed.bitcoin.sipa.be"
(https://github.com/bitcoin/bitcoin/pull/29809)
This PR updates the comment regarding the supported service bits for `seed.bitcoin.sipa.be` based on this reference: https://github.com/sipa/bitcoin-seeder/blob/ff482e465ff84ea6fa276d858ccb7ef32e3355d3/main.cpp#L208

✅ brunoerg closed a pull request: "addrman: improve performance of `GetAddr` when specifying network"
(https://github.com/bitcoin/bitcoin/pull/29578)

💬 brunoerg commented on pull request "addrman: improve performance of `GetAddr` when specifying network":
(https://github.com/bitcoin/bitcoin/pull/29578#issuecomment-2044281593)
Closing for now.

🚀 fanquake merged a pull request: "depends: add new LLVM debug macro"
(https://github.com/bitcoin/bitcoin/pull/29781)

📝 fanquake opened a pull request: "Revert "ci: Temporarily disable bpfcc-tools""
(https://github.com/bitcoin/bitcoin/pull/29832)
This reverts commit fac012c7262f036e9b6f5800e57dcd63870a871c.

Closes #29804.

💬 laanwj commented on pull request "refactor: Optimize IsSpace function for common non-whitespace characters":
(https://github.com/bitcoin/bitcoin/pull/29602#issuecomment-2044334498)
Is this really a bottleneck? A benchmark would be useful to see if this isn't something the compiler already does.

ACK on the test.

💬 maflcko commented on pull request "depends: add new LLVM debug macro":
(https://github.com/bitcoin/bitcoin/pull/29781#issuecomment-2044347304)
I wonder why the CI runtime didn't change. Are the checks so cheap?

Before:

https://cirrus-ci.com/task/6133955961815040?logs=ci#L4016

```
ALL | ✓ Passed | 4916 s (accumulated)
```

This pull:

https://cirrus-ci.com/task/4514150150307840?logs=ci#L6071

```
ALL | ✓ Passed | 4906 s (accumulated)
```

My understanding is that the previous macro does not exist in llvm 18 (wh
...

🚀 fanquake merged a pull request: "test: Update --tmpdir doc string to say directory must not exist"
(https://github.com/bitcoin/bitcoin/pull/29498)

💬 maflcko commented on pull request "Revert "ci: Temporarily disable bpfcc-tools"":
(https://github.com/bitcoin/bitcoin/pull/29832#issuecomment-2044354811)
lgtm ACK c15170c27d96a66422cb86c6653c931aa204bbb0

on green CI

👍 hebasto approved a pull request: "Revert "ci: Temporarily disable bpfcc-tools""
(https://github.com/bitcoin/bitcoin/pull/29832#pullrequestreview-1988360200)
ACK c15170c27d96a66422cb86c6653c931aa204bbb0.

📝 brunoerg opened a pull request: "i2p: fix and improve logs"
(https://github.com/bitcoin/bitcoin/pull/29833)
This PR improves and fixes i2p logs (joint work with @vasild).

- It replaces `LogPrint` to `LogPrintLevel` so we can log according to the severity.
- Fix log when interruption happens during `Accept`. Before this PR, when an interruption happens, it just logs "Error accepting:", no reason is logged as it does for other situations. This PR changes it to log "Accept interrupted".
- Log errors according to the severity. Stuff like creating SAM session, destroying SAM session, etc... are logge
...

👋 fanquake's pull request is ready for review: "[27.x] More backports (and maybe finalize)"
(https://github.com/bitcoin/bitcoin/pull/29780)

🤔 maflcko reviewed a pull request: "tracing: Only prepare tracepoint arguments when actually tracing"
(https://github.com/bitcoin/bitcoin/pull/26593#pullrequestreview-1986597703)
left a nit/question

💬 maflcko commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#discussion_r1555929881)
```suggestion
#define TRACEPOINT(context, event, ...) \
do { \
if (TRACEPOINT_ACTIVE(context, event)) { \
STAP_PROBEV(context, event, __VA_ARGS__); \
```

question: Does the following not work in C++20?

💬 maflcko commented on pull request "validation: Use witness maleation flag for non-segwit blocks":
(https://github.com/bitcoin/bitcoin/pull/29540#issuecomment-2044401032)
-0. Not sure if this is worth it? While the race should have been fixed, are there any benefits that warrant the review?

💬 theuni commented on pull request "build: replace custom `MAC_OSX` macro with standard `__APPLE__` for consistent macOS detection":
(https://github.com/bitcoin/bitcoin/pull/29450#issuecomment-2044405345)
@paplorinc I agree with @fanquake that some of these are macos specific.

Would you be interested in opening a new PR for just the crypto changes? Otherwise I will.

💬 fanquake commented on pull request "Drop Windows Socket dependency for `randomenv.cpp`":
(https://github.com/bitcoin/bitcoin/pull/29786#discussion_r1557183407)
> I guess we should be doing this anyway at some point,

I will try follow up here with something more globally applicable.

👍 fanquake approved a pull request: "Drop Windows Socket dependency for `randomenv.cpp`"
(https://github.com/bitcoin/bitcoin/pull/29786#pullrequestreview-1988414244)
ACK 03b87a3e64305ba651e22a730e35271dea8fea64