💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1964166748)
seems like we're leaving only one of 3 loops? is this intended?

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1964465497)
IIUC this is basically a way of slowing down the parent resolution of a target package by 2 minutes, thereby increasing the window of possible random eviction when under real cpfp traffic.

Without this delay we can still have natural overflow if there are too many (or too large) real packages in flight, the time window here just gets a lot longer

Unfortunately we have no way of peers to communicate which orphans they claim are higher paying (to preferentially evict those last f.e.). Maybe
...

💬 ryanofsky commented on issue "intermittent ipc_tests (Timeout)":
(https://github.com/bitcoin/bitcoin/issues/31921#issuecomment-2672910522)
Seems possible this is the same bug as https://github.com/chaincodelabs/libmultiprocess/issues/154 but causing a deadlock instead of a "mutex lock failed" error due to platform differences. The fix for the bug is https://github.com/chaincodelabs/libmultiprocess/pull/159 and is included in 1746618e081d582673b458091c46bb3f71ba3a56 from #31741. But maybe I should make that commit a separate PR?

💬 loriwwa11 commented on issue "crypto: secure erase memory":
(https://github.com/bitcoin/bitcoin/issues/31744#issuecomment-2673031884)
Still can't believe I'm typing this. First started trading crypto in 2020, mostly small gains and losses until this year's bull run. I finally consolidated all my accounts/wallets and realized I crossed a net worth of $1.5M! 
Here’s my breakdown as of today:
* Crypto: $1.2M (started with $50k)
* WIF: $379k
* HYPE: $289k (got airdropped $150k for using HyperLiquid)
* DOGE: $279k
* BTC: $84k
* SOL: $71k
* ETH: $67k
* Some random other meme coin trades: $54k
* Stocks / E
...

💬 hugohn commented on pull request "Revert merge of PR #31826":
(https://github.com/bitcoin/bitcoin/pull/31908#issuecomment-2673063011)
> To be clear my point is not that this isn't worth fixing. The question is where.

> I think it's ridiculous to have to implement a fix for a CPU issue in all end-user software. But it's not totally unheard of. These kind of model-specific workarounds happen a lot in gamedev GPU programming, historically a wasteland of out-of-spec hardware and drivers, but it's somewhat worrying to see it crossing to CPUs which you'd expect to have a higher standard.

> As said, ideally a fix would land in
...

💬 hugohn commented on pull request "Revert merge of PR #31826":
(https://github.com/bitcoin/bitcoin/pull/31908#issuecomment-2673103583)
Some additional context: the PR that introduced RNDR/RNDRRS support for AArch64 ([#26839](https://github.com/bitcoin/bitcoin/pull/26839)) was merged about a year ago, and AFAIK it was only tested on Amazon Graviton 3 before merging—so it’s not like the feature itself has been extensively battle-tested across different SoCs.

If someone feels this fix is too specific to a single chip and wants to revert it, the same reasoning could apply to the original PR itself. That might imply reverting the
...

💬 yancyribbens commented on pull request "scripted-diff: Type-safe settings retrieval":
(https://github.com/bitcoin/bitcoin/pull/31260#discussion_r1964613599)
nit: is `definiting-time` a typo? maybe `definition-time`?

💬 yancyribbens commented on pull request "qa: clarify and document assumeutxo tests":
(https://github.com/bitcoin/bitcoin/pull/31907#discussion_r1964626252)
Ah, I see thanks. I guess the underlying call would be more clear to return `coin(s)` although that's besides the point.

👋 jirijakes's pull request is ready for review: "doc: Fix and clarify description of ZMQ message format"
(https://github.com/bitcoin/bitcoin/pull/31862)

💬 jirijakes commented on pull request "doc: Fix and clarify description of ZMQ message format":
(https://github.com/bitcoin/bitcoin/pull/31862#issuecomment-2673161731)
Ready for review again.

When trying to use ZMQ, I realized that the description of `sequence` was not clear about the format of the message. Therefore I expanded this PR to also clarify that.

This PR now:

- clarifies that all messages share the same structure (three parts)
- adds that sequence numbers are distinct for each topic
- replaces endianness of 32-byte hashes with their byte order
- puts note about byte order before specification of body formats
- from descriptions of topic
...

⚠️ yancyribbens opened an issue: "Fuzzing Bitcoin Core with clang-16"
(https://github.com/bitcoin/bitcoin/issues/31922)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

building fails during linking CXX executable bitcoin-wallet
```
[ 27%] Building CXX object src/util/CMakeFiles/bitcoin_util.dir/__/sync.cpp.o
[ 27%] Linking CXX static library libbitcoin_util.a
...

💬 yancyribbens commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1964890610)
nit missing period after match

💬 yancyribbens commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1964892210)
nit period after `ALL`

💬 1440000bytes commented on pull request "Implement OP_CHECKSIGFROMSTACK(VERIFY)":
(https://github.com/bitcoin/bitcoin/pull/29270#discussion_r1964906975)
Maybe named constants can be used instead of 0,32 and 64. I am not sure if this works according to coding practices used in this repository.

https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md

Or a comment.

💬 Sjors commented on pull request "fuzz: add targets for PCP and NAT-PMP port mapping requests":
(https://github.com/bitcoin/bitcoin/pull/31676#issuecomment-2673832941)
Given that https://github.com/bitcoin/bitcoin/pull/31916 plans to enable PCP for a subset of GUI users, this PR should probably be marked v29.

💬 maflcko commented on issue "intermittent ipc_tests (Timeout)":
(https://github.com/bitcoin/bitcoin/issues/31921#issuecomment-2673834652)
Yeah, it looks like https://github.com/bitcoin/bitcoin/pull/31741 needs rebase and feature freeze was yesterday, so I am not sure if it is going to make it in as a bugfix into 29.x.

Might be best carve out the bugfix commit for now.

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2673834899)
`69e076664d...76feae207d`: rebase due to conflicts and allow to log but not treat as an error detected internet traffic (`$INTERNET_TRAFFIC_EXPECTED` mentioned above). Can be used when running manually outside of CI where other programs on the host can generate internet traffic.

💬 Sjors commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#issuecomment-2673838834)
re-ACK 61885ad9d406e25ec3b6523d01918e886996f1c3

Dropped the else: https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1963881147

💬 laanwj commented on pull request "init: Handle dropped UPnP support more gracefully":
(https://github.com/bitcoin/bitcoin/pull/31916#issuecomment-2673844440)
> I think it is a little bit unfortunate that there might be some users of the GUI who don't check the release notes or debug.log whose networks did support UPnP but don't support NAT-PMP or PCP will never be warned that their port forwarding setup is broken

> Maybe a better and more general solution to that class of problem that is outside the scope of this PR and the issue it resolves is to provide some kind of indication to users about whether or not their node is reachable, maybe using a
...

💬 laanwj commented on pull request "init: Handle dropped UPnP support more gracefully":
(https://github.com/bitcoin/bitcoin/pull/31916#discussion_r1965038094)
No, that's not intended. i'll look into a way to query the json only.