Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 eval-exec commented on pull request "random: Initialize variables in hardware RNG functions":
(https://github.com/bitcoin/bitcoin/pull/31863#discussion_r1955991364)
Agreed, seting `r1 = 0` is problematic.
💬 hebasto commented on pull request "depends: set `CMAKE_*_COMPILER_TARGET` in toolchain":
(https://github.com/bitcoin/bitcoin/pull/31849#issuecomment-2659057610)
> > Would it be worth filtering out --target=$ARCH-apple-darwin from CMAKE__COMPILER in the toolchain file?
>
> It probably depends on how much code is needed, but I'm not sure filtering should be done unless its generic.

For example:
```diff
--- a/depends/toolchain.cmake.in
+++ b/depends/toolchain.cmake.in
@@ -29,8 +29,15 @@ if(NOT DEFINED CMAKE_C_FLAGS_DEBUG_INIT)
set(CMAKE_C_FLAGS_DEBUG_INIT "@CFLAGS_DEBUG@")
endif()

+macro(fliter_flags compiler_invocation_line)
+ if(CMA
...
💬 Sjors commented on pull request "cmake: Improve compatibility with Python version managers":
(https://github.com/bitcoin/bitcoin/pull/31421#issuecomment-2659113645)
`brew unlink python3` "fixes" it for me. We could mention it as a build hint if someone runs into a problem. Not sure how likely that is to be problem though.
💬 fanquake commented on pull request "cmake: Improve compatibility with Python version managers":
(https://github.com/bitcoin/bitcoin/pull/31421#issuecomment-2659124802)
It's not clear what the problem is here. Looking at https://github.com/bitcoin/bitcoin/pull/31421#issuecomment-2659034075, it seems like the correct Python is being picked, because the brew installed python is `python3` on your system. If you want the pyenv Python to be picked, you should setup pyenv such that it's python is `python3` on your system.
💬 hebasto commented on pull request "cmake: Improve compatibility with Python version managers":
(https://github.com/bitcoin/bitcoin/pull/31421#issuecomment-2659125423)
The initial [issue](https://github.com/bitcoin/bitcoin/pull/31411#issuecomment-2516745547) was that CMake finds the Python version other than reported by `which python3` command.

> ```
> which -a python3
> /opt/homebrew/bin//python3
> /Users/sjors/.pyenv/shims/python3
> /opt/homebrew/bin/python3
> /usr/bin/python3
> ```

Your `PATH` looks a bit messy :)
👍 willcl-ark approved a pull request: "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries"
(https://github.com/bitcoin/bitcoin/pull/31407#pullrequestreview-2617591133)
tACK 096525e92cc2f5a4318bae13cedd2cf36b928d5f

I have not thoroughly reviewed the code changes here, but I did read through all the changes. I was more interested in verifying the result of the changes was correct.

I tested the MacOS binaries on both an M3 and x86_64 (Intel) Macs. Of note however is the fact that the Intel Mac is 2017, and so can only run MacOS 10.13 (where 10.15 is the version giving us trouble with the new mandatory gatekeeper policies).

With that aside I can confirm t
...
💬 willcl-ark commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2659146803)
I don't want to delay getting this particular PR in before 29.0, but I do wonder if we might in the future consider tidying the the guix output directory a little? Currently I see:

```bash
$ fd -uu -e .tar.gz -e .zip -e .exe | cut -d '/' -f 2- | sort
bitcoin-096525e92cc2-aarch64-linux-gnu-debug.tar.gz
bitcoin-096525e92cc2-aarch64-linux-gnu.tar.gz
bitcoin-096525e92cc2-arm64-apple-darwin-codesigning.tar.gz
bitcoin-096525e92cc2-arm64-apple-darwin.tar.gz
bitcoin-096525e92cc2-arm64-apple-dar
...
📝 maflcko converted_to_draft a pull request: "test: Rename send_message to send_without_ping"
(https://github.com/bitcoin/bitcoin/pull/31859)
`send_message` is problematic, because it is easy to forget a `sync_with_ping` (or other `wait_until`), leading to intermittent test failures. (Example: https://github.com/bitcoin/bitcoin/pull/31837#discussion_r1950370246)

There are more uses of `send_and_ping` in the codebase than `send_message`, so in most cases `send_and_ping` is needed anyway.

For the remaining cases, clearly document that no sync happens by renaming `send_message` to `send_without_ping`.
💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1956058967)
Yes, done!
💬 willcl-ark commented on pull request "cmake: Improve compatibility with Python version managers":
(https://github.com/bitcoin/bitcoin/pull/31421#issuecomment-2659226739)
> If you want the pyenv Python to be picked, you should setup pyenv such that it's python is `python3` on your system.

Or said another way, rather than fixing by unlinking brew's python, if you want `pyenv` python to be picked up first, the usual move is to ensure the `$PYENV_ROOT/bin` component appears earlier than `/opt/homebrew/bin/` on your `$PATH`.

This is explained further in the [shell setup](https://github.com/pyenv/pyenv?tab=readme-ov-file#b-set-up-your-shell-environment-for-pyen
...
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1956103968)
Hold on, I don't see why it'd throw an assertion? `ancestor_package[-1]` is the last child right? Added some more comments but didn't change the code
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1956104380)
Added a commit before that one, just adding the list and sanity checking
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1956104543)
good point, added a comment
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1956105056)
nice, thanks - added a `setmocktime()` after all the restarts
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1956106767)
Thanks! Added. It does take a long time but that's the only way to get evictions due to announcements in a functional test.
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1956106958)
Added thanks!
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1956109943)
Thanks! Fixed
👍 stickies-v approved a pull request: "cmake: add a component for each binary"
(https://github.com/bitcoin/bitcoin/pull/31844#pullrequestreview-2617756135)
re-ACK 9b033bebb18dfd609c02736292f37cc6589fcc8d
fanquake closed an issue: "build: cmake installs empty manpages for non-configured targets"
(https://github.com/bitcoin/bitcoin/issues/31762)