Convey tx verification failure reasons to the RPC client

This allows appropriate action to be taken, like displaying
the reason to the user.

Do just that in simplewallet, which should help a lot in
determining why users fail to send.

Also make it so a tx which is accepted but not relayed is
seen as a success rather than a failure.
This commit is contained in:
moneromooo-monero 2016-03-27 12:35:36 +01:00
parent 1559c71ef2
commit 24b3e9007a
No known key found for this signature in database
GPG key ID: 686F07454D6CEFC3
10 changed files with 98 additions and 23 deletions

View file

@ -1991,7 +1991,7 @@ bool Blockchain::get_tx_outputs_gindexs(const crypto::hash& tx_id, std::vector<u
// This function overloads its sister function with // This function overloads its sister function with
// an extra value (hash of highest block that holds an output used as input) // an extra value (hash of highest block that holds an output used as input)
// as a return-by-reference. // as a return-by-reference.
bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t& max_used_block_height, crypto::hash& max_used_block_id, bool kept_by_block) bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t& max_used_block_height, crypto::hash& max_used_block_id, tx_verification_context &tvc, bool kept_by_block)
{ {
LOG_PRINT_L3("Blockchain::" << __func__); LOG_PRINT_L3("Blockchain::" << __func__);
CRITICAL_REGION_LOCAL(m_blockchain_lock); CRITICAL_REGION_LOCAL(m_blockchain_lock);
@ -2013,7 +2013,7 @@ bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t& max_used_block
#endif #endif
TIME_MEASURE_START(a); TIME_MEASURE_START(a);
bool res = check_tx_inputs(tx, &max_used_block_height); bool res = check_tx_inputs(tx, tvc, &max_used_block_height);
TIME_MEASURE_FINISH(a); TIME_MEASURE_FINISH(a);
crypto::hash tx_prefix_hash = get_transaction_prefix_hash(tx); crypto::hash tx_prefix_hash = get_transaction_prefix_hash(tx);
if(m_show_time_stats) if(m_show_time_stats)
@ -2032,7 +2032,7 @@ bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t& max_used_block
return true; return true;
} }
//------------------------------------------------------------------ //------------------------------------------------------------------
bool Blockchain::check_tx_outputs(const transaction& tx) bool Blockchain::check_tx_outputs(const transaction& tx, tx_verification_context &tvc)
{ {
LOG_PRINT_L3("Blockchain::" << __func__); LOG_PRINT_L3("Blockchain::" << __func__);
CRITICAL_REGION_LOCAL(m_blockchain_lock); CRITICAL_REGION_LOCAL(m_blockchain_lock);
@ -2041,6 +2041,7 @@ bool Blockchain::check_tx_outputs(const transaction& tx)
if (m_hardfork->get_current_version() >= 2) { if (m_hardfork->get_current_version() >= 2) {
for (auto &o: tx.vout) { for (auto &o: tx.vout) {
if (!is_valid_decomposed_amount(o.amount)) { if (!is_valid_decomposed_amount(o.amount)) {
tvc.m_invalid_output = true;
return false; return false;
} }
} }
@ -2066,7 +2067,7 @@ bool Blockchain::have_tx_keyimges_as_spent(const transaction &tx) const
// check_tx_input() rather than here, and use this function simply // check_tx_input() rather than here, and use this function simply
// to iterate the inputs as necessary (splitting the task // to iterate the inputs as necessary (splitting the task
// using threads, etc.) // using threads, etc.)
bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t* pmax_used_block_height) bool Blockchain::check_tx_inputs(const transaction& tx, tx_verification_context &tvc, uint64_t* pmax_used_block_height)
{ {
LOG_PRINT_L3("Blockchain::" << __func__); LOG_PRINT_L3("Blockchain::" << __func__);
size_t sig_index = 0; size_t sig_index = 0;
@ -2113,11 +2114,13 @@ bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t* pmax_used_bloc
if (n_unmixable == 0) if (n_unmixable == 0)
{ {
LOG_PRINT_L1("Tx " << get_transaction_hash(tx) << " has too low mixin (" << mixin << "), and no unmixable inputs"); LOG_PRINT_L1("Tx " << get_transaction_hash(tx) << " has too low mixin (" << mixin << "), and no unmixable inputs");
tvc.m_low_mixin = true;
return false; return false;
} }
if (n_mixable > 1) if (n_mixable > 1)
{ {
LOG_PRINT_L1("Tx " << get_transaction_hash(tx) << " has too low mixin (" << mixin << "), and more than one mixable input with unmixable inputs"); LOG_PRINT_L1("Tx " << get_transaction_hash(tx) << " has too low mixin (" << mixin << "), and more than one mixable input with unmixable inputs");
tvc.m_low_mixin = true;
return false; return false;
} }
} }
@ -2176,6 +2179,7 @@ bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t* pmax_used_bloc
if(have_tx_keyimg_as_spent(in_to_key.k_image)) if(have_tx_keyimg_as_spent(in_to_key.k_image))
{ {
LOG_PRINT_L1("Key image already spent in blockchain: " << epee::string_tools::pod_to_hex(in_to_key.k_image)); LOG_PRINT_L1("Key image already spent in blockchain: " << epee::string_tools::pod_to_hex(in_to_key.k_image));
tvc.m_double_spend = true;
return false; return false;
} }
@ -2667,7 +2671,8 @@ leave:
#endif #endif
{ {
// validate that transaction inputs and the keys spending them are correct. // validate that transaction inputs and the keys spending them are correct.
if(!check_tx_inputs(tx)) tx_verification_context tvc;
if(!check_tx_inputs(tx, tvc))
{ {
LOG_PRINT_L1("Block with id: " << id << " has at least one transaction (id: " << tx_id << ") with wrong inputs."); LOG_PRINT_L1("Block with id: " << id << " has at least one transaction (id: " << tx_id << ") with wrong inputs.");

View file

@ -472,11 +472,12 @@ namespace cryptonote
* @param tx the transaction to validate * @param tx the transaction to validate
* @param pmax_used_block_height return-by-reference block height of most recent input * @param pmax_used_block_height return-by-reference block height of most recent input
* @param max_used_block_id return-by-reference block hash of most recent input * @param max_used_block_id return-by-reference block hash of most recent input
* @param tvc returned information about tx verification
* @param kept_by_block whether or not the transaction is from a previously-verified block * @param kept_by_block whether or not the transaction is from a previously-verified block
* *
* @return false if any input is invalid, otherwise true * @return false if any input is invalid, otherwise true
*/ */
bool check_tx_inputs(const transaction& tx, uint64_t& pmax_used_block_height, crypto::hash& max_used_block_id, bool kept_by_block = false); bool check_tx_inputs(const transaction& tx, uint64_t& pmax_used_block_height, crypto::hash& max_used_block_id, tx_verification_context &tvc, bool kept_by_block = false);
/** /**
* @brief check that a transaction's outputs conform to current standards * @brief check that a transaction's outputs conform to current standards
@ -486,10 +487,11 @@ namespace cryptonote
* written out would have only one non-zero digit in base 10). * written out would have only one non-zero digit in base 10).
* *
* @param tx the transaction to check the outputs of * @param tx the transaction to check the outputs of
* @param tvc returned info about tx verification
* *
* @return false if any outputs do not conform, otherwise true * @return false if any outputs do not conform, otherwise true
*/ */
bool check_tx_outputs(const transaction& tx); bool check_tx_outputs(const transaction& tx, tx_verification_context &tvc);
/** /**
* @brief gets the blocksize limit based on recent blocks * @brief gets the blocksize limit based on recent blocks
@ -874,11 +876,12 @@ namespace cryptonote
* transaction. * transaction.
* *
* @param tx the transaction to validate * @param tx the transaction to validate
* @param tvc returned information about tx verification
* @param pmax_related_block_height return-by-pointer the height of the most recent block in the input set * @param pmax_related_block_height return-by-pointer the height of the most recent block in the input set
* *
* @return false if any validation step fails, otherwise true * @return false if any validation step fails, otherwise true
*/ */
bool check_tx_inputs(const transaction& tx, uint64_t* pmax_used_block_height = NULL); bool check_tx_inputs(const transaction& tx, tx_verification_context &tvc, uint64_t* pmax_used_block_height = NULL);
/** /**
* @brief performs a blockchain reorganization according to the longest chain rule * @brief performs a blockchain reorganization according to the longest chain rule

View file

@ -489,6 +489,7 @@ namespace cryptonote
{ {
LOG_PRINT_L1("WRONG TRANSACTION BLOB, too big size " << tx_blob.size() << ", rejected"); LOG_PRINT_L1("WRONG TRANSACTION BLOB, too big size " << tx_blob.size() << ", rejected");
tvc.m_verifivation_failed = true; tvc.m_verifivation_failed = true;
tvc.m_too_big = true;
return false; return false;
} }

View file

@ -97,6 +97,7 @@ namespace cryptonote
if(!check_inputs_types_supported(tx)) if(!check_inputs_types_supported(tx))
{ {
tvc.m_verifivation_failed = true; tvc.m_verifivation_failed = true;
tvc.m_invalid_input = true;
return false; return false;
} }
@ -113,6 +114,7 @@ namespace cryptonote
{ {
LOG_PRINT_L1("transaction use more money then it has: use " << print_money(outputs_amount) << ", have " << print_money(inputs_amount)); LOG_PRINT_L1("transaction use more money then it has: use " << print_money(outputs_amount) << ", have " << print_money(inputs_amount));
tvc.m_verifivation_failed = true; tvc.m_verifivation_failed = true;
tvc.m_overspend = true;
return false; return false;
} }
@ -124,6 +126,7 @@ namespace cryptonote
{ {
LOG_PRINT_L1("transaction fee is not enough: " << print_money(fee) << ", minimum fee: " << print_money(needed_fee)); LOG_PRINT_L1("transaction fee is not enough: " << print_money(fee) << ", minimum fee: " << print_money(needed_fee));
tvc.m_verifivation_failed = true; tvc.m_verifivation_failed = true;
tvc.m_fee_too_low = true;
return false; return false;
} }
@ -132,6 +135,7 @@ namespace cryptonote
{ {
LOG_PRINT_L1("transaction is too big: " << blob_size << " bytes, maximum size: " << tx_size_limit); LOG_PRINT_L1("transaction is too big: " << blob_size << " bytes, maximum size: " << tx_size_limit);
tvc.m_verifivation_failed = true; tvc.m_verifivation_failed = true;
tvc.m_too_big = true;
return false; return false;
} }
@ -142,21 +146,23 @@ namespace cryptonote
{ {
LOG_PRINT_L1("Transaction with id= "<< id << " used already spent key images"); LOG_PRINT_L1("Transaction with id= "<< id << " used already spent key images");
tvc.m_verifivation_failed = true; tvc.m_verifivation_failed = true;
tvc.m_double_spend = true;
return false; return false;
} }
} }
if (!m_blockchain.check_tx_outputs(tx)) if (!m_blockchain.check_tx_outputs(tx, tvc))
{ {
LOG_PRINT_L1("Transaction with id= "<< id << " has at least one invalid outout"); LOG_PRINT_L1("Transaction with id= "<< id << " has at least one invalid outout");
tvc.m_verifivation_failed = true; tvc.m_verifivation_failed = true;
tvc.m_invalid_output = true;
return false; return false;
} }
crypto::hash max_used_block_id = null_hash; crypto::hash max_used_block_id = null_hash;
uint64_t max_used_block_height = 0; uint64_t max_used_block_height = 0;
#if BLOCKCHAIN_DB == DB_LMDB #if BLOCKCHAIN_DB == DB_LMDB
bool ch_inp_res = m_blockchain.check_tx_inputs(tx, max_used_block_height, max_used_block_id, kept_by_block); bool ch_inp_res = m_blockchain.check_tx_inputs(tx, max_used_block_height, max_used_block_id, tvc, kept_by_block);
#else #else
bool ch_inp_res = m_blockchain.check_tx_inputs(tx, max_used_block_height, max_used_block_id); bool ch_inp_res = m_blockchain.check_tx_inputs(tx, max_used_block_height, max_used_block_id);
#endif #endif
@ -480,7 +486,8 @@ namespace cryptonote
if(txd.last_failed_id != null_hash && m_blockchain.get_current_blockchain_height() > txd.last_failed_height && txd.last_failed_id == m_blockchain.get_block_id_by_height(txd.last_failed_height)) if(txd.last_failed_id != null_hash && m_blockchain.get_current_blockchain_height() > txd.last_failed_height && txd.last_failed_id == m_blockchain.get_block_id_by_height(txd.last_failed_height))
return false;//we already sure that this tx is broken for this height return false;//we already sure that this tx is broken for this height
if(!m_blockchain.check_tx_inputs(txd.tx, txd.max_used_block_height, txd.max_used_block_id)) tx_verification_context tvc;
if(!m_blockchain.check_tx_inputs(txd.tx, txd.max_used_block_height, txd.max_used_block_id, tvc))
{ {
txd.last_failed_height = m_blockchain.get_current_blockchain_height()-1; txd.last_failed_height = m_blockchain.get_current_blockchain_height()-1;
txd.last_failed_id = m_blockchain.get_block_id_by_height(txd.last_failed_height); txd.last_failed_id = m_blockchain.get_block_id_by_height(txd.last_failed_height);
@ -496,7 +503,12 @@ namespace cryptonote
if(txd.last_failed_id == m_blockchain.get_block_id_by_height(txd.last_failed_height)) if(txd.last_failed_id == m_blockchain.get_block_id_by_height(txd.last_failed_height))
return false; return false;
//check ring signature again, it is possible (with very small chance) that this transaction become again valid //check ring signature again, it is possible (with very small chance) that this transaction become again valid
#if BLOCKCHAIN_DB == DB_LMDB
tx_verification_context tvc;
if(!m_blockchain.check_tx_inputs(txd.tx, txd.max_used_block_height, txd.max_used_block_id, tvc))
#else
if(!m_blockchain.check_tx_inputs(txd.tx, txd.max_used_block_height, txd.max_used_block_id)) if(!m_blockchain.check_tx_inputs(txd.tx, txd.max_used_block_height, txd.max_used_block_id))
#endif
{ {
txd.last_failed_height = m_blockchain.get_current_blockchain_height()-1; txd.last_failed_height = m_blockchain.get_current_blockchain_height()-1;
txd.last_failed_id = m_blockchain.get_block_id_by_height(txd.last_failed_height); txd.last_failed_id = m_blockchain.get_block_id_by_height(txd.last_failed_height);

View file

@ -40,6 +40,13 @@ namespace cryptonote
bool m_verifivation_failed; //bad tx, should drop connection bool m_verifivation_failed; //bad tx, should drop connection
bool m_verifivation_impossible; //the transaction is related with an alternative blockchain bool m_verifivation_impossible; //the transaction is related with an alternative blockchain
bool m_added_to_pool; bool m_added_to_pool;
bool m_low_mixin;
bool m_double_spend;
bool m_invalid_input;
bool m_invalid_output;
bool m_too_big;
bool m_overspend;
bool m_fee_too_low;
}; };
struct block_verification_context struct block_verification_context

View file

@ -355,24 +355,40 @@ namespace cryptonote
cryptonote_connection_context fake_context = AUTO_VAL_INIT(fake_context); cryptonote_connection_context fake_context = AUTO_VAL_INIT(fake_context);
tx_verification_context tvc = AUTO_VAL_INIT(tvc); tx_verification_context tvc = AUTO_VAL_INIT(tvc);
if(!m_core.handle_incoming_tx(tx_blob, tvc, false, false)) if(!m_core.handle_incoming_tx(tx_blob, tvc, false, false) || tvc.m_verifivation_failed)
{ {
LOG_PRINT_L0("[on_send_raw_tx]: Failed to process tx");
res.status = "Failed";
return true;
}
if (tvc.m_verifivation_failed) if (tvc.m_verifivation_failed)
{ {
LOG_PRINT_L0("[on_send_raw_tx]: tx verification failed"); LOG_PRINT_L0("[on_send_raw_tx]: tx verification failed");
}
else
{
LOG_PRINT_L0("[on_send_raw_tx]: Failed to process tx");
}
res.status = "Failed"; res.status = "Failed";
if ((res.low_mixin = tvc.m_low_mixin))
res.reason = "mixin too low";
if ((res.double_spend = tvc.m_double_spend))
res.reason = "double spend";
if ((res.invalid_input = tvc.m_invalid_input))
res.reason = "invalid input";
if ((res.invalid_output = tvc.m_invalid_output))
res.reason = "invalid output";
if ((res.too_big = tvc.m_too_big))
res.reason = "too big";
if ((res.overspend = tvc.m_overspend))
res.reason = "overspend";
if ((res.fee_too_low = tvc.m_fee_too_low))
res.reason = "fee too low";
return true; return true;
} }
if(!tvc.m_should_be_relayed) if(!tvc.m_should_be_relayed)
{ {
LOG_PRINT_L0("[on_send_raw_tx]: tx accepted, but not relayed"); LOG_PRINT_L0("[on_send_raw_tx]: tx accepted, but not relayed");
res.status = "Not relayed"; res.reason = "Not relayed";
res.not_relayed = true;
res.status = CORE_RPC_STATUS_OK;
return true; return true;
} }

View file

@ -234,9 +234,27 @@ namespace cryptonote
struct response struct response
{ {
std::string status; std::string status;
std::string reason;
bool not_relayed;
bool low_mixin;
bool double_spend;
bool invalid_input;
bool invalid_output;
bool too_big;
bool overspend;
bool fee_too_low;
BEGIN_KV_SERIALIZE_MAP() BEGIN_KV_SERIALIZE_MAP()
KV_SERIALIZE(status) KV_SERIALIZE(status)
KV_SERIALIZE(reason)
KV_SERIALIZE(not_relayed)
KV_SERIALIZE(low_mixin)
KV_SERIALIZE(double_spend)
KV_SERIALIZE(invalid_input)
KV_SERIALIZE(invalid_output)
KV_SERIALIZE(too_big)
KV_SERIALIZE(overspend)
KV_SERIALIZE(fee_too_low)
END_KV_SERIALIZE_MAP() END_KV_SERIALIZE_MAP()
}; };
}; };

View file

@ -2160,6 +2160,9 @@ bool simple_wallet::transfer_main(bool new_algorithm, const std::vector<std::str
catch (const tools::error::tx_rejected& e) catch (const tools::error::tx_rejected& e)
{ {
fail_msg_writer() << (boost::format(tr("transaction %s was rejected by daemon with status: ")) % get_transaction_hash(e.tx())) << e.status(); fail_msg_writer() << (boost::format(tr("transaction %s was rejected by daemon with status: ")) % get_transaction_hash(e.tx())) << e.status();
std::string reason = e.reason();
if (!reason.empty())
fail_msg_writer() << tr("Reason: ") << reason;
} }
catch (const tools::error::tx_sum_overflow& e) catch (const tools::error::tx_sum_overflow& e)
{ {
@ -2307,6 +2310,9 @@ bool simple_wallet::sweep_dust(const std::vector<std::string> &args_)
catch (const tools::error::tx_rejected& e) catch (const tools::error::tx_rejected& e)
{ {
fail_msg_writer() << (boost::format(tr("transaction %s was rejected by daemon with status: ")) % get_transaction_hash(e.tx())) << e.status(); fail_msg_writer() << (boost::format(tr("transaction %s was rejected by daemon with status: ")) % get_transaction_hash(e.tx())) << e.status();
std::string reason = e.reason();
if (!reason.empty())
fail_msg_writer() << tr("Reason: ") << reason;
} }
catch (const tools::error::tx_sum_overflow& e) catch (const tools::error::tx_sum_overflow& e)
{ {

View file

@ -1958,7 +1958,7 @@ void wallet2::commit_tx(pending_tx& ptx)
m_daemon_rpc_mutex.unlock(); m_daemon_rpc_mutex.unlock();
THROW_WALLET_EXCEPTION_IF(!r, error::no_connection_to_daemon, "sendrawtransaction"); THROW_WALLET_EXCEPTION_IF(!r, error::no_connection_to_daemon, "sendrawtransaction");
THROW_WALLET_EXCEPTION_IF(daemon_send_resp.status == CORE_RPC_STATUS_BUSY, error::daemon_busy, "sendrawtransaction"); THROW_WALLET_EXCEPTION_IF(daemon_send_resp.status == CORE_RPC_STATUS_BUSY, error::daemon_busy, "sendrawtransaction");
THROW_WALLET_EXCEPTION_IF(daemon_send_resp.status != CORE_RPC_STATUS_OK, error::tx_rejected, ptx.tx, daemon_send_resp.status); THROW_WALLET_EXCEPTION_IF(daemon_send_resp.status != CORE_RPC_STATUS_OK, error::tx_rejected, ptx.tx, daemon_send_resp.status, daemon_send_resp.reason);
txid = get_transaction_hash(ptx.tx); txid = get_transaction_hash(ptx.tx);
crypto::hash payment_id = cryptonote::null_hash; crypto::hash payment_id = cryptonote::null_hash;

View file

@ -457,15 +457,17 @@ namespace tools
//---------------------------------------------------------------------------------------------------- //----------------------------------------------------------------------------------------------------
struct tx_rejected : public transfer_error struct tx_rejected : public transfer_error
{ {
explicit tx_rejected(std::string&& loc, const cryptonote::transaction& tx, const std::string& status) explicit tx_rejected(std::string&& loc, const cryptonote::transaction& tx, const std::string& status, const std::string& reason)
: transfer_error(std::move(loc), "transaction was rejected by daemon") : transfer_error(std::move(loc), "transaction was rejected by daemon")
, m_tx(tx) , m_tx(tx)
, m_status(status) , m_status(status)
, m_reason(reason)
{ {
} }
const cryptonote::transaction& tx() const { return m_tx; } const cryptonote::transaction& tx() const { return m_tx; }
const std::string& status() const { return m_status; } const std::string& status() const { return m_status; }
const std::string& reason() const { return m_reason; }
std::string to_string() const std::string to_string() const
{ {
@ -473,12 +475,17 @@ namespace tools
ss << transfer_error::to_string() << ", status = " << m_status << ", tx:\n"; ss << transfer_error::to_string() << ", status = " << m_status << ", tx:\n";
cryptonote::transaction tx = m_tx; cryptonote::transaction tx = m_tx;
ss << cryptonote::obj_to_json_str(tx); ss << cryptonote::obj_to_json_str(tx);
if (!m_reason.empty())
{
ss << " (" << m_reason << ")";
}
return ss.str(); return ss.str();
} }
private: private:
cryptonote::transaction m_tx; cryptonote::transaction m_tx;
std::string m_status; std::string m_status;
std::string m_reason;
}; };
//---------------------------------------------------------------------------------------------------- //----------------------------------------------------------------------------------------------------
struct tx_sum_overflow : public transfer_error struct tx_sum_overflow : public transfer_error