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

💬 brunoerg commented on pull request "net: Modernize logging in UPnP and nat-pmp code":
(https://github.com/bitcoin/bitcoin/pull/29978#issuecomment-2081177346)
Concept ACK

💬 fjahr commented on pull request "chainparams: Change nChainTx type to uint64_t":
(https://github.com/bitcoin/bitcoin/pull/29656#issuecomment-2081190733)
> Concept ACK. I think the pull description can be rewritten, since it ends up in plain text in the merge commit. Instead of writing "Thing A is not included. It is now", it would be better to say why it is included and remove the formatting/negation: "Thing A is included, because (...motivation...)"

I have updated the description.

💬 davidgumberg commented on pull request "test: Add missing Assert(mock_time_in >= 0s) to SetMockTime":
(https://github.com/bitcoin/bitcoin/pull/29872#discussion_r1581912776)
nit:

```suggestion
void SetMockTime(int64_t mock_time_in) { SetMockTime(std::chrono::seconds{mock_time_in}); }
```

💬 davidgumberg commented on pull request "test: Add missing Assert(mock_time_in >= 0s) to SetMockTime":
(https://github.com/bitcoin/bitcoin/pull/29872#issuecomment-2081205293)
crACK https://github.com/bitcoin/bitcoin/pull/29872/commits/fae0db555c12dca75fb09e5fa7bbabdf39b8c1df