💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2231054406)
Getting a mismatch again. This time for block 716120 (00000000000000000009bc9db59a266e367f54fdd4cda7b5ddb31f6ae9723083).
```
bitcoin-cli getsilentpaymentblockdata 00000000000000000009bc9db59a266e367f54fdd4cda7b5ddb31f6ae9723083 | jq '.bip352_tweaks | sort'
[
"031799df7770cfdabe460e47f4571fe4ded7d7a7922afa0f5ad91753473269cdbb",
"0323fefb4e4bf637d7819c2030996dbe88d97c6a2f5708841df0e65fafd32c5a0a",
"03b2bda3a513123fe810cb1e65cd8a71a2495ea9229c50e4acc8d4b5baa51b4147",
"03cdc8c9f07d
...
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2231054406)
Getting a mismatch again. This time for block 716120 (00000000000000000009bc9db59a266e367f54fdd4cda7b5ddb31f6ae9723083).
```
bitcoin-cli getsilentpaymentblockdata 00000000000000000009bc9db59a266e367f54fdd4cda7b5ddb31f6ae9723083 | jq '.bip352_tweaks | sort'
[
"031799df7770cfdabe460e47f4571fe4ded7d7a7922afa0f5ad91753473269cdbb",
"0323fefb4e4bf637d7819c2030996dbe88d97c6a2f5708841df0e65fafd32c5a0a",
"03b2bda3a513123fe810cb1e65cd8a71a2495ea9229c50e4acc8d4b5baa51b4147",
"03cdc8c9f07d
...
🤔 hebasto reviewed a pull request: "build: use `-no_exported_symbols` on macOS"
(https://github.com/bitcoin/bitcoin/pull/29072#pullrequestreview-2180434936)
Tested 81d4dc8e8739df0e9a8e92f55071733f6500617b on macOS 14.5 Sonoma (Apple Silicon):
- the master branch @ 9c5cdf07f30f816cd134e2cd2dca9c27ef7067a5:
```
% nm --extern-only src/bitcoind | wc -l
579
```
- this PR:
```
% nm --extern-only src/bitcoind | wc -l
537
```
I don't know the reasons, but `bitcoind` still exports
```
0000000100000000 T __mh_execute_header
```
The new `-no_exported_symbols` linker flag is
...
(https://github.com/bitcoin/bitcoin/pull/29072#pullrequestreview-2180434936)
Tested 81d4dc8e8739df0e9a8e92f55071733f6500617b on macOS 14.5 Sonoma (Apple Silicon):
- the master branch @ 9c5cdf07f30f816cd134e2cd2dca9c27ef7067a5:
```
% nm --extern-only src/bitcoind | wc -l
579
```
- this PR:
```
% nm --extern-only src/bitcoind | wc -l
537
```
I don't know the reasons, but `bitcoind` still exports
```
0000000100000000 T __mh_execute_header
```
The new `-no_exported_symbols` linker flag is
...
💬 theStack commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1679523217)
> Can you retest after todays rebase?
Sure. Retested on commit d0019f290ab67ab4190274bb325bfc29bbfcea56, can confirm that the BIP352 indexer on signet runs through (up to current block 204598) now without crashing.
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1679523217)
> Can you retest after todays rebase?
Sure. Retested on commit d0019f290ab67ab4190274bb325bfc29bbfcea56, can confirm that the BIP352 indexer on signet runs through (up to current block 204598) now without crashing.
💬 fanquake commented on pull request "build: use `-no_exported_symbols` on macOS":
(https://github.com/bitcoin/bitcoin/pull/29072#issuecomment-2231067816)
> I don't know the reasons, but bitcoind still exports
That is expected. See the PR description.
> The new -no_exported_symbols linker flag is not printed in the configure summary.
That's expected. We don't print the RE LD flags.
(https://github.com/bitcoin/bitcoin/pull/29072#issuecomment-2231067816)
> I don't know the reasons, but bitcoind still exports
That is expected. See the PR description.
> The new -no_exported_symbols linker flag is not printed in the configure summary.
That's expected. We don't print the RE LD flags.
💬 mzumsande commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#issuecomment-2231073104)
I didn't find a reference in a BIP (there isn't any for the general version message flow as far as I can see) and maybe "violation" is too strong, but any message received before the version would be ignored by bitcoin core peers:
https://github.com/bitcoin/bitcoin/blob/46878326808f643f08ec3f69dcec5c8fafeb7b59/src/net_processing.cpp#L3889-L3893
So this is clearly not the intended way the protocol should work.
(https://github.com/bitcoin/bitcoin/pull/30394#issuecomment-2231073104)
I didn't find a reference in a BIP (there isn't any for the general version message flow as far as I can see) and maybe "violation" is too strong, but any message received before the version would be ignored by bitcoin core peers:
https://github.com/bitcoin/bitcoin/blob/46878326808f643f08ec3f69dcec5c8fafeb7b59/src/net_processing.cpp#L3889-L3893
So this is clearly not the intended way the protocol should work.
👍 theuni approved a pull request: "build: use `-no_exported_symbols` on macOS"
(https://github.com/bitcoin/bitcoin/pull/29072#pullrequestreview-2180454092)
utACK 81d4dc8e8739df0e9a8e92f55071733f6500617b
(https://github.com/bitcoin/bitcoin/pull/29072#pullrequestreview-2180454092)
utACK 81d4dc8e8739df0e9a8e92f55071733f6500617b
👍 hebasto approved a pull request: "build: use `-no_exported_symbols` on macOS"
(https://github.com/bitcoin/bitcoin/pull/29072#pullrequestreview-2180461920)
ACK 81d4dc8e8739df0e9a8e92f55071733f6500617b.
> > The new -no_exported_symbols linker flag is not printed in the configure summary.
>
> That's expected. We don't print the RE LD flags.
Not related to this PR changes, but that's odd as it undermines the summary's goal.
(https://github.com/bitcoin/bitcoin/pull/29072#pullrequestreview-2180461920)
ACK 81d4dc8e8739df0e9a8e92f55071733f6500617b.
> > The new -no_exported_symbols linker flag is not printed in the configure summary.
>
> That's expected. We don't print the RE LD flags.
Not related to this PR changes, but that's odd as it undermines the summary's goal.
🚀 fanquake merged a pull request: "build: use `-no_exported_symbols` on macOS"
(https://github.com/bitcoin/bitcoin/pull/29072)
(https://github.com/bitcoin/bitcoin/pull/29072)
💬 mzumsande commented on pull request "locks: introduce mutex for tx download, flush rejection filters once per tip change":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1679560788)
Makes sense! In `net.cpp`, the `AssertLockNotHeld(x)` / `LOCK(x)` pattern is so common that I didn't even consider the possibility that this could be on purpose.
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1679560788)
Makes sense! In `net.cpp`, the `AssertLockNotHeld(x)` / `LOCK(x)` pattern is so common that I didn't even consider the possibility that this could be on purpose.
📝 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)