Skip to content
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

[main] Crypto extension wrappers and data types for C++ #52

Merged
merged 7 commits into from
Nov 7, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
246 changes: 244 additions & 2 deletions libraries/eosiolib/core/eosio/crypto_ext.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/
#pragma once

#include "check.hpp"
#include "fixed_bytes.hpp"
#include "varint.hpp"
#include "serialize.hpp"
Expand Down Expand Up @@ -48,14 +49,182 @@ namespace eosio {
return dg;
}
}

/**
* @defgroup crypto Crypto
* @ingroup core
* @brief Defines API for calculating and checking hashes which
* require activating crypto protocol feature
*/

/**
* Abstracts G1 and G2 points. Ensures sizes of x and y are valid
*
* @ingroup crypto
*/
struct point_view {
/**
* Pointer to the x coordinate
*/
char* x;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this a view, we should make these const char*

Copy link
Contributor

Choose a reason for hiding this comment

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

Because you need to be able to return a point_view in some of the functions, I suggest that we have point_view and point. Where point holds std::vectors for x and y so we don't need to worry about memory issues.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for your suggestions! I was struggling on how to mutate point_view to return a result.


/**
* Pointer to the y coordinate
*/
char* y;

/**
* Number of bytes in each of x and y
*/
uint32_t size;

/**
* Construct a point given x and y
*
* @param x_ - The x coordinate, a vector of chars
* @param y_ - The y coordinate, a vector of chars
*/
point_view(std::vector<char>& x_, std::vector<char>& y_)
:x(x_.data()), y(y_.data()), size(x_.size())
{
eosio::check( x_.size() == y_.size(), "x's size must be equal to y's" );
};

/**
* Construct a point given a serialized point
*
* @param p - The serialized point
*/
point_view(std::vector<char>& p)
:x(p.data()), y(p.data() + p.size()/2), size(p.size()/2)
{
};

/**
* Returns packed x and y
*/
std::vector<char> packed() const {
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless there are some recent changes that I'm not aware of, we are using non-class member functions like:
eosio::pack and eosio::unpack all over the code. So I suggest to adhere to those standards.

Copy link
Member Author

@linh2931 linh2931 Oct 22, 2022

Choose a reason for hiding this comment

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

The name here is misleading. The intention was to provide a way to serialize x and y only, and populate a G1 or G2 point from a serialized form. Will change the names.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, got you. Anyway it looks like some non-standard approach for serialization. There is CDT_REFLECT macro in the eosiolib, have you considered it? it looks like a good approach for your purpose. it could be a bit tricky to use in your case but that way we can stick to standard approach. please let me know what do you think.

std::vector<char> x_and_y( x, x + size );
x_and_y.insert( x_and_y.end(), y, y + size );
return x_and_y;
}

/**
* Unpacks a seralized point and populates current point
*
* @param packed - The source serialized point
*/
void unpack(const std::vector<char>& packed) {
eosio::check( packed.size() == 2 * size, "size of packed point must be equal to x's size" );
std::memcpy(x, packed.data(), size);
std::memcpy(y, packed.data() + size, size);
}
};

static constexpr size_t g1_coordinate_size = 32;
static constexpr size_t g2_coordinate_size = 64;
static constexpr size_t blake2f_result_size = 64;

/**
* Abstracts G1 point. Ensures sizes of x and y are 32
*
* @ingroup crypto
*/
struct g1_view : public point_view {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would just make these templated and add a typedefs.

So, something like

template <std::size_t Size = 32>
struct point_view {
   point_view(const std::vector<char>& x, const std::vector<char>& y) {
      >> The other constructor code here from above <<
      eosio::check ( size == Size, "point size must match");
   }
   
   .
   .
   .

   using g1_view = point_view<g1_coordinate_size>;
   using g2_view = point_view<g2_coordinate_size>;

Copy link
Member Author

Choose a reason for hiding this comment

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

Will change. Thanks.

/**
* Construct a G1 point given x and y
*
* @param x_ - The x coordinate, a vector of chars
* @param y_ - The y coordinate, a vector of chars
*/
g1_view(std::vector<char>& x_, std::vector<char>& y_)
Copy link

Choose a reason for hiding this comment

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

I would suggest using std::span<char> instead of std::vector<char> so other containers can be used (e.g. compile-time defined std::array<char>) or offsets into binary data.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for your suggestion here and in the associated issue. std::span is in C++20 while we still need to support C++17 at the moment.

Copy link

Choose a reason for hiding this comment

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

Oh sorry I didn't realize support for C++17 is still required.

Ok I guess this would be proper for separate issue/PR, but how about creating a new span like type for C++17 (internal namespace) and exposing only 'bytes view' like span alias to cdt? When support for C++17 is dropped in the future only this internal span implementation has to be removed but the alias would stay intact without breaking api.

Just for reference, I made one simple type imitating span for such cases: https://github.com/ZeroPass/antelope.ck/blob/1bf203db28d053e9d5b171fb92f27c44b94acdc4/include/eosiock/span.hpp

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of the 3 separate types these can be collapsed in to point_view and have point_view become templates.

template <std::size_t Size = 32>
struct point_view

Then,

using g1_view = point_view<g1_coordinate_size>;
using g2_view = point_view<g2_coordinate_size>;

: point_view(x_, y_)
{
eosio::check( size == g1_coordinate_size, "G1 coordinate size must be 32" );
}

/**
* Construct a point given a serialized point
*
* @param p - The serialized point
*/
g1_view(std::vector<char>& p)
: point_view(p)
{
eosio::check( p.size() == g1_coordinate_size * 2, "G1 serialized size must be 64" );
};
};

/**
* Abstracts G2 point. Ensures sizes of x and y are 64
*
* @ingroup crypto
*/
struct g2_view : public point_view {
/**
* Construct a G2 point given x and y
*
* @param x_ - The x coordinate, a vector of chars
* @param y_ - The y coordinate, a vector of chars
*/
g2_view(std::vector<char>& x_, std::vector<char>& y_)
: point_view(x_, y_)
{
eosio::check( size == g2_coordinate_size, "G2 coordinate size must be 64" );
}

/**
* Construct a point given a serialized point
*
* @param p - The serialized point
*/
g2_view(std::vector<char>& p)
: point_view(p)
{
eosio::check( p.size() == g2_coordinate_size * 2, "G2 serialized size must be 128" );
};
};

/**
* Abstracts big integer.
*
* @ingroup crypto
*/
struct bigint {
/**
* Construct a bigint given a vector of chars (bytes)
*
Copy link
Contributor

Choose a reason for hiding this comment

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

Given our uses of bigint at this point, I would suggest we just go with an std::vector or a typedef.

So,

using bigint = std::vector<char>;

Copy link

Choose a reason for hiding this comment

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

I think std::vector<char> would be better aliased as bytes since it could be used in other non-crypto related cases. Also, since ABI already defines bytes type for std::vector<char> I believe such alias would also improve code readability.

Copy link
Member Author

Choose a reason for hiding this comment

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

Did you mean

using bytes = std::vector<char>;
using bigint = bytes;

?

Copy link

Choose a reason for hiding this comment

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

I guess you could abstract it that way. Tho in this case, the bigint alias just brings another abstraction layer with no additional functionality and I don't know if such abstraction is ok in terms of code readability and usage (i.e. 2 layers bigint -> bytes to know it's an alias for std::vector<char>). If it were up to me I'd avoid bigint alias.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. Two layers of abstraction is not good. Keep bigint as is for future extensions.

* @param s - The source bytes
*/
bigint(std::vector<char>& s)
:data(s.data()), size(s.size())
{
};

char* data;
uint32_t size;
};
Copy link

Choose a reason for hiding this comment

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

Instead of introducing new type I suggest making an alias for std::span<char> aka bytes_view.


/**
* Addition operation on the elliptic curve `alt_bn128`
*
* @ingroup crypto
* @param op1 - operand 1
* @param op2 - operand 2
* @param result - result of the addition operation
* @return -1 if there is an error otherwise 0
*/
inline int32_t alt_bn128_add( const g1_view& op1, const g1_view& op2, g1_view& result) {
Copy link
Contributor

Choose a reason for hiding this comment

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

For these we can check the internal return value and assert. And return a "result" from this instead of making it an in/out param.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will change.

auto op_1 = op1.packed();
auto op_2 = op2.packed();
std::vector<char> rslt(2*result.size); // buffer storing x and y
auto ret = internal_use_do_not_use::alt_bn128_add( op_1.data(), op_1.size(), op_2.data(), op_2.size(), rslt.data(), rslt.size());
if ( ret == 0 ) {
result.unpack(rslt); // unpack rslt into result
}
return ret;
}

/**
* Addition operation on the elliptic curve `alt_bn128`
*
Expand All @@ -72,6 +241,25 @@ namespace eosio {
return internal_use_do_not_use::alt_bn128_add( op1, op1_len, op2, op2_len, result, result_len);
}

/**
* Scalar multiplication operation on the elliptic curve `alt_bn128`
*
* @ingroup crypto
* @param g1 - G1 point
* @param scalar - scalar factor
* @param result - result of the scalar multiplication operation
* @return -1 if there is an error otherwise 0
*/
inline int32_t alt_bn128_mul( const g1_view& g1, const bigint& scalar, g1_view& result) {
auto g1_bin = g1.packed();
std::vector<char> rslt(2*result.size);
auto ret = internal_use_do_not_use::alt_bn128_mul( g1_bin.data(), g1_bin.size(), scalar.data, scalar.size, rslt.data(), rslt.size());
if ( ret == 0 ) {
result.unpack(rslt);
}
return ret;
}

/**
* Scalar multiplication operation on the elliptic curve `alt_bn128`
*
Expand All @@ -88,19 +276,54 @@ namespace eosio {
return internal_use_do_not_use::alt_bn128_mul( g1, g1_len, scalar, scalar_len, result, result_len );
}

/**
* Optimal-Ate pairing check elliptic curve `alt_bn128`
*
* @ingroup crypto
* @param pairs - g1 and g2 pairs
* @return -1 if there is an error, 1 if false and 0 if true and successful
*/
inline int32_t alt_bn128_pair( const std::vector<std::pair<g1_view, g2_view>>& pairs ) {
std::vector<char> g1_g2_pairs;
for ( const auto& pair: pairs ) {
auto g1_bin = pair.first.packed();
auto g2_bin = pair.second.packed();
g1_g2_pairs.insert( g1_g2_pairs.end(), g1_bin.begin(), g1_bin.end() );
g1_g2_pairs.insert( g1_g2_pairs.end(), g2_bin.begin(), g2_bin.end() );
}
return internal_use_do_not_use::alt_bn128_pair( g1_g2_pairs.data(), g1_g2_pairs.size() );
}

/**
* Optimal-Ate pairing check elliptic curve `alt_bn128`
*
* @ingroup crypto
* @param pairs - g1 and g2 pairs
* @param pairs_len - size of pairs
* @param result - result of the addition operation
* @return -1 if there is an error, 1 if false and 0 if true and successful
*/
inline int32_t alt_bn128_pair( const char* pairs, uint32_t pairs_len ) {
return internal_use_do_not_use::alt_bn128_pair( pairs, pairs_len );
}

/**
* Big integer modular exponentiation
* returns an output ( BASE^EXP ) % MOD
*
* @ingroup crypto
* @param base - base of the exponentiation (BASE)
* @param exp - exponent to raise to that power (EXP)
* @param mod - modulus (MOD)
* @param result - result of the modular exponentiation
* @return -1 if there is an error otherwise 0
*/

inline int32_t mod_exp( const bigint& base, const bigint& exp, const bigint& mod, bigint& result) {
eosio::check( result.size >= mod.size, "mod_exp result parameter's size must be >= mod's size" );
auto ret = internal_use_do_not_use::mod_exp( base.data, base.size, exp.data, exp.size, mod.data, mod.size, result.data, result.size);
return ret;
}

/**
* Big integer modular exponentiation
* returns an output ( BASE^EXP ) % MOD
Expand All @@ -121,6 +344,25 @@ namespace eosio {
return internal_use_do_not_use::mod_exp( base, base_len, exp, exp_len, mod, mod_len, result, result_len);
}

/**
* BLAKE2 compression function "F"
* https://eips.ethereum.org/EIPS/eip-152
*
* @ingroup crypto
* @param rounds - the number of rounds
* @param state - state vector
* @param msg - message block vector
* @param t0_offset - offset counters
* @param t1_offset - offset counters
* @param final - final block flag
* @param result - the result of the compression
* @return -1 if there is an error otherwise 0
*/
int32_t blake2_f( uint32_t rounds, const std::vector<char>& state, const std::vector<char>& msg, const std::vector<char>& t0_offset, const std::vector<char>& t1_offset, bool final, std::vector<char>& result) {
Copy link

Choose a reason for hiding this comment

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

I suggest using std::span based 'bytes view' type here as well for the inputs instead of const std::vector<char>.

eosio::check( result.size() >= blake2f_result_size, "blake2_f result parameter's size must be >= 64" );
return internal_use_do_not_use::blake2_f( rounds, state.data(), state.size(), msg.data(), msg.size(), t0_offset.data(), t0_offset.size(), t1_offset.data(), t1_offset.size(), final, result.data(), result.size());
}

/**
* BLAKE2 compression function "F"
* https://eips.ethereum.org/EIPS/eip-152
Expand Down
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ endmacro()
add_unit_test( asset_tests )
add_unit_test( binary_extension_tests )
add_unit_test( crypto_tests )
add_unit_test( crypto_ext_tests )
add_unit_test( datastream_tests )
add_unit_test( fixed_bytes_tests )
add_unit_test( name_tests )
Expand Down
Loading