From dd47d03cf2b5b3dc59cd99aca247d1025679f83a Mon Sep 17 00:00:00 2001 From: jeffro256 Date: Sat, 3 Feb 2024 21:59:58 -0600 Subject: Enforce Tx unlock_time is Zero by Relay Rule [RELEASE] Related to https://github.com/monero-project/research-lab/issues/78 Added a relay rule that enforces the `unlock_time` field is equal to 0 for non-coinbase transactions. UIs changed: * Removed `locked_transfer` and `locked_sweep_all` commands from `monero-wallet-cli` APIs changed: * Removed `unlock_time` parameters from `wallet2` transfer methods * Wallet RPC transfer endpoints send error codes when requested unlock time is not 0 * Removed `unlock_time` parameters from `construct_tx*` cryptonote core functions @tobtoht: undo rebase changes tx.dsts -> tx_dsts --- src/wallet/api/wallet.cpp | 4 +- src/wallet/wallet2.cpp | 59 ++++++++++++++---------------- src/wallet/wallet2.h | 12 +++--- src/wallet/wallet_errors.h | 15 +++++--- src/wallet/wallet_rpc_server.cpp | 32 ++++++++++++++-- src/wallet/wallet_rpc_server_error_codes.h | 2 + 6 files changed, 75 insertions(+), 49 deletions(-) (limited to 'src/wallet') diff --git a/src/wallet/api/wallet.cpp b/src/wallet/api/wallet.cpp index 8d7364cba..fc4f89128 100644 --- a/src/wallet/api/wallet.cpp +++ b/src/wallet/api/wallet.cpp @@ -1588,11 +1588,11 @@ PendingTransaction *WalletImpl::createTransactionMultDest(const std::vectoradjust_mixin(mixin_count); if (amount) { - transaction->m_pending_tx = m_wallet->create_transactions_2(dsts, fake_outs_count, 0 /* unlock_time */, + transaction->m_pending_tx = m_wallet->create_transactions_2(dsts, fake_outs_count, adjusted_priority, extra, subaddr_account, subaddr_indices); } else { - transaction->m_pending_tx = m_wallet->create_transactions_all(0, info.address, info.is_subaddress, 1, fake_outs_count, 0 /* unlock_time */, + transaction->m_pending_tx = m_wallet->create_transactions_all(0, info.address, info.is_subaddress, 1, fake_outs_count, adjusted_priority, extra, subaddr_account, subaddr_indices); } diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index f34b10988..2321151f3 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -7333,14 +7333,15 @@ bool wallet2::sign_tx(unsigned_tx_set &exported_txs, std::vector additional_tx_keys; - bool r = cryptonote::construct_tx_and_get_tx_key(m_account.get_keys(), m_subaddresses, sd.sources, sd.splitted_dsts, sd.change_dts.addr, sd.extra, ptx.tx, sd.unlock_time, tx_key, additional_tx_keys, sd.use_rct, rct_config, sd.use_view_tags); - THROW_WALLET_EXCEPTION_IF(!r, error::tx_not_constructed, sd.sources, sd.splitted_dsts, sd.unlock_time, m_nettype); + bool r = cryptonote::construct_tx_and_get_tx_key(m_account.get_keys(), m_subaddresses, sd.sources, sd.splitted_dsts, sd.change_dts.addr, sd.extra, ptx.tx, tx_key, additional_tx_keys, sd.use_rct, rct_config, sd.use_view_tags); + THROW_WALLET_EXCEPTION_IF(!r, error::tx_not_constructed, sd.sources, sd.splitted_dsts, m_nettype); // we don't test tx size, because we don't know the current limit, due to not having a blockchain, // and it's a bit pointless to fail there anyway, since it'd be a (good) guess only. We sign anyway, // and if we really go over limit, the daemon will reject when it gets submitted. Chances are it's @@ -7870,7 +7871,6 @@ bool wallet2::sign_multisig_tx(multisig_tx_set &exported_txs, std::vector> template void wallet2::transfer_selected(const std::vector& dsts, const std::vector& selected_transfers, size_t fake_outputs_count, std::vector> &outs, std::unordered_set &valid_public_keys_cache, - uint64_t unlock_time, uint64_t fee, const std::vector& extra, T destination_split_strategy, const tx_dust_policy& dust_policy, cryptonote::transaction& tx, pending_tx &ptx, + uint64_t fee, const std::vector& extra, T destination_split_strategy, const tx_dust_policy& dust_policy, cryptonote::transaction& tx, pending_tx &ptx, bool use_view_tags) { using namespace cryptonote; @@ -9533,9 +9533,9 @@ void wallet2::transfer_selected(const std::vector additional_tx_keys; LOG_PRINT_L2("constructing tx"); - bool r = cryptonote::construct_tx_and_get_tx_key(m_account.get_keys(), m_subaddresses, sources, splitted_dsts, change_dts.addr, extra, tx, unlock_time, tx_key, additional_tx_keys, false, {}, use_view_tags); + bool r = cryptonote::construct_tx_and_get_tx_key(m_account.get_keys(), m_subaddresses, sources, splitted_dsts, change_dts.addr, extra, tx, tx_key, additional_tx_keys, false, {}, use_view_tags); LOG_PRINT_L2("constructed tx, r="< dsts, const std::vector& selected_transfers, size_t fake_outputs_count, std::vector> &outs, std::unordered_set &valid_public_keys_cache, - uint64_t unlock_time, uint64_t fee, const std::vector& extra, cryptonote::transaction& tx, pending_tx &ptx, const rct::RCTConfig &rct_config, bool use_view_tags) + uint64_t fee, const std::vector& extra, cryptonote::transaction& tx, pending_tx &ptx, const rct::RCTConfig &rct_config, bool use_view_tags) { using namespace cryptonote; // throw if attempting a transaction with no destinations @@ -9779,7 +9779,6 @@ void wallet2::transfer_selected_rct(std::vector wallet2::create_transactions_2(std::vector> &outs, */ valid_public_keys_cache, - unlock_time, /* CONST uint64_t unlock_time, */ tx.needed_fee, /* CONST uint64_t fee, */ extra, /* const std::vector& extra, */ test_tx, /* OUT cryptonote::transaction& tx, */ @@ -11213,7 +11211,6 @@ skip_tx: fake_outs_count, tx.outs, valid_public_keys_cache, - unlock_time, tx.needed_fee, extra, detail::digit_split_strategy, @@ -11327,7 +11324,7 @@ bool wallet2::sanity_check(const std::vector &ptx_vector, c return true; } -std::vector wallet2::create_transactions_all(uint64_t below, const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, const size_t fake_outs_count, const uint64_t unlock_time, uint32_t priority, const std::vector& extra, uint32_t subaddr_account, std::set subaddr_indices) +std::vector wallet2::create_transactions_all(uint64_t below, const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, const size_t fake_outs_count, uint32_t priority, const std::vector& extra, uint32_t subaddr_account, std::set subaddr_indices) { std::vector unused_transfers_indices; std::vector unused_dust_indices; @@ -11397,10 +11394,10 @@ std::vector wallet2::create_transactions_all(uint64_t below } } - return create_transactions_from(address, is_subaddress, outputs, unused_transfers_indices, unused_dust_indices, fake_outs_count, unlock_time, priority, extra); + return create_transactions_from(address, is_subaddress, outputs, unused_transfers_indices, unused_dust_indices, fake_outs_count, priority, extra); } -std::vector wallet2::create_transactions_single(const crypto::key_image &ki, const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, const size_t fake_outs_count, const uint64_t unlock_time, uint32_t priority, const std::vector& extra) +std::vector wallet2::create_transactions_single(const crypto::key_image &ki, const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, const size_t fake_outs_count, uint32_t priority, const std::vector& extra) { std::vector unused_transfers_indices; std::vector unused_dust_indices; @@ -11418,10 +11415,10 @@ std::vector wallet2::create_transactions_single(const crypt break; } } - return create_transactions_from(address, is_subaddress, outputs, unused_transfers_indices, unused_dust_indices, fake_outs_count, unlock_time, priority, extra); + return create_transactions_from(address, is_subaddress, outputs, unused_transfers_indices, unused_dust_indices, fake_outs_count, priority, extra); } -std::vector wallet2::create_transactions_from(const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, std::vector unused_transfers_indices, std::vector unused_dust_indices, const size_t fake_outs_count, const uint64_t unlock_time, uint32_t priority, const std::vector& extra) +std::vector wallet2::create_transactions_from(const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, std::vector unused_transfers_indices, std::vector unused_dust_indices, const size_t fake_outs_count, uint32_t priority, const std::vector& extra) { //ensure device is let in NONE mode in any case hw::device &hwdev = m_account.get_device(); @@ -11530,10 +11527,10 @@ std::vector wallet2::create_transactions_from(const crypton LOG_PRINT_L2("Trying to create a tx now, with " << tx.dsts.size() << " destinations and " << tx.selected_transfers.size() << " outputs"); if (use_rct) - transfer_selected_rct(tx.dsts, tx.selected_transfers, fake_outs_count, outs, valid_public_keys_cache, unlock_time, needed_fee, extra, + transfer_selected_rct(tx.dsts, tx.selected_transfers, fake_outs_count, outs, valid_public_keys_cache, needed_fee, extra, test_tx, test_ptx, rct_config, use_view_tags); else - transfer_selected(tx.dsts, tx.selected_transfers, fake_outs_count, outs, valid_public_keys_cache, unlock_time, needed_fee, extra, + transfer_selected(tx.dsts, tx.selected_transfers, fake_outs_count, outs, valid_public_keys_cache, needed_fee, extra, detail::digit_split_strategy, tx_dust_policy(::config::DEFAULT_DUST_THRESHOLD), test_tx, test_ptx, use_view_tags); auto txBlob = t_serializable_object_to_blob(test_ptx.tx); needed_fee = calculate_fee(use_per_byte_fee, test_ptx.tx, txBlob.size(), base_fee, fee_quantization_mask); @@ -11567,10 +11564,10 @@ std::vector wallet2::create_transactions_from(const crypton dt.amount = dt_amount + dt_residue; } if (use_rct) - transfer_selected_rct(tx.dsts, tx.selected_transfers, fake_outs_count, outs, valid_public_keys_cache, unlock_time, needed_fee, extra, + transfer_selected_rct(tx.dsts, tx.selected_transfers, fake_outs_count, outs, valid_public_keys_cache, needed_fee, extra, test_tx, test_ptx, rct_config, use_view_tags); else - transfer_selected(tx.dsts, tx.selected_transfers, fake_outs_count, outs, valid_public_keys_cache, unlock_time, needed_fee, extra, + transfer_selected(tx.dsts, tx.selected_transfers, fake_outs_count, outs, valid_public_keys_cache, needed_fee, extra, detail::digit_split_strategy, tx_dust_policy(::config::DEFAULT_DUST_THRESHOLD), test_tx, test_ptx, use_view_tags); txBlob = t_serializable_object_to_blob(test_ptx.tx); needed_fee = calculate_fee(use_per_byte_fee, test_ptx.tx, txBlob.size(), base_fee, fee_quantization_mask); @@ -11606,10 +11603,10 @@ std::vector wallet2::create_transactions_from(const crypton cryptonote::transaction test_tx; pending_tx test_ptx; if (use_rct) { - transfer_selected_rct(tx.dsts, tx.selected_transfers, fake_outs_count, tx.outs, valid_public_keys_cache, unlock_time, tx.needed_fee, extra, + transfer_selected_rct(tx.dsts, tx.selected_transfers, fake_outs_count, tx.outs, valid_public_keys_cache, tx.needed_fee, extra, test_tx, test_ptx, rct_config, use_view_tags); } else { - transfer_selected(tx.dsts, tx.selected_transfers, fake_outs_count, tx.outs, valid_public_keys_cache, unlock_time, tx.needed_fee, extra, + transfer_selected(tx.dsts, tx.selected_transfers, fake_outs_count, tx.outs, valid_public_keys_cache, tx.needed_fee, extra, detail::digit_split_strategy, tx_dust_policy(::config::DEFAULT_DUST_THRESHOLD), test_tx, test_ptx, use_view_tags); } auto txBlob = t_serializable_object_to_blob(test_ptx.tx); @@ -11921,7 +11918,7 @@ std::vector wallet2::create_unmixable_sweep_transactions() unmixable_transfer_outputs.push_back(n); } - return create_transactions_from(m_account_public_address, false, 1, unmixable_transfer_outputs, unmixable_dust_outputs, 0 /*fake_outs_count */, 0 /* unlock_time */, 1 /*priority */, std::vector()); + return create_transactions_from(m_account_public_address, false, 1, unmixable_transfer_outputs, unmixable_dust_outputs, 0 /*fake_outs_count */, 1 /*priority */, std::vector()); } //---------------------------------------------------------------------------------------------------- void wallet2::discard_unmixable_outputs() diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index 3144a8fd3..c38d77675 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -1088,10 +1088,10 @@ private: template void transfer_selected(const std::vector& dsts, const std::vector& selected_transfers, size_t fake_outputs_count, std::vector> &outs, std::unordered_set &valid_public_keys_cache, - uint64_t unlock_time, uint64_t fee, const std::vector& extra, T destination_split_strategy, const tx_dust_policy& dust_policy, cryptonote::transaction& tx, pending_tx &ptx, const bool use_view_tags); + uint64_t fee, const std::vector& extra, T destination_split_strategy, const tx_dust_policy& dust_policy, cryptonote::transaction& tx, pending_tx &ptx, const bool use_view_tags); void transfer_selected_rct(std::vector dsts, const std::vector& selected_transfers, size_t fake_outputs_count, std::vector> &outs, std::unordered_set &valid_public_keys_cache, - uint64_t unlock_time, uint64_t fee, const std::vector& extra, cryptonote::transaction& tx, pending_tx &ptx, const rct::RCTConfig &rct_config, const bool use_view_tags); + uint64_t fee, const std::vector& extra, cryptonote::transaction& tx, pending_tx &ptx, const rct::RCTConfig &rct_config, const bool use_view_tags); void commit_tx(pending_tx& ptx_vector); void commit_tx(std::vector& ptx_vector); @@ -1113,10 +1113,10 @@ private: bool parse_unsigned_tx_from_str(const std::string &unsigned_tx_st, unsigned_tx_set &exported_txs) const; bool load_tx(const std::string &signed_filename, std::vector &ptx, std::function accept_func = NULL); bool parse_tx_from_str(const std::string &signed_tx_st, std::vector &ptx, std::function accept_func); - std::vector create_transactions_2(std::vector dsts, const size_t fake_outs_count, const uint64_t unlock_time, uint32_t priority, const std::vector& extra, uint32_t subaddr_account, std::set subaddr_indices, const unique_index_container& subtract_fee_from_outputs = {}); // pass subaddr_indices by value on purpose - std::vector create_transactions_all(uint64_t below, const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, const size_t fake_outs_count, const uint64_t unlock_time, uint32_t priority, const std::vector& extra, uint32_t subaddr_account, std::set subaddr_indices); - std::vector create_transactions_single(const crypto::key_image &ki, const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, const size_t fake_outs_count, const uint64_t unlock_time, uint32_t priority, const std::vector& extra); - std::vector create_transactions_from(const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, std::vector unused_transfers_indices, std::vector unused_dust_indices, const size_t fake_outs_count, const uint64_t unlock_time, uint32_t priority, const std::vector& extra); + std::vector create_transactions_2(std::vector dsts, const size_t fake_outs_count, uint32_t priority, const std::vector& extra, uint32_t subaddr_account, std::set subaddr_indices, const unique_index_container& subtract_fee_from_outputs = {}); // pass subaddr_indices by value on purpose + std::vector create_transactions_all(uint64_t below, const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, const size_t fake_outs_count, uint32_t priority, const std::vector& extra, uint32_t subaddr_account, std::set subaddr_indices); + std::vector create_transactions_single(const crypto::key_image &ki, const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, const size_t fake_outs_count, uint32_t priority, const std::vector& extra); + std::vector create_transactions_from(const cryptonote::account_public_address &address, bool is_subaddress, const size_t outputs, std::vector unused_transfers_indices, std::vector unused_dust_indices, const size_t fake_outs_count, uint32_t priority, const std::vector& extra); bool sanity_check(const std::vector &ptx_vector, const std::vector& dsts, const unique_index_container& subtract_fee_from_outputs = {}) const; void cold_tx_aux_import(const std::vector& ptx, const std::vector& tx_device_aux); void cold_sign_tx(const std::vector& ptx_vector, signed_tx_set &exported_txs, std::vector &dsts_info, std::vector & tx_device_aux); diff --git a/src/wallet/wallet_errors.h b/src/wallet/wallet_errors.h index 1f7e1c75d..c077313d4 100644 --- a/src/wallet/wallet_errors.h +++ b/src/wallet/wallet_errors.h @@ -86,6 +86,7 @@ namespace tools // zero_amount // zero_destination // subtract_fee_from_bad_index + // nonzero_unlock_time // wallet_rpc_error * // daemon_busy // no_connection_to_daemon @@ -592,20 +593,17 @@ namespace tools std::string && loc , sources_t const & sources , destinations_t const & destinations - , uint64_t unlock_time , cryptonote::network_type nettype ) : transfer_error(std::move(loc), "transaction was not constructed") , m_sources(sources) , m_destinations(destinations) - , m_unlock_time(unlock_time) , m_nettype(nettype) { } const sources_t& sources() const { return m_sources; } const destinations_t& destinations() const { return m_destinations; } - uint64_t unlock_time() const { return m_unlock_time; } std::string to_string() const { @@ -637,15 +635,12 @@ namespace tools cryptonote::print_money(dst.amount); } - ss << "\nunlock_time: " << m_unlock_time; - return ss.str(); } private: sources_t m_sources; destinations_t m_destinations; - uint64_t m_unlock_time; cryptonote::network_type m_nettype; }; //---------------------------------------------------------------------------------------------------- @@ -789,6 +784,14 @@ namespace tools } }; //---------------------------------------------------------------------------------------------------- + struct nonzero_unlock_time : public transfer_error + { + explicit nonzero_unlock_time(std::string&& loc) + : transfer_error(std::move(loc), "transaction cannot have non-zero unlock time") + { + } + }; + //---------------------------------------------------------------------------------------------------- struct wallet_rpc_error : public wallet_logic_error { const std::string& request() const { return m_request; } diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index 4e42d64cd..58c9c5af6 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -1080,6 +1080,12 @@ namespace tools er.message = "Command unavailable in restricted mode."; return false; } + else if (req.unlock_time) + { + er.code = WALLET_RPC_ERROR_CODE_NONZERO_UNLOCK_TIME; + er.message = "Transaction cannot have non-zero unlock time"; + return false; + } CHECK_MULTISIG_ENABLED(); @@ -1093,7 +1099,7 @@ namespace tools { uint64_t mixin = m_wallet->adjust_mixin(req.ring_size ? req.ring_size - 1 : 0); uint32_t priority = m_wallet->adjust_priority(req.priority); - std::vector ptx_vector = m_wallet->create_transactions_2(dsts, mixin, req.unlock_time, priority, extra, req.account_index, req.subaddr_indices, req.subtract_fee_from_outputs); + std::vector ptx_vector = m_wallet->create_transactions_2(dsts, mixin, priority, extra, req.account_index, req.subaddr_indices, req.subtract_fee_from_outputs); if (ptx_vector.empty()) { @@ -1134,6 +1140,12 @@ namespace tools er.message = "Command unavailable in restricted mode."; return false; } + else if (req.unlock_time) + { + er.code = WALLET_RPC_ERROR_CODE_NONZERO_UNLOCK_TIME; + er.message = "Transaction cannot have non-zero unlock time"; + return false; + } CHECK_MULTISIG_ENABLED(); @@ -1148,7 +1160,7 @@ namespace tools uint64_t mixin = m_wallet->adjust_mixin(req.ring_size ? req.ring_size - 1 : 0); uint32_t priority = m_wallet->adjust_priority(req.priority); LOG_PRINT_L2("on_transfer_split calling create_transactions_2"); - std::vector ptx_vector = m_wallet->create_transactions_2(dsts, mixin, req.unlock_time, priority, extra, req.account_index, req.subaddr_indices); + std::vector ptx_vector = m_wallet->create_transactions_2(dsts, mixin, priority, extra, req.account_index, req.subaddr_indices); LOG_PRINT_L2("on_transfer_split called create_transactions_2"); if (ptx_vector.empty()) @@ -1570,6 +1582,12 @@ namespace tools er.message = "Command unavailable in restricted mode."; return false; } + else if (req.unlock_time) + { + er.code = WALLET_RPC_ERROR_CODE_NONZERO_UNLOCK_TIME; + er.message = "Transaction cannot have non-zero unlock time"; + return false; + } CHECK_MULTISIG_ENABLED(); @@ -1605,7 +1623,7 @@ namespace tools { uint64_t mixin = m_wallet->adjust_mixin(req.ring_size ? req.ring_size - 1 : 0); uint32_t priority = m_wallet->adjust_priority(req.priority); - std::vector ptx_vector = m_wallet->create_transactions_all(req.below_amount, dsts[0].addr, dsts[0].is_subaddress, req.outputs, mixin, req.unlock_time, priority, extra, req.account_index, subaddr_indices); + std::vector ptx_vector = m_wallet->create_transactions_all(req.below_amount, dsts[0].addr, dsts[0].is_subaddress, req.outputs, mixin, priority, extra, req.account_index, subaddr_indices); return fill_response(ptx_vector, req.get_tx_keys, res.tx_key_list, res.amount_list, res.amounts_by_dest_list, res.fee_list, res.weight_list, res.multisig_txset, res.unsigned_txset, req.do_not_relay, res.tx_hash_list, req.get_tx_hex, res.tx_blob_list, req.get_tx_metadata, res.tx_metadata_list, res.spent_key_images_list, er); @@ -1630,6 +1648,12 @@ namespace tools er.message = "Command unavailable in restricted mode."; return false; } + else if (req.unlock_time) + { + er.code = WALLET_RPC_ERROR_CODE_NONZERO_UNLOCK_TIME; + er.message = "Transaction cannot have non-zero unlock time"; + return false; + } if (req.outputs < 1) { @@ -1662,7 +1686,7 @@ namespace tools { uint64_t mixin = m_wallet->adjust_mixin(req.ring_size ? req.ring_size - 1 : 0); uint32_t priority = m_wallet->adjust_priority(req.priority); - std::vector ptx_vector = m_wallet->create_transactions_single(ki, dsts[0].addr, dsts[0].is_subaddress, req.outputs, mixin, req.unlock_time, priority, extra); + std::vector ptx_vector = m_wallet->create_transactions_single(ki, dsts[0].addr, dsts[0].is_subaddress, req.outputs, mixin, priority, extra); if (ptx_vector.empty()) { diff --git a/src/wallet/wallet_rpc_server_error_codes.h b/src/wallet/wallet_rpc_server_error_codes.h index 734229380..541d29f86 100644 --- a/src/wallet/wallet_rpc_server_error_codes.h +++ b/src/wallet/wallet_rpc_server_error_codes.h @@ -79,3 +79,5 @@ #define WALLET_RPC_ERROR_CODE_ZERO_AMOUNT -46 #define WALLET_RPC_ERROR_CODE_INVALID_SIGNATURE_TYPE -47 #define WALLET_RPC_ERROR_CODE_DISABLED -48 +#define WALLET_RPC_ERROR_CODE_PROXY_ALREADY_DEFINED -49 +#define WALLET_RPC_ERROR_CODE_NONZERO_UNLOCK_TIME -50 -- cgit v1.2.3