Bitcoin Core Github
43 subscribers
122K links
Download Telegram
πŸš€ fanquake merged a pull request: "refactor: Replace `std::optional<bilingual_str>` with `util::Result`"
(https://github.com/bitcoin/bitcoin/pull/25977)
πŸ’¬ hebasto commented on pull request "build: Add CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/25797#discussion_r1206695310)
Oh... Bad rebasing. Thanks!
πŸ’¬ hebasto commented on issue "ci: failure in Docker build step":
(https://github.com/bitcoin/bitcoin/issues/27492#issuecomment-1564303999)
> That seems unrelated, see [#25797 (files)](https://github.com/bitcoin/bitcoin/pull/25797/files#r1206672381)

Indeed. Sorry for noise.
πŸ’¬ joostjager commented on pull request "Allow accepting non-standard transactions on mainnet":
(https://github.com/bitcoin/bitcoin/pull/27578#issuecomment-1564315193)
>Its direct peers will simply discard these when your node broadcasts them, and they will eventually be purged from your mempool as any other non-confirmed transaction.

Unless you have peers that run a patch to accept non-standard transactions?

Also if you are mining yourself and using core's block template, you need those txes to enter your mempool first?
πŸš€ fanquake merged a pull request: "init: Error if ignored bitcoin.conf file is found"
(https://github.com/bitcoin/bitcoin/pull/27302)
πŸ’¬ TheCharlatan commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564324755)
Re https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564274015

> Can you share the details where it is first set?

Here is the entire output I am willing to share (the rest above prints my env, which I don't want to paste here)

<p>
<details>
Creating ubuntu:lunar container to run in
[+] Building 1.0s (9/9) FINISHED
=> [in
...
πŸ‘ stickies-v approved a pull request: "[23.x] rpc: Fix invalid bech32 handling"
(https://github.com/bitcoin/bitcoin/pull/27756#pullrequestreview-1446142241)
ACK d98770720885cdce1bbe2109a6d214c63fa1814a

Verified that the commit is identical to c2e9214effe9abecae6f81cb10158f9661065da3 (#27755). Change lgtm.

<details>
<summary><code>diff <(git show c2e9214effe9abecae6f81cb10158f9661065da3) <(git show d98770720885cdce1bbe2109a6d214c63fa1814a)</code></summary>

```
1c1
< commit c2e9214effe9abecae6f81cb10158f9661065da3
---
> commit d98770720885cdce1bbe2109a6d214c63fa1814a
336c336
< index b9233ef2c..9ade9e9ea 100755
---
> index b0f24e3b9..
...
πŸ’¬ fanquake commented on pull request "doc: Add Python install notes to build-osx.md.":
(https://github.com/bitcoin/bitcoin/pull/27728#issuecomment-1564333068)
> Still Concept Nack?

Yes. Developers are free to use pyenv if they want, but it's not a requirement. I'd also say that most developers are likely using a version of Python which is actually much newer, than our oldest supported version.

Also, note that the `ds_store` and `mac_alias` packages will be dropped entirely after #27099, so I think there is even less need to make these changes at this point.
πŸ’¬ MarcoFalke commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564348866)
Well, it isn't building the image, just printing `CACHED`. I'd need the output from when it was built of the step that ran base_install:


```
CACHED [4/4] RUN ["bash", "-c", "cd /ci_base_install/ && set -o errexit && source ./ci/test/00_setup_env.sh && ./ci/test/01_base_install.sh"]
πŸ’¬ MarcoFalke commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564349776)
> the rest above prints my env, which I don't want to paste here

If you can reproduce on a fresh install of your OS, that'd help too
πŸ’¬ hebasto commented on pull request "refactor: Replace `std::optional<bilingual_str>` with `util::Result`":
(https://github.com/bitcoin/bitcoin/pull/25977#issuecomment-1564350880)
> Looks like there is a clang bug. Could be fixed by either bumping it again, see #27682, but I am not sure how to do that for macOS. Or by replacing `return addrman;` with `return {std::move(addrman)};`, or something like that (temporarily).

Although GCC complains:
```
$ ./configure CONFIG_SITE=$PWD/depends/arm-linux-gnueabihf/share/config.site --enable-suppress-external-warnings CXXFLAGS="-Wextra -Wno-psabi"
$ make > /dev/null
addrdb.cpp: In function β€˜util::Result<std::unique_ptr<AddrM
...
πŸ’¬ dergoegge commented on pull request "fuzz: Change LIMIT_TO_MESSAGE_TYPE from a compile-time to a run-time setting":
(https://github.com/bitcoin/bitcoin/pull/27766#discussion_r1206750051)
I think we can just get rid of the `FUZZ_TARGET_MSG` macro including `MsgTypes()` and use `getAllNetMessageTypes` directly?
πŸ€” dergoegge reviewed a pull request: "fuzz: Change LIMIT_TO_MESSAGE_TYPE from a compile-time to a run-time setting"
(https://github.com/bitcoin/bitcoin/pull/27766#pullrequestreview-1446186895)
Concept ACK
πŸ’¬ Crypt-iQ commented on issue "bitcoind hangs waiting for `g_requests.empty()`":
(https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1564380223)
@willcl-ark can you find what libevent version you're using?
πŸ’¬ TheCharlatan commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564383177)
Re https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564349776

> If you can reproduce on a fresh install of your OS, that'd help too

Not a fresh install, but another OS where I have not run this before:
<p>
<details>
MAKEJOBS="-j12" FILE_ENV="./ci/test/00_setup_env_native_tidy.sh" ./ci/test_run_all.sh
Setting specific values in env
Fallback to default values in env (if not yet set)
...
CONTAINER_NAME=ci_native_tidy
DEPENDS_DIR=/home/drgrid/bitcoin/depends
TERM=xterm-256
...
πŸ’¬ MarcoFalke commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#issuecomment-1564406303)
`void` can be removed from OP?
πŸ’¬ hebasto commented on pull request "refactor: Make `CCheckQueue` RAII-styled":
(https://github.com/bitcoin/bitcoin/pull/26762#issuecomment-1564429422)
Rebased 52129e335cbc68ac5d863f283f7d1a328ce79581 -> 2076d846cc917cbafe61937a99b7867067011341 ([pr26762.08](https://github.com/hebasto/bitcoin/commits/pr26762.08) -> [pr26762.09](https://github.com/hebasto/bitcoin/commits/pr26762.09)) due to the conflict with #25977.
πŸš€ fanquake merged a pull request: "test: Move test_chain_listunspent wallet check from mempool_packages to wallet_basic"
(https://github.com/bitcoin/bitcoin/pull/27735)
πŸ’¬ stickies-v commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#issuecomment-1564441571)
@brunoerg are you going to address nits or leave as is? I'd really like to get this merged asap to prevent further rebase conflicts. Happy to quickly re-ack nits too, though.
πŸ’¬ 1ma commented on pull request "Allow accepting non-standard transactions on mainnet":
(https://github.com/bitcoin/bitcoin/pull/27578#issuecomment-1564474817)
Even then they won't go further than that, unless these peers are miners. This option would be only somewhat relevant if you run a solo mining node or a pool coordinator.