Bitcoin Core Github
44 subscribers
121K links
Download Telegram
🚀 achow101 merged a pull request: "fix: increase consistency of rpcauth parsing"
(https://github.com/bitcoin/bitcoin/pull/30401)
💬 achow101 commented on pull request "init: fix init fatal error on invalid negated option value":
(https://github.com/bitcoin/bitcoin/pull/30684#issuecomment-2338581569)
ACK ee47ca29d6ef55650a0af63bca817c5d494f31ef
🚀 achow101 merged a pull request: "init: fix init fatal error on invalid negated option value"
(https://github.com/bitcoin/bitcoin/pull/30684)
💬 achow101 commented on pull request "test: Add coverage for dumptxoutset failure robustness":
(https://github.com/bitcoin/bitcoin/pull/30817#issuecomment-2338601019)
ACK c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7
🚀 achow101 merged a pull request: "test: Add coverage for dumptxoutset failure robustness"
(https://github.com/bitcoin/bitcoin/pull/30817)
📝 l0rinc converted_to_draft a pull request: "refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors"
(https://github.com/bitcoin/bitcoin/pull/30765)
Split out of https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1722326803.

Extracted existing serialization to append size & data in separate private methods to clarify that it does more than just a simple data insertion.

This enables using `CScript() << "..."_hex_u8` (followed by `CScript() << "..."_hex`), skipping vector conversion before serializing to the prevector in CScript.

Originally I've added separate `std::vector` and `std::array` overloads, but based on the comments
...
⚠️ rkrux opened an issue: "28.0 RC Testing Guide Feedback"
(https://github.com/bitcoin/bitcoin/issues/30854)
This issue is to discuss the [28.0 Release Candidate Testing Guide](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/28.0-Release-Candidate-Testing-Guide). If you have any feedback on the document, please leave a comment here.

Note: This is for feedback on the document, not on Bitcoin Core or on the 28.0 changes.

Thank you for taking a look at the guide and leaving your feedback.
💬 marcofleon commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#discussion_r1750661750)
You're right, fixed. This also made me realize that I don't really need the loop. But I'm thinking to do a followup where it would build on the chain of 16 headers, as opposed to just sending 16 headers one time and resetting. So I think I'll keep the loop there for now, as it shouldn't make a difference as far as I'm aware.
💬 marcofleon commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#discussion_r1750674964)
With REGTEST enabled, it's blocked here:
https://github.com/bitcoin/bitcoin/blob/712a2b5453cdf2568fece94b969d6e0923b6ba16/src/validation.cpp#L4204-L4209

Using MAIN params it passes because those soft forks aren't deployed and so the header is added to the block index and you'll get the failure. I could add `header.nVersion = 4` in the `ConsumeHeader` function of the harness to make it pass for both main and regtest. What do you think?
💬 marcofleon commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#discussion_r1750676867)
Good suggestion, thank you.
💬 instagibbs commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#discussion_r1750677345)
Let the fuzzer choose the version, I think?
👍 itornaza approved a pull request: "multiprocess: Add -ipcbind option to bitcoin-node"
(https://github.com/bitcoin/bitcoin/pull/30509#pullrequestreview-2290623740)
Code review ACK 30073e6b3a24cbe417c45cd5df6a3a2de0251e9d

Followed step by step the implementation through the code and especially the `src/ipc/*` changed files and it looks solid. Great that it provides yet another step to facilitate the stratum v2 integration into core.
ouziel-slama closed an issue: "No ZMQ `sequence` notifications on Regtest"
(https://github.com/bitcoin/bitcoin/issues/30853)
💬 ouziel-slama commented on issue "No ZMQ `sequence` notifications on Regtest":
(https://github.com/bitcoin/bitcoin/issues/30853#issuecomment-2338810297)
Thanks for the link! actually it works fine, the error comes from elsewhere! sorry for the false alarm!
💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2338890606)
Added 1 commits 083b886bc6b98fab53e2c6b7dc99a017daa9726e -> 3749b730171e93a0beb3dcc458d440ee7035b3b9 ([`pr/mine-types.5`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.5) -> [`pr/mine-types.6`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.6), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine-types.5...pr/mine-types.6)) testing https://github.com/chaincodelabs/libmultiprocess/pull/110 fix to add explicit cmake generated file dependency and fix unreliable pa
...
💬 l0rinc commented on pull request "refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1750804998)
Dropped the commit after the suggested simplification
💬 l0rinc commented on pull request "refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1750805360)
> I'm not sure I see benefit of changing this from unsigned char to uint8_t

`uint8_t` was already used in the other branches

> changing uint8_t generally to value_type

I like that, thanks!
💬 l0rinc commented on pull request "refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1750805634)
Thanks
💬 l0rinc commented on pull request "refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1750806256)
Dropped these after your simplification suggestion
💬 l0rinc commented on pull request "refactor: Allow `CScript`'s `operator<<` to accept spans, not just vectors":
(https://github.com/bitcoin/bitcoin/pull/30765#discussion_r1750811705)
Based on what I read C++23 solves some of these problems (e.g. `Every specialization of std::span is a TriviallyCopyable type. (since C++23)`) and while array and vector casts would likely be safe, thanks for pointing out that the layout may not always be the same and we need something more predictable here.
Wanted to avoid explicit pointer arithmetic or extra copying, let me know if the latest push is a good compromise in this area.