🤔 furszy reviewed a pull request: "blockstorage: Separate reindexing from saving new blocks"
(https://github.com/bitcoin/bitcoin/pull/29975#pullrequestreview-2026654546)
Concept ACK

💬 stickies-v commented on pull request "Disable util::Result copying and assignment":
(https://github.com/bitcoin/bitcoin/pull/29906#issuecomment-2080752984)
Re-ACK 9552619961049d7673f84c40917b0385be70b782 , just addressing nits, no other changes

💬 furszy commented on pull request "rpc, wallet: fix incorrect segwit redeem script size limit":
(https://github.com/bitcoin/bitcoin/pull/28307#discussion_r1581838289)
sure, will change it if I have to retouch the code.

💬 furszy commented on pull request "rpc, wallet: fix incorrect segwit redeem script size limit":
(https://github.com/bitcoin/bitcoin/pull/28307#discussion_r1581838453)
sure, will change it if I have to retouch this code.

💬 maflcko commented on pull request "build: Bump clang minimum supported version to 15":
(https://github.com/bitcoin/bitcoin/pull/29165#issuecomment-2080804374)
Rebased and taken out of draft. I've opened two pull requests against oss-fuzz to bump the compiler there (one globally and one for bitcoin-core), but they are waiting for merge. If they are not merged, I think we can consider oss-fuzz as unsupported and drop it temporarily. Oss-fuzz is nice, but it seems odd to consider it a blocker here.

💬 maflcko commented on pull request "rpc: Avoid getchaintxstats invalid results":
(https://github.com/bitcoin/bitcoin/pull/29720#issuecomment-2080828548)
> 03a0f31

I think `assert_equal` can generally not be used on floating point values, no?

💬 maflcko commented on pull request "rpc: Avoid getchaintxstats invalid results":
(https://github.com/bitcoin/bitcoin/pull/29720#discussion_r1581842446)
Thanks, done

💬 Randy808 commented on issue "intermittent issue in wallet_backwards_compatibility.py: line 245, in run_test assert txs[3]["abandoned"] AssertionError":
(https://github.com/bitcoin/bitcoin/issues/29806#issuecomment-2080840395)
The [relevant part of the test](https://github.com/bitcoin/bitcoin/blob/7fee0ca014b1513e836d2550d1b7512739d5a79a/test/functional/wallet_backwards_compatibility.py#L173-L176) in `wallet_backwards_compatibility.py` creates tx3, calls bump fee to replace it in the wallet (creating tx4), and calls `abandontransaction` on tx3. Later on there's an assertion to check if tx3 is marked as abandoned, but this is the call that sporadically fails.

The failure seems to occur because the `TransactionAddedT
...

💬 maflcko commented on pull request "rpc: Avoid getchaintxstats invalid results":
(https://github.com/bitcoin/bitcoin/pull/29720#discussion_r1581842910)
I think both should behave the same in this context.

💬 stickies-v commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#issuecomment-2080843513)
> The problem is that https://github.com/bitcoin/bitcoin/pull/29700 updates some validation functions to return util::Result instead of bool.

I see, that was not apparent in the [link](https://github.com/ryanofsky/bitcoin/blob/55d7de92bbbe035c1833c89f885af14e5b243932/src/node/chainstate.cpp#L38-L179) you shared earlier. Would it make sense to do that refactoring in #29700 then, instead of in this PR? Because in this PR, I don't think the change makes a lot of sense on its own.

> I think a
...

💬 maflcko commented on issue "intermittent issue in wallet_backwards_compatibility.py: line 245, in run_test assert txs[3]["abandoned"] AssertionError":
(https://github.com/bitcoin/bitcoin/issues/29806#issuecomment-2080852359)
If the transaction is eventually considered abandoned in all cases, then adding the `sync_mempools` (or I guess `syncwithvalidationinterface` would be enough) seems fine.

⚠️ luke-jr opened an issue: ""Migrate Wallet" is unclear to translators"
(https://github.com/bitcoin/bitcoin/issues/29979)
Going through translations, I'm seeing things that suggest "migration" is being misinterpreted in the sense of animals migrating (moving/relocating). We could clarify with translator comments (assuming translators actually see them), but that won't help if non-native English speakers are using Bitcoin Core in English, so maybe a better approach is to come up with another word for this?

💬 m3dwards commented on pull request "depends: pass verbose through to cmake based makefiles":
(https://github.com/bitcoin/bitcoin/pull/29960#issuecomment-2081071753)
I have tried to look for an alternative variable that will enable verbose output regardless of build system but I couldn't find one. I thought this looked promising [https://cmake.org/cmake/help/latest/envvar/VERBOSE.html](https://cmake.org/cmake/help/latest/envvar/VERBOSE.html) but it didn't work for me.

💬 tdb3 commented on pull request "rpc: return warnings as an array instead of just a single one":
(https://github.com/bitcoin/bitcoin/pull/29845#issuecomment-2081107100)
> > both curl and bitcoin-cli returned an error:
>
> I suspect you're running with `-rpcdoccheck`, you shouldn't get any errors without it? I'm not sure if `-rpcdoccheck` is meant to be an internal-only testing flag (as per #24695), but it [doesn't seem to be documented as such](https://github.com/bitcoin/bitcoin/blob/2a07c4662d7266158d47f79fa2433ab22e22c907/src/init.cpp#L655), so I guess better to be safe - force pushed to make the docs contingent on the `-rpcdeprecated` too.
>
> Thanks f
...

⚠️ hebasto opened an issue: "depends: Cross-compiling `qt` for `arm-linux-gnueabihf` fails"
(https://github.com/bitcoin/bitcoin/issues/29980)
On Ubuntu 24.04 with glibc 2.39:
```
compiling ../3rdparty/zlib/src/gzclose.c
In file included from /usr/arm-linux-gnueabihf/include/features.h:394,
from /usr/arm-linux-gnueabihf/include/bits/libc-header-start.h:33,
from /usr/arm-linux-gnueabihf/include/stdio.h:28,
from ../3rdparty/zlib/src/gzguts.h:40,
from ../3rdparty/zlib/src/gzclose.c:6:
/usr/arm-linux-gnueabihf/include/features-time64.h:26:5: error: #error "_TIME_BI
...

💬 fjahr commented on pull request "refactor: extract CCheckQueue's data handling into a separate container "Bag"":
(https://github.com/bitcoin/bitcoin/pull/27331#issuecomment-2081161297)
I didn't dive too deeply into this yet but it's a bit sad that we can not remove more code due to this, that would certainly make it a more interesting change. Do you already have other places in mind where we could use this?

You also mention the order of elements, but it's unclear why that is important. This hasn't changed right? Or is this just about the naming choice?

I can't really make sense of the CI failure. Could you do a rebase? Since this is a fairly old change it might be some h
...

🤔 fjahr reviewed a pull request: "sync: improve CCoinsViewCache ReallocateCache"
(https://github.com/bitcoin/bitcoin/pull/28945#pullrequestreview-2026733390)
@martinus Are you still working on this? If yes, it would be great if you could rebase and address @andrewtoth 's comment. If not, I can try to take this over and get it the attention it deserves.

💬 fjahr commented on pull request "sync: improve CCoinsViewCache ReallocateCache":
(https://github.com/bitcoin/bitcoin/pull/28945#discussion_r1581896321)
Should probably add something about this to the docstring.

💬 martinus commented on pull request "sync: improve CCoinsViewCache ReallocateCache":
(https://github.com/bitcoin/bitcoin/pull/28945#issuecomment-2081170953)
@fjahr I'm not working on this any more, feel free to take over!

💬 fjahr commented on pull request "refactor: remove remaining unused code from cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/29961#discussion_r1581902620)
The numbering of this list is off after this change. That might be confusing to someone in the future. I would suggest either updating the following numbering or something like.

```
* 6. (deleted)
* 7. (deleted)
```