Bitcoin Core Github
42 subscribers
124K links
Download Telegram
πŸ’¬ sdaftuar commented on pull request "Cluster mempool followups":
(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.
πŸ’¬ 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()`.
πŸ’¬ sdaftuar commented on pull request "Cluster mempool followups":
(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.
πŸ’¬ 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.)
πŸ’¬ 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.
πŸ’¬ sdaftuar commented on pull request "Cluster mempool followups":
(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
πŸ“ 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.
πŸ’¬ 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
...
βœ… pinheadmz closed a pull request: "doc: fix typo in MAX_MONEY comment"
(https://github.com/bitcoin/bitcoin/pull/33970)
πŸ’¬ pinheadmz commented on pull request "doc: fix typo in MAX_MONEY comment":
(https://github.com/bitcoin/bitcoin/pull/33970#issuecomment-3591960468)
Please don't open new pull requests just for tiny typo fixes, they are a drag on our integration testing infrastructure, maintainer time and reviewer time. See https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#refactoring:

> Trivial pull requests or pull requests that refactor the code with no clear benefits may be immediately closed by the maintainers to reduce unnecessary workload on reviewing.

There are many more significant ways to contribute to Bitcoin -- for example, loo
...
πŸ’¬ 151henry151 commented on pull request "Defer transaction signing until user clicks Send":
(https://github.com/bitcoin-core/gui/pull/915#issuecomment-3591963820)
@achow101 would you like to check this out and let me know if it looks OK?
πŸ’¬ 151henry151 commented on pull request "Check required interfaces before generating manpages":
(https://github.com/bitcoin/bitcoin/pull/33828#issuecomment-3591967154)
> This duplicates #33085. If the author doesn't follow up there shortly, we could reopen here.

It's been a few weeks, would it be appropriate to reopen here? I did try to reach out to @BrandonOdiwuor on the comments of #33085 and haven't heard back from him yet.
πŸ’¬ sdaftuar commented on pull request "Cluster mempool followups":
(https://github.com/bitcoin/bitcoin/pull/33591#issuecomment-3592108964)
I believe I've responded to all the previous review comments, so this should be ready for re-review.

Also fyi, I do have some additional changes to propose beyond what's included here already, but I think I'll stop adding additional scope so that this PR can make forward progress with the improvements already suggested.
πŸ’¬ andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2573348088)
Hmm the thread is just an input fetcher though. That's all it does. I like this name for it.
πŸ’¬ andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2573348285)
Renamed to `m_connect_block_view`.
πŸ’¬ andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >40% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2573348456)
I prefer the current version.
πŸ’¬ wiz commented on pull request "chainparams: remove dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3592429159)
ACK b0c706795ce6a3a00bf068a81ee99fef2ee9bf7e

> A DNS seed operating organization or person is expected to follow good host security practices

I have nothing against Luke personally, but if his server was known to have been compromised and not immediately wiped/rebuilt, he is clearly not following good host security practices... let alone running it for years like that.