💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1951962043)
not entirely sure... could be cleaned up

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1951964695)
made consistent + made both logs %u

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1951964799)
fixed

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1951965002)
(thanks, I took your test with just a few tweaks)

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1951966069)
Added a couple pushes ago, but gone again. After a bit of offline discussion with @mzumsande and @sipa, it seemed better to just synchronously remove wtxids from worksets when they are removed as announcements. This means that the work set is always a subset of announcement set (added this to `SanityCheck`). Also, potentially failing to add things to work set seemed to make this less useful.

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1951966734)
Yes, they're both meant to be ratios :+1:

💬 davidgumberg commented on pull request "Benchmark Chainstate::ConnectBlock duration":
(https://github.com/bitcoin/bitcoin/pull/31689#discussion_r1952062784)
not blocking, just a question: here and below, why `reserve()` for vectors that are outside of the benchmark's path?

💬 giahuy98 commented on pull request "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop":
(https://github.com/bitcoin/bitcoin/pull/31826#issuecomment-2652823871)
> re-ACK [585aba6](https://github.com/bitcoin/bitcoin/commit/585aba6eec858e5b1411ae9a8684ef8f82a7e435)
>
> i could test it on amazon graviton again like i did the original PR, but only the good-weather case, that won't reproduce the edge case that led to this.
>
> Would be good if @giahuy98 (who opened #31817) could test it on the specific target hardware.

I tested [585aba6](https://github.com/bitcoin/bitcoin/commit/585aba6eec858e5b1411ae9a8684ef8f82a7e435) on a Samsung Galaxy S25, and
...

💬 maflcko commented on pull request "contrib: Add deterministic-fuzz-coverage":
(https://github.com/bitcoin/bitcoin/pull/31836#issuecomment-2652981989)
Went ahead and added the other check (as well as comments to explain the reasoning). I hope this addresses all outstanding feedback.

💬 ajtowns commented on pull request "doc: improve NODE_NETWORK_LIMITED documentation per BIP159":
(https://github.com/bitcoin/bitcoin/pull/31805#discussion_r1952205343)
Dropping "; signals a pruned ... (see BIP 159)" seems clearer, and matches the definition of `NETWORK_LIMITED` provided by [BIP 159](https://github.com/bitcoin/bips/blob/master/bip-0159.mediawiki).

🚀 fanquake merged a pull request: "guix: remove test-security/symbol-check scripts"
(https://github.com/bitcoin/bitcoin/pull/31818)

✅ fanquake closed an issue: "contrib: Autoconf fragments left in test-*-check scripts"
(https://github.com/bitcoin/bitcoin/issues/31698)

💬 naiyoma commented on pull request "rpc: print P2WSH and P2SH redem Script in getrawtransaction":
(https://github.com/bitcoin/bitcoin/pull/31252#discussion_r1951583490)
the value of `have_undo` is always going to be 0 when calling` getrawtransaction` hence the reason why this only works for `getblock..2`

💬 naiyoma commented on pull request "rpc: print P2WSH and P2SH redem Script in getrawtransaction":
(https://github.com/bitcoin/bitcoin/pull/31252#discussion_r1952236280)
This is not a suggestion to replace your current approach, but rather an alternative way. I have an implementation here ->[https://github.com/naiyoma/bitcoin/pull/1/commits/ff82e551404e55c75b1950e01e4f4626f5ad452e](https://github.com/naiyoma/bitcoin/pull/1/commits/ff82e551404e55c75b1950e01e4f4626f5ad452e) that shows how to decompile outside of `if(have_undo) `. While it needs more testing, it successfully works for both `getrawtransaction `and `getblock..2`. You might find some ideas there for
...

💬 naiyoma commented on pull request "rpc: print P2WSH and P2SH redem Script in getrawtransaction":
(https://github.com/bitcoin/bitcoin/pull/31252#discussion_r1952168882)
this outputs `redeemScript` for both P2SH and P2WSH, but it should be
P2SH `redeemScript` and P2WSH `witnessScript`

💬 Sjors commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2653056523)
I'm unable to apply the code signatures. E.g. for arm64:

```
HOSTS="arm64-apple-darwin" ./contrib/guix/guix-codesign
Checking that we can connect to the guix-daemon...

Hint: If this hangs, you may want to try turning your guix-daemon off and on
again.

INFO: Codesigning 096525e92cc2 for platform triple arm64-apple-darwin:
...using reference timestamp: 1733177891
...from worktree directory: '/home/sjors/bitcoin'
...bind-mounted in container to: '/bitcoin
...

👍 vasild approved a pull request: "multiprocess: Add libmultiprocess git subtree"
(https://github.com/bitcoin/bitcoin/pull/31741#pullrequestreview-2611252684)
ACK c6658d675d0bb945f53dc62f7e9b95c7bba973bb

> `CAPNP_EXECUTABLE` and `CAPNPC_CXX_EXECUTABLE` ... `multiprocess.md` should mention them as well

:+1: I guess in the "Alternately ..." paragraph.

💬 Sjors commented on pull request "test: clarify timewarp grace period griefing attack":
(https://github.com/bitcoin/bitcoin/pull/31725#discussion_r1952264789)
> more stricter equality.

Stricter than what? I don't fully remember what the original test was trying to do here. But we know the exact value to expect.

`curtime` is equal to `nTime` of the template, which is controlled by `UpdateTime()` in `node/miner.cpp`. It starts with the actual current time, but then increases if needed to account for the MTP+1 rule and timewarp - and no more than that.

💬 Sjors commented on pull request "test: clarify timewarp grace period griefing attack":
(https://github.com/bitcoin/bitcoin/pull/31725#discussion_r1952265557)
Indeed, fixed.

💬 Sjors commented on pull request "test: clarify timewarp grace period griefing attack":
(https://github.com/bitcoin/bitcoin/pull/31725#discussion_r1952265961)
Changed the log statement to `<=`