💬 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)

💬 brunoerg commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#issuecomment-1451836694)
Force-pushed changing the approach.

The logic:
- if you don't specify a `txid`, it will return the largest utxo desconsidering immature coinbase. In case the only ones available are immature ones, it will return the largest of them.
_Why?_ - We can't filter the immature coinbase ones by default because in some tests they're the only available ones.

- Added a flag `avoid_immature_coinbase` to not break some mempool tests, in that case, they need to fetch it considering the immature ones
...

💬 brunoerg commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1123068867)
removing `Any` it throw a lint error, I believe it's because it can be any value from `filter` or `reversed`.

💬 brunoerg commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1123069732)
Not sure. In case of specifying a `txid`, it will not have to return the largest one.

💬 TheCharlatan commented on pull request "rpc: Optimize serialization disk space of dumptxoutset":
(https://github.com/bitcoin/bitcoin/pull/26045#issuecomment-1451900443)
> The main downside of this implementation is that the entire UTXO set must be loaded in RAM (in mapCoins) before being written to disk. So running dumptxoutset will consome a lot of RAM on mainnet since the utxo set is large.
> Not sure how to improve this.

The keys in leveldb are guaranteed to be lexicographically sorted. You just need to cache the last txid and flush the txid and outpoint tuples once the next one is reached in the database iterator. I pushed a dirty proof of concept [her
...

💬 pablomartin4btc 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#issuecomment-1451948439)
> The only thing i suggest changing is giving a more descriptive error on the following situation :
> ```
> $ bitcoin-cli -regtest -generate 1 -rpcwallet=tst2
> error: Error: this is not a valid cli-command argument "-rpcwallet=tst2", if that is a valid option you might try passing it before -generate
> ```

Thanks, I agree with it, updated it.