💬 fanquake commented on pull request "[27.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/30305#discussion_r1650958819)
Ok. Lets follow up with ways we might want to streamline this for `28.x`.

👍 rkrux approved a pull request: "Wallet: Add `max_tx_weight` to transaction funding options (take 2)"
(https://github.com/bitcoin/bitcoin/pull/29523#pullrequestreview-2135369361)
Concept ACK [723a8da](https://github.com/bitcoin/bitcoin/pull/29523/commits/723a8da8e562451facc8daa5d6ba61977f68cafe), but I would look at [this](https://github.com/bitcoin/bitcoin/pull/29523/commits/723a8da8e562451facc8daa5d6ba61977f68cafe) commit a bit more again; `make, test/functional` are successful though.

Following transaction funding RPCs are affected with the addition of `max_tx_weight` option:
1. `fundrawtransaction`
2. `walletcreatefundedpsbt`
3. `send`

💬 rkrux commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1650773911)
As per the following 2 statements in `ChooseSelectionResult`:
- https://github.com/bitcoin/bitcoin/blob/master/src/wallet/spend.cpp#L699
- https://github.com/bitcoin/bitcoin/blob/master/src/wallet/spend.cpp#L709

The `no_input` and `change_output` weights are accounted for, but where is the weight of the destination output accounted for?

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