💬 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
...

💬 fanquake commented on pull request "[28.x] Further backports and rc2":
(https://github.com/bitcoin/bitcoin/pull/30827#issuecomment-2349321778)
> Feel free to pick [this](https://github.com/hebasto/bitcoin/commit/be6354db410d162f82b5e3c76d964c0365e51d96) translation update

There's not so much of a rush here that this should bypass the normal PR & review process. This should first be PR'd and merged into master and then backported. Otherwise any broken string are still in master. There is at least one other change waiting for 28.x, so rc2 also needs to either wait for that, or we'll be having an rc3 (which can include the ts update) i
...

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2349326264)
@sdaftuar is that number of iterations per search invocation, or total over the entire linearization?

Could a version of this graph show the total fraction of *transactions* that are optimally linearized? Or maybe just do a chart where a cluster min size of <larger number> is used to ignore the many small clusters we know we can optimally linearize?

💬 hebasto commented on pull request "[28.x] Further backports and rc2":
(https://github.com/bitcoin/bitcoin/pull/30827#issuecomment-2349332544)
> There's not so much of a rush here that this should bypass the normal PR & review process.

Sure.

> This should first be PR'd and merged into master and then backported.

After branching off, the Transifex.com has translation for the version branches only. Fetching 28x translations into the master branch does not look reasonable.

> Otherwise any broken string are still in master.

Maybe this case is a special one.

💬 rkrux commented on issue "28.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/30854#issuecomment-2349335396)
@fjahr I've updated the guide adding instructions for assumeutxo mainnet.

📝 hebasto opened a pull request: "qt: Translations update"
(https://github.com/bitcoin/bitcoin/pull/30899)
The recent translations from Transifex.com fetched with the bitcoin-maintainer-tools/update-translations.py tool.

Fixes https://github.com/bitcoin/bitcoin/issues/30897.

💬 hebasto commented on pull request "[28.x] Further backports and rc2":
(https://github.com/bitcoin/bitcoin/pull/30827#issuecomment-2349337355)
> > There's not so much of a rush here that this should bypass the normal PR & review process.
>
> Sure.

Please see https://github.com/bitcoin/bitcoin/pull/30899.

🤔 jonatack reviewed a pull request: "doc: cmake: prepend "build" to functional/test_runner.py"
(https://github.com/bitcoin/bitcoin/pull/30859#pullrequestreview-2303553463)
Concept ACK. I came across this as well, and the new error message would indeed be helpful. (Bonus points if the message can be defined in one place, didn't check.)

💬 jonatack commented on pull request "doc: cmake: prepend "build" to functional/test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/30859#discussion_r1759154738)
```suggestion
raise Exception(f"config.ini file {self.options.configfile} not found, be sure to run this script from within the build directory")
```

💬 jonatack commented on pull request "doc: cmake: prepend "build" to functional/test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/30859#discussion_r1759154535)
```suggestion
raise Exception(f"config.ini file {configfile} not found, be sure to run this script from within the build directory")
```