Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1572364608)
added a small check here using `GetEntry` that takes `Wtxid`s.
💬 fanquake commented on pull request "[WIP] build: remove need to test for endianness":
(https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2066586806)
I don't think that's better. We have to keep all the redundant code, and now we are diveraging from upstream for no reason.
💬 BrandonOdiwuor commented on pull request "test: fix RPC coverage check":
(https://github.com/bitcoin/bitcoin/pull/29387#issuecomment-2066602344)
@maflcko @hernanmarino sorry I misunderstood https://github.com/bitcoin/bitcoin/pull/29387#issuecomment-1956896591 earlier

When the wallet is disabled in the configure, the wallet RPCs are not included in the `rpc_interface.txt` list, hence not part of the coverage check


(example below from my test)
[rpc_interface.txt](https://github.com/bitcoin/bitcoin/files/15041102/rpc_interface.txt)
💬 fanquake commented on pull request "[WIP] build: remove need to test for endianness":
(https://github.com/bitcoin/bitcoin/pull/29852#issuecomment-2066606404)
Also, you're just moving the endianess testing into the code. The point is to drop all of this, and use generic code that doesn't require any tests at all. The fact that MSVC fails to perform basic optimisations is annoying, but I don't really see why it's a blocker here. If we'd just pulled the subtree, and this change came in as part of that, I doubt anyone would have even noticed anything MSVC related (still haven't seen any benchmarks showing any meaningful difference for this change as-is)?
...
💬 pablomartin4btc commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-2066644693)
Updates:
- I've been working on @jonatack's latest feedback, I'm performing some tests at the moment and will update the code soon.
👍 instagibbs approved a pull request: "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)"
(https://github.com/bitcoin/bitcoin/pull/29827#pullrequestreview-2011520053)
ACK 60ca5d55081275a011ccfc9546e0c4a8c4030493

please don't feel obligated to touch due to nit
💬 instagibbs commented on pull request "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)":
(https://github.com/bitcoin/bitcoin/pull/29827#discussion_r1572424009)
micro-nit: this line can be removed safely now :partying_face:
💬 instagibbs commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1572436712)
duplicated `cleanup`, or something?
💬 achow101 commented on issue "DNS seed "seed.bitcoinstats.com" doesn't support filtering while the comments says it does":
(https://github.com/bitcoin/bitcoin/issues/29911#issuecomment-2066674660)
cc @cdecker
💬 achow101 commented on issue "RFC: Formal description of the RPC API":
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2066677901)
Note that automatically generated RPC docs are also available for each major version at https://bitcoincore.org/en/doc/
💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1572451256)
Ah forgot to delete, yes
💬 furszy commented on issue "importdescriptors hanging on importing/updating descriptor with large range":
(https://github.com/bitcoin/bitcoin/issues/25895#issuecomment-2066698109)
> @furszy is this fixed in master?

Locally, the test doesn't take much. Around 100 secs. But I can see how it could take a while on a spinning disk. #28894 should have improved the import situation but we are still far from getting #25297.
Once #28574 gets merged, will continue decoupling #25297 in smaller PRs to improve this scenario.
🤔 mzumsande reviewed a pull request: "test: Fix intermittent issue in p2p_handshake.py"
(https://github.com/bitcoin/bitcoin/pull/29898#pullrequestreview-2011586560)
Tested ACK 6b02c11d667adff24daf611f9b14815d27963674
💬 maflcko commented on issue "RFC: Formal description of the RPC API":
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2066720293)
> https://bitcoincore.org/en/doc/

Yes, I am aware. They are based on the `help` RPC reply. However, they are not machine readable, at least not in a standard format, so I don't think it helps to solve the issues highlighted in the original post.
💬 sipa commented on issue "RFC: Formal description of the RPC API":
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2066724167)
I vaguely recall an issue or discussion around machine-readable APIs in the past, but I cannot find it. I believe @laanwj commented on it.
💬 maflcko commented on issue "RFC: Formal description of the RPC API":
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2066731848)
I can only recall the gRPC (https://github.com/bitcoin/bitcoin/issues/16719) and CBOR (https://github.com/bitcoin/bitcoin/issues/22866) discussions, but those are serialization related, not about a standardized machine-readable interface documentation and specification.
💬 maflcko commented on pull request "guix: remove bzip2 from deps":
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2066751093)
The current set of the changes look fine. However, to actually review all 10 changed packages, it would be nice to have a script that prints whether the tarball has any differences from the upstream source control it claims to come from.
💬 instagibbs commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1572504707)
duplicate log?
⚠️ maflcko opened an issue: "RFC: In guix compile the GUI sequentially from everything else?"
(https://github.com/bitcoin/bitcoin/issues/29914)
Compiling the GUI pulls in quite a few dependencies, which could theoretically include backdoors that are leaked into bitcoind (or other non-GUI utils) as well.

A possible mitigation would be to compile the GUI in a separate guix container from the rest of the binaries. The downside would be that the node library, and the `depends` dependencies of the node library would have to be compiled twice, but the overhead may be worth it?

(I won't be working on this, but I wanted to keep track of t
...
💬 fanquake commented on pull request "guix: remove bzip2 from deps":
(https://github.com/bitcoin/bitcoin/pull/29895#issuecomment-2066790159)
Changed to drop the xz changes, and just switch tar.bzip2 tarballs to using tar.gz (same as release tarballs), so we can drop bzip2 as a Guix dep.

> it would be nice to have a script that prints whether the tarball has any differences from the upstream source control it claims to come from.

Sure, we can have a script. Note that the current tarballs will already differ in that sense.