💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1860154761)
The issue is re-defining `wtxid` (for `parent_wtxid`) while it's already defined as a transaction-in-question :)
That was the source in my confusion originally
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1860154761)
The issue is re-defining `wtxid` (for `parent_wtxid`) while it's already defined as a transaction-in-question :)
That was the source in my confusion originally
💬 laanwj commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#discussion_r1860193767)
Yes, it's unrelated to the scope of the PR anyway.
(https://github.com/bitcoin/bitcoin/pull/30986#discussion_r1860193767)
Yes, it's unrelated to the scope of the PR anyway.
👍 laanwj approved a pull request: "contrib: skip missing binaries in gen-manpages"
(https://github.com/bitcoin/bitcoin/pull/30986#pullrequestreview-2464281216)
re-ACK ee6185372fc317d3948690997117e42f6b79a5ff
(https://github.com/bitcoin/bitcoin/pull/30986#pullrequestreview-2464281216)
re-ACK ee6185372fc317d3948690997117e42f6b79a5ff
💬 maflcko commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1860200766)
My preference would be to drop this comment blob. Otherwise, there will be bike-shedding when it is fine to remove it. Also, the code has to be touched again at that time. It seems better to just touch it once and then be done with it.
Suggested diff:
```diff
diff --git a/src/test/result_tests.cpp b/src/test/result_tests.cpp
index 6a23a7b..5e18d23 100644
--- a/src/test/result_tests.cpp
+++ b/src/test/result_tests.cpp
@@ -63,7 +63,8 @@ void ExpectSuccess(const util::Result<T>& result
...
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1860200766)
My preference would be to drop this comment blob. Otherwise, there will be bike-shedding when it is fine to remove it. Also, the code has to be touched again at that time. It seems better to just touch it once and then be done with it.
Suggested diff:
```diff
diff --git a/src/test/result_tests.cpp b/src/test/result_tests.cpp
index 6a23a7b..5e18d23 100644
--- a/src/test/result_tests.cpp
+++ b/src/test/result_tests.cpp
@@ -63,7 +63,8 @@ void ExpectSuccess(const util::Result<T>& result
...
💬 naumenkogs commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#issuecomment-2503243488)
ACK https://github.com/bitcoin/bitcoin/commit/32fc59796f74a2941772b5ec2755b1319132cd9c
(https://github.com/bitcoin/bitcoin/pull/31096#issuecomment-2503243488)
ACK https://github.com/bitcoin/bitcoin/commit/32fc59796f74a2941772b5ec2755b1319132cd9c
💬 dexizer7799 commented on issue "Release Schedule for 29.0":
(https://github.com/bitcoin/bitcoin/issues/31029#issuecomment-2503389081)
Please fix the Dust/Dusting/Vector76/Double Spend Attack for Bitcoin.
(https://github.com/bitcoin/bitcoin/issues/31029#issuecomment-2503389081)
Please fix the Dust/Dusting/Vector76/Double Spend Attack for Bitcoin.
💬 dexizer7799 commented on issue "Release Schedule for 29.0":
(https://github.com/bitcoin/bitcoin/issues/31029#issuecomment-2503390537)
Add support for escrow transactions.
(https://github.com/bitcoin/bitcoin/issues/31029#issuecomment-2503390537)
Add support for escrow transactions.
💬 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.
(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.
(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
...
(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.
(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.
(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?
(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.
(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?
(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
(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
...
(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)