oxen-io
/
oxen-core
mirror of https://github.com/oxen-io/oxen-core.git
2
1
Fork 0
Code Issues Projects Releases Wiki Activity

More string_view (mostly in wallet code) 

This started out with a few string_view simplifications, but unwinding
the string_view calls resulted in quite a few changes.

- use constexpr string_view instead of macros for constants

- use constexpr ints for other macros instead of constants (to be
consistent with the string_view constexprs); also eliminated a couple of
unused variables.

- convert amount parsing to string_view, and rewrite it with comments to
be far less confusing.  (Previously it was remove a "." from a string,
chopping 0s off the end then putting them back on which was hard to
follow).

- fixed some not-quite-right amount parsing unit tests

- replace a bunch of string code (including lots of code allocating new
strings with .substr on a string) with std::string_view.

- avoid using C functions for strings

- convert epee http uri conversion methods to string_view (and fix one
typoed func name "conver" -> "convert")

- Add a `tools::hex_to_type` that converts hex directly into a (simple)
type, with hex and length verification for the type, and use it to
eliminate a bunch of intermediate std::string conversions.

- Convert a bunch of error-prone raw pointer string appends into
more type-safe `tools::view_guts(val)` calls.  (In particular view_guts
always gets the sizeof() correct, while the individual call could easily
put the wrong type in the `sizeof()`).
This commit is contained in:
Jason Rhinelander 2020-06-23 16:50:20 -03:00
parent e7030ae708
commit f09857de2b
10 changed files with 404 additions and 402 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
contrib/epee/include/net/abstract_http_client.h
View File

@ -53,9 +53,8 @@ namespace net_utils
int get_index(const char *s, char c);
std::string hex_to_dec_2bytes(const char *s);
std::string convert(char val);
std::string conver_to_url_format(const std::string& uri);
std::string convert_from_url_format(const std::string& uri);
std::string convert_to_url_format_force_all(const std::string& uri);
std::string convert_to_url_format(std::string_view uri);
std::string convert_from_url_format(std::string_view uri);
namespace http
{

1
contrib/epee/include/wipeable_string.h
View File

@ -64,6 +64,7 @@ namespace epee
size_t length() const noexcept { return buffer.size(); }
bool empty() const noexcept { return buffer.empty(); }
void trim();
std::string_view view() const noexcept { return std::string_view{data(), size()}; }
void split(std::vector<wipeable_string> &fields) const;
std::optional<wipeable_string> parse_hexstr() const;
template<typename T> inline bool hex_to_pod(T &pod) const;

17
contrib/epee/src/abstract_http_client.cpp
View File

@ -78,7 +78,7 @@ namespace net_utils
return csRet;
}
//----------------------------------------------------------------------------------------------------
std::string conver_to_url_format(const std::string& uri)
std::string convert_to_url_format(std::string_view uri)
{
std::string result;
@ -95,7 +95,7 @@ namespace net_utils
return result;
}
//----------------------------------------------------------------------------------------------------
std::string convert_from_url_format(const std::string& uri)
std::string convert_from_url_format(std::string_view uri)
{
std::string result;
@ -104,7 +104,7 @@ namespace net_utils
{
if(uri[i] == '%' && i + 2 < uri.size())
{
result += hex_to_dec_2bytes(uri.c_str() + i + 1);
result += hex_to_dec_2bytes(uri.data() + i + 1);
i += 2;
}
else
@ -114,17 +114,6 @@ namespace net_utils
return result;
}
//----------------------------------------------------------------------------------------------------
std::string convert_to_url_format_force_all(const std::string& uri)
{
std::string result;
for(size_t i = 0; i!= uri.size(); i++)
{
result += convert(uri[i]);
}
return result;
}
namespace http
{

19
src/common/hex.h Normal file
View File

@ -0,0 +1,19 @@
#pragma once
#include <lokimq/hex.h>
#include <type_traits>
#include "span.h" // epee
namespace tools {
// Reads a hex string directly into a trivially copyable type T without performing any temporary
// allocation. Returns false if the given string is not hex or does not match T in length,
// otherwise copies directly into `x` and returns true.
template <typename T, typename = std::enable_if_t<
!std::is_const_v<T> && (std::is_trivially_copyable_v<T> || epee::is_byte_spannable<T>)
>>
bool hex_to_type(std::string_view hex, T& x) {
if (!lokimq::is_hex(hex) || hex.size() != 2*sizeof(T))
return false;
lokimq::to_hex(hex.begin(), hex.end(), reinterpret_cast<char*>(&x));
return true;
}
}

73
src/cryptonote_basic/cryptonote_format_utils.cpp
View File

@ -376,39 +376,68 @@ namespace cryptonote
return total;
}
//---------------------------------------------------------------
bool parse_amount(uint64_t& amount, const std::string& str_amount_)
bool parse_amount(uint64_t& amount, std::string_view str_amount)
{
std::string str_amount = str_amount_;
boost::algorithm::trim(str_amount);
tools::trim(str_amount);
size_t point_index = str_amount.find_first_of('.');
size_t fraction_size;
if (std::string::npos != point_index)
{
fraction_size = str_amount.size() - point_index - 1;
while (default_decimal_point < fraction_size && '0' == str_amount.back())
{
str_amount.erase(str_amount.size() - 1, 1);
--fraction_size;
}
if (default_decimal_point < fraction_size)
auto parts = tools::split(str_amount, "."sv);
if (parts.size() > 2)
return false; // 123.456.789 no thanks.
if (parts.size() == 2 && parts[1].empty())
parts.pop_back(); // allow "123." (treat it as as "123")
if (parts[0].find_first_not_of("0123456789"sv) != std::string::npos)
return false; // whole part contains non-digit
const unsigned int decimal_point = default_decimal_point; // to avoid needing a bunch of atomic reads below
if (parts[0].empty()) {
// Only allow an empty whole number part if there is a fractional part.
if (parts.size() == 1)
return false;
str_amount.erase(point_index, 1);
amount = 0;
}
else
{
fraction_size = 0;
if (!tools::parse_int(parts[0], amount))
return false;
// Scale up the number (e.g. 12 from "12.45") to atomic units.
//
// TODO: get rid of the user-configurable default_decimal_point nonsense and just multiply
// this value by the `COIN` constant.
for (size_t i = 0; i < decimal_point; i++)
amount *= 10;
}
if (str_amount.empty())
if (parts.size() == 1)
return true;
if (parts[1].find_first_not_of("0123456789"sv) != std::string::npos)
return false; // fractional part contains non-digit
// If too long, but with insignificant 0's, trim them off
while (parts[1].size() > decimal_point && parts[1].back() == '0')
parts[1].remove_suffix(1);
if (parts[1].size() > decimal_point)
return false; // fractional part has too many significant digits
uint64_t fractional;
if (!tools::parse_int(parts[1], fractional))
return false;
if (fraction_size < default_decimal_point)
{
str_amount.append(default_decimal_point - fraction_size, '0');
}
// Scale up the value if it wasn't a full fractional value, e.g. if we have "10.45" then we
// need to convert the 45 we just parsed to 450'000'000.
for (size_t i = parts[1].size(); i < decimal_point; i++)
fractional *= 10;
return epee::string_tools::get_xtype_from_string(amount, str_amount);
if (fractional > std::numeric_limits<uint64_t>::max() - amount)
return false; // would overflow
amount += fractional;
return true;
}
//---------------------------------------------------------------
uint64_t get_transaction_weight(const transaction &tx, size_t blob_size)

2
src/cryptonote_basic/cryptonote_format_utils.h
View File

@ -203,7 +203,7 @@ namespace cryptonote
uint64_t get_outs_money_amount(const transaction& tx);
bool check_inputs_types_supported(const transaction& tx);
bool check_outs_valid(const transaction& tx);
bool parse_amount(uint64_t& amount, const std::string& str_amount);
bool parse_amount(uint64_t& amount, std::string_view str_amount);
uint64_t get_transaction_weight(const transaction &tx);
uint64_t get_transaction_weight(const transaction &tx, size_t blob_size);
uint64_t get_pruned_transaction_weight(const transaction &tx);

4
src/simplewallet/simplewallet.cpp
View File

@ -2541,7 +2541,7 @@ bool simple_wallet::set_inactivity_lock_timeout(const std::vector<std::string> &
if (pwd_container)
{
uint32_t r;
if (epee::string_tools::get_xtype_from_string(r, args[1]))
if (tools::parse_int(args[1], r))
{
m_wallet->inactivity_lock_timeout(r);
m_wallet->rewrite(m_wallet_file, pwd_container->password());
@ -3602,7 +3602,7 @@ bool simple_wallet::init(const boost::program_options::variables_map& vm)
crypto::secret_key key;
crypto::cn_slow_hash(seed_pass.data(), seed_pass.size(), (crypto::hash&)key, crypto::cn_slow_hash_type::heavy_v1);
sc_reduce32((unsigned char*)key.data);
multisig_keys = m_wallet->decrypt<epee::wipeable_string>(std::string(multisig_keys.data(), multisig_keys.size()), key, true);
multisig_keys = m_wallet->decrypt(std::string(multisig_keys.data(), multisig_keys.size()), key, true);
}
else
m_recovery_key = cryptonote::decrypt_key(m_recovery_key, seed_pass);

617
src/wallet/wallet2.cpp
View File

File diff suppressed because it is too large Load Diff

55
src/wallet/wallet2.h
View File

@ -67,6 +67,7 @@
#include "wallet_light_rpc.h"
#include "common/loki_integration_test_hooks.h"
#include "wipeable_string.h"
#undef LOKI_DEFAULT_LOG_CATEGORY
#define LOKI_DEFAULT_LOG_CATEGORY "wallet.wallet2"
@ -1000,9 +1001,9 @@ private:
std::string sign_tx_dump_to_str(unsigned_tx_set &exported_txs, std::vector<wallet2::pending_tx> &ptx, signed_tx_set &signed_txes);
// load unsigned_tx_set from file.
bool load_unsigned_tx(const std::string &unsigned_filename, unsigned_tx_set &exported_txs) const;
bool parse_unsigned_tx_from_str(const std::string &unsigned_tx_st, unsigned_tx_set &exported_txs) const;
bool parse_unsigned_tx_from_str(std::string_view unsigned_tx_st, unsigned_tx_set &exported_txs) const;
bool load_tx(const std::string &signed_filename, std::vector<tools::wallet2::pending_tx> &ptx, std::function<bool(const signed_tx_set&)> accept_func = NULL);
bool parse_tx_from_str(const std::string &signed_tx_st, std::vector<tools::wallet2::pending_tx> &ptx, std::function<bool(const signed_tx_set &)> accept_func);
bool parse_tx_from_str(std::string_view signed_tx_st, std::vector<tools::wallet2::pending_tx> &ptx, std::function<bool(const signed_tx_set &)> accept_func);
std::vector<wallet2::pending_tx> create_transactions_2(std::vector<cryptonote::tx_destination_entry> dsts, const size_t fake_outs_count, const uint64_t unlock_time, uint32_t priority, const std::vector<uint8_t>& extra_base, uint32_t subaddr_account, std::set<uint32_t> subaddr_indices, cryptonote::loki_construct_tx_params &tx_params);
std::vector<wallet2::pending_tx> create_transactions_all(uint64_t below, const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, const size_t fake_outs_count, const uint64_t unlock_time, uint32_t priority, const std::vector<uint8_t>& extra, uint32_t subaddr_account, std::set<uint32_t> subaddr_indices, cryptonote::txtype tx_type = cryptonote::txtype::standard);
@ -1014,7 +1015,7 @@ private:
void cold_sign_tx(const std::vector<pending_tx>& ptx_vector, signed_tx_set &exported_txs, std::vector<cryptonote::address_parse_info> const &dsts_info, std::vector<std::string> & tx_device_aux);
uint64_t cold_key_image_sync(uint64_t &spent, uint64_t &unspent);
void device_show_address(uint32_t account_index, uint32_t address_index, const std::optional<crypto::hash8> &payment_id);
bool parse_multisig_tx_from_str(std::string multisig_tx_st, multisig_tx_set &exported_txs) const;
bool parse_multisig_tx_from_str(std::string_view multisig_tx_st, multisig_tx_set &exported_txs) const;
bool load_multisig_tx(cryptonote::blobdata blob, multisig_tx_set &exported_txs, std::function<bool(const multisig_tx_set&)> accept_func = NULL);
bool load_multisig_tx_from_file(const std::string &filename, multisig_tx_set &exported_txs, std::function<bool(const multisig_tx_set&)> accept_func = NULL);
bool sign_multisig_tx_from_file(const std::string &filename, std::vector<crypto::hash> &txids, std::function<bool(const multisig_tx_set&)> accept_func);
@ -1202,10 +1203,10 @@ private:
* \param file_path Wallet file path
* \return Whether path is valid format
*/
static bool wallet_valid_path_format(const std::string& file_path);
static bool parse_long_payment_id(const std::string& payment_id_str, crypto::hash& payment_id);
static bool parse_short_payment_id(const std::string& payment_id_str, crypto::hash8& payment_id);
static bool parse_payment_id(const std::string& payment_id_str, crypto::hash& payment_id);
static bool wallet_valid_path_format(std::string_view file_path);
static bool parse_long_payment_id(std::string_view payment_id_str, crypto::hash& payment_id);
static bool parse_short_payment_id(std::string_view payment_id_str, crypto::hash8& payment_id);
static bool parse_payment_id(std::string_view payment_id_str, crypto::hash& payment_id);
bool always_confirm_transfers() const { return m_always_confirm_transfers; }
void always_confirm_transfers(bool always) { m_always_confirm_transfers = always; }
@ -1260,13 +1261,13 @@ private:
void check_tx_key(const crypto::hash &txid, const crypto::secret_key &tx_key, const std::vector<crypto::secret_key> &additional_tx_keys, const cryptonote::account_public_address &address, uint64_t &received, bool &in_pool, uint64_t &confirmations);
void check_tx_key_helper(const crypto::hash &txid, const crypto::key_derivation &derivation, const std::vector<crypto::key_derivation> &additional_derivations, const cryptonote::account_public_address &address, uint64_t &received, bool &in_pool, uint64_t &confirmations);
void check_tx_key_helper(const cryptonote::transaction &tx, const crypto::key_derivation &derivation, const std::vector<crypto::key_derivation> &additional_derivations, const cryptonote::account_public_address &address, uint64_t &received) const;
std::string get_tx_proof(const crypto::hash &txid, const cryptonote::account_public_address &address, bool is_subaddress, const std::string &message);
std::string get_tx_proof(const cryptonote::transaction &tx, const crypto::secret_key &tx_key, const std::vector<crypto::secret_key> &additional_tx_keys, const cryptonote::account_public_address &address, bool is_subaddress, const std::string &message) const;
bool check_tx_proof(const crypto::hash &txid, const cryptonote::account_public_address &address, bool is_subaddress, const std::string &message, const std::string &sig_str, uint64_t &received, bool &in_pool, uint64_t &confirmations);
bool check_tx_proof(const cryptonote::transaction &tx, const cryptonote::account_public_address &address, bool is_subaddress, const std::string &message, const std::string &sig_str, uint64_t &received) const;
std::string get_tx_proof(const crypto::hash &txid, const cryptonote::account_public_address &address, bool is_subaddress, std::string_view message);
std::string get_tx_proof(const cryptonote::transaction &tx, const crypto::secret_key &tx_key, const std::vector<crypto::secret_key> &additional_tx_keys, const cryptonote::account_public_address &address, bool is_subaddress, std::string_view message) const;
bool check_tx_proof(const crypto::hash &txid, const cryptonote::account_public_address &address, bool is_subaddress, std::string_view message, std::string_view sig_str, uint64_t &received, bool &in_pool, uint64_t &confirmations);
bool check_tx_proof(const cryptonote::transaction &tx, const cryptonote::account_public_address &address, bool is_subaddress, std::string_view message, std::string_view sig_str, uint64_t &received) const;
std::string get_spend_proof(const crypto::hash &txid, const std::string &message);
bool check_spend_proof(const crypto::hash &txid, const std::string &message, const std::string &sig_str);
std::string get_spend_proof(const crypto::hash &txid, std::string_view message);
bool check_spend_proof(const crypto::hash &txid, std::string_view message, std::string_view sig_str);
/*!
* \brief Generates a proof that proves the reserve of unspent funds
@ -1275,7 +1276,7 @@ private:
* \param message Arbitrary challenge message to be signed together
* \return Signature string
*/
std::string get_reserve_proof(const std::optional<std::pair<uint32_t, uint64_t>> &account_minreserve, const std::string &message);
std::string get_reserve_proof(const std::optional<std::pair<uint32_t, uint64_t>> &account_minreserve, std::string message);
/*!
* \brief Verifies a proof of reserve
* \param address The signer's address
@ -1285,7 +1286,7 @@ private:
* \param spent [OUT] the sum of spent funds included in the signature
* \return true if the signature verifies correctly
*/
bool check_reserve_proof(const cryptonote::account_public_address &address, const std::string &message, const std::string_view sig_str, uint64_t &total, uint64_t &spent);
bool check_reserve_proof(const cryptonote::account_public_address &address, std::string_view message, std::string_view sig_str, uint64_t &total, uint64_t &spent);
/*!
* \brief GUI Address book get/store
@ -1349,8 +1350,8 @@ private:
*/
void set_account_tag_description(const std::string& tag, const std::string& description);
std::string sign(const std::string &data, cryptonote::subaddress_index index = {0, 0}) const;
bool verify(const std::string &data, const cryptonote::account_public_address &address, const std::string &signature) const;
std::string sign(std::string_view data, cryptonote::subaddress_index index = {0, 0}) const;
bool verify(std::string_view data, const cryptonote::account_public_address &address, std::string_view signature) const;
/*!
* \brief sign_multisig_participant signs given message with the multisig public signer key
@ -1358,7 +1359,7 @@ private:
* \throws if wallet is not multisig
* \return signature
*/
std::string sign_multisig_participant(const std::string& data) const;
std::string sign_multisig_participant(std::string_view data) const;
/*!
* \brief verify_with_public_key verifies message was signed with given public key
* \param data message
@ -1366,13 +1367,13 @@ private:
* \param signature signature of the message
* \return true if the signature is correct
*/
bool verify_with_public_key(const std::string &data, const crypto::public_key &public_key, const std::string &signature) const;
bool verify_with_public_key(std::string_view data, const crypto::public_key &public_key, std::string_view signature) const;
// Import/Export wallet data
std::pair<size_t, std::vector<tools::wallet2::transfer_details>> export_outputs(bool all = false) const;
std::string export_outputs_to_str(bool all = false) const;
size_t import_outputs(const std::pair<size_t, std::vector<tools::wallet2::transfer_details>> &outputs);
size_t import_outputs_from_str(const std::string &outputs_st);
size_t import_outputs_from_str(std::string outputs_st);
payment_container export_payments() const;
void import_payments(const payment_container &payments);
void import_payments_out(const std::list<std::pair<crypto::hash,wallet2::confirmed_transfer_details>> &confirmed_payments);
@ -1415,16 +1416,14 @@ private:
void remove_obsolete_pool_txs(const std::vector<crypto::hash> &tx_hashes);
std::string encrypt(const char *plaintext, size_t len, const crypto::secret_key &skey, bool authenticated = true) const;
std::string encrypt(std::string_view plaintext, const crypto::secret_key &skey, bool authenticated = true) const;
std::string encrypt(const epee::span<char> &span, const crypto::secret_key &skey, bool authenticated = true) const;
std::string encrypt(const std::string &plaintext, const crypto::secret_key &skey, bool authenticated = true) const;
std::string encrypt(const epee::wipeable_string &plaintext, const crypto::secret_key &skey, bool authenticated = true) const;
std::string encrypt_with_view_secret_key(const std::string &plaintext, bool authenticated = true) const;
template<typename T=std::string> T decrypt(const std::string &ciphertext, const crypto::secret_key &skey, bool authenticated = true) const;
std::string decrypt_with_view_secret_key(const std::string &ciphertext, bool authenticated = true) const;
std::string encrypt_with_view_secret_key(std::string_view plaintext, bool authenticated = true) const;
epee::wipeable_string decrypt(std::string_view ciphertext, const crypto::secret_key &skey, bool authenticated = true) const;
epee::wipeable_string decrypt_with_view_secret_key(std::string_view ciphertext, bool authenticated = true) const;
std::string make_uri(const std::string &address, const std::string &payment_id, uint64_t amount, const std::string &tx_description, const std::string &recipient_name, std::string &error) const;
bool parse_uri(const std::string &uri, std::string &address, std::string &payment_id, uint64_t &amount, std::string &tx_description, std::string &recipient_name, std::vector<std::string> &unknown_parameters, std::string &error);
bool parse_uri(std::string_view uri, std::string &address, std::string &payment_id, uint64_t &amount, std::string &tx_description, std::string &recipient_name, std::vector<std::string> &unknown_parameters, std::string &error);
uint64_t get_blockchain_height_by_date(uint16_t year, uint8_t month, uint8_t day); // 1<=month<=12, 1<=day<=31
@ -1894,7 +1893,7 @@ private:
// TODO(loki): The better question is if anyone is ever going to try use
// register service node funded by multiple subaddresses. This is unlikely.
constexpr std::array<const char* const, 6> allowed_priority_strings = {{"default", "unimportant", "normal", "elevated", "priority", "blink"}};
bool parse_subaddress_indices(const std::string& arg, std::set<uint32_t>& subaddr_indices, std::string *err_msg = nullptr);
bool parse_subaddress_indices(std::string_view arg, std::set<uint32_t>& subaddr_indices, std::string *err_msg = nullptr);
bool parse_priority (const std::string& arg, uint32_t& priority);
}

13
tests/unit_tests/parse_amount.cpp
View File

@ -125,7 +125,9 @@ TEST_pos(18446744073700000, 18446744_073700000);
TEST_pos(18446744073700000, 18446744_0737000000000000000);
/* Max supply */
TEST_pos(18446744073709551614, 18446744073_709551614);
TEST_pos(18446744073709551615, 18446744073_709551615);
TEST_neg(18446744073_709551616); // overflows
// Invalid numbers
TEST_neg_n(~, empty_string);
@ -135,14 +137,13 @@ TEST_neg_n(-1, minus_1);
TEST_neg_n(+1, plus_1);
TEST_neg_n(_, only_point);
// Don't go below 10^-12
TEST_neg(0_0000000000001);
TEST_neg(0_0000000000009);
TEST_neg(184467440737_000000001);
// Don't go below 10^-9
TEST_neg(0_0000000001);
TEST_neg(0_0000000009);
TEST_neg(18446744073_0000000001);
// Overflow
TEST_neg(184467440737_09551616);
TEST_neg(184467440738);
TEST_neg(18446744073_8);
TEST_neg(18446744073709551616);
// Two or more points