💬 maflcko commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1878580405)
[clusterfuzz-testcase-tx_pool_standard-6515998551244800.bin.not.txt](https://github.com/bitcoin/bitcoin/files/13841869/clusterfuzz-testcase-tx_pool_standard-6515998551244800.bin.not.txt)
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1878580405)
[clusterfuzz-testcase-tx_pool_standard-6515998551244800.bin.not.txt](https://github.com/bitcoin/bitcoin/files/13841869/clusterfuzz-testcase-tx_pool_standard-6515998551244800.bin.not.txt)
✅ TheCharlatan closed a pull request: "mempool: Don't sort in entryAll"
(https://github.com/bitcoin/bitcoin/pull/29019)
(https://github.com/bitcoin/bitcoin/pull/29019)
💬 TheCharlatan commented on pull request "mempool: Don't sort in entryAll":
(https://github.com/bitcoin/bitcoin/pull/29019#issuecomment-1878590817)
Seems like the new behavior incidentally fixed a bug, so closing this again in favor of #29179. Could do a follow-up eventually to make the `entryAll` call when processing the `getrawmempool` RPC call more efficient by skipping the sorting.
(https://github.com/bitcoin/bitcoin/pull/29019#issuecomment-1878590817)
Seems like the new behavior incidentally fixed a bug, so closing this again in favor of #29179. Could do a follow-up eventually to make the `entryAll` call when processing the `getrawmempool` RPC call more efficient by skipping the sorting.
📝 glozow unlocked a pull request: "doc: revert clarify -datacarriersize"
(https://github.com/bitcoin/bitcoin/pull/29173)
The latest update of the help text of `-datacarriersize` is incorrect.
The purpose of this standardization rule is not to target only the data contained in the raw scriptPubKey, but all forms of arbitrary data.
It is incorrect to change the description of this option if an attempt to update it was made without being merged.
Context:
The [first inscription](https://mempool.space/tx/6fb976ab49dcec017f1e201e84395983204ae1a7c2abf7ced0a85d692e442799) appeared December 14, 2022 at the heig
...
(https://github.com/bitcoin/bitcoin/pull/29173)
The latest update of the help text of `-datacarriersize` is incorrect.
The purpose of this standardization rule is not to target only the data contained in the raw scriptPubKey, but all forms of arbitrary data.
It is incorrect to change the description of this option if an attempt to update it was made without being merged.
Context:
The [first inscription](https://mempool.space/tx/6fb976ab49dcec017f1e201e84395983204ae1a7c2abf7ced0a85d692e442799) appeared December 14, 2022 at the heig
...
💬 theuni commented on pull request "crypto: remove use of BUILD_BITCOIN_INTERNAL macro in sha256":
(https://github.com/bitcoin/bitcoin/pull/29180#issuecomment-1878604328)
Added a brief description of crypto_base.
(https://github.com/bitcoin/bitcoin/pull/29180#issuecomment-1878604328)
Added a brief description of crypto_base.
💬 brunoerg commented on pull request "fuzz: set `nMaxOutboundLimit` in connman target":
(https://github.com/bitcoin/bitcoin/pull/29172#discussion_r1442825960)
The `ConsumeBool` is to ensure that the default value will be used in most cases.
(https://github.com/bitcoin/bitcoin/pull/29172#discussion_r1442825960)
The `ConsumeBool` is to ensure that the default value will be used in most cases.
👍 hebasto approved a pull request: "doc: Clarify C++20 comments"
(https://github.com/bitcoin/bitcoin/pull/29042#pullrequestreview-1805901540)
ACK fa87f8feb76da42eeb5c4d32ee7be070b2bd559f, I verified the code with clang-{16,17}.
(https://github.com/bitcoin/bitcoin/pull/29042#pullrequestreview-1805901540)
ACK fa87f8feb76da42eeb5c4d32ee7be070b2bd559f, I verified the code with clang-{16,17}.
💬 brunoerg commented on pull request "fuzz: set `nMaxOutboundLimit` in connman target":
(https://github.com/bitcoin/bitcoin/pull/29172#discussion_r1442832240)
No objections on calling `CConnman::Init` instead. With `SetMaxOutboundLimit` we ensure we're only messing with `nMaxOutboundLimit` and perhaps we avoid other locks (?).
(https://github.com/bitcoin/bitcoin/pull/29172#discussion_r1442832240)
No objections on calling `CConnman::Init` instead. With `SetMaxOutboundLimit` we ensure we're only messing with `nMaxOutboundLimit` and perhaps we avoid other locks (?).
💬 jangorecki commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1878619928)
You missed the most important, censorship
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1878619928)
You missed the most important, censorship
💬 dergoegge commented on pull request "fuzz: set `nMaxOutboundLimit` in connman target":
(https://github.com/bitcoin/bitcoin/pull/29172#discussion_r1442916555)
`Init` is called in the connman constructor: https://github.com/bitcoin/bitcoin/blob/c80f57ba575af96890f185765a53a62ef58ef2c8/src/net.cpp#L3117-L3118
Calling it again and only changing `nMaxOutboundLimit` achieves the same as you have here with less new code.
(https://github.com/bitcoin/bitcoin/pull/29172#discussion_r1442916555)
`Init` is called in the connman constructor: https://github.com/bitcoin/bitcoin/blob/c80f57ba575af96890f185765a53a62ef58ef2c8/src/net.cpp#L3117-L3118
Calling it again and only changing `nMaxOutboundLimit` achieves the same as you have here with less new code.
💬 dergoegge commented on pull request "fuzz: set `nMaxOutboundLimit` in connman target":
(https://github.com/bitcoin/bitcoin/pull/29172#discussion_r1442914249)
> The ConsumeBool is to ensure that the default value will be used in most cases.
How does it ensure that and why is that useful?
(https://github.com/bitcoin/bitcoin/pull/29172#discussion_r1442914249)
> The ConsumeBool is to ensure that the default value will be used in most cases.
How does it ensure that and why is that useful?
💬 maflcko commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-1878712706)
Just tested https://github.com/bitcoin/bitcoin/pull/28657 on oss-fuzz:
```
AR libbitcoin_util.a
AR libbitcoin_consensus.a
CXXLD crypto/libbitcoin_crypto_base.la
CXXLD crypto/libbitcoin_crypto_sse41.la
CXXLD crypto/libbitcoin_crypto_avx2.la
CXXLD crypto/libbitcoin_crypto_x86_shani.la
AR libbitcoin_cli.a
CXXLD libunivalue.la
CXXLD crc32c/libcrc32c.la
CXXLD leveldb/libmemenv.la
AR minisketch/libminisketch.a
CXXLD
...
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-1878712706)
Just tested https://github.com/bitcoin/bitcoin/pull/28657 on oss-fuzz:
```
AR libbitcoin_util.a
AR libbitcoin_consensus.a
CXXLD crypto/libbitcoin_crypto_base.la
CXXLD crypto/libbitcoin_crypto_sse41.la
CXXLD crypto/libbitcoin_crypto_avx2.la
CXXLD crypto/libbitcoin_crypto_x86_shani.la
AR libbitcoin_cli.a
CXXLD libunivalue.la
CXXLD crc32c/libcrc32c.la
CXXLD leveldb/libmemenv.la
AR minisketch/libminisketch.a
CXXLD
...
📝 maflcko converted_to_draft a pull request: "build: Bump clang minimum supported version to 15"
(https://github.com/bitcoin/bitcoin/pull/29165)
Most supported operating systems ship with clang-15 (or later), so bump the minimum to that and allow new code to drop workarounds for previous clang bugs.
For reference:
* https://packages.debian.org/bookworm/clang-15
* https://packages.ubuntu.com/jammy/clang-15
* CentOS-like 8/9 Stream: All Clang versions from 15 to 17
* FreeBSD 12/13: All Clang versions from 15 to 16
* OpenSuse Tumbleweed ships with https://software.opensuse.org/package/clang (`clang17`); No idea about OpenSuse Leap
...
(https://github.com/bitcoin/bitcoin/pull/29165)
Most supported operating systems ship with clang-15 (or later), so bump the minimum to that and allow new code to drop workarounds for previous clang bugs.
For reference:
* https://packages.debian.org/bookworm/clang-15
* https://packages.ubuntu.com/jammy/clang-15
* CentOS-like 8/9 Stream: All Clang versions from 15 to 17
* FreeBSD 12/13: All Clang versions from 15 to 16
* OpenSuse Tumbleweed ships with https://software.opensuse.org/package/clang (`clang17`); No idea about OpenSuse Leap
...
💬 murchandamus commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1878746998)
Concept NACK
### Purpose of `datacarriersize`
`datacarriersize` refers to the payload of OP_RETURN outputs, the output type that was introduced as a data carrier to provide an alternative for more harmful ways of embedding data. At that time, there was a worry that any blockspace production in excess of the demand from monetary transactions would be filled for cheap since demand for highly replicated perpetual storage is unbounded. Bounding standard transactions to a single data carrier ou
...
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1878746998)
Concept NACK
### Purpose of `datacarriersize`
`datacarriersize` refers to the payload of OP_RETURN outputs, the output type that was introduced as a data carrier to provide an alternative for more harmful ways of embedding data. At that time, there was a worry that any blockspace production in excess of the demand from monetary transactions would be filled for cheap since demand for highly replicated perpetual storage is unbounded. Bounding standard transactions to a single data carrier ou
...
💬 cliobitcoinbank commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1878754502)
> > eragmus
>
> You are quite literally word for word repeating talking points made by BSV users who intend to harm Bitcoin by spamming the UTXO set and creating an unreasonably high fee environment in which the average end user is incentivised not to participate or if so forced into a custodial solution from which their Bitcoin fails to actually be a bearer instrument.
>
> https://www.youtube.com/watch?v=-RswZDQTqsI
>
> All the connections were made by a random pleb in this video, don'
...
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1878754502)
> > eragmus
>
> You are quite literally word for word repeating talking points made by BSV users who intend to harm Bitcoin by spamming the UTXO set and creating an unreasonably high fee environment in which the average end user is incentivised not to participate or if so forced into a custodial solution from which their Bitcoin fails to actually be a bearer instrument.
>
> https://www.youtube.com/watch?v=-RswZDQTqsI
>
> All the connections were made by a random pleb in this video, don'
...
💬 cliobitcoinbank commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1878760436)
> > "Bitcoin works on economic incentives"
>
> is not just some magical phrase you can throw around and make your assertions true. the incentives for full node runners are becoming smaller and smaller every single day. this demonstrably hurts decentralization of bitcoin. you people would care about that if you actually cared about the state of the mempool, but instead you are concern trolling that setting a filter to EVENLY DISTRIBUTE COST for all transactions will kill the mempool. Your fall
...
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1878760436)
> > "Bitcoin works on economic incentives"
>
> is not just some magical phrase you can throw around and make your assertions true. the incentives for full node runners are becoming smaller and smaller every single day. this demonstrably hurts decentralization of bitcoin. you people would care about that if you actually cared about the state of the mempool, but instead you are concern trolling that setting a filter to EVENLY DISTRIBUTE COST for all transactions will kill the mempool. Your fall
...
💬 pablomartin4btc commented on pull request "rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block":
(https://github.com/bitcoin/bitcoin/pull/26415#discussion_r1442950533)
Thought was due to #28438 & #28448... but just found #28890, nice! This PR required rebase anyways.
(https://github.com/bitcoin/bitcoin/pull/26415#discussion_r1442950533)
Thought was due to #28438 & #28448... but just found #28890, nice! This PR required rebase anyways.
💬 ajtowns commented on pull request "Change Luke Dashjr seed to dashjr-list-of-p2p-nodes-maybe-malware.us":
(https://github.com/bitcoin/bitcoin/pull/29145#issuecomment-1878797165)
> The DNS seeds resolve to IPs of random peers. It's entirely possible (and apparently reality) that some of those host malware.
It's entirely possible that one could be hosting a nascent superintelligence that's just escaped from a AI lab, but you're not adding "maybe-superintelligence" to the name. Even more likely for some of them to be chainanalysis spy nodes, or any number of other potentially worrying things.
FWIW, knots hasn't yet been updated to be malware adjacent: https://github.
...
(https://github.com/bitcoin/bitcoin/pull/29145#issuecomment-1878797165)
> The DNS seeds resolve to IPs of random peers. It's entirely possible (and apparently reality) that some of those host malware.
It's entirely possible that one could be hosting a nascent superintelligence that's just escaped from a AI lab, but you're not adding "maybe-superintelligence" to the name. Even more likely for some of them to be chainanalysis spy nodes, or any number of other potentially worrying things.
FWIW, knots hasn't yet been updated to be malware adjacent: https://github.
...
💬 pablomartin4btc commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1442990315)
I agree to move it to the `CConman::AddWhitelistPermissionFlags` (since the obj has both `whitelist_forcerelay` & `whitelist_relay` vars already set) and you could have something like `CConman:UpdateNetPermissionsFlags`with the logic below.
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1442990315)
I agree to move it to the `CConman::AddWhitelistPermissionFlags` (since the obj has both `whitelist_forcerelay` & `whitelist_relay` vars already set) and you could have something like `CConman:UpdateNetPermissionsFlags`with the logic below.
💬 ajtowns commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1878831217)
Seems pointless to argue the politics of the change when CI doesn't even pass:
```
tests.cpp:1570:35: error: use emplace_back instead of push_back [modernize-use-emplace,-warnings-as-errors]
test/script_tests.cpp(1492): error: in "script_tests/script_DataCarrierBytes": check 13 == (CScript() << zeros(11) << OP_DROP).DatacarrierBytes() has failed [13 != 0]
```
There are two different aspects of mempool policy: what you can configure the software to enforce, and the defaults. From that
...
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1878831217)
Seems pointless to argue the politics of the change when CI doesn't even pass:
```
tests.cpp:1570:35: error: use emplace_back instead of push_back [modernize-use-emplace,-warnings-as-errors]
test/script_tests.cpp(1492): error: in "script_tests/script_DataCarrierBytes": check 13 == (CScript() << zeros(11) << OP_DROP).DatacarrierBytes() has failed [13 != 0]
```
There are two different aspects of mempool policy: what you can configure the software to enforce, and the defaults. From that
...