💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1686332889)
I see this uses little-endian encoding, while e.g. `TryParseHex` uses big-endian, right?
Do you happen to know what the reason is? Should we maybe document this?
Do we need to keep the endianness in every scenario (since that's why we need the digit counting at the beginning) or could we use big-endian in any of the usages, e.g. if these hex strings aren't persisted or someting (in a different PR, of course)?

See: https://github.com/bitcoin/bitcoin/pull/30482/commits/f77d961d926405637dfbdfb
...

💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1686364804)
we could avoid double assignment (and casting) by using a temp variable for the high nibble as such:
```C++
for (auto it = m_data.begin(); it < m_data.end(); ++it) {
auto hi = (digits > 0) ? ::HexDigit(trimmed[--digits]) : 0;
*it = (digits > 0) ? hi | (::HexDigit(trimmed[--digits]) << 4) : hi;
}
```

💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1686365416)
we should be able to use `m_data.begin()` and `m_data.end()` instead

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

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

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

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

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

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1686371093)
posted to https://github.com/bitcoin/bitcoin/pull/30436, please resolve it 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#discussion_r1686372826)
As far as I understand, `AssertLockNotHeld(m_mempool.cs);` is an attempt to force the mutex hierarchy mentioned in here: https://github.com/bitcoin/bitcoin/blob/c85accecafc20f6a6ae94bdf6cdd3ba9747218fd/src/net_processing.cpp#L784

As `m_mempool.cs` is not used in this function, I suggest to add a comment to prevent tempting of removing this line on the "clean up" purpose in the future.

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