Add Keys package #54
Add Keys package #54
Conversation
|
As we discussed, rebasing on #55 will remove the need for redefining all the |
keys/signer_secp256k1.go
Outdated
| } | ||
| privKeyBytes := s.KeyPair.PrivateKey.Bytes | ||
|
|
||
| if payload.SignatureType != sigType { |
There was a problem hiding this comment.
this will error if a sigType is passed in when payload.SignatureType is not populated. You should use the same logic as here: https://github.com/coinbase/rosetta-sdk-go/pull/54/files#diff-e27a53bd15dfa9595612a921778dd566R43-R45
keys/keys_test.go
Outdated
| assert.Equal(t, keypair.PrivateKey.CurveType, curve) | ||
|
|
||
| privKeyJSON, _ := json.Marshal(keypair.PrivateKey) | ||
| var privKey PrivateKey |
There was a problem hiding this comment.
just to make sure hex marshaling is working, I'd add this test for both the secp256k1 and edwards25519 keypair:
| var privKey PrivateKey | |
| // Simple Hex Check | |
| simpleType := struct { | |
| HexBytes string `json:"hex_bytes"` | |
| }{} | |
| err = json.Unmarshal(j, &simpleType) | |
| assert.NoError(t, err) | |
| b, err := hex.DecodeString(simpleType.HexBytes) | |
| assert.NoError(t, err) | |
| assert.Equal(t, keypair.PrivateKey.Bytes, b) |
keys/keys_test.go
Outdated
| var privKey PrivateKey | ||
| err = json.Unmarshal(privKeyJSON, &privKey) | ||
| assert.NoError(t, err) | ||
| assert.Equal(t, privKey.Bytes, keypair.PrivateKey.Bytes) |
There was a problem hiding this comment.
nit: expected should go first:
| assert.Equal(t, privKey.Bytes, keypair.PrivateKey.Bytes) | |
| assert.Equal(t, keypair.PrivateKey.Bytes, privKey.Bytes) |
keys/keys_test.go
Outdated
| var privKey PrivateKey | ||
| err = json.Unmarshal(privKeyJSON, &privKey) | ||
| assert.NoError(t, err) | ||
| assert.Equal(t, privKey.Bytes, keypair.PrivateKey.Bytes) |
There was a problem hiding this comment.
nit: expected should go first:
| assert.Equal(t, privKey.Bytes, keypair.PrivateKey.Bytes) | |
| assert.Equal(t, keypair.PrivateKey.Bytes, privKey.Bytes) |
keys/keys.go
Outdated
| "github.com/btcsuite/btcd/btcec" | ||
| ) | ||
|
|
||
| // Currently all suppported privkeys are 32-bytes |
There was a problem hiding this comment.
nit: comment should start with PrivKeyBytesLen
|
Make sure to add the You should also add a line item to the |
keys/keys_test.go
Outdated
| assert.Equal(t, privKey.Bytes, keypair.PrivateKey.Bytes) | ||
| } | ||
|
|
||
| func TestGenerateKeypairEd25519(t *testing.T) { |
There was a problem hiding this comment.
nit: this should be TestGenerateKeypairEdwards25519
| assert.NoError(t, err) | ||
|
|
||
| // Simple Hex Check | ||
| simpleType := struct { |
keys/signer_secp256k1.go
Outdated
| if err != nil { | ||
| return nil, fmt.Errorf("sign: unable to sign. %w", err) | ||
| } | ||
| sig = sig[:64] |
There was a problem hiding this comment.
nit: use a constant for all references of 64
Motivation
Added a
keyspackage to rosetta-sdk-go, which includes keygen, verify, and sign methods for Secp256k1 and Edwards25519 curves. This will be primarily used within tests for implementations of the Construction API, where we can sign & verify the constructed transactions.Solution
Open questions