🤔 mzumsande reviewed a pull request: "Change Luke Dashjr seed to dashjr-list-of-p2p-nodes-maybe-malware.us"
(https://github.com/bitcoin/bitcoin/pull/29145#pullrequestreview-1797445282)
@luke-jr : Slightly unrelated to this PR, but looking at the results from all seeders, it seems that yours returns only nodes running old versions `0.21.x` and `22.x`. I didn't get a single result with a newer subversion.
Given the current composition of nodes on the network and that none of the other DNS seeds show a similar pattern, this seems unlikely to just be an unlucky result of a random selection. Could this be a bug in your seeder?

👍 kristapsk approved a pull request: "Update Node window title with the chain type"
(https://github.com/bitcoin-core/gui/pull/758#pullrequestreview-1797448565)
ACK 9d37886a3b6ce24f4a4a05193eb0d071655a8457

🤔 murchandamus reviewed a pull request: "wallet: track mempool conflicts with wallet transactions"
(https://github.com/bitcoin/bitcoin/pull/27307#pullrequestreview-1797438722)
ACK 22778ac98f1bcb695de8ccf0b02fe9ebe39b01aa

💬 murchandamus commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1437201563)
In "wallet: use CWalletTx member functions to determine tx state" (80ba1d97f179121736d702b2d54bd3938cb57d96):

Did you mean "An output is considered spent…"?

💬 dimitaracev commented on pull request "wallet: add meaningful error message and fix test":
(https://github.com/bitcoin/bitcoin/pull/29143#issuecomment-1870566016)
> Also, an assertion crash fix, is not a "test" fix. I presume this can happen in production, no?

Initially I thought that this was the expected behavior, which is why this pull request just returns a different error message so that it can be caught in the test. But as @furszy said, the issue appears to be something completely different that needs fixing.

> Do you have steps to reproduce the test failure (usually a race can be reproduced by adding a sleep in the right place in the C++ cod
...

🤔 murchandamus reviewed a pull request: "wallet, rpc: add anti-fee-sniping to `send` and `sendall`"
(https://github.com/bitcoin/bitcoin/pull/28944#pullrequestreview-1797462130)
Concept ACK and almost crACK d76f88051524971def5137a07ce2940dce1c33a9 except that there seems to be a type mix-up:

```
wallet/rpc/spend.cpp:1299:20: error: no matching function for call to 'FinishTransaction'
return FinishTransaction(pwallet, options, CMutableTransaction(*txr.tx));
^~~~~~~~~~~~~~~~~
wallet/rpc/spend.cpp:79:17: note: candidate function not viable: expects an lvalue for 3rd argument
static UniValue FinishTransaction(const std::shared_ptr<CW
...

🤔 murchandamus reviewed a pull request: "wallet, rpc: document and update `sendall` behavior around unconfirmed inputs"
(https://github.com/bitcoin/bitcoin/pull/28979#pullrequestreview-1797464732)
Concept ACK, the ancestor aware funding could use a test

💬 murchandamus commented on pull request "wallet, rpc: document and update `sendall` behavior around unconfirmed inputs":
(https://github.com/bitcoin/bitcoin/pull/28979#discussion_r1437219363)
In "rpc, test: test sendall spends unconfirmed change and unconfirmed inputs when specified" 69408120a64316624b9ec379771dbac142dc6646, perhaps you could confirm here that the wallet is now empty:

```
assert_equal(self.wallet.getbalance(), 0)
```

💬 murchandamus commented on pull request "wallet, rpc: document and update `sendall` behavior around unconfirmed inputs":
(https://github.com/bitcoin/bitcoin/pull/28979#discussion_r1437221305)
This could be tested by first spending an input whose parent transaction has a higher feerate than what we aim for, then an input of the same type with the same amount whose parent transaction has a lower feerate and showing that the amount sent to the receiver is amended correspondingly.

💬 ishaanam commented on pull request "wallet, rpc: add anti-fee-sniping to `send` and `sendall`":
(https://github.com/bitcoin/bitcoin/pull/28944#issuecomment-1870602431)
Fixed silent merge conflict.

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

🤔 murchandamus reviewed a pull request: "wallet, rpc: add anti-fee-sniping to `send` and `sendall`"
(https://github.com/bitcoin/bitcoin/pull/28944#pullrequestreview-1797513405)
ACK c8de459de6d22e37164583f078747facaeae69c6

💬 murchandamus commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#issuecomment-1870633370)
ACK https://github.com/bitcoin/bitcoin/pull/27307/commits/d4259cdaff2128c25e877d6a64e6ce15ecb9beca, only documentation change

💬 mzumsande commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#discussion_r1437279158)
Ok, I clarified the comments added in this PR.
I think that a single connection / reconnection attempt doesn't waste a meaningful amount of resources of either side, so I'm not sure if there is an actual problem here. Also, often one might not want the other side to know that you evicted them because they are banned (after all you banned them for a reason) - that might just help them evade the ban.

💬 mzumsande commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1870670097)
[a30a1d9 ](https://github.com/bitcoin/bitcoin/commit/a30a1d9318d08ddd1007d7fe7c19e4175faa8b64)to [fb5bfed](https://github.com/bitcoin/bitcoin/commit/fb5bfed26a564014b83ccfc96ff00b630930fc61): small doc-only update.

💬 luke-jr commented on pull request "RPC/Wallet: Convert walletprocesspsbt to use options parameter":
(https://github.com/bitcoin/bitcoin/pull/24963#discussion_r1437296646)
Fixed

💬 luke-jr commented on pull request "RPC/Wallet: Convert walletprocesspsbt to use options parameter":
(https://github.com/bitcoin/bitcoin/pull/24963#discussion_r1437297031)
Done

💬 andrewtoth commented on pull request "During IBD, prune as much as possible until we get close to where we will eventually keep blocks":
(https://github.com/bitcoin/bitcoin/pull/20827#discussion_r1437296904)
nit: Any reason this declaration is up here and not directly above where it is used below?

💬 andrewtoth commented on pull request "During IBD, prune as much as possible until we get close to where we will eventually keep blocks":
(https://github.com/bitcoin/bitcoin/pull/20827#discussion_r1437297033)
nit: could remove the cast in the next line with
```suggestion
const uint64_t chain_tip_height = chain.m_chain.Height();
```

💬 ns-xvrn commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1870828421)
@petertodd

> As long as miners continue to mine these transactions they will end up in blocks, requiring node operators to process those transactions. Censoring those transactions simply shifts when that bandwidth usage happens. Indeed, it'll make block propagation happen somewhat slower for some users/miners due to interference with compact blocks. This is harmful to many users, as well as some smaller miners.

What kind of magnitude for the impact on block propagation here? Is it bigger
...