💬 sr-gi commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1569461340)
in 91f4efa420958a93f4620379f8830231f276b23b

This should also be comparable by `CTransactionRef`, shouldn't it? So `GetWitnessHash` doesn't need to be called here and in following loops

💬 sr-gi commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1569471367)
in: 91f4efa420958a93f4620379f8830231f276b23b

nit: Use `contains` instead of `count(...) > 0` for consistency with the previous check (previous loop)

👍 pinheadmz approved a pull request: "test: extend the SOCKS5 Python proxy to actually connect to a destination"
(https://github.com/bitcoin/bitcoin/pull/29420#pullrequestreview-2006844953)
ACK 9c50b2fe44e4744204c51b82e86174db1d749cfe

Reviewed each commit and left some questions below. Built and ran all unit and functional tests on arm/macos. I am already familiar with this feature from #29415 and have run those tests as well.

One nitty nitty nit nit: commit message 9a9b545c7fc044242c4dd8a4916b2ba13b3ea850 uses c++ syntax instead of python ;-)


<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK 9c50b2fe44e4744204c51b
...

💬 pinheadmz commented on pull request "test: extend the SOCKS5 Python proxy to actually connect to a destination":
(https://github.com/bitcoin/bitcoin/pull/29420#discussion_r1569322866)
https://github.com/bitcoin/bitcoin/pull/29420/commits/1e76be04318a484124b82199260f23b72801e459

any reason for this particular buffer size? some quick research suggests that 4kB and 8kB are common values for this, I'm just wondering if it really affects us.

💬 pinheadmz commented on pull request "test: extend the SOCKS5 Python proxy to actually connect to a destination":
(https://github.com/bitcoin/bitcoin/pull/29420#discussion_r1569436155)
https://github.com/bitcoin/bitcoin/pull/29420/commits/1e76be04318a484124b82199260f23b72801e459

Instead of the counter, you could also use `dest = self.destiantions.pop()` below and then log your warning once the array is empty

💬 pinheadmz commented on pull request "test: extend the SOCKS5 Python proxy to actually connect to a destination":
(https://github.com/bitcoin/bitcoin/pull/29420#discussion_r1569289481)
https://github.com/bitcoin/bitcoin/pull/29420/commits/9a9b545c7fc044242c4dd8a4916b2ba13b3ea850

Why do we need to replace these values?
I changed this to:
```python
assert self.dstaddr == them[0]
assert self.dstport == them[1]
```
...and all the p2p tests still passed

💬 pinheadmz commented on pull request "test: extend the SOCKS5 Python proxy to actually connect to a destination":
(https://github.com/bitcoin/bitcoin/pull/29420#discussion_r1569437411)
https://github.com/bitcoin/bitcoin/pull/29420/commits/1e76be04318a484124b82199260f23b72801e459

had to read the docs on this one but I agree it is correct 👍

🤔 alfonsoromanz reviewed a pull request: "Bugfix - don't allow multiple dialogs for same tx in TransactionView"
(https://github.com/bitcoin-core/gui/pull/817#pullrequestreview-2007054657)
Tested ACK b26d2e81ec6ac4b5bd97bc7f7ec0c44e502b6a18. Only one dialog is opened for a single tx.

💬 alfonsoromanz commented on pull request "Bugfix - don't allow multiple dialogs for same tx in TransactionView":
(https://github.com/bitcoin-core/gui/pull/817#discussion_r1569483043)
The dialog is not brought to the foreground in my case (I am using Mac). In order to make it work I had to call `raise()` instead. Thoughts?

```suggestion
dlg->raise()
```

🤔 mzumsande reviewed a pull request: "test: Fix intermittent issue in p2p_handshake.py"
(https://github.com/bitcoin/bitcoin/pull/29898#pullrequestreview-2007043243)
Concept ACK

💬 mzumsande commented on pull request "test: Fix intermittent issue in p2p_handshake.py":
(https://github.com/bitcoin/bitcoin/pull/29898#discussion_r1569470049)
I think it would be more stable to have a global counter, so that `peer_id` is never reused. Currently, it is still reused over different `test_desirable_service_flags` calls.
E.g., if I add the net sleep and also change `CONNECTION_TYPES = ["outbound-full-relay"]` so that `test_desirable_service_flags` only makes one connection, I run into the timeout even with your fix.

🤔 glozow reviewed a pull request: "fuzz: explicitly cap the vsize of RBFs for diagram checks"
(https://github.com/bitcoin/bitcoin/pull/29879#pullrequestreview-2007103312)
ACK 016ed248ba0ae64e3f0c93bb47a2cd9b5e49cd85

💬 Joudyadam commented on pull request "deploy: remove some tools when cross-compiling for macOS":
(https://github.com/bitcoin/bitcoin/pull/29890#issuecomment-2062469035)
how can i withdrawl my 3500 USDT from okx wallet to binance please help me.
12 wallet phrase:

family nature fashion project scrub obscure bus crop coconut ship person winner

💬 Joudyadam commented on pull request "[26.x] archive 26.1 release notes + backports":
(https://github.com/bitcoin/bitcoin/pull/29899#issuecomment-2062471732)
how can i withdrawl my 3500 USDT from okx wallet to binance please help me.
12 wallet phrase:

family nature fashion project scrub obscure bus crop coconut ship person winner

🤔 furszy reviewed a pull request: "script/sign: avoid duplicated signature verification after signing (+introduce signing benchmarks)"
(https://github.com/bitcoin/bitcoin/pull/28923#pullrequestreview-2007381946)
Code ACK 3a3ccf00f0b

🤔 marcofleon reviewed a pull request: "fuzz: explicitly cap the vsize of RBFs for diagram checks"
(https://github.com/bitcoin/bitcoin/pull/29879#pullrequestreview-2007557346)
I ran into this bug when I first started fuzzing `package_rbf` a couple weeks ago... was unsure of what to do about it at the time. But glad to see it addressed here.

ACK 016ed248ba0ae64e3f0c93bb47a2cd9b5e49cd85. I ran libFuzzer on `package_rbf` on the current master branch until the overflow was encountered. Then I built the PR branch and ran the fuzzer using the crash input.
```bash
FUZZ=package_rbf ./src/test/fuzz/fuzz ../crash-d96abc99c0e424a47aa8f4080040e1bc3e3851f7
```
No crash.
``
...

💬 kosuodhmwa commented on issue "Tons of Socks5() connect to x.x.x.x:8333 failed: connection refused-messages when i use TOR - why?":
(https://github.com/bitcoin/bitcoin/issues/29759#issuecomment-2062862158)
Maybe it's an idea to check these servers (they will be listed via console output) via "telnet [IP_ADDRESS] 8333"...

If i dont get an anwser from telnet (TCP), then the problem is on these hosts / servers. If i get an answser, there is a problem on my side

💬 pablomartin4btc commented on pull request "Bugfix - don't allow multiple dialogs for same tx in TransactionView":
(https://github.com/bitcoin-core/gui/pull/817#discussion_r1569924233)
Thanks for reviewing and spotting that. I tried on Ubuntu to call **only** `dlg->raise()` but doesn't seem to work (also try other things like `dlg->setFocus()`, `dlg->setFocusPolicy(Qt::StrongFocus)`, `dlg->focusWidget()` and some combinations), until I found this in the QT documentation about [activateWindow()](https://doc.qt.io/qt-5/qwidget.html#activateWindow) (which I should have checked first):

_This function performs the same operation as clicking the mouse on the title bar of a top-le
...

💬 stratospher commented on pull request "test: Fix intermittent issue in p2p_handshake.py":
(https://github.com/bitcoin/bitcoin/pull/29898#discussion_r1569994243)
makes sense, less diff too. but `peer_id` is bounded to [0, 11]/`p2p_idx = peer_id + 1` is bounded to [1, 12] [here](https://github.com/bitcoin/bitcoin/blob/dbd2000b34903b87ae2e02eb2fc6c4a2f7d11451/test/functional/test_framework/p2p.py#L745). so we'd cross that limit if we do global counters.

💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for Spend":
(https://github.com/bitcoin/bitcoin/pull/28236#issuecomment-2063026719)
> Are you still working on this?

Sorry I got a little occupied with some other things and would want to continue working on this. I've the approach in mind and will force push the update by this end of this month.

- Will create a new file for `CoinsResult ` and remove global Singleton from the `Spend` file.