💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1121328934)
The comment is wrong

💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1122739758)
nit: it'd be great if we can disambiguate the term `mixed`. Now it applies to both a) mixed type groups b) mixed effects value group

💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1121336522)
Not sure why this is needed

💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1122748374)
nit: `All` groups is somewhat misleading here, because these are actually filtered groups, but for all output types. Unfortunately, I don't have better suggestion yet.

💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1122749611)
nit: Judging from the name it's not clear what's the difference between `Groups` and `OutputGroups`

💬 MarcoFalke commented on pull request "build: use _FORTIFY_SOURCE=3":
(https://github.com/bitcoin/bitcoin/pull/27027#issuecomment-1451572322)
This triggered https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=56529 ?

💬 MarcoFalke commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1451587856)
ci didn't run?

💬 glozow commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1122931688)
Mm thanks, I agree. Was able to reproduce and trace the issue. @Xekyo see previous suggestion to fix.

💬 glozow commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1122928753)
This fixes the crash from https://github.com/bitcoin/bitcoin/pull/27021/files#r1121611301

```suggestion
auto it = available_coins.begin();
mtx.vin.push_back(CTxIn(*it, CScript()));
available_coins.erase(available_coins.begin(), it);
```

💬 glozow commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1122932387)
Also remember to delete the "All outputs available to spend" comment

💬 TheCharlatan commented on pull request "Introduce `MockableDatabase` for wallet unit tests":
(https://github.com/bitcoin/bitcoin/pull/26715#discussion_r1122945291)
Couldn't this class definition be moved out of the header?

💬 TheCharlatan commented on pull request "Introduce `MockableDatabase` for wallet unit tests":
(https://github.com/bitcoin/bitcoin/pull/26715#discussion_r1122947093)
Nit: Couldn't this just be used directly at its single call site?

⚠️ GregTonoski opened an 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)
It is impossible to load and use a wallet neither in Bitcoin-qt GUI nor console/RPC commands. The wallet is only about 2 weeks old and a typical settings are used (data pruned to 2 GB).

**Expected behavior**

- A wallet is loaded (either manually or automatically on startup) and addresses are shown in Bitcoin-qt,
- successful execution of wallet RPC commands, e.g. getwalletinfo, dumpwallet, walletpassphrasechange etc.

(not different from the way it had worked and looked the previous t
...

💬 willcl-ark 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-1451750906)
Possible (future) solution: https://github.com/bitcoin/bitcoin/issues/21267

💬 glozow commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1122981731)
Sorry ignore my previous review, it was wrong. But this should work:

```suggestion
if (outpoint.has_value() && std::find(outpoints.begin(), outpoints.end(), *outpoint) == outpoints.end()) {
outpoints.push_back(*outpoint);
}
```

💬 glozow commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1122985863)
Just noticed this was added for `CalculateTotalBumpFees()` - it is redundant with `m_requested_outpoints_by_txid`, so I think you should remove it and have `CalculateTotalBumpFees()` iterate through `m_requested_outpoints_by_txid`'s keys instead.

💬 glozow commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1122983669)
This input produces an `outpoints` vector with 2 of the same outpoint, which is what causes it to crash on the line `assert(bump_fees.size() == outpoints.size()`.

💬 glozow commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1123037832)
This is more precise than making sure the lengths are the same (and would also fix the crash):
```suggestion
for (const auto& outpoint : outpoints) {
auto it = bump_fees.find(outpoint);
assert(it != bump_fees.end());
assert(it->second >= 0);
}
```

📝 MarcoFalke opened a pull request: "Use steady clock in SeedStrengthen and FindBestImplementation"
(https://github.com/bitcoin/bitcoin/pull/27189)
There may be a theoretical deadlock for the duration of the offset when the system clock is adjusted into a past time while executing `SeedStrengthen`.

Fix this by using steady clock.

Do the same in `FindBestImplementation`, which shouldn't be affected, because it discards outlier measurements. However, doing the same there for consistency seems fine.

👋 brunoerg's pull request is ready for review: "test: fix intermittent issue in `feature_bip68_sequence`"
(https://github.com/bitcoin/bitcoin/pull/27177)