Skip to content
This repository has been archived by the owner on Feb 21, 2019. It is now read-only.

Commit

Permalink
Track delegate ids and other ids separately in slate_records; #1358
Browse files Browse the repository at this point in the history
  • Loading branch information
@vikramrajkumar
vikramrajkumar committed Feb 6, 2015
1 parent 4a73089 commit d4e9b35
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 40 deletions.
2 changes: 1 addition & 1 deletion libraries/blockchain/account_operations.cpp
View file
Expand Up @@ -155,7 +155,7 @@ namespace bts { namespace blockchain {
FC_CAPTURE_AND_THROW( missing_signature, (*this) );

account->delegate_info->pay_balance -= this->amount;
eval_state.delta_votes[ account_id ] -= this->amount;
eval_state.delegate_vote_deltas[ account_id ] -= this->amount;
eval_state.add_balance( asset( this->amount, 0 ) );

eval_state._current_state->store_account_record( *account );
Expand Down
2 changes: 1 addition & 1 deletion libraries/blockchain/chain_database.cpp
View file
Expand Up @@ -2409,7 +2409,7 @@ namespace bts { namespace blockchain {
oslate_record slate_record = get_slate_record( balance.slate_id() );
FC_ASSERT(slate_record.valid(), "Unknown slate ID found in balance.");

for (account_id_type delegate : slate_record->slate)
for (account_id_type delegate : slate_record->delegate_slate)
calculated_balances[delegate] += balance.balance;
}
}
Expand Down
2 changes: 1 addition & 1 deletion libraries/blockchain/include/bts/blockchain/config.hpp
View file
Expand Up @@ -7,7 +7,7 @@

#define BTS_TEST_NETWORK_VERSION 82 // autogenerated

#define BTS_BLOCKCHAIN_DATABASE_VERSION 191
#define BTS_BLOCKCHAIN_DATABASE_VERSION 192

#define BTS_ADDRESS_PREFIX "XTS"
#define BTS_BLOCKCHAIN_SYMBOL "XTS"
Expand Down
7 changes: 4 additions & 3 deletions libraries/blockchain/include/bts/blockchain/slate_record.hpp
View file
Expand Up @@ -8,15 +8,16 @@ class chain_interface;
struct slate_db_interface;
struct slate_record
{
set<account_id_type> slate;
set<account_id_type> delegate_slate;
set<account_id_type> other_slate;

slate_id_type id()const;

static const slate_db_interface& db_interface( const chain_interface& );
void sanity_check( const chain_interface& )const;
};
typedef optional<slate_record> oslate_record;

struct slate_db_interface
{
std::function<oslate_record( const slate_id_type )> lookup_by_id;
Expand All @@ -30,4 +31,4 @@ struct slate_db_interface

} } // bts::blockchain

FC_REFLECT( bts::blockchain::slate_record, (slate) )
FC_REFLECT( bts::blockchain::slate_record, (delegate_slate)(other_slate) )
23 changes: 12 additions & 11 deletions libraries/blockchain/include/bts/blockchain/transaction_evaluation_state.hpp
View file
Expand Up @@ -25,28 +25,27 @@ namespace bts { namespace blockchain {
public:
transaction_evaluation_state( pending_chain_state* current_state = nullptr );

virtual ~transaction_evaluation_state();
virtual share_type get_fees( asset_id_type id = 0)const;
~transaction_evaluation_state();
share_type get_fees( asset_id_type id = 0 )const;

virtual void evaluate( const signed_transaction& trx );
virtual void evaluate_operation( const operation& op );
virtual bool verify_authority( const multisig_meta_info& siginfo );
void evaluate( const signed_transaction& trx );
void evaluate_operation( const operation& op );
bool verify_authority( const multisig_meta_info& siginfo );

/** perform any final operations based upon the current state of
* the operation such as updating fees paid etc.
*/
virtual void post_evaluate();
void post_evaluate();

/** can be specalized to handle many different forms of
* fee payment.
*/
virtual void validate_required_fee();
void validate_required_fee();

/**
* apply collected vote changes
*/
virtual void update_delegate_votes();
virtual void verify_delegate_id( const account_id_type id )const;
void update_delegate_votes();

bool check_signature( const address& a )const;
bool check_multisig( const multisig_condition& a )const;
Expand Down Expand Up @@ -101,7 +100,7 @@ namespace bts { namespace blockchain {
*/
map<asset_id_type, share_type> balance;

unordered_map<account_id_type, share_type> delta_votes;
unordered_map<account_id_type, share_type> delegate_vote_deltas;

// Not serialized
chain_interface* _current_state = nullptr;
Expand All @@ -110,7 +109,9 @@ namespace bts { namespace blockchain {
bool _enforce_canonical_signatures = false;
bool _skip_vote_adjustment = false;

// For pay_fee op
unordered_map<asset_id_type, share_type> _max_fee;

uint32_t current_op_index = 0;
};
typedef shared_ptr<transaction_evaluation_state> transaction_evaluation_state_ptr;
Expand All @@ -128,5 +129,5 @@ FC_REFLECT( bts::blockchain::transaction_evaluation_state,
(required_fees)
(alt_fees_paid)
(balance)
(delta_votes)
(delegate_vote_deltas)
)
5 changes: 4 additions & 1 deletion libraries/blockchain/slate_operations.cpp
View file
Expand Up @@ -6,14 +6,17 @@ namespace bts { namespace blockchain {

void define_slate_operation::evaluate( transaction_evaluation_state& eval_state )const
{ try {
FC_ASSERT( !slate.empty() );
if( this->slate.size() > BTS_BLOCKCHAIN_MAX_SLATE_SIZE )
FC_CAPTURE_AND_THROW( too_may_delegates_in_slate, (slate.size()) );

slate_record record;
for( const signed_int id : this->slate )
{
FC_ASSERT( id >= 0 );
record.slate.insert( id );
const oaccount_record delegate_record = eval_state._current_state->get_account_record( id );
if( delegate_record.valid() && delegate_record->is_delegate() ) record.delegate_slate.insert( id );
else record.other_slate.insert( id );
}

const slate_id_type slate_id = record.id();
Expand Down
7 changes: 4 additions & 3 deletions libraries/blockchain/slate_record.cpp
View file
Expand Up @@ -8,9 +8,10 @@ namespace bts { namespace blockchain {

slate_id_type slate_record::id()const
{
if( slate.empty() ) return 0;
if( delegate_slate.empty() && other_slate.empty() ) return 0;
fc::sha256::encoder enc;
fc::raw::pack( enc, this->slate );
if( !delegate_slate.empty() ) fc::raw::pack( enc, delegate_slate );
if( !other_slate.empty() ) fc::raw::pack( enc, other_slate );
return enc.result()._hash[ 0 ];
}

Expand All @@ -21,7 +22,7 @@ const slate_db_interface& slate_record::db_interface( const chain_interface& db

void slate_record::sanity_check( const chain_interface& db )const
{ try {
FC_ASSERT( !slate.empty() );
FC_ASSERT( !delegate_slate.empty() || !other_slate.empty() );
} FC_CAPTURE_AND_RETHROW( (*this) ) }

oslate_record slate_db_interface::lookup( const slate_id_type id )const
Expand Down
16 changes: 4 additions & 12 deletions libraries/blockchain/transaction_evaluation_state.cpp
View file
Expand Up @@ -95,21 +95,13 @@ namespace bts { namespace blockchain {
return any_parent_has_signed( record.name );
} FC_CAPTURE_AND_RETHROW( (record) ) }

void transaction_evaluation_state::verify_delegate_id( const account_id_type id )const
{ try {
auto current_account = _current_state->get_account_record( id );
if( !current_account ) FC_CAPTURE_AND_THROW( unknown_account_id, (id) );
if( !current_account->is_delegate() ) FC_CAPTURE_AND_THROW( not_a_delegate, (id) );
} FC_CAPTURE_AND_RETHROW( (id) ) }

void transaction_evaluation_state::update_delegate_votes()
{ try {
for( const auto& item : delta_votes )
for( const auto& item : delegate_vote_deltas )
{
const account_id_type id = item.first;
oaccount_record delegate_record = _current_state->get_account_record( id );
if( !delegate_record.valid() || !delegate_record->is_delegate() )
continue;
FC_ASSERT( delegate_record.valid() && delegate_record->is_delegate() );

const share_type amount = item.second;
delegate_record->adjust_votes_for( amount );
Expand Down Expand Up @@ -259,8 +251,8 @@ namespace bts { namespace blockchain {
if( !slate_record.valid() )
FC_CAPTURE_AND_THROW( unknown_delegate_slate, (slate_id) );

for( const account_id_type id : slate_record->slate )
delta_votes[ id ] += amount;
for( const account_id_type id : slate_record->delegate_slate )
delegate_vote_deltas[ id ] += amount;
} FC_CAPTURE_AND_RETHROW( (slate_id)(amount) ) }

share_type transaction_evaluation_state::get_fees( asset_id_type id )const
Expand Down
5 changes: 4 additions & 1 deletion libraries/client/blockchain_api.cpp
View file
Expand Up @@ -183,13 +183,16 @@ map<account_id_type, string> detail::client_impl::blockchain_get_slate( const st
const oslate_record slate_record = _chain_db->get_slate_record( slate_id );
FC_ASSERT( slate_record.valid() );

for( const account_id_type id : slate_record->slate )
for( const account_id_type id : slate_record->delegate_slate )
{
const oaccount_record delegate_record = _chain_db->get_account_record( id );
if( delegate_record.valid() ) delegates[ id ] = delegate_record->name;
else delegates[ id ] = std::to_string( id );
}

for( const account_id_type id : slate_record->other_slate )
delegates[ id ] = std::to_string( id );

return delegates;
}

Expand Down
12 changes: 6 additions & 6 deletions libraries/wallet/wallet.cpp
View file
Expand Up @@ -706,7 +706,7 @@ namespace detail {
slate_id = record.id();
if( slate_id == 0 ) return slate_id;

if( !_blockchain->get_slate_record( slate_id ).valid() ) transaction.define_slate( record.slate );
if( !_blockchain->get_slate_record( slate_id ).valid() ) transaction.define_slate( record.delegate_slate );
transaction.set_slates( slate_id );
return slate_id;
} FC_CAPTURE_AND_RETHROW( (transaction)(strategy) ) }
Expand Down Expand Up @@ -816,7 +816,7 @@ namespace detail {
continue;
}

for( const auto recommended_candidate : recommendations->slate )
for( const auto recommended_candidate : recommendations->delegate_slate )
++recommended_candidate_ranks[recommended_candidate];
}

Expand Down Expand Up @@ -864,8 +864,8 @@ namespace detail {
}

slate_record record;
for( const account_id_type id : for_candidates ) record.slate.insert( id );
FC_ASSERT( record.slate.size() <= BTS_BLOCKCHAIN_MAX_SLATE_SIZE );
for( const account_id_type id : for_candidates ) record.delegate_slate.insert( id );
FC_ASSERT( record.delegate_slate.size() <= BTS_BLOCKCHAIN_MAX_SLATE_SIZE );

return record;
}
Expand Down Expand Up @@ -1655,7 +1655,7 @@ namespace detail {
summary.up_to_date_with_recommendation = false;
auto oslate = my->_blockchain->get_slate_record( record.slate_id() );
if( oslate.valid() )
total += record.get_spendable_balance( my->_blockchain->now() ).amount * oslate->slate.size();
total += record.get_spendable_balance( my->_blockchain->now() ).amount * oslate->delegate_slate.size();
total_possible += record.get_spendable_balance( my->_blockchain->now() ).amount * BTS_BLOCKCHAIN_MAX_SLATE_SIZE;
}
}
Expand Down Expand Up @@ -4502,7 +4502,7 @@ namespace detail {
const auto slate_record = pending_state->get_slate_record( slate_id );
if( !slate_record.valid() ) FC_THROW_EXCEPTION( unknown_slate, "Unknown slate!" );

for( const account_id_type delegate_id : slate_record->slate )
for( const account_id_type delegate_id : slate_record->delegate_slate )
{
if( raw_votes.count( delegate_id ) <= 0 ) raw_votes[ delegate_id ] = balance.amount;
else raw_votes[ delegate_id ] += balance.amount;
Expand Down

3 comments on commit d4e9b35

@arhag
Copy link

@arhag arhag commented on d4e9b35 Feb 6, 2015

Choose a reason for hiding this comment

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

What if the client wants to use an int in other_slate that is currently a delegate ID? What if the client uses an int that is currently not a delegate ID, but between the time the signed transaction is broadcast and included (before expiration) into the blockchain, someone registers a delegate that happens to have that same ID? I don't think the system compares to the state of account registration at the time the transaction was signed, does it? If not, that means that the transaction would have inadvertently voted for that delegate. And even if so, time would not be good enough because of possible forks (the transaction would need to include the hash of the last block it saw prior to crafting the transaction).

There needs to be a clearer separation in the slate definition of a transaction between an int meant as a vote for a delegate and an arbitrary int used for non-binding proposal systems. Perhaps you can require that all positive ints in the slate be for currently registered delegates (whether retracted or not), and allow negative ints to be included in the slate (which would be stored in other_slate) for whatever non-binding proposal purposes people want to use them for. Although the disadvantage there is that we cannot efficiently upvote or downvote a proposal ID and would instead require two proposals representing an upvote or a downvote of the actual proposal they both refer to (that is a hack, but then again the entire non-binding proposal system is a hack for now).

@vikramrajkumar
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the negatives idea--this will ensure that we have no collisions. So I will restore the original requirement that all positives be existing delegates, and save the negatives but ignore them for now.

@bytemaster
Copy link
Contributor

Choose a reason for hiding this comment

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

I am modifying this to use the negative numbers and also check for hash collisions on the 64 bit slate id.

Please sign in to comment.