💬 glozow commented on pull request "qa: Functional test improvements":
(https://github.com/bitcoin/bitcoin/pull/30463#issuecomment-2242695352)
ACK a8e3af1a82dd584a1cc3ffbe587e66889f72e3c7, tested with the steps in op

🚀 glozow merged a pull request: "qa: Functional test improvements"
(https://github.com/bitcoin/bitcoin/pull/30463)

💬 ismaelsadeeq commented on pull request "Fee Estimation: change `estimatesmartfee` default mode to `economical`":
(https://github.com/bitcoin/bitcoin/pull/30275#issuecomment-2242703454)
> Slightly surprised no tests need to be changed.

Because the fee estimation functional test for `estimatesmartfee` is not deterministic (transaction fee rates are generated randomly), the `check_raw_estimates` function tests for a range, ensuring the returned fee rate is reasonable.

> A boolean flip where the intended behavior isn't followed and we are conservative again?

I added a simple deterministic test which test the default mode is `economical` in 307ceb46587c83a7ab0f587915cdcbf9
...

📝 maflcko opened a pull request: " lint: Use consistent out-of-tree build for python and test_runner "
(https://github.com/bitcoin/bitcoin/pull/30499)
Fixes https://github.com/bitcoin/bitcoin/issues/30496

Seems odd to sometimes do an out-of-tree build (via `./ci/lint_imagefile`, see `test/lint/README.md`) and sometimes not (via Cirrus CI, see `./ci/lint_run_all.sh`).

Fix it by doing an out-of-tree build consistently in the same location.

Also, fix `$CI_RETRY_EXE`, while touching this.

💬 josibake commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#issuecomment-2242742189)
lint CI failure is unrelated and fixed by https://github.com/bitcoin/bitcoin/pull/30499

💬 paplorinc commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#issuecomment-2242746610)
> lint CI failure is unrelated and fixed by #30499

seems that PR is still failing

💬 Sjors commented on pull request "guix: bump time-machine to 1aa8dfaeec3c6e4e587aadf7440246f7c5c04b9f":
(https://github.com/bitcoin/bitcoin/pull/30452#issuecomment-2242754464)
```
fe54323a249ac1096a68176b866d35a455cbb49d940da58dd53782f83599b3c0 guix-build-1653d78cf386/output/aarch64-linux-gnu/SHA256SUMS.part
04663adf3fca66d3987cffaa0937b949b589a5c4be7bbbe19e94018758687596 guix-build-1653d78cf386/output/aarch64-linux-gnu/bitcoin-1653d78cf386-aarch64-linux-gnu-debug.tar.gz
e01e7f8bfe6cd32d78b5fe5ee327bf972f5dbd5242b6c2bf6799234fc3194986 guix-build-1653d78cf386/output/aarch64-linux-gnu/bitcoin-1653d78cf386-aarch64-linux-gnu.tar.gz
993359a7e1a18d7a581882e7891fc47a8
...

💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1686421886)
> because more values become valid

Those values are not "valid". For example, why would turning `aa` into `00aa`, or `aa00` be valid? Both should be rejected and code using it is fragile.

Happy to rename it to `SetHexUnchecked`, but I don't think it matters much, as the code is deprecated anyway.

💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1686423705)
Again, I may fix this in a follow-up. Right now the focus is on the bugfix explained in the pull request description.

👍 theStack approved a pull request: "locks: introduce mutex for tx download, flush rejection filters once per tip change"
(https://github.com/bitcoin/bitcoin/pull/30111#pullrequestreview-2186394084)
Light code-review ACK c85accecafc20f6a6ae94bdf6cdd3ba9747218fd

Fwiw, I've also spun up a fresh mainnet node instance on a publicly reachable machine running this PR for the last few days (out of IBD since Friday morning) and didn't notice any problems so far.

💬 theStack commented on pull request "locks: introduce mutex for tx download, flush rejection filters once per tip change":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1683186792)
nit: as a follow-up, could maybe consolidate those two loops into a single one, as they operate now under the same lock

📝 paplorinc opened a pull request: "Fix lint-spelling warnings"
(https://github.com/bitcoin/bitcoin/pull/30500)
These warnings were often polluting the CI output, e.g. https://github.com/bitcoin/bitcoin/pull/30499/checks?check_run_id=27745036545

> ./test/lint/lint-spelling.py

before the change:
```
doc/design/libraries.md:100: targetted ==> targeted
doc/developer-notes.md:495: dependant ==> dependent
src/bench/sign_transaction.cpp:49: hashIn ==> hashing, hash in
src/bitcoin-chainstate.cpp:213: hashIn ==> hashing, hash in
src/bitcoin-chainstate.cpp:213: hashIn ==> hashing, hash in
src/coins.cp
...

💬 paplorinc commented on pull request "lint: Use consistent out-of-tree build for python and test_runner":
(https://github.com/bitcoin/bitcoin/pull/30499#issuecomment-2242789623)
Thanks for fixing this so quickly!
I've added https://github.com/bitcoin/bitcoin/pull/30500 to fix the unrelated warnings that made it more difficult to see what the underlying problem was here.

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1686444220)
I think it would help a bit, but I'm more concerned about the setter name, when in fact it's a [complex method, changing internal state](https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1686312388), it's why I recommended something like `FromHexUnchecked` (the naming suggestion also applies to the non-deprected method calling this, of course).

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1686444687)
posted to https://github.com/bitcoin/bitcoin/pull/30436, please resolve it here

💬 maflcko commented on pull request "lint: Use consistent out-of-tree build for python and test_runner":
(https://github.com/bitcoin/bitcoin/pull/30499#issuecomment-2242801922)
`lint` is green now, sorry for the force pushes, but this can now be reviewed and tested.

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1686446390)
👍, I follow-up PR makes sense here

💬 hebasto commented on pull request "locks: introduce mutex for tx download, flush rejection filters once per tip change":
(https://github.com/bitcoin/bitcoin/pull/30111#issuecomment-2242819514)
> * Introduce a new `m_tx_download_mutex` which guards the transaction download data structures including `m_txrequest`, the rolling bloom filters, and `m_orphanage`.
>
> * `m_orphanage` doesn't need its own lock anymore

In the master branch, the design of the `TxOrphanage` class is thread-safe for all purposes. I don't think that the fact that `m_orphanage` does not require additional synchronization justifies the change of `TxOrphanage` class design. The new `m_tx_download_
...

💬 hodlinator commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#issuecomment-2242837403)
> Also, you can adjust the pull request description with a motivation and fix. Otherwise, it is just two random links, and reviewers will have a hard time seeing the motivation and gist of the changes.

Done. Thank you for you patience @maflcko.

👍 paplorinc approved a pull request: "lint: Use consistent out-of-tree build for python and test_runner"
(https://github.com/bitcoin/bitcoin/pull/30499#pullrequestreview-2191431156)
utACK fa8d73e86e1c11cdfe8154ab84edc1948283454b