💬 hebasto commented on pull request "ci: A few fixes of `ccache` issues":
(https://github.com/bitcoin/bitcoin/pull/27084#issuecomment-1438441083)
The questionable commit has been dropped.

Ready to re-review.

📝 MarcoFalke opened a pull request: "Revert "[contrib] verify-commits: Add MarcoFalke fingerprint""
(https://github.com/bitcoin/bitcoin/pull/27135)
This reverts commit fa243293343eb964bfee5b91cc52b91f16232ab6.

The commit may be signed by my key, but I haven't checked it. Also, I haven't checked the new `contrib/verify-commits/trusted-git-root`.

💬 Sjors commented on pull request "Add Signet launch shortcut for Windows":
(https://github.com/bitcoin/bitcoin/pull/26334#issuecomment-1438484759)
I think I found the right incantation, using the high res PNG version as input:

```
convert src/qt/res/icons/bitcoin.png -modulate 100,87,119.4 -define icon:auto-resize="256,48,32,16" src/qt/res/icons/bitcoin_signet.ico
```

Will update screenshot and push the updated file after I finish my Guix build.

💬 willcl-ark commented on issue "Sync slow":
(https://github.com/bitcoin/bitcoin/issues/26063#issuecomment-1438496796)
@popkian, see [this previous comment](https://github.com/bitcoin/bitcoin/issues/26063#issuecomment-1243253449) on what you can do to help us diagnose the issue.

👍 hebasto approved a pull request: "Convert ArgsManager::GetDataDir to a read-only function"
(https://github.com/bitcoin/bitcoin/pull/27073)
ACK 0af17e161d751b15f90615800411f36e11b3fcde, tested on Ubuntu 22.04.

> Fixes https://github.com/bitcoin/bitcoin/issues/20070

Confirming the `bitcoin-cli` does not create a data directory and its subdirectories.

I've noticed another behavior change which looks good for me:
- master:
```
$ ./src/bitcoind rubbish
Error: Command line contains unexpected token 'rubbish', see bitcoind -h for a list of options.

$ tree ~/.bitcoin
/home/hebasto/.bitcoin
└── wallets

1 directory, 0 fi
...

💬 Sjors commented on pull request "Add Signet launch shortcut for Windows":
(https://github.com/bitcoin/bitcoin/pull/26334#discussion_r1113070519)
Forgot to add `test ` here, pushing fix.

💬 Sjors commented on pull request "Revert "[contrib] verify-commits: Add MarcoFalke fingerprint"":
(https://github.com/bitcoin/bitcoin/pull/27135#issuecomment-1438545680)
Commit signature for fab17f08e24f0db687dc25c5e10eb62293070048 looks good. I'm running a rebased version of #27058 against the proposed new trusted git root.

Calling the commit `Revert` both confusing and omits the rather critical bit about updating the trusted root.

💬 jamesob commented on pull request "OP_VAULT draft":
(https://github.com/bitcoin/bitcoin/pull/26857#discussion_r1113120278)
Good point. This has been done; see the `limit_recursion` parameter. Thanks to @john-moffett for outlining a particular test case to ensure that recursion is limited to a single call.

💬 jamesob commented on pull request "OP_VAULT draft":
(https://github.com/bitcoin/bitcoin/pull/26857#discussion_r1113121652)
Removed. Vault opcodes now only exist as OP_SUCCESSx overrides in tapscript.

💬 jamesob commented on pull request "OP_VAULT draft":
(https://github.com/bitcoin/bitcoin/pull/26857#discussion_r1113122540)
Fixed. OP_UNVAULT outputs (and recovery outputs) are now specified explicitly by an index on the witness stack. Thanks for this feedback.

📝 ccdle12 opened a pull request: "2023.02.21 ci prep"
(https://github.com/bitcoin/bitcoin/pull/27136)
null

✅ ccdle12 closed a pull request: "2023.02.21 ci prep"
(https://github.com/bitcoin/bitcoin/pull/27136)

👍 vasild approved a pull request: "p2p: return `CSubNet` in `LookupSubNet`"
(https://github.com/bitcoin/bitcoin/pull/26078)
ACK 9fb86661cf81e27131eb5f8d901397a25e2cd4b4

Thanks!

💬 Sjors commented on pull request "contrib: Improve verify-commits.py to work with maintainers leaving":
(https://github.com/bitcoin/bitcoin/pull/27058#issuecomment-1438609866)
For testing purposes, I tried rebasing the PR on master, removing @MarcoFalke's key without updating the trusted root and instead adding revsigs:

```
git log --format="%H %GK" --merges $(cat contrib/verify-commits/trusted-git-root)..master | grep -E "CE2B75697E69A548" | cut -c -40
```

It seems to ignore them though, because `contrib/verify-commits/verify-commits.py HEAD~4` fails: `No parent of 75f0e0b607cd7ff7afd56853eb34a2b285b22ad2 was signed with a trusted key!`. I checked that the fi
...

💬 MarcoFalke commented on pull request "Remove MarcoFalke fingerprint, update trusted-git-root":
(https://github.com/bitcoin/bitcoin/pull/27135#issuecomment-1438626295)
Thanks, changed title. Also happy to close this if someone wants to open a better alternative.

👍 MarcoFalke approved a pull request: "ci: A few fixes of `ccache` issues"
(https://github.com/bitcoin/bitcoin/pull/27084)
lgtm

💬 MarcoFalke commented on pull request "ci: A few fixes of `ccache` issues":
(https://github.com/bitcoin/bitcoin/pull/27084#discussion_r1113188750)
Isn't this already the default?

> true otherwise

https://cirrus-ci.org/guide/writing-tasks/#cache-instruction

💬 MarcoFalke commented on pull request "ci: A few fixes of `ccache` issues":
(https://github.com/bitcoin/bitcoin/pull/27084#discussion_r1113187573)
Isn't this already the default?

> By default the task name is used as a fingerprint value.

https://cirrus-ci.org/guide/writing-tasks/#cache-instruction

💬 Sjors commented on pull request "contrib: Improve verify-commits.py to work with maintainers leaving":
(https://github.com/bitcoin/bitcoin/pull/27058#issuecomment-1438641491)
Ah wait, I'm misunderstanding what `allow-revsig-commits` does. It merely allows a signature fron a revoked key. Not a key that we removed from the trusted list, but a revoked PGP key.

So the only way to verify earlier history is to check out the trusted root commit, copy the most recent `verify-commits.py` and run it again. That way it will use the trusted keys at the time.

I suggest we merge this and then later rethink how we want to handle changing maintainers.

tACK bb86887527d817ee2
...

💬 MarcoFalke commented on pull request "lint: enable E722 do not use bare except":
(https://github.com/bitcoin/bitcoin/pull/25867#issuecomment-1438643927)
Thanks. I've read https://www.flake8rules.com/rules/E722.html and checked that the changes implement the description.

lgtm ACK 61bb4e783b3acc62b121a228f6b14c2462e23315