💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1764229313)
thank you! Addressed this in [bc4a929](https://github.com/bitcoin/bitcoin/pull/30875/commits/bc4a929cd716cd2b412c70754749d4fda0ca2a10)

I will take a look at the backtick nit, to see if I can find already existing instances of `cmake` with no backticks

💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1764229340)
thank you! Addressed this in [bc4a929](https://github.com/bitcoin/bitcoin/pull/30875/commits/bc4a929cd716cd2b412c70754749d4fda0ca2a10)

💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2357229647)
> Concept ACK
>
> Nit: could also fix
>
> ```diff
> --- a/test/util/test_runner.py
> +++ b/test/util/test_runner.py
> @@ -5,7 +5,7 @@
> # file COPYING or http://www.opensource.org/licenses/mit-license.php.
> """Test framework for bitcoin utils.
>
> -Runs automatically during `make check`.
> +Runs automatically during `ctest --test-dir build/`.
>
> Can also be run manually."""
> ```

thank you! Addressed this in [bc4a929](https://github.com/bitcoin/bitcoin/pull/30875/commi
...

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

pico-nit: your branch messes up the 80 character wrapping in a few spots in `doc/developer-notes.md`. This is not really consistent through the docs anyways, so I don't mind personally.

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