💬 mzumsande commented on issue "Race condition between ZMQ UpdateTip and getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/30862#issuecomment-2347297919)
> I believe that's answered positively by the timestamps and structure of the calling code.

not really. I don't see a `"Calling bitcoin-cli: getblocktemplate"` entry in the [log](https://github.com/braidpool/bitcoin/issues/1#issuecomment-2341397203) excerpt, but I do see a `DEBUG result =` log entry.

Both logs are part of `async def bitcoin_cli(self, *args, input=b""):` in your script. That's why I thought the rpc call may have happened before the block was connected by bitcoind.

📝 fjahr opened a pull request: "test: Introduce ensure helper"
(https://github.com/bitcoin/bitcoin/pull/30893)
A repeating pattern in the functional tests is that the test sleeps for a while to ensure that a certain condition is still true after some amount of time has elapsed. Most recently a new case of this was added in #30807. This PR here introduces an `ensure` helper to streamline this functionality.

Some approach considerations:
- It is possible to construct this by reusing `wait_until` and wrapping it in `try` internally. However, the logger output of the failing wait would still be printed w
...

💬 sipa commented on pull request "streams: cache file position within AutoFile":
(https://github.com/bitcoin/bitcoin/pull/30884#issuecomment-2347300303)
@davidgumberg Good points. I've made minimal changes in this PR to make sure no position-effecting operations remain, as we're past the feature freeze. Extending the `AutoFile` interface so that `AutoFile::Get` is no longer needed can be done as a follow-up.

💬 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