💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1757628003)
yes, this was my open question https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2332066227

Even if we do this change it can still be prioritized for mining *after* entering the mempool; we would need heavier machinery to stop ever treating it as prioritised. This would mean someone would have to create a CPFP transaction then cycle it away, while also using `prioritisetransaction` to get it mined without the dust spent?

@sdaftuar you have any thoughts?

💬 mzumsande commented on pull request "scripted-diff: Modernize nLocalServices naming":
(https://github.com/bitcoin/bitcoin/pull/30885#issuecomment-2347306044)
For the record, I don't like refactor PRs that just rename things to a new convention. It's fine for me if the code is touched anyway, but opening a PR just for that? We probably have hundreds of variables that could be modernized the same way.

🤔 jonatack reviewed a pull request: "test: Introduce ensure helper"
(https://github.com/bitcoin/bitcoin/pull/30893#pullrequestreview-2301482520)
Concept ACK

💬 jonatack commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1757631578)
For clarity/readability, suggest using a `timeout=` kwarg in the `ensure` invocations where you pass a timeout value.

💬 fabioBaraDev commented on issue "No such file or directory: bitcoind Error":
(https://github.com/bitcoin/bitcoin/issues/30891#issuecomment-2347309054)
Seems like the bitcoind.cpp file was not compiled and when I try to run the Functional tests, it does not find the folder .... :/

💬 fjahr commented on pull request "scripted-diff: Modernize nLocalServices naming":
(https://github.com/bitcoin/bitcoin/pull/30885#issuecomment-2347309527)
> For the record, I don't like refactor PRs that just rename things to a new convention. It's fine for me if the code is touched anyway, but opening a PR just for that? We probably have hundreds of variables that could be modernized the same way.

Sure, I agree and I had not opened it if I had not already written the PR and had been encouraged to open it as a follow-up even though the PR where the code was touched was merged without it, see https://github.com/bitcoin/bitcoin/pull/30807#issueco
...

💬 maflcko commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1757633150)
```suggestion
def ensure(self, test_function, *, duration):
```

I think calling this "timeout" may be a bit confusing, because it will always "timeout". Maybe "duration" can be used to communicate that this always takes a fixed span of time?

💬 maflcko commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1757636065)
Why should this be scaled by the timeout factor? If someone increased that sufficiently to not have to deal with timeouts, they will be bitten by tests that never terminate. IIRC the CI has a large factor, so it may lead to timeouts there.

I think just making the duration parameter a required named arg would be cleaner and also better document that the test will spend that amount of time "sleeping".

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

📝 furszy opened a pull request: "gui: fix crash when closing wallet"
(https://github.com/bitcoin-core/gui/pull/835)
The crash occurs because `WalletController::removeAndDeleteWallet` is called twice for the
same wallet model: first in the GUI's button connected function `WalletController::closeWallet`,
and then again when the backend emits the `WalletModel::unload` signal.

This causes the issue because `removeAndDeleteWallet` inlines an `erase(std::remove())`.
So, if `std::remove` returns an iterator to the end (indicating the element wasn't found
because it was already erased), the subsequent call to
...

💬 furszy commented on issue "Closing a wallet using the fa46088440 28.x QT client segfaults":
(https://github.com/bitcoin/bitcoin/issues/30887#issuecomment-2347322116)
See https://github.com/bitcoin-core/gui/pull/835

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