💬 hebasto commented on pull request "depends: Build the `native_capnp` and `capnp` packages with CMake":
(https://github.com/bitcoin/bitcoin/pull/28856#issuecomment-1840695572)
> > The PR description has been updated to mention one more fixed bug.
>
> Is this #28993, which turned out to not be a bug, or is this a different bug?

A different one, which is outdated `config.guess` and `config.sub` files in the `capnp` package.

> Because that conclusion from #28993 was that cross-compilation for Darwin, using master, does work?

Right. Cross-compiling for `x86_64-apple-darwin` works indeed.

> If so, what is this PR fixing in regards to cross-compiling for `ar
...

💬 furszy commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1415543551)
31 -> p2wpkh output size.
68 -> p2wpkh input size (high-r signature).

The previous test version didn't have any hardcoded value. However, since we are now calling a low-level function, we need to hardcode them. Setting them to 0 would be incorrect. Even though they are not currently in use on this process, these values must align with those used in the wallet. We learned this lesson the hard way; the `coinselector_tests` is full of invalid values that lack real meaning, affecting the introdu
...

💬 josibake commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1415562873)
Cool, agree with using real-world values, but I think it's worth documenting why those specific values were chosen in a comment since the choice of values isn't relevant to the test.

💬 furszy commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#issuecomment-1840765663)
> I am uncomfortable with adding the log message as it has nothing to do with the bug fix and I think it could confuse (or worse, alarm) users to see "Waste" in their logs in the context of coin selection when waste is a not well-documented, implementation-specific term. Can we drop this commit for now and revisit debug logging in a separate PR, unrelated to the v26 release?

Logs aren't meant for end users; logs are useful for us to understand what is going on internally. Debug issues and pro
...

💬 josibake commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#issuecomment-1840778172)
> Logs aren't meant for end users

Strongly disagree with this statement, and regardless of who they are intended for, users (people who run nodes) have access to these logs and do look at them.

> This is not different to what we do for the [fee calculation](https://github.com/bitcoin/bitcoin/blob/b3ab0c3819cf36e902989e0544365807ac0823e6/src/wallet/spend.cpp#L1263). And there is more documentation about the selection waste than doc about each of the logged [fee reasons](https://github.com/b
...

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-1840779612)
@martinus something similar was mentioned in https://github.com/bitcoin/bitcoin/pull/15265#issuecomment-457720636. However, with your approach every access would require 4 lookups.

💬 hebasto commented on issue "`capnp` fails when cross-compiling":
(https://github.com/bitcoin/bitcoin/issues/28993#issuecomment-1840785721)
> That makes more sense. Thanks for the bug report in any case!

I was careless and didn't run `make distclean`.

💬 furszy commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#issuecomment-1840791636)
> This is irrelevant to my point that this is not a bug fix and shouldn't be included in a bug fix PR. I also don't agree with the justification and claim that coin selection is well-documented.

Where did I claim that coin selection is well-documented? I said that it is more documented than fee estimation. And fee estimation is being logged.

> Strongly disagree with this statement, and regardless of who they are intended for, users (people who run nodes) have access to these logs and do lo
...

🚀 fanquake merged a pull request: "depends: Build the `native_capnp` and `capnp` packages with CMake"
(https://github.com/bitcoin/bitcoin/pull/28856)

💬 fanquake commented on pull request "depends: fix libmultiprocess build on aarch64":
(https://github.com/bitcoin/bitcoin/pull/28846#issuecomment-1840801375)
Rebased after #28856.

🤔 stickies-v reviewed a pull request: "[refactor] Check CTxMemPool options in ctor"
(https://github.com/bitcoin/bitcoin/pull/28830#pullrequestreview-1764747210)
Concept ACK

💬 stickies-v commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1415619542)
I think this approach is more unsafe than it has to be, it's pretty easy to not handle the `error`. Would it make more sense to make the ctor private and add a `util::Result<CTxMemPool> Make(const Options& opts)` factory method?

💬 stickies-v commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1415372320)
I don't think this actually does anything? (+[here](https://github.com/bitcoin/bitcoin/pull/28830/files#diff-07fe07ecfff4dd7c233a453006567140ff20d226fd73a20406df14982b1e415bR132))

💬 josibake commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#issuecomment-1840805224)
> Where did I claim that coin selection is well-documented?

Sorry, my comment should say `s/well-documented/more documented/`

💬 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
...