💬 jarolrod commented on issue "cmake: adjust devtools scripts to work under cmake build environment":
(https://github.com/bitcoin/bitcoin/issues/30923#issuecomment-2358904932)
documentation changes in the readme are sufficient imo, closing as is.

💬 mcelrath commented on issue "Race condition between ZMQ UpdateTip and getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/30862#issuecomment-2358912616)
Not yet. I've been running my morning script all week and haven't seen it
happen again.

On Wed, Sep 18, 2024, 12:14 PM maflcko ***@***.***> wrote:

> A better test would be to reproduce this with regtest which I'll create a
> script to demonstrate soon.
>
> Did you get a chance to write the reproducer?
>
> But at the same time it seems there should be a mutex around the chain
> tip. If there isn't this is basically guaranteed to happen. I haven't
> looked yet.
>
> Yes, the mutex i
...

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

Tried to find other inconsistencies, but couldn't.

💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1765381096)
Thanks, yeah it's a bit too implicit so I added `assert_equal(len(node.getorphantxs()), 1)` to make it clearer and more explicit.

💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1765381138)
Seemed reasonable to add a check `assert not node.tx_in_orphanage(tx_child_2["tx"])` rather than a new method, but happy to be convinced otherwise.

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

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