Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix line length, add preimage to HTLC virtual operation #2161

Merged
merged 2 commits into from
Apr 28, 2020
Merged

Fix line length, add preimage to HTLC virtual operation #2161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
06b0fe3
Fix line length, store preimage in virt op
jmjatlanta Apr 28, 2020
ccef9fd
Replace BOOST_TEST with BOOST_CHECK
jmjatlanta Apr 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions libraries/chain/htlc_evaluator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ namespace graphene {
// attempted on an HTLC with a 0 preimage size before the hardfork date.
if ( htlc_obj->conditions.hash_lock.preimage_size > 0U ||
block_time < HARDFORK_CORE_BSIP64_TIME )
FC_ASSERT(op.preimage.size() == htlc_obj->conditions.hash_lock.preimage_size, "Preimage size mismatch.");
FC_ASSERT(op.preimage.size() == htlc_obj->conditions.hash_lock.preimage_size,
"Preimage size mismatch.");
}
} // end of graphene::chain::details

Expand All @@ -78,9 +79,11 @@ namespace graphene {
FC_ASSERT(htlc_options, "HTLC Committee options are not set.");

// make sure the expiration is reasonable
FC_ASSERT( o.claim_period_seconds <= htlc_options->max_timeout_secs, "HTLC Timeout exceeds allowed length" );
FC_ASSERT( o.claim_period_seconds <= htlc_options->max_timeout_secs,
"HTLC Timeout exceeds allowed length" );
// make sure the preimage length is reasonable
FC_ASSERT( o.preimage_size <= htlc_options->max_preimage_size, "HTLC preimage length exceeds allowed length" );
FC_ASSERT( o.preimage_size <= htlc_options->max_preimage_size,
"HTLC preimage length exceeds allowed length" );
// make sure the sender has the funds for the HTLC
FC_ASSERT( d.get_balance( o.from, o.amount.asset_id) >= (o.amount), "Insufficient funds") ;
const auto& asset_to_transfer = o.amount.asset_id( d );
Expand Down Expand Up @@ -154,7 +157,8 @@ namespace graphene {
db().adjust_balance(htlc_obj->transfer.to, amount);
// notify related parties
htlc_redeemed_operation virt_op( htlc_obj->id, htlc_obj->transfer.from, htlc_obj->transfer.to, o.redeemer,
amount, htlc_obj->conditions.hash_lock.preimage_hash, htlc_obj->conditions.hash_lock.preimage_size );
amount, htlc_obj->conditions.hash_lock.preimage_hash, htlc_obj->conditions.hash_lock.preimage_size,
o.preimage );
db().push_applied_operation( virt_op );
db().remove(*htlc_obj);
return void_result();
Expand Down
21 changes: 12 additions & 9 deletions libraries/chain/proposal_evaluator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ struct proposal_operation_hardfork_visitor

void operator()(const graphene::chain::committee_member_update_global_parameters_operation &op) const {
if (block_time < HARDFORK_CORE_1468_TIME) {
FC_ASSERT(!op.new_parameters.extensions.value.updatable_htlc_options.valid(), "Unable to set HTLC options before hardfork 1468");
FC_ASSERT(!op.new_parameters.extensions.value.updatable_htlc_options.valid(),
"Unable to set HTLC options before hardfork 1468");
FC_ASSERT(!op.new_parameters.current_fees->exists<htlc_create_operation>());
FC_ASSERT(!op.new_parameters.current_fees->exists<htlc_redeem_operation>());
FC_ASSERT(!op.new_parameters.current_fees->exists<htlc_extend_operation>());
Expand Down Expand Up @@ -126,7 +127,8 @@ struct proposal_operation_hardfork_visitor
// Do not allow more than 1 proposal_update in a proposal
if ( op.op.is_type<proposal_update_operation>() )
{
FC_ASSERT( !already_contains_proposal_update, "At most one proposal update can be nested in a proposal!" );
FC_ASSERT( !already_contains_proposal_update,
"At most one proposal update can be nested in a proposal!" );
already_contains_proposal_update = true;
}
}
Expand Down Expand Up @@ -188,8 +190,9 @@ void_result proposal_create_evaluator::do_evaluate( const proposal_create_operat
FC_ASSERT( o.expiration_time > block_time, "Proposal has already expired on creation." );
FC_ASSERT( o.expiration_time <= block_time + global_parameters.maximum_proposal_lifetime,
"Proposal expiration time is too far in the future." );
FC_ASSERT( !o.review_period_seconds || fc::seconds( *o.review_period_seconds ) < ( o.expiration_time - block_time ),
"Proposal review period must be less than its overall lifetime." );
FC_ASSERT( !o.review_period_seconds ||
fc::seconds( *o.review_period_seconds ) < ( o.expiration_time - block_time ),
"Proposal review period must be less than its overall lifetime." );

// Find all authorities required by the proposed operations
flat_set<account_id_type> tmp_required_active_auths;
Expand All @@ -199,8 +202,8 @@ void_result proposal_create_evaluator::do_evaluate( const proposal_create_operat
operation_get_required_authorities( op.op, tmp_required_active_auths, _required_owner_auths, other,
MUST_IGNORE_CUSTOM_OP_REQD_AUTHS( block_time ) );
}
// All accounts which must provide both owner and active authority should be omitted from the active authority set;
// owner authority approval implies active authority approval.
// All accounts which must provide both owner and active authority should be omitted from the
// active authority set; owner authority approval implies active authority approval.
std::set_difference( tmp_required_active_auths.begin(), tmp_required_active_auths.end(),
_required_owner_auths.begin(), _required_owner_auths.end(),
std::inserter( _required_active_auths, _required_active_auths.begin() ) );
Expand Down Expand Up @@ -295,9 +298,9 @@ void_result proposal_update_evaluator::do_apply(const proposal_update_operation&
{ try {
database& d = db();

// Potential optimization: if _executed_proposal is true, we can skip the modify step and make push_proposal skip
// signature checks. This isn't done now because I just wrote all the proposals code, and I'm not yet 100% sure the
// required approvals are sufficient to authorize the transaction.
// Potential optimization: if _executed_proposal is true, we can skip the modify step and make push_proposal
// skip signature checks. This isn't done now because I just wrote all the proposals code, and I'm not yet
// 100% sure the required approvals are sufficient to authorize the transaction.
d.modify(*_proposal, [&o](proposal_object& p) {
p.available_active_approvals.insert(o.active_approvals_to_add.begin(), o.active_approvals_to_add.end());
p.available_owner_approvals.insert(o.owner_approvals_to_add.begin(), o.owner_approvals_to_add.end());
Expand Down
3 changes: 2 additions & 1 deletion libraries/protocol/htlc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ namespace graphene { namespace protocol {
FC_ASSERT( amount.amount > 0, "HTLC amount should be greater than zero" );
}

share_type htlc_create_operation::calculate_fee( const fee_parameters_type& fee_params, uint32_t fee_per_kb )const
share_type htlc_create_operation::calculate_fee( const fee_parameters_type& fee_params,
uint32_t fee_per_kb )const
{
uint64_t days = ( claim_period_seconds + SECONDS_PER_DAY - 1 ) / SECONDS_PER_DAY;
// multiply with overflow check
Expand Down
5 changes: 3 additions & 2 deletions libraries/protocol/include/graphene/protocol/htlc.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,10 @@ namespace graphene { namespace protocol {

htlc_redeemed_operation() {}
htlc_redeemed_operation( htlc_id_type htlc_id, account_id_type from, account_id_type to,
account_id_type redeemer, asset amount, const htlc_hash& preimage_hash, uint16_t preimage_size ) :
account_id_type redeemer, asset amount, const htlc_hash& preimage_hash, uint16_t preimage_size,
const std::vector<char>& preimage ) :
htlc_id(htlc_id), from(from), to(to), redeemer(redeemer), amount(amount),
htlc_preimage_hash(preimage_hash), htlc_preimage_size(preimage_size) {}
htlc_preimage_hash(preimage_hash), htlc_preimage_size(preimage_size), preimage(preimage) {}

account_id_type fee_payer()const { return to; }
void validate()const { FC_ASSERT( !"virtual operation" ); }
Expand Down
6 changes: 3 additions & 3 deletions libraries/wallet/operation_printer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,13 @@ void operation_printer::print_preimage(const std::vector<char>& preimage)const
if (preimage.size() == 0)
return;
out << " with preimage \"";
// cut it at 50 bytes max
// cut it at 300 bytes max
auto flags = out.flags();
out << std::hex << setw(2) << setfill('0');
for (size_t i = 0; i < std::min<size_t>(50, preimage.size()); i++)
for (size_t i = 0; i < std::min<size_t>(300, preimage.size()); i++)
out << +preimage[i];
out.flags(flags);
if (preimage.size() > 50)
if (preimage.size() > 300)
out << "...(truncated due to size)";
out << "\"";
}
Expand Down
6 changes: 3 additions & 3 deletions libraries/wallet/wallet_api_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,7 @@ class wallet_api_impl
transaction preview_builder_transaction(transaction_handle_type handle);
signed_transaction sign_builder_transaction(transaction_handle_type transaction_handle, bool broadcast = true);
signed_transaction sign_builder_transaction2(transaction_handle_type transaction_handle,
const vector<public_key_type>& signing_keys = vector<public_key_type>(),
bool broadcast = true);
const vector<public_key_type>& signing_keys = vector<public_key_type>(), bool broadcast = true);

pair<transaction_id_type,signed_transaction> broadcast_transaction(signed_transaction tx);

Expand Down Expand Up @@ -302,7 +301,8 @@ class wallet_api_impl
string hash_algorithm, const std::string& preimage_hash, uint32_t preimage_size,
const uint32_t claim_period_seconds, const std::string& memo, bool broadcast = false );

signed_transaction htlc_redeem( string htlc_id, string issuer, const std::vector<char>& preimage, bool broadcast );
signed_transaction htlc_redeem( string htlc_id, string issuer, const std::vector<char>& preimage,
bool broadcast );

signed_transaction htlc_extend ( string htlc_id, string issuer, const uint32_t seconds_to_add, bool broadcast);

Expand Down
10 changes: 5 additions & 5 deletions libraries/wallet/wallet_transfer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ namespace graphene { namespace wallet { namespace detail {
return sign_transaction(tx, broadcast);
} FC_CAPTURE_AND_RETHROW( (from)(to)(amount)(asset_symbol)(memo)(broadcast) ) }

signed_transaction wallet_api_impl::htlc_create( string source, string destination, string amount, string asset_symbol,
string hash_algorithm, const std::string& preimage_hash, uint32_t preimage_size,
signed_transaction wallet_api_impl::htlc_create( string source, string destination, string amount,
string asset_symbol, string hash_algorithm, const std::string& preimage_hash, uint32_t preimage_size,
const uint32_t claim_period_seconds, const std::string& memo, bool broadcast )
{
try
Expand Down Expand Up @@ -122,8 +122,8 @@ namespace graphene { namespace wallet { namespace detail {
(preimage_hash)(preimage_size)(claim_period_seconds)(broadcast) )
}

signed_transaction wallet_api_impl::htlc_redeem( string htlc_id, string issuer, const std::vector<char>& preimage,
bool broadcast )
signed_transaction wallet_api_impl::htlc_redeem( string htlc_id, string issuer,
const std::vector<char>& preimage, bool broadcast )
{
try
{
Expand All @@ -147,7 +147,7 @@ namespace graphene { namespace wallet { namespace detail {
} FC_CAPTURE_AND_RETHROW( (htlc_id)(issuer)(preimage)(broadcast) )
}

signed_transaction wallet_api_impl::htlc_extend ( string htlc_id, string issuer, const uint32_t seconds_to_add,
signed_transaction wallet_api_impl::htlc_extend ( string htlc_id, string issuer, const uint32_t seconds_to_add,
bool broadcast)
{
try
Expand Down
38 changes: 28 additions & 10 deletions tests/cli/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1601,7 +1601,8 @@ BOOST_AUTO_TEST_CASE( cli_create_htlc_bsip64 )
int server_port_number = 0;
app1 = start_application(app_dir, server_port_number);
// set committee parameters
app1->chain_database()->modify(app1->chain_database()->get_global_properties(), [](global_property_object& p) {
app1->chain_database()->modify(app1->chain_database()->get_global_properties(), [](global_property_object& p)
{
graphene::chain::htlc_options params;
params.max_preimage_size = 1024;
params.max_timeout_secs = 60 * 60 * 24 * 28;
Expand Down Expand Up @@ -1634,7 +1635,8 @@ BOOST_AUTO_TEST_CASE( cli_create_htlc_bsip64 )
account_object nathan_acct_after_upgrade = con.wallet_api_ptr->get_account("nathan");

// verify that the upgrade was successful
BOOST_CHECK_PREDICATE( std::not_equal_to<uint32_t>(), (nathan_acct_before_upgrade.membership_expiration_date.sec_since_epoch())
BOOST_CHECK_PREDICATE( std::not_equal_to<uint32_t>(),
(nathan_acct_before_upgrade.membership_expiration_date.sec_since_epoch())
(nathan_acct_after_upgrade.membership_expiration_date.sec_since_epoch()) );
BOOST_CHECK(nathan_acct_after_upgrade.is_lifetime_member());

Expand All @@ -1660,8 +1662,8 @@ BOOST_AUTO_TEST_CASE( cli_create_htlc_bsip64 )
{
graphene::wallet::brain_key_info bki = con.wallet_api_ptr->suggest_brain_key();
BOOST_CHECK(!bki.brain_priv_key.empty());
signed_transaction create_acct_tx = con.wallet_api_ptr->create_account_with_brain_key(bki.brain_priv_key, "alice",
"nathan", "nathan", true);
signed_transaction create_acct_tx = con.wallet_api_ptr->create_account_with_brain_key(bki.brain_priv_key,
"alice", "nathan", "nathan", true);
con.wallet_api_ptr->save_wallet_file(con.wallet_filename);
// attempt to give alice some bitshares
BOOST_TEST_MESSAGE("Transferring bitshares from Nathan to alice");
Expand All @@ -1673,8 +1675,8 @@ BOOST_AUTO_TEST_CASE( cli_create_htlc_bsip64 )
{
graphene::wallet::brain_key_info bki = con.wallet_api_ptr->suggest_brain_key();
BOOST_CHECK(!bki.brain_priv_key.empty());
signed_transaction create_acct_tx = con.wallet_api_ptr->create_account_with_brain_key(bki.brain_priv_key, "bob",
"nathan", "nathan", true);
signed_transaction create_acct_tx = con.wallet_api_ptr->create_account_with_brain_key(bki.brain_priv_key,
"bob", "nathan", "nathan", true);
// this should cause resync which will import the keys of alice and bob
generate_block(app1);
// attempt to give bob some bitshares
Expand Down Expand Up @@ -1710,7 +1712,8 @@ BOOST_AUTO_TEST_CASE( cli_create_htlc_bsip64 )
BOOST_CHECK(generate_block(app1, result_block));

// get the ID:
htlc_id_type htlc_id = result_block.transactions[result_block.transactions.size()-1].operation_results[0].get<object_id_type>();
htlc_id_type htlc_id = result_block.transactions[result_block.transactions.size()-1]
.operation_results[0].get<object_id_type>();
alice_htlc_id_as_string = (std::string)(object_id_type)htlc_id;
BOOST_TEST_MESSAGE("Alice shares the HTLC ID with Bob. The HTLC ID is: " + alice_htlc_id_as_string);
}
Expand All @@ -1722,7 +1725,8 @@ BOOST_AUTO_TEST_CASE( cli_create_htlc_bsip64 )

// Bob likes what he sees, so he creates an HTLC, using the info he retrieved from Alice's HTLC
con.wallet_api_ptr->htlc_create("bob", "alice",
"3", "BOBCOIN", "HASH160", hash_str, preimage_string.size(), fc::hours(12).to_seconds(), "Bob to Alice", true);
"3", "BOBCOIN", "HASH160", hash_str, preimage_string.size(), fc::hours(12).to_seconds(),
"Bob to Alice", true);

// normally, a wallet would watch block production, and find the transaction. Here, we can cheat:
std::string bob_htlc_id_as_string;
Expand All @@ -1732,7 +1736,8 @@ BOOST_AUTO_TEST_CASE( cli_create_htlc_bsip64 )
BOOST_CHECK(generate_block(app1, result_block));

// get the ID:
htlc_id_type htlc_id = result_block.transactions[result_block.transactions.size()-1].operation_results[0].get<object_id_type>();
htlc_id_type htlc_id = result_block.transactions[result_block.transactions.size()-1]
.operation_results[0].get<object_id_type>();
bob_htlc_id_as_string = (std::string)(object_id_type)htlc_id;
BOOST_TEST_MESSAGE("Bob shares the HTLC ID with Alice. The HTLC ID is: " + bob_htlc_id_as_string);
}
Expand All @@ -1750,7 +1755,20 @@ BOOST_AUTO_TEST_CASE( cli_create_htlc_bsip64 )
BOOST_CHECK(generate_block(app1));
}

// TODO: Bob can look at Alice's history to see her preimage
// Bob can look at Alice's history to see her preimage
{
BOOST_TEST_MESSAGE("Bob can look at the history of Alice to see the preimage");
std::vector<graphene::wallet::operation_detail> hist = con.wallet_api_ptr->get_account_history("alice", 1);
BOOST_CHECK( hist[0].description.find("with preimage \"4d792") != hist[0].description.npos);
}

// Bob can also look at his own history to see Alice's preimage
{
BOOST_TEST_MESSAGE("Bob can look at his own history to see the preimage");
std::vector<graphene::wallet::operation_detail> hist = con.wallet_api_ptr->get_account_history("bob", 1);
BOOST_CHECK( hist[0].description.find("with preimage \"4d792") != hist[0].description.npos);
}

// Bob can use the preimage to retrieve his BTS
{
BOOST_TEST_MESSAGE("Bob uses Alice's preimage to retrieve the BOBCOIN");
Expand Down
Loading