Bitcoin Core Github
44 subscribers
121K links
Download Telegram
📝 jordancampa23-ctrl opened a pull request: "Create Jordan Finesse👌"
(https://github.com/bitcoin/bitcoin/pull/33092)
<!--
*** Please remove the following help
[free.pdf](https://github.com/user-attachments/files/21497432/free.pdf)
[free.pdf](https://github.com/user-attachments/files/21497438/free.pdf)
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 ho
...
💬 luke-jr commented on pull request "test: Add functional tests for blockreconstructionextratxn and extra pool (compactblocks)":
(https://github.com/bitcoin/bitcoin/pull/33023#discussion_r2241039496)
Missing a space
💬 bigshiny90 commented on pull request "test: Add functional tests for blockreconstructionextratxn and extra pool (compactblocks)":
(https://github.com/bitcoin/bitcoin/pull/33023#discussion_r2241107374)
fixed. Thanks!
💬 murchandamus commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3134232262)
> > should probably lower all three settings, not just the one. However, this breaks a dozen tests or so, there seem to be some more missing that it should be breaking, and feerate estimation currently doesn’t do below 1 s/vB
>
> @murchandamus I'm busy with this to make some progress while @RobinLinus is away, what's the process - create a new PR as a continuation of this one?

Given that this branch is on Robin’s repository, it would be the easiest to create your own branch and PR (unless
...
🤔 mzumsande reviewed a pull request: "init: [gui] Avoid UB/crash in InitAndLoadChainstate"
(https://github.com/bitcoin/bitcoin/pull/32987#pullrequestreview-3069373388)
Tested ACK fac90e5261b811739ada56e06ea793a12f9c2c3d

Unrelated, but just mentioning that reindex with the GUI is not great: I tried it with a regtest chain of 1000 empty blocks and the app froze for a few seconds / my OS asked me whether it should terminate the non-responding program. Didn't try it with mainnet, but it's probably not a smooth experience.
📝 l0rinc opened a pull request: "refactor: remove unused `ser_writedata16be` and `ser_readdata16be`"
(https://github.com/bitcoin/bitcoin/pull/33093)
Remove dead code from serialization logic - extracted from https://github.com/bitcoin/bitcoin/pull/31868
💬 l0rinc commented on pull request "[IBD] specialize block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#issuecomment-3134384578)
Rebased to solve conflict, the PR is still in draft mode until I remeasure its effect on IBD - and I need to find a nicer way to do the single-byte specializations, since they have a measurable effect but make the code super-ugly... Suggestions are welcome.
In the meantime I have split out the first refactor into https://github.com/bitcoin/bitcoin/pull/33093.
📝 l0rinc opened a pull request: "refactor: unify container presence checks"
(https://github.com/bitcoin/bitcoin/pull/33094)
### Summary
Instead of counting occurrences in sets and maps, the C++20 `::contains` method expresses the intent unambiguously and can return early on first encounter.

### Context
Applied clang‑tidy's [readability‑container‑contains](https://clang.llvm.org/extra/clang-tidy/checks/readability/container-contains.html) check, though many cases required manual changes since tidy couldn't fix them automatically.

### Changes
The changes made here were:

| From | To
...
💬 Christewart commented on pull request "rpc: dumptxoutset height parameter follow-ups (29553)":
(https://github.com/bitcoin/bitcoin/pull/30808#issuecomment-3134394012)
I believe this deserves a release note as it can break existing calls to `dumptxoutset` if you were previously omitting the `type` parameter?

> This parameter can be omitted if a separate \"rollback\" named parameter is specified indicating the height or hash of a specific historical block.
💬 kevkevinpal commented on pull request "qa: test that we do not disconnect a peer for submitting an invalid compact block":
(https://github.com/bitcoin/bitcoin/pull/33083#issuecomment-3134529754)
ACK [c157438](https://github.com/bitcoin/bitcoin/pull/33083/commits/c1574381168573c561ebddf1945d2debefb340f7)

I was able to reproduce the errors with the diffs provided above, and was also able to validate that the master branch tests pass when the diffs are applied as well
💬 ryanofsky commented on pull request "rpc: dumptxoutset height parameter follow-ups (29553)":
(https://github.com/bitcoin/bitcoin/pull/30808#issuecomment-3134531569)
> I believe this deserves a release note as it can break existing calls to `dumptxoutset` if you were previously omitting the `type` parameter?

Yes it would be good to add a release note saying that `dumptxoutset` can no longer be called with just a filename to produce a snapshot with the latest UTXO state. It now needs an additional `type` argument that can be set to "latest" to match previous behavior. It can also take other arguments and is now capable of rolling back the chain and produci
...
💬 maflcko commented on pull request "rpc: dumptxoutset height parameter follow-ups (29553)":
(https://github.com/bitcoin/bitcoin/pull/30808#issuecomment-3134929957)
29.0 is already tagged (archived), built, and released for a long time, so it isn't possible to change the release notes there now.
👋 l0rinc's pull request is ready for review: "refactor: unify container presence checks"
(https://github.com/bitcoin/bitcoin/pull/33094)
💬 rkrux commented on pull request "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key":
(https://github.com/bitcoin/bitcoin/pull/32471#issuecomment-3135054321)
Thanks for addressing the comments, I will take one final look and then a-c-k.

Tagging @furszy for review as he noticed this issue as well few months back.
📝 maflcko opened a pull request: "ci: Remove unused ci/lint_run_all.sh"
(https://github.com/bitcoin/bitcoin/pull/33095)
As per the file content, this can be deleted now. (Stale re-runs are no longer performed for `lint` right now and the CI will be reworked anyway)
maflcko closed a pull request: "ci: Remove unused ci/lint_run_all.sh"
(https://github.com/bitcoin/bitcoin/pull/33095)
💬 maflcko commented on pull request "ci: Remove unused ci/lint_run_all.sh":
(https://github.com/bitcoin/bitcoin/pull/33095#issuecomment-3135184921)
Hmm, let's just do it in the other pull.
👍 maflcko approved a pull request: "Migrate CI to hosted Cirrus Runners"
(https://github.com/bitcoin/bitcoin/pull/32989#pullrequestreview-3070236084)
everything looks good and this is probably ready for final review before merge? I just found some nits, which can be done along with follow-ups like https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2236533948. Also, the merge_base issue mentioned above can be addressed in a follow-up.


However, I am not familiar with the GitHub-style CI, including the (docker) caching, but the pull request motivation claims that many are familiar with this, so it should be easy to find reviewers for i
...
💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2241797039)
nit in 01746e60eb82b9bedb3afa1e791b5abce75ebf75: Unused? This task is using nproc?
💬 maflcko commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2241816383)
nit in 4a1c5b04691266f1441fd966a4be65140f9a5cc4: Outdated doc?