💬 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

💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1686320395)
I see that `BOOST_CHECK` was use throughout, but we could use `BOOST_CHECK_EQUAL` in a few places for better error messages - at least for the new code:
```suggestion
BOOST_CHECK_EQUAL(uint256S("\t \n \n \f\n\r\t\v\t 0x"+R1L.ToString()+" \t \n \n \f\n\r\t\v\t "), R1L);
```

We could do the old ones in a different PR, but let's not introduce already deprecated code.

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

💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1686328446)
we could keep the const here as well (and simplify a few other structures, please see my other comments for details):
```C++
template <unsigned int BITS>
void base_blob<BITS>::SetHex(const std::string_view str)
{
const auto trimmed = util::RemovePrefixView(util::TrimStringView(str), "0x");
size_t digits = 0;
while (digits < trimmed.size() && ::HexDigit(trimmed[digits]) != -1)
++digits;
for (auto it = m_data.begin(); it < m_data.end(); ++it) {
auto hi = (
...

💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1686339331)
we should be able to keep the const in most places:
```suggestion
inline uint160 uint160S(const std::string_view str)
```

💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1686345800)
We shouldn't need zeroing, if we're processing the values exhaustively - but if we do, we could use
```suggestion
SetNull();
```

See: https://github.com/bitcoin/bitcoin/pull/30482/files#r1685396384