💬 glozow commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1764235865)
Maybe should have a method for testing that a tx isn't in orphanage so we can test that here

💬 glozow commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1764240141)
thought so... how is it intended to be used?

💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#issuecomment-2357254842)
@glozow It's already used in two places in this PR.

Maybe I should clarify in the PR description that this is just a generic utility feature, not something the next layer up will need per se (AFAIK). But because I was introducing a second place where it's invoked, and it's already used somewhere in the test, I felt it made sense to turn it into a proper DepGraph function, with proper tests.

💬 davidgumberg commented on something "":
(https://github.com/bitcoin/bitcoin/commit/32680702808557517c77507e9f42a1bcf94a7ab2#r146852995)
I believe your PR makes sure that `DisconnectMsg` gets printed any time `fDisconnect == true` so this line results in a duplicate log in each of those instances.

For example when doing `bitcoin-cli setnetworkactive false`:

```console
2024-09-18T00:32:49Z SetNetworkActive: false
2024-09-18T00:32:49Z [net] Network not active, disconnecting peer=0 peeraddr=xx.xx.xx.xx:8333
2024-09-18T00:32:49Z [net] Network not active, disconnecting peer=1 peeraddr=xx.xx.xx.xx:8333
2024-09-18T00:32:49Z [n
...

💬 glozow commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#issuecomment-2357265641)
Sorry, I don't know how I missed the calls within this PR :facepalm: I think I was grepping only on the first commit.

💬 davidgumberg commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1764247666)
I believe your PR makes sure that `DisconnectMsg` gets printed any time `fDisconnect == true` so this line results in a duplicate log in each of those instances.

For example when doing `bitcoin-cli setnetworkactive false`:

```console
2024-09-18T00:32:49Z SetNetworkActive: false
2024-09-18T00:32:49Z [net] Network not active, disconnecting peer=0 peeraddr=xx.xx.xx.xx:8333
2024-09-18T00:32:49Z [net] Network not active, disconnecting peer=1 peeraddr=xx.xx.xx.xx:8333
2024-09-18T00:32:49Z [n
...

💬 tdb3 commented on pull request "fix: handle invalid `-rpcbind` port earlier":
(https://github.com/bitcoin/bitcoin/pull/30679#issuecomment-2357292820)
Rebased to address conflict

💬 maflcko commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2357565564)
CI failure looks unrelated (like an old RPC shutdown bug in 0.16.3). https://cirrus-ci.com/task/6654771535282176?logs=ci#L2818

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1764488736)
Done

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1764497635)
Done

📝 maflcko opened a pull request: "test: Remove 0.16.3 test from wallet_backwards_compatibility.py"
(https://github.com/bitcoin/bitcoin/pull/30920)
The test checks that any wallet created with current master can not be loaded with `v0.16.3`. This is interesting documentation, however it is probably not something to keep as a test, because:

* It seems like an extremely unlikely (and unsupported) edge case that someone creates a wallet with master and then goes ahead to open it with a long EOL software version.
* A better test would be the inverse: Create a wallet with `v0.16.3` and open it with current master. It may be good to add such
...

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1764507421)
Done, thanks!

👋 l0rinc's pull request is ready for review: "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests"
(https://github.com/bitcoin/bitcoin/pull/30906)

💬 maflcko commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1764514580)
This should use the preset `dev-mode`. Otherwise, deps may be missed.

💬 maflcko commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1764520558)
```suggestion
// for posix_fallocate, in cmake/introspection.cmake we check if it is present after this
```

💬 maflcko commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1764514975)
```suggestion
This can be checked by building the `translate` target with `cmake` ([instructions](translation_process.md)), then verifying that `bitcoin_en.ts` remains unchanged.
```

💬 maflcko commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2357683381)
review ACK bc4a929cd716cd2b412c70754749d4fda0ca2a10

✅ maflcko closed an issue: "CI timeouts"
(https://github.com/bitcoin/bitcoin/issues/30851)

💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2357742653)
Closing for now. Possibly commit a95e742b69207d7541afc6903b3fd72131f4d6de (x86_64-apple-darwin22.6.0 -> arm64-apple-darwin23.6.0) may have switched to using more recent hardware, which may be less likely to be corrupt.

In any case, if someone is still seeing a CI timeout (that is a CI failure caused by a 2 hour timeout), or a CI task that takes longer than half of the timeout (1h), it would be good to mention it either in this issue, or in a new issue.

The 2 hour CI timeout is picked, so t
...

🤔 hodlinator reviewed a pull request: "refactor: migrate `bool GetCoin` to return `optional<Coin>`"
(https://github.com/bitcoin/bitcoin/pull/30849#pullrequestreview-2311839788)
Drive-by minimum-context, admittedly low-PoW review, prompted by author.

Dislike adding of new TODO's but if full-context reviewers accept, I guess it's okay.