💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1765381277)
"bytes" seems clearer to me as well. Updated.

Originally "size" was chosen because it aligns with `getrawtransaction`.
If there's friction with using "bytes", I can change it back to "size".

> Argument and field naming: please consider whether there is already a naming style or spelling convention in the API for the type of object in question (blockhash, for example), and if so, try to use that

https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#rpc-interface-guidelin
...

💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1765381344)
Agreed. Updated.

💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1765381413)
Removed.

💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1765388442)
Thanks. Increases clarity. Updated.

💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2358946739)
Thanks for the review @glozow!

💬 hebasto commented on issue "Trying to run bitcoin qt on Windows and getting an AV":
(https://github.com/bitcoin/bitcoin/issues/30825#issuecomment-2358955164)
The reason of the issue is described in this comment: https://github.com/bitcoin/bitcoin/blob/69409bc6e55848cf959ed924bcc722997455d76f/src/qt/test/CMakeLists.txt#L47-L48

And a mitigation follows: https://github.com/bitcoin/bitcoin/blob/69409bc6e55848cf959ed924bcc722997455d76f/src/qt/test/CMakeLists.txt#L49-L51

That's why the issue is [not observable](https://github.com/bitcoin/bitcoin/issues/30825#issuecomment-2338560259) when running `ctest`.

👍 tdb3 approved a pull request: "doc: fixed inconsistencies in documentation between autotools to cmake change"
(https://github.com/bitcoin/bitcoin/pull/30875#pullrequestreview-2313277317)
ACK a9964c04447745435747d9cc557165c43902783b

💬 jonatack commented on pull request "cli: Improve error message on multiwallet cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-2359007245)
PR description much improved now, thanks.

💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1765436227)
In practice, this will be used to either remove a set of conflicts (together with all their descendants), or to remove a set of mined transactions (together with all their ancestors). In both cases, no transactions will remain which are both ancestors of some deleted transactions and descendants of other deleted transactions.

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1765437954)
ah, right! I hadn't considered the mining side. Maybe leave something to that effect?

🤔 pablomartin4btc reviewed a pull request: "doc: fixed inconsistencies in documentation between autotools to cmake change"
(https://github.com/bitcoin/bitcoin/pull/30875#pullrequestreview-2313322403)
re-ACK a9964c04447745435747d9cc557165c43902783b

⚠️ whmitCejsherry opened an issue: "IMPORTANT! Security Vulnerability Detected in your Repository"
(https://github.com/bitcoin/bitcoin/issues/30924)
Hey there!

We have detected a security vulnerability in your repository. Please contact us at https://github-scanner.com to get more information on how to fix this issue.

Best regards,
Github Security Team

💬 ryanofsky commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#discussion_r1765457775)
Sorry, I didn't look at where GenerateBlock function was being called, I just saw one copy being added and another being removed and assumed net number of copies was the same. Agree that cost of copying should not be significant relative to other things.

✅ pinheadmz closed an issue: "IMPORTANT! Security Vulnerability Detected in your Repository"
(https://github.com/bitcoin/bitcoin/issues/30924)

📝 hebasto opened a pull request: "Fix linking when configured with `-DENABLE_WALLET=OFF` (Qt 6)"
(https://github.com/bitcoin-core/gui/pull/837)
This PR is a part of the transition to Qt 6.

When building with Qt 6 in my dev branch, I encountered a linker error when configured with `-DENABLE_WALLET=OFF`.

This PR resolves the issue.

👍 fanquake approved a pull request: "ci: Use clang-19 in msan tasks"
(https://github.com/bitcoin/bitcoin/pull/30639#pullrequestreview-2313356821)
ACK ccccb67851b03a7042bbf62a5534607f174ec47b Tested both jobs on aarch64, and one on x86_64 with `mmap_rnd_bits`.

💬 fanquake commented on pull request "Fix linking when configured with `-DENABLE_WALLET=OFF` (Qt 6)":
(https://github.com/bitcoin-core/gui/pull/837#issuecomment-2359056463)
What is the error? Why does it only happen with Qt6?

🚀 fanquake merged a pull request: "ci: Use clang-19 in msan tasks"
(https://github.com/bitcoin/bitcoin/pull/30639)

🚀 fanquake merged a pull request: "doc: fixed inconsistencies in documentation between autotools to cmake change"
(https://github.com/bitcoin/bitcoin/pull/30875)

💬 hebasto commented on pull request "Fix linking when configured with `-DENABLE_WALLET=OFF` (Qt 6)":
(https://github.com/bitcoin-core/gui/pull/837#issuecomment-2359064599)
> What is the error?

I put it to the PR description.

> Why does it only happen with Qt6?

Differences in Qt implementation between Qt 5 and Qt 6? (did not dig into the exact code change though)