💬 sipsorcery commented on pull request "Fixes compile errors in MSVC build #27332":
(https://github.com/bitcoin/bitcoin/pull/27335#issuecomment-1494973574)
tACK 28436965b6422bd92d23033a5ddbd60a6c183cd7.

The version pinning is nice. It was badly missed feature when we first started relying on vcpkg. We pinned the version of vcpkg instead but individual package pinning should be more flexible.

👋 brunoerg's pull request is ready for review: "logging, net: add ASN from peers on logs"
(https://github.com/bitcoin/bitcoin/pull/27412)

💬 hebasto commented on pull request "build: Add CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/25797#issuecomment-1494989972)
Rebased due to the conflict with bitcoin/bitcoin#27254.

All CI tasks are :green_circle:

Guix builds:
```
aa34dfbb1956fa04b56efa3cbbc798d75f62141787ea01e28796c3c7fc1743d4 guix-build-7f09e7118a59/output/aarch64-linux-gnu/SHA256SUMS.part
a7ceb4b814eb41558c5b8dcbe57e443505dac96181b9727fac235197ed729ef8 guix-build-7f09e7118a59/output/aarch64-linux-gnu/bitcoin-7f09e7118a59-aarch64-linux-gnu-debug.tar.gz
b336955cf07c04c69cc03040fab63a5c862c7eb984ae56ddbec9b45345f1af99 guix-build-7f09e7118
...

💬 jonatack commented on pull request "logging, net: add ASN from peers on logs":
(https://github.com/bitcoin/bitcoin/pull/27412#discussion_r1156464266)
Would there be any downside to not adding a config option and just logging the asmap if it is non-zero?

👍 aureleoules approved a pull request: "wallet: coin selection, don't return results that exceed the max allowed weight"
(https://github.com/bitcoin/bitcoin/pull/26720)
ACK 1284223691127e76135a46d251c52416104f0ff1 - I verified the tests are correct and the code looks good to me.

Sidenote, It seems this chunk of code is unused.
<details>
<summary>Unused code</summary>

```diff
diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index 92d33bd38..26405d63a 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -448,13 +448,6 @@ void OutputGroupTypeMap::Push(const OutputGroup& group, OutputType type, bool in
...

💬 aureleoules commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1156435700)
1284223691127e76135a46d251c52416104f0ff1 (same below)

> If an if only has a single-statement then-clause, it can appear on the same line as the if, without braces. In every other case, braces are required, and the then and else clauses must appear correctly indented on a new line.
https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#coding-style-c

```suggestion
} else {
append_error(bnb_result);
}
```

💬 aureleoules commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1156451001)
nit: Maybe rename the variable to `max_inputs_weight` to make it clearer?

💬 aureleoules commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1156453195)
fd8cf58d736ed9614f46b4f5b3c92f71ff9c46d4
nit
```suggestion
const int operator() (const OutputGroup& group1, const OutputGroup& group2)
```

💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1156500169)
Hmm, what about something like this https://github.com/furszy/bitcoin-core/commit/0ddd8f4e2672f4826b87b1cfda617a3c15caabea

💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1156501392)
it's the maximum allowed transaction weight, not only for the inputs.

💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#issuecomment-1495056232)
Thanks aureleoules for the feedback.

> Sidenote, It seems this chunk of code is unused.

That chunk of code is not in master, #27227 already cleaned it.

💬 aureleoules commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#issuecomment-1495134375)
> That chunk of code is not in master, https://github.com/bitcoin/bitcoin/pull/27227 already cleaned it.

Ah great, I'm not up-to-date with all the recently merged PRs, it's been a while since I reviewed any!

💬 theStack commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1156541825)
in commit 1284223691127e76135a46d251c52416104f0ff1: this doxygen comment about the return-value has to be adapted w.r.t. `std::nullopt`

💬 theStack commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1156571087)
> it's the maximum allowed transaction weight, not only for the inputs.

I don't think so? The coin selection algos don't have any knowledge about the additional data of the to-be-created tx (static header size + outputs), so they can only work with a weight limit on the inputs.

📝 meglio opened a pull request: "doc: Add example of how to mix private and public keys in descriptors"
(https://github.com/bitcoin/bitcoin/pull/27414)
It took me quite a while to understand how to use both xpriv and public keys in a single descriptor. The trick was to use xprv key + derivation path. As a newbie to Bitcoin Core, I had to consult stackexchange to understand that.

Adding an example would make it easier to "gotcha" for the reader.

⚠️ kylezs opened an issue: "-fallbackfee should apply to estimatesmartfee"
(https://github.com/bitcoin/bitcoin/issues/27415)
### Please describe the feature you'd like to see added.

`-fallback` fee is used by `sendtoaddress` to provide a fallback fee when sending transactions. However, this does not apply to estimatesmartfee, which still returns the error.

### Is your feature related to a problem, if so please describe it.

The reason for this is because it's difficult to test client applications on regtest, that would use `estimatesmartfee` on mainnet, but because it requires a lot of transactions before `estimates
...

⚠️ Satoshingmx opened an issue: "Bitcoin.org/https://www.bitcoin.org/www.bitcoin.org"
(https://github.com/bitcoin/bitcoin/issues/27416)
Bitcoin or Bitcoin/Bitcoin are not with any way or form part of Bitcoin.org aka Satoshi nakamoto has returned to
Fix many issues that where cause by updates unauthorized and should never been allowed by GitHub. And this is clear by doing something that would not pass but by removing and updating changes not allowed where allowed so Satoshi Nakamoto will show POW and finally take command over all bitcoin.org and exe.

✅ fanquake closed an issue: "Bitcoin.org/https://www.bitcoin.org/www.bitcoin.org"
(https://github.com/bitcoin/bitcoin/issues/27416)

💬 S3RK commented on pull request "doc: Add example of how to mix private and public keys in descriptors":
(https://github.com/bitcoin/bitcoin/pull/27414#discussion_r1156788080)
typo: to sign a transaction with the first ~~signature~~ key

💬 S3RK commented on pull request "doc: Add example of how to mix private and public keys in descriptors":
(https://github.com/bitcoin/bitcoin/pull/27414#issuecomment-1495422350)
ACK 42107710a35dd4b489757dd9317251f4b27cd3be