💬 chrisguida commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1909202976)
@DoctorBuzz1 you should make an issue/PR formally proposing your idea, I would be happy to test and review. This PR discussion is probably not the best place.

@ns-xvrn I'm also happy to help review and test any PR for #29285

@1440000bytes as was stated earlier, incremental progress is better than no progress. It sounds like you would only be happy with a perfect solution, which of course is impossible.

💬 luke-jr commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1465818605)
I dislike using `auto` both here and the default. It could conceivably not be a type that supports all permission masks.

💬 luke-jr commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1465819385)
Feels weird to log this unconditionally. Maybe the option should just be unavailable (and implicitly error) on Windows, and hard-code setting it user-read-only here?

💬 luke-jr commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1465819696)
Missing a space in the indentation

💬 luke-jr commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1465820310)
Feels like a hack

```suggestion
for perm in "440", "640", "444":
test_perm(perm)
```

📝 vostrnad opened a pull request: "Add a `-permitbarepubkey` option"
(https://github.com/bitcoin/bitcoin/pull/29309)
Fixes #29285 (adds the option but leaves the default policy unchanged)

No tests so far, if there's consensus to move forward with this I'll look into adding them.

Disclaimer: I don't care if this gets merged, personally I don't see any benefit of having this option (or `-permitbaremultisig`) and I would object to changing the default policy in this regard (similar to my objections to #28217). However, as I think someone would inevitably open a PR for this, I thought this might be a good op
...

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