Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 dexizer7799 commented on issue "Release Schedule for 29.0":
(https://github.com/bitcoin/bitcoin/issues/31029#issuecomment-2503397975)
Add support for choose algorithm of signing I mean support for not only Secp256k1 add Curve25519 with Ristretto.
💬 dexizer7799 commented on issue "Release Schedule for 29.0":
(https://github.com/bitcoin/bitcoin/issues/31029#issuecomment-2503405789)
Add advanced support for blind signing transactions to solve attacks like Lattice attack and other.
📝 hebasto opened a pull request: "cmake: Fix passing `APPEND_*FLAGS` to `secp256k1` subtree"
(https://github.com/bitcoin/bitcoin/pull/31379)
On the master branch @ 70e20ea024ce4f39abc4022e1ba19d5a6db2a207, the `APPEND_CPPFLAGS`, `APPEND_CFLAGS` and `APPEND_LDFLAGS` are not correctly applied when building C code in the `secp256k1` subtree, as intended.

This behaviour occurs due to two issues:
1. The command here: https://github.com/bitcoin/bitcoin/blob/70e20ea024ce4f39abc4022e1ba19d5a6db2a207/src/CMakeLists.txt#L77
does not affect the code in `add_subdirectory(secp256k1)` above it.

2. `APPEND_LDFLAGS` is not passed to the
...
💬 fanquake commented on pull request "build: Fix coverage builds":
(https://github.com/bitcoin/bitcoin/pull/31337#discussion_r1860351148)
I'm not sure this is a good inline comment, because it doesn't actually document the problem, just the side-effects.
💬 fanquake commented on pull request "cmake: Add `CheckLinkerSupportsPIE` module":
(https://github.com/bitcoin/bitcoin/pull/31359#issuecomment-2503431774)
> Fixes an upstream bug
> Enhances robustness

Wondering if in this case given CMakes tooling has not only been buggy, but then is also apparently not robust enough for production use without wrapping it in more layers of abstraction, maybe we should just revert to (as we did in Autotools) something like using the flags we want directly.
💬 hebasto commented on pull request "build: Fix coverage builds":
(https://github.com/bitcoin/bitcoin/pull/31337#discussion_r1860355374)
Mind suggesting a better wording for the comment?
💬 dergoegge commented on pull request "cmake: Fix passing `APPEND_*FLAGS` to `secp256k1` subtree":
(https://github.com/bitcoin/bitcoin/pull/31379#issuecomment-2503447959)
How can the broken behavior be observed? It appears to be working to me based on: the MSan job not spitting out false positives and coverage reports including coverage for secp.
💬 fanquake commented on pull request "cmake: Fix passing `APPEND_*FLAGS` to `secp256k1` subtree":
(https://github.com/bitcoin/bitcoin/pull/31379#issuecomment-2503455571)
> APPEND_LDFLAGS is not passed to the subtree's build system at all.

I thought #30791 fixed an issue related to passing down linker flags via `APPEND_LDFLAGS`. So has that regressed / broken since, or just never worked?
💬 maflcko commented on pull request "build: Fix coverage builds":
(https://github.com/bitcoin/bitcoin/pull/31337#issuecomment-2503468035)
review ACK 01a7298818d7bd66bb8c800d7cd30ae0a4d3b1d1
💬 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
...
⚠️ 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
...
💬 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
...
willcl-ark closed an issue: "Feature Request"
(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
...
🚀 fanquake merged a pull request: "guix: swap `moreutils` for just `sponge`"
(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
...
💬 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
...
📝 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.
💬 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_
...
💬 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 :)