📝 hebasto opened a pull request: "build: Introduce CMake-based buid system"
(https://github.com/bitcoin/bitcoin/pull/30454)
This PR introduces a new CMake-based build system, which is a drop-in replacement for the current Autotools-based build system.
As discussed during the recent CoreDev meetup in April, the switch from Autotools to CMake is intended to happen as soon as possible after branching 28.x off, which means that 29.0 will be built using CMake.
This PR branch is essentially the [staging branch](https://github.com/hebasto/bitcoin/tree/cmake-staging), with every change reviewed and tested by a group of
...
(https://github.com/bitcoin/bitcoin/pull/30454)
This PR introduces a new CMake-based build system, which is a drop-in replacement for the current Autotools-based build system.
As discussed during the recent CoreDev meetup in April, the switch from Autotools to CMake is intended to happen as soon as possible after branching 28.x off, which means that 29.0 will be built using CMake.
This PR branch is essentially the [staging branch](https://github.com/hebasto/bitcoin/tree/cmake-staging), with every change reviewed and tested by a group of
...
✅ hebasto closed a pull request: "[DO NOT MERGE] cmake: Migrate CI scripts to CMake-based build system -- WIP"
(https://github.com/bitcoin/bitcoin/pull/29790)
(https://github.com/bitcoin/bitcoin/pull/29790)
💬 hebasto commented on pull request "[DO NOT MERGE] cmake: Migrate CI scripts to CMake-based build system -- WIP":
(https://github.com/bitcoin/bitcoin/pull/29790#issuecomment-2231130195)
Closing. See https://github.com/bitcoin/bitcoin/pull/30454.
(https://github.com/bitcoin/bitcoin/pull/29790#issuecomment-2231130195)
Closing. See https://github.com/bitcoin/bitcoin/pull/30454.
💬 maflcko commented on pull request "build: Introduce CMake-based buid system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2231166194)
> The only differences from the staging branch are:
I'd say the section can be removed (or moved to a separate comment), because when this will be merged, I presume many more differences will accumulate. Even looking at the outstanding ports (https://github.com/bitcoin/bitcoin/issues?q=label%3A%22Needs+CMake+port%22+is%3Aclosed) right now, there are some.
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2231166194)
> The only differences from the staging branch are:
I'd say the section can be removed (or moved to a separate comment), because when this will be merged, I presume many more differences will accumulate. Even looking at the outstanding ports (https://github.com/bitcoin/bitcoin/issues?q=label%3A%22Needs+CMake+port%22+is%3Aclosed) right now, there are some.
💬 theStack commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2231180251)
> Getting a mismatch again. This time for block 716120 (00000000000000000009bc9db59a266e367f54fdd4cda7b5ddb31f6ae9723083).
Is there some easy way to find out the exact txs for which the tweak mismatches happen? (`getsilentpaymentblockdata` doesn't seem to return txids...) Would interesting to see if there is anything special/different in those (e.g. uncommon spending types).
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2231180251)
> Getting a mismatch again. This time for block 716120 (00000000000000000009bc9db59a266e367f54fdd4cda7b5ddb31f6ae9723083).
Is there some easy way to find out the exact txs for which the tweak mismatches happen? (`getsilentpaymentblockdata` doesn't seem to return txids...) Would interesting to see if there is anything special/different in those (e.g. uncommon spending types).
💬 maflcko commented on issue "Fuzzing related configuration/build options can be improved":
(https://github.com/bitcoin/bitcoin/issues/30318#issuecomment-2231182809)
However, it would be good, to implement this before https://github.com/bitcoin/bitcoin/pull/30454 is merged. Otherwise there will be two breaking changes for each fuzz infra deployment.
(https://github.com/bitcoin/bitcoin/issues/30318#issuecomment-2231182809)
However, it would be good, to implement this before https://github.com/bitcoin/bitcoin/pull/30454 is merged. Otherwise there will be two breaking changes for each fuzz infra deployment.
💬 hebasto commented on issue "Fuzzing related configuration/build options can be improved":
(https://github.com/bitcoin/bitcoin/issues/30318#issuecomment-2231190195)
> > * `--enable-fuzz-binary=yes|no|exclusive`, but renaming it would break every fuzzing script. Maybe the transition to CMake is a good time to improve this since scripts would have to be adjusted anyway.
>
> Seems fine to do, if `-DBUILD_FUZZ_BINARY=ON/OFF/EXCL...` is easy to implement.
It's not a problem at all.
(https://github.com/bitcoin/bitcoin/issues/30318#issuecomment-2231190195)
> > * `--enable-fuzz-binary=yes|no|exclusive`, but renaming it would break every fuzzing script. Maybe the transition to CMake is a good time to improve this since scripts would have to be adjusted anyway.
>
> Seems fine to do, if `-DBUILD_FUZZ_BINARY=ON/OFF/EXCL...` is easy to implement.
It's not a problem at all.
💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2231192536)
Recompiling BlindBit had no effect. Pushed some fixes for various CI failures, but that also doesn't impact the above mismatch.
@theStack that would be useful. Our index doesn't store it. We could add an optional `list_txid` option to the RPC that reprocesses the block. The RPC result would then be a map of txid -> tweak.
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2231192536)
Recompiling BlindBit had no effect. Pushed some fixes for various CI failures, but that also doesn't impact the above mismatch.
@theStack that would be useful. Our index doesn't store it. We could add an optional `list_txid` option to the RPC that reprocesses the block. The RPC result would then be a map of txid -> tweak.
💬 maflcko commented on pull request "build: Introduce CMake-based buid system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2231205319)
> Please refer to the build options parity table.
Missing link/reference?
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2231205319)
> Please refer to the build options parity table.
Missing link/reference?
💬 hebasto commented on pull request "build: Introduce CMake-based buid system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2231220385)
> > Please refer to the build options parity table.
>
> Missing link/reference?
Thanks! Added.
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2231220385)
> > Please refer to the build options parity table.
>
> Missing link/reference?
Thanks! Added.
✅ brunoerg closed a pull request: "wallet, rpc: add BIP44 `account` in `createwallet`"
(https://github.com/bitcoin/bitcoin/pull/29129)
(https://github.com/bitcoin/bitcoin/pull/29129)
💬 brunoerg commented on pull request "wallet, rpc: add BIP44 `account` in `createwallet`":
(https://github.com/bitcoin/bitcoin/pull/29129#issuecomment-2231224581)
Closing for now.
(https://github.com/bitcoin/bitcoin/pull/29129#issuecomment-2231224581)
Closing for now.
💬 achow101 commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1679633885)
Added a help text.
(https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1679633885)
Added a help text.
🤔 ismaelsadeeq reviewed a pull request: "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending"
(https://github.com/bitcoin/bitcoin/pull/30352#pullrequestreview-2180579805)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30352#pullrequestreview-2180579805)
Concept ACK
💬 ismaelsadeeq commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1679609544)
whats the `0x02`?
P2Anchor is `OP_TRUE <0x4e73>`
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1679609544)
whats the `0x02`?
P2Anchor is `OP_TRUE <0x4e73>`
💬 ismaelsadeeq commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1679615452)
```suggestion
ANCHOR, //!< anyone can spend script
```
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1679615452)
```suggestion
ANCHOR, //!< anyone can spend script
```
💬 ismaelsadeeq commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1679638523)
A test for this will be nice!
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1679638523)
A test for this will be nice!
💬 ismaelsadeeq commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1679627045)
In 7013da222720fde5ea1c76bba99b1c91ae18558f "policy: Add OP_TRUE <0x4e73> as a standard output type"
Add `ANCHOR` txout script type description in the `CTxDestination` docstring.
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1679627045)
In 7013da222720fde5ea1c76bba99b1c91ae18558f "policy: Add OP_TRUE <0x4e73> as a standard output type"
Add `ANCHOR` txout script type description in the `CTxDestination` docstring.
💬 theStack commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1679647788)
AFAICT this assert would lead to a crash if a tx is scanned with a taproot output where the x-only-pubkey is not on the curve (note that such outputs adhere to standardness rules, i.e. it's trivial to create such a tx, and probably there are already a good amount of them on mainnet). Proposed fix, not tested yet:
```suggestion
if (!secp256k1_xonly_pubkey_parse(secp256k1_context_static, &tx_output_obj, txoutputs[i].data())) {
continue;
}
```
For consistency reaso
...
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1679647788)
AFAICT this assert would lead to a crash if a tx is scanned with a taproot output where the x-only-pubkey is not on the curve (note that such outputs adhere to standardness rules, i.e. it's trivial to create such a tx, and probably there are already a good amount of them on mainnet). Proposed fix, not tested yet:
```suggestion
if (!secp256k1_xonly_pubkey_parse(secp256k1_context_static, &tx_output_obj, txoutputs[i].data())) {
continue;
}
```
For consistency reaso
...
👍 maflcko approved a pull request: "fix: Make TxidFromString() respect string_view length"
(https://github.com/bitcoin/bitcoin/pull/30436#pullrequestreview-2180659057)
ACK c3a9c71c7077324bf0aa40f834f7537a14619340 🐞
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK c3a9c71c7077324bf0aa40f834
...
(https://github.com/bitcoin/bitcoin/pull/30436#pullrequestreview-2180659057)
ACK c3a9c71c7077324bf0aa40f834f7537a14619340 🐞
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK c3a9c71c7077324bf0aa40f834
...