💬 josibake commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2207706515)
This could be moved to the output type commit.

💬 josibake commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2207717440)
I think its more appropriate to move this commit into the PR where we first introduce `V0SilentPaymentDestination`. I'll cherry pick this out into https://github.com/bitcoin/bitcoin/pull/28122

💬 josibake commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2207703326)
Feels like the SigningProvider should only care about the `spend_key + tweak` for signing, so another place to potentially simplify things if scan key is a custom type instead of a `CKey`.

🤔 maflcko reviewed a pull request: "kernel: Move block tree db open to block manager"
(https://github.com/bitcoin/bitcoin/pull/30965#pullrequestreview-3020935890)
sorry for missing the bug in review

💬 maflcko commented on pull request "kernel: Move block tree db open to block manager":
(https://github.com/bitcoin/bitcoin/pull/30965#discussion_r2207822121)
this is my fault for reviewing this without testing, but I don't think you got rid of the "previous loop", the two iterations can still happen and removing this will now deadlock the program.

You'll still have to reset and delete the pointer before constructing the new one.

💬 Sjors commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2207029910)
cb44e3812c741e3a939a64799130bbf0787ef89a : I think we should set `bool IsRange() const override { return false; }` so we don't have to think about ranged descriptors.

💬 waketraindev commented on pull request "rpc: add optional peer_ids param to filter getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/32741#issuecomment-3074170972)
Updated to use `peer_ids` (array) instead of a single `peer_id`. This allows filtering by multiple peers in one call and aligns better with RPC design. Functional tests adjusted.

📝 Galoretka opened a pull request: "fix: Python 3 bytes comparison in linearize-data.py"
(https://github.com/bitcoin/bitcoin/pull/32978)
Replace comparison of a bytes element to the string literal "\0" with an integer 0 in linearize-data.py to ensure correct behavior in Python 3. In Python 3, indexing a bytes object returns an integer, not a single-character string, so the previous comparison would always be false. This change improves compatibility and correctness of the script when detecting end-of-file or null bytes.

💬 maflcko commented on pull request "Set notifications m_tip_block in LoadChainTip()":
(https://github.com/bitcoin/bitcoin/pull/31346#discussion_r2207914726)
This is causing a crash in the GUI, when the user selects "Yes" to the question "Do you want to rebuild the databases now?":

```
init.cpp:1831 AppInitMain: Assertion `chainman.ActiveTip()' failed.
```

I presume the reason is that the first notification *before* the reindex here will trick the `kernel_notifications.m_tip_block_cv.wait` to skip immediately, thinking it is already good to continue.

However, it needs to wait for the second notification *after* the reindex.

A possible fix would
...

📝 maflcko opened a pull request: "test: Check that the GUI interactive reindex works"
(https://github.com/bitcoin/bitcoin/pull/32979)
Given that several bugs have been introduced in the last release alone (https://github.com/bitcoin/bitcoin/pull/31346#discussion_r2207914726, https://github.com/bitcoin/bitcoin/pull/30965#discussion_r2207822121, ...), it could make sense to test this code path.

💬 instagibbs commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3074256706)
utACK 2c2acb37ecdeed1582b9835f9075d44e313d787c

Easy to read and appears to comply with the BIP text. Logic seems sound, seems utterly unreasonable to make these kinds of transactions, and in general breaking up these types of transactions will result in transactions working again. This along with not seeing these on-chain for 10 years seems to suggest a minuscule confiscatory surface.

I'm still mulling over the larger accounting changes vs other possible tradeoffs, but this doesn't have to
...

💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3074275673)
re-ACK 2c2acb37ecdeed1582b9835f9075d44e313d787c

🤔 ismaelsadeeq reviewed a pull request: "cluster mempool: add TxGraph work controls"
(https://github.com/bitcoin/bitcoin/pull/32263#pullrequestreview-3020830689)
Code review ACK 62ed1f92efff42bc79c50935e6dbd9da4e072020

Just a question and some nits.

💬 ismaelsadeeq commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2207756051)
Why size two?
Isn't this supposed to be one? That is, individual splits are already optimal, but any component with size greater than one needs to be linearized, no?

💬 ismaelsadeeq commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2207883583)
this is a duplication because `AddDependency` already made `real_is_optimal` to be `false`

💬 ismaelsadeeq commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2207856137)
```suggestion
// Update the Cluster's quality if it has improved.
```

💬 ismaelsadeeq commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2207935885)
yeah will make internal comments of `DoWork` easier to follow as well.

💬 instagibbs commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2207956569)
It's already will be in a topologically valid ordering, and there's only a single ordering possible in a connected component of size 2, thus optimal.

💬 ismaelsadeeq commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2207967507)
Makes sense, I overlooked that even when parent pays for it self the ordering is the same.

💬 w0xlt commented on pull request "wallet: Remove `upgradewallet` RPC":
(https://github.com/bitcoin/bitcoin/pull/32944#discussion_r2207984864)
Done in https://github.com/bitcoin/bitcoin/pull/32944/commits/305884c859f717a85f5bf9e728c6382d929fc01b

✅ fanquake closed an issue: "[BTC signet v22.0] websocket not working as expect"
(https://github.com/bitcoin/bitcoin/issues/32848)