Bitcoin Core Github
42 subscribers
126K links
Download Telegram
fanquake closed a pull request: "Update SECURITY.md"
(https://github.com/bitcoin/bitcoin/pull/28915)
📝 fanquake locked a pull request: "Update SECURITY.md"
(https://github.com/bitcoin/bitcoin/pull/28915)
ايلون ماسك رموزات مميزة بملايين البيتكوين ولم تتجاوب معي

<!--
*** 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
signif
...
⚠️ hethm999 opened an issue: "<!--e57a25ab6845829454e8d69fc972939a-->"
(https://github.com/bitcoin/bitcoin/issues/28916)
<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--006a51241073e994b41acfe9ec718e94-->
### Code Coverage
For detailed information about the code coverage, see the [test coverage report](https://corecheck.dev/bitcoin/bitcoin/pulls/28915).
<!--021abf342d371248e50ceaed478a90ca-->
### Reviews
See [the guideline](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#co
...
💬 hethm999 commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1818764628)
> ### هل هناك مشكلة موجودة لهذا؟
> * [x] لقد بحثت في القضايا الموجودة
>
> ### السلوك الحالي
> فشل الأمر `bitcoind -dbcache=100 -par=4 -disablewallet -reindex-chainstate`مع OOM. ورد لأول مرة في [رقم 28718 (تعليق)](https://github.com/bitcoin/bitcoin/issues/28718#issuecomment-1807197107) .
>
> ### سلوك متوقع
> استخدام ذاكرة الوصول العشوائي لا يتجاوز 0.9 جيجابايت.
>
> ### خطوات التكاثر
> إمكانية تكرار نتائج بنسبة 100% باستخدام الثنائيات المجمعة من Guix.
>
> ### إخراج السجل ذي الصلة

...
fanquake closed an issue: "<!--e57a25ab6845829454e8d69fc972939a-->"
(https://github.com/bitcoin/bitcoin/issues/28916)
:lock: fanquake locked an issue: "<!--e57a25ab6845829454e8d69fc972939a-->"
(https://github.com/bitcoin/bitcoin/issues/28916)
💬 martinus commented on pull request "coins: make sure PoolAllocator uses the correct alignment":
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1818769524)
@pinheadmz I don't think there's a need to test any any RPC calls, if sync doesn't go OOM it means it is now working :)
💬 TheCharlatan commented on pull request "lint: Report all lint errors instead of early exit":
(https://github.com/bitcoin/bitcoin/pull/28862#discussion_r1398979031)
It's a bit jarring to see `Err("")`, but I guess there is little point in adding a parser for the script output, or duplicating the failure reporting, so this can just be improved if/when the lint checks are converted to Rust.
💬 maflcko commented on pull request "util: Replace std::filesystem with util/fs.h":
(https://github.com/bitcoin/bitcoin/pull/28076#issuecomment-1818774328)
> relying on CIs

This pull request does not change how much CI is relied upon. Previously, CI was running on all patches, and it is still after this pull request. Previously, it was possible to run any CI task locally, and it is still possible, in the same way. I am running the CI locally regularly and it obviously helps if others do the same. If you have issues running anything locally, it would be good to open an issue, so that it can be fixed. When filing an issue, adding context which doc
...
👍 TheCharlatan approved a pull request: "lint: Report all lint errors instead of early exit"
(https://github.com/bitcoin/bitcoin/pull/28862#pullrequestreview-1739454098)
lgtm ACK fa01f884d3ac128f09bfa57ac2648a19a19d854e

I checked that the CI can now correctly report multiple lint failures.
💬 maflcko commented on pull request "lint: Report all lint errors instead of early exit":
(https://github.com/bitcoin/bitcoin/pull/28862#discussion_r1398984491)
yeah, my primary goal is to improve the experience when running the lint stuff (both locally and on CI). Anything else can be discussed and tackled in other threads, if there is a need and interest.
💬 maflcko commented on pull request "refactor: Make CTxMemPoolEntry non-copyable":
(https://github.com/bitcoin/bitcoin/pull/28903#issuecomment-1818789234)
Needs the title adjusted, according to the commit title?
💬 maflcko commented on pull request "refactor: Make CTxMemPoolEntry only explicitly copyable":
(https://github.com/bitcoin/bitcoin/pull/28903#issuecomment-1818809689)
Would be nice to completely remove the need to create copies, but this involves a lot more code changes. This small fixup makes code and the mentioned code changes easier to review, so further improvements can be done in the future, if there is need and interest.

ACK 705e3f1de00bf30d728addd52a790a139d948e32 🌯

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_fi
...
💬 maflcko commented on pull request "refactor: Make CTxMemPoolEntry only explicitly copyable":
(https://github.com/bitcoin/bitcoin/pull/28903#issuecomment-1818810741)
For reference, the move constructor is already deleted, according to a compiler:

```
./kernel/mempool_entry.h:188:27: note: move constructor of 'CTxMemPoolEntry' is implicitly deleted because field 'm_epoch_marker' has a deleted move constructor
💬 TheCharlatan commented on pull request "refactor: Make CTxMemPoolEntry only explicitly copyable":
(https://github.com/bitcoin/bitcoin/pull/28903#issuecomment-1818847461)
Thanks for the review @maflcko

Re https://github.com/bitcoin/bitcoin/pull/28903#issuecomment-1818810741

> For reference, the move constructor is already deleted, according to a compiler:

Removed the explicit move constructor deletion again.
💬 maflcko commented on pull request "fuzz: add target for `DescriptorScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/28578#issuecomment-1818847784)
I think this can go ahead without having to wait on the unrelated and separately handled pull request to fix a timeout in a function that is called in this target? Seems odd to block this, if it is useful. Better to have some fuzz inputs time out than to have no fuzz coverage?
💬 maflcko commented on pull request "refactor: Make CTxMemPoolEntry only explicitly copyable":
(https://github.com/bitcoin/bitcoin/pull/28903#issuecomment-1818857039)
Ah sorry, didn't mean it as a feedback to change the commit. It seems fine to keep it explicitly deleted, because a move is no better than a copy when either results in a runtime error. Having it deleted also prevents it form accidentally appearing again.
💬 TheCharlatan commented on pull request "refactor: Make CTxMemPoolEntry only explicitly copyable":
(https://github.com/bitcoin/bitcoin/pull/28903#issuecomment-1818861865)
Reverted the last change again.
💬 maflcko commented on pull request "Use serialization parameters for CTransaction":
(https://github.com/bitcoin/bitcoin/pull/28438#issuecomment-1818877296)
> SpanReader

Done in #28912. I guess the `CDataStream` one can be done as part of #https://github.com/bitcoin/bitcoin/pull/28451
💬 maflcko commented on pull request "mempool / rpc: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/28885#discussion_r1399066691)
I don't understand the firs commit message:

```
test: Directly constructing 2 entry map for getprioritisedtransactions

Directly constructing the map so that we wouldn't miss extraneous
entries in future regressions.
```

What does it mean and why is the code changed? Can you give an example or explanation?