Bitcoin Core Github
43 subscribers
122K links
Download Telegram
πŸ’¬ maflcko commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#discussion_r2067406415)
it would be better to file this separately and explain why it isn't needed (or rather since when)
πŸ’¬ achow101 commented on pull request "versionbits refactoring":
(https://github.com/bitcoin/bitcoin/pull/29039#issuecomment-2840221541)
ACK e3014017bacff42d8d69f3061ce1ee621aaa450a
πŸ’¬ TheCharlatan commented on pull request "refactor: validation: mark CheckBlockIndex as const":
(https://github.com/bitcoin/bitcoin/pull/32364#issuecomment-2840221780)
Concept ACK

I wrote this simpler approach here https://github.com/TheCharlatan/bitcoin/commit/fe13fa082e3d6befa6d5332428584570ead608ab after seeing you mentioning it last week. I think making sure that all block indexes are declared const in the method might be beneficial though. Maybe instead of adding a separate `GetAll` method just make it const and then declare the chainstate pointers as const in the for loop?
πŸ’¬ BTCMcBoatface commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2840235544)
ACK, but actually NACK. What if you leave the `datacarrier` and `datacarriersize` config options but remove the enforcement code: filter-enjoyers don’t get mad, code is cleaner, limits remain ineffective.
πŸš€ achow101 merged a pull request: "versionbits refactoring"
(https://github.com/bitcoin/bitcoin/pull/29039)
πŸ’¬ mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2067435003)
We don't "need" the check - but what we want to guarantee is that whenever `cs_main` is not held, the block index must be in a consistent state (i.e. `CheckBlockIndex()` would pass) - because at this point, other threads can change the block index / call CBI, leading to issues such as #16444.
This spot is pretty critical, because even though `InvalidateBlock` will clean up in the end, it releases `cs_main` for stability reasons multiple times (so that the node doesn't become unresponsive for to
...
πŸ’¬ achow101 commented on pull request "net: remove unnecessary check from AlreadyConnectedToAddress()":
(https://github.com/bitcoin/bitcoin/pull/32338#issuecomment-2840281637)
ACK f1b142856a4ecd0a0d90bc3d73ef5137997b14ff
πŸ’¬ BrazyDevelopment commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2840289771)
I have significant concerns about removing the `datacarrier` and `datacarriersize` limits without thoroughly addressing the potential for exacerbating existing attack vectors.

Bitcoin's mempool and network already face challenges from known attack vectors like flood and loot attacks, replacement cycling, and irreversible fee attacks, which exploit transaction propagation and fee mechanics to burden nodes or disrupt confirmation reliability.

By removing OP_RETURN limits, we risk introducin
...
πŸ’¬ achow101 commented on pull request "validation: log which peer sent us a header":
(https://github.com/bitcoin/bitcoin/pull/27826#issuecomment-2840297221)
ACK abe43dfadd6325f80975a76aea57a549c3162191
πŸš€ achow101 merged a pull request: "net: remove unnecessary check from AlreadyConnectedToAddress()"
(https://github.com/bitcoin/bitcoin/pull/32338)
πŸ’¬ NicolaLS commented on pull request "doc: Improve `dependencies.md`":
(https://github.com/bitcoin/bitcoin/pull/31895#discussion_r2067466850)
> Is there any value in documenting the version used twice with the additional maintenance overhead and risk of it being stale? The depends package definition is already linked, so anyone can get the real version with just one more click/open. If they want to find out the pull request/commit it will be just another step (git blame).
>
> Could consider removing it?

I absolutely agree, I can add a commit to remove the "Version used" column from all tables if everyone is fine with this.
βœ… achow101 closed an issue: "Log which peer sent us a header (first)"
(https://github.com/bitcoin/bitcoin/issues/27744)
πŸš€ achow101 merged a pull request: "validation: log which peer sent us a header"
(https://github.com/bitcoin/bitcoin/pull/27826)
πŸ’¬ achow101 commented on pull request "net: Use GetAdaptersAddresses to get local addresses on Windows":
(https://github.com/bitcoin/bitcoin/pull/31014#issuecomment-2840345673)
ACK b9d4d5f66a5a35c47e7abc9ec6ef5ab242b3f1e1
πŸ’¬ owenkemeys commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2840348273)
Concept ACK - but go further and clean this up once and for all.
We may not want data, but many do, and are happy to work around frictions including creating tooling to automate that.
Similarly, the fullrbf saga demonstrated that a minority of more permissive nodes is enough to undermine all filtering.

I entirely sympathise with the desire of many to minimise arbitrary data - I even agree. Bitcoin is not for that.
But the mechanics and economics of the situation dictate that as soon as any
...
πŸ’¬ joecool1029 commented on issue "PIE+LTO causes Bitcoin-Qt to segfault at startup":
(https://github.com/bitcoin-core/gui/issues/867#issuecomment-2840353696)
@hebasto I can confirm the above resolves the issue. (I am the Gentoo bug reporter on this)
πŸš€ achow101 merged a pull request: "net: Use GetAdaptersAddresses to get local addresses on Windows"
(https://github.com/bitcoin/bitcoin/pull/31014)
πŸ€” polespinasa reviewed a pull request: "policy: allow more than one OP_RETURN outputs per tx"
(https://github.com/bitcoin/bitcoin/pull/32381#pullrequestreview-2805192347)
uACK

Code lgtm, just a small question on the test code.
πŸ’¬ polespinasa commented on pull request "policy: allow more than one OP_RETURN outputs per tx":
(https://github.com/bitcoin/bitcoin/pull/32381#discussion_r2067499756)
Why is this needed?
πŸ’¬ achow101 commented on pull request "test: avoid stack overflow in `FindChallenges` via manual iteration":
(https://github.com/bitcoin/bitcoin/pull/32351#issuecomment-2840371986)
ACK 7e8ef959d0637ca5543ed33d3919937e0d053e70