💬 pinheadmz commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2931233423)
> This PR will not queue new private broadcast connections if the same tx is submitted again.

Where is this implemented in the PR?

🤔 maflcko reviewed a pull request: "ci, iwyu: Treat warnings as errors for specific directories"
(https://github.com/bitcoin/bitcoin/pull/31308#pullrequestreview-2888771192)
review ACK e9143a68f34f350d7fa4405389222da4dfa9a394 🎰

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK e9143a68f34f
...

💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121478117)
5cd4bbb67c3c7e651b3d83c77ffb936f60a2c496: Looks wrong. This should be a fwd dcl

💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121385023)
nit in the first commit: Seems odd to exclude this without explanation in the code. Would an alternative be:

```diff
diff --git a/ci/test/03_test_script.sh b/ci/test/03_test_script.sh
index 3394c50b8b..db3d3d64de 100755
--- a/ci/test/03_test_script.sh
+++ b/ci/test/03_test_script.sh
@@ -33,7 +33,9 @@ echo "=== BEGIN env ==="
env
echo "=== END env ==="

-(
+if [ "$RUN_TIDY" != "true" ]; then
+ # The tidy task has no UB detector, so skip the patch there to avoid issues
+ # late
...

💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121492797)
nit in eb4be21cc0bb93b0931a61b2af342946f09a9721: Wrong section and should be cstring to avoid confusing with string.h in our repo?

💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121493309)
same...

💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121507603)
e9143a68f34f350d7fa4405389222da4dfa9a394: Why not just `git diff --no-pager --exit-code`?

💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121387189)
q in the first commit: This is no longer needed, because cmake uses absolute paths in the compile commands?

💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2931272929)
@pinheadmz in [ScheduleTxForPrivateBroadcast](https://github.com/bitcoin/bitcoin/pull/29415/files#diff-6875de769e90cec84d2e8a9c1b962cdbcda44d870d42e4215827e599e11e90e3R2295-R2313) we check if the tx was inserted into the private broadcast queue, and only then do we queue more private broadcast connections to open. The tx is removed from the private broadcast queue when we receive the tx in our mempool [here](https://github.com/bitcoin/bitcoin/pull/29415/files#diff-6875de769e90cec84d2e8a9c1b962cd
...

💬 maflcko commented on pull request "rpc: Use type-safe HelpException":
(https://github.com/bitcoin/bitcoin/pull/32660#discussion_r2121522700)
I am just using clang-format, so if you want to change that, please do so in https://github.com/bitcoin/bitcoin/blob/1c6602399be6de0148938a3fd7b51e6c48b985d2/src/.clang-format#L17-L20

💬 pinheadmz commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2931283927)
Ok and then after the tx is received in mempool and removed from queue, I send it again. I think it's probably harmless but we will continue to re-send the tx until it finally gets confirmed and then `ReattemptPrivateBroadcast()` will catch it as txn-already-known

💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2931287755)
> Ok and then after the tx is received in mempool and removed from queue, I send it again.

But `sendrawtransaction` will fail if the tx is already in our mempool, no? How will this succeed?

💬 pinheadmz commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2931306622)
> But `sendrawtransaction` will fail if the tx is already in our mempool, no? How will this succeed?

Are you sure about this?

```

// There's already a transaction in the mempool with this txid. Don't
// try to submit this transaction to the mempool (since it'll be
// rejected as a TX_CONFLICT), but do attempt to reannounce the mempool
// transaction if broadcast_method is not ADD_TO_MEMPOOL_NO_BROADCAST.
//
//
...

👋 m3dwards's pull request is ready for review: "test: enabling wallet migration functional test on windows"
(https://github.com/bitcoin/bitcoin/pull/32219)

💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2931340529)
> Are you sure about this?

I was not sure, thanks for the correction :). I see we only check for acceptance if the tx is not already in our mempool. If it is, then we skip checking acceptance and try and broadcast again. That behavior makes sense to me, and I think this is another reason why we should treat 3 minute timeouts waiting for `GETDATA` to be a successful private broadcast connection.

📝 RandyMcMillan opened a pull request: "uint256 cxx-20 constexpr patch"
(https://github.com/bitcoin/bitcoin/pull/32663)

💬 mabu44 commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2121610418)
nHeight is indeed an int. Am I missing something?

👍 ryanofsky approved a pull request: "depends: hard-code necessary c(xx)flags rather than setting them per-host"
(https://github.com/bitcoin/bitcoin/pull/32584#pullrequestreview-2889108835)
Code review ACK 51f6aa8ac47b7b5412553af2e7152d250a858d0d. This seems like a good change that deduplicates host definitions, and removes nonsensical inclusion of `-std` in host options rather than package options, since c/c++ standard versions should not vary by host.

---

IIUC, this change is mostly just a refactoring. For example, it rearranges the libevent flags shown by `make print-libevent_cflags` from:

```sh
libevent_cflags=-pipe -std=c11 -O2 -fdebug-prefix-map=...
```

to:

`
...

💬 benthecarman commented on pull request "rpc: Note in fundrawtransaction doc, fee rate is for package":
(https://github.com/bitcoin/bitcoin/pull/32607#issuecomment-2931455300)
impl'd @achow101's suggestions, also added missing `\n` at end of the doc

📝 RandyMcMillan converted_to_draft a pull request: "uint256 cxx-20 constexpr patch"
(https://github.com/bitcoin/bitcoin/pull/32663)