Simpilfy user id derivation from key #475
Conversation
smallhive
requested review from
roman-khimov,
cthulhu-rider and
carpawell
as code owners
| @@ -354,7 +354,8 @@ func (b Token) SigningKeyBytes() []byte { | |||
| // Returns zero user.ID if Token is unsigned or key has incorrect format. | |||
| // | |||
| // See also SigningKeyBytes. | |||
| func ResolveIssuer(b Token) (usr user.ID) { | |||
| func (b Token) ResolveIssuer() user.ID { | |||
There was a problem hiding this comment.
That's where neofsid or anything like that fails miserably, btw.
There was a problem hiding this comment.
Should I drop this commit?
There was a problem hiding this comment.
I'm not sure about keeping user.IDFromKey. We can try moving its code to bearer thus removing it from public API. If anyone's got a keys.PublicKey, he can do id.SetScriptHash(pk.GetScriptHash()). If he's got bytes, then pk.DecodeBytes(key) (and associated error handling) is not that hard as well.
There was a problem hiding this comment.
Moved IDFromKey to bearer
smallhive
force-pushed
the
428-simpilfy-user-id-derivation-from-key
branch
2 times, most recently
from
7afb608
to
ecef897
Compare
| // ECDSA with SHA-256 hashing (RFC 6979). Provides [Signer] interface. | ||
| // | ||
| // Instances SHOULD be initialized with [NewSignerRFC6979] or [NewSignerRFC6979WithID]. | ||
| type SignerRFC6979 struct { |
There was a problem hiding this comment.
they are literally named the same (user.SignerRFC6979 and neofsecdsa.SignerRFC6979). was it discussed too? @cthulhu-rider, @roman-khimov
There was a problem hiding this comment.
well, one is the "original" while the other one is a wrapper over the first one. also user.Signer* confuses me (but *ecdsa.Signer* is clear). also, it reminds me of the problem of reading some neofs-node code when it is not clear what struct is here: sdk or api-go
There was a problem hiding this comment.
as i understand, you have already discussed it so i am only able to suggest naming with User, Wrapper, WithID. all of them are bad if the whole concept is taken as the only way
There was a problem hiding this comment.
user.UserSmth is not a good thing. SignerRFC6979WithID seems to be longer than most people would like it to be. Wrapper is meaningless. If we don't have any better options then we can go with the current one.
|
Conflict. |
Signed-off-by: Evgenii Baidakov <evgenii@nspcc.io>
Signed-off-by: Evgenii Baidakov <evgenii@nspcc.io>
Signed-off-by: Evgenii Baidakov <evgenii@nspcc.io>
Signed-off-by: Evgenii Baidakov <evgenii@nspcc.io>
smallhive
force-pushed
the
428-simpilfy-user-id-derivation-from-key
branch
from
c74f989
to
8be458e
Compare
Rebased |
closes #428