💬 achow101 commented on pull request "Fix peers abruptly disconnecting from AssumeUTXO nodes during IBD":
(https://github.com/bitcoin/bitcoin/pull/30807#issuecomment-2332722279)
ACK bb08c227de158dbd88a80d904edb209e1734ab91

💬 achow101 commented on pull request "[28.x] rc backports":
(https://github.com/bitcoin/bitcoin/pull/30762#issuecomment-2332729594)
ACK b2a137929a20baed161988e24de592b1f59c0096

🚀 achow101 merged a pull request: "[28.x] rc backports"
(https://github.com/bitcoin/bitcoin/pull/30762)

📝 achow101 opened a pull request: "[28.x] Further backports and rc2"
(https://github.com/bitcoin/bitcoin/pull/30827)
* #30821

📝 achow101 converted_to_draft a pull request: "[28.x] Further backports and rc2"
(https://github.com/bitcoin/bitcoin/pull/30827)
* #30821

💬 ariard commented on pull request "Halt processing of unrequested transactions v2":
(https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2332788365)
@mzumsande

> I don't get this at all.
> The suggestion above was, if you receive an unrequested TX, and you already have it, ignore it but don't disconnect the
> peer. Which is the same as current behavior on master.

Let’s say you have Peer1 and Peer2 announcing the Transaction ABC, if ABC is concurrently fetch from both Peer1 and Peer2, the `GETDATA` (32-bytes) for ABC would have been wasted as outgoing bandwidth by Local Node. This 32 byte leak can be amplified if batch of transactio
...

📝 ismaelsadeeq opened a pull request: "interfaces: #30697 follow ups"
(https://github.com/bitcoin/bitcoin/pull/30828)

This PR addresses the remaining review comments from #30697

1. Disallowed overwriting settings values with a `null` value.
2. Uniformly used the `SettingsAction` enum in all settings methods instead of a boolean parameter.
3. Updated `overwriteRwSetting` to receive the `common::SettingsValue` parameter by value, enabling it to be moved safely.
4. Removed wallet context from the `write_wallet_settings_concurrently` unit test, as it is not needed.

💬 ismaelsadeeq commented on pull request "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface":
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1746265728)
Taken your suggestion @ryanofsky this is fixed in #30828

💬 ismaelsadeeq commented on pull request "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface":
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1746267705)
I've tested this locally by passing an unknown settings name this does not throw.

💬 ismaelsadeeq commented on pull request "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface":
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1746269227)
Taken in #30828

💬 ismaelsadeeq commented on pull request "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface":
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1746269588)
Removed in #30828

💬 ismaelsadeeq commented on pull request "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface":
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1746271611)
Agreed, I see no reason for allowing a null value.
Removed in #30828

💬 ismaelsadeeq commented on pull request "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface":
(https://github.com/bitcoin/bitcoin/pull/30697#discussion_r1746271896)
Done in #30828

💬 ismaelsadeeq commented on pull request "Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface":
(https://github.com/bitcoin/bitcoin/pull/30697#issuecomment-2332810776)
Thanks for your reviews, @stickies-v and @ryanofsky.
I've addressed the review comments in #30828.


> Thanks. I think a natural cleanup or follow-up to this PR could involve moving the `interfaces::Node` settings methods (`isSettingIgnored`, `getPersistentSetting`, `updateRwSetting`, `forceSetting`, and `resetSettings`) and the `interfaces::Chain` settings methods (`getSetting`, `getSettingList`, `getRwSetting`, `updateRwSetting`, `overwriteRwSetting`, `deleteRwSettings`) into a new `inte
...

🤔 ismaelsadeeq reviewed a pull request: "init: fix init fatal error on invalid negated option value"
(https://github.com/bitcoin/bitcoin/pull/30684#pullrequestreview-2284265281)
Concept ACK
will review

🤔 ajtowns reviewed a pull request: "signet: fixing mining for OP_TRUE challenge"
(https://github.com/bitcoin/bitcoin/pull/29032#pullrequestreview-2284282682)
Concept ACK

💬 ajtowns commented on pull request "signet: fixing mining for OP_TRUE challenge":
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r1746298241)
Might be better as something like:

```python
signet_spk = tmpl["signet_challenge"]
if trivial_challenge(signet_spk):
signet_solution = None:
else:
...
```

and

```
def trivial_challenge(spkhex):
"""BIP325 allows ..."""
if spkhex == "51":
return True
return False
```

💬 ajtowns commented on pull request "signet: fixing mining for OP_TRUE challenge":
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r1746296470)
`OP_RETURN` ?

💬 ajtowns commented on pull request "signet: fixing mining for OP_TRUE challenge":
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r1746301277)
Better to move `def generate_psbt(..)` down to its code, rather than moving the code up?

💬 ajtowns commented on pull request "signet: fixing mining for OP_TRUE challenge":
(https://github.com/bitcoin/bitcoin/pull/29032#discussion_r1746299406)
nit: I'd probably write this as

```python
if signet_solution is None:
pass # Don't need to add a signet commitment if there's no signet signature needed
else:
...
```