💬 hebasto commented on pull request "cmake: Fix passing `APPEND_*FLAGS` to `secp256k1` subtree":
(https://github.com/bitcoin/bitcoin/pull/31379#issuecomment-2503483293)
> How can the broken behavior be observed?
For example:
```
cmake -B build -DAPPEND_CFLAGS="-fprofile-update=prefer-atomic"
# Note the absence of the `SECP256K1_APPEND_CFLAGS ...` line in the secp256k1 summary
cmake --build build --target secp256k1 --verbose
```
> It appears to be working to me based on: the MSan job not spitting out false positives and coverage reports including coverage for secp.
Yes, passing sanitizers flags works on the master branch.
**Another use case to
...
(https://github.com/bitcoin/bitcoin/pull/31379#issuecomment-2503483293)
> How can the broken behavior be observed?
For example:
```
cmake -B build -DAPPEND_CFLAGS="-fprofile-update=prefer-atomic"
# Note the absence of the `SECP256K1_APPEND_CFLAGS ...` line in the secp256k1 summary
cmake --build build --target secp256k1 --verbose
```
> It appears to be working to me based on: the MSan job not spitting out false positives and coverage reports including coverage for secp.
Yes, passing sanitizers flags works on the master branch.
**Another use case to
...
⚠️ dexizer7799 opened an issue: "Feature Request"
(https://github.com/bitcoin/bitcoin/issues/31380)
### Please describe the feature you'd like to see added.
1. Fix the Dust/Dusting/Vector76/Double Spend Attack for Bitcoin.
2. Add support for escrow transactions.
3. Add support for choose algorithm of signing I mean support for not only Secp256k1 add Curve25519 with Ristretto.
4. Add advanced support for blind signing transactions to solve attacks like Lattice attack and other.
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you
...
(https://github.com/bitcoin/bitcoin/issues/31380)
### Please describe the feature you'd like to see added.
1. Fix the Dust/Dusting/Vector76/Double Spend Attack for Bitcoin.
2. Add support for escrow transactions.
3. Add support for choose algorithm of signing I mean support for not only Secp256k1 add Curve25519 with Ristretto.
4. Add advanced support for blind signing transactions to solve attacks like Lattice attack and other.
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you
...
💬 danielabrozzoni commented on issue "Unable to compile for test coverage on Nixos 24.05":
(https://github.com/bitcoin/bitcoin/issues/31087#issuecomment-2503528973)
@hebasto yes, I had posted it in the issue description under "Relevant log output" -> "> Configure build", but I noticed that on current master (f7144b24be09e7db2173a0b15d73f99a08b98118) the output is slightly different, so here's the current one:
```
bitcoin git:master*
❯ cmake -B build -DCMAKE_BUILD_TYPE=Coverage
Configuring secp256k1 subtree...
-- Could NOT find Valgrind (missing: Valgrind_INCLUDE_DIR Valgrind_WORKS)
secp256k1 configure summary
===========================
Bui
...
(https://github.com/bitcoin/bitcoin/issues/31087#issuecomment-2503528973)
@hebasto yes, I had posted it in the issue description under "Relevant log output" -> "> Configure build", but I noticed that on current master (f7144b24be09e7db2173a0b15d73f99a08b98118) the output is slightly different, so here's the current one:
```
bitcoin git:master*
❯ cmake -B build -DCMAKE_BUILD_TYPE=Coverage
Configuring secp256k1 subtree...
-- Could NOT find Valgrind (missing: Valgrind_INCLUDE_DIR Valgrind_WORKS)
secp256k1 configure summary
===========================
Bui
...
✅ willcl-ark closed an issue: "Feature Request"
(https://github.com/bitcoin/bitcoin/issues/31380)
(https://github.com/bitcoin/bitcoin/issues/31380)
💬 willcl-ark commented on issue "Feature Request":
(https://github.com/bitcoin/bitcoin/issues/31380#issuecomment-2503536436)
@dexizer7799
Thanks for taking the time to submit these feature suggestions.
While these are interesting proposals, we need to keep our issue tracker focused on specific, actionable items. These suggestions cover multiple substantial protocol changes that would each require extensive design, discussion, and review processes.
Pull requests that implment any of these features would be welcome for review here, and these would allow the community to properly evaluate the specific implement
...
(https://github.com/bitcoin/bitcoin/issues/31380#issuecomment-2503536436)
@dexizer7799
Thanks for taking the time to submit these feature suggestions.
While these are interesting proposals, we need to keep our issue tracker focused on specific, actionable items. These suggestions cover multiple substantial protocol changes that would each require extensive design, discussion, and review processes.
Pull requests that implment any of these features would be welcome for review here, and these would allow the community to properly evaluate the specific implement
...
🚀 fanquake merged a pull request: "guix: swap `moreutils` for just `sponge`"
(https://github.com/bitcoin/bitcoin/pull/31323)
(https://github.com/bitcoin/bitcoin/pull/31323)
💬 dergoegge commented on pull request "cmake: Fix passing `APPEND_*FLAGS` to `secp256k1` subtree":
(https://github.com/bitcoin/bitcoin/pull/31379#issuecomment-2503562852)
> Yes, passing sanitizers flags works on the master branch.
Our MSan ci job [passes C/CXXFLAGS to depends](https://github.com/bitcoin/bitcoin/blob/efdb49afb9e24962958ad4445458ad8da253183b/ci/test/00_setup_env_native_msan.sh#L18) (including non-"-fsanitize=" flags e.g. `-fno-omit-frame-pointer -g -O1 -fno-optimize-sibling-calls`), why do those flags propagate to secp but the `APPEND_CFLAGS` flags do not? Would it make sense to use the same mechanism for both?
(I'm very confused by all the d
...
(https://github.com/bitcoin/bitcoin/pull/31379#issuecomment-2503562852)
> Yes, passing sanitizers flags works on the master branch.
Our MSan ci job [passes C/CXXFLAGS to depends](https://github.com/bitcoin/bitcoin/blob/efdb49afb9e24962958ad4445458ad8da253183b/ci/test/00_setup_env_native_msan.sh#L18) (including non-"-fsanitize=" flags e.g. `-fno-omit-frame-pointer -g -O1 -fno-optimize-sibling-calls`), why do those flags propagate to secp but the `APPEND_CFLAGS` flags do not? Would it make sense to use the same mechanism for both?
(I'm very confused by all the d
...
💬 0xB10C commented on pull request "ci: allow passing extra args to docker build with CI_IMAGE_BUILD_EXTRA_ARGS":
(https://github.com/bitcoin/bitcoin/pull/31377#issuecomment-2503596845)
It occurred to me that the local cache needs to be keyed by e.g. `${CONTAINER_NAME}` since different tasks build different images. This wasn't clear to me from the build cache documentation at first. Setting `CI_IMAGE_BUILD_EXTRA_ARGS="--cache-to type=local,dest=/cache/docker/${CONTAINER_NAME}"` doesn't work, as bash does not expand variables in variables. I'll mark this as draft for now until I find a solution.
> However, the images are quite large (especially msan), so my worry would be tha
...
(https://github.com/bitcoin/bitcoin/pull/31377#issuecomment-2503596845)
It occurred to me that the local cache needs to be keyed by e.g. `${CONTAINER_NAME}` since different tasks build different images. This wasn't clear to me from the build cache documentation at first. Setting `CI_IMAGE_BUILD_EXTRA_ARGS="--cache-to type=local,dest=/cache/docker/${CONTAINER_NAME}"` doesn't work, as bash does not expand variables in variables. I'll mark this as draft for now until I find a solution.
> However, the images are quite large (especially msan), so my worry would be tha
...
📝 0xB10C converted_to_draft a pull request: "ci: allow passing extra args to docker build with CI_IMAGE_BUILD_EXTRA_ARGS"
(https://github.com/bitcoin/bitcoin/pull/31377)
By setting `CI_IMAGE_BUILD_EXTRA_ARGS`, a CI runner can influence the docker build of the CI container image. This allows to, for example, pass `--cache-to` and `--cache-from` to the build, which is useful for ephemeral, self-hosted CI runners.
(https://github.com/bitcoin/bitcoin/pull/31377)
By setting `CI_IMAGE_BUILD_EXTRA_ARGS`, a CI runner can influence the docker build of the CI container image. This allows to, for example, pass `--cache-to` and `--cache-from` to the build, which is useful for ephemeral, self-hosted CI runners.
💬 maflcko commented on pull request "ci: allow passing extra args to docker build with CI_IMAGE_BUILD_EXTRA_ARGS":
(https://github.com/bitcoin/bitcoin/pull/31377#issuecomment-2503617213)
> It occurred to me that the local cache needs to be keyed by e.g. `${CONTAINER_NAME}` since different tasks build different images.
Are you sure? The final layers should be different for each task, so should never collide. (Possibly early layers may be shared, if they happen to be the same). If this was an issue, shouldn't the CI normally fail when running several tasks on the same machine in the same user account?
> After the build, overwrite `/path/to/docker/build/cache/${CONTAINER_
...
(https://github.com/bitcoin/bitcoin/pull/31377#issuecomment-2503617213)
> It occurred to me that the local cache needs to be keyed by e.g. `${CONTAINER_NAME}` since different tasks build different images.
Are you sure? The final layers should be different for each task, so should never collide. (Possibly early layers may be shared, if they happen to be the same). If this was an issue, shouldn't the CI normally fail when running several tasks on the same machine in the same user account?
> After the build, overwrite `/path/to/docker/build/cache/${CONTAINER_
...
💬 0xB10C commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1860486568)
> (unless they are switched to @0xB10C's workers, which are running as root?)
the runner setup I'm working on explicitly **doesn't** run as root and is far from finished :)
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1860486568)
> (unless they are switched to @0xB10C's workers, which are running as root?)
the runner setup I'm working on explicitly **doesn't** run as root and is far from finished :)
💬 maflcko commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1860526331)
> the runner setup I'm working on explicitly **doesn't** run as root
Are you sure, because the current CI (in this run) is run in a user account (not root), and gives a permission error. The same CI in your run does not give a permission error, so there seems to be a difference.
The only thing I see is that you are using docker, which IIRC is running rootful by default.
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1860526331)
> the runner setup I'm working on explicitly **doesn't** run as root
Are you sure, because the current CI (in this run) is run in a user account (not root), and gives a permission error. The same CI in your run does not give a permission error, so there seems to be a difference.
The only thing I see is that you are using docker, which IIRC is running rootful by default.
👍 scgbckbone approved a pull request: "test: Add `leaf_version` parameter to `taproot_tree_helper()`"
(https://github.com/bitcoin/bitcoin/pull/29371#pullrequestreview-2464807260)
ACK
(https://github.com/bitcoin/bitcoin/pull/29371#pullrequestreview-2464807260)
ACK
📝 sky-coderay opened a pull request: "Update init.md"
(https://github.com/bitcoin/bitcoin/pull/31381)
###Motivation for the patch:
The change corrects a spelling error in the documentation, where "behaviour" was mistakenly used instead of the correct American English spelling "behavior." Since Bitcoin Core's official documentation follows American English conventions, this fix ensures consistency and accuracy, making the documentation clearer and more professional for users and developers.
(https://github.com/bitcoin/bitcoin/pull/31381)
###Motivation for the patch:
The change corrects a spelling error in the documentation, where "behaviour" was mistakenly used instead of the correct American English spelling "behavior." Since Bitcoin Core's official documentation follows American English conventions, this fix ensures consistency and accuracy, making the documentation clearer and more professional for users and developers.
💬 willcl-ark commented on pull request "Update init.md":
(https://github.com/bitcoin/bitcoin/pull/31381#issuecomment-2503726337)
Please don't contribute these AI-generated fixes here in the future, thank you.
(https://github.com/bitcoin/bitcoin/pull/31381#issuecomment-2503726337)
Please don't contribute these AI-generated fixes here in the future, thank you.
💬 fanquake commented on pull request "cmake: Check `-Wno-*` compiler options for `leveldb` target":
(https://github.com/bitcoin/bitcoin/pull/31366#issuecomment-2503734949)
In Autotools we filtered out the unwanted flags, ifor CMake, the `-Wno-*` variants were added (but in a way that didn't quite work), and this PR fixes their usage by adding checks for the flags. However that means we duplicate checks that we do for the same flags earlier in the buildsystem, and we end up with something like:
`clang++ ... -Wconditional-uninitialized -Wsuggest-override ... -Wno-conditional-uninitialized -Wno-suggest-override`
when building leveldb. Wondering if we could have
...
(https://github.com/bitcoin/bitcoin/pull/31366#issuecomment-2503734949)
In Autotools we filtered out the unwanted flags, ifor CMake, the `-Wno-*` variants were added (but in a way that didn't quite work), and this PR fixes their usage by adding checks for the flags. However that means we duplicate checks that we do for the same flags earlier in the buildsystem, and we end up with something like:
`clang++ ... -Wconditional-uninitialized -Wsuggest-override ... -Wno-conditional-uninitialized -Wno-suggest-override`
when building leveldb. Wondering if we could have
...
💬 0xB10C commented on pull request "ci: allow passing extra args to docker build with CI_IMAGE_BUILD_EXTRA_ARGS":
(https://github.com/bitcoin/bitcoin/pull/31377#issuecomment-2503777192)
> > It occurred to me that the local cache needs to be keyed by e.g. `${CONTAINER_NAME}` since different tasks build different images.
>
> Are you sure? The final layers should be different for each task, so should never collide. (Possibly early layers may be shared, if they happen to be the same). If this was an issue, shouldn't the CI normally fail when running several tasks on the same machine in the same user account?
I'm not sure if we are talking about the same thing. I mean the `doc
...
(https://github.com/bitcoin/bitcoin/pull/31377#issuecomment-2503777192)
> > It occurred to me that the local cache needs to be keyed by e.g. `${CONTAINER_NAME}` since different tasks build different images.
>
> Are you sure? The final layers should be different for each task, so should never collide. (Possibly early layers may be shared, if they happen to be the same). If this was an issue, shouldn't the CI normally fail when running several tasks on the same machine in the same user account?
I'm not sure if we are talking about the same thing. I mean the `doc
...
🤔 willcl-ark reviewed a pull request: "RFC: build: support for pre-compiled headers."
(https://github.com/bitcoin/bitcoin/pull/31053#pullrequestreview-2462100101)
Took a first pass over this. It seems to work for me locally, both with and without `ccache`.
I see both clang and gcc running faster with this change than merge-base @ 5fb94550638 (with clang being much faster than gcc in both cases):
```log
Summary
env CCACHE_DISABLE=1 CC=clang CXX=clang++ just build (commit = 3cec3508a79c08db5b97e93bc4225ccdd7d32ad8) ran
1.32 ± 0.02 times faster than env CCACHE_DISABLE=1 CC=clang CXX=clang++ just build (commit = 5fb94550638)
1.43 ± 0.00 ti
...
(https://github.com/bitcoin/bitcoin/pull/31053#pullrequestreview-2462100101)
Took a first pass over this. It seems to work for me locally, both with and without `ccache`.
I see both clang and gcc running faster with this change than merge-base @ 5fb94550638 (with clang being much faster than gcc in both cases):
```log
Summary
env CCACHE_DISABLE=1 CC=clang CXX=clang++ just build (commit = 3cec3508a79c08db5b97e93bc4225ccdd7d32ad8) ran
1.32 ± 0.02 times faster than env CCACHE_DISABLE=1 CC=clang CXX=clang++ just build (commit = 5fb94550638)
1.43 ± 0.00 ti
...
💬 willcl-ark commented on pull request "RFC: build: support for pre-compiled headers.":
(https://github.com/bitcoin/bitcoin/pull/31053#discussion_r1858875514)
They also give [GCC flags](https://ccache.dev/manual/latest.html#_precompiled_headers). Perhaps here we can therefore use something like:
```patch
diff --git a/cmake/ccache.cmake b/cmake/ccache.cmake
index 08f078e627b..cb7059884fa 100644
--- a/cmake/ccache.cmake
+++ b/cmake/ccache.cmake
@@ -35,7 +35,12 @@ if(NOT MSVC)
if(NOT CCACHE_TEST_RESULT)
list(APPEND CMAKE_CXX_COMPILER_LAUNCHER sloppiness=pch_defines,time_macros)
# According to the ccache docs clang requ
...
(https://github.com/bitcoin/bitcoin/pull/31053#discussion_r1858875514)
They also give [GCC flags](https://ccache.dev/manual/latest.html#_precompiled_headers). Perhaps here we can therefore use something like:
```patch
diff --git a/cmake/ccache.cmake b/cmake/ccache.cmake
index 08f078e627b..cb7059884fa 100644
--- a/cmake/ccache.cmake
+++ b/cmake/ccache.cmake
@@ -35,7 +35,12 @@ if(NOT MSVC)
if(NOT CCACHE_TEST_RESULT)
list(APPEND CMAKE_CXX_COMPILER_LAUNCHER sloppiness=pch_defines,time_macros)
# According to the ccache docs clang requ
...