Bitcoin Core Github
43 subscribers
123K links
Download Telegram
πŸ’¬ 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.
πŸ‘ l0rinc approved a pull request: "ci: Only write docker build images to Cirrus cache"
(https://github.com/bitcoin/bitcoin/pull/33639#pullrequestreview-3360383974)
Concept ACK, I understand that you already have an ACK but please consider my suggestions
πŸ’¬ l0rinc commented on pull request "ci: Only write docker build images to Cirrus cache":
(https://github.com/bitcoin/bitcoin/pull/33639#discussion_r2448019150)
hmmm, not sure I get why we need this randomness here, is it to allow multiple runs without synchronization?
πŸ’¬ l0rinc commented on pull request "ci: Only write docker build images to Cirrus cache":
(https://github.com/bitcoin/bitcoin/pull/33639#discussion_r2448054647)
I assume that changing the `MAYBE_CPUSET` position is irrelevant, right?
πŸ’¬ l0rinc commented on pull request "ci: Only write docker build images to Cirrus cache":
(https://github.com/bitcoin/bitcoin/pull/33639#discussion_r2448072260)
nit: we were logging `CI_IMAGE_NAME_TAG` before (just resolve if that's intentional)
πŸ’¬ l0rinc commented on pull request "ci: Only write docker build images to Cirrus cache":
(https://github.com/bitcoin/bitcoin/pull/33639#discussion_r2448057503)
for symmetry can we inline + unpack this?
πŸ’¬ l0rinc commented on pull request "ci: Only write docker build images to Cirrus cache":
(https://github.com/bitcoin/bitcoin/pull/33639#discussion_r2448120770)
slightly tangential, but maybe there's room for optimizing the docker layers by putting the variable parts as the last layers, using ARGs over ENVs, combining multiple layers, pinning versions and paths and variables early, pruning old layers, multi-stage builds, etc.
πŸ’¬ l0rinc commented on pull request "ci: Only write docker build images to Cirrus cache":
(https://github.com/bitcoin/bitcoin/pull/33639#discussion_r2448149644)
I don't yet understand why we're pinning CPUs here, what problem is that solving that `--cpus=` wouldn't?
The comment is a bit misleading since my understanding is that `--cpuset-cpus` doesn't actually limit CPU usage.
πŸ’¬ l0rinc commented on pull request "ci: Only write docker build images to Cirrus cache":
(https://github.com/bitcoin/bitcoin/pull/33639#discussion_r2448113232)
Do we still need this? My understanding is that we're not really using this - can we delete and add back when there's a well-defined usecase for it?

If we do, we might want to add `MAKEJOBS` guard as well since that is also needed
πŸ’¬ l0rinc commented on pull request "ci: Only write docker build images to Cirrus cache":
(https://github.com/bitcoin/bitcoin/pull/33639#discussion_r2448139543)
Is this related to the mentioned https://www.shellcheck.net/wiki/SC2086? Because we need split to handle quotes and escaping properly?
πŸ’¬ l0rinc commented on pull request "ci: Only write docker build images to Cirrus cache":
(https://github.com/bitcoin/bitcoin/pull/33639#discussion_r2448081592)
Can we inline `cmd_build`?
```python
run(["docker", "buildx", "build",
f"--file={os.environ['BASE_READ_ONLY_DIR']}/ci/test_imagefile",
f"--build-arg=CI_IMAGE_NAME_TAG={os.environ['CI_IMAGE_NAME_TAG']}",
f"--build-arg=FILE_ENV={os.environ['FILE_ENV']}",
f"--build-arg=BASE_ROOT_DIR={os.environ['BASE_ROOT_DIR']}",
f"--platform={os.environ['CI_IMAGE_PLATFORM']}",
f"--label={CI_IMAGE_LABEL}",
f"--tag={os.environ['CONTAINER_NAME']}",
*maybe_cpuset(),
*shlex.split(os
...
πŸ’¬ maflcko commented on pull request "ci: Only write docker build images to Cirrus cache":
(https://github.com/bitcoin/bitcoin/pull/33639#discussion_r2448342637)
Ah nice, changed all
πŸ’¬ maflcko commented on pull request "ci: Only write docker build images to Cirrus cache":
(https://github.com/bitcoin/bitcoin/pull/33639#discussion_r2448344025)
thx, took your doc comment
πŸ’¬ maflcko commented on pull request "ci: Only write docker build images to Cirrus cache":
(https://github.com/bitcoin/bitcoin/pull/33639#discussion_r2448344564)
(removed)
πŸ’¬ maflcko commented on pull request "ci: Only write docker build images to Cirrus cache":
(https://github.com/bitcoin/bitcoin/pull/33639#discussion_r2448344952)
Yes, exactly, see also the commit message :)