💬 maflcko commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457721848)
But that doesn't help if two tasks are run at the same time for the same user

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457728654)
I don't think you're supposed to do that. One user == one worker.

If we want to support multiple workers per user, we also have to randomise the container names, since you might be running CI for two pull requests at the same time.

💬 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`?