💬 maflcko commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457728802)
For reference, I said to use `CONTAINER_NAME` in the pull request you closed, which should be a more complete and correct fix?

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457730915)
Oh, I assume it that explicit install was there for a reason, the comment "Unconditionally install git (used in fingerprint_script)." is a bit vague.

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457732083)
That's indeed nicer, if the line can't be dropped entirely.

💬 petertodd commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1898859416)
> I may be missing something here, can you describe in more details how _anyone_ could turn them into large pinning txs? They do require signatures from _both_ participants, and one of them is going to be `SIGHASH_ALL`.

Thanks. Yes, that's a mistake on my part and you are correct. I hide my comment as "outdated" to reflect that.

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457735631)
I missed that.

But the file is owned by the user. In a multi-user setup each user builds its own containers. So this would break afaik. So random UUID could work though.

💬 real-or-random commented on pull request "build: Pass sanitize flags to instrument `libsecp256k1` code":
(https://github.com/bitcoin/bitcoin/pull/28875#discussion_r1457736462)
I see. I think it still makes sense to move the line up so that it appears below `# Suppressions in dependencies that are developed outside this repository.`

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457736806)
> Could be a separate pull request, because this is a bug fix?

I might split it out if this PR takes too long on its own.

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457739319)
Preserving @maflcko's comment on the previous PR:

> unrelated: Generally I'd find it better if we get rid of the commit range completely. For example, when running the whitespace linter, it seems easier to just lint all code at `HEAD`, instead of trying to guess which code was modified and only lint that.

💬 maflcko commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457739745)
When running locally, usually only one user exists, and someone may be interested in running *different* tasks for the same (local) code.

Running the *same* task in parallel is indeed not supported right now.

💬 maflcko commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457741094)
It can't be dropped, because then the lint CI would fail, but you are welcome to remove it and try

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457741145)
It would be nicer to fix the actual problem, but I don't understand what this job is doing.

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457748096)
Ok, so maybe `/tmp/env-$USER-$CONTAINER_NAME`

👍 brunoerg approved a pull request: "test: Remove all-lint.py script"
(https://github.com/bitcoin/bitcoin/pull/29228#pullrequestreview-1830193328)
utACK fa2b95cf3f5148d27a8fd4fb3763ca1fc139bdd9

💬 vasild commented on pull request "Avoid returning references to mutex guarded members":
(https://github.com/bitcoin/bitcoin/pull/28774#issuecomment-1898900596)
`7a8e5310e7...32a9f13cb8`: rebase due to conflicts and change the approach to use a callback, see https://github.com/bitcoin/bitcoin/pull/28774#discussion_r1456489383.

💬 pablomartin4btc commented on pull request "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1457765120)
done, thanks!

💬 vasild commented on pull request "Avoid returning references to mutex guarded members":
(https://github.com/bitcoin/bitcoin/pull/28774#discussion_r1457766636)
Right, those methods were not very natural for the `WalletStorage` interface which contained a method to retrieve the encryption key. Adding a new interface seems a bit too complicated for this. Instead, I changed it to give the key to a callback function instead of returning the key to the caller.

💬 pablomartin4btc commented on pull request "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28670#issuecomment-1898907043)
Addressed @hebasto's [suggestion](https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1457712659) in order to fix the MSVC CI job.

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457771731)
@hebasto @ryanofsky any thoughts on how to make the `test-each-commit` task work for a scenario like in https://github.com/Sjors/bitcoin/pull/30 where the base branch is `fork/some-branch` and the PR branch is `fork/some-pr`?

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457773126)
Example of earlier failure: https://github.com/Sjors/bitcoin/actions/runs/7556559882/job/20573828392?pr=28#step:4:42

```
shell: /usr/bin/bash -e {0}
env:
DANGER_RUN_CI_ON_HOST: 1
CI_FAILFAST_TEST_LEAVE_DANGLING: 1
MAKEJOBS: -j10
MAX_COUNT: 6
FETCH_DEPTH: 8
Warning: you are leaving 1 commit behind, not connected to
any of your branches:

3b5c5d4 ci: add self-hosted runners for GitHub

If you want to keep it by creating a new branch, this may be a good time
...

💬 achow101 commented on pull request "test: Remove all-lint.py script":
(https://github.com/bitcoin/bitcoin/pull/29228#issuecomment-1898958526)
ACK fa2b95cf3f5148d27a8fd4fb3763ca1fc139bdd9