💬 fanquake commented on pull request "bench: update logging benchmarks":
(https://github.com/bitcoin/bitcoin/pull/26957#discussion_r1118885979)
In 4db9dc8ac3baa9751702ca575965661e63384692:

Looks like you're introducing these benchmarks, with `BCLog::NET`, only to change them in a later commit (dfb40748911b9553a6e10c8376b032dac36bf938) to `BCLog::VALIDATION`? It's not obvious to me why the category needs changing, but this can probably be re-ordered so we aren't changing the same (newly introduced) lines of code, multiple times in the same PR.

💬 pinheadmz commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1446525945)
Rebased on master and prepended an extra commit with a unit test for BlockManager. The test passes on master and the branch without modification, hopefully illustrating that at least some behaviors are not affected.

It's hard to test precisely for the branch's change in a unit test because file-flushing is not something we can really predict or test for (sometimes it happens as soon as we `fwrite()`).

The actual error in issue #2039 comes from an attempt to OPEN a file with `rw` when the
...

💬 mzumsande commented on pull request "init: Return ChainstateLoadStatus::INTERRUPTED when verification was interrupted.":
(https://github.com/bitcoin/bitcoin/pull/27157#issuecomment-1446567630)
> It might be good to explain and motivate the behaviour change. Calling it "25574 followups" is less useful than, for example, mentioning if the exit code is changed, if a warning will be printed, or if this turns an operation into an error?

I changed the title and expanded the explanation.

💬 MarcoFalke commented on pull request "init: Return ChainstateLoadStatus::INTERRUPTED when verification was interrupted.":
(https://github.com/bitcoin/bitcoin/pull/27157#issuecomment-1446574280)
thanks lgtm ACK c5825e14f8999a8c5f5121027af9e07ac51ab42e

💬 pinheadmz commented on pull request "rpc, test: remove newline escape sequence from wallet warning fields":
(https://github.com/bitcoin/bitcoin/pull/27138#issuecomment-1446629012)
Code changes and tests look OK to me but I don't understand the motivation, are newlines a problem in RPC responses?

I found a few other similar mechanisms in other modules, dunno if this change should apply to them as well:

https://github.com/bitcoin/bitcoin/blob/82793f1984911774b111117f2e81d5f3b0bbec68/src/warnings.cpp#L55-L57

https://github.com/bitcoin/bitcoin/blob/82793f1984911774b111117f2e81d5f3b0bbec68/src/validation.cpp#L2565-L2569

💬 achow101 commented on pull request "wallet: skip R-value signature grinding for external signers":
(https://github.com/bitcoin/bitcoin/pull/26032#issuecomment-1446655693)
ACK 807de2cebdad960c2b52185528ca8960ec694f49

👍 hebasto approved a pull request: "[24.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/26878)
ACK 784a754aa47ce10c6fd99c09cdfc76ee9bc91652, I've made backporting locally and got a diff between my branch and this PR as follows:
```diff
--- a/test/functional/wallet_importdescriptors.py
+++ b/test/functional/wallet_importdescriptors.py
@@ -484,8 +484,8 @@ class ImportDescriptorsTest(BitcoinTestFramework):
assert_equal(addr, 'bcrt1qp8s25ckjl7gr6x2q3dx3tn2pytwp05upkjztk6ey857tt50r5aeqn6mvr9') # Derived at m/84'/0'/0'/1
change_addr = wmulti_pub.getrawchangeaddress('bech
...

💬 hebasto commented on pull request "[24.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/26878#issuecomment-1446678056)
> ACK [784a754](https://github.com/bitcoin/bitcoin/commit/784a754aa47ce10c6fd99c09cdfc76ee9bc91652)
>
> trivial backports as far as I can see

✅ hebasto closed a pull request: "[24.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/26878)

📝 hebasto reopened a pull request: "[24.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/26878)
Backports:
* https://github.com/bitcoin/bitcoin/pull/26595
* https://github.com/bitcoin/bitcoin/pull/26675
* https://github.com/bitcoin/bitcoin/pull/26679
* https://github.com/bitcoin/bitcoin/pull/26761
* https://github.com/bitcoin/bitcoin/pull/26837
* https://github.com/bitcoin/bitcoin/pull/26909
* https://github.com/bitcoin/bitcoin/pull/26924
* https://github.com/bitcoin/bitcoin/pull/26944
* https://github.com/bitcoin-core/gui/pull/704
* https://github.com/bitcoin/bitcoin/pull/27053
...

💬 LarryRuane commented on pull request "bench: update logging benchmarks":
(https://github.com/bitcoin/bitcoin/pull/26957#discussion_r1119061620)
Resetting categories would also touch real code, wouldn't it?

✅ achow101 closed an issue: "Don't assume signature grinding for external signers"
(https://github.com/bitcoin/bitcoin/issues/26030)

🚀 achow101 merged a pull request: "wallet: skip R-value signature grinding for external signers"
(https://github.com/bitcoin/bitcoin/pull/26032)

💬 achow101 commented on pull request "[24.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/26878#issuecomment-1446809743)
ACK 784a754aa47ce10c6fd99c09cdfc76ee9bc91652

🚀 glozow merged a pull request: "[24.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/26878)

💬 pablomartin4btc commented on pull request "cli: add validation to -generate command when it's used with -rpcwallet":
(https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-1446919139)
@jonatack thanks for reviewing; as you and @kouloumos recommended and as I commented [here](https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-1411046166), I've made the validation more generic for any cli-command moving it up in the code.
As part of this validation, I've added 3 more checks that were not verified before:
- duplication of cli-command (at the moment you can specify -rpcwallet many times, only the last one will be taken into account,
- only 1 cli-command can run at a ti
...

💬 brunoerg commented on issue "failure in feature_bip68_sequence.py":
(https://github.com/bitcoin/bitcoin/issues/27129#issuecomment-1446919358)
@MarcoFalke I think so, because `send_self_transfer` will call `create_self_transfer` which will call `get_utxo` (since we're not specifying it). `get_utxo` will return the largest utxo and it's usually the coinbase one. However, coinbase transactions needs 100 confs and we're not checking it what may be causing a "bad-txns-premature-spend-of-coinbase" error.

perhaps a fix:
```py
diff --git a/test/functional/feature_bip68_sequence.py b/test/functional/feature_bip68_sequence.py
index 894aff
...

💬 furszy commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1119204773)
> I'm not sure this is how APS works. When we remove negative UTXOs we assess the whole group and not each UTXO individually, so you don't need to regroup anything after filtering negative groups.

No wait, that would leave negative UTXOs in positive-only groups if the sum of the positive UTXOs surpasses the sum of the negative ones.

Currently, a "positive-only group" is essentially a container who stores only positive UTXOs. Not a mix of UTXOs which values sum is positive. That is differen
...

💬 LarryRuane commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1119107935)
```suggestion
std::optional<std::vector<CTxMemPool::txiter>> CTxMemPool::GetIterVec(const std::vector<uint256>& txids) const
```
Would this be better than giving a special meaning to an empty vector? I'm unsure, but may be worth considering.

💬 LarryRuane commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1119109899)
```suggestion
std::optional<std::vector<CTxMemPool::txiter>> CTxMemPool::CalculateCluster(const std::vector<uint256>& txids) const
```