💬 sr-gi commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1465595068)
Oh, I thought the `recvbuf` was a growing-only buffer that was not cleared until the handshake was over.

💬 1440000bytes commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1908992212)
> but I'm guessing that since [the utxoset has doubled in size over the last year](https://statoshi.info/d/000000009/unspent-transaction-output-set?orgId=1&refresh=10m&from=now-1y&to=now&viewPanel=8), these devices too will soon start to become unviable.

UTXO set has not doubled because of bare multisig: https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1666471945

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

we know it, unfortunately no mempool option for inscriptions is available at the moment, so even if Baremultisig is less used it remains spam for my node.

💬 furszy commented on pull request "init: settings, do not load auto-generated warning msg":
(https://github.com/bitcoin/bitcoin/pull/29301#discussion_r1465610725)
> In commit "init: settings, do not load auto-generated warning msg" ([987a1b5](https://github.com/bitcoin/bitcoin/commit/987a1b51eeb72c7fcb085470817a4fe85fcc3c7c))
>
> Not sure, but maybe this can be changed to std::string_view to avoid the need to allocate memory before main() runs.

As the settings map take strings for keys, wouldn't this change mean that we would need to static cast string_view to string to not create a copy during the push/erase methods call?

💬 chrisguida commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1909013326)
> > but I'm guessing that since [the utxoset has doubled in size over the last year](https://statoshi.info/d/000000009/unspent-transaction-output-set?orgId=1&refresh=10m&from=now-1y&to=now&viewPanel=8), these devices too will soon start to become unviable.
>
> UTXO set has not doubled because of bare multisig: [#28217 (comment)](https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1666471945)

Yes, I know. Just because there isn't a perfect solution for all utxoset abuse doesn't mean
...

👍 TheCharlatan approved a pull request: "set `DEFAULT_PERMIT_BAREMULTISIG` to false"
(https://github.com/bitcoin/bitcoin/pull/28217#pullrequestreview-1842509290)
ACK 8672721deb06e66854a085c9994e99c99160b8a1

Close to the only current usage of P2MS is data embedding and meta protocols, of which the majority is through the Counterparty protocol. I am not familiar with the Counterparty protocol, so I might have gotten some points here wrong. The [Bitcoin Stamps](https://github.com/mikeinspace/stamps/blob/main/BitcoinStamps.md), which are mentioned throughout this pull request discussion, are built with Counterparty transactions. P2MS is one of three metho
...

💬 furszy commented on issue "Prune Node Rescan Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/29183#issuecomment-1909019274)
> What's the process for building the blockfilterindex on an already-synced pruned node? Enable blockfilterindex, reindex?

Yes. For further node support, please come to IRC or use bitcoin.stackexchange. This is a project tracking issue.

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