📝 darosior opened a pull request: "fuzz: a target for the block index database"
(https://github.com/bitcoin/bitcoin/pull/28209)
This introduces a small fuzz target for `CBlockTreeDB` which asserts a few invariants by using an in-memory LevelDb.
(https://github.com/bitcoin/bitcoin/pull/28209)
This introduces a small fuzz target for `CBlockTreeDB` which asserts a few invariants by using an in-memory LevelDb.
💬 TheCharlatan commented on pull request "kernel: Prune leveldb headers":
(https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1283055797)
Not sure about making this a static var. Now the allocations survive beyond the lifetime of the `CDBBatch` objects and I am not sure if this is entirely safe.
(https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1283055797)
Not sure about making this a static var. Now the allocations survive beyond the lifetime of the `CDBBatch` objects and I am not sure if this is entirely safe.
💬 ajtowns commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1283068149)
Updated. This also changes the initial value for `m_last_inv_sequence` to `1` instead of `0`. (Note that I reversed the order of the `if .. else` in `info_for_relay` so `GetSeq() < last` becomes `! (GetSeq() <= last)` which is `last < GetSeq()`)
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1283068149)
Updated. This also changes the initial value for `m_last_inv_sequence` to `1` instead of `0`. (Note that I reversed the order of the `if .. else` in `info_for_relay` so `GetSeq() < last` becomes `! (GetSeq() <= last)` which is `last < GetSeq()`)
💬 MarcoFalke commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1283055992)
not sure about adding test-only code to "real" code. What about moving this to the only fuzz test that needs it?
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1283055992)
not sure about adding test-only code to "real" code. What about moving this to the only fuzz test that needs it?
👍 MarcoFalke approved a pull request: "fuzz: a target for the block index database"
(https://github.com/bitcoin/bitcoin/pull/28209#pullrequestreview-1560871534)
Concept ACK, left two nits
(https://github.com/bitcoin/bitcoin/pull/28209#pullrequestreview-1560871534)
Concept ACK, left two nits
💬 MarcoFalke commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1283056551)
nit: remove newline?
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1283056551)
nit: remove newline?
💬 ajtowns commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#issuecomment-1663824558)
Update for glozow's review; fixes off-by-one error and adds some glozow's test coverage of reorg behaviour
(https://github.com/bitcoin/bitcoin/pull/27675#issuecomment-1663824558)
Update for glozow's review; fixes off-by-one error and adds some glozow's test coverage of reorg behaviour
💬 darosior commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1283080177)
Sure
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1283080177)
Sure
💬 darosior commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1283080569)
Oh, i forgot to push my last change, this include shouldn't be here (that's why i had separated it).
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1283080569)
Oh, i forgot to push my last change, this include shouldn't be here (that's why i had separated it).
💬 stickies-v commented on pull request "kernel: Prune leveldb headers":
(https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1283080816)
Good point. Okay, let's mark as resolved.
(https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1283080816)
Good point. Okay, let's mark as resolved.
💬 petertodd commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1663835792)
> Even assuming pinning was an issue, it wouldn't be a blocker. It could simply be released at the same time as some other policy change (package relay, #27677, v3 transactions, the next soft fork, etc). Or even just with sufficient heads-up.
I've sent an email to the bitcoin-dev mailing list announcing this proposed change: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2023-August/021840.html
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1663835792)
> Even assuming pinning was an issue, it wouldn't be a blocker. It could simply be released at the same time as some other policy change (package relay, #27677, v3 transactions, the next soft fork, etc). Or even just with sufficient heads-up.
I've sent an email to the bitcoin-dev mailing list announcing this proposed change: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2023-August/021840.html
💬 petertodd commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1663840167)
> Enacting this change of policy would be reason enough for me to stop updating Bitcoin Core forever. If anything node runners are asking for ways to filter the recent tidal wave of spam while it is still unconfirmed, not surrender to it.
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
...
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1663840167)
> Enacting this change of policy would be reason enough for me to stop updating Bitcoin Core forever. If anything node runners are asking for ways to filter the recent tidal wave of spam while it is still unconfirmed, not surrender to it.
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
...
💬 hebasto commented on pull request "refactor: Remove unused includes from wallet.cpp":
(https://github.com/bitcoin/bitcoin/pull/28200#discussion_r1283086785)
> I think this bug should be reported to IWYU...
https://github.com/include-what-you-use/include-what-you-use/issues/1280
(https://github.com/bitcoin/bitcoin/pull/28200#discussion_r1283086785)
> I think this bug should be reported to IWYU...
https://github.com/include-what-you-use/include-what-you-use/issues/1280
💬 darosior commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#issuecomment-1663841536)
Thanks, addressed your comments.
(https://github.com/bitcoin/bitcoin/pull/28209#issuecomment-1663841536)
Thanks, addressed your comments.
💬 dergoegge commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#issuecomment-1663851387)
Concept ACK
(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
...
(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.
(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.
(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.
(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)
(https://github.com/bitcoin/bitcoin/pull/28210#issuecomment-1663903532)
(Draft for now because CI will likely fail)