💬 instagibbs commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#issuecomment-2432809937)
@tdb3 reasonable suggestion, shouldn't take much effort?

💬 ismaelsadeeq commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1813159728)
> For the package case, given that the txpackages log level is already reporting what transactions are in the package, does it make sense to duplicate that information in the mempool log category as well

No it does not

> An issue that occurred to me with splitting up log messages for what's in a package across multiple lines is that other threads may be logging simultaneously and cause messages to be interleaved, making the resulting log difficult to parse.

Good point

> if we logged
...

💬 tdb3 commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#issuecomment-2432815018)
> @tdb3 reasonable suggestion, shouldn't take much effort?

Yeah, I think it would just be moving the logic over to `p2p_orphan_handling`.

💬 achow101 commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1813164285)
"The default wallets directory is the 'wallets' folder inside the data directory, unless the `-walletdir` option is specified."

This sentence is a little bit confusing as it suggests `-walletdir` changes the default somehow. I would rephrase this as "The wallets directory is set by the `-walletdir` option and defaults to the 'wallets' folder within the data directory."

💬 wonder75 commented on issue "LevelDB read failure: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2432829746)
I now upgraded to bitcoind 28.0, deleted the blockchain and all indexes. I started a fresh new initial block download. Will report here, if it crashes again. I have a feeling that it could have something to do with starting and stopping the node regulary. My bitcoind only runs for 30 minutes a day to catch up on the blockchain. I start and stop it with cron and systemd.

🤔 stickies-v reviewed a pull request: "Remove mempoolfullrbf"
(https://github.com/bitcoin/bitcoin/pull/30592#pullrequestreview-2389456426)
Concept ACK

💬 edilmedeiros commented on pull request "depends: bump miniupnpc to 2.2.8":
(https://github.com/bitcoin/bitcoin/pull/30301#issuecomment-2432882285)
Just curious if this is still relevant after #30043?

💬 edilmedeiros commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2432886068)
Concept ACK

💬 apulsifer commented on issue "LevelDB read failure: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/30159#issuecomment-2432889367)
When I was initially setting up these systems, I was starting and stopping bitcoind. But since the setup was completed, bitcoind has been running continuously without stopping, and I still saw data corruption.

💬 maflcko commented on issue "How to compile the GUI on opensuse tumbleweed with cmake?":
(https://github.com/bitcoin-core/gui/issues/842#issuecomment-2432924953)
(This worked fine with autotools)

💬 maflcko commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#issuecomment-2432947751)
Coverage for this target: https://drahtbot.space/host_reports/DrahtBot/reports/coverage_fuzz/monotree/2e7ac47bbc28c2cb/300ddaa72f7e4b9b/fuzz.coverage/index.html

💬 edilmedeiros commented on pull request "doc: replace `-?` with `-h` and `-help`":
(https://github.com/bitcoin/bitcoin/pull/31118#issuecomment-2433055899)
Concept ACK

💬 theStack commented on issue "TestFramework: TestShell.reset() will always fail":
(https://github.com/bitcoin/bitcoin/issues/31131#issuecomment-2433056070)
It seems that the TestShell instructions generally don't work anymore since we switched to CMake. For me they fail already earlier at the `TestShell` instantiation, as the generated `config.ini` file is searched in a path that still assumes in-tree builds:
```
$ git rev-parse HEAD
ffe4261cb0669b1e1a926638e0498ae5b63f3599
$ python3
Python 3.10.12 (main, Sep 11 2024, 15:47:36) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sy
...

💬 l0rinc commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1813088395)
might as well put the whole test suite inside instead of having an empty passing test

💬 l0rinc commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1813079479)
nit: we're inside `namespace tinyformat`:
```suggestion
} catch (format_error& fmterr) {
```

(same for `formatImpl`)

📝 laanwj opened a pull request: "doc: Make list of targets in depends README consistent"
(https://github.com/bitcoin/bitcoin/pull/31141)
The description of `i686-pc-linux-gnu` and `x86_64-pc-linux-gnu` is incomplete and inconsistent with the others. Fix this. Also use "64 bit" consistently instead of "64-bit".

💬 maflcko commented on pull request "doc: Make list of targets in depends README consistent":
(https://github.com/bitcoin/bitcoin/pull/31141#issuecomment-2433122900)
lgtm ACK a0c9595810c7d8bb17d8b5bea8d916db194b5239

💬 edilmedeiros commented on pull request "doc: replace `-?` with `-h` and `-help`":
(https://github.com/bitcoin/bitcoin/pull/31118#issuecomment-2433144931)
tACK 33a28e252a7349c0aa284005aee97873b965fcfe

I would prefer everything in a single commit, but it's OK.

👍 hebasto approved a pull request: "doc: Make list of targets in depends README consistent"
(https://github.com/bitcoin/bitcoin/pull/31141#pullrequestreview-2389771410)
ACK a0c9595810c7d8bb17d8b5bea8d916db194b5239.

💬 hodlinator commented on pull request "fees: Remove CLIENT_VERSION serialization":
(https://github.com/bitcoin/bitcoin/pull/29702#issuecomment-2433178606)
re-ACK fa1c5cc9df116411edca172f8b80fc225c776554

Thanks for adjusting the version error for too high values!

Makes sense to remove explicit newlines given #30929.