💬 sipa commented on issue "Use of a wallet shouldn't be blocked in prune mode ("wallet loading failed... beyond pruned data")":
(https://github.com/bitcoin/bitcoin/issues/27188#issuecomment-1452563398)
@willcl-ark That still won't work when the pruning point is later than the wallet birth date.

👍 ryanofsky approved a pull request: "refactor: Use move semantics instead of custom swap functions"
(https://github.com/bitcoin/bitcoin/pull/26749)
Conditional code review ACK 142718890e718397e0026c315c8940102b9657ce, assuming this shouldn't use emplace_back https://github.com/bitcoin/bitcoin/pull/26749#discussion_r1123317784 or avoid temporary vector allocations https://github.com/bitcoin/bitcoin/pull/26749#discussion_r1123429117.

I mostly reviewed the overall PR diff and didn't review individual commits in detail since it was easier to think about class definitions with only move operations or only swap operations, not combinations of
...

💬 furszy commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#issuecomment-1452678639)
Thanks @S3RK for the review.

Updated per feedback ([small diff](https://github.com/bitcoin/bitcoin/compare/0bb90cabc8e2c6bbbc3c52ec1170e725435b625d..ae0dd2becd4fa01666793c3f48e8725d5e585c30)), tackled the following points that made sense for me to include here:

* Renamed `OutputGroups` to `OutputGroupTypeMap` to reflect what the struct really does and differentiate it from the inner struct.
* Corrected the `OutputGroups`, now `OutputGroupTypeMap`, members comments.
* And tackled @Xekyo’s
...

💬 furszy commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1123825816)
Ended up renaming `OutputGroups` to `OutputGroupTypeMap` to reflect what the struct really does and differentiate it from the inner struct.

💬 furszy commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1123826202)
Fixed the "filter" comment mention.

👍 furszy approved a pull request: "wallet: Migrate wallets that are not in a wallet dir"
(https://github.com/bitcoin/bitcoin/pull/26740)
rebase ACK 52634dba

💬 mzumsande commented on pull request "blockstorage: add an assert to avoid running oom with `-fastprune`":
(https://github.com/bitcoin/bitcoin/pull/27191#discussion_r1124047278)
Good, should make rebasing easy! Here, it's not a cleanup but needed to use `max_blockfile_size` in the added assert.

💬 mzumsande commented on pull request "blockstorage: add an assert to avoid running oom with `-fastprune`":
(https://github.com/bitcoin/bitcoin/pull/27191#issuecomment-1453010117)
> It might also make sense to not allow `-fastprune` on anything besides regtest as well. I'm not sure if that is covered by `ArgsManager::DEBUG_ONLY`

Not sure if suppressing options for specific networks is something we usually do (are there any other examples for this?). But I added some more explanation to the `-fastprune` documentation in `init` with the latest push.

⚠️ anmode opened an issue: "[Bug] Bitcoin-core"
(https://github.com/bitcoin/bitcoin/issues/27193)
<!-- This issue tracker is only for technical issues related to Bitcoin Core.

General bitcoin questions and/or support requests are best directed to the Bitcoin StackExchange at https://bitcoin.stackexchange.com.

For reporting security issues, please read instructions at https://bitcoincore.org/en/contact/.

If the node is "stuck" during sync or giving "block checksum mismatch" errors, please ensure your hardware is stable by running memtest and observe CPU temperature with a load-test t
...

👍 TheCharlatan approved a pull request: "util: add missing include and fix function signature"
(https://github.com/bitcoin/bitcoin/pull/27192)
ACK 101601a1ff7ff8a0eb58171df1227dc7b006cbb9

👍 MarcoFalke approved a pull request: "util: add missing include and fix function signature"
(https://github.com/bitcoin/bitcoin/pull/27192)
lgtm

💬 MarcoFalke commented on pull request "util: add missing include and fix function signature":
(https://github.com/bitcoin/bitcoin/pull/27192#discussion_r1124195745)
nit: Including the own module goes on the first line/section, usually, to catch missing includes in the header file.

Also, could add it to iwyu in ci/test/06_...sh, so that reviewers can look at the ci output if they want?

💬 MarcoFalke commented on issue "[Bug] Bitcoin-core. MacOs":
(https://github.com/bitcoin/bitcoin/issues/27193#issuecomment-1453214266)
Is this still an issue with a recent version of Bitcoin Core? If yes, what are the steps to reproduce?

💬 MarcoFalke commented on pull request "refactor: Use move semantics instead of custom swap functions":
(https://github.com/bitcoin/bitcoin/pull/26749#discussion_r1124203980)
> Should this use emplace_back instead of push_back?

I didn't mention this because I assumed the two functions would do the same if they received a moved object?

💬 MarcoFalke commented on issue "Use of a wallet shouldn't be blocked in prune mode ("wallet loading failed... beyond pruned data")":
(https://github.com/bitcoin/bitcoin/issues/27188#issuecomment-1453221836)
Did you create the wallet with the wallet tool? If yes, see https://github.com/bitcoin/bitcoin/pull/26679

💬 MarcoFalke commented on pull request "wallet: ensure the wallet is unlocked when needed for rescanning":
(https://github.com/bitcoin/bitcoin/pull/26347#discussion_r1124232090)
> Do you mean calling walletlock to ensure that the wallet is still unlocked?

No, I mean calling `walletlock` to lock the wallet. So the execution would look like:

```
-> walletpassphrase 99999 (start)
<- walletpassphrase (end)
-> rescanblockchain (start)
-> walletlock (start)
<- rescanblockchain (finish, release mutex)
<- walletlock (take mutex, end)
```

Without the fix in this pull, `walletlock` might lock immediately and then likely cause `rescanblockchain` to fail?

💬 anmode commented on issue "[Bug] Bitcoin-core. MacOs":
(https://github.com/bitcoin/bitcoin/issues/27193#issuecomment-1453266728)
> Is this still an issue with a recent version of Bitcoin Core? If yes, what are the steps to reproduce?

Yess yess...
1. Install bitcoin-core from official website.
2. Move bitcoin to application folder
3. When u open set the data_dir and then it will start downloading the blockchain data.

Now there will ne .bitcoin folder as mentioned. Nor able to locate .bitcoin folder

💬 MarcoFalke commented on issue "[Bug] Bitcoin-core. MacOs":
(https://github.com/bitcoin/bitcoin/issues/27193#issuecomment-1453287463)
There won't be a `.bitcoin` folder, unless you set it as datadir. Also, you can open the "Information" windows to see the datadir location.

💬 anmode commented on issue "[Bug] Bitcoin-core. MacOs":
(https://github.com/bitcoin/bitcoin/issues/27193#issuecomment-1453305239)
> There won't be a `.bitcoin` folder, unless you set it as datadir. Also, you can open the "Information" windows to see the datadir location.

Just wanted to know...
As i download bitcoin-core app.
And make folder bitcoin[data dir] on desktop.
But node is not running when i ran bitcoind command. I get no command.
So i look bitcoin folder to add that path in env.

How to proceed?

👍 MarcoFalke approved a pull request: "refactor: RPC: pass named argument value as string_view"
(https://github.com/bitcoin/bitcoin/pull/26612)
nice

review ACK 545ff924ab6303ffabd91fdfc4f0a4962daf133c 📻

<details><summary>Show signature</summary>

Signature:

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

review ACK 545ff924ab6303ffabd91fdfc4f0a4962daf133c 📻
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUhxDQwAnkUGV9vWw6I02mle/ZFfeIWLL4UhIJjNmcc0UH+0DwtK7IM9V74ST0QZ
8iv5AOTv43xJ9Ltye54nIbJnjIhW1WmZ/nhwTHDqJtQHP2GqF4ao2uQo2nGMEt/G
O3BixrcDyYHQDOR92bou9pzc7BOJpQ
...