👍 hodlinator approved a pull request: "merkle: migrate `path` arg to reference and drop unused args"
(https://github.com/bitcoin/bitcoin/pull/33805#pullrequestreview-3540270081)
ACK 24ed820d4f0d8f7fa2f69e1909c2d98f809d2f94

Effectively removes dead code.

💬 hodlinator commented on pull request "merkle: migrate `path` arg to reference and drop unused args":
(https://github.com/bitcoin/bitcoin/pull/33805#discussion_r2589269947)
meganit: Could declare parameter `const` to decrease cognitive complexity
```suggestion
static void MerkleComputation(const std::vector<uint256>& leaves, const uint32_t leaf_pos, std::vector<uint256>& path)
```

💬 hodlinator commented on pull request "merkle: migrate `path` arg to reference and drop unused args":
(https://github.com/bitcoin/bitcoin/pull/33805#discussion_r2592258492)
remark: The expression on the right side is side-effect free, so removing it is safe. (`inner` is a raw C++ array - no overloaded `operator[]`, comparing `uint256` values doesn't mutate them).

👍 sedited approved a pull request: "cmake: Check dependencies after build option interaction"
(https://github.com/bitcoin/bitcoin/pull/33974#pullrequestreview-3543500447)
ACK 56d0a0929d8974a490c57bf84a994ba7c9f19863

Guix build:
```
5bd1a78f590d48e5ceb5815b5ed346d06d738871c9ea26b754c9a851dcb7ee85 guix-build-56d0a0929d89/output/aarch64-linux-gnu/SHA256SUMS.part
cc0525310c1e1c1b1cb69c2256b8089b187830c27da9dd7ba9830884c35a0e95 guix-build-56d0a0929d89/output/aarch64-linux-gnu/bitcoin-56d0a0929d89-aarch64-linux-gnu-debug.tar.gz
bc891ef995772a5f09c48000b0b0e7d6dc142ae184490ef711c800f42c48cdeb guix-build-56d0a0929d89/output/aarch64-linux-gnu/bitcoin-56d0a0929d8
...

💬 sedited commented on pull request "cmake: Check dependencies after build option interaction":
(https://github.com/bitcoin/bitcoin/pull/33974#discussion_r2591737457)
Nit: Extra line.

💬 fanquake commented on pull request "cmake: Check dependencies after build option interaction":
(https://github.com/bitcoin/bitcoin/pull/33974#discussion_r2592292207)
What is this comment meant to be?

💬 hebasto commented on pull request "cmake: Check dependencies after build option interaction":
(https://github.com/bitcoin/bitcoin/pull/33974#discussion_r2592302403)
Some build options, such as `BUILD_FOR_FUZZING`, may alter other user-specified build option values depending on how they are set.

💬 Sjors commented on pull request "mining: add getMemoryLoad() and track template non-mempool memory footprint":
(https://github.com/bitcoin/bitcoin/pull/33922#discussion_r2592303231)
To limit the scope of this PR, I only added the mutex, but called it `template_state_mutex` in anticipation.

💬 fanquake commented on pull request "cmake: Check dependencies after build option interaction":
(https://github.com/bitcoin/bitcoin/pull/33974#discussion_r2592308392)
Can the comment explain that.

💬 fanquake commented on pull request "cmake: Check dependencies after build option interaction":
(https://github.com/bitcoin/bitcoin/pull/33974#discussion_r2592329786)
Also, is this meant to mark that no interaction happens after this point, or is the only place there is any interaction?

💬 fanquake commented on pull request "stabilize translations by reverting old ids by text content":
(https://github.com/bitcoin/bitcoin/pull/33270#issuecomment-3616461192)
Ok. I think @hebasto needs to make a call here then.

💬 Fi3 commented on issue "rpc: getrawtransaction lookup by witness txid":
(https://github.com/bitcoin/bitcoin/issues/34013#issuecomment-3616470606)
ACK for dmnd or other pools that will implement sv2 and want to do job declaration and being able to relay the blocks founded by the miners, having fast way to retrieve tx data from the node using a wtxid would be very useful.

💬 laanwj commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3616479283)
Here's a patch to `manifest.scm` that makes it easier to test with arbitrary gcc commits: https://github.com/laanwj/bitcoin/commit/a323ddaf5ad88b3429e2ee079566efee4e0d8f7e

Allows:
```
GCC_COMMIT=953536524a198efa932ee2cccf5d29d4fc1d24f3 GCC_HASH=3b59e003321ac50462f626bba07826c2d4c203d4fd1500c64d2ea332e61eaab0 HOSTS="x86_64-w64-mingw32" contrib/guix/guix-build
```
- GCC_COMMIT is any arbitrary commit from the gcc repository (any will work as long as the patches apply).
- GCC_HASH is the he
...

👍 hebasto approved a pull request: "[30.x] Backports & 30.1rc1"
(https://github.com/bitcoin/bitcoin/pull/33997#pullrequestreview-3544326443)
re-ACK d59ebac7187dceb60f880ee3d2c5fab8368da176.

🤔 alexanderwiederin reviewed a pull request: "kernel: Add block header support and validation"
(https://github.com/bitcoin/bitcoin/pull/33822#pullrequestreview-3544319469)
Concept ACK

💬 alexanderwiederin commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2592351617)
I noticed that `ProcessNewBlockHeaders` is called with `min_pow_checked` hard-coded to true.

Was this an intentional design choice? If callers are not supposed to to control this, it might be good to mention in the function docs that min-pow checks are never performed.

Otherwise, should we have min_pow checks be an option `btck_chainstate_manager_process_block_header`?

💬 sedited commented on issue "rpc: getrawtransaction lookup by witness txid":
(https://github.com/bitcoin/bitcoin/issues/34013#issuecomment-3616489509)
It's not clear to me why this should happen through RPC if they are using the IPC interface already.

💬 Sjors commented on issue "rpc: getrawtransaction lookup by witness txid":
(https://github.com/bitcoin/bitcoin/issues/34013#issuecomment-3616505380)
@sedited there's different "they"s here:

1. The miner who proposes a block, they use IPC
2. The pool software that verifies the template and broadcasts a solution: the current SRI implementation doesn't use IPC

That said, I'm thinking about making an IPC method to fetch transactions more efficiently.

💬 Fi3 commented on issue "rpc: getrawtransaction lookup by witness txid":
(https://github.com/bitcoin/bitcoin/issues/34013#issuecomment-3616513802)
> It's not clear to me why this should happen through RPC if they are using the IPC interface already.

The [Template Provider](https://github.com/Sjors/sv2-tp/) provider is using the IPC interface but the Job Declarator Server is not, and it could need to get tx data for tx in jobs declared by the miner. Having a method to do that without asking them to the miner is very helpful. The Job Declarator Server could use the IPC interface, but I don't see why these should not be possible trough the
...

💬 fanquake commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3616532437)
Slightly more minified:
```cpp
#include <filesystem>
#include <string>
#include <vector>
#include <cstring> // Unused, but removing this makes it determinstic

namespace fs {

using namespace std::filesystem;

// Dummy class. Using "path = std::filesystem::path" makes it deterministic.
class path : public std::filesystem::path
{
public:
using std::filesystem::path::path;

path(std::filesystem::path path) : std::filesystem::path::path(std::move(path)) {}
};

}

bool
...