📝 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
...
(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.
(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)
(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.
(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.
(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
(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
(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
(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.
(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`
(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?
(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?
(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.
(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`
(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?
(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.
(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.
(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?
(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?
(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)
(https://github.com/bitcoin/bitcoin/pull/31740)