💬 furszy commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1123685274)
Ok understood it, the `mixed` term is also used for the `allow_mixed_output_types` flag. Which is not from this PR.
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1123685274)
Ok understood it, the `mixed` term is also used for the `allow_mixed_output_types` flag. Which is not from this PR.
📝 theuni opened a pull request: "util: add missing include and fix function signature"
(https://github.com/bitcoin/bitcoin/pull/27192)
ping @hebasto
Discovered while testing pre-compiled header support with CMake: https://github.com/theuni/bitcoin/commits/cmake-pch-poc. Compilation of that branch fails without this fix and succeeds with it.
Similar to the fix in #27144.
The problem of having a default argument in the definition was masked by the missing include. Using PCH forces that include, so we end up with the compiler error we should've been getting all along.
(https://github.com/bitcoin/bitcoin/pull/27192)
ping @hebasto
Discovered while testing pre-compiled header support with CMake: https://github.com/theuni/bitcoin/commits/cmake-pch-poc. Compilation of that branch fails without this fix and succeeds with it.
Similar to the fix in #27144.
The problem of having a default argument in the definition was masked by the missing include. Using PCH forces that include, so we end up with the compiler error we should've been getting all along.
🚀 fanquake merged a pull request: "doc: Update Transifex links and slug format in Release Process"
(https://github.com/bitcoin/bitcoin/pull/27183)
(https://github.com/bitcoin/bitcoin/pull/27183)
💬 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.
(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
...
(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
...
(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.
(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.
(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
(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.
(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.
(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
...
(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
(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
(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?
(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?
(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?
(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
(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?
(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
(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