Bitcoin Core Github
44 subscribers
121K links
Download Telegram
🤔 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}));
```
💬 achow101 commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r1802989706)
Done
💬 achow101 commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r1802989927)
Done
💬 achow101 commented on pull request "Bump python minimum supported version to 3.10":
(https://github.com/bitcoin/bitcoin/pull/30527#issuecomment-2416676868)
ACK fa1b139d17d04cb23bdfb1dd9c2abcdad4bdcd27
💬 darosior commented on pull request "validation: Improve input script check error reporting":
(https://github.com/bitcoin/bitcoin/pull/31097#issuecomment-2416686539)
Here is a functional test which creates a transaction using an `OP_CODESEPARATOR` in its `scriptSig` (non-standard) but also uses an `OP_RETURN` right after, which makes it invalid by consensus. It returns the invalid combination of error strings fixed in this PR ("mandatory-script-verify-flag-failed" because it's a consensus failure, but "Using OP_CODESEPARATOR in non-witness script" because it returns the error string for the first check).

You should be able to trivially adapt it to the fix
...
Sjors closed a pull request: "CKey: add Serialize and Unserialize"
(https://github.com/bitcoin/bitcoin/pull/29295)
🚀 achow101 merged a pull request: "Bump python minimum supported version to 3.10"
(https://github.com/bitcoin/bitcoin/pull/30527)
💬 dergoegge commented on pull request "validation: Improve input script check error reporting":
(https://github.com/bitcoin/bitcoin/pull/31097#issuecomment-2416720400)
@darosior added the test to this PR, thanks!
💬 achow101 commented on pull request "optimization: reserve memory allocation for transaction inputs/outputs":
(https://github.com/bitcoin/bitcoin/pull/30093#issuecomment-2416725377)
ACK ec585f11c38bbe277a596dcea3c813e7c6626050
💬 maflcko commented on pull request "build: Bump minimum supported macOS to 13.0":
(https://github.com/bitcoin/bitcoin/pull/31048#discussion_r1803029870)
Shouldn't this be removed? C.f. 22574046c90c0662f3aa9b1baea074aff54f92a9 ?
⚠️ Sjors opened an issue: "Stratum v2 via IPC Mining Interface tracking issue"
(https://github.com/bitcoin/bitcoin/issues/31098)
After much discussion in https://github.com/bitcoin/bitcoin/pull/29432 it appears there's currently little support for directly supporting Stratum v2 in Bitcoin Core, with most contributors favouring the creation of a Mining IPC interface that external applications can use.

The only such application currently is the Template Provider implemented by me in https://github.com/Sjors/bitcoin/pull/48. This also serves to illustrate how the interface is used and that it's complete. The folks working
...
Sjors closed a pull request: "Stratum v2 Template Provider (take 3)"
(https://github.com/bitcoin/bitcoin/pull/29432)
💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2416743789)
I'm moving this pull request to https://github.com/Sjors/bitcoin/pull/68. The `sv2` branch will continue to work as before for the time being. See https://github.com/bitcoin/bitcoin/issues/31098 the plan moving forward.
💬 kevkevinpal commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#discussion_r1803048632)
thanks! Updated in [60dbd42](https://github.com/bitcoin/bitcoin/pull/31040/commits/60dbd42257c0605ab8d83f47babe2baa8f42a71c)
💬 kevkevinpal commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#discussion_r1803050849)
Thanks! I updated in [60dbd42](https://github.com/bitcoin/bitcoin/pull/31040/commits/60dbd42257c0605ab8d83f47babe2baa8f42a71c)

Its supposed to be a random orphan but I removed "and that the first orphan has been dropped" since we are not checking if a random orphan has been dropped and just checking the max orphan amount
💬 Sjors commented on pull request "build: Bump minimum supported macOS to 13.0":
(https://github.com/bitcoin/bitcoin/pull/31048#discussion_r1803058288)
Yes that should be removed, along with the rest of the paragraph below.
💬 fjahr commented on pull request "rpc: net: follow-ups for #30062":
(https://github.com/bitcoin/bitcoin/pull/30183#discussion_r1803064149)
nit: I am not super happy with the wording that the help text here is converged to but we can keep it since I think everyone who understands asmap and BGP knows what is meant by this. The part I don't like is "in the BGP route to the peer" because a BGP route in a routing table includes several ASNs (the actual route), just the last one is the ASN which controls the IP and that is the mapping that we have in ASMap. Better wordings would be "at the end of the BGP route" or (closer to the old text
...
🤔 fjahr reviewed a pull request: "rpc: net: follow-ups for #30062"
(https://github.com/bitcoin/bitcoin/pull/30183#pullrequestreview-2372405981)
utACK b33eb137e39c434a7be69e1453a708b0c52553c4