Mul 1212 golos signing with private key #76
Conversation
PashaKlybik
commented
Apr 3, 2018
PashaKlybik
commented
- add implementatino golos private_key::sign
- add test to verify golos signature
- add method secp256k1_ecdsa_sign_compact to libsecp256k1
| return nullptr; | ||
| auto data_hash = do_hash<SHA2, 256>(data); | ||
|
|
||
| std::array<unsigned char,65> _result; |
There was a problem hiding this comment.
Please make a named constant from 65, something like GOLOS_SIGNATURE_SIZE, and add a typedef for `typedef std::array<unsigned char, GOLOS_SIGNATURE_SIZE> GolosSignatureType;
|
|
||
| } | ||
|
|
||
| bool is_canonical( const std::array<unsigned char,65>& c ) { |
There was a problem hiding this comment.
Please rename that function to something like is_canonical_signature, make a named constant form 0x80 with some reasonable name.
Also there should be a comment describing what a canonical signature is and link to some sort of documentation.
| && !(c[33] == 0 && !(c[34] & 0x80)); | ||
| } | ||
|
|
||
| static int extended_nonce_function( unsigned char *nonce32, const unsigned char *msg32, |
There was a problem hiding this comment.
Please add a comment above, that this extended nonce function is based on Golos implementation.
| ) { | ||
| unsigned int* extra = (unsigned int*) data; | ||
| (*extra)++; | ||
| return secp256k1_nonce_function_default( nonce32, msg32, key32, nullptr, nullptr, *extra); |
There was a problem hiding this comment.
Please delete extra whitespace.
| return nullptr; | ||
| auto data_hash = do_hash<SHA2, 256>(data); | ||
|
|
||
| std::array<unsigned char,65> _result; |
There was a problem hiding this comment.
Poor naming, please rename to 'result'
| _result.data(), _result.size(), | ||
| reset_sp(result))); | ||
|
|
||
| return result; |
There was a problem hiding this comment.
You could just do:
return make_clone(as_binary_data(result));
multy_test/test_golos_account.cpp
Outdated
| make_account(BLOCKCHAIN_GOLOS, "5Hq2ZdSGZokcgLoNxNGL5kHKi4e3kUUCqorgFK6T6ka7KtSvYLj", reset_sp(account)); | ||
| PrivateKeyPtr prv_key = account->get_private_key(); | ||
| BinaryDataPtr signature = prv_key->sign(as_binary_data(test_utility::from_hex("782a3039b478c839e4cb0c941ff4eaeb7df40bdd68bd441afd444b9da763de1269466c1944189" | ||
| "fb3bb5a0102096d756c7479746573740b70617368616b6c7962696b010000000000000003474f" |
There was a problem hiding this comment.
Indent is off, we use 8 spaces for continuation lines per level.
Also, please add a comment describing what this huge HEX string is.
PashaKlybik
force-pushed
the
MUL-1212-GOLOS-signing-with-private-key
branch
from
4c1a1ff
to
864f2a0
Compare
|
|
||
| } | ||
|
|
||
| // This method check that signature will be certain format |
There was a problem hiding this comment.
Check that signature is in canonical format
For details please see: bitshares/bitshares1-core#1129
| @@ -41,6 +50,29 @@ uint32_t get_chain_index(BlockchainType blockchain_type) | |||
| return CHAIN_INDEX_TEST; | |||
| } | |||
| return blockchain_type.blockchain; | |||
|
|
|||
There was a problem hiding this comment.
Please remove those empty lines
| do | ||
| { | ||
| secp256k1_ecdsa_sign_compact( secp_ctx(), data_hash.data(), result.begin() + 1, | ||
| (unsigned char*) m_data.data(), extended_nonce_function, &counter, &recovery_id); |
multy_test/test_golos_account.cpp
Outdated
| PrivateKeyPtr prv_key = account->get_private_key(); | ||
| // data to sign golos private key | ||
| BinaryDataPtr signature = prv_key->sign(as_binary_data(from_hex("782a3039b478c839e4cb0c941ff4eaeb7df40bdd68bd441afd444b9da763de1269466c1944189" | ||
| "fb3bb5a0102096d756c7479746573740b70617368616b6c7962696b010000000000000003474f4c4f5300000000"))); |
There was a problem hiding this comment.
to much leading whitespace here too
multy_test/test_golos_account.cpp
Outdated
| "fb3bb5a0102096d756c7479746573740b70617368616b6c7962696b010000000000000003474f4c4f5300000000"))); | ||
| // signature generated form golos-core | ||
| ASSERT_EQ(as_binary_data(from_hex( | ||
| "1f2e6bb028760bacddd31dc9772e63240fd297ee2f9fcd29f3605aeb79f774fa4b7d1b4e6dc4a1cd6fd2d4e08b2ea2758680d6a5b1e49664522f391ff949b70018")), |
| * Out: sig: pointer to a 64-byte array where the signature will be placed (cannot be NULL) | ||
| * In case 0 is returned, the returned signature length will be zero. | ||
| * recid: pointer to an int, which will be updated to contain the recovery id (can be NULL) | ||
| */ |
There was a problem hiding this comment.
Please add a link to this fork github
| int overflow = 0; | ||
| unsigned int count = 0; | ||
| VERIFY_CHECK(ctx != NULL); | ||
| ARG_CHECK(secp256k1_ecmult_gen_context_is_built(&ctx->ecmult_gen_ctx)); //TODO !!!!!!!!!! Look this function |
There was a problem hiding this comment.
Could you put a comment with brief explanation of why there is such TODO?
PashaKlybik
force-pushed
the
MUL-1212-GOLOS-signing-with-private-key
branch
2 times, most recently
from
619091b
to
0debf47
Compare
* add implementatino golos private_key::sign * add test to verify golos signature
PashaKlybik
force-pushed
the
MUL-1212-GOLOS-signing-with-private-key
branch
from
0debf47
to
6ab6969
Compare
