💬 l0rinc commented on pull request "doc: Adjust path in comment":
(https://github.com/bitcoin/bitcoin/pull/32056#issuecomment-2721060655)
utACK de1ada079bf52b9d751bbeb1e7bb3011beb62429

Searched the code couldn't find any other obvious leftovers

🚀 fanquake merged a pull request: "doc: Adjust path in comment"
(https://github.com/bitcoin/bitcoin/pull/32056)

💬 yancyribbens commented on pull request "test: avoid treating hash results as integers (part 1)":
(https://github.com/bitcoin/bitcoin/pull/32050#issuecomment-2721071281)
Concept ACK. It looks like the result of the sample changes is less casting which is a good sign.

🤔 l0rinc reviewed a pull request: "cmake: Set top-level target output locations"
(https://github.com/bitcoin/bitcoin/pull/31161#pullrequestreview-2681684064)
post-merge ACK

💬 l0rinc commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#discussion_r1993404965)
post-merge nit: if you touch again please realign the comments

📝 Sjors opened a pull request: "test: testnet4 could log a disk space warning"
(https://github.com/bitcoin/bitcoin/pull/32057)
`feature_config_args.py` incorrectly assumed that its testnet4 node would not log a disk space warning for testnet4.

But when #31978 increased `m_assumed_blockchain_size` on testnet4 from 1 to 11 GiB, it triggered this bug on my RAM disk, see https://github.com/bitcoin/bitcoin/tree/master/test#speed-up-test-runs-with-a-ram-disk

This PR fixes the issue by allowing the space warning on both testnet3 and testnet4.

💬 Sjors commented on pull request "test: testnet4 could log a disk space warning":
(https://github.com/bitcoin/bitcoin/pull/32057#issuecomment-2721109159)
cc @fjahr

💬 ajtowns commented on pull request "[IBD] Tracking PR for speeding up Initial Block Download":
(https://github.com/bitcoin/bitcoin/pull/32043#issuecomment-2721136168)
> Something like this? Conceptually seems useful, but here I don't know how to interpret it, seems too far zoomed in

Fair; that might work better with a time on the x-axis rather than height, something like:

```python
for time_commit in [time_commit_X, time_commit_Y, time_commit_Z]:
for height in range(1,850000):
y = time_baseline[height] / time_commit[height] # keep this one the same
x = time_baseline[height] # changed from x = height
add_datapoint(x,y)

...

💬 rkrux commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#discussion_r1993431200)
If accepting a new argument can't be avoided for this PSBT RPC because of the same `CreateTxDoc()` being reused underneath, then I think we can explore updating functional tests for PSBT in this PR itself because `createpsbt` RPC uses the same `ConstructTransaction()` as well.

`rpc_psbt.py` is the file that would need to be updated. Using a similar approach like done in the `rpc_rawtransaction` would be preferred that is reusing bunch of the existing testing methods.

💬 rkrux commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#discussion_r1993416704)
Although this works but there seems to be a note here regarding legacy wallets, which are being deprecated. I think a more suitable place could be in the `getrawtransaction_verbosity_tests` that use `create_self_transfer()`, which already accepts a version argument in the tests - just need to pass down 3.

Also, testing the value of the transaction version after being mined would complete the all-round testing of this functionality for which only adding a new `field` for version in the `getraw
...

💬 rkrux commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#discussion_r1993433641)
Is this comment outdated? I don't understand where are the default args here that are referred to in the above comment.

💬 rkrux commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#discussion_r1993326051)
I am inclining towards not adding a positional argument for this. Mostly because this is not something the user would use frequently and instead would let the defaults take over. I think using an optional `options` object here (used in many RPCs) would suffice for this use case.
@glozow WDYT?

💬 rkrux commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#discussion_r1993398098)
Let's use `uint32_t` because that is what is used in both `CTransaction` and `CMutableTransaction`.
https://github.com/bitcoin/bitcoin/blob/72c150dfe7619c2b6e2fd06a1fa431b545a3a225/src/primitives/transaction.h#L308

https://github.com/bitcoin/bitcoin/blob/72c150dfe7619c2b6e2fd06a1fa431b545a3a225/src/primitives/transaction.h#L381

Those docs might be outdated because this change was done recently: https://github.com/bitcoin/bitcoin/pull/29325

💬 rkrux commented on pull request "rpc: Support v3 raw transactions creation":
(https://github.com/bitcoin/bitcoin/pull/31936#discussion_r1993472740)
See: https://github.com/bitcoin/bitcoin/pull/31936#discussion_r1993416704

💬 TheCharlatan commented on pull request "versionbits refactoring":
(https://github.com/bitcoin/bitcoin/pull/29039#discussion_r1993480984)
Doc nit: Maybe add some docstrings here, just like currently done with BIP9Stats?

💬 l0rinc commented on pull request "[IBD] Tracking PR for speeding up Initial Block Download":
(https://github.com/bitcoin/bitcoin/pull/32043#issuecomment-2721197918)
> time on the x-axis rather than height

If we do that we don't even need to filter out the first 400k blocks since they're so insignificant.
What do you think of this?

<img width="1000" alt="image" src="https://github.com/user-attachments/assets/1b3fb11a-f4f4-4277-99ba-decd2c60da6a" />

💬 hodlinator commented on pull request "[IBD] specialize CheckBlock's input & coinbase checks":
(https://github.com/bitcoin/bitcoin/pull/31682#discussion_r1993519848)
My previous measurement was on Clang 19.1.7. My GCC 14.2.1 results show a similar 2.5% improvement.

<details><summary>GCC Benchmarks</summary>

#### PR

```
₿ build/src/bench/bench_bitcoin -filter=CheckBlockBench -min-time=30000

| ns/block | block/s | err% | ins/block | cyc/block | IPC | bra/block | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------
...

📝 Chand-ra opened a pull request: "test: replace hardcoded fee with node relay fee based calculation"
(https://github.com/bitcoin/bitcoin/pull/32058)
Replace the hardcoded fee of 1000 satoshis with a dynamic calculation that retrieves the node’s current minimum relay fee (minrelaytxfee) via RPC, as directed by the TODO tag.

💬 ismaelsadeeq commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r1993472154)
```suggestion
void Cluster::AppendChunkFeerates(std::vector<FeeFrac>& ret) const noexcept
{
auto chunk_feerates = ChunkLinearization(m_depgraph, m_linearization);
ret.reserve(ret.size() + chunk_feerates.size());
ret.insert(ret.end(), chunk_feerates.begin(), chunk_feerates.end());
}
```

💬 ismaelsadeeq commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r1993479930)
Shouldn't this be <= instead.
```suggestion
Assume(m_clustersets.size() <= 2);

```