Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 andrewtoth commented on pull request "rpc, rest: Improve block rpc error handling, check header before attempting to read block data.":
(https://github.com/bitcoin/bitcoin/pull/30410#discussion_r1759057303)
nit: why remove the extra context that we are testing with verbosity 2 and 3 specifically?
👍 itornaza approved a pull request: "Have createNewBlock() return a BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/30440#pullrequestreview-2303431473)
Code review ACK a93c171faa7b4426823466e972c8f24260918bbf

Offering an option for the Template Provider to get block header data as soon as possible is a great addition.

Code changes look straight forward. Also, built the pr and successfully run all tests on it locally.
💬 fanquake commented on pull request "guix: Pointer Authentication and Branch Target Identification for aarch64 Linux":
(https://github.com/bitcoin/bitcoin/pull/24123#issuecomment-2349221011)
Rebased for #30433, and updated to add an export allowance for `__libc_single_threaded`.
💬 maflcko commented on pull request "refactor: move m_is_inbound out of CNodeState":
(https://github.com/bitcoin/bitcoin/pull/30233#issuecomment-2349225738)
ACK 07f4cebe5780f1039541d989e64b70eccc5b4eb5 🗞

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 07f4cebe5780f1039541d989e6
...
💬 rkrux commented on issue "28.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/30854#issuecomment-2349229940)
@monlovesmango Thanks for sharing your feedback. I've gone through them and these points are correct, I've updated the guide.
💬 achow101 commented on pull request "[28.x] Further backports and rc2":
(https://github.com/bitcoin/bitcoin/pull/30827#issuecomment-2349234397)
> @achow101
>
> Could you please add [bitcoin-core/gui#835](https://github.com/bitcoin-core/gui/pull/835) as well?

Done
💬 sdaftuar commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2349244010)
I have been gathering more data to try to measure the effect of the improvements in this PR. I decided to take my cluster mempool branch (#28676), combined with my simulation environment for playing back historical data, and measure the effect of a series of 4 commits from this PR on the number of clusters encountered in 2023 that would NOT have been optimally linearized, for each of 5 different values of the maximum iterations we'd permit when linearizing.

I think plotted these values on
...
💬 mzumsande commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#issuecomment-2349247257)
> Ping also @mzumsande, given you https://github.com/bitcoin/bitcoin/pull/28043#pullrequestreview-1846077000 to this approach last time.

No reservations with the current approach. Concept ACK, I haven't reviewed the actual fuzz target in detail (and don't know if I would get to that in time, considering this has already multiple acks).

Would it make sense to additionally have some belt-and-suspenders check somewhere at the start of bitcoind that `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` i
...
TheCharlatan closed an issue: "cmake: Installed static kernel library is unusable"
(https://github.com/bitcoin/bitcoin/issues/30801)
💬 TheCharlatan commented on issue "cmake: Installed static kernel library is unusable":
(https://github.com/bitcoin/bitcoin/issues/30801#issuecomment-2349255516)
> @TheCharlatan what is the status of this now?

Just tested that this works with the kernel rust crate now for statically linking the library into a rust binary. So I think we are done here.
💬 fanquake commented on pull request "code style: update .editorconfig file":
(https://github.com/bitcoin/bitcoin/pull/30877#discussion_r1759098222)
@theStack did you want to push to take this, given this file is rarely touched?
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1759111178)
@petertodd I know for their slipstream accelerator at a minimum they do not, and perhaps they updated all their nodes to not enforce, I'm not sure.

@sdaftuar Ok that's not much machinery, and seems to make sense combined with base and modified fee both being checked.

I think any node runner who wants to ignore dust rules they'll simply set `-dustrelayfee=0`, which works out of the box as expected in this PR, and even if we're more controlling on when `prioritisetransaction` can be called.
...
💬 theStack commented on pull request "code style: update .editorconfig file":
(https://github.com/bitcoin/bitcoin/pull/30877#discussion_r1759111338)
Yes good point, done.
🚀 fanquake merged a pull request: "doc: unit test runner help fixup and improvements"
(https://github.com/bitcoin/bitcoin/pull/30890)
💬 jonatack commented on pull request "doc: cmake: prepend "build" to functional/test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/30859#discussion_r1759115295)
Maybe begin this sentence with something like

"Assuming the build directory is named `build`, running..."

or

`<build_directory>/test/functional/test_runner.py`
🚀 fanquake merged a pull request: "code style: update .editorconfig file"
(https://github.com/bitcoin/bitcoin/pull/30877)
💬 jonatack commented on pull request "doc: cmake: prepend "build" to functional/test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/30859#discussion_r1759118276)
Not sure if needed, but maybe good to use

`<build_directory>/test/functional/test_runner.py`

or

(if the [test dependencies](/test) are installed and the build directory is named `build`)

---

> This extra line seems like overkill if the proceeding line is being updated to include "build"

Agree
🤔 jonatack reviewed a pull request: "tidy: Use 'starts_with' instead of substr/find"
(https://github.com/bitcoin/bitcoin/pull/30868#pullrequestreview-2303511695)
ACK 22bc9fdca314549bf204d3e3f577d7f014858f42

Maybe name this PR (and the commit, if you need to retouch):

tidy: add clang-tidy `modernize-use-starts-ends-with` check
💬 fjahr commented on pull request "rpc, rest: Improve block rpc error handling, check header before attempting to read block data.":
(https://github.com/bitcoin/bitcoin/pull/30410#issuecomment-2349304886)
re-ACK 6a1aa510e31e8b77793341473aa5afc9d023a6e3
💬 jonatack commented on pull request "tidy: Use 'starts_with' instead of substr/find":
(https://github.com/bitcoin/bitcoin/pull/30868#issuecomment-2349305495)
I'm still adapting to the new build system, but fwiw, locally it looks like no unit tests fail if I flip the changed conditionals to the opposite tests. Functional tests do fail.