💬 rkrux commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1650938860)
Was the intention to write "In order to ..."?

💬 rkrux commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1650867991)
> This change ensures consistency in transaction size and weight calculation
within the wallet and prevents conversion overflow when calculating
`max_selection_weight`.

Are the inconsistencies because size_t is platform dependent and int is not (at least for the purposes of core because of this: https://github.com/bitcoin/bitcoin/blob/v26.0/src/compat/assumptions.h#L44)?
I still need to digest this comment: https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1528745048

💬 rkrux commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1650862134)
Took me a while to figure but as I see they are accounted for within `coin_selection_params.tx_noinputs_size` here: https://github.com/bitcoin/bitcoin/blob/master/src/wallet/spend.cpp#L1100

💬 rkrux commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1650936364)
Nit: In case the file is changed.

```
# ensure the transaction's weight is below the specified max_tx_weight.
```

🚀 fanquake merged a pull request: "[27.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/30305)

🚀 fanquake merged a pull request: "[26.x] upnp: fix build with miniupnpc 2.2.8"
(https://github.com/bitcoin/bitcoin/pull/30319)

💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1650961956)
Dropped the new constant leaving the current behaviour as default, unless an optional `fs::perms` is provided in 7ad5349a2ec

💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1650962883)
Thanks, i took this simplification in 7ad5349a2ec

💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1650962320)
Taken in 7ad5349a2ec

💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1650960724)
Thanks, now done in 7ad5349a2ec

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1650969447)
I have addressed this by making `StartScheduledTasks` not use `m_rng`, but just grab a new `FastRandomContext` instead; performance doesn't matter for this function.

👍 brunoerg approved a pull request: "contrib: asmap-tool - Compare ASMaps with respect to specific addresses"
(https://github.com/bitcoin/bitcoin/pull/30246#pullrequestreview-2135729296)
ACK 42eb7b144c09f0404487f2f5d17ca1a3e91c9365

🚀 fanquake merged a pull request: "ci: add option for running tests without volume"
(https://github.com/bitcoin/bitcoin/pull/30310)

👍 hebasto approved a pull request: "build: add `-Wundef`"
(https://github.com/bitcoin/bitcoin/pull/29876#pullrequestreview-2135788139)
ACK e3dc64f4990a15df3fd6147831f66fc2a31c71ad, I have reviewed the code and it looks OK.

💬 furszy commented on pull request "wallet: notify when preset + automatic inputs exceed max weight":
(https://github.com/bitcoin/bitcoin/pull/30309#discussion_r1651026133)
> Any specific reason to generate 1472 outputs when 1471 are picked?

All are picked. The extra output is to cover the fee.

📝 hebasto opened a pull request: "build: Drop redundant `sys/sysctl.h` header check"
(https://github.com/bitcoin/bitcoin/pull/30327)
The `AC_CHECK_HEADERS` macro defines `HAVE_SYS_SYSCTL_H` if the `sys/sysctl.h` header is found. However, in the source code, this header is guarded by `HAVE_SYSCTL` and `HAVE_SYSCTL_ARND` macros, which have their own checks. Since `HAVE_SYS_SYSCTL_H` is not used, we can skip the `AC_CHECK_HEADERS(... sys/sysctl.h ...)` check.

💬 willcl-ark commented on issue "Porting bcc tools to libbpf":
(https://github.com/bitcoin/bitcoin/issues/30298#issuecomment-2186613731)
@rob-scheepens Thanks for opening this up for discussion again after the developments in `libbpf`.

Is this something that you plan on implementing yourself and wanted to brainstorm on first?

If not (and @0xB10C is not interested in implementing currently due to lack of review) then I'm not sure that keeping this issue open longer than the above discussion is useful and might recommend closing it again until developer interest in this feature picks up...

🤔 fjahr reviewed a pull request: "contrib: asmap-tool - Compare ASMaps with respect to specific addresses"
(https://github.com/bitcoin/bitcoin/pull/30246#pullrequestreview-2135834115)
Looks good to me, just some additional documentation on documentation on the addrs input file would be good to avoid people getting strange results.

💬 fjahr commented on pull request "contrib: asmap-tool - Compare ASMaps with respect to specific addresses":
(https://github.com/bitcoin/bitcoin/pull/30246#discussion_r1651050889)
nit: sys isn't in the right alphabetical order here. You could reorder it while you are at it.

💬 fjahr commented on pull request "contrib: asmap-tool - Compare ASMaps with respect to specific addresses":
(https://github.com/bitcoin/bitcoin/pull/30246#discussion_r1651055643)
You should name the specific settings to run getnodeaddresses with so that the result is as expected. I guess that would be 0 and all (default) right? I fear that if people do it wrong they could get a result like "nothing was reassigned" and then think: oh great, the internet is so stable nowadays ;)