Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 achow101 commented on pull request "[28.x] Further backports and rc2":
(https://github.com/bitcoin/bitcoin/pull/30827#issuecomment-2349234397)
> @achow101
>
> Could you please add [bitcoin-core/gui#835](https://github.com/bitcoin-core/gui/pull/835) as well?

Done
💬 sdaftuar commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2349244010)
I have been gathering more data to try to measure the effect of the improvements in this PR. I decided to take my cluster mempool branch (#28676), combined with my simulation environment for playing back historical data, and measure the effect of a series of 4 commits from this PR on the number of clusters encountered in 2023 that would NOT have been optimally linearized, for each of 5 different values of the maximum iterations we'd permit when linearizing.

I think plotted these values on
...
💬 mzumsande commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#issuecomment-2349247257)
> Ping also @mzumsande, given you https://github.com/bitcoin/bitcoin/pull/28043#pullrequestreview-1846077000 to this approach last time.

No reservations with the current approach. Concept ACK, I haven't reviewed the actual fuzz target in detail (and don't know if I would get to that in time, considering this has already multiple acks).

Would it make sense to additionally have some belt-and-suspenders check somewhere at the start of bitcoind that `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` i
...
TheCharlatan closed an issue: "cmake: Installed static kernel library is unusable"
(https://github.com/bitcoin/bitcoin/issues/30801)
💬 TheCharlatan commented on issue "cmake: Installed static kernel library is unusable":
(https://github.com/bitcoin/bitcoin/issues/30801#issuecomment-2349255516)
> @TheCharlatan what is the status of this now?

Just tested that this works with the kernel rust crate now for statically linking the library into a rust binary. So I think we are done here.
💬 fanquake commented on pull request "code style: update .editorconfig file":
(https://github.com/bitcoin/bitcoin/pull/30877#discussion_r1759098222)
@theStack did you want to push to take this, given this file is rarely touched?
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1759111178)
@petertodd I know for their slipstream accelerator at a minimum they do not, and perhaps they updated all their nodes to not enforce, I'm not sure.

@sdaftuar Ok that's not much machinery, and seems to make sense combined with base and modified fee both being checked.

I think any node runner who wants to ignore dust rules they'll simply set `-dustrelayfee=0`, which works out of the box as expected in this PR, and even if we're more controlling on when `prioritisetransaction` can be called.
...
💬 theStack commented on pull request "code style: update .editorconfig file":
(https://github.com/bitcoin/bitcoin/pull/30877#discussion_r1759111338)
Yes good point, done.
🚀 fanquake merged a pull request: "doc: unit test runner help fixup and improvements"
(https://github.com/bitcoin/bitcoin/pull/30890)
💬 jonatack commented on pull request "doc: cmake: prepend "build" to functional/test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/30859#discussion_r1759115295)
Maybe begin this sentence with something like

"Assuming the build directory is named `build`, running..."

or

`<build_directory>/test/functional/test_runner.py`
🚀 fanquake merged a pull request: "code style: update .editorconfig file"
(https://github.com/bitcoin/bitcoin/pull/30877)
💬 jonatack commented on pull request "doc: cmake: prepend "build" to functional/test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/30859#discussion_r1759118276)
Not sure if needed, but maybe good to use

`<build_directory>/test/functional/test_runner.py`

or

(if the [test dependencies](/test) are installed and the build directory is named `build`)

---

> This extra line seems like overkill if the proceeding line is being updated to include "build"

Agree
🤔 jonatack reviewed a pull request: "tidy: Use 'starts_with' instead of substr/find"
(https://github.com/bitcoin/bitcoin/pull/30868#pullrequestreview-2303511695)
ACK 22bc9fdca314549bf204d3e3f577d7f014858f42

Maybe name this PR (and the commit, if you need to retouch):

tidy: add clang-tidy `modernize-use-starts-ends-with` check
💬 fjahr commented on pull request "rpc, rest: Improve block rpc error handling, check header before attempting to read block data.":
(https://github.com/bitcoin/bitcoin/pull/30410#issuecomment-2349304886)
re-ACK 6a1aa510e31e8b77793341473aa5afc9d023a6e3
💬 jonatack commented on pull request "tidy: Use 'starts_with' instead of substr/find":
(https://github.com/bitcoin/bitcoin/pull/30868#issuecomment-2349305495)
I'm still adapting to the new build system, but fwiw, locally it looks like no unit tests fail if I flip the changed conditionals to the opposite tests. Functional tests do fail.
💬 rkrux commented on issue "28.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/30854#issuecomment-2349306179)
Thanks @tdb3 for trying the guide out and sharing your feedback. I think I addressed all of it, and good that you raised the last point - the previous transaction details being passed in the sign command would not be apparent to anyone following the guide, mentioning that in the guide is helpful.
💬 achow101 commented on pull request "streams: cache file position within AutoFile":
(https://github.com/bitcoin/bitcoin/pull/30884#issuecomment-2349311621)
ACK e624a9bef16b6335fd119c10698352b59bf2930a

> Extending the `AutoFile` interface so that `AutoFile::Get` is no longer needed can be done as a follow-up.

It does seem a bit fragile to still have `AutoFile::Get`. I would okay with backporting additional commits that remove it if they are not too complicated.
💬 hebasto commented on pull request "[28.x] Further backports and rc2":
(https://github.com/bitcoin/bitcoin/pull/30827#issuecomment-2349312031)
@achow101

Feel free to pick [this](https://github.com/hebasto/bitcoin/commit/be6354db410d162f82b5e3c76d964c0365e51d96) translation update, which resolves https://github.com/bitcoin/bitcoin/issues/30897 and includes new translations made over the nearly 3 weeks since https://github.com/bitcoin/bitcoin/pull/30715.
💬 pablomartin4btc commented on issue "translations: `28.x` update pulled in random strings?":
(https://github.com/bitcoin/bitcoin/issues/30897#issuecomment-2349313438)
In the meantime, is it too ugly to add a regex into `update-translations.py`? (or a function like the existent ones that already parse the strings and validate their format)

```
# regex patterns for malicious content and symbols (just an example)
MALICIOUS_PATTERN = re.compile(r'[\x00-\x1F\x7F-\x9F<>&\'";`\\\xFFFD]|'
r'(\.\./|\.\.\\|\%2e%2e/|\%2e%2e\\|'
r'\$HOME/|\%USERPROFILE\%|\%APPDATA\%|\$USER|\$PATH|'

...
💬 dergoegge commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#issuecomment-2349317870)
>Would it make sense to additionally have some belt-and-suspenders check somewhere at the start of bitcoind that FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION is not set, so it's not only in the build system?

I guess we could (e.g. in `bitcoind.cpp`) do something like:

```c++
#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
static_assert(false, "bitcoind can't be build with FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION");
#endif
```

But I would be surprised if our existing tests do not already
...