Bitcoin Core Github
44 subscribers
120K links
Download Telegram
🚀 fanquake merged a pull request: "build: Bump minimum supported macOS to 13.0"
(https://github.com/bitcoin/bitcoin/pull/31048)
🤔 mzumsande reviewed a pull request: "Halt processing of unrequested transactions v2"
(https://github.com/bitcoin/bitcoin/pull/30572#pullrequestreview-2372049564)
> See the latest state of the code (`23551ef`), which I think have been pushed before your comment publication. There is a `ExpectedTx()` method checking in the `TxRequestTracker` if the transaction has been requested either by wtxid or txid, orphan or not. Currently, Bitcoin Core is doing parent orphan fetching by txid: https://github.com/bitcoin/bitcoin/blob/master/src/net_processing.cpp#L4682
> so there should no parent orphan received as an unsolicited transaction, and not found as `REQUEST
...
💬 nvk commented on issue "Feature Request: Broadcast Pool":
(https://github.com/bitcoin/bitcoin/issues/30471#issuecomment-2416464868)
Would love to see this happen.
💬 maflcko commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#discussion_r1802879192)
clang-format for new code :sweat_smile:
📝 Sjors reopened a pull request: "ci: skip Github CI on branch pushes for forks"
(https://github.com/bitcoin/bitcoin/pull/30487)
When a contributor maintains a fork of the repo, any pull request they make to their own fork, or to the main repository, will trigger two CI runs one for the branch push and one for the pull request.

This was fixed for Cirrus in e9bfbb5414ab14ca14d8edcfdf77f28c9ed67c33, but it relies on setting a custom variable which Github CI lacks.

This PR therefore disables Github CI runs on branch pushes for forks entirely.

After this PR, pushes made to git branches inside fork repositories will n
...
💬 Sjors commented on pull request "ci: skip Github CI on branch pushes for forks":
(https://github.com/bitcoin/bitcoin/pull/30487#issuecomment-2416470424)
Oh wow that is remarkably well hidden! In that case I will change this PR to apply the same behaviour as Cirruss.

<img width="811" alt="SKIP_BRANCH_PUSH=true" src="https://github.com/user-attachments/assets/16995ba5-e37d-4819-8a0b-ef83f830422b">
📝 Sjors converted_to_draft a pull request: "ci: skip Github CI on branch pushes for forks"
(https://github.com/bitcoin/bitcoin/pull/30487)
When a contributor maintains a fork of the repo, any pull request they make to their own fork, or to the main repository, will trigger two CI runs one for the branch push and one for the pull request.

This was fixed for Cirrus in e9bfbb5414ab14ca14d8edcfdf77f28c9ed67c33, but it relies on setting a custom variable which Github CI lacks.

This PR therefore disables Github CI runs on branch pushes for forks entirely.

After this PR, pushes made to git branches inside fork repositories will n
...
💬 stickies-v commented on issue "Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/issues/31017#issuecomment-2416494027)
The "should work without having access to the RPC interface" is the part that I'd like to understand better. For example, the electrs docs specifically mention the bitcoind jsonrpc authentication: https://github.com/romanz/electrs/blob/master/doc/config.md#electrs-configuration
💬 maflcko commented on pull request "validation: Improve input script check error reporting":
(https://github.com/bitcoin/bitcoin/pull/31097#issuecomment-2416498223)
A bit surprising that all tests passed. Maybe a test-case can be modified, or a new test can be added, so that the test changes fail on master and pass on this pull request? Otherwise, this may be broken again in the future. Also, it is harder to review, because each reviewer will have to write the test themselves.
💬 Sjors commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1802901394)
Silent merge conflict with #30937
💬 achow101 commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#discussion_r1802902566)
`HaveKeys` uses `std::vector<valtype>`
💬 achow101 commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#discussion_r1802902988)
Done
💬 achow101 commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#discussion_r1802903174)
Updated the comment.
💬 brunoerg commented on issue "Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/issues/31017#issuecomment-2416516487)
> The "should work without having access to the RPC interface" is the part that I'd like to understand better. For example, the electrs docs specifically mention the bitcoind jsonrpc authentication: https://github.com/romanz/electrs/blob/master/doc/config.md#electrs-configuration

It's not clear to me as well. Maybe they need it to start using the REST interface?
💬 achow101 commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#discussion_r1802908802)
Done
💬 achow101 commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#discussion_r1802908907)
Done
🤔 stickies-v reviewed a pull request: "Package validation: accept packages of size 1"
(https://github.com/bitcoin/bitcoin/pull/31096#pullrequestreview-2372128688)
Approach ACK, code LGTM 72872c7073e99da0828e788b25520dcecf721c1b and change looks safe at first sight but want to take some more time to think about it as I'm not super familiar with this code.
💬 stickies-v commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1802914437)
No need to special-case this:

<details>
<summary>git diff on 72872c7073</summary>

```diff
diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py
index d4da0f9b06..ce42bf810e 100755
--- a/test/functional/rpc_packages.py
+++ b/test/functional/rpc_packages.py
@@ -370,7 +370,7 @@ class RPCPackagesTest(BitcoinTestFramework):
node = self.nodes[0]

self.log.info("Submitpackage valid packages with 1 child and some number of parents")
- fo
...
💬 stickies-v commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1802941507)
Note: this also requires updating this documentation: https://github.com/bitcoin/bitcoin/blob/538ccaed004ff89ec8b2b71d8711feed624ffccc/doc/policy/packages.md?plain=1#L78-L79
💬 stickies-v commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1802916686)
nit: we require exactly one child, so I think using `tx_parent` is unnecessarily confusing
```suggestion
BOOST_CHECK(IsChildWithParents({tx_child}));
BOOST_CHECK(IsChildWithParentsTree({tx_child}));
```