🤔 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
...

📝 luke-jr opened a pull request: "guix: Use DOS newlines for SHA256SUMS files"
(https://github.com/bitcoin/bitcoin/pull/29147)
OpenPGP specifies that plain text should use CR LF for newlines. By doing so, it becomes possible to include the hashes directly in the .asc file.

(Currently untested, looking for Concept ACKs)

💬 Sjors commented on pull request "guix: Use DOS newlines for SHA256SUMS files":
(https://github.com/bitcoin/bitcoin/pull/29147#discussion_r1437517112)
Maybe refer to https://www.rfc-editor.org/rfc/rfc4880.html section 5.2.4 here.

💬 maflcko commented on pull request "guix: Use DOS newlines for SHA256SUMS files":
(https://github.com/bitcoin/bitcoin/pull/29147#issuecomment-1871026463)
> By doing so, it becomes possible to include the hashes directly in the .asc file.

Not sure. Wasn't the goal the exact opposite, so that it is easier to `cat` the hashes once and as many signatures as one wants?

🤔 BrandonOdiwuor reviewed a pull request: "fuzz: set `m_fallback_fee` and `m_fee_mode` in `wallet_fees` target"
(https://github.com/bitcoin/bitcoin/pull/29076#pullrequestreview-1797930387)
Concept ACK

🤔 BrandonOdiwuor reviewed a pull request: "Update about logo icon (colour) to denote the chain type of the QT instance in About/ Help Message Window/ Dialog"
(https://github.com/bitcoin-core/gui/pull/762#pullrequestreview-1797941528)
Concept ACK