💬 hebasto commented on pull request "Avoid non-self-contained Windows header":
(https://github.com/bitcoin-core/gui/pull/789#issuecomment-1904383894)
> So, this makes me think there must be some compilation units where this somehow doesn't get defined?

I suppose, they might be the `qt*` stuff built by vcpkg.

📝 achow101 unlocked a pull request: "set `DEFAULT_PERMIT_BAREMULTISIG` to false"
(https://github.com/bitcoin/bitcoin/pull/28217)
The default activation of the `permitbaremultisig=0` option proposes an enhancement for the Bitcoin network. By refusing non-P2SH multisignature transactions from the outset, this modification would contribute to reducing spam attempts and maintaining a healthy decentralization by discouraging undesirable activities.

💬 freezoloto commented on issue "berkeley database failed to open database environment":
(https://github.com/bitcoin/bitcoin/issues/29286#issuecomment-1904409193)
Thank you my friend, I have already deleted my wallet and am re-uploading the blockchain :-)

💬 maflcko commented on pull request "depends: Do not override `CFLAGS` when building SQLite with `DEBUG=1`":
(https://github.com/bitcoin/bitcoin/pull/29287#discussion_r1462141265)
Why would the latter two be needed? Using only runtime checks/asserts would be more in line with the documentation. See: `DEBUG: Disable some optimizations and enable more runtime checking`. Also they are not supported for third-party code, see https://sqlite.org/debugging.html

Would be better to only use SQLITE_DEBUG? https://sqlite.org/compile.html#debug

💬 ryanofsky commented on pull request "depends: Update libmultiprocess library to fix C++20 macos build error":
(https://github.com/bitcoin/bitcoin/pull/29276#issuecomment-1904414778)
Updated 5dfd24581a7c5497601966a5371d8c33eabcee8a -> b8105b3ed7c97cd6545dba99d0e13c33f183e450 ([`pr/c14.1`](https://github.com/ryanofsky/bitcoin/commits/pr/c14.1) -> [`pr/c14.2`](https://github.com/ryanofsky/bitcoin/commits/pr/c14.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/c14.1..pr/c14.2)) just adding a new macos build fix: https://github.com/chaincodelabs/libmultiprocess/pull/93.

I'm pretty sure the new fix is not needed for any current depends builds because depends build
...

💬 brunoerg commented on pull request "fuzz: compare scripts from `Expand` and `ExpandFromCache`":
(https://github.com/bitcoin/bitcoin/pull/28908#issuecomment-1904416619)
Rebased

💬 brunoerg commented on pull request "test: add coverage for bech32m in `wallet_keypool_topup`":
(https://github.com/bitcoin/bitcoin/pull/28928#issuecomment-1904416959)
Rebased

💬 brunoerg commented on pull request "contrib: add test for bucketing with asmap":
(https://github.com/bitcoin/bitcoin/pull/28869#issuecomment-1904418492)
Rebased

💬 vasild commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#issuecomment-1904423201)
Ok, fine. Then, just a comments nit, the recently added comments "Per BIP159, safety buffer" and "BIP159 connections safety window" better be removed? Because that is not "A safety buffer of 144 blocks to handle chain reorganizations"?

💬 hebasto commented on pull request "depends: Do not override `CFLAGS` when building SQLite with `DEBUG=1`":
(https://github.com/bitcoin/bitcoin/pull/29287#discussion_r1462153876)
> Would be better to only use SQLITE_DEBUG?

Thanks! Done.

💬 theStack commented on pull request "rpc: parse legacy pubkeys consistently with specific error messages":
(https://github.com/bitcoin/bitcoin/pull/28336#issuecomment-1904438007)
Rebased on master (to fix CI).

💬 hebasto commented on pull request "Avoid non-self-contained Windows header":
(https://github.com/bitcoin-core/gui/pull/789#issuecomment-1904455841)
@theuni

You might be interested to consider the following example: https://godbolt.org/z/b8rns1e6x

💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#issuecomment-1904479665)
> In https://github.com/sipa/bitcoin/commits/pr29242 I pushed another commit which makes CompareFeerateDiagram not modify the diagrams in-place.

Taken only with minor comment changes, and added another test case or two to cover the iterative nature of the check

💬 hebasto commented on pull request "build: always set `-g -O2` in `CORE_CXXFLAGS`":
(https://github.com/bitcoin/bitcoin/pull/29205#issuecomment-1904504426)
While always overriding Autotools' defaults, are there any particular reasons not using [`-O3`](https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#index-O3) instead of `-O2`?

👍 hebasto approved a pull request: "build: always set `-g -O2` in `CORE_CXXFLAGS`"
(https://github.com/bitcoin/bitcoin/pull/29205#pullrequestreview-1837055782)
ACK 00c1e2aa4496b5f038ae5199dbd16d8313766533, I have reviewed the code and it looks OK. Also tested `ci/test/00_setup_env_native_valgrind.sh`.

✅ willcl-ark closed an issue: "berkeley database failed to open database environment"
(https://github.com/bitcoin/bitcoin/issues/29286)

💬 willcl-ark commented on issue "berkeley database failed to open database environment":
(https://github.com/bitcoin/bitcoin/issues/29286#issuecomment-1904555895)
OK great. I'll close this for now then.

Do open a new issue if it happens again and you confirm that the wallet database (directory) is present, but failing to load.

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1462248344)
I actually had this as `params=[]` a few updates ago but that fails lint (see below) but I'll clean it up with a different work-around:

```
A mutable list or dict seems to be used as default parameter value:

test/functional/interface_rpc.py:41:def expect_http_rpc_status(expected_http_status, expected_rpc_error_code, node, method, params={}):

This is how mutable list and dict default parameter values behave:

>>> def f(i, j=[], k={}):
... j.append(i)
... k[i] = True
...
...

💬 darosior commented on pull request "Add test for negative transaction version w/ CSV to tx_valid.json":
(https://github.com/bitcoin/bitcoin/pull/29291#issuecomment-1904569442)
ACK 97181decf5726aab6c5cd01b3e1964072f2531ff

💬 theStack commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#issuecomment-1904606796)
Concept ACK