📝 pinheadmz opened a pull request: "Add wallet method to detect if a key is "fresh""
(https://github.com/bitcoin/bitcoin/pull/27216)
This is a first step towards closing https://github.com/bitcoin/bitcoin/issues/3314 by implementing functions in Wallet and ScriptPubKeyMan that determine if a key is "still in the keypool" aka "fresh". This wording and the function names may need some brainstorming because the concept of "used" keys already has a few meanings, especially the `avoid_reuse` flag.

A key is "still in the keypool" or "fresh" if:
- it has never been seen on chain
- its index does not precede any keys seen on cha
...

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1127069573)
Great, thanks

💬 brunoerg commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1127073061)
nit (for 9bf078f66c8f286e1ab5e34b8eeed7d80290a897):
```cpp
diff --git a/src/addrman.cpp b/src/addrman.cpp
index ec5b0213b..f608d60f0 100644
--- a/src/addrman.cpp
+++ b/src/addrman.cpp
@@ -722,15 +722,13 @@ std::pair<CAddress, NodeSeconds> AddrManImpl::Select_(bool newOnly) const
if (newOnly && nNew == 0) return {};

// Decide if we are going to search the new or tried table
- bool search_tried;
+ bool search_tried{false};

// Use a 50% chance for choosing betw
...

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1127076941)
Good catch

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1127077341)
Okay, added

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1127079196)
You’re right, fixed

💬 brunoerg commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1127079909)
perhaps rename `newOnly` to `new_only`?

💬 brunoerg commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1127083746)
Suggestion:
```diff
diff --git a/src/addrman.cpp b/src/addrman.cpp
index 8c36af13f..66d54d2b4 100644
--- a/src/addrman.cpp
+++ b/src/addrman.cpp
@@ -746,8 +746,7 @@ std::pair<CAddress, NodeSeconds> AddrManImpl::Select_(bool newOnly, std::optiona
search_tried = insecure_rand.randbool();
}

- int bucket_count;
- search_tried ? bucket_count = ADDRMAN_TRIED_BUCKET_COUNT : bucket_count = ADDRMAN_NEW_BUCKET_COUNT;
+ const int bucket_count{search_tried ? ADDRMAN_TRIE
...

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1127091583)
I decided to leave it as is for now.

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1457092354)
Changes since https://github.com/bitcoin/bitcoin/commit/32123cfc11ad22cdd7fe5ef26ac0550e1ab2fe7b:
- Addressed remaining review from @LarryRuane
- Fixed crash bug surfaced by @dergoegge
- Addressed review by @furszy

📝 achow101 opened a pull request: "wallet: Replace use of purpose strings with an enum"
(https://github.com/bitcoin/bitcoin/pull/27217)
Instead of storing and passing around fixed strings for the purpose of an address, use an enum.

💬 achow101 commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#issuecomment-1457199020)
ACK 6a302d40dfcc33b15bf27a1d3fcc04bfd3cbda44

🚀 achow101 merged a pull request: "wallet: group outputs only once, decouple it from Coin Selection"
(https://github.com/bitcoin/bitcoin/pull/25806)

💬 achow101 commented on pull request "http: Track active requests and wait for last to finish - 2nd attempt":
(https://github.com/bitcoin/bitcoin/pull/26742#issuecomment-1457271622)
ACK 60978c8080ec13ff4571c8a89e742517b2aca692

🚀 achow101 merged a pull request: "http: Track active requests and wait for last to finish - 2nd attempt"
(https://github.com/bitcoin/bitcoin/pull/26742)

💬 achow101 commented on pull request "assumeutxo: background validation completion":
(https://github.com/bitcoin/bitcoin/pull/25740#issuecomment-1457287964)
Missed this on last review:

```
../../../src/validation.cpp: In member function ‘SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation(std::function<void(bilingual_str)>)’:
../../../src/validation.cpp:5450:14: error: catching polymorphic type ‘struct StopHashingException’ by value [-Werror=catch-value=]
5450 | } catch (StopHashingException) {
| ^~~~~~~~~~~~~~~~~~~~
```

👍 rserranon approved a pull request: "wallet: finish addressbook encapsulation"
(https://github.com/bitcoin/bitcoin/pull/26836)
tAck
* unit tests
* functional tests
* qt & bitcoin-cli manual tests

🤔 andrewtoth requested changes to a pull request: "cli: add validation to cli side commands besides when it's used with -rpcwallet"
(https://github.com/bitcoin/bitcoin/pull/26990)

💬 andrewtoth commented on pull request "cli: add validation to cli side commands besides when it's used with -rpcwallet":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1127232884)
I'm not sure if this new message makes it more clear. Specifically, "For the CLI" seems like it might confuse users because there's nothing opposed to that in the message.

💬 andrewtoth commented on pull request "cli: add validation to cli side commands besides when it's used with -rpcwallet":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1127233443)
Where does `bitcoin-cli -h` show how to solve this problem specifically? Telling users to run that here might not be very helpful. The second part could be useful information though.