💬 dergoegge commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#issuecomment-1663851387)
Concept ACK

💬 MarcoFalke commented on pull request "refactor: serialization simplifications":
(https://github.com/bitcoin/bitcoin/pull/28203#issuecomment-1663857107)
only change is to add a missing `&`. lgtm, re-ACK f054bd072afb72d8dae7adc521ce15c13b236700 📦

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
t
...

💬 petertodd commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#discussion_r1283105646)
Thanks! Good idea; fixed.

💬 RobinLinus commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1663887625)
Concept NACK

This will most likely increase the spam problems significantly.

📝 MarcoFalke opened a pull request: "build: Bump minimum supported Clang to clang-13"
(https://github.com/bitcoin/bitcoin/pull/28210)
All supported operating systems ship with clang-13 (or later), so bump the minimum to that and remove now unused workarounds for previous clang bugs.

For reference:
* https://packages.debian.org/bullseye/clang-13
* https://packages.ubuntu.com/jammy/clang (`clang-14`) and https://packages.ubuntu.com/jammy/clang-15
* CentOS 8 Stream: All Clang versions from 11.0 to 15.0

This is for Bitcoin Core 27.0 in 2024 (next year), not the soon upcoming 26.0 next month.

💬 MarcoFalke commented on pull request "build: Bump minimum supported Clang to clang-13":
(https://github.com/bitcoin/bitcoin/pull/28210#issuecomment-1663903532)
(Draft for now because CI will likely fail)

📝 MarcoFalke opened a pull request: "Bump python minimum version to 3.9"
(https://github.com/bitcoin/bitcoin/pull/28211)
All supported operating systems ship with python 3.9 (or later), so bumping the minimum should not cause any issues. A bump will allow new code to use new python 3.9 features.

For reference:
* https://packages.debian.org/bullseye/python3
* https://packages.ubuntu.com/focal/python3.9
* CentOS Stream also ships with 3.9

This is for Bitcoin Core 27.0 in 2024 (next year), not the soon upcoming 26.0 next month.

💬 MarcoFalke commented on pull request "Bump python minimum version to 3.9":
(https://github.com/bitcoin/bitcoin/pull/28211#issuecomment-1663927124)
Currently a draft to see the CI result

💬 petertodd commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1663927559)
> Concept NACK
>
> This will most likely increase the spam problems significantly.

Since OP_Return outputs to not benefit from the segwit discount, publishing data via them is about 4x more expensive than publishing data in witness space. Eg via the taproot mechanism used by inscriptions.

Why do you believe that expanding a publication mechanism that is 4x more expensive than the heavily used taproot witness mechanism will "increase the spam problems significantly"?

💬 MarcoFalke commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#discussion_r1283165130)
Any reason to remove this option? Seems fine to keep it as an alias to completely disable any OP_RETURN. I know it may be redundant with the other option, but for some reason most other devs and users like the UX of it, similar to `-regtest` and `-chain=regtest`, or other options.

💬 TheCharlatan commented on pull request "kernel: Prune leveldb headers":
(https://github.com/bitcoin/bitcoin/pull/28186#issuecomment-1663941035)
Thank you for the re-review @MarcoFalke,

Updated 8c4481ed3713931247e4cedcb5733d3598050eb7 -> 8c4481ed3713931247e4cedcb5733d3598050eb7 ([cleaveLeveldbHeaders_2](https://github.com/TheCharlatan/bitcoin/tree/cleaveLeveldbHeaders_2) -> [cleaveLeveldbHeaders_3](https://github.com/TheCharlatan/bitcoin/tree/cleaveLeveldbHeaders_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/cleaveLeveldbHeaders_2..cleaveLeveldbHeaders_3))
* Addressed @MarcoFalke's [comment](https://github.com/bitcoi
...

👍 dergoegge approved a pull request: "fuzz: addrman, avoid `ConsumeDeserializable` when possible"
(https://github.com/bitcoin/bitcoin/pull/27918#pullrequestreview-1561046861)
Code review ACK 025fda0a76893d09d19ec9a6c0ba86ad11c466f7

💬 sp-marcel-hernandez commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1663944157)
> FYI if merged, this pull-req would allow you to continue to limit or block data-carrying OP_Return outputs by setting `-datacarriersize=n` or `-datacarriersize=0` respectively.

I understand that, it's a matter of trust. What I mean is that if the Bitcoin developers end up sanctioning such an obvious sabotage on the system I wouldn't trust them as a group to continue developing the reference implementation.

💬 1ma commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1663948260)
> FYI if merged, this pull-req would allow you to continue to limit or block data-carrying OP_Return outputs by setting `-datacarriersize=n` or `-datacarriersize=0` respectively. Of course, if a non-trivial minority of nodes and miners choose to relay and mine such transactions, you blocking them will have no impact as they will be propagated and mined anyway.

I understand that, it's a matter of trust. What I mean is that if the Bitcoin developers end up sanctioning such an obvious sabotage o
...

💬 ismaelsadeeq commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#discussion_r1283174480)
Okay, I was asking because I see a test for that in `mempool_compatibility.py`, probably that's why CI is failing, `mempool.dat` future versions are to be backwards incompatible then we can remove the `mempool_compatibility.py` test along with this?

💬 petertodd commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#discussion_r1283175570)
1) I don't think the redundancy is warranted going forwards.
2) Re: backwards compatibility, I expect that only a very small number of people have set `-datacarrier=0`, so I'm fine with those few people having to set `-datacarriersize=0`.

Though if there is strong demand, I could make `-datacarrier=0` act as an override on `-datacarriersize`, forcing the latter to zero.

💬 petertodd commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1663953595)
> I understand that, it's a matter of trust. What I mean is that if the Bitcoin developers end up sanctioning such an obvious sabotage on the system I won't trust them as a group to continue developing the reference implementation.

We still "sanction" even more "obvious sabotage" by the fact that bare multisig outputs are relayed by default. If you have a complaint, I'd suggest you start there rather than on OP_Return, a mechanism designed specifically for low impact and vigorously discussed
...

💬 MarcoFalke commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#discussion_r1283182371)
I think you also changed `-datacarriersize` to always be `+1`, so setting it to zero has a different meaning.

The cost seems low to keep the meaning and features the same, so personally I think it is fine to keep them the same.

In any case, release notes are needed
* for all removed features,
* for all changed meanings,
* for all lifted restrictions, and
* for all default value changes.

💬 eriknylund commented on issue "test: 999 of 999 multisig":
(https://github.com/bitcoin/bitcoin/issues/28179#issuecomment-1663965600)
@Sjors I've had a look at adding the tests mentioned in the issue by extending the tests in `wallet_taproot.py`. I'll open a PR - happy to get feedback on it.

💬 MarcoFalke commented on pull request "build: Bump minimum supported Clang to clang-13":
(https://github.com/bitcoin/bitcoin/pull/28210#issuecomment-1663968993)
This fails on `macOS 11.0`, according to CI. So I guess this can be closed for now.