Skip to content

Commit

Permalink
Clean up some non-determisitic behavior, maybe fix bitshares#485
Browse files Browse the repository at this point in the history
1. Replace ordered_non_unique indexes with composite keys / ordered_unique, using object_id as tiebreaker.
2. Make some casts more explicit.

This commit was rebased by theoreticalbts due to conflicts with the patches for bitshares#466 bitshares#562 including bugfixes

- Fix flipped comparison operator
- Implement operator> and operator!= for object_id_type
  • Loading branch information
bytemaster authored and theoreticalbts committed Feb 11, 2016
1 parent 2eaa1b5 commit 146c0c4
Show file tree
Hide file tree
Showing 14 changed files with 134 additions and 67 deletions.
4 changes: 2 additions & 2 deletions libraries/app/database_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -960,8 +960,8 @@ vector<call_order_object> database_api_impl::get_margin_positions( const account
{
const auto& idx = _db.get_index_type<call_order_index>();
const auto& aidx = idx.indices().get<by_account>();
auto start = aidx.lower_bound( boost::make_tuple( id, 0 ) );
auto end = aidx.lower_bound( boost::make_tuple( id+1, 0 ) );
auto start = aidx.lower_bound( boost::make_tuple( id, asset_id_type(0) ) );
auto end = aidx.lower_bound( boost::make_tuple( id+1, asset_id_type(0) ) );
vector<call_order_object> result;
while( start != end )
{
Expand Down
18 changes: 9 additions & 9 deletions libraries/chain/db_init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ void database::init_genesis(const genesis_state_type& genesis_state)
a.statistics = create<account_statistics_object>([&](account_statistics_object& s){s.owner = a.id;}).id;
a.owner.weight_threshold = 1;
a.active.weight_threshold = 1;
a.registrar = a.lifetime_referrer = a.referrer = id;
a.registrar = a.lifetime_referrer = a.referrer = account_id_type(id);
a.membership_expiration_date = time_point_sec::maximum();
a.network_fee_percentage = GRAPHENE_DEFAULT_NETWORK_PERCENT_OF_FEE;
a.lifetime_referrer_fee_percentage = GRAPHENE_100_PERCENT - GRAPHENE_DEFAULT_NETWORK_PERCENT_OF_FEE;
Expand All @@ -339,9 +339,9 @@ void database::init_genesis(const genesis_state_type& genesis_state)
a.options.issuer_permissions = 0;
a.issuer = GRAPHENE_NULL_ACCOUNT;
a.options.core_exchange_rate.base.amount = 1;
a.options.core_exchange_rate.base.asset_id = 0;
a.options.core_exchange_rate.base.asset_id = asset_id_type(0);
a.options.core_exchange_rate.quote.amount = 1;
a.options.core_exchange_rate.quote.asset_id = 0;
a.options.core_exchange_rate.quote.asset_id = asset_id_type(0);
a.dynamic_asset_data_id = dyn_asset.id;
});
assert( asset_id_type(core_asset.id) == asset().asset_id );
Expand All @@ -364,9 +364,9 @@ void database::init_genesis(const genesis_state_type& genesis_state)
a.options.issuer_permissions = 0;
a.issuer = GRAPHENE_NULL_ACCOUNT;
a.options.core_exchange_rate.base.amount = 1;
a.options.core_exchange_rate.base.asset_id = 0;
a.options.core_exchange_rate.base.asset_id = asset_id_type(0);
a.options.core_exchange_rate.quote.amount = 1;
a.options.core_exchange_rate.quote.asset_id = 0;
a.options.core_exchange_rate.quote.asset_id = asset_id_type(0);
a.dynamic_asset_data_id = dyn_asset.id;
});
FC_ASSERT( asset_obj.get_id() == asset_id_type(id) );
Expand Down Expand Up @@ -491,7 +491,7 @@ void database::init_genesis(const genesis_state_type& genesis_state)
GRAPHENE_DEFAULT_MAINTENANCE_COLLATERAL_RATIO);
});

total_supplies[ 0 ] += collateral_rec.collateral;
total_supplies[ asset_id_type(0) ] += collateral_rec.collateral;
total_debts[ new_asset_id ] += collateral_rec.debt;
++collateral_holder_number;
}
Expand Down Expand Up @@ -555,13 +555,13 @@ void database::init_genesis(const genesis_state_type& genesis_state)
total_supplies[ asset_id ] += vest.amount;
}

if( total_supplies[ 0 ] > 0 )
if( total_supplies[ asset_id_type(0) ] > 0 )
{
adjust_balance(GRAPHENE_COMMITTEE_ACCOUNT, -get_balance(GRAPHENE_COMMITTEE_ACCOUNT,{}));
}
else
{
total_supplies[ 0 ] = GRAPHENE_MAX_SHARE_SUPPLY;
total_supplies[ asset_id_type(0) ] = GRAPHENE_MAX_SHARE_SUPPLY;
}

const auto& idx = get_index_type<asset_index>().indices().get<by_symbol>();
Expand Down Expand Up @@ -645,7 +645,7 @@ void database::init_genesis(const genesis_state_type& genesis_state)
modify(get_global_properties(), [&](global_property_object& p) {
for( uint32_t i = 1; i <= genesis_state.initial_active_witnesses; ++i )
{
p.active_witnesses.insert(i);
p.active_witnesses.insert(witness_id_type(i));
}
});

Expand Down
7 changes: 6 additions & 1 deletion libraries/chain/include/graphene/chain/asset_object.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,12 @@ namespace graphene { namespace chain {
indexed_by<
ordered_unique< tag<by_id>, member< object, object_id_type, &object::id > >,
ordered_unique< tag<by_symbol>, member<asset_object, string, &asset_object::symbol> >,
ordered_non_unique< tag<by_type>, const_mem_fun<asset_object, bool, &asset_object::is_market_issued> >
ordered_unique< tag<by_type>,
composite_key< asset_object,
const_mem_fun<asset_object, bool, &asset_object::is_market_issued>,
member< object, object_id_type, &object::id >
>
>
>
> asset_object_multi_index_type;
typedef generic_index<asset_object, asset_object_multi_index_type> asset_index;
Expand Down
30 changes: 21 additions & 9 deletions libraries/chain/include/graphene/chain/market_object.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,17 +72,26 @@ struct by_account;
typedef multi_index_container<
limit_order_object,
indexed_by<
ordered_unique< tag<by_id>,
member< object, object_id_type, &object::id > >,
ordered_non_unique< tag<by_expiration>, member< limit_order_object, time_point_sec, &limit_order_object::expiration> >,
ordered_unique< tag<by_id>, member< object, object_id_type, &object::id > >,
ordered_unique< tag<by_expiration>,
composite_key< limit_order_object,
member< limit_order_object, time_point_sec, &limit_order_object::expiration>,
member< object, object_id_type, &object::id>
>
>,
ordered_unique< tag<by_price>,
composite_key< limit_order_object,
member< limit_order_object, price, &limit_order_object::sell_price>,
member< object, object_id_type, &object::id>
>,
composite_key_compare< std::greater<price>, std::less<object_id_type> >
>,
ordered_non_unique< tag<by_account>, member<limit_order_object, account_id_type, &limit_order_object::seller>>
ordered_unique< tag<by_account>,
composite_key< limit_order_object,
member<limit_order_object, account_id_type, &limit_order_object::seller>,
member<object, object_id_type, &object::id>
>
>
>
> limit_order_multi_index_type;

Expand Down Expand Up @@ -168,19 +177,22 @@ typedef multi_index_container<
force_settlement_object,
indexed_by<
ordered_unique< tag<by_id>, member< object, object_id_type, &object::id > >,
ordered_non_unique< tag<by_account>,
member<force_settlement_object, account_id_type, &force_settlement_object::owner>
ordered_unique< tag<by_account>,
composite_key< force_settlement_object,
member<force_settlement_object, account_id_type, &force_settlement_object::owner>,
member< object, object_id_type, &object::id >
>
>,
ordered_non_unique< tag<by_expiration>,
ordered_unique< tag<by_expiration>,
composite_key< force_settlement_object,
const_mem_fun<force_settlement_object, asset_id_type, &force_settlement_object::settlement_asset_id>,
member<force_settlement_object, time_point_sec, &force_settlement_object::settlement_date>
member<force_settlement_object, time_point_sec, &force_settlement_object::settlement_date>,
member< object, object_id_type, &object::id >
>
>
>
> force_settlement_object_multi_index_type;


typedef generic_index<call_order_object, call_order_multi_index_type> call_order_index;
typedef generic_index<force_settlement_object, force_settlement_object_multi_index_type> force_settlement_index;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#pragma once
#include <graphene/chain/protocol/authority.hpp>
#include <graphene/db/generic_index.hpp>
#include <boost/multi_index/composite_key.hpp>

namespace graphene { namespace chain {

Expand Down Expand Up @@ -78,9 +79,24 @@ namespace graphene { namespace chain {
withdraw_permission_object,
indexed_by<
ordered_unique< tag<by_id>, member< object, object_id_type, &object::id > >,
ordered_non_unique< tag<by_from>, member<withdraw_permission_object, account_id_type, &withdraw_permission_object::withdraw_from_account> >,
ordered_non_unique< tag<by_authorized>, member<withdraw_permission_object, account_id_type, &withdraw_permission_object::authorized_account> >,
ordered_non_unique< tag<by_expiration>, member<withdraw_permission_object, time_point_sec, &withdraw_permission_object::expiration> >
ordered_unique< tag<by_from>,
composite_key< withdraw_permission_object,
member<withdraw_permission_object, account_id_type, &withdraw_permission_object::withdraw_from_account>,
member< object, object_id_type, &object::id >
>
>,
ordered_unique< tag<by_authorized>,
composite_key< withdraw_permission_object,
member<withdraw_permission_object, account_id_type, &withdraw_permission_object::authorized_account>,
member< object, object_id_type, &object::id >
>
>,
ordered_unique< tag<by_expiration>,
composite_key< withdraw_permission_object,
member<withdraw_permission_object, time_point_sec, &withdraw_permission_object::expiration>,
member< object, object_id_type, &object::id >
>
>
>
> withdraw_permission_object_multi_index_type;

Expand Down
2 changes: 1 addition & 1 deletion libraries/chain/protocol/asset_ops.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ void asset_issue_operation::validate()const
FC_ASSERT( fee.amount >= 0 );
FC_ASSERT( asset_to_issue.amount.value <= GRAPHENE_MAX_SHARE_SUPPLY );
FC_ASSERT( asset_to_issue.amount.value > 0 );
FC_ASSERT( asset_to_issue.asset_id != 0 );
FC_ASSERT( asset_to_issue.asset_id != asset_id_type(0) );
}

void asset_fund_fee_pool_operation::validate() const
Expand Down
2 changes: 1 addition & 1 deletion libraries/chain/protocol/fee_schedule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ namespace graphene { namespace chain {
scaled /= GRAPHENE_100_PERCENT;
FC_ASSERT( scaled <= GRAPHENE_MAX_SHARE_SUPPLY );
//idump( (base_value)(scaled)(core_exchange_rate) );
auto result = asset( scaled.to_uint64(), 0 ) * core_exchange_rate;
auto result = asset( scaled.to_uint64(), asset_id_type(0) ) * core_exchange_rate;
//FC_ASSERT( result * core_exchange_rate >= asset( scaled.to_uint64()) );

while( result * core_exchange_rate < asset( scaled.to_uint64()) )
Expand Down
37 changes: 33 additions & 4 deletions libraries/db/include/graphene/db/object_id.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,18 @@ namespace graphene { namespace db {
uint16_t space_type()const { return number >> 48; }
uint64_t instance()const { return number & GRAPHENE_DB_MAX_INSTANCE_ID; }
bool is_null()const { return number == 0; }
operator uint64_t()const { return number; }
explicit operator uint64_t()const { return number; }

friend bool operator == ( const object_id_type& a, const object_id_type& b )
{
return a.number == b.number;
}

friend bool operator != ( const object_id_type& a, const object_id_type& b )
{
return a.number != b.number;
}

object_id_type& operator++(int) { ++number; return *this; }
object_id_type& operator++() { ++number; return *this; }

Expand All @@ -68,11 +74,16 @@ namespace graphene { namespace db {
}
friend size_t hash_value( object_id_type v ) { return std::hash<uint64_t>()(v.number); }

friend bool operator < ( const object_id_type& a, const object_id_type& b )
friend bool operator< ( const object_id_type& a, const object_id_type& b )
{
return a.number < b.number;
}

friend bool operator> ( const object_id_type& a, const object_id_type& b )
{
return a.number > b.number;
}

template< typename T >
bool is() const
{
Expand Down Expand Up @@ -106,7 +117,7 @@ namespace graphene { namespace db {

object_id(){}
object_id( unsigned_int i ):instance(i){}
object_id( uint64_t i ):instance(i)
explicit object_id( uint64_t i ):instance(i)
{
FC_ASSERT( (i >> 48) == 0 );
}
Expand All @@ -118,11 +129,25 @@ namespace graphene { namespace db {
friend object_id operator+(const object_id a, int delta ) { return object_id( uint64_t(a.instance.value+delta) ); }

operator object_id_type()const { return object_id_type( SpaceID, TypeID, instance.value ); }
operator uint64_t()const { return object_id_type( *this ).number; }
explicit operator uint64_t()const { return object_id_type( *this ).number; }

template<typename DB>
const T& operator()(const DB& db)const { return db.get(*this); }

friend bool operator != ( const object_id& a, const object_id& b )
{
return a.instance != b.instance;
}
friend bool operator != ( const object_id_type& a, const object_id& b )
{
return a != object_id_type(b);
}
friend bool operator != ( const object_id& a, const object_id_type& b )
{
return object_id_type(a) != b;
}


friend bool operator == ( const object_id& a, const object_id& b )
{
return a.instance == b.instance;
Expand All @@ -139,6 +164,10 @@ namespace graphene { namespace db {
{
return a.instance.value < b.instance.value;
}
friend bool operator > ( const object_id& a, const object_id& b )
{
return a.instance.value > b.instance.value;
}
friend size_t hash_value( object_id v ) { return std::hash<uint64_t>()(v.instance.value); }

unsigned_int instance;
Expand Down
8 changes: 4 additions & 4 deletions libraries/wallet/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2324,7 +2324,7 @@ class wallet_api_impl
asset_options opts;
opts.flags &= ~(white_list | disable_force_settle | global_settle);
opts.issuer_permissions = opts.flags;
opts.core_exchange_rate = price(asset(1), asset(1,1));
opts.core_exchange_rate = price(asset(1), asset(1,asset_id_type(1)));
create_asset(get_account(creator).name, symbol, 2, opts, {}, true);
}

Expand All @@ -2333,7 +2333,7 @@ class wallet_api_impl
asset_options opts;
opts.flags &= ~white_list;
opts.issuer_permissions = opts.flags;
opts.core_exchange_rate = price(asset(1), asset(1,1));
opts.core_exchange_rate = price(asset(1), asset(1,asset_id_type(1)));
bitasset_options bopts;
create_asset(get_account(creator).name, symbol, 2, opts, bopts, true);
}
Expand Down Expand Up @@ -3644,8 +3644,8 @@ vector<asset> wallet_api::get_blind_balances( string key_or_label )

auto pub_key = get_public_key( key_or_label );
auto& to_asset_used_idx = my->_wallet.blind_receipts.get<by_to_asset_used>();
auto start = to_asset_used_idx.lower_bound( std::make_tuple(pub_key,0,false) );
auto end = to_asset_used_idx.lower_bound( std::make_tuple(pub_key,uint32_t(0xffffffff),true) );
auto start = to_asset_used_idx.lower_bound( std::make_tuple(pub_key,asset_id_type(0),false) );
auto end = to_asset_used_idx.lower_bound( std::make_tuple(pub_key,asset_id_type(uint32_t(0xffffffff)),true) );
while( start != end )
{
if( !start->used )
Expand Down
8 changes: 4 additions & 4 deletions tests/common/database_fixture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ const asset_object& database_fixture::create_bitasset(
flags |= witness_fed_asset;
creator.common_options.issuer_permissions = flags;
creator.common_options.flags = flags & ~global_settle;
creator.common_options.core_exchange_rate = price({asset(1,1),asset(1)});
creator.common_options.core_exchange_rate = price({asset(1,asset_id_type(1)),asset(1)});
creator.bitasset_opts = bitasset_options();
trx.operations.push_back(std::move(creator));
trx.validate();
Expand All @@ -466,7 +466,7 @@ const asset_object& database_fixture::create_prediction_market(
creator.common_options.flags = flags & ~global_settle;
if( issuer == GRAPHENE_WITNESS_ACCOUNT )
creator.common_options.flags |= witness_fed_asset;
creator.common_options.core_exchange_rate = price({asset(1,1),asset(1)});
creator.common_options.core_exchange_rate = price({asset(1,asset_id_type(1)),asset(1)});
creator.bitasset_opts = bitasset_options();
creator.is_prediction_market = true;
trx.operations.push_back(std::move(creator));
Expand All @@ -484,7 +484,7 @@ const asset_object& database_fixture::create_user_issued_asset( const string& na
creator.symbol = name;
creator.common_options.max_supply = 0;
creator.precision = 2;
creator.common_options.core_exchange_rate = price({asset(1,1),asset(1)});
creator.common_options.core_exchange_rate = price({asset(1,asset_id_type(1)),asset(1)});
creator.common_options.max_supply = GRAPHENE_MAX_SHARE_SUPPLY;
creator.common_options.flags = charge_market_fee;
creator.common_options.issuer_permissions = charge_market_fee;
Expand All @@ -503,7 +503,7 @@ const asset_object& database_fixture::create_user_issued_asset( const string& na
creator.symbol = name;
creator.common_options.max_supply = 0;
creator.precision = 2;
creator.common_options.core_exchange_rate = price({asset(1,1),asset(1)});
creator.common_options.core_exchange_rate = price({asset(1,asset_id_type(1)),asset(1)});
creator.common_options.max_supply = GRAPHENE_MAX_SHARE_SUPPLY;
creator.common_options.flags = flags;
creator.common_options.issuer_permissions = flags;
Expand Down
Loading

0 comments on commit 146c0c4

Please sign in to comment.