💬 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

💬 ariard commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1898959237)
> It's easy for Alice to make the revoked commitment confirm, she can use any output of that transaction to CPFP and she doesn't need to add any of her wallet inputs, she can just use the channel funds for that.

No you can’t spend in-mempool htlc outputs 1 CSV post-anchor. Assuming no 1 CSV, yes in my scenario. However attacker can construct revoked or valid pinning state to have the 483 outputs as inbound HTLC to inflate. Alice no preimage to spend them. Only anchor output available and liqu
...

💬 maflcko commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457806226)
why is USER needed?