💬 pinheadmz commented on issue "getdescriptorinfo returns unusable descriptor":
(https://github.com/bitcoin/bitcoin/issues/29320#issuecomment-1910678143)
Great catch! To summarize, the RPC is returning an extended PUBLIC key followed by a hardened derivation path. Which is usable.
  (https://github.com/bitcoin/bitcoin/issues/29320#issuecomment-1910678143)
Great catch! To summarize, the RPC is returning an extended PUBLIC key followed by a hardened derivation path. Which is usable.
💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1466715496)
Oh, I see where my confusion comes from. I thought podman was just a wrapper for Docker. But it's actually an alternative which can run the same containers. Why not switch over entirely to Podman? It's quite confusing that we're using both.
  (https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1466715496)
Oh, I see where my confusion comes from. I thought podman was just a wrapper for Docker. But it's actually an alternative which can run the same containers. Why not switch over entirely to Podman? It's quite confusing that we're using both.
💬 theStack commented on pull request "test: p2p: check disconnect due to lack of desirable service flags":
(https://github.com/bitcoin/bitcoin/pull/29279#issuecomment-1910708559)
> Concept ACK! i think flaky tests are possible because disconnection could happen before [`p2p_conn.wait_for_connect()`](https://github.com/bitcoin/bitcoin/blob/7699a1aab8fa2a7a602391025f3cfcde29ff2613/test/functional/test_framework/test_node.py#L728) in `add_outbound_connection()`?
True, good catch. The flaky case can be reproduced by adding an artificial delay before the `wait_for_connect()` call, e.g.:
```diff
diff --git a/test/functional/test_framework/test_node.py b/test/functional/te
...
  (https://github.com/bitcoin/bitcoin/pull/29279#issuecomment-1910708559)
> Concept ACK! i think flaky tests are possible because disconnection could happen before [`p2p_conn.wait_for_connect()`](https://github.com/bitcoin/bitcoin/blob/7699a1aab8fa2a7a602391025f3cfcde29ff2613/test/functional/test_framework/test_node.py#L728) in `add_outbound_connection()`?
True, good catch. The flaky case can be reproduced by adding an artificial delay before the `wait_for_connect()` call, e.g.:
```diff
diff --git a/test/functional/test_framework/test_node.py b/test/functional/te
...
📝 theStack converted_to_draft a pull request: "test: p2p: check disconnect due to lack of desirable service flags"
(https://github.com/bitcoin/bitcoin/pull/29279)
This PR adds missing test coverage for disconnecting peers which don't offer the desirable service flags in their VERSION message:
https://github.com/bitcoin/bitcoin/blob/5f3a0574c45477288bc678b15f24940486084576/src/net_processing.cpp#L3384-L3389
This check is relevant for the connection types "outbound-full-relay", "block-relay-only" and "addr-fetch" (see `CNode::ExpectServicesFromConn(...)`). Feeler connections always disconnect, which is also tested here.
In lack of finding a proper file
...
  (https://github.com/bitcoin/bitcoin/pull/29279)
This PR adds missing test coverage for disconnecting peers which don't offer the desirable service flags in their VERSION message:
https://github.com/bitcoin/bitcoin/blob/5f3a0574c45477288bc678b15f24940486084576/src/net_processing.cpp#L3384-L3389
This check is relevant for the connection types "outbound-full-relay", "block-relay-only" and "addr-fetch" (see `CNode::ExpectServicesFromConn(...)`). Feeler connections always disconnect, which is also tested here.
In lack of finding a proper file
...
👍 Retropex approved a pull request: "Add a `-permitbarepubkey` option"
(https://github.com/bitcoin/bitcoin/pull/29309#pullrequestreview-1844359954)
tACK https://github.com/vostrnad/bitcoin/commit/8c1114aa61c46e58efb44368b5428d3ccb65f9d0
Steps to test:
1. Build [vostrnad branch](https://github.com/vostrnad/bitcoin/tree/permitbarepubkey)
1. Start a regtest with `-chain=regtest`
1. Create a wallet with `bitcoin-cli createwallet`
1. Get address with `bitcoin-cli getnewaddress`
1. Generate blocks to get bitcoins `generatetoaddress 500 <address obtained previously>`
1. Create a P2PK tx, this [repository](https://github.com/dianerey/bec
...
  (https://github.com/bitcoin/bitcoin/pull/29309#pullrequestreview-1844359954)
tACK https://github.com/vostrnad/bitcoin/commit/8c1114aa61c46e58efb44368b5428d3ccb65f9d0
Steps to test:
1. Build [vostrnad branch](https://github.com/vostrnad/bitcoin/tree/permitbarepubkey)
1. Start a regtest with `-chain=regtest`
1. Create a wallet with `bitcoin-cli createwallet`
1. Get address with `bitcoin-cli getnewaddress`
1. Generate blocks to get bitcoins `generatetoaddress 500 <address obtained previously>`
1. Create a P2PK tx, this [repository](https://github.com/dianerey/bec
...
💬 luke-jr commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#issuecomment-1910735981)
Actually, why wouldn't fs::permissions work on Windows?
  (https://github.com/bitcoin/bitcoin/pull/28167#issuecomment-1910735981)
Actually, why wouldn't fs::permissions work on Windows?
💬 MarnixCroes commented on pull request "debugwindow: update session ID tooltip":
(https://github.com/bitcoin-core/gui/pull/788#issuecomment-1910736573)
> Concept ACK
>
> nit: @MarnixCroes, maybe you can add the screenshot for transport v1 and session ID is empty/ not shown, if possible, even this PR doesn't affect that logic.
done
  (https://github.com/bitcoin-core/gui/pull/788#issuecomment-1910736573)
> Concept ACK
>
> nit: @MarnixCroes, maybe you can add the screenshot for transport v1 and session ID is empty/ not shown, if possible, even this PR doesn't affect that logic.
done
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466747189)
old comment?
  (https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466747189)
old comment?
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466751943)
at the introductory commit, I don't see how `m_ancestors_of_in_package_ancestors` is non-empty. unused?
I suspect replaced by `package_with_ancestors` usage?
  (https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466751943)
at the introductory commit, I don't see how `m_ancestors_of_in_package_ancestors` is non-empty. unused?
I suspect replaced by `package_with_ancestors` usage?
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466727264)
`has_mempool_sibling` since I'm thinking in reference to `ptx` and it's a binary result?
  (https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466727264)
`has_mempool_sibling` since I'm thinking in reference to `ptx` and it's a binary result?
🤔 instagibbs reviewed a pull request: "v3 transaction policy for anti-pinning"
(https://github.com/bitcoin/bitcoin/pull/28948#pullrequestreview-1844316858)
`PreCheck`s loop looks so much cleaner, but there's some dead code and like before I'm not super clear which checks are *required* in `AcceptMultipleTransactions` vs anti-DoS type checks. For example, does `ApplyV3Rules` matter inside `AcceptMultipleTransactions`?
I'd like it to be crystal clear to not introduce regressions, and moving forward to cluster mempool we keep all this in mind for future simplification.
  (https://github.com/bitcoin/bitcoin/pull/28948#pullrequestreview-1844316858)
`PreCheck`s loop looks so much cleaner, but there's some dead code and like before I'm not super clear which checks are *required* in `AcceptMultipleTransactions` vs anti-DoS type checks. For example, does `ApplyV3Rules` matter inside `AcceptMultipleTransactions`?
I'd like it to be crystal clear to not introduce regressions, and moving forward to cluster mempool we keep all this in mind for future simplification.
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466746854)
can assert that `package_with_ancestors.ancestor_counts.size() == workspaces.size()`?
  (https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466746854)
can assert that `package_with_ancestors.ancestor_counts.size() == workspaces.size()`?
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466754879)
unused
  (https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466754879)
unused
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466744432)
random newline
  (https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466744432)
random newline
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466754974)
unused
  (https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1466754974)
unused
💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1910755371)
concept ACK, for the record
  (https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1910755371)
concept ACK, for the record
📝 1amhesus opened a pull request: "Add new kernel"
(https://github.com/bitcoin/bitcoin/pull/29322)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
  (https://github.com/bitcoin/bitcoin/pull/29322)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
✅ 1amhesus closed a pull request: "Add new kernel"
(https://github.com/bitcoin/bitcoin/pull/29322)
  (https://github.com/bitcoin/bitcoin/pull/29322)
📝 fanquake locked a pull request: "[temp] Add new kernel"
(https://github.com/bitcoin/bitcoin/pull/29322)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
  (https://github.com/bitcoin/bitcoin/pull/29322)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-1910846023)
I switched my own machine from Docker to Podman. This removes the need for 7cdb75d572e825b4c0cf74a516f912f94807853f so that's dropped.
Added a commit that explains the process a bit more clearly.
  (https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-1910846023)
I switched my own machine from Docker to Podman. This removes the need for 7cdb75d572e825b4c0cf74a516f912f94807853f so that's dropped.
Added a commit that explains the process a bit more clearly.