Bitcoin Core Github
44 subscribers
121K links
Download Telegram
📝 espi3 opened a pull request: "doc: Add test coverage troubleshooting tip"
(https://github.com/bitcoin/bitcoin/pull/31755)
This PR adds troubleshooting information to explain how to overcome two errors that may arise when running the tests with coverage as described in doc/developer-notes.md.

Specifically, running the `cmake -P build/Coverage.cmake` step may produces geninfo mismatch errors of the following sort (Ubuntu 24.04, g++ compiler):

```
geninfo: ERROR: mismatched end line for _ZN7Num30723SerI10DataStreamEEvRT_RKS_ at /home/espi3/src/bitcoin/src/crypto/muhash.h:62: 67 -> 62
(use "geninfo --ig
...
💬 Sjors commented on pull request "rpc: fix mintime field testnet4":
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2621017382)
I was indeed waiting for more people to chime in.

Pushed the simplification to always apply the rule.
fanquake closed an issue: "guix SWH vault Internal Server Error"
(https://github.com/bitcoin/bitcoin/issues/31754)
💬 fanquake commented on issue "guix SWH vault Internal Server Error":
(https://github.com/bitcoin/bitcoin/issues/31754#issuecomment-2621122709)
I haven't seen this before, but it looks like a (hopefully intermittent) upstream issue.
💬 TheCharlatan commented on pull request "rpc: have getblocktemplate mintime account for timewarp":
(https://github.com/bitcoin/bitcoin/pull/31600#discussion_r1933573379)
I think there is room for the functional tests to demonstrate some "common misconfigurations" to help avoid regressions, but agree that this case here is probably best documented and described elsewhere.
💬 Sjors commented on pull request "rpc: have getblocktemplate mintime account for timewarp":
(https://github.com/bitcoin/bitcoin/pull/31600#discussion_r1933586381)
It's in https://github.com/bitcoin/bitcoin/pull/31725
💬 Sjors commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1933669516)
01452f8c0da58db549bc46d0cfa7de715344efc4: the Apple version of tar doesn't have `--sort`, see https://github.com/chaincodelabs/libmultiprocess/issues/139#issuecomment-2621080394
👍 TheCharlatan approved a pull request: "depends: Update libmultiprocess library before converting to subtree"
(https://github.com/bitcoin/bitcoin/pull/31740#pullrequestreview-2580718512)
ACK 4e0aa1835b3e980ceda29ec90e7115d7fef53f51
🤔 rkrux reviewed a pull request: "psbt: add non-default sighash types to PSBTs and unify sighash type match checking"
(https://github.com/bitcoin/bitcoin/pull/31622#pullrequestreview-2580366855)
Concept ACK fefe0636d4ae7c246042276cacd60b22f5fc6bb9

Good PR with enough context to unpack for me. Left few comments, will review again soon.
💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1933484246)
`If only the parameter is provided`
💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1933559100)
`std::optional<int> sighash_type = 1 /* SIGHASH_ALL */`
Does it not require `std::nullopt` as the default like in other cases?
💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1933618280)
Confirming - it's being removed from here because this check now happens inside `SignPSBTInput` that is called down below?
💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1933698058)
Can this commit be a separate PR? It would be nice to have a functional test that checks non witness utxos are dropped if sighash type is not `SIGHASH_ANYONECANPAY`. It can be added in this PR as well at the cost of a larger diff.
💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1933483132)
`parameter`
💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1933604449)
`If only the psbt field is provided, refuse to sign.`

How does it refuse to sign in this case?
💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1933734099)
Nit feel free to ignore: I ignored this duplication in the first commit but now that it's being done twice, a refactor could be to just iterate this in a loop of sighash types.
💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1933731594)
Atm, there is only 1 input in the PSBT. It would be nice to have another input that is Taproot and check for its sighash.
💬 rkrux commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1933654773)
Is the downside of adding sighash in PSBT in case of default or all is that it increases PSBT size?
💬 TheCharlatan commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2621465199)
As an alternative to reaching into depends or committing generated files, could it be feasible to set native compilers in the toolchain file instead for compiling the code generators?
🚀 fanquake merged a pull request: "depends: Update libmultiprocess library before converting to subtree"
(https://github.com/bitcoin/bitcoin/pull/31740)