🤔 furszy reviewed a pull request: "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response"
(https://github.com/bitcoin/bitcoin/pull/32517#pullrequestreview-3393853741)
ACK ab06c3f16063701cfa25504a0d3cd4d5a5eb3a97

💬 furszy commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2473437500)
nit:
Missing ref here `const CTxOut& txout`
```suggestion
/*is_change_func=*/[&pwallet](const CTxOut& txout) NO_THREAD_SAFETY_ANALYSIS {
```

Also, why not `EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)` instead of `NO_THREAD_SAFETY_ANALYSIS`?

💬 pinheadmz commented on issue "dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us appears to be violating DNS seed policy":
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3461865877)
> [!CAUTION]
> Personal attacks are not allowed and will result in a ban

- Be extremely clear and concise when posting on this thread.
- Every comment must relate directly to the topic described in the issue description
- Be mature, respectful and sensible.

💬 Rob1Ham commented on issue "dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us appears to be violating DNS seed policy":
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3461875545)
Calling out @kanzure's comment in [#33723 ](https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3461716939) to ask the question on if there are advantages to having DNS seeds that are more conservative with their versioning.

> In the event that this DNS seed server is removed from the configuration, it's worth considering (or considering the effects of not) replacing it with another DNS seed node that [implements a conservative strategy with regards to node versions](https://delvingbitco
...

👋 maflcko's pull request is ready for review: "ci: Call docker exec from Python script to fix word splitting"
(https://github.com/bitcoin/bitcoin/pull/33732)

💬 Crypt-iQ commented on issue "RFC: Do we want to support fuzzing on MacOS?":
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3461908503)
> The only downside of it would be not attracting macOS people to fuzzing development (?)

I was also thinking about this, and it could turn away some people from writing fuzz tests. Maybe if the docs mentioned a specific working version as suggested by @maflcko above, but then somebody would need to test these versions. I've had issues where even updating to a new macOS version isn't possible without some hacks.

💬 fanquake commented on pull request "ci: run native fuzz with MSAN job":
(https://github.com/bitcoin/bitcoin/pull/33626#issuecomment-3461989252)
`-md` runtime was 43 minutes with no caches (libccxx/depends/cacche).

💬 furszy commented on issue "RFC: Do we want to support fuzzing on MacOS?":
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3461995390)
It’s definitely painful to run it on Mac. I'm not against officially deprecating it if that is the general consensus but it’s quite useful to be able to troubleshoot individual crashes/timeouts locally. It would be handy to have pinned versions with specific docs on how to make it work.

💬 maflcko commented on issue "Can I compile on OSX Tahoe?":
(https://github.com/bitcoin/bitcoin/issues/33733#issuecomment-3462061936)
What is the configure summary, before you start to compile? (The output of `cmake -B build -DENABLE_IPC=OFF -DBUILD_GUI=OFF -DWITH_QRENCODE=OFF`)

What is your XCode version?

You are certainly not using the brew installed boost lib.

What other boost libs are on your system?

💬 maflcko commented on pull request "random: scope environ extern to macOS, BSDs and Illumos":
(https://github.com/bitcoin/bitcoin/pull/33714#discussion_r2472906261)
nit: I think the comment can be kept as-is, or just refer to "Necessary on the above platforms". Otherwise, it is just one more line to maintain.

Also, I wonder if this can just be checked at configure-time, but no strong opinion.

💬 kanzure commented on issue "dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us appears to be violating DNS seed policy":
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3462103592)
I also have a question as to whether user agent version filtering was previously discussed as a matter of DNS seed policy. If it was, then perhaps someone would be kind enough to give a link to that discussion. This however seems to me like a smaller issue compared to the other considerations that are necessary for resolving this ticket.

> We also learned of an investigation following the hack and are unsure whether Luke has shared access to the server with investigators. It would be helpful if
...

💬 stringintech commented on pull request "test: Add bitcoin-chainstate test for assumeutxo functionality":
(https://github.com/bitcoin/bitcoin/pull/33728#discussion_r2473721206)
I thought if I want to treat it as boolean, it might be better to introduce it as a flag with the dash prefix. Would it make more sense to you if I drop the dash and treat it as a chain name instead (where only regtest and mainnet are supported, and if absent, defaults to mainnet)?

💬 fanquake commented on pull request "random: scope environ extern to macOS, BSDs and Illumos":
(https://github.com/bitcoin/bitcoin/pull/33714#discussion_r2473728115)
Re-modified the comment. Not sure there is too much value in moving this to configure.

💬 stringintech commented on pull request "test: Add bitcoin-chainstate test for assumeutxo functionality":
(https://github.com/bitcoin/bitcoin/pull/33728#discussion_r2473728951)
Yes I think it is better to have validation on this.

🤔 Crypt-iQ reviewed a pull request: "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer"
(https://github.com/bitcoin/bitcoin/pull/32606#pullrequestreview-3393285716)
I agree with @dergoegge that it's easy to become a HB peer if you're already the first to send unsolicited compact blocks to try and fingerprint a node's mempool. I think the benefit with this change is being more strict with what we'll accept and following the protocol closer (and I think there's even more improvements we can make). Left some test-related nits.

💬 Crypt-iQ commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#discussion_r2473220795)
nit: can remove this line, `stalling_peer` is already HB

💬 Crypt-iQ commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#discussion_r2472976009)
These two lines can be removed if `self.segwit_node` is marked as HB above?

💬 Crypt-iQ commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#discussion_r2473738447)
Future improvements to fully "lock down" this code path could be to:
- check `m_opts.ignore_incoming_txs`
- check `m_provides_cmpctblocks` is set
- check that if the block was requested, we used `MSG_CMPCT_BLOCK` (requires state)

💬 Crypt-iQ commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#discussion_r2472970994)
Since `self.segwit_node` was disconnected in the prior test it's not yet HB, plus it doesn't look like it has sent over `sendcmpct` yet. I agree that we should assert that it is marked as HB though.

💬 Crypt-iQ commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#discussion_r2473635706)
`unsolicited_peer` also hasn't sent `sendcmpct` so its `m_provides_cmpctblocks` is false, and the node still processes the compact block! I think it is useful to have coverage for this case.