Bitcoin Core Github
43 subscribers
123K links
Download Telegram
πŸ’¬ l0rinc commented on pull request "ArgsManager: support subcommand-specific options":
(https://github.com/bitcoin/bitcoin/pull/28802#discussion_r2447492960)
This seems more complicated than I think it should be, `strprintf` has baked-in indentation
```suggestion
return strprintf("%*s%s%s\n%*s%s\n\n",
optIndent + indent, "",option, help_param,
msgIndent + indent, "", FormatParagraph(message, screenWidth - msgIndent - indent, msgIndent + indent));
```

(Though I would have expected `FormatParagraph` to be able to apply the indentation at the beginning as well, which would enable)
```suggestion
return s
...
πŸ’¬ l0rinc commented on pull request "ArgsManager: support subcommand-specific options":
(https://github.com/bitcoin/bitcoin/pull/28802#discussion_r2447591467)
Most of the new code is completely uncovered: https://corecheck.dev/bitcoin/bitcoin/pulls/28802

Adding temporary exceptions inside indicates that `tool_wallet.py` and `wallet_encryption.py` cover some of the functions here.
I understand that `argsman_tests.cpp` is already very weak coverage-wise, but would be great to add some coverage for the new code at least.
πŸ’¬ l0rinc commented on pull request "ArgsManager: support subcommand-specific options":
(https://github.com/bitcoin/bitcoin/pull/28802#discussion_r2447499771)
slightly unrelated: maybe we could use `std::string_view` here instead
πŸ’¬ l0rinc commented on pull request "ArgsManager: support subcommand-specific options":
(https://github.com/bitcoin/bitcoin/pull/28802#discussion_r2447318308)
nit: formatting
πŸ’¬ pinheadmz commented on pull request "Introduce SockMan ("lite"): low-level socket handling for HTTP":
(https://github.com/bitcoin/bitcoin/pull/32747#issuecomment-3425956618)
@theuni I hear you, maybe I should re write the PR description: The purpose is not to establish a generic socket manager but to just advance the bigger goal of replacing libevent with #32061. In that context, this PR is just the first 11 commits of the HTTP server replacement, broken off for more focused unit testing and review (which has all been great so far).

So with that in mind, what do you recommend? Is it higher level organization like removing the abstract class stuff and moving all t
...
βœ… willcl-ark closed an issue: "Data corruption on MacOS when using exFAT datadir or blocksdir"
(https://github.com/bitcoin/bitcoin/issues/31454)
πŸ’¬ willcl-ark commented on issue "Data corruption on MacOS when using exFAT datadir or blocksdir":
(https://github.com/bitcoin/bitcoin/issues/31454#issuecomment-3425966534)
closing since #31453
⚠️ rkrux opened an issue: "Unable to fuzz in local on MacOS 15.4.1"
(https://github.com/bitcoin/bitcoin/issues/33667)
Reproduction steps as per [libfuzzer](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md#quickstart-guide):
```zsh
$ brew install llvm lld
$ cmake --preset=libfuzzer \
-DCMAKE_C_COMPILER="$(brew --prefix llvm)/bin/clang" \
-DCMAKE_CXX_COMPILER="$(brew --prefix llvm)/bin/clang++" \
-DCMAKE_EXE_LINKER_FLAGS="-fuse-ld=lld"
$ cmake --build build_fuzz
```


<details open>
<summary>
Getting the following error instead on the last step:
</summary>

```zsh
[ 99%] Building CXX object
...
πŸ’¬ sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2447930605)
After cluster mempool, mini_miner should be modified anyway (and I think this whole MiniMinerMempoolEntry object will disappear, see for example my draft [here](https://github.com/bitcoin/bitcoin/commit/d24f02699ec4af215a4ffae3a2ae2457f71bf93f#diff-5dfb364874cfa42dd98492c3c3c1b07aafbe73ef7ecd895246d2fea00499fd6c)). So I don't think it's worth re-implementing this interface in this PR.
πŸ’¬ fanquake commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3426285363)
Yea. Still failing for me with Podman (5.6.2) on my Fedora box.
πŸ’¬ sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2447949351)
Can you clarify what you have in mind here?
πŸ’¬ hebasto commented on pull request "refactor: throw `std::string_view` instead of `const char*` in constexpr/consteval functions":
(https://github.com/bitcoin/bitcoin/pull/33569#issuecomment-3426298790)
> Here are the recent updates from the upstream projects:
>
> 1. Clang has [merged](https://github.com/llvm/llvm-project/commit/10be254587da24d56e2c6817b382beaca612b6c3) the fix and [backported](https://github.com/llvm/llvm-project/commit/570c4c9443387b756ed3e4cb94ca708841f2472a) it to 21.x.

Available in the recent release: https://github.com/llvm/llvm-project/releases/tag/llvmorg-21.1.4.
πŸ’¬ ajtowns commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#issuecomment-3426393945)
I don't think it makes much sense to split this out; it doesn't reduce the size of the parent PR meaningfully, and renaming a function doesn't really seem like it justifies a PR on its own.
πŸ’¬ maflcko commented on pull request "doc: add AGENTS.md":
(https://github.com/bitcoin/bitcoin/pull/33662#issuecomment-3426413950)
> which risks unintentional disclosure.
>
> Realistically, though, no one’s going to sift through a wall of LLM-generated β€œvibe” commits to check disclosure tags.

I'd say this is one of the use-cases to cover. If someone does not want to disclose their vibe-coding, but they fail to read their own submission of code (and the corresponding commit messages), the error is on their part. The requirement for pull request authors to understand their own changes is documented in essence in the pre
...
πŸ’¬ kanzure commented on pull request "doc: add AGENTS.md":
(https://github.com/bitcoin/bitcoin/pull/33662#issuecomment-3426491048)
One of the norms ought to be minimizing the waste of time of the project's
contributors. Whether such a waste is achieved by LLM (or not) does not
matter. Using autogenerated code is not itself negligent or malicious, just
like using any other tools could theoretically be productive.
πŸ’¬ maflcko commented on issue "Unable to fuzz in local on MacOS 15.4.1":
(https://github.com/bitcoin/bitcoin/issues/33667#issuecomment-3426502657)
There are some prior issues/pulls/discussions:

* https://github.com/bitcoin/bitcoin/pull/32084
* https://github.com/bitcoin/bitcoin/issues/32089

Though, this one seems to be a new link-time error.
πŸ’¬ kanzure commented on pull request "doc: add AGENTS.md":
(https://github.com/bitcoin/bitcoin/pull/33662#issuecomment-3426538091)
One other concern I have is that this is essentially a change that is meant
to be subversive to a developer's tools or development environment. In
part, it is justified as a defense against the waste of other project
contributor's time. I see the logic there. Indeed this is not far from a
recommending and providing a default linter config, or a default commit
message format recommendation. However there should be some thought put
into the subject of where the limit is with regards to other
possi
...
πŸ’¬ maflcko commented on pull request "doc: add AGENTS.md":
(https://github.com/bitcoin/bitcoin/pull/33662#issuecomment-3426568442)
> However there should be some thought put into the subject of where the limit is with regards to other possibly-hostile prompt injection

I'd say using agents to amend the prompt is well-understood and well-docuemented. I think the change here is harmless and mildly useful. Though, if it doesn't work in the future, it can be removed again and it is probably not worth to bend over backwards to achieve undocumented prompt injection.
πŸ’¬ maflcko commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3426586630)
From the output, I think there may be several unit tests affected, and they should probably be fixed, even if the GHA CI does not catch this issue.

Also, I tried on a fresh Ubuntu VM with a fresh user account (not root) and the issue persists. So I think the issue generally uncovers via podman.