Bitcoin Core Github
44 subscribers
122K links
Download Telegram
🤔 ryanofsky reviewed a pull request: "doc: Clarify comments about endianness after #30526"
(https://github.com/bitcoin/bitcoin/pull/31596#pullrequestreview-2533252948)
Thanks for the reviews. Since there are more followups that could be made here, I created a [branch](https://github.com/ryanofsky/bitcoin/commits/pr/docblob2) to keep track of them. I don't want to create too much churn with documentation PRs so I won't open another one right away, but I can implement any suggestions made here, open a pr if requested, or wait for a related PR to be opened to attach these changes to.
💬 ryanofsky commented on pull request "doc: Clarify comments about endianness after #30526":
(https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1904820053)
re: https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1903895648

> Nit: Call it `hexadecimal` instead of `hex` like done in the `"data"` field for better consistency?

I would probably prefer to go the other way since the term "hex" occurs more frequently than "hexadecimal" in RPC code and documentation (115 vs 11 times). I'm hoping we might be able to improve consistency by moving this information from descriptions of RPC fields to descriptions of RPC types and left another comment
...
💬 ryanofsky commented on pull request "doc: Clarify comments about endianness after #30526":
(https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1904818269)
re: https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1901981925

Thanks, fixed in a followup branch.
💬 ryanofsky commented on pull request "doc: Clarify comments about endianness after #30526":
(https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1904820403)
re: https://github.com/bitcoin/bitcoin/pull/31596#discussion_r1901655760

> Could we unify the rest as well

To follow up, I looked at occurrences of `"txid"`, `"wtxid"`, and `"hash"` (quoted strings) in the RPC code and found dozens of fields that could be updated to be consistent. It seems more challenging than I expected to update all the documentation, and now I'm also wondering if might actually be harmful to mention reversed bytes in some cases but not others, because this might give t
...
💬 starius commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2574305884)
@achow101 I tried the updated version (2da3f0e659d3e89da0cdf525f8ce370bb35365a1).
I tested GUI flow - it works the same (doesn't produce a transaction).
I also re-tested `walletprocesspsbt` flow - now it is also broken:

```
walletprocesspsbt "cHNidP8BAH4CAAAAAfkoG4WU8+OG7ihR9ax1V+NQK6C9ZIEbsNH8qfB/A90YAAAAAAD9////AlcDAAAAAAAAF6kUp6q1daWOXVcRwue0FRtYgEAxvTCHKCMAAAAAAAAiUSBug0N9qhtsEJizov7RUbZtdDokLCvlo5+zNl+ocwJn636BAwAAAQErECcAAAAAAAAiUSBZP7q1V48G5XegaVSU+plRyc0hddLNxEwKjgaxGnEVXwEDBAEAAAA
...
💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2574390155)
> @achow101 I tried the updated version ([2da3f0e](https://github.com/bitcoin/bitcoin/commit/2da3f0e659d3e89da0cdf525f8ce370bb35365a1)). I tested GUI flow - it works the same (doesn't produce a transaction). I also re-tested `walletprocesspsbt` flow - now it is also broken:

Ah, the sighash stuff needs a bit more work, and it also has impacts outside of MuSig support.

For the GUI workflow, if you apply https://github.com/bitcoin-core/gui/pull/850 on top of 3649c2e, it should "work". However
...
💬 vasild commented on pull request "ci: build msan's libc++ with _LIBCPP_ABI_BOUNDED_*":
(https://github.com/bitcoin/bitcoin/pull/31612#issuecomment-2574425798)
@maflcko, good question. I guess the answer is "yes". @ldionne, @var-const is it indeed the case that if libc++ itself is compiled with `_LIBCPP_ABI_BOUNDED_*`, then the user programs that link against such a libc++ must be compiled with `_LIBCPP_ABI_BOUNDED_*` as well?

In this PR we have a libc++ with and a user program without `_LIBCPP_ABI_BOUNDED_*` and it results in:
```
undefined reference to `std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::insert
...
💬 luke-jr commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#issuecomment-2574452128)
Can we make the exists/is_fifo/open atomic somehow? Seems liable to have a race here someday...
💬 Bloodsworth24 commented on issue "Fuzz: Runtime errors when running fuzz tests on MacOs":
(https://github.com/bitcoin/bitcoin/issues/31591#issuecomment-2574469504)
cmake --preset=libfuzzer
💬 luke-jr commented on pull request "Add checkblock RPC and checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31564#issuecomment-2574470589)
The new RPC interface is redundant with the existing BIP 23 block proposals support.

P.S. DATUM does not provide a way for the pool to reject valid blocks. There is no approval of templates.
💬 Eunovo commented on issue "assumevalid is not always applied when reindexing":
(https://github.com/bitcoin/bitcoin/issues/31494#issuecomment-2574487485)
> Thanks, nice test (which should fail on master if I understand it correctly)!

Yes, it does.
💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2574567897)
Rebased after #31581 landed, marking ready for review again.
👋 Sjors's pull request is ready for review: "Add waitNext() to BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/31283)
maflcko closed an issue: "failure in wallet_sendall.py --descriptors"
(https://github.com/bitcoin/bitcoin/issues/31571)
💬 maflcko commented on issue "failure in wallet_sendall.py --descriptors":
(https://github.com/bitcoin/bitcoin/issues/31571#issuecomment-2574650741)
Ok, closing for now
💬 rkrux commented on pull request "BlockAssembler: return selected packages virtual size and fee":
(https://github.com/bitcoin/bitcoin/pull/30391#discussion_r1905067911)
> However, we do not name these vectors vCAmounts, vInts, or vUnsignedChars. Instead, they are named based on some context, such as vTxFees, vTxSigOpsCost, and vchCoinbaseCommitment.
For this reason, I believe naming this vector vFeeFrac does not add much clarity because thats obvious.
Instead, I consider vFeerateHistogram to be the closest meaningful name, based on the context of https://github.com/bitcoin/bitcoin/pull/21422.

I agree with naming variables based on context, this is quite a
...
maflcko closed an issue: "Building `--with-experimental-kernel-lib` fails on OpenBSD 7.2"
(https://github.com/bitcoin/bitcoin/issues/27242)
💬 maflcko commented on issue "Building `--with-experimental-kernel-lib` fails on OpenBSD 7.2":
(https://github.com/bitcoin/bitcoin/issues/27242#issuecomment-2574675548)
Closing for now. A new issue can be opened, if this still happens on current master.
maflcko closed an issue: " (bitcoin core warning: unknown new rules activated versionbit 2) "
(https://github.com/bitcoin/bitcoin/issues/31536)
💬 Sjors commented on pull request "Add checkblock RPC and checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31564#issuecomment-2574685525)
Rebased after https://github.com/bitcoin/bitcoin/pull/31581 landed. I also cherry-picked the latest relevant commits from #31583.

@luke-jr I dropped DATUM from the description, since there's still no spec it's hard to understand what it actually does.

The `checkblock` RPC is indeed almost the same as `getblocktemplate` in `proposal` mode. The main difference is that it can check (reduced) proof-of-work. My long term goal for this new method is to make it more powerful by e.g. adding new tr
...