π¬ romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2572915212)
Done in [f50c0d9a3e..f33eafff0f](https://github.com/bitcoin/bitcoin/compare/f50c0d9a3ec3b830de79d64082e9f0ab4a9fc4d2..f33eafff0f3c2698ee69ccef800bbc70a4eeae86) (by allowing the caller to pass raw string `query_params`).
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2572915212)
Done in [f50c0d9a3e..f33eafff0f](https://github.com/bitcoin/bitcoin/compare/f50c0d9a3ec3b830de79d64082e9f0ab4a9fc4d2..f33eafff0f3c2698ee69ccef800bbc70a4eeae86) (by allowing the caller to pass raw string `query_params`).
π¬ sedited commented on pull request "p2p: avoid traversing blocks (twice) during IBD":
(https://github.com/bitcoin/bitcoin/pull/32730#issuecomment-3591511638)
This should be re-opened. At the least the tx orphanage should not require work when it is empty.
(https://github.com/bitcoin/bitcoin/pull/32730#issuecomment-3591511638)
This should be re-opened. At the least the tx orphanage should not require work when it is empty.
π¬ romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#issuecomment-3591536574)
Many thanks for the review!
Pushed [`f50c0d9a3e..f33eafff0f`](https://github.com/bitcoin/bitcoin/compare/f50c0d9a3ec3b830de79d64082e9f0ab4a9fc4d2..f33eafff0f3c2698ee69ccef800bbc70a4eeae86).
(https://github.com/bitcoin/bitcoin/pull/33657#issuecomment-3591536574)
Many thanks for the review!
Pushed [`f50c0d9a3e..f33eafff0f`](https://github.com/bitcoin/bitcoin/compare/f50c0d9a3ec3b830de79d64082e9f0ab4a9fc4d2..f33eafff0f3c2698ee69ccef800bbc70a4eeae86).
π fanquake reopened a pull request: "p2p: avoid traversing blocks (twice) during IBD"
(https://github.com/bitcoin/bitcoin/pull/32730)
Context:
Every time a block is connected, a `BlockConnected()` event is appended to the validation interface queue. This queue is consumed sequentially by a single worker thread. To avoid excessive memory usage, the queue is limited to 10 events at a time, and we stop processing new blocks until the queued events are handled.
Issue:
Within the `PeerManager::BlockConnected()` listener, we traverse the block twice inside the transaction download manager β despite not needing to handle orphans
...
(https://github.com/bitcoin/bitcoin/pull/32730)
Context:
Every time a block is connected, a `BlockConnected()` event is appended to the validation interface queue. This queue is consumed sequentially by a single worker thread. To avoid excessive memory usage, the queue is limited to 10 events at a time, and we stop processing new blocks until the queued events are handled.
Issue:
Within the `PeerManager::BlockConnected()` listener, we traverse the block twice inside the transaction download manager β despite not needing to handle orphans
...
π¬ hebasto commented on issue "build: use UCRT runtime for Windows (release) binaries":
(https://github.com/bitcoin/bitcoin/issues/30210#issuecomment-3591608313)
> - [ ] Add CI to cover new runtime.
Since bitcoin/bitcoin#33764 has been merged, this checkbox can now be ticked.
(https://github.com/bitcoin/bitcoin/issues/30210#issuecomment-3591608313)
> - [ ] Add CI to cover new runtime.
Since bitcoin/bitcoin#33764 has been merged, this checkbox can now be ticked.
π JeremyRubin opened a pull request: "[CI] remove `doc/release-notes.md` from lint-spelling.py"
(https://github.com/bitcoin/bitcoin/pull/33968)
`/doc/release-notes/` is ignored, but during release branch-off, the latest release is copied to `doc/release-notes.md`. This means that CI runs based off of the branch off point (e.g., for backport dev) will fail CI if there are spelling errors, or if there are e.g. contributors names that resemble spelling errors (not a hypothetical example).
(https://github.com/bitcoin/bitcoin/pull/33968)
`/doc/release-notes/` is ignored, but during release branch-off, the latest release is copied to `doc/release-notes.md`. This means that CI runs based off of the branch off point (e.g., for backport dev) will fail CI if there are spelling errors, or if there are e.g. contributors names that resemble spelling errors (not a hypothetical example).
π¬ plebhash commented on pull request "[30.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33609#issuecomment-3591792110)
reporting here some testing methodology and results
---
originally I reported https://github.com/bitcoin/bitcoin/issues/33554, which I can still reliably reproduce with branch `2025-10-06-abort-proxy-io` of https://github.com/plebhash/sv2-bitcoin-core against Bitcoin Core:
- `v30.0rc3`
- `v30.0`
basically, the rust code runs for a little bit, and then Bitcoin Core crashes with:
```
Assertion failed: (m_loop), function operator->, file proxy.h, line 60.
[1] 8022 abort ./buil
...
(https://github.com/bitcoin/bitcoin/pull/33609#issuecomment-3591792110)
reporting here some testing methodology and results
---
originally I reported https://github.com/bitcoin/bitcoin/issues/33554, which I can still reliably reproduce with branch `2025-10-06-abort-proxy-io` of https://github.com/plebhash/sv2-bitcoin-core against Bitcoin Core:
- `v30.0rc3`
- `v30.0`
basically, the rust code runs for a little bit, and then Bitcoin Core crashes with:
```
Assertion failed: (m_loop), function operator->, file proxy.h, line 60.
[1] 8022 abort ./buil
...
π th30neee opened a pull request: "Update ci.yml"
(https://github.com/bitcoin/bitcoin/pull/33969)
<!--
*** 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/33969)
<!--
*** 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
...
β
hebasto closed a pull request: "Update ci.yml"
(https://github.com/bitcoin/bitcoin/pull/33969)
(https://github.com/bitcoin/bitcoin/pull/33969)
π¬ sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573131768)
Made private in 6797c71ef9323efd2db9dc07d6c1156d077b38cc
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573131768)
Made private in 6797c71ef9323efd2db9dc07d6c1156d077b38cc
π¬ sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573131909)
Done.
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573131909)
Done.
π¬ sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573132334)
This is now re-written to not use epochs; I also added a test that would have caught the previous bug (when the call to `visited()` came before the call to `check_final_an_mature()`.
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573132334)
This is now re-written to not use epochs; I also added a test that would have caught the previous bug (when the call to `visited()` came before the call to `check_final_an_mature()`.
π¬ sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573132624)
Fixed in 6129b8ecb77aafdc4d80ffdc3e85eb05889917f7
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573132624)
Fixed in 6129b8ecb77aafdc4d80ffdc3e85eb05889917f7
π¬ sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573133141)
I ended up reworking the docs and merged the mempool-limits.md file (which was quite short and included a few definitions) in with the new mempool-design.md file (that also has a bunch of definitions and a brief overview of how cluster mempool works).
I took your language suggestion and incorporated in 9d25e2cbcd011b3046f9c3cd8de4e1b9112d0dfd.
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573133141)
I ended up reworking the docs and merged the mempool-limits.md file (which was quite short and included a few definitions) in with the new mempool-design.md file (that also has a bunch of definitions and a brief overview of how cluster mempool works).
I took your language suggestion and incorporated in 9d25e2cbcd011b3046f9c3cd8de4e1b9112d0dfd.
π¬ sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573134065)
I think it was just a single bug, a missing line in the `if` block:
```
current_chunk_feerate = pool.GetMainChunkFeerate(*tx);
```
Please let me know if you spot anything else wrong. Your test passes with that fix, and I incorporated your code in that same commit. (Also, I deleted the "FIXME" comment you left in your python test, and added an additional test that 2nd and 3rd chunks get merged after a `prioritisetransaction` call.)
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573134065)
I think it was just a single bug, a missing line in the `if` block:
```
current_chunk_feerate = pool.GetMainChunkFeerate(*tx);
```
Please let me know if you spot anything else wrong. Your test passes with that fix, and I incorporated your code in that same commit. (Also, I deleted the "FIXME" comment you left in your python test, and added an additional test that 2nd and 3rd chunks get merged after a `prioritisetransaction` call.)
π¬ sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573134658)
Please take a look at the new `mempool-design.md` docs and let me know what you think -- in the course of many edits, I incorporated your suggestion with some modifications.
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573134658)
Please take a look at the new `mempool-design.md` docs and let me know what you think -- in the course of many edits, I incorporated your suggestion with some modifications.
π¬ sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573135145)
Made an attempt at this in 9d25e2cbcd011b3046f9c3cd8de4e1b9112d0dfd
(https://github.com/bitcoin/bitcoin/pull/33591#discussion_r2573135145)
Made an attempt at this in 9d25e2cbcd011b3046f9c3cd8de4e1b9112d0dfd
π¬ pinheadmz commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#issuecomment-3591868282)
push to 8106ac5d0f71fbd6445810f0f6388b36a7191b72:
- multiple nits and suggestions taken from corecheck / sonarcloud
(https://github.com/bitcoin/bitcoin/pull/32061#issuecomment-3591868282)
push to 8106ac5d0f71fbd6445810f0f6388b36a7191b72:
- multiple nits and suggestions taken from corecheck / sonarcloud
π sunmoonron opened a pull request: "doc: fix typo in MAX_MONEY comment"
(https://github.com/bitcoin/bitcoin/pull/33970)
This PR fixes a small typo for grammar in the MAX_MONEY comment in src/consensus/amount.h.
* Replace βa(nother)β with βanotherβ.
No functional changes; comment-only.
(https://github.com/bitcoin/bitcoin/pull/33970)
This PR fixes a small typo for grammar in the MAX_MONEY comment in src/consensus/amount.h.
* Replace βa(nother)β with βanotherβ.
No functional changes; comment-only.
π¬ 151henry151 commented on pull request "Align legacy script policy with P2SH policy in AreInputsStandard":
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3591953100)
@maflcko I think I've resolved the issue.
Witness scripts (P2WSH, P2WPKH, etc.) were incorrectly falling into the legacy script path, so they're now explicitly skipped since they're handled by `IsWitnessStandard`. Anyone-can-spend scripts like `CScript([OP_TRUE])` were being rejected as NONSTANDARD, so they're now explicitly allowed. NONSTANDARD scripts also needed sigop limit checking applied to match P2SH behavior.
All tests now pass locally: `p2p_segwit.py`, `feature_taproot.py`, `scrip
...
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3591953100)
@maflcko I think I've resolved the issue.
Witness scripts (P2WSH, P2WPKH, etc.) were incorrectly falling into the legacy script path, so they're now explicitly skipped since they're handled by `IsWitnessStandard`. Anyone-can-spend scripts like `CScript([OP_TRUE])` were being rejected as NONSTANDARD, so they're now explicitly allowed. NONSTANDARD scripts also needed sigop limit checking applied to match P2SH behavior.
All tests now pass locally: `p2p_segwit.py`, `feature_taproot.py`, `scrip
...