Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 vasild commented on pull request "test: extend the SOCKS5 Python proxy to actually connect to a destination":
(https://github.com/bitcoin/bitcoin/pull/29420#issuecomment-2247173779)
`9c50b2fe44...a179b1cbf5`: rebase due to conflicts
🚀 fanquake merged a pull request: "depends: Bump `libmultiprocess` for CMake fixes"
(https://github.com/bitcoin/bitcoin/pull/30513)
⚠️ KonradStaniec opened an issue: "Unexpected behaviour when using `sortedmulti_a` descriptor"
(https://github.com/bitcoin/bitcoin/issues/30518)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Currently running:
```
getdescriptorinfo "tr(50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0,and_v(v:pk(b6df9cc452123b137ae8ad15927ff78b7e4e010a97b8ef6732d6d9d692abd993),multi_a(1,0153089cb23a34755aeba2737cb2134add1708e09dca8ae128ccdb1f035f3197,572706d9cf6b553f179daaf22f456b8e31200f59f51d9dcef936cde61918220e)))"
```
will respond with success for such descriptor:
```

...
💬 dergoegge commented on pull request "refactor: Add FlatFileSeq member variables in BlockManager":
(https://github.com/bitcoin/bitcoin/pull/30517#issuecomment-2247197570)
Concept ACK
💬 dergoegge commented on pull request "m_tx_download_mutex followups":
(https://github.com/bitcoin/bitcoin/pull/30507#discussion_r1689355519)
style nit: Indenting the body of the new scope would be nice
👍 dergoegge approved a pull request: "m_tx_download_mutex followups"
(https://github.com/bitcoin/bitcoin/pull/30507#pullrequestreview-2195972297)
ACK 1a0212c8e5c25fce2e0a2cab3113b25f2fbc38b7
🤔 hodlinator reviewed a pull request: "rest: Reject truncated hex txid early in getutxos parsing"
(https://github.com/bitcoin/bitcoin/pull/30482#pullrequestreview-2195931519)
Concept ACK aaaa66d348f874b8acd3a9f6208dd90dc322b16b
💬 hodlinator commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1689341052)
Not sure whether we should only do `FromHex` testing or do it in addition to `SetHexDeprecated` testing.

Sure, we want to remove `SetHexDeprecated` but it is unclear when or how easily it could be done, do you have an exploratory branch doing the removal?
💬 hodlinator commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1689329336)
nit: Missed opportunity to convert to curly braces? :)

Same below for `COutPoint`.
💬 hodlinator commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1689354816)
Curious: Why introduce this namespace instead of making a protected method in `base_blob` as I did in my exploration (404c115ea0f53785a640818c2058ef0591c8c6ac)?
💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1689361143)
Heh, may do if I retouch for other reasons.
💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1689363198)
It is still tested in this test case (see above), and it other test cases (indirectly). Am I missing something, or is there other missing test coverage?
💬 fanquake commented on pull request "guix: GCC 12 consolidation":
(https://github.com/bitcoin/bitcoin/pull/30511#issuecomment-2247215570)
Guix Build (aarch64, x86_64):
```bash
4fba0aba8be59fd967b099e19faf6ba6770a83de1e7bee745c16f91fa3eb92fe guix-build-d1592d2eee19/output/aarch64-linux-gnu/SHA256SUMS.part
37e93c9235db88f58e5a45e8a54e1d57602f9107cd23676c91252509b2fe3bb5 guix-build-d1592d2eee19/output/aarch64-linux-gnu/bitcoin-d1592d2eee19-aarch64-linux-gnu-debug.tar.gz
184fb61be0843b069c4c0d92119b9e29e895f11ef8687b220ab3898999c06afa guix-build-d1592d2eee19/output/aarch64-linux-gnu/bitcoin-d1592d2eee19-aarch64-linux-gnu.tar.gz
...
🚀 fanquake merged a pull request: "locks: introduce mutex for tx download, flush rejection filters once per tip change"
(https://github.com/bitcoin/bitcoin/pull/30111)
💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1689370440)
I don't think it matters much either way, because it is an internal implementation detail. It doesn't need access to private or protected members, so standalone seems better. Otherwise, there could be confusion when calling `base_blob<160>::FromHex<uint256>(hex)`, as to what values are used. Also, if the function is changed to access the private or protected members, it may be moved to the cpp file, or otherwise "optimized", so spending too much time here now is probably not worth it.

Happy t
...
💬 hodlinator commented on pull request "refactor: Add consteval uint256("str") and ParseHex("str")":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1689373817)
Thanks! That is a piece of art. :)

(Think I would put any `Assert()`s and `vector` initialization inside of `RuntimeParse()`).
🚀 fanquake merged a pull request: "contrib: simplify `test-security-check`"
(https://github.com/bitcoin/bitcoin/pull/30423)
💬 hodlinator commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1689387173)
Hm.. yeah, to me the namespace itself is a cognitive speed-bump but I see your point about keeping to the public members.
💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1689387930)
No, see:

```
bitcoin-tx.cpp:288:23: error: non-constant-expression cannot be narrowed from type 'int64_t' (aka 'long') to 'uint32_t' (aka 'unsigned int') in initializer list [-Wc++11-narrowing]
288 | CTxIn txin{*txid, vout, CScript(), nSequenceIn};
| ^~~~
bitcoin-tx.cpp:288:23: note: insert an explicit cast to silence this issue
288 | CTxIn txin{*txid, vout, CScript(), nSequenceIn};
| ^~~~
|
...
💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#issuecomment-2247260119)
Addressed all feedback.

Can be tested by running the new regression test in `test/functional/interface_rest.py` from this pull on current master to see it fail.
💬 maflcko commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1689395871)
Ok, resolving for now. Happy to change if there is a convincing reason.