💬 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
...

💬 rkrux commented on pull request "log: don't rate-limit "new peer" with -debug=net":
(https://github.com/bitcoin/bitcoin/pull/34008#discussion_r2592395736)
It's documented that `LogPrintf` is deprecated, prefer to avoid its usage?

https://github.com/bitcoin/bitcoin/blob/0c9ab0f8f8c85719ff3aa4aefe3198cd2f8d63d1/src/logging.h#L372-L373

💬 hebasto commented on issue "ci: failure in Windows native job":
(https://github.com/bitcoin/bitcoin/issues/34012#issuecomment-3616545810)
> I haven't looked here, but generally CI does not work well with re-running tasks, when the code is merged with current master, but the ci config is stale.
>
> As the error is zmq related, it could be due to [49c6728](https://github.com/bitcoin/bitcoin/commit/49c672853503e561cd1e7fed19a32f21ad345370), but again, I haven't checked this closely.

I concur with both assessments above.

The `.github/workflows/ci.yml` for the failed run looks outdated: https://github.com/bitcoin/bitcoin/actions/run
...

💬 plebhash commented on issue "rpc: getrawtransaction lookup by witness txid":
(https://github.com/bitcoin/bitcoin/issues/34013#issuecomment-3616550030)
currently SRI Job Declarator Server monitors the mempool via RPC and we have considered switching to ZMQ on this issue https://github.com/stratum-mining/sv2-apps/issues/26, which is about to be tackled soon

but I like @Sjors idea of leveraging IPC

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

✅ hebasto closed a pull request: "qa: Remove no longer needed `feature_dirsymlinks.py`"
(https://github.com/bitcoin/bitcoin/pull/33924)

💬 sedited commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2592425740)
This is on purpose from my part. I think it is confusing and not helpful to expose. If you look at where it is used, you'll see that all it does is invalidate a header that does not extend the pre-synced best work chain. The only case it is still used in our code is when processing unrequested blocks, but since we're processing headers here, I don't think that is relevant. I also don't think that this should be documented either. The current `min_pow_checked` handling in our validation code is c
...