Modifications following code review

libraries/chain/asset.cpp

@@ -14,17 +14,6 @@ uint8_t asset::decimals()const {
    return sym.decimals();
 }
 
-#if 0
-void asset::set_decimals(uint8_t d){
-   /*
-   auto a = (char*)&symbol;
-   a[0] = d;
-   */
-   
-   //TODO: implement this using class symbol
-}
-#endif
-
 string asset::symbol_name()const {
    return sym.name();
 }
@@ -45,64 +34,31 @@ string asset::to_string()const {
 
 asset asset::from_string(const string& from)
 {
-   /*
-   try
-   {
-      string s = fc::trim( from );
-      auto space_pos = s.find( " " );
-      auto dot_pos = s.find( "." );
+   try { 
+      string s = fc::trim(from);
+      auto dot_pos = s.find(".");
+      FC_ASSERT(dot_pos != string::npos, "dot missing in asset from string");
+      auto space_pos = s.find(" ", dot_pos);
+      FC_ASSERT(space_pos != string::npos, "space missing in asset from string");
 
       asset result;
-      result.symbol = uint64_t(0);
-      auto sy = (char*)&result.symbol;
-      *sy = (char) dot_pos; // Mask due to undefined architecture behavior
-
-      auto intpart = s.substr( 0, dot_pos );
+      
+      auto intpart = s.substr(0, dot_pos);
       result.amount = fc::to_int64(intpart);
-      string fractpart;
-      if( dot_pos != string::npos )
-      {
-         auto fractpart = "1" + s.substr( dot_pos + 1, space_pos - dot_pos - 1 );
-         result.set_decimals( fractpart.size() - 1 );
-
+      string symbol_part;
+      if (dot_pos != string::npos && space_pos != string::npos) {
+         symbol_part = eosio::chain::to_string(space_pos - dot_pos - 1);
+         symbol_part += ',';
+         symbol_part += s.substr(space_pos + 1);
+      }
+      result.sym = symbol::from_string(symbol_part);
+      if (dot_pos != string::npos) {
+         auto fractpart = "1" + s.substr(dot_pos + 1, space_pos - dot_pos - 1);
+         
          result.amount *= int64_t(result.precision());
          result.amount += int64_t(fc::to_int64(fractpart));
          result.amount -= int64_t(result.precision());
       }
-      auto symbol = s.substr( space_pos + 1 );
-
-      if( symbol.size() )
-         memcpy( sy+1, symbol.c_str(), std::min(symbol.size(),size_t(6)) );
-
-      return result;
-   }
-   FC_CAPTURE_LOG_AND_RETHROW( (from) )
-   */
-   try { 
-      string s = fc::trim( from );
-      auto space_pos = s.find( " " );
-      auto dot_pos = s.find( "." );
-      
-      asset result;
-      
-      auto intpart = s.substr( 0, dot_pos );
-      result.amount = fc::to_int64(intpart);
-      string symbol_part;
-      if (dot_pos != string::npos && space_pos != string::npos)
-         {
-            symbol_part = eosio::chain::to_string(space_pos - dot_pos - 1);
-            symbol_part += ',';
-            symbol_part += s.substr(space_pos + 1);
-         }
-      result.sym = symbol::from_string(symbol_part);
-      if( dot_pos != string::npos )
-         {
-            auto fractpart = "1" + s.substr( dot_pos + 1, space_pos - dot_pos - 1 );
-            
-            result.amount *= int64_t(result.precision());
-            result.amount += int64_t(fc::to_int64(fractpart));
-            result.amount -= int64_t(result.precision());
-         }
       
       return result;
    }

libraries/chain/contracts/chain_initializer.cpp

@@ -312,7 +312,7 @@ std::vector<action> chain_initializer::prepare_database( chain_controller& chain
 
       messages_to_process.emplace_back(move(message));
 
-      if (acct.liquid_balance > 0) {
+      if (acct.liquid_balance > asset(0)) {
          message = action( {{config::system_account_name, config::active_name}},
                            transfer{ .from = config::system_account_name, .to = acct.name,
                                      .amount = acct.liquid_balance.amount, .memo = "Genesis Allocation"});

libraries/chain/include/eosio/chain/asset.hpp

@@ -8,18 +8,27 @@
 #include <eosio/chain/symbol.hpp>
 
 /// eos with 8 digits of precision
-#define EOS_SYMBOL  (int64_t(4) | (uint64_t('E') << 8) | (uint64_t('O') << 16) | (uint64_t('S') << 24))
+#define EOS_SYMBOL_VALUE  (int64_t(4) | (uint64_t('E') << 8) | (uint64_t('O') << 16) | (uint64_t('S') << 24))
+static const eosio::chain::symbol EOS_SYMBOL(EOS_SYMBOL_VALUE);
 
 /// Defined to be largest power of 10 that fits in 53 bits of precision
 #define EOS_MAX_SHARE_SUPPLY   int64_t(1'000'000'000'000'000ll)
 
 namespace eosio { namespace chain {
 
-      //using asset_symbol = uint64_t;
+/**
+
+asset includes amount and currency symbol
+
+asset::from_string takes a string of the form "10.0000 CUR" and constructs an asset 
+with amount = 10 and symbol(4,"CUR")
+
+*/
+
 
 struct asset
 {
-   asset(share_type a = 0, symbol id = EOS_SYMBOL)
+   explicit asset(share_type a = 0, symbol id = EOS_SYMBOL)
       :amount(a), sym(id){}
 
    share_type amount;
@@ -30,7 +39,6 @@ struct asset
    uint8_t     decimals()const;
    string      symbol_name()const;
    int64_t     precision()const;
-//   void        set_decimals(uint8_t d);
    const symbol& symbol() const { return sym; }
 
    static asset from_string(const string& from);

libraries/chain/include/eosio/chain/symbol.hpp

@@ -6,9 +6,19 @@
 #include <fc/exception/exception.hpp>
 #include <eosio/chain/types.hpp>
 #include <string>
+#include <functional>
 
 namespace eosio {
    namespace chain {
+
+      /**
+         class symbol represents a token and contains precision and name.
+         When encoded as a uint64_t, first byte represents the number of decimals, remaining bytes
+         represent token name.
+         Name must only include upper case alphabets.
+         from_string constructs a symbol from an input a string of the form "4,EOS"
+         where the integer represents number of decimals. Number of decimals must be larger than zero.
+       */
       
       static constexpr uint64_t string_to_symbol_c(uint8_t precision, const char* str) {
          uint32_t len = 0;
@@ -43,34 +53,28 @@ namespace eosio {
 
       class symbol {
          public:
-            symbol(uint8_t p, const char* s): m_value(string_to_symbol(p, s)) { }
-            symbol(uint64_t v = SY(4, EOS)): m_value(v) { }
+            explicit symbol(uint8_t p, const char* s): m_value(string_to_symbol(p, s)) { }
+            explicit symbol(uint64_t v = SY(4, EOS)): m_value(v) { }
             static symbol from_string(const string& from)
             {
                try {
                   string s = fc::trim(from);
-                  if (s.empty()) {
-                     return SY(4, EOS);
-                  }
+                  FC_ASSERT(!s.empty(), "creating symbol from empty string");
                   auto comma_pos = s.find(',');
-                  if (comma_pos == string::npos) {
-                     return string_to_symbol(4, s.c_str());
-                  }
-                  if (comma_pos == 0) {
-                     return string_to_symbol(4, s.substr(1).c_str());
-                  }
+                  FC_ASSERT(comma_pos != string::npos, "missing comma in symbol");
                   auto prec_part = s.substr(0, comma_pos);
                   uint8_t p = fc::to_int64(prec_part);
+                  FC_ASSERT(p > 0, "zero decimals in symbol");
                   string name_part = s.substr(comma_pos + 1);
-                  return string_to_symbol(p, name_part.c_str());
+                  return symbol(string_to_symbol(p, name_part.c_str()));
                } FC_CAPTURE_LOG_AND_RETHROW((from))
             }
             uint64_t value() const { return m_value; }
-            operator uint64_t() const { return m_value; }
             bool valid() const
-            { 
+            {
+               if (decimals() == 0) return false;
                const auto& s = name();
-               return all_of(s.begin(), s.end(), [](char c)->bool { return (c >= 'A' && c <= 'Z'); });
+               return valid_name(s);
             }
             static bool valid_name(const string& name)
             {
@@ -113,7 +117,6 @@ namespace eosio {
             }
 
             string to_string() const { return string(*this); }
-
             template <typename DataStream>
             friend DataStream& operator<< (DataStream& ds, const symbol& s)
             {
@@ -129,6 +132,10 @@ namespace eosio {
       {
          return lhs.value() == rhs.value();
       }
+      inline bool operator!= (const symbol& lhs, const symbol& rhs)
+      {
+         return lhs.value() != rhs.value();
+      }
       inline bool operator< (const symbol& lhs, const symbol& rhs)
       {
          return lhs.value() < rhs.value();

libraries/testing/tester.cpp

@@ -260,7 +260,7 @@ namespace eosio { namespace testing {
 
       // the balance is implied to be 0 if either the table or row does not exist
       if (tbl) {
-         const auto *obj = db.find<contracts::key_value_object, contracts::by_scope_primary>(boost::make_tuple(tbl->id, asset_symbol));
+         const auto *obj = db.find<contracts::key_value_object, contracts::by_scope_primary>(boost::make_tuple(tbl->id, asset_symbol.value()));
          if (obj) {
             fc::datastream<const char *> ds(obj->value.data(), obj->value.size());
             fc::raw::unpack(ds, result);

plugins/faucet_testnet_plugin/faucet_testnet_plugin.cpp

@@ -222,7 +222,7 @@ struct faucet_testnet_plugin_impl {
       auto& plugin = _app.get_plugin<chain_plugin>();
       plugin.get_chain_id(chainid);
       chain_controller& cc = plugin.chain();
-      const uint64_t deposit = 1;
+      const asset deposit(1);
 
       signed_transaction trx;
       auto memo = fc::variant(fc::time_point::now()).as_string() + " " + fc::variant(fc::time_point::now().time_since_epoch()).as_string();

plugins/txn_test_gen_plugin/txn_test_gen_plugin.cpp

@@ -62,7 +62,7 @@ struct txn_test_gen_plugin_impl {
       chain_controller& cc = app().get_plugin<chain_plugin>().chain();
       chain::chain_id_type chainid;
       app().get_plugin<chain_plugin>().get_chain_id(chainid);
-      uint64_t stake = 10000;
+      asset stake(10000);
 
       fc::crypto::private_key txn_test_receiver_A_priv_key = fc::crypto::private_key::regenerate(fc::sha256(std::string(64, 'a')));
       fc::crypto::private_key txn_test_receiver_B_priv_key = fc::crypto::private_key::regenerate(fc::sha256(std::string(64, 'b')));

programs/eosioc/main.cpp

@@ -289,7 +289,7 @@ void create_account(name creator, name newaccount, public_key_type owner, public
       auto active_auth  = eosio::chain::authority{1, {{active, 1}}, {}};
       auto recovery_auth = eosio::chain::authority{1, {}, {{{creator, "active"}, 1}}};
 
-      uint64_t deposit = staked_deposit;
+      asset deposit(staked_deposit);
 
       signed_transaction trx;
       trx.actions.emplace_back( vector<chain::permission_level>{{creator,"active"}},
@@ -986,7 +986,7 @@ int main( int argc, char** argv ) {
         auto active_auth  = eosio::chain::authority{1, {{active, 1}}, {}};
         auto recovery_auth = eosio::chain::authority{1, {}, {{{creator, "active"}, 1}}};
         
-        uint64_t deposit = 1;
+        asset deposit(1);
         
         signed_transaction trx;
         trx.actions.emplace_back( vector<chain::permission_level>{{creator,"active"}},

tests/chain_tests/transfer_tests.cpp

@@ -21,14 +21,14 @@ BOOST_AUTO_TEST_CASE( transfer_test ) { try {
   test.transfer( N(inita), N(dan), "10.0000 EOS", "memo" );
 
   {
-     const auto& dans_balance = test.get_balance( N(dan) );
+     const asset dans_balance( test.get_balance( N(dan) ) );
      FC_ASSERT( dans_balance == asset::from_string("10.0000 EOS") );
   }
 
   test.produce_block();
 
   {
-     const auto& dans_balance = test.get_balance( N(dan) );
+     const asset dans_balance( test.get_balance( N(dan) ) );
      FC_ASSERT( dans_balance == asset::from_string("10.0000 EOS") );
   }
 
@@ -40,12 +40,12 @@ BOOST_AUTO_TEST_CASE( transfer_test ) { try {
 
 
   /// verify that bart now has 10.000
-  const auto& barts_balance = test.get_balance( N(bart) );
+  const asset barts_balance( test.get_balance( N(bart) ) );
   FC_ASSERT( barts_balance == asset::from_string("10.0000 EOS") );
 
   {
      /// verify that dan now has 0.000
-     const auto& dans_balance = test.get_balance( N(dan) );
+     const asset dans_balance( test.get_balance( N(dan) ) );
      FC_ASSERT( dans_balance == asset::from_string("0.0000 EOS") );
   }
 

tests/wasm_tests/currency_tests.cpp

@@ -185,7 +185,7 @@ BOOST_FIXTURE_TEST_CASE(test_symbol, tester) try {
 
    {
       symbol eos(4, "EOS");
-      BOOST_REQUIRE_EQUAL(EOS_SYMBOL, eos.value());
+      BOOST_REQUIRE_EQUAL(EOS_SYMBOL_VALUE, eos.value());
       BOOST_REQUIRE_EQUAL("4,EOS", eos.to_string());
       BOOST_REQUIRE_EQUAL("EOS", eos.name());
       BOOST_REQUIRE_EQUAL(4, eos.decimals());
@@ -204,25 +204,24 @@ BOOST_FIXTURE_TEST_CASE(test_symbol, tester) try {
       BOOST_REQUIRE_EQUAL("YEN", y.name());
    }
 
-   // from empty string - default
+   // from empty string
    {
-      symbol empty = symbol::from_string("");
-      BOOST_REQUIRE_EQUAL(4, empty.decimals());
-      BOOST_REQUIRE_EQUAL("EOS", empty.name());
+      BOOST_CHECK_EXCEPTION(symbol empty = symbol::from_string(""),
+                            fc::assert_exception, assert_message_is("creating symbol from empty string"));
    }
 
-   // precision part missing - default is 4
+   
+   // precision part missing
    {
-      symbol incomplete = symbol::from_string("RND");
-      BOOST_REQUIRE_EQUAL(4, incomplete.decimals());
-      BOOST_REQUIRE_EQUAL("RND", incomplete.name());
+      BOOST_CHECK_EXCEPTION(symbol incomplete = symbol::from_string("RND"),
+                            fc::assert_exception, assert_message_is("missing comma in symbol"));
    }
 
-   // precision part missing - default is 4
+
+   // precision part missing
    {
-      symbol incomplete = symbol::from_string(",EURO");
-      BOOST_REQUIRE_EQUAL(4, incomplete.decimals());
-      BOOST_REQUIRE_EQUAL("EURO", incomplete.name());
+      BOOST_CHECK_EXCEPTION(symbol incomplete = symbol::from_string("0,EURO"),
+                            fc::assert_exception, assert_message_is("zero decimals in symbol"));
    }
 
    // invalid - contains lower case characters, no validation
@@ -235,7 +234,14 @@ BOOST_FIXTURE_TEST_CASE(test_symbol, tester) try {
 
    // invalid - contains lower case characters, exception thrown 
    {
-      BOOST_CHECK_EXCEPTION(symbol(5,"EoS"), fc::assert_exception, assert_message_is("invalid character in symbol name"));
+      BOOST_CHECK_EXCEPTION(symbol(5,"EoS"),
+                            fc::assert_exception, assert_message_is("invalid character in symbol name"));
+   }
+
+   // invalid - missing decimal point
+   {
+      BOOST_CHECK_EXCEPTION(asset a = asset::from_string("10 CUR"),
+                            fc::assert_exception, assert_message_is("dot missing in asset from string"));
    }
 
 } FC_LOG_AND_RETHROW() /// test_symbol