💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1686309888)
I understood, that's why I recommended the name change. We don't say `trim` is fragile, just because more values become valid after they're trimmed, we call it lenient, right?

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1686312388)
The reason is that it's not a setter, but a complex method that changes the internal state.

💬 stickies-v commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1686313096)
I don't see the point of bloating the build logs with this, a tracking issue seems more appropriate if we're worried it'll be forgotten.

💬 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_r1686314984)
Do we need to hold `m_tx_download_mutex` for this call: https://github.com/bitcoin/bitcoin/blob/c85accecafc20f6a6ae94bdf6cdd3ba9747218fd/src/net_processing.cpp#L6327-L6328 a few lines down?

💬 darosior commented on pull request "fuzz: Deglobalize signature cache in sigcache test":
(https://github.com/bitcoin/bitcoin/pull/30447#discussion_r1686316001)
nit: this would invalidate existing seeds.

⚠️ Sjors opened an issue: "Spurious markdown linter failure"
(https://github.com/bitcoin/bitcoin/issues/30496)
The linter job failed for https://github.com/bitcoin/bitcoin/pull/30440

```
One or more markdown links are broken.

Relative links are preferred (but not required) as jumping to file works natively within Emacs.

Markdown link errors found:
[Err ] ./pyenv/README.md (12, 1) => /terminal_output.png - Target filename not found.


^---- ⚠️ Failure generated from lint check 'markdown'!
Check that markdown links resolve
```

The PR does not touch any markdown files.

...

💬 maflcko commented on issue "Spurious markdown linter failure":
(https://github.com/bitcoin/bitcoin/issues/30496#issuecomment-2242612557)
I guess the folder is dirty after building python (after the cache got evicted randomly).

So possibly a solution would be to clean the folder after building python.

💬 stickies-v commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#issuecomment-2242614404)
re-ACK 788fe9cc9ab979c5e14f544ae05dc46436892b81

No changes except for commit message update.

📝 maflcko opened a pull request: " rpc: Return precise loadtxoutset error messages "
(https://github.com/bitcoin/bitcoin/pull/30497)
The error messages should never happen in normal operation. However, if
they do, they are helpful to return to the user to debug the issue. For
example, to notice a truncated file.

This fixes https://github.com/bitcoin/bitcoin/issues/28621

Also includes a minor refactor commit.

💬 fanquake commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#discussion_r1686334519)
> My question was about a change that introduced such non-determinism, given that the source code is the same.

The change is CMake. It's passing the full/path/to/the/source/files during compilation, rather than something like src/file which autotools does, which in turn causes the `__FILE__` macros in the zmq error handling code expand to non-deterministic paths.

💬 vasild commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-2242633991)
`ea89a6e368...4533445fd7`: rebase due to conflicts

💬 Sjors commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2242645460)
If you want to simplify this PR a bit further, you can drop `waitFeesChanged`, as well as `getCoinbaseMerklePath()` and `submitSolution` from the `BlockTemplate` interface.

I moved those out of the PRs on the repo and into my own fork (https://github.com/Sjors/bitcoin/pull/52 and https://github.com/Sjors/bitcoin/pull/53). I can apply the required changes to `ipc/capnp/mining.capnp` myself in https://github.com/Sjors/bitcoin/pull/48; that's much easier than I expected.

That should remove an
...

🚀 fanquake merged a pull request: "fuzz: Limit parse_univalue input length"
(https://github.com/bitcoin/bitcoin/pull/30473)

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1686350738)
I don't see why silencing warnings and putting them to dead comments that cannot bite back is better than "bloating the logs"

⚠️ maflcko opened an issue: "fuzz: Apply HasTooManySubFrag (et al) to miniscript_string (et al)"
(https://github.com/bitcoin/bitcoin/issues/30498)
Should the check be applied to miniscript_string, for example the `miniscript_string/ae395bdc087e233d7f8e1844d4814b2c00cc9d21` input, as well?

_Originally posted by @maflcko in https://github.com/bitcoin/bitcoin/issues/30197#issuecomment-2233404249_


Seems useful to at least apply to that fuzz target as well. In theory it could be applied to parse_univalue as well (reverting commit a1b8a917b176ee36961203ccee96457d85102e60).

💬 josibake commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#issuecomment-2242659405)
Update https://github.com/bitcoin/bitcoin/commit/56267cf84281789186035f09898cce3d749748ec -> https://github.com/bitcoin/bitcoin/commit/e998ca2c53bb8320e102a02a06182f8bc49a4f90 ([apply-taptweak-method-05](https://github.com/josibake/bitcoin/tree/apply-taptweak-method-05) -> [apply-taptweak-method-06](https://github.com/josibake/bitcoin/tree/apply-taptweak-method-06) ([compare](https://github.com/josibake/bitcoin/compare/apply-taptweak-method-05..josibake:apply-taptweak-method-06))

* Fix CI fai
...

💬 josibake commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#issuecomment-2242668252)
Rebased on master https://github.com/bitcoin/bitcoin/commit/e998ca2c53bb8320e102a02a06182f8bc49a4f90 -> https://github.com/bitcoin/bitcoin/commit/9afa2c9e50370b2918377f3f3eac0950a4296ec0 ([apply-taptweak-method-06](https://github.com/josibake/bitcoin/tree/apply-taptweak-method-06) -> [apply-taptweak-method-06](https://github.com/josibake/bitcoin/tree/apply-taptweak-method-06-rebase) ([compare](https://github.com/josibake/bitcoin/compare/apply-taptweak-method-05..josibake:apply-taptweak-method-06
...

💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1686322412)
We have many different flavors of hex parsers, to document the differences better, let's add a test where e.g. `TryParseHash` would behave differently, e.g. `"0123456789 ABCDEF"` where `TryParseHex` continues, `SetHexFragile` stops:
```C++
TryParseHex: 0123456789abcdef
SetHexFragile: efcdab8967452301000000000000000000000000000000000000000000000000
```

See: https://github.com/bitcoin/bitcoin/pull/30482/commits/801404719dbdbbc327b2f1bc24cb11b6d52e3f27#r1685500849

🤔 paplorinc requested changes to a pull request: "fix: Make TxidFromString() respect string_view length"
(https://github.com/bitcoin/bitcoin/pull/30436#pullrequestreview-2191194232)
Thanks for tackling these, please my comments, most of them originally posted to https://github.com/bitcoin/bitcoin/pull/30482/files

💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1686329623)
previous version was cleaner in my opinion.
Since str is random access, we could simplify to:
```suggestion
while (digits < trimmed.size() && ::HexDigit(trimmed[digits]) != -1)
++digits;
```


See: https://github.com/bitcoin/bitcoin/pull/30482/commits/f77d961d926405637dfbdfb9a9baea1fab4f1b7b#r1685498257