chore: Audit crypto folder #11932
chore: Audit crypto folder #11932
Conversation
github-actions
bot
added
C:CLI
C:Keys
| @@ -100,13 +100,6 @@ type Keyring interface { | |||
| Migrator | |||
| } | |||
|
|
|||
| // UnsafeKeyring exposes unsafe operations such as unsafe unarmored export in | |||
| // addition to those that are made available by the Keyring interface. | |||
| type UnsafeKeyring interface { | |||
There was a problem hiding this comment.
The keyring has a lot of Unsafe* stuff. They are only used for exporting the private key. This is why I moved all Unsafe* types into client/keys/export.go, as private names
There was a problem hiding this comment.
If you think callers won't need these, then this is fine 👍
There was a problem hiding this comment.
Yes, that's the idea. If they wanted, they could still access them, via interface casting. But it's some more hurdles on purpose to access Unsafe methods.
I added a changelog entry
| Offline offline = 6; | ||
| } | ||
|
|
||
| // Item is a keyring item stored in a keyring backend. | ||
| // Local item | ||
| message Local { | ||
| google.protobuf.Any priv_key = 1; | ||
| string priv_key_type = 2; |
There was a problem hiding this comment.
I removed this 2nd field. For me it's redundant, as the Any field already contains the type, e.g. in JSON:
"@type": "cosmos.crypto.secp256k1.PrivKey"
| @@ -100,13 +100,6 @@ type Keyring interface { | |||
| Migrator | |||
| } | |||
|
|
|||
| // UnsafeKeyring exposes unsafe operations such as unsafe unarmored export in | |||
| // addition to those that are made available by the Keyring interface. | |||
| type UnsafeKeyring interface { | |||
There was a problem hiding this comment.
If you think callers won't need these, then this is fine 👍
…into am/crypto-audit
copy from tendermint you can skip
the changes come from a fork of Geth so we can skip, they are already on main nets |
Co-authored-by: Levi Schoen <levismschoen@gmail.com>
…into am/crypto-audit
Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>
Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>
tac0turtle
added
the
A:automerge
Codecov Report
@@ Coverage Diff @@
## main #11932 +/- ##
==========================================
- Coverage 65.98% 65.97% -0.01%
==========================================
Files 677 677
Lines 71329 71324 -5
==========================================
- Hits 47063 47056 -7
- Misses 21582 21583 +1
- Partials 2684 2685 +1
|
Description
ref: #11362
I did NOT review the following folders, as they contain cryptography which I don't think I'm competent enough to give a useful review:
crypto/xsalsa20symmetric(new in v046, ported from TM i think)crypto/keys/secp256k1(some new stuff in v046 too)Also performed some manual tests as part of #11939:
--multisigflag works with an address that's not in the keyring (see repro)Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change