👍 paplorinc approved a pull request: "fix: Make TxidFromString() respect string_view length"
(https://github.com/bitcoin/bitcoin/pull/30436#pullrequestreview-2193359685)
Thanks for fixing this!
ACK bfcc016825d8febf366a6b5eec5ed193c2c313bc

💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1687673170)
nit: if you edit again, consider adding @ryanofsky as co-author to the commit

💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1687693556)
nice!

💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1687739273)
+1

💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1687698438)
I wonder what this does exactly :)

💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1687749691)
+1, verified, fails as expected in previous commit
nit, could probably be simplifed to:
```suggestion
BOOST_CHECK_EQUAL(TxidFromString(std::string_view("ABCD1234", 4)),
uint256S("000000000000000000000000000000000000000000000000000000000000abcd"));
```

💬 josibake commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1687754852)
(just realised I need to update the description of this PR to be a bit more clear on the motivation)

@sipa - my main motivations for having the `KeyPair` class is to a) encapsulate the taptweak logic so that it can be used outside of signing and b) to be able to pass the `KeyPair` directly to `secp256k1_*` functions expecting a `const secp256k1_keypair *`. For the latter, I think we will need some sort of public function. Here is an example of how this used in #28201:

https://github.com/bi
...

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1687759165)
But production code calls it indirectly, so it's not exactly deprecated, just direct access is discouraged, right?

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1687760355)
Makes sense to do the deprecation removal in a separate PR

💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1687761450)
Yes, happy to rename it to `SetHexDiscouraged`, but I don't think it matters much, as the code is deprecated to be called directly anyway.

💬 josibake commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1687763266)
This is how @paplorinc originally proposed the test: https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1685696117

I felt like computing multiple keys in a loop was overkill and removed the merkle root switching since that's not really what's being tested here. The goal here is to sanity check that we get the same public key bytes from GetPubKey() as we would get directly accessing the `secp256k1_keypair` using the `_xonly_pub` and `_serialize`.

So perhaps better to just remove the
...

💬 maflcko commented on pull request "guix: use GCC 13 to builds releases":
(https://github.com/bitcoin/bitcoin/pull/29881#discussion_r1687766168)
I ran the task 10 times, and it failed consistently, so I don't think it is the same bug:

https://cirrus-ci.com/task/4925521652350976

📝 vasild opened a pull request: "doc: add release notes for #22729"
(https://github.com/bitcoin/bitcoin/pull/30502)
Add release notes for #22729.

🚀 fanquake merged a pull request: "fuzz: Speed up PickValue in txorphan"
(https://github.com/bitcoin/bitcoin/pull/30474)

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1687771391)
will leave it up to you, but if I see a `Set.*` I assume it's a property setter (which has an equivalent Getter), not that something is calculated and internal state (maybe) modified accordingly.

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1687765720)
I'm fine with that if you are

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1687764790)
you may want to revert it in the commit that introduced it instead

💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1687776017)
> will leave it up to you, but if I see a `Set.*` I assume it's a property setter (which has an equivalent Getter)

Yes, there is a corresponding `GetHex`.

💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1687777514)
> you may want to revert it in the commit that introduced it instead

It is a scripted-diff, so it is correct and self-consistent, no?

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1687781534)
ah, got it, sure!