💬 theStack commented on pull request "test: Make existing functional tests compatible with --v2transport":
(https://github.com/bitcoin/bitcoin/pull/28805#discussion_r1390101144)
One small drawback of the current way of inspecting `extra_args` for the v2transport option is that it doesn't work if the argument is specified in a slightly different format with same meaning. For example, `-v2transport=0`/`-v2transport=1` could also be passed as `-nov2transport`/`-v2transport`, `-nov2transport=1`/`-nov2transport=0` (admittedly, no sane person would use the last one though). It might be worth it to consider that in a follow-up, e.g. by enforcing with an assertion that argument
...

👍 theStack approved a pull request: "test: Make existing functional tests compatible with --v2transport"
(https://github.com/bitcoin/bitcoin/pull/28805#pullrequestreview-1725906389)
ACK 35fb9930adb3501b29d3ad20d2e74c0114f2bcbe

💬 stratospher commented on pull request "test: Make existing functional tests compatible with --v2transport":
(https://github.com/bitcoin/bitcoin/pull/28805#discussion_r1390105880)
> "if no extra_args are passed, restart with the ones that were initially specified (self.extra_args)"

oh interesting! so https://github.com/bitcoin/bitcoin/pull/28805#discussion_r1388909333 is out of scope for this PR.

🤔 stratospher reviewed a pull request: "test: Make existing functional tests compatible with --v2transport"
(https://github.com/bitcoin/bitcoin/pull/28805#pullrequestreview-1725912169)
ACK 35fb993.

💬 sipa commented on pull request "test: Make existing functional tests compatible with --v2transport":
(https://github.com/bitcoin/bitcoin/pull/28805#issuecomment-1806662743)
utACK 35fb9930adb3501b29d3ad20d2e74c0114f2bcbe. Thanks for taking care of this.

💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1390128297)
Done

💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1390128370)
Done

💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1390128432)
I've added a comment.

💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1390128566)
Fixed

💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1390129010)
I've added this comment explaining why 10 blocks need to be mined:
```
# mine 10 blocks so that when the blk is invalidated, the transactions are not
# returned to the mempool
```

💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1390129173)
I've replaced `tx1` with `tx1_txid` here and for all other instances of this.

💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#issuecomment-1806687648)
Rebased and addressed most of the review comments (thanks @achow101 and @murchandamus). I will address the remaining comments soon.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1390149591)
done.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1390149597)
nice! included `MAGIC_BYTES` in messages.py

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1390149601)
true! done.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1390149609)
makes sense. done.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1390149630)
but there's also an else condition below where if the `received_garbage` doesn't match the garbage terminator, we keep appending 1 byte to `received_garbage` until there's a match with garbage terminator/we've exceed max garbage limit. so even though initially `received_garbage` length is 16 bytes, it can take values from 17 bytes, ..., 17 + 4095 bytes.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1390149640)
done.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1390149641)
done.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1390149646)
done.