💬 dergoegge commented on issue "build: RFC Coverage build type":
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2405012504)
> I liked the summary stats per folder produced by the current approach, but if others like other stuff better, I don't mind.

The LLVM approach also supports that:


💬 ismaelsadeeq commented on issue "Fee Estimation via Fee rate Forecasters tracking issue":
(https://github.com/bitcoin/bitcoin/issues/30392#issuecomment-2405065185)
This issue was opened to track the effort to improve the Bitcoin core fee estimator. This comment will provide information on the status of this project and some key ideas on how to achieve this.

**Completed Tasks so far:**
- **Enabled asynchronous block policy fee estimator updates**: By making it a client of the validation interface.

- **Empirical data analysis**: To determine which forecasting strategy provides the most accurate estimates while withstanding common attack vectors.
- De
...

💬 brunoerg commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2405071446)
> Can you provide more details here? IIUC a hash will still be calculated, just not compared, so this may only drop one hash call, with a possible maximum speed-up of 2x. However, it would be good to actually measure the difference. Otherwise, I am not sure if this is worth it.

This is not about performance. It makes the `p2p_transport_serialization` fuzz target simpler because we can remove the code that assists the checksum and magic bytes creation. So, we can achieve the same with less LoC
...

💬 maflcko commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2405082968)
Ok, then I am nack-ish on this. Generally, test code should follow real code, not the other way round, unless there is a reason. Saving 15 lines of test-only code seems a weak reason to me, but I don't mind if others like this.

💬 brunoerg commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2405088673)
Since everyone agrees on removing Honggfuzz netdriver, I think we can change the approach to simply just remove it from the documentation.

💬 jonatack commented on issue "Fee Estimation via Fee rate Forecasters tracking issue":
(https://github.com/bitcoin/bitcoin/issues/30392#issuecomment-2405100792)
> Create estimatefee RPC

Four years ago I was beginning to implement the roadmap as described in https://github.com/bitcoin/bitcoin/pull/20484#issuecomment-734786305, beginning with a `setfeerate` RPC in https://github.com/bitcoin/bitcoin/pull/20391 and a planned `estimatefeerate` RPC, both using sat/vB units. Was considering picking these up again. Presumably orthogonal, apart from coordinating on the RPC naming.

🤔 TheCharlatan reviewed a pull request: "log: Enforce trailing newline"
(https://github.com/bitcoin/bitcoin/pull/30929#pullrequestreview-2360363711)
Post-merge ACK

I think this really showed the limits of having our own clang-tidy checks. They don't lend themselves well to code that is evolving, may receive renames, or new methods.

💬 jonatack commented on pull request "doc: cmake: prepend "build" to functional/test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/30859#discussion_r1795456841)
Per feedback in https://github.com/bitcoin/bitcoin/pull/30859#discussion_r1759115295, begin this sentence with "Assuming the build directory is named `build`, running..."

I agreed with the feedback not to add it where it was already mentioned in other files, but in this file there is no other mention.

🤔 jonatack reviewed a pull request: "doc: cmake: prepend "build" to functional/test_runner.py"
(https://github.com/bitcoin/bitcoin/pull/30859#pullrequestreview-2360372434)
LGTM modulo comment

👍 TheCharlatan approved a pull request: "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment"
(https://github.com/bitcoin/bitcoin/pull/30666#pullrequestreview-2360426595)
Re-ACK 0bd53d913c1c2ffd2d0779f01bc51c81537b6992

💬 maflcko commented on pull request "build: Rename `PACKAGE_*` variables to `CLIENT_*`":
(https://github.com/bitcoin/bitcoin/pull/31042#discussion_r1795480094)
It seems `CLIENT_NAME` is already used in a different (non-build) context to denote the "user agent name". Maybe a preparatory commit could be added to change that to `UA_NAME`, for clarity and to avoid a name clash?


```
$ git grep --line-number CLIENT_NAME
src/clientversion.cpp:23:const std::string CLIENT_NAME("Satoshi");
src/clientversion.h:36:extern const std::string CLIENT_NAME;
src/init.cpp:1470: strSubVersion = FormatSubVersion(CLIENT_NAME, CLIENT_VERSION, uacomments);

💬 TheCharlatan commented on pull request "build: scripted-diff: drop config/ subdir for bitcoin-config.h":
(https://github.com/bitcoin/bitcoin/pull/30937#issuecomment-2405170985)
Post-merge ACK 882f736d0a60

Guix build (aarch64):
```
9cf65a99fa046a1a3acbee14d6dec658b9fe7f4e55a4b6f50d063ebb4029e3eb guix-build-882f736d0a60/output/aarch64-linux-gnu/SHA256SUMS.part
46780332298b988bc9b4c2903d077f49978dc92250cf5221557ce4459383b18e guix-build-882f736d0a60/output/aarch64-linux-gnu/bitcoin-882f736d0a60-aarch64-linux-gnu-debug.tar.gz
72362b6f0394f4b8b1e69e4520c892d5b3e9f3d86788c8f0ee59f2b0d9e51717 guix-build-882f736d0a60/output/aarch64-linux-gnu/bitcoin-882f736d0a60-aarch
...

💬 ismaelsadeeq commented on issue "Fee Estimation via Fee rate Forecasters tracking issue":
(https://github.com/bitcoin/bitcoin/issues/30392#issuecomment-2405172852)
> Both using sat/vB units.

This is a good idea.

> Beginning with a setfeerate RPC in https://github.com/bitcoin/bitcoin/pull/20391.

I learned about this issue after @murchandamus's review comment https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1526664705. I think we should just unify `settxfee` and your `setfeerate` into one RPC?

> Was considering picking these up again. Presumably orthogonal, apart from coordinating on the RPC naming.

Happy to review when you do.
...

💬 fanquake commented on pull request "build: Rename `PACKAGE_*` variables to `CLIENT_*`":
(https://github.com/bitcoin/bitcoin/pull/31042#discussion_r1795498723)
I think if we end up doingthat, we should extend this to renaming other source variables that exist in the PACKAGE_ namespace. This PR as-is is already changing things such that naming becomes more inconsistent in that regard.

💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1795560402)
this is definitely problematic, but I can't seem to figure out a way to hit it. In prod this would "just" cause it to not call `m_txrequest.ForgetTxHash(ptx->GetWitnessHash());` for it, but would have to be in a 1p1c package setting.

To retain current behavior, I think it can just be expanded to
```suggestion
// DIFFERENT_WITNESS has "valid" status, but we still want to stop requesting it
if (!Assume(state.IsInvalid() ||
tx_result.m_result_type == MempoolAcceptResult::Res
...

📝 danielabrozzoni opened a pull request: "rest: Support transaction broadcast in REST interface"
(https://github.com/bitcoin/bitcoin/pull/31065)
This adds a REST endpoint to allow broadcasting a transaction:
`POST /rest/broadcast.hex`

The transaction hex must be passed in the body of the request;
on success, the txid of the broadcasted transaction will be returned.

Fixes #31017

💬 ryanofsky commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2405280136)
> Is it ok for other external users of `processNewBlock` to skip over the pre-checks and optimizations that are normally done in `submitblock`?

I assumed it was ok, but now that `interfaces::BlockTemplate::submitSolution` in added in 525e9dcba0b8c6744bcd3725864f39786afc8ed5 (to deal with the concern in https://github.com/bitcoin/bitcoin/pull/30440#issuecomment-2229233817) maybe the `interfaces::Mining::processNewBlock` method could be dropped and RPC code can go back to calling `ProcessNewBlo
...

💬 glozow commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1795517895)
tiny typo unusued

🤔 glozow reviewed a pull request: "cluster mempool: extend DepGraph functionality"
(https://github.com/bitcoin/bitcoin/pull/30857#pullrequestreview-2360504869)
ACK 0b3ec8c59b2b6d598531cd4e688eb276e597c825, reviewed range-diff from aab53ddcd8fcbc3c0be0da9383f8e06abe5badda and `clusterlin_depgraph_sim`

💬 glozow commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1795548751)
Nice, no more naming clash for `Cluster` in my mind