💬 fjahr commented on pull request "test, assumeutxo: Add test to ensure failure when mempool not empty":
(https://github.com/bitcoin/bitcoin/pull/29394#issuecomment-1938591797)
utACK da13cc2c8010cbccf37324cca4403cb346ecdd30

I still think my suggestions [here](https://github.com/bitcoin/bitcoin/pull/29394#discussion_r1483608953) would be an improvement but are not strictly needed.

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-1938598929)
It's indeed a strange error. I nuked the existing silent payment index before running, so not sure what's going on here.

💬 epiccurious commented on pull request "mempool: Log added for dumping mempool transactions to disk":
(https://github.com/bitcoin/bitcoin/pull/29402#discussion_r1486152101)
+1

👍 hebasto approved a pull request: "Update Node window title with the chain type"
(https://github.com/bitcoin-core/gui/pull/758#pullrequestreview-1875194209)
ACK 9d37886a3b6ce24f4a4a05193eb0d071655a8457, tested on Ubuntu 23.10.

💬 hebasto commented on pull request "Update Node window title with the chain type":
(https://github.com/bitcoin-core/gui/pull/758#discussion_r1486154180)
nit: Missed EOL.

✅ hebasto closed an issue: "Indicate network in the Node window title - similar to the main window"
(https://github.com/bitcoin-core/gui/issues/544)

🚀 hebasto merged a pull request: "Update Node window title with the chain type"
(https://github.com/bitcoin-core/gui/pull/758)

💬 josibake commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1486149819)
in "[node]: update BroadcastTransaction to check fee rate limit" (https://github.com/bitcoin/bitcoin/pull/29278/commits/eedbeccbdb274841fc2cf2c53931e58fbea50dfc):


If a `max_tx_fee` is set, and a `max_tx_feerate` is not set, `BroadcastTransaction` might hit the max_tx_fee limit, so its not quite correct to say that "if CFeeRate(0), accept any fee." What about something like the following:

```
* @param[in] max_tx_fee reject txs with fees higher than this (if 0, the fee is not checked)
*
...

🤔 josibake reviewed a pull request: "Wallet: Add `maxfeerate` wallet startup option"
(https://github.com/bitcoin/bitcoin/pull/29278#pullrequestreview-1875177902)
Looking good! I think moving https://github.com/bitcoin/bitcoin/pull/29278/commits/d83ecdde0dfdbf46806be1310ad63716e33edc96 to be the 3rd commit (right after introducing the new option) makes more sense as there are still at least a few spots where you return a `MAX_TX_FEE` error incorrectly, and then fix it up in a later commit.

Also, small typo in https://github.com/bitcoin/bitcoin/pull/29278/commits/eedbeccbdb274841fc2cf2c53931e58fbea50dfc:

s/differenciating/differentiating/

💬 josibake commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1486143881)
in "[node]: update BroadcastTransaction to check fee rate limit" (https://github.com/bitcoin/bitcoin/pull/29278/commits/eedbeccbdb274841fc2cf2c53931e58fbea50dfc):

This should be `MAX_FEERATE_EXCEEDED`

💬 josibake commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1486152509)
in "[node]: update BroadcastTransaction to check fee rate limit" (https://github.com/bitcoin/bitcoin/commit/eedbeccbdb274841fc2cf2c53931e58fbea50dfc):

nit:
```suggestion
const TransactionError err = BroadcastTransaction(node, tx, err_string, /*max_tx_fee=*/0, max_raw_tx_fee_rate, /*relay=*/true, /*wait_callback=*/true);
```

💬 hebasto commented on pull request "Bugfix: Address broken things around Peers detail view":
(https://github.com/bitcoin-core/gui/pull/677#issuecomment-1938671029)
Why the first commit "net: Ensure CNode.cleanSubVer is always assigned before nVersion" is needed?

💬 TheCharlatan commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#discussion_r1486097145)
In commit b8900adcd232bcb2b7475be19b5457c91052ab5a:
Can you either add a line about its removal in the commit message, or do it in a separate commit?

💬 TheCharlatan commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#discussion_r1485612372)
There is one `wallet` too many here.

💬 TheCharlatan commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#discussion_r1486106049)
In commit b8900adcd232bcb2b7475be19b5457c91052ab5a:
Need to slightly adjust the comment here, how about:
`* @return true if we successfully combined the transactions, false if they were not compatible`

💬 hebasto commented on pull request "getrawtransaction implementation":
(https://github.com/bitcoin-core/gui/pull/777#issuecomment-1938694223)
Concept ACK. However, I still agree with @promag's https://github.com/bitcoin/bitcoin/issues/19161#issuecomment-667726216:
> GUI users can use the RPC console.

💬 willcl-ark commented on issue "Assertion chainman().ActiveChain()[height] fails on startup on memory-constrained system":
(https://github.com/bitcoin/bitcoin/issues/29422#issuecomment-1938696443)
I have not been able to reproduce this error using v26.0 binaries in a docker container limited to 1GB ram. At the same reported moment of your first crash I was only using ~550MB of ram:



<details>
<summary>Log excerpt</summary>

```log
2024-02-12T12:14:50Z Bitcoin Core version v26.0.0 (release build)
2024-02-12T12:14:50Z InitParameterInteraction: parameter interaction: -listen=0 -> sett
...

💬 russeree commented on pull request "redeclare nChainTx to use uint64_t":
(https://github.com/bitcoin/bitcoin/pull/29331#issuecomment-1938698865)
Could I please have just a bit more time to work on this?

Thanks, Reese

💬 sr-gi commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1486199775)
I may be used to the pattern due to rustlang, but I have no strong opinions about it

🤔 hebasto reviewed a pull request: "Dialog for allowing the user to choose the change output when bumping a tx"
(https://github.com/bitcoin-core/gui/pull/700#pullrequestreview-1875289512)
The first commit e1ea0ba0f2aa9b36df6b74b664087b236f174990 fails to compile:
```
CXX qt/libbitcoinqt_a-walletmodel.o
qt/walletmodel.cpp: In member function ‘bool WalletModel::bumpFee(uint256, uint256&)’:
qt/walletmodel.cpp:486:41: error: no matching function for call to ‘interfaces::Wallet::createBumpTransaction(uint256&, wallet::CCoinControl&, std::vector<bilingual_str>&, CAmount&, CAmount&, CMutableTransaction&)’
486 | if (!m_wallet->createBumpTransaction(hash, coin_control, e
...