Skip to content

contrib: Remove keys that are no longer used for merging #25197

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 25, 2022

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented May 24, 2022

See:

Also updated trusted-git-root to be right after meshcollider's last merge.

The latest similar change was #7713.

A related discussion on IRC:

12:28 <MarcoFalke> jonasschnelli: I was about to ask you whether you planned to remove your fingerprint from the "trusted-keys" for merging, but it looks like this will break verify-commits ...
12:31 <laanwj> you would also have a add all his merge commits to exceptions, i guess
12:32 <laanwj> or patch the script to allow different key for different ranges of commits
13:15 <jonasschnelli> MarcoFalke: I had no plan to remove my keyid,… would that make sense and how would you fix verify commits?
13:16 <jonasschnelli> Ideally, we should set en expiration date next to those keyid

Copy link
Member

@laanwj laanwj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM, however when running locally I run into this verification issue:

bitcoin$ contrib/verify-commits/verify-commits.py
Using verify-commits data from …

No parent of e658b0e49bee257e197dd8ca37ee3ffa73092d92 was signed with a trusted key!
Parents are:
commit 19e3e654293858141f39eef8fdae0badd224090f
Merge: 9b48b3ac42294410cb56e7da24a261fa69ddb503 915e34112b5a4c2ef391d7e3706603bcd6f62a2a
Author: MarcoFalke <falke.marco@gmail.com>
Date:   Fri Mar 26 17:30:18 2021 +0100

This seems to be an old expired key of Marco Falke (60B0B8A402FB386B24A039ACD2EA4850E7528B25). I don't think it's caused by this change.

@@ -1,8 +1,6 @@
71A3B16735405025D447E8F274810B012346C9A6
133EAC179436F14A5CF1B794860FEB804E669320
32EE5C4C3FA15CCADB46ABE529D4BCB6416F53EC
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pub   rsa4096/0x29D4BCB6416F53EC 2015-05-22 [SC] [expires: 2027-05-19]
      32EE5C4C3FA15CCADB46ABE529D4BCB6416F53EC
uid                   [ unknown] Jonas Schnelli <dev@jonasschnelli.ch>

B8B3F1C0E58C15DB6A81D30C3648A882F4316B9B
CA03882CB1FC067B5D3ACFE4D300116E1C875A3D
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pub   rsa4096/0xD300116E1C875A3D 2017-07-13 [SC] [expires: 2033-07-09]
      CA03882CB1FC067B5D3ACFE4D300116E1C875A3D
uid                   [ unknown] MeshCollider <dobsonsa68@gmail.com>

@fanquake
Copy link
Member

Concept ACK

@maflcko
Copy link
Member

maflcko commented May 25, 2022

Commit 19e3e65 should be buried far under the trusted root 577bd51, so I don't see how it could be considered even or cause a failure.

@laanwj
Copy link
Member

laanwj commented May 25, 2022

Commit 19e3e65 should be buried far under the trusted root 577bd51, so I don't see how it could be considered even or cause a failure.

Apparently I don't know how to use the script, then.
It also took almost a hour to run.

@laanwj
Copy link
Member

laanwj commented May 25, 2022

I just retried, it works now:

$ git fetch upstream pull/25197/head
From github.com:bitcoin/bitcoin
 * branch                                                                              refs/pull/25197/head -> FETCH_HEAD
$ git checkout FETCH_HEAD
Note: switching to 'FETCH_HEAD'.
…
HEAD is now at d4b3dc5b0a726cc4cc7a8467be43126e78f841cf contrib: Remove keys that are no longer used for merging
$ git reset --soft HEAD~1  # get rid of unsigned top-level commit, but keep changes in local tree
$ contrib/verify-commits/verify-commits.py
Using verify-commits data from /store/orion/projects/bitcoin/bitcoin/contrib/verify-commits
There is a valid path from "HEAD" to 577bd51a4b8de066466a445192c1c653872657e2 where all commits are signed!

ACK d4b3dc5

@laanwj laanwj merged commit b4f6869 into bitcoin:master May 25, 2022
@hebasto hebasto deleted the 220524-keys branch May 25, 2022 12:02
@bitcoin bitcoin locked and limited conversation to collaborators May 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants