💬 glozow commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1764229521)
Maybe self-explanatory :sweat_smile:

💬 glozow commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1764236837)
I was briefly confused about ordering stability, and then realized this has length 1

💬 glozow commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1764231050)
maybe "bytes" instead? to make it hard to confuse with vsize

💬 glozow commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1764230149)
Perhaps `GetOrphanTransactions`, since this is on the peerman and orphan blocks can exist too

💬 glozow commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1764231219)
mention BIP 141?

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