💬 feed3r commented on issue "Master doesn't compile on MacOS X 10.13 High Sierra":
(https://github.com/bitcoin/bitcoin/issues/28206#issuecomment-1663735680)
Thank you all guys, yes this is not a bug and I was only looking for any common experience.
I had a solution anyway and will go with a release checking the compatibility in the release notes.
Thank you all again!
(https://github.com/bitcoin/bitcoin/issues/28206#issuecomment-1663735680)
Thank you all guys, yes this is not a bug and I was only looking for any common experience.
I had a solution anyway and will go with a release checking the compatibility in the release notes.
Thank you all again!
💬 josibake commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1283012394)
Note to self, this should be 1023
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1283012394)
Note to self, this should be 1023
🤔 glozow reviewed a pull request: "refactor: Make more transaction size variables signed"
(https://github.com/bitcoin/bitcoin/pull/28059#pullrequestreview-1560810508)
ACK 92de74ef181b42d774bc6b12329bc0c27caf0081
(https://github.com/bitcoin/bitcoin/pull/28059#pullrequestreview-1560810508)
ACK 92de74ef181b42d774bc6b12329bc0c27caf0081
🚀 glozow merged a pull request: "refactor: Make more transaction size variables signed"
(https://github.com/bitcoin/bitcoin/pull/28059)
(https://github.com/bitcoin/bitcoin/pull/28059)
💬 stickies-v commented on pull request "kernel: Prune leveldb headers":
(https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1283023840)
Yeah, unless any objections this would have my preference, less side effects and easier to understand imo:
<details>
<summary>git diff on 26fe348e6e05d9b50614fdcba9cfb7a711ae86dc</summary>
```diff
diff --git a/src/dbwrapper.h b/src/dbwrapper.h
index 54c4685c48..48a6c9af00 100644
--- a/src/dbwrapper.h
+++ b/src/dbwrapper.h
@@ -92,9 +92,6 @@ private:
const CDBWrapper &parent;
leveldb::WriteBatch batch;
- DataStream ssKey{};
- CDataStream ssValue;
-
size_t
...
(https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1283023840)
Yeah, unless any objections this would have my preference, less side effects and easier to understand imo:
<details>
<summary>git diff on 26fe348e6e05d9b50614fdcba9cfb7a711ae86dc</summary>
```diff
diff --git a/src/dbwrapper.h b/src/dbwrapper.h
index 54c4685c48..48a6c9af00 100644
--- a/src/dbwrapper.h
+++ b/src/dbwrapper.h
@@ -92,9 +92,6 @@ private:
const CDBWrapper &parent;
leveldb::WriteBatch batch;
- DataStream ssKey{};
- CDataStream ssValue;
-
size_t
...
📝 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