💬 DoctorBuzz1 commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1909020625)
> UTXO set has not doubled because of bare multisig: [#28217 (comment)](https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1666471945)

UTXO has rapidly increased over the past year. It *started* with Inscriptions but is now primarily due to BRC-20 tokens. A static dust limit of 3 sat /vByte fees makes zero sense anymore, when those fees have only been seen like for a hot second since the spam started a year ago... https://statoshi.info/d/000000009/unspent-transaction-output-set?orgI
...

💬 ns-xvrn commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1909047847)
ACK 8672721deb06e66854a085c9994e99c99160b8a1

Tested locally, all unit and functional tests pass.

Transaction tested on mainnet using txid: `883b35fbd5a8b703574752b879a027ef81ae091b0bc61527309b0cf53760854f` (which worked fine without this change and confirmed using `getmempoolentry` and `getrawtransaction` after doing `sendrawtransaction` while it was still in the mempool). Output of `sendrawtransaction` after compiling and running this PR:
```
error code: -26
error message:
bare-multi
...

💬 1440000bytes commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1909048780)



Since DrahtBot has incorrectly assumed one of my previous comment as ` A-C-K`, this comment is to correct it:

NACK

Reason: https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1860915662

Apart from the reason shared in linked comment, other reasons why this pull request makes no sense:

1. Some nodes will keep using older versions of core and this is enough to relay bare mul
...

👍 ryanofsky approved a pull request: "validation: improve checkblockindex comments"
(https://github.com/bitcoin/bitcoin/pull/29299#pullrequestreview-1842689957)
Code review ACK 9819db4ccaa03519a78d4d9ecce9f89f5be669e5. Thanks for figuring this issue out and fixing it. Would suggest change pr name from "improve comments" to "fix misleading comments" since previous comments were wrong about the reasons the conditions are needed.

I also think it would be good to split up the assert and make it stricter, so feel free to use the code from https://github.com/bitcoin/bitcoin/issues/29261#issuecomment-1908513362 to use in the second commit, if that help. (Wo
...

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