From cd9ff2de8354820c14363e88b8286fc121c09c10 Mon Sep 17 00:00:00 2001 From: arhag Date: Thu, 31 May 2018 18:51:09 -0400 Subject: [PATCH 1/2] better name bidding assert messages and cleanup eosio.system tests --- contracts/eosio.system/eosio.system.cpp | 17 +++++++---------- unittests/eosio.system_tests.cpp | 16 +++++++++------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/contracts/eosio.system/eosio.system.cpp b/contracts/eosio.system/eosio.system.cpp index 838c3ff1659..d6cc50c16de 100644 --- a/contracts/eosio.system/eosio.system.cpp +++ b/contracts/eosio.system/eosio.system.cpp @@ -117,7 +117,7 @@ namespace eosiosystem { } else { eosio_assert( current->high_bid > 0, "this auction has already closed" ); eosio_assert( bid.amount - current->high_bid > (current->high_bid / 10), "must increase bid by 10%" ); - eosio_assert( current->high_bidder != bidder, "account is already high bidder" ); + eosio_assert( current->high_bidder != bidder, "account is already highest bidder" ); INLINE_ACTION_SENDER(eosio::token, transfer)( N(eosio.token), {N(eosio.names),N(active)}, { N(eosio.names), current->high_bidder, asset(current->high_bid), @@ -135,13 +135,10 @@ namespace eosiosystem { * Called after a new account is created. This code enforces resource-limits rules * for new accounts as well as new account naming conventions. * - * 1. accounts cannot contain '.' symbols which forces all acccounts to be 12 - * characters long without '.' until a future account auction process is implemented - * which prevents name squatting. + * Account names containing '.' symbols must have a suffix equal to the name of the creator. + * This allows users who buy a premium name (shorter than 12 characters with no dots) to be the only ones + * who can create accounts with the creator's name as a suffix. * - * 2. new accounts must stake a minimal number of tokens (as set in system parameters) - * therefore, this method will execute an inline buyram from receiver for newacnt in - * an amount equal to the current new account creation fee. */ void native::newaccount( account_name creator, account_name newact @@ -157,13 +154,13 @@ namespace eosiosystem { has_dot |= !(tmp & 0x1f); tmp >>= 5; } - auto suffix = eosio::name_suffix(newact); - if( has_dot ) { + if( has_dot ) { // or is less than 12 characters + auto suffix = eosio::name_suffix(newact); if( suffix == newact ) { name_bid_table bids(_self,_self); auto current = bids.find( newact ); eosio_assert( current != bids.end(), "no active bid for name" ); - eosio_assert( current->high_bidder == creator, "only high bidder can claim" ); + eosio_assert( current->high_bidder == creator, "only highest bidder can claim" ); eosio_assert( current->high_bid < 0, "auction for name is not closed yet" ); bids.erase( current ); } else { diff --git a/unittests/eosio.system_tests.cpp b/unittests/eosio.system_tests.cpp index 94372df7760..59197a7f9af 100644 --- a/unittests/eosio.system_tests.cpp +++ b/unittests/eosio.system_tests.cpp @@ -2229,22 +2229,24 @@ BOOST_FIXTURE_TEST_CASE( buyname, eosio_system_tester ) try { produce_block( fc::days(14) ); produce_block(); - BOOST_REQUIRE_THROW( create_accounts_with_resources( { N(fail) }, N(dan) ), fc::exception ); // dan shouldn't be able to create fail - + BOOST_REQUIRE_EXCEPTION( create_accounts_with_resources( { N(fail) }, N(dan) ), // dan shouldn't be able to create fail + eosio_assert_message_exception, eosio_assert_message_is( "no active bid for name" ) ); bidname( "dan", "nofail", core_from_string( "1.0000" ) ); BOOST_REQUIRE_EQUAL( "assertion failure with message: must increase bid by 10%", bidname( "sam", "nofail", core_from_string( "1.0000" ) )); // didn't increase bid by 10% BOOST_REQUIRE_EQUAL( success(), bidname( "sam", "nofail", core_from_string( "2.0000" ) )); // didn't increase bid by 10% produce_block( fc::days(1) ); produce_block(); - BOOST_REQUIRE_THROW( create_accounts_with_resources( { N(nofail) }, N(dan) ), fc::exception); // dan shoudn't be able to do this, sam won + BOOST_REQUIRE_EXCEPTION( create_accounts_with_resources( { N(nofail) }, N(dan) ), // dan shoudn't be able to do this, sam won + eosio_assert_message_exception, eosio_assert_message_is( "only highest bidder can claim" ) ); //wlog( "verify sam can create nofail" ); create_accounts_with_resources( { N(nofail) }, N(sam) ); // sam should be able to do this, he won the bid //wlog( "verify nofail can create test.nofail" ); transfer( "eosio", "nofail", core_from_string( "1000.0000" ) ); create_accounts_with_resources( { N(test.nofail) }, N(nofail) ); // only nofail can create test.nofail //wlog( "verify dan cannot create test.fail" ); - BOOST_REQUIRE_THROW( create_accounts_with_resources( { N(test.fail) }, N(dan) ), fc::exception ); // dan shouldn't be able to do this + BOOST_REQUIRE_EXCEPTION( create_accounts_with_resources( { N(test.fail) }, N(dan) ), // dan shouldn't be able to do this + eosio_assert_message_exception, eosio_assert_message_is( "only suffix may create this account" ) ); create_accounts_with_resources( { N(goodgoodgood) }, N(dan) ); /// 12 char names should succeed } FC_LOG_AND_RETHROW() @@ -2290,7 +2292,7 @@ BOOST_FIXTURE_TEST_CASE( multiple_namebids, eosio_system_tester ) try { bidname( "carl", "prefe", core_from_string("1.0000") ); BOOST_REQUIRE_EQUAL( core_from_string( "9998.0000" ), get_balance("carl") ); - BOOST_REQUIRE_EQUAL( error("assertion failure with message: account is already high bidder"), + BOOST_REQUIRE_EQUAL( error("assertion failure with message: account is already highest bidder"), bidname( "bob", "prefb", core_from_string("1.1001") ) ); BOOST_REQUIRE_EQUAL( error("assertion failure with message: must increase bid by 10%"), bidname( "alice", "prefb", core_from_string("1.0999") ) ); @@ -2379,9 +2381,9 @@ BOOST_FIXTURE_TEST_CASE( multiple_namebids, eosio_system_tester ) try { produce_blocks(2); // bid for prefb has closed, only highest bidder can claim BOOST_REQUIRE_EXCEPTION( create_account_with_resources( N(prefb), N(alice) ), - fc::exception, fc_assert_exception_message_is("only high bidder can claim") ); + eosio_assert_message_exception, eosio_assert_message_is( "only highest bidder can claim" ) ); BOOST_REQUIRE_EXCEPTION( create_account_with_resources( N(prefb), N(carl) ), - fc::exception, fc_assert_exception_message_is("only high bidder can claim") ); + eosio_assert_message_exception, eosio_assert_message_is( "only highest bidder can claim" ) ); create_account_with_resources( N(prefb), N(eve) ); BOOST_REQUIRE_EXCEPTION( create_account_with_resources( N(prefe), N(carl) ), From eb9e2737cad84fda93bd3455a27a799bb45faaf7 Mon Sep 17 00:00:00 2001 From: arhag Date: Thu, 31 May 2018 21:31:41 -0400 Subject: [PATCH 2/2] improve name_suffix and add unit tests for them Old version was more difficult to decipher and also did not work for 13 character names. Confirmed that the old version and new version behave the same way with randomly generated names that are no greater than 12 characters. (That code is not in this commit.) --- contracts/eosiolib/types.hpp | 46 ++++++++++++++++++------------- unittests/misc_tests.cpp | 52 ++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 19 deletions(-) diff --git a/contracts/eosiolib/types.hpp b/contracts/eosiolib/types.hpp index db79c767e37..e10b383eda7 100644 --- a/contracts/eosiolib/types.hpp +++ b/contracts/eosiolib/types.hpp @@ -67,28 +67,36 @@ namespace eosio { #define N(X) ::eosio::string_to_name(#X) - static constexpr uint64_t name_suffix( uint64_t tmp ) { - uint64_t suffix = 0; - bool endsuffix = false; - uint32_t offset = 0; - for( uint32_t i = 0; i <= 12; ++i, ++offset ) { - auto p = tmp >> 59; - if( !p ) { - endsuffix = true; - } else { - if( !endsuffix ) { - suffix |= uint64_t(p) << (59-(5*offset)); - } + static constexpr uint64_t name_suffix( uint64_t n ) { + uint32_t remaining_bits_after_last_actual_dot = 0; + uint32_t tmp = 0; + for( int32_t remaining_bits = 59; remaining_bits >= 4; remaining_bits -= 5 ) { // Note: remaining_bits must remain signed integer + // Get characters one-by-one in name in order from left to right (not including the 13th character) + auto c = (n >> remaining_bits) & 0x1Full; + if( !c ) { // if this character is a dot + tmp = static_cast(remaining_bits); + } else { // if this character is not a dot + remaining_bits_after_last_actual_dot = tmp; } - if( endsuffix && p ) { - endsuffix = false; - offset = 0; - suffix = uint64_t(p) << (59-(5*offset)); - } - tmp <<= 5; } - return suffix; + + uint64_t thirteenth_character = n & 0x0Full; + if( thirteenth_character ) { // if 13th character is not a dot + remaining_bits_after_last_actual_dot = tmp; + } + + if( remaining_bits_after_last_actual_dot == 0 ) // there is no actual dot in the name other than potentially leading dots + return n; + + // At this point remaining_bits_after_last_actual_dot has to be within the range of 4 to 59 (and restricted to increments of 5). + + // Mask for remaining bits corresponding to characters after last actual dot, except for 4 least significant bits (corresponds to 13th character). + uint64_t mask = (1ull << remaining_bits_after_last_actual_dot) - 16; + uint32_t shift = 64 - remaining_bits_after_last_actual_dot; + + return ( ((n & mask) << shift) + (thirteenth_character << (shift-1)) ); } + /** * @brief wraps a uint64_t to ensure it is only passed to methods that expect a Name * @details wraps a uint64_t to ensure it is only passed to methods that expect a Name and diff --git a/unittests/misc_tests.cpp b/unittests/misc_tests.cpp index 3f8fb23e10b..e0854e5b5e1 100644 --- a/unittests/misc_tests.cpp +++ b/unittests/misc_tests.cpp @@ -25,13 +25,65 @@ using namespace eosio::chain; using namespace eosio::testing; +#include +#include + namespace eosio { using namespace chain; using namespace std; +static constexpr uint64_t name_suffix( uint64_t n ) { + uint32_t remaining_bits_after_last_actual_dot = 0; + uint32_t tmp = 0; + for( int32_t remaining_bits = 59; remaining_bits >= 4; remaining_bits -= 5 ) { // Note: remaining_bits must remain signed integer + // Get characters one-by-one in name in order from left to right (not including the 13th character) + auto c = (n >> remaining_bits) & 0x1Full; + if( !c ) { // if this character is a dot + tmp = static_cast(remaining_bits); + } else { // if this character is not a dot + remaining_bits_after_last_actual_dot = tmp; + } + } + + uint64_t thirteenth_character = n & 0x0Full; + if( thirteenth_character ) { // if 13th character is not a dot + remaining_bits_after_last_actual_dot = tmp; + } + + if( remaining_bits_after_last_actual_dot == 0 ) // there is no actual dot in the name other than potentially leading dots + return n; + + // At this point remaining_bits_after_last_actual_dot has to be within the range of 4 to 59 (and restricted to increments of 5). + + // Mask for remaining bits corresponding to characters after last actual dot, except for 4 least significant bits (corresponds to 13th character). + uint64_t mask = (1ull << remaining_bits_after_last_actual_dot) - 16; + uint32_t shift = 64 - remaining_bits_after_last_actual_dot; + + return ( ((n & mask) << shift) + (thirteenth_character << (shift-1)) ); +} + BOOST_AUTO_TEST_SUITE(misc_tests) +BOOST_AUTO_TEST_CASE(name_suffix_tests) +{ + BOOST_CHECK_EQUAL( name{name_suffix(0)}, name{0} ); + BOOST_CHECK_EQUAL( name{name_suffix(N(abcdehijklmn))}, name{N(abcdehijklmn)} ); + BOOST_CHECK_EQUAL( name{name_suffix(N(abcdehijklmn1))}, name{N(abcdehijklmn1)} ); + BOOST_CHECK_EQUAL( name{name_suffix(N(abc.def))}, name{N(def)} ); + BOOST_CHECK_EQUAL( name{name_suffix(N(.abc.def))}, name{N(def)} ); + BOOST_CHECK_EQUAL( name{name_suffix(N(..abc.def))}, name{N(def)} ); + BOOST_CHECK_EQUAL( name{name_suffix(N(abc..def))}, name{N(def)} ); + BOOST_CHECK_EQUAL( name{name_suffix(N(abc.def.ghi))}, name{N(ghi)} ); + BOOST_CHECK_EQUAL( name{name_suffix(N(.abcdefghij))}, name{N(abcdefghij)} ); + BOOST_CHECK_EQUAL( name{name_suffix(N(.abcdefghij.1))}, name{N(1)} ); + BOOST_CHECK_EQUAL( name{name_suffix(N(a.bcdefghij))}, name{N(bcdefghij)} ); + BOOST_CHECK_EQUAL( name{name_suffix(N(a.bcdefghij.1))}, name{N(1)} ); + BOOST_CHECK_EQUAL( name{name_suffix(N(......a.b.c))}, name{N(c)} ); + BOOST_CHECK_EQUAL( name{name_suffix(N(abcdefhi.123))}, name{N(123)} ); + BOOST_CHECK_EQUAL( name{name_suffix(N(abcdefhij.123))}, name{N(123)} ); +} + /// Test processing of unbalanced strings BOOST_AUTO_TEST_CASE(json_from_string_test) {