💬 mzumsande commented on issue "Race condition between ZMQ UpdateTip and getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/30862#issuecomment-2347161468)
can you rule out that someone else on the network just mined a block, and the inconclusive `getblocktemplate` result belongs to a previous call that was sent off before that new block was connected?

💬 fjahr commented on pull request "scripted-diff: Modernize nLocalServices to m_local_services":
(https://github.com/bitcoin/bitcoin/pull/30885#discussion_r1757532379)
Hm, ok, I interpreted the comment on the namespace above as that we ignore that here. But I missed the variable below and I the others are just older. Changed.

💬 davidgumberg commented on pull request "streams: cache file position within AutoFile":
(https://github.com/bitcoin/bitcoin/pull/30884#issuecomment-2347208216)
I think that there are also some calls to `std::rewind`:
https://github.com/bitcoin/bitcoin/blob/cf0120ff024aa73a56f2975c832fda6aa8146dfa/src/test/streams_tests.cpp#L264

and `std::fgetc`:
https://github.com/bitcoin/bitcoin/blob/cf0120ff024aa73a56f2975c832fda6aa8146dfa/src/node/utxo_snapshot.cpp#L76

that are problematic if we are caching file position.

Also, if it seems sensible to mitigate the risk of someone modifying the file position in the future without changing `m_position`
...

🤔 furszy reviewed a pull request: "interfaces: #30697 follow ups"
(https://github.com/bitcoin/bitcoin/pull/30828#pullrequestreview-2301395429)
Code review ACK 84663291275248fd52da644b0c2566bbf9cc780b

It seems that with the latest changes, the `deleteRwSettings` is no longer used.
It would be nice to add test coverage for it and for the null value usage as well.

👍 marcofleon approved a pull request: "refactor: move m_is_inbound out of CNodeState"
(https://github.com/bitcoin/bitcoin/pull/30233#pullrequestreview-2301402833)
ACK 07f4cebe5780f1039541d989e64b70eccc5b4eb5

I took a look at c308e3200d6968be553ed031d09b4bccb46b504b and 2e0b6742b82e60ea685afd25f2d19b8b558678ce in https://github.com/bitcoin/bitcoin/pull/30116 (assuming those commits are mostly final) to understand a bit better how `m_is_inbound` will be used for Erlay. Based on that, I'd say the changes in this PR make sense.

🤔 furszy reviewed a pull request: "scripted-diff: Modernize nLocalServices to m_local_services"
(https://github.com/bitcoin/bitcoin/pull/30885#pullrequestreview-2301405397)
utACK 33381ea530ad79ac1e04c37f5707e93d3e0509ca

💬 furszy commented on pull request "scripted-diff: Modernize nLocalServices to m_local_services":
(https://github.com/bitcoin/bitcoin/pull/30885#discussion_r1757575935)
tiny nit:
I know this is an scripted-diff but.. could call `GetLocalServices()` here.

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

A better test would be to reproduce this with regtest which I'll create a
script to demonstrate soon.

But at the same time it seems there should be a mutex around the chain tip.
If there isn't this is basically guaranteed to happen. I haven't looked yet.

On Thu, Sep 12, 2024, 4:18 PM Martin Zumsande ***@***.***>
wrote:

> can you rule out that someone else on the network just mined a block, a
...

💬 katesalazar commented on issue "Closing a wallet using the fa46088440 28.x QT client segfaults":
(https://github.com/bitcoin/bitcoin/issues/30887#issuecomment-2347237302)
In my environment, 5d15485a is a crashing rev
and inmediate ancestor 1a41e635 is a non-crashing rev.

🤔 jonatack reviewed a pull request: "scripted-diff: Modernize nLocalServices to m_local_services"
(https://github.com/bitcoin/bitcoin/pull/30885#pullrequestreview-2301419943)
Review ACK 33381ea530ad79ac1e04c37f5707e93d3e0509ca

Following the last push, suggest renaming the pull title to `scripted-diff: Modernize nLocalServices naming`

💬 katesalazar commented on issue "Closing a wallet using the fa46088440 28.x QT client segfaults":
(https://github.com/bitcoin/bitcoin/issues/30887#issuecomment-2347251385)
I came back to fa46088440, then reverted 5d15485a on top, _looks like healed_.

💬 fjahr commented on pull request "scripted-diff: Modernize nLocalServices naming":
(https://github.com/bitcoin/bitcoin/pull/30885#issuecomment-2347258648)
> Following the last push, suggest renaming the pull title to `scripted-diff: Modernize nLocalServices naming` (edit: and also the commit name)

done

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2347263134)
Rebased.

💬 furszy commented on issue "Closing a wallet using the fa46088440 28.x QT client segfaults":
(https://github.com/bitcoin/bitcoin/issues/30887#issuecomment-2347263442)
Replicated. It happens closing it through the GUI toolbar button. Not the command-line. Will investigate it.

🤔 ryanofsky reviewed a pull request: "multiprocess: Add IPC wrapper for Mining interface"
(https://github.com/bitcoin/bitcoin/pull/30510#pullrequestreview-2301289042)
Updated 1be749c771cd9fd80361ebb69c87482920b25cd1 -> b95bb2179610183d9398d50d8c8fd24b1450ad4d ([`pr/mine-types.10`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.10) -> [`pr/mine-types.11`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.11), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine-types.10..pr/mine-types.11)) switching to concepts instead of enable_if

💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1757602140)
re: https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1757372497

Removed enable_if so this should be simpler now. Thanks for bringing this up!

📝 fjahr opened a pull request: "test: Check already deactivated network stays suspended after dumptxoutset"
(https://github.com/bitcoin/bitcoin/pull/30892)
Follow-up to #30817 which covered the robustness of `dumptxoutset`: network is deactivated during the run but re-activated even when an issue was encountered. But it did not cover the case if the user had deactivated the network themselves before. In that case the user may want the network to stay off so the network is not reactivated after `dumptxoutset` finishes. A test for this behavior is added here.

💬 fjahr commented on pull request "test: Add coverage for dumptxoutset failure robustness":
(https://github.com/bitcoin/bitcoin/pull/30817#issuecomment-2347273844)
> could go one step further and even test that if the network was already disabled (via setnetworkactive False), it won't be enabled after a dumptxoutset call.

Added in #30892

💬 fabioBaraDev commented on issue "No such file or directory: bitcoind Error":
(https://github.com/bitcoin/bitcoin/issues/30891#issuecomment-2347283479)
I was not using the right tutorial version, I did everything over again, but still got the same problem, I ran all the commands from this link
`https://github.com/bitcoin/bitcoin/blob/v27.1/doc/build-osx.md`

but this time it says that at leat 21 tests ran OK...
still [Errno 2] No such file or directory: '/Users/user/Documents/personalProjects/bitcoin/src/bitcoind'

check it out

```
Temporary test directory at /var/folders/_t/zldzv3sn05qdk78yltr69h340000gp/T/test_runner_₿_🏃_20240912
...

🤔 glozow reviewed a pull request: "Ephemeral Dust"
(https://github.com/bitcoin/bitcoin/pull/30239#pullrequestreview-2301347857)
Finer comb of the first 2 commits