💬 ns-xvrn commented on issue "./bitcoin.conf file should not cause confusion with ./datadir/bitcoin.conf":
(https://github.com/bitcoin/bitcoin/issues/29139#issuecomment-1909348928)
Somewhat unrelated but the `datadir` option in the `bitcoin.conf` is also a bit confusing. If you set it inside the default location of `bitcoin.conf` then it also ends up creating another `bitcoin.conf` inside the set datadir folder, which then confuses a user on which `bitcoin.conf` is used for the rest of the settings(I assume the one in the `datadir`).
I always end up specifying the `-datadir` with the `bitcoind`/`bitcoin-cli` commands. May be it's fine this way but just thought I should me
...

💬 ns-xvrn commented on pull request "Add a `-permitbarepubkey` option":
(https://github.com/bitcoin/bitcoin/pull/29309#issuecomment-1909384123)
Concept NACK, there's no point of adding this if default policy is with P2PK enabled so it certainly doesn't fix https://github.com/bitcoin/bitcoin/issues/29285.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1465874369)
> The reverse direction (P2PConnection actively does reconnections itself for outgoing connections) is not supported and I don't think it needs to be.

+1 since we only really care if TestNode/bitcoind (and not P2PConnection) does reconnection logic.

> I don't understand why in the existing v1 master code, the test framework, when receiving an inbound connection, sends out the version message in connection_made instead of doing it in on_version (similar to the way it is done in bitcoind, se
...

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1465875760)
done.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#issuecomment-1909407947)
> There are many more tests possible, right? E.g. sending more than 4095 bytes of garbage.
I assume the goal here wasn't to cover everything?

yes! goal here was to just introduce support for v2 in the test framework. we need more tests after this.

💬 Eunovo commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1465892947)
Understood. Thanks for clearing that up @josibake

⚠️ bayareaunicorn opened an issue: "Fido2 lock after 21 Million Mint "
(https://github.com/bitcoin/bitcoin/issues/29310)
### Please describe the feature you'd like to see added.

I would like to see the addition of Fido2 address lock for all addresses minted to prevent quantum attacks.

### Is your feature related to a problem, if so please describe it.

Technology advances in ai ect...

### Describe the solution you'd like

Fork for a newer quantum resist protocol

### Describe any alternatives you've considered

_No response_

### Please leave any additional context

_No response_

💬 S3RK commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1465966822)
nit: `lowest_larger` can't be above `max_input_weight`. You already checked it above.

I prefer if we filter above weight inputs once at the top of the function.

💬 vasild commented on pull request "util: check for errors after close and read in AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r1465997866)
I realized that `auto_file` used in this fuzz test is not associated with a regular file on the filesystem, but with a "cookie" returned by `fuzzed_file_provider.open()` returned by `fopencookie(3)` which uses a custom close function `FuzzedFileProvider::close()` which can arbitrarily return `-1`:

https://github.com/bitcoin/bitcoin/blob/207220ce8b767d8efdb5abf042ecf23d846ded73/src/test/fuzz/util.cpp#L352

changed this to ignore the return value of `auto_file.fclose()`.

💬 vasild commented on pull request "util: check for errors after close and read in AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r1466018964)
Alright, moved to the caller.

IMO this is a bugfix, but I am not 100% sure either. Might as well be viewed as "belt-and-suspenders". What is certain is that it can't hurt even if there is never an error on any platform if `detail_fread(dst) == dst.size()`. A unit test would require simulating an IO error under the `fread(3)` call and identically behaving `fread(3)` on all platforms.

💬 Retropex commented on pull request "Add a `-permitbarepubkey` option":
(https://github.com/bitcoin/bitcoin/pull/29309#issuecomment-1909680644)
> Concept NACK, there's no point of adding this if default policy is with P2PK enabled so it certainly doesn't fix #29285.

For me it's a good idea to already have code to do it, once merged we can discuss a default activation.

💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1466107824)
Changed to `fs::perms` here and the default in fcd100cb8e35e7cb00a58a697274a1f2cdb3cc75

💬 fanquake commented on pull request "fuzz: Exit and log stderr for parse_test_list errors":
(https://github.com/bitcoin/bitcoin/pull/29304#issuecomment-1909776685)
Looks like this is working as intended https://cirrus-ci.com/task/5482198250291200?logs=ci#L9646:
```bash
+ test/fuzz/test_runner.py -j6 -l DEBUG /ci_container_base/ci/scratch/qa-assets/fuzz_seed_corpus/ --empty_min_time=60
==29805==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x55ef0724925d (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0x93625d)
#1 0x55ef09320bb9 (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/
...

💬 fanquake commented on pull request "fuzz: Exit and log stderr for parse_test_list errors":
(https://github.com/bitcoin/bitcoin/pull/29304#issuecomment-1909777779)
We should also fix the symbolizing in that env.

💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1466110010)
The cookie should be read-only on Windows by deafult, so I've moved the logging up into `InitRPCAuthentication` [here](https://github.com/bitcoin/bitcoin/pull/28167/commits/fcd100cb8e35e7cb00a58a697274a1f2cdb3cc75#diff-a8ffd831b801d5170ec47bd2cde728ada0ffb3571f03ad70d93fc66c2009309aR269-R272), which now also returns an error if the user tries to set `rpccookieperms` on windows.

✅ fanquake closed an issue: "Fido2 lock after 21 Million Mint "
(https://github.com/bitcoin/bitcoin/issues/29310)

💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1466111038)
Thanks! My formatter is having some difficulty with diff/range-based formatting at the moment. Fixed.

💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1466112815)
IMO list comprehensions are fine, but seeing as I voided half the point of using one (creating a one-liner), so took the change to a regular for loop in 7516e9120e202395dabebd5b7212d9c2d3787c43

✅ willcl-ark closed an issue: "followups: Various potential improvements arising out of chainman-deglobalizing review"
(https://github.com/bitcoin/bitcoin/issues/21766)

💬 willcl-ark commented on issue "followups: Various potential improvements arising out of chainman-deglobalizing review":
(https://github.com/bitcoin/bitcoin/issues/21766#issuecomment-1909787062)
Going to close this tracking issue as all of the changes listed in OP are now merged.