Bitcoin Core Github
43 subscribers
122K links
Download Telegram
👍 theuni approved a pull request: "net: remove unnecessary check from AlreadyConnectedToAddress()"
(https://github.com/bitcoin/bitcoin/pull/32338#pullrequestreview-2799968661)
utACK f1b142856a4ecd0a0d90bc3d73ef5137997b14ff

As I mentioned above, I believe this was needed at one point when connecting to an IP through a proxy, or when using addnode for an IP, as those were stored as strings rather than resolved addresses.

But now that we always attempt to resolve ips, `m_addr_name` is only interesting for non-ip connections. So the check here should be redundant and unnecessary.
💬 rstmsn commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2835842668)
Concept NACK.

- Between blocks 877325 (01 Jan 2025) and 894289 (today) there's 30 non-std OP_RETURN txs out of 7 million, or 0.004246903% of the total.

- That's a 99.995753097% success rate for the 83 byte limit spam filter in 2025.
💬 hebasto commented on issue "build: CMake caching failing PIE check":
(https://github.com/bitcoin/bitcoin/issues/32323#issuecomment-2835842776)
After further consideration, I believe this issue exposes a bug that is fixed in https://github.com/bitcoin/bitcoin/pull/32367.
📝 hebasto converted_to_draft a pull request: "cmake: Check user-defined `APPEND_*FLAGS` variables early"
(https://github.com/bitcoin/bitcoin/pull/32367)
For the purpose of checks performed by the build system, we strive to handle user-defined `APPEND_*FLAGS` in the same way as flags provided by other standard means. In particular, they are considered when the `try_compile()` command is used.

However, these flags are not considered during `enable_language()` command invocation due to design limitations, which might cause issues, such as https://github.com/bitcoin/bitcoin/issues/32323.

This PR addresses the issue by introducing an additional
...
👋 hebasto's pull request is ready for review: "cmake: Check user-defined `APPEND_*FLAGS` variables early"
(https://github.com/bitcoin/bitcoin/pull/32367)
💬 Sjors commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2835862040)
@rstmsn that's not a sensible metric of "success", which you haven't defined either. If I were to define it as "percentage of attempted OP_RETURN transactions included in a block", and if I then consider the lack evidence of a single attempt, the success rate is 100%. The fact that one can find such wildly different metrics and definitions, implies there's all useless.
💬 laanwj commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#discussion_r2064099087)
Thanks, yes, that would be betterr.
💬 wizkid057 commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2835923552)
> @rstmsn that's not a sensible metric of "success", which you haven't defined either. If I were to define "success" as "percentage of attempted OP_RETURN transactions permanently excluded from the blockchain", and if I then consider the lack of evidence for a single failed attempt, the success rate is 0%. The fact that one can find such wildly different metrics and definitions, implies they're all useless.
>
> (updated to be more in line with your sense that more filtering means more success
...
🤔 brunoerg reviewed a pull request: "descriptors: Reject + sign while parsing unsigned"
(https://github.com/bitcoin/bitcoin/pull/32365#pullrequestreview-2800083002)
Concept ACK
💬 theuni commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2064139694)
> Hmm, in `CConnman::OpenNetworkConnection()` the code does:
>
> * if `pszDest` is not set then use `AlreadyConnectedToAddress(addrConnect)` which ignores the port
>
> * if `pszDest` is set then `OutboundConnectedToStr(pszDest)` which compares the port as well.
>
>
> Changing that logic is, I guess, out of the scope of this non-functional refactor PR. If to be done it would better be assessed in isolation in its own PR.

Also, later on in `ConnectNode()`:

```c++
if (
...
tomasandroil closed a pull request: "Fix missing error check in `set_clo_on_exec` for FD_CLOEXEC handling"
(https://github.com/bitcoin/bitcoin/pull/32342)
💬 tomasandroil commented on pull request "Fix missing error check in `set_clo_on_exec` for FD_CLOEXEC handling":
(https://github.com/bitcoin/bitcoin/pull/32342#issuecomment-2835987967)
Closing as requested — changes were upstreamed and backported in #32358. Thanks everyone
💬 laanwj commented on pull request "common: Close non-std fds before exec in RunCommandJSON":
(https://github.com/bitcoin/bitcoin/pull/32343#issuecomment-2835990608)
[3acd934311477ac88e1c3176aaeaec2b3ad35425 → bdf634918c32d3c0da2c2262252470d1755bb085](https://github.com/bitcoin/bitcoin/compare/3acd934311477ac88e1c3176aaeaec2b3ad35425..bdf634918c32d3c0da2c2262252470d1755bb085)
- Use ToIntegral instead of ParseInt
💬 theStack commented on pull request "subprocess: Backport upstream changes":
(https://github.com/bitcoin/bitcoin/pull/32358#issuecomment-2835996184)
Concept ACK
💬 D33r-Gee commented on pull request "interface: Expose load utxo snapshot functionality":
(https://github.com/bitcoin-core/gui/pull/869#issuecomment-2836004928)
> can you show this works with some sort of POC on the bitcoin-core/gui side?

Great idea! Just updated the description with a link to a POC and screenshots...
💬 brunoerg commented on pull request "fuzz: doc: add info about `afl-system-config` for macOS":
(https://github.com/bitcoin/bitcoin/pull/32175#issuecomment-2836044421)
Pushed 66572c27454e1464173dc318d62fdfc11d4b7832..6e026606f368d8d1139b266c382076685e76d0b2 to address https://github.com/bitcoin/bitcoin/pull/32175#discussion_r2042642262
💬 brunoerg commented on pull request "fuzz: doc: add info about `afl-system-config` for macOS":
(https://github.com/bitcoin/bitcoin/pull/32175#discussion_r2064193303)
Done. Thanks.
🤔 janb84 reviewed a pull request: "fuzz: doc: add info about `afl-system-config` for macOS"
(https://github.com/bitcoin/bitcoin/pull/32175#pullrequestreview-2800214659)
re ACK [6e02660](https://github.com/bitcoin/bitcoin/pull/32175/commits/6e026606f368d8d1139b266c382076685e76d0b2)

Changes sinds last ACK:
- Minor style change in comments to align with rest of comment style.
💬 instagibbs commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2836118449)
I've historically been -0 on these kinds of restrictions, and as soon as we run into non-trivial usage outside the bounds of standardness, there needs to be strong motivations to keep those restrictions otherwise they become a centralizing force. This is doubly so in a world where witness stuffing is a highly dynamic and well-paid method of publishing arbitrary data on the blockchain, so we're not even accomplishing the ostensible goals of those who oppose removal.

There are roughly 3 ways to
...
💬 portlandhodl commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2836160454)
Concept Ack,

As a note the current MARA non-standard mempool policy is
- Unlimited OP_RETURNS
- No size limitations
- No restrictions on burn amount.

MARA currently has 3-5% of the network hashrate and charges 3x priority feerate for these transactions arbitraging standardness rules. The ability to profit from this activity would be reduced if these types of transactions became standard.