💬 maflcko commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1415634415)
Funny that compilers compile this code

💬 furszy commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#issuecomment-1840824986)
> > Where did I claim that coin selection is well-documented?
>
> Sorry, my comment should say `s/well-documented/more documented/`

np. Happens on any convo.

I'm not that strong on the new logging introduction, still, sharing the change rationale and opening the convo helps moving forward.

✅ fanquake closed an issue: "build: multiprocess link issues on fedora aarch64"
(https://github.com/bitcoin/bitcoin/issues/26943)

💬 fanquake commented on issue "build: multiprocess link issues on fedora aarch64":
(https://github.com/bitcoin/bitcoin/issues/26943#issuecomment-1840826873)
Going to close, as this issue specifically should be fixed, but building on aarch64 is still broken until atleast #28846.

👍 hebasto approved a pull request: "depends: fix libmultiprocess build on aarch64"
(https://github.com/bitcoin/bitcoin/pull/28846#pullrequestreview-1765128120)
ACK 6293a3f5a2b61e4429c6016cc941e1ee3dc22eb3, tested on Fedora 37, aarch64.

💬 furszy commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1415650951)
Sure. Will add the following comment:
```
/*change_output_size=*/ 31, // unused value, use p2wpkh output size (wallet default change type)
/*change_spend_size=*/ 68, // unused value, use p2wpkh input size (high-r signature)
```

💬 fanquake commented on pull request "depends: fix libmultiprocess build on aarch64":
(https://github.com/bitcoin/bitcoin/pull/28846#issuecomment-1840838481)
> ACK https://github.com/bitcoin/bitcoin/commit/6293a3f5a2b61e4429c6016cc941e1ee3dc22eb3, tested on Fedora 37, aarch64.

The changes here don't link after #28856:
```bash
/usr/bin/ld: cannot find -lcapnp-rpc: No such file or directory
/usr/bin/ld: cannot find -lcapnp: No such file or directory
/usr/bin/ld: cannot find -lkj-async: No such file or directory
/usr/bin/ld: cannot find -lkj: No such file or directory
collect2: error: ld returned 1 exit status
```
because by switching to CMak
...

💬 furszy commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1415654787)
Pushed it. Thanks josi.

💬 Sjors commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1840847022)
Why did the Boost issue stop CI, but not the Guix build?

💬 fanquake commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1840848907)
Because the CI explicitly passes --enable-external-signer.

💬 Okponko commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1840853183)
![Uploading IMG_3704.jpeg…]()

💬 Okponko commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1840853720)
The Shop right here

💬 maflcko commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1415690508)
Clang has an `-Wunreachable-code`. Maybe someone can integrate this into the build system by default?

📝 0xB10C opened a pull request: "rpc: addpeeraddress tried return error on failure"
(https://github.com/bitcoin/bitcoin/pull/28998)
When trying to add an address to the IP address manager tried table, it's first added to the new table and then moved to the tried table. Previously, adding a conflicting address to the address manager's tried table with test-only `addpeeraddress tried=true` RPC would return `{ "success": true }`. However, the address would not be added to the tried table, but would remain in the new table. This caused, e.g., issue #28964.

This is fixed by new returning `{ "success": false, "error": "..." }`
...

💬 Sjors commented on pull request "Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1840891563)
The code changes for external signing look pretty minimal, so that's nice. I have no informed opinion on the difference between Boost Process and cpp-process. Except that the latter seems self contained.

> Which means it should be an entirely external dependency we just check for in configure…

In theory that makes sense to me as well, e.g. a fork under the bitcoin-core project, with functionality (mostly) limited to what we need. However in practice it means having to package it and get in
...

👍 ryanofsky approved a pull request: "depends: fix libmultiprocess build on aarch64"
(https://github.com/bitcoin/bitcoin/pull/28846#pullrequestreview-1765213631)
Code review ACK 6293a3f5a2b61e4429c6016cc941e1ee3dc22eb3. No changes since last review other than rebasing after #28856 merge

💬 0xB10C commented on issue "rpc_net.py intemittent failure / assert_equal(len(getrawaddrman[table_name]), len(table_info["entries"]))":
(https://github.com/bitcoin/bitcoin/issues/28964#issuecomment-1840892172)
Improving `addpeeraddress tried=true` failure behavior in #28998

💬 maflcko commented on pull request "rpc: addpeeraddress tried return error on failure":
(https://github.com/bitcoin/bitcoin/pull/28998#discussion_r1415701162)
nit: no need for those

```suggestion
"Add the address of a potential peer to an address manager table. This RPC is for testing only.",
```

💬 maflcko commented on pull request "rpc: addpeeraddress tried return error on failure":
(https://github.com/bitcoin/bitcoin/pull/28998#discussion_r1415708737)
Is a copy the right way, or does it need to be removed from the expected tried table?

Maybe add steps to reproduce the test code?

📝 maflcko opened a pull request: "build: Enable -Wunreachable-code"
(https://github.com/bitcoin/bitcoin/pull/28999)
It seems a bit confusing to write code after a `return`. This can even lead to bugs, or incorrect code, such as https://github.com/bitcoin/bitcoin/pull/28830/files#r1415372320 .

Fix all issues by enabling `-Wunreachable-code`.

This flag also enables `-Wunreachable-code-loop-increment`, according to https://clang.llvm.org/docs/DiagnosticsReference.html#wunreachable-code, so remove that.