oxen-io
/
oxen-core
mirror of https://github.com/oxen-io/oxen-core.git
2
1
Fork 0
Code Issues Projects Releases Wiki Activity

Add locks around the refresh thread 

Wallet API is really messy with threads -- the refresh thread can run at
any time and change internal wallet state which breaks just about
everything.

This puts all non-trivial wallet access from wallet_api behind a lock of
the refresh thread to lock it out from refresh while other operations
are underway.
This commit is contained in:
Jason Rhinelander 2021-06-22 22:08:12 -03:00
parent 2e7e47a005
commit 81eddcd26a
8 changed files with 355 additions and 311 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
src/wallet/api/address_book.cpp
View File

@ -51,14 +51,15 @@ bool AddressBookImpl::addRow(const std::string &dst_addr, const std::string &des
{
clearStatus();
auto w = m_wallet->wallet();
cryptonote::address_parse_info info;
if(!cryptonote::get_account_address_from_str(info, m_wallet->m_wallet->nettype(), dst_addr)) {
if(!cryptonote::get_account_address_from_str(info, w->nettype(), dst_addr)) {
m_errorString = tr("Invalid destination address");
m_errorCode = Invalid_Address;
return false;
}
bool r = m_wallet->m_wallet->add_address_book_row(info.address, info.has_payment_id ? &info.payment_id : NULL,description,info.is_subaddress);
bool r = w->add_address_book_row(info.address, info.has_payment_id ? &info.payment_id : NULL,description,info.is_subaddress);
if (r)
refresh();
else
@ -74,15 +75,15 @@ void AddressBookImpl::refresh()
clearRows();
// Fetch from Wallet2 and create vector of AddressBookRow objects
std::vector<tools::wallet2::address_book_row> rows = m_wallet->m_wallet->get_address_book();
std::vector<tools::wallet2::address_book_row> rows = m_wallet->wallet()->get_address_book();
for (size_t i = 0; i < rows.size(); ++i) {
tools::wallet2::address_book_row * row = &rows.at(i);
std::string address;
if (row->m_has_payment_id)
address = cryptonote::get_account_integrated_address_as_str(m_wallet->m_wallet->nettype(), row->m_address, row->m_payment_id);
address = cryptonote::get_account_integrated_address_as_str(m_wallet->m_wallet_ptr->nettype(), row->m_address, row->m_payment_id);
else
address = get_account_address_as_str(m_wallet->m_wallet->nettype(), row->m_is_subaddress, row->m_address);
address = get_account_address_as_str(m_wallet->m_wallet_ptr->nettype(), row->m_is_subaddress, row->m_address);
AddressBookRow* abr = new AddressBookRow(i, address, row->m_description);
m_rows.push_back(abr);
}
@ -93,7 +94,7 @@ EXPORT
bool AddressBookImpl::deleteRow(std::size_t rowId)
{
LOG_PRINT_L2("Deleting address book row " << rowId);
bool r = m_wallet->m_wallet->delete_address_book_row(rowId);
bool r = m_wallet->wallet()->delete_address_book_row(rowId);
if (r)
refresh();
return r;

25
src/wallet/api/pending_transaction.cpp
View File

@ -87,6 +87,7 @@ bool PendingTransactionImpl::commit(std::string_view filename_, bool overwrite,
auto filename = fs::u8path(filename_);
auto w = m_wallet.wallet();
try {
// Save tx to file
if (!filename.empty()) {
@ -95,7 +96,7 @@ bool PendingTransactionImpl::commit(std::string_view filename_, bool overwrite,
LOG_ERROR(m_status.second);
return false;
}
bool r = m_wallet.m_wallet->save_tx(m_pending_tx, filename);
bool r = w->save_tx(m_pending_tx, filename);
if (!r) {
m_status = {Status_Error, tr("Failed to write transaction(s) to file")};
} else {
@ -104,14 +105,12 @@ bool PendingTransactionImpl::commit(std::string_view filename_, bool overwrite,
}
// Commit tx
else {
auto multisigState = m_wallet.multisig();
auto multisigState = m_wallet.multisig(w);
if (multisigState.isMultisig && m_signers.size() < multisigState.threshold) {
throw std::runtime_error("Not enough signers to send multisig transaction");
}
m_wallet.pauseRefresh();
const bool tx_cold_signed = m_wallet.m_wallet->get_account().get_device().has_tx_cold_sign();
const bool tx_cold_signed = w->get_account().get_device().has_tx_cold_sign();
if (tx_cold_signed){
std::unordered_set<size_t> selected_transfers;
for(const tools::wallet2::pending_tx & ptx : m_pending_tx){
@ -120,8 +119,8 @@ bool PendingTransactionImpl::commit(std::string_view filename_, bool overwrite,
}
}
m_wallet.m_wallet->cold_tx_aux_import(m_pending_tx, m_tx_device_aux);
bool r = m_wallet.m_wallet->import_key_images(m_key_images, 0, selected_transfers);
w->cold_tx_aux_import(m_pending_tx, m_tx_device_aux);
bool r = w->import_key_images(m_key_images, 0, selected_transfers);
if (!r){
throw std::runtime_error("Cold sign transaction submit failed - key image sync fail");
}
@ -129,7 +128,7 @@ bool PendingTransactionImpl::commit(std::string_view filename_, bool overwrite,
while (!m_pending_tx.empty()) {
auto & ptx = m_pending_tx.back();
m_wallet.m_wallet->commit_tx(ptx, blink);
w->commit_tx(ptx, blink);
// if no exception, remove element from vector
m_pending_tx.pop_back();
} // TODO: extract method;
@ -151,7 +150,6 @@ bool PendingTransactionImpl::commit(std::string_view filename_, bool overwrite,
LOG_ERROR(m_status.second);
}
m_wallet.startRefresh();
return good();
}
@ -159,16 +157,17 @@ EXPORT
uint64_t PendingTransactionImpl::amount() const
{
uint64_t result = 0;
auto w = m_wallet.wallet();
for (const auto &ptx : m_pending_tx) {
for (const auto &dest : ptx.dests) {
result += dest.amount;
}
service_nodes::staking_components sc;
uint64_t height = m_wallet.blockChainHeight();
uint64_t height = m_wallet.blockChainHeight(w);
std::optional<uint8_t> hf_version = m_wallet.hardForkVersion();
if (hf_version)
{
if (service_nodes::tx_get_staking_components_and_amounts(static_cast<cryptonote::network_type>(m_wallet.nettype()), *hf_version, ptx.tx, height, &sc)
if (service_nodes::tx_get_staking_components_and_amounts(static_cast<cryptonote::network_type>(w->nettype()), *hf_version, ptx.tx, height, &sc)
&& sc.transferred > 0)
result = sc.transferred;
}
@ -230,7 +229,7 @@ std::string PendingTransactionImpl::multisigSignData() {
tools::wallet2::multisig_tx_set txSet;
txSet.m_ptx = m_pending_tx;
txSet.m_signers = m_signers;
auto cipher = m_wallet.m_wallet->save_multisig_tx(txSet);
auto cipher = m_wallet.wallet()->save_multisig_tx(txSet);
return oxenmq::to_hex(cipher);
} catch (const std::exception& e) {
@ -249,7 +248,7 @@ void PendingTransactionImpl::signMultisigTx() {
txSet.m_ptx = m_pending_tx;
txSet.m_signers = m_signers;
if (!m_wallet.m_wallet->sign_multisig_tx(txSet, ignore)) {
if (!m_wallet.wallet()->sign_multisig_tx(txSet, ignore)) {
throw std::runtime_error("couldn't sign multisig transaction");
}

9
src/wallet/api/subaddress.cpp
View File

@ -46,7 +46,7 @@ SubaddressImpl::SubaddressImpl(WalletImpl *wallet)
EXPORT
void SubaddressImpl::addRow(uint32_t accountIndex, const std::string &label)
{
m_wallet->m_wallet->add_subaddress(accountIndex, label);
m_wallet->wallet()->add_subaddress(accountIndex, label);
refresh(accountIndex);
}
@ -55,7 +55,7 @@ void SubaddressImpl::setLabel(uint32_t accountIndex, uint32_t addressIndex, cons
{
try
{
m_wallet->m_wallet->set_subaddress_label({accountIndex, addressIndex}, label);
m_wallet->wallet()->set_subaddress_label({accountIndex, addressIndex}, label);
refresh(accountIndex);
}
catch (const std::exception& e)
@ -70,9 +70,10 @@ void SubaddressImpl::refresh(uint32_t accountIndex)
LOG_PRINT_L2("Refreshing subaddress");
clearRows();
for (size_t i = 0; i < m_wallet->m_wallet->get_num_subaddresses(accountIndex); ++i)
auto w = m_wallet->wallet();
for (size_t i = 0; i < w->get_num_subaddresses(accountIndex); ++i)
{
m_rows.push_back(new SubaddressRow(i, m_wallet->m_wallet->get_subaddress_as_str({accountIndex, (uint32_t)i}), m_wallet->m_wallet->get_subaddress_label({accountIndex, (uint32_t)i})));
m_rows.push_back(new SubaddressRow(i, w->get_subaddress_as_str({accountIndex, (uint32_t)i}), w->get_subaddress_label({accountIndex, (uint32_t)i})));
}
}

15
src/wallet/api/subaddress_account.cpp
View File

@ -46,14 +46,14 @@ SubaddressAccountImpl::SubaddressAccountImpl(WalletImpl *wallet)
EXPORT
void SubaddressAccountImpl::addRow(const std::string &label)
{
m_wallet->m_wallet->add_subaddress_account(label);
m_wallet->wallet()->add_subaddress_account(label);
refresh();
}
EXPORT
void SubaddressAccountImpl::setLabel(uint32_t accountIndex, const std::string &label)
{
m_wallet->m_wallet->set_subaddress_label({accountIndex, 0}, label);
m_wallet->wallet()->set_subaddress_label({accountIndex, 0}, label);
refresh();
}
@ -63,14 +63,15 @@ void SubaddressAccountImpl::refresh()
LOG_PRINT_L2("Refreshing subaddress account");
clearRows();
for (uint32_t i = 0; i < m_wallet->m_wallet->get_num_subaddress_accounts(); ++i)
auto w = m_wallet->wallet();
for (uint32_t i = 0; i < w->get_num_subaddress_accounts(); ++i)
{
m_rows.push_back(new SubaddressAccountRow(
i,
m_wallet->m_wallet->get_subaddress_as_str({i,0}),
m_wallet->m_wallet->get_subaddress_label({i,0}),
cryptonote::print_money(m_wallet->m_wallet->balance(i, false)),
cryptonote::print_money(m_wallet->m_wallet->unlocked_balance(i, false))
w->get_subaddress_as_str({i,0}),
w->get_subaddress_label({i,0}),
cryptonote::print_money(w->balance(i, false)),
cryptonote::print_money(w->unlocked_balance(i, false))
));
}
}

19
src/wallet/api/transaction_history.cpp
View File

@ -132,7 +132,8 @@ void TransactionHistoryImpl::refresh()
// one input transaction contains only one transfer. e.g. <transaction_id> - <100XMR>
std::list<std::pair<crypto::hash, tools::wallet2::payment_details>> in_payments;
m_wallet->m_wallet->get_payments(in_payments, min_height, max_height);
auto w = m_wallet->wallet();
w->get_payments(in_payments, min_height, max_height);
for (std::list<std::pair<crypto::hash, tools::wallet2::payment_details>>::const_iterator i = in_payments.begin(); i != in_payments.end(); ++i) {
const tools::wallet2::payment_details &pd = i->second;
std::string payment_id = tools::type_to_hex(i->first);
@ -146,7 +147,7 @@ void TransactionHistoryImpl::refresh()
ti->m_blockheight = pd.m_block_height;
ti->m_subaddrIndex = { pd.m_subaddr_index.minor };
ti->m_subaddrAccount = pd.m_subaddr_index.major;
ti->m_label = m_wallet->m_wallet->get_subaddress_label(pd.m_subaddr_index);
ti->m_label = w->get_subaddress_label(pd.m_subaddr_index);
ti->m_timestamp = pd.m_timestamp;
ti->m_confirmations = (wallet_height > pd.m_block_height) ? wallet_height - pd.m_block_height : 0;
ti->m_unlock_time = pd.m_unlock_time;
@ -164,7 +165,7 @@ void TransactionHistoryImpl::refresh()
//
std::list<std::pair<crypto::hash, tools::wallet2::confirmed_transfer_details>> out_payments;
m_wallet->m_wallet->get_payments_out(out_payments, min_height, max_height);
w->get_payments_out(out_payments, min_height, max_height);
for (std::list<std::pair<crypto::hash, tools::wallet2::confirmed_transfer_details>>::const_iterator i = out_payments.begin();
i != out_payments.end(); ++i) {
@ -190,20 +191,20 @@ void TransactionHistoryImpl::refresh()
ti->m_blockheight = pd.m_block_height;
ti->m_subaddrIndex = pd.m_subaddr_indices;
ti->m_subaddrAccount = pd.m_subaddr_account;
ti->m_label = pd.m_subaddr_indices.size() == 1 ? m_wallet->m_wallet->get_subaddress_label({pd.m_subaddr_account, *pd.m_subaddr_indices.begin()}) : "";
ti->m_label = pd.m_subaddr_indices.size() == 1 ? w->get_subaddress_label({pd.m_subaddr_account, *pd.m_subaddr_indices.begin()}) : "";
ti->m_timestamp = pd.m_timestamp;
ti->m_confirmations = (wallet_height > pd.m_block_height) ? wallet_height - pd.m_block_height : 0;
// single output transaction might contain multiple transfers
for (const auto &d: pd.m_dests) {
ti->m_transfers.push_back({d.amount, d.address(m_wallet->m_wallet->nettype(), pd.m_payment_id)});
ti->m_transfers.push_back({d.amount, d.address(w->nettype(), pd.m_payment_id)});
}
m_history.push_back(ti);
}
// unconfirmed output transactions
std::list<std::pair<crypto::hash, tools::wallet2::unconfirmed_transfer_details>> upayments_out;
m_wallet->m_wallet->get_unconfirmed_payments_out(upayments_out);
w->get_unconfirmed_payments_out(upayments_out);
for (std::list<std::pair<crypto::hash, tools::wallet2::unconfirmed_transfer_details>>::const_iterator i = upayments_out.begin(); i != upayments_out.end(); ++i) {
const tools::wallet2::unconfirmed_transfer_details &pd = i->second;
const crypto::hash &hash = i->first;
@ -224,7 +225,7 @@ void TransactionHistoryImpl::refresh()
ti->m_hash = tools::type_to_hex(hash);
ti->m_subaddrIndex = pd.m_subaddr_indices;
ti->m_subaddrAccount = pd.m_subaddr_account;
ti->m_label = pd.m_subaddr_indices.size() == 1 ? m_wallet->m_wallet->get_subaddress_label({pd.m_subaddr_account, *pd.m_subaddr_indices.begin()}) : "";
ti->m_label = pd.m_subaddr_indices.size() == 1 ? w->get_subaddress_label({pd.m_subaddr_account, *pd.m_subaddr_indices.begin()}) : "";
ti->m_timestamp = pd.m_timestamp;
ti->m_confirmations = 0;
m_history.push_back(ti);
@ -233,7 +234,7 @@ void TransactionHistoryImpl::refresh()
// unconfirmed payments (tx pool)
std::list<std::pair<crypto::hash, tools::wallet2::pool_payment_details>> upayments;
m_wallet->m_wallet->get_unconfirmed_payments(upayments);
w->get_unconfirmed_payments(upayments);
for (std::list<std::pair<crypto::hash, tools::wallet2::pool_payment_details>>::const_iterator i = upayments.begin(); i != upayments.end(); ++i) {
const tools::wallet2::payment_details &pd = i->second.m_pd;
std::string payment_id = tools::type_to_hex(i->first);
@ -248,7 +249,7 @@ void TransactionHistoryImpl::refresh()
ti->m_pending = true;
ti->m_subaddrIndex = { pd.m_subaddr_index.minor };
ti->m_subaddrAccount = pd.m_subaddr_index.major;
ti->m_label = m_wallet->m_wallet->get_subaddress_label(pd.m_subaddr_index);
ti->m_label = w->get_subaddress_label(pd.m_subaddr_index);
ti->m_timestamp = pd.m_timestamp;
ti->m_confirmations = 0;
ti->m_reward_type = from_pay_type(pd.m_type);

11
src/wallet/api/unsigned_transaction.cpp
View File

@ -70,7 +70,7 @@ bool UnsignedTransactionImpl::sign(std::string_view signedFileName_)
std::vector<tools::wallet2::pending_tx> ptx;
try
{
bool r = m_wallet.m_wallet->sign_tx(m_unsigned_tx_set, signedFileName, ptx);
bool r = m_wallet.wallet()->sign_tx(m_unsigned_tx_set, signedFileName, ptx);
if (!r)
{
m_status = {Status_Error, tr("Failed to sign transaction")};
@ -95,6 +95,7 @@ bool UnsignedTransactionImpl::checkLoadedTx(const std::function<size_t()> get_nu
std::unordered_map<cryptonote::account_public_address, std::pair<std::string, uint64_t>> dests;
int first_known_non_zero_change_index = -1;
std::string payment_id_string = "";
auto nettype = m_wallet.m_wallet_ptr->nettype();
for (size_t n = 0; n < get_num_txes(); ++n)
{
const wallet::tx_construction_data &cd = get_tx(n);
@ -134,10 +135,10 @@ bool UnsignedTransactionImpl::checkLoadedTx(const std::function<size_t()> get_nu
for (size_t d = 0; d < cd.splitted_dsts.size(); ++d)
{
const cryptonote::tx_destination_entry &entry = cd.splitted_dsts[d];
std::string address, standard_address = get_account_address_as_str(m_wallet.m_wallet->nettype(), entry.is_subaddress, entry.addr);
std::string address, standard_address = get_account_address_as_str(nettype, entry.is_subaddress, entry.addr);
if (has_encrypted_payment_id && !entry.is_subaddress)
{
address = get_account_integrated_address_as_str(m_wallet.m_wallet->nettype(), entry.addr, payment_id8);
address = get_account_integrated_address_as_str(nettype, entry.addr, payment_id8);
address += std::string(" (" + standard_address + " with encrypted payment id " + tools::type_to_hex(payment_id8) + ")");
}
else
@ -192,7 +193,7 @@ bool UnsignedTransactionImpl::checkLoadedTx(const std::function<size_t()> get_nu
std::string change_string;
if (change > 0)
{
std::string address = get_account_address_as_str(m_wallet.m_wallet->nettype(), get_tx(0).subaddr_account > 0, get_tx(0).change_dts.addr);
std::string address = get_account_address_as_str(nettype, get_tx(0).subaddr_account > 0, get_tx(0).change_dts.addr);
change_string += (boost::format(tr("%s change to %s")) % cryptonote::print_money(change) % address).str();
}
else
@ -290,7 +291,7 @@ std::vector<std::string> UnsignedTransactionImpl::recipientAddress() const
MERROR("empty destinations, skipped");
continue;
}
result.push_back(cryptonote::get_account_address_as_str(m_wallet.m_wallet->nettype(), utx.dests[0].is_subaddress, utx.dests[0].addr));
result.push_back(cryptonote::get_account_address_as_str(m_wallet.m_wallet_ptr->nettype(), utx.dests[0].is_subaddress, utx.dests[0].addr));
}
return result;
}

551
src/wallet/api/wallet.cpp
View File

File diff suppressed because it is too large Load Diff

23
src/wallet/api/wallet.h
View File

@ -49,6 +49,16 @@ class SubaddressImpl;
class SubaddressAccountImpl;
struct Wallet2CallbackImpl;
// Wrapper that holds a lock to prevent background refreshes, which kill things; provides ->
// indirection into the tools::wallet2 instance.
struct LockedWallet {
std::unique_lock<std::mutex> refresh_lock;
tools::wallet2* wallet;
LockedWallet(const std::unique_ptr<tools::wallet2>& w, std::mutex& refresh_mutex)
: refresh_lock{refresh_mutex}, wallet{w.get()} {}
tools::wallet2* operator->() { return wallet; }
};
class WalletImpl : public Wallet
{
public:
@ -99,6 +109,7 @@ public:
uint64_t balance(uint32_t accountIndex = 0) const override;
uint64_t unlockedBalance(uint32_t accountIndex = 0) const override;
std::vector<std::pair<std::string, uint64_t>>* listCurrentStakes() const override;
static uint64_t blockChainHeight(LockedWallet& w);
uint64_t blockChainHeight() const override;
uint64_t approximateBlockChainHeight() const override;
uint64_t estimateBlockChainHeight() const override;
@ -112,13 +123,13 @@ public:
void setAutoRefreshInterval(int millis) override;
int autoRefreshInterval() const override;
void setRefreshFromBlockHeight(uint64_t refresh_from_block_height) override;
uint64_t getRefreshFromBlockHeight() const override { return m_wallet->get_refresh_from_block_height(); };
uint64_t getRefreshFromBlockHeight() const override { return m_wallet_ptr->get_refresh_from_block_height(); };
void setRecoveringFromSeed(bool recoveringFromSeed) override;
void setRecoveringFromDevice(bool recoveringFromDevice) override;
void setSubaddressLookahead(uint32_t major, uint32_t minor) override;
bool watchOnly() const override;
bool rescanSpent() override;
NetworkType nettype() const override {return static_cast<NetworkType>(m_wallet->nettype());}
NetworkType nettype() const override {return static_cast<NetworkType>(m_wallet_ptr->nettype());}
void hardForkInfo(uint8_t &version, uint64_t &earliest_height) const override;
std::optional<uint8_t> hardForkVersion() const override;
bool useForkRules(uint8_t version, int64_t early_blocks) const override;
@ -136,6 +147,7 @@ public:
StakeUnlockResult* requestStakeUnlock(const std::string &sn_key) override;
static MultisigState multisig(LockedWallet& w);
MultisigState multisig() const override;
std::string getMultisigInfo() const override;
std::string makeMultisig(const std::vector<std::string>& info, uint32_t threshold) override;
@ -219,7 +231,6 @@ private:
void pendingTxPostProcess(PendingTransactionImpl * pending);
bool doInit(const std::string &daemon_address, uint64_t upper_transaction_size_limit = 0, bool ssl = false);
private:
friend class PendingTransactionImpl;
friend class UnsignedTransactionImpl;
friend class TransactionHistoryImpl;
@ -228,8 +239,9 @@ private:
friend class SubaddressImpl;
friend class SubaddressAccountImpl;
std::unique_ptr<tools::wallet2> m_wallet;
std::unique_ptr<tools::wallet2> m_wallet_ptr;
mutable std::mutex m_statusMutex;
LockedWallet wallet() const { return {m_wallet_ptr, m_refreshMutex2}; }
mutable std::pair<int, std::string> m_status;
std::string m_password;
std::unique_ptr<TransactionHistoryImpl> m_history;
@ -247,7 +259,7 @@ private:
std::mutex m_refreshMutex;
// synchronizing sync and async refresh
std::mutex m_refreshMutex2;
mutable std::mutex m_refreshMutex2;
std::condition_variable m_refreshCV;
std::thread m_refreshThread;
std::thread m_longPollThread;
@ -264,5 +276,4 @@ private:
std::optional<tools::login> m_daemon_login;
};
} // namespace