💬 maflcko commented on pull request "log: Use ConstevalFormatString":
(https://github.com/bitcoin/bitcoin/pull/30889#issuecomment-2347330955)
Sure, dropped the commit. It probably matters so little that reviewing it isn't worth it.

💬 fabioBaraDev commented on issue "No such file or directory: bitcoind Error":
(https://github.com/bitcoin/bitcoin/issues/30891#issuecomment-2347334417)
> Please share the configure summary, the make output, and the exact commands.

I do not have the commands result when I installed the dependencies, the only issue that I had was about the miniupnpc, I had to install the 2.27 version using brew to make it work...
I will re install the commands and send you the response ....

but this is the make command result (I used root user this time):

```
sh-3.2# make
CXX bitcoind-bitcoind.o
CXX init/bitcoind-bitcoind.o
CXX
...

💬 maflcko commented on issue "No such file or directory: bitcoind Error":
(https://github.com/bitcoin/bitcoin/issues/30891#issuecomment-2347337791)
What is `ls src/bitcoind && pwd`? Is it `/Users/user/Documents/personalProjects/bitcoin/src/bitcoind`? What is the test command you call?

💬 fabioBaraDev commented on issue "No such file or directory: bitcoind Error":
(https://github.com/bitcoin/bitcoin/issues/30891#issuecomment-2347341623)
I called `test/functional/test_runner.py` after the make command...

in this `make`command log, there is a warning related to berkeley-db and other libs... can it be a version problem?

💬 hodlinator commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#issuecomment-2347355369)
New variant inspired by @antonilol with same disk space requirements as my 2nd variant above (avoiding my messy and not 100% functional block-lookup-by-height code from variant 3 for now). Much cleaner, removing requirement for additional locking and removes dependencies on `BlockManager` and `TxIndex` - on parity with variant 1 but with the disk space savings of variant 2!

### Variant 4: [txidprefix+vout_CDiskTxPos](https://github.com/bitcoin/bitcoin/compare/20b259332ff1d2e6ca6b291ffeda00b51
...

🤔 pablomartin4btc reviewed a pull request: "test: Check already deactivated network stays suspended after dumptxoutset"
(https://github.com/bitcoin/bitcoin/pull/30892#pullrequestreview-2301535327)
ACK 72c9a1fe94f927220d3159f516fda684ae9d4caa

💬 pablomartin4btc commented on pull request "test: Check already deactivated network stays suspended after dumptxoutset":
(https://github.com/bitcoin/bitcoin/pull/30892#discussion_r1757670575)
nit:
```suggestion
self.log.info(f"Test that dumptxoutset with rollback type fails given an invalid height")
assert_raises_rpc_error(
```

💬 fjahr commented on pull request "test: Check already deactivated network stays suspended after dumptxoutset":
(https://github.com/bitcoin/bitcoin/pull/30892#discussion_r1757681797)
Hm, I don't think it makes sense to add that. First, the actual error here is not the point of this test. This test is about the network activity and even if it covers something else I would like that to be structured differently then and have the logs be in `run_test`. Where it is now it would also be printed twice. Second, the content isn't right: The reason this fails is not the rollback height, it's because the rev file can not be read, so the rolling back action failed as in, it could not r
...

💬 Roasbeef commented on pull request "rename TransactionError:ALREADY_IN_CHAIN":
(https://github.com/bitcoin/bitcoin/pull/30212#issuecomment-2347371722)
Does `bitcoind` consider this to be a breaking change? The new error message may be "more accurate", but for clients that were matching on this error to figure out why a transaction was rejected, this breaks old behavior.

Alternatively, the error constant could be renamed, with the error text/string left in place.

💬 pablomartin4btc commented on pull request "test: Check already deactivated network stays suspended after dumptxoutset":
(https://github.com/bitcoin/bitcoin/pull/30892#discussion_r1757707660)
Yes, that's correct. Thanks! I got confused by the error message, block height was actually valid (`COINBASE_MATURITY = 100`), then we are missing testing the rollback type given an invalid height (?).

🤔 pablomartin4btc reviewed a pull request: "gui: fix crash when closing wallet"
(https://github.com/bitcoin-core/gui/pull/835#pullrequestreview-2301586812)
ACK a965f2bc07a3588f8c2b8d6a542961562e3f5d0e

Went thru the code and as commented in the new `WalletController::removeWallet` function that's now been called instead of `removeAndDeleteWallet`, it will end up calling later `removeAndDeleteWallet` (within the connection to `unload` signal in `WalletController::getOrCreateWallet` - `unload` triggered by `wallet().remove()`) which will emmit `walletRemoved` connected to `BitcoinGUI::removeWallet` (this for example will remove the wallet from the
...

💬 vostrnad commented on pull request "streams: cache file position within AutoFile":
(https://github.com/bitcoin/bitcoin/pull/30884#issuecomment-2347429510)
I'm happy to report that a quick benchmark of 4cfff4e58c6d806e4bc5a12386f84ff207c83419 cherry-picked onto v28.0rc1 shows that the slowdown is almost or entirely gone. I'll do a more thorough benchmark later once the code is ACKed.

💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1757737177)
I don't have a strong view, but I'm trying to think about what a mining pool might want the behavior to be. One guess is that some miners may want to have a way to prioritise normal-looking transactions but not inadvertently prioritise something that would create a dust output and spam the UTXO set with garbage that will never be spent.

If that is the case, then we could try to build the heavier machinery you're talking about: perhaps change the behavior of `prioritisetransaction` to produce
...

✅ fabioBaraDev closed an issue: "No such file or directory: bitcoind Error"
(https://github.com/bitcoin/bitcoin/issues/30891)

💬 fabioBaraDev commented on issue "No such file or directory: bitcoind Error":
(https://github.com/bitcoin/bitcoin/issues/30891#issuecomment-2347671266)
I found out...
I use sentinel one as antivirus,

💬 BenWestgate commented on pull request "Drop -dbcache limit":
(https://github.com/bitcoin/bitcoin/pull/28358#issuecomment-2347927495)
> I ran benchmarks to evaluate the impact of removing the limit on IBD performance. ... I synced up 3 times to block height 600,000...

Thanks for this benchmark. The current block height is 861,090 and the size of the utxo set has more than doubled since 600,000 so this is why you do not see performance improvements from dbcache > 10 GB.

> Based on this data I think removing the limit may not be substantiated...

Try syncing to 860,000 there should be performance benefit up to 30 GiB dbc
...

💬 davidgumberg commented on pull request "streams: cache file position within AutoFile":
(https://github.com/bitcoin/bitcoin/pull/30884#discussion_r1758096576)
Only commenting because this confused me:

This makes sense since the third possibility here is that `origin == SEEK_END`, in which case the only way for `fseek` to have returned zero is if `offset` is a negative number offset from the end of the file, and `m_position = length_of_file + negative_offset`

But, in order to get the length of the fil you need to `fseek(m_file, 0, SEEK_END)` and `ftell(m_file)`.

So, just `ftell` here is the best way to get the offset.

🤔 BenWestgate reviewed a pull request: "Drop -dbcache limit"
(https://github.com/bitcoin/bitcoin/pull/28358#pullrequestreview-2302029807)
crACK bb3b980dfd96d9a89e6b04aef2859ce0555c6807.

Setting `dbcache` below available RAM performs better and swaps less than setting it below total RAM, which may even crash the machine or OOM kill other applications.

I feel the tool tip and argument texts should say "available RAM" but it's not a blocker for me.

💬 BenWestgate commented on pull request "Drop -dbcache limit":
(https://github.com/bitcoin/bitcoin/pull/28358#discussion_r1758106872)
Nit: "Make sure you have enough available RAM." seems more correct.

💬 BenWestgate commented on pull request "Drop -dbcache limit":
(https://github.com/bitcoin/bitcoin/pull/28358#discussion_r1758098327)
nit: This should say "RAM available" rather than "RAM".

On a 64 GB system without swap: it will crash if you set `dbcache=32000` but only have only have 20 GB available.

On a 64 GB system with enough swap: it will perform MUCH worse and prematurely wear out the flash storage, if you set `dbcache=32000` but only have only have 20 GB available RAM.

(I've destroyed new USB sticks overnight, putting a swapfile on them and doing IBD with `dbcache` set a little too high.)

The only reason
...