💬 l0rinc commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2472810064)
This value will be returned and converted to a `size_t` again
https://github.com/bitcoin/bitcoin/blob/fab75c2d0332fdb25030abb441363478cd486dc5/src/index/blockfilterindex.cpp#L201
and added to a `FlatFilePos::nPos`
https://github.com/bitcoin/bitcoin/blob/fab75c2d0332fdb25030abb441363478cd486dc5/src/index/blockfilterindex.cpp#L285
which is currently a `uint32_t`:
https://github.com/bitcoin/bitcoin/blob/fa128d1488c08d9816f425c73d01db4c1597ee68/src/flatfile.h#L17

Can we avoid converting this back a
...

💬 l0rinc commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2472815299)
is there any scenario where where a signed `UniValue` value is different from an unsigned one?

💬 l0rinc commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2472880211)
https://github.com/bitcoin/bitcoin/blob/fa128d1488c08d9816f425c73d01db4c1597ee68/src/node/blockstorage.h#L120
should also likely be updated now to:
```C++
static constexpr uint32_t STORAGE_HEADER_BYTES{std::tuple_size_v<MessageStartChars> + sizeof(uint32_t)};
```

----

Note: I understand this works on most 32 bit systems we support, but https://en.cppreference.com/w/cpp/language/types.html indicates it can vary between 16 and 32 bits.
Also, https://learn.microsoft.com/en-us/cpp/cpp/dat
...

💬 l0rinc commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2472903256)
Not sure I understand why `GetBlockWeight` is `int64_t`
https://github.com/bitcoin/bitcoin/blob/3c5d1a468199722da620f1f3d8ae3319980a46d5/src/consensus/validation.h#L136
but `MAX_BLOCK_WEIGHT` and `MAX_BLOCK_SERIALIZED_SIZE` are only `unsigned int`
https://github.com/bitcoin/bitcoin/blob/3c5d1a468199722da620f1f3d8ae3319980a46d5/src/consensus/consensus.h#L15

💬 l0rinc commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2472976911)
`MAX_FLTR_FILE_SIZE` is only used here
https://github.com/bitcoin/bitcoin/blob/fab75c2d0332fdb25030abb441363478cd486dc5/src/index/blockfilterindex.cpp#L39
we should adjust its type to match the usage
```C++
constexpr uint32_t FLTR_FILE_CHUNK_SIZE{0x100000}; // 1 MiB
```

💬 l0rinc commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2472910698)
`FindNextBlockPos` has `unsigned int nAddSize` arg still:
```C++
FlatFilePos BlockManager::FindNextBlockPos(uint32_t nAddSize, unsigned int nHeight, uint64_t nTime)
```

💬 l0rinc commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2473369894)
If we're changing this, we should likely adjust the usage as well:
https://github.com/bitcoin/bitcoin/blob/fab75c2d0332fdb25030abb441363478cd486dc5/src/wallet/spend.cpp#L1085
```C++
int32_t tx_noinputs_size = 0;
```

💬 l0rinc commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2473712142)
https://github.com/bitcoin/bitcoin/blob/fa128d1488c08d9816f425c73d01db4c1597ee68/src/flatfile.h#L16
is `int32_t` now so we should update these as well:
```C++
const int32_t nFile = pos.nFile;
if (static_cast<int32_t>(m_blockfile_info.size()) <= nFile) {
```

💬 hebasto commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3464387417)
Rebased. Some of the feedback has been addressed.

My Guix build:
```
x86_64
366c53942430c0f310fc42445693e7ba1ff5dce25f2df8e42d3c4b044747f385 guix-build-4d31ddc6dfe6/output/dist-archive/bitcoin-4d31ddc6dfe6.tar.gz
1b0c2e70c8f85650a4b4238c74dd22b960b2f4036042d2f8c6b302dbc50ae3b7 guix-build-4d31ddc6dfe6/output/x86_64-w64-mingw32/SHA256SUMS.part
ed292458da2126949ee0a35495ba59272066364d7b458b28576dcd229ed984b5 guix-build-4d31ddc6dfe6/output/x86_64-w64-mingw32/bitcoin-4d31ddc6dfe6-win64-cod
...

👋 hebasto's pull request is ready for review: "guix: Use UCRT runtime for Windows release binaries"
(https://github.com/bitcoin/bitcoin/pull/33593)

💬 ajtowns commented on pull request "test: add valid tx test with minimum-sized ECDSA signature (8 bytes DER-encoded)":
(https://github.com/bitcoin/bitcoin/pull/32924#issuecomment-3464418432)
ACK 5fa81e239a39d161a6d5aba7bcc7e1f22a5be777 lgtm

Presumably it would also be possible to use the same signature on a p2wsh tx (`<sig> SWAP CHECKSIG`) which would lose the `CONST_SCRIPTCODE` requirement. Could also presumably just have the script be `CHECKSIG` and provide both the pubkey and the short signature in either p2sh or p2wsh.

💬 JeremyRubin commented on issue "Think about tuning the script cache/sigcache ratios":
(https://github.com/bitcoin/bitcoin/issues/10754#issuecomment-3464424185)
@willcl-ark you can also look at the memory model and seeing if/how you can make the cache more parallel during block validation. IIRC there is some unnecessary locking which can be eliminated/reduced to something like one acquire/release per thread and a single thread holding a unique lock to prevent writers.

💬 andrewtoth commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2475738139)
> But as you said, the thread sanitizer would likely complain about that. We’re accessing the same variable from different threads (main and worker).

It won't complain if we synchronize access to the same non-atomic variable, which we do here because we lock when calling `Submit`.

> The main issue, however, will probably be related to thread visibility; changes made in one thread aren’t guaranteed to be visible to another.

I don't think this is the main issue. If a future change breaks
...

💬 umrashrf commented on issue "Can I compile on OSX Tahoe?":
(https://github.com/bitcoin/bitcoin/issues/33733#issuecomment-3464623966)
@l0rinc still fails

1. brew upgrade
2. brew reinstall boost
3. use clang from homebrew (update $PATH)
4. configure logs https://gist.github.com/umrashrf/6ca23317de227f3c8173a0031ced26f8
5. build logs https://gist.github.com/umrashrf/d4e0ef25921f78fe9fc8909bff043ccb

💬 l0rinc commented on issue "Can I compile on OSX Tahoe?":
(https://github.com/bitcoin/bitcoin/issues/33733#issuecomment-3464678834)
Seems the llvm upgrade was successful, but you still have two boost installations, try removing the old one.

📝 l0rinc opened a pull request: "log: avoid collecting `GetSerializeSize` data when compact block logging is disabled"
(https://github.com/bitcoin/bitcoin/pull/33738)
The new accounting options introduced in https://github.com/bitcoin/bitcoin/pull/32582 can be quite heavy, and not needed when debug logging is disabled.

`PartiallyDownloadedBlock::FillBlock()` and `PeerManagerImpl::SendBlockTransactions()` accumulate transaction sizes for debug logging by calling `GetTotalSize()` in loops, which invokes expensive `GetSerializeSize()` serializations.

Guard the size calculations with `LogAcceptCategory()` checks so serialization only occurs when compact bl
...

⚠️ ukml opened an issue: "Link"
(https://github.com/bitcoin/bitcoin/issues/33739)
This link dos not work

💬 ukml commented on issue "Link":
(https://github.com/bitcoin/bitcoin/issues/33739#issuecomment-3464792014)
This link does not work

✅ pinheadmz closed an issue: "Link"
(https://github.com/bitcoin/bitcoin/issues/33739)

📝 fjahr opened a pull request: "RFC: bench: Add multi thread benchmarking"
(https://github.com/bitcoin/bitcoin/pull/33740)
This adds a rough way to run specific benchmarks with different numbers of worker threads to see how these impact performance. This is useful for me in the batch validation context and for potential refactorings of checkqueue but I am not sure if this is useful in many other contexts so I am leaving this as a RFC draft for now to see if there is any interest in merging something like this.

Example output:

```
$ build/bin/bench_bitcoin -filter=ConnectBlockAllSchnorr -min-time=1000 -scale-t
...