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.

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

simplewallet: add set_tx_key for importing tx keys from 3rd party wallets #4188

Merged
merged 1 commit into from
Aug 15, 2018

Conversation

stoffu
Copy link
Contributor

@stoffu stoffu commented Jul 29, 2018

Motivated by this discussion: https://www.reddit.com/r/Monero/comments/92ekxo/i_sent_monero_to_binance_from_mymonero_but_havent/

This patch allows users to import tx secret keys after restoring a wallet that was originally created using 3rd party wallet services, like MyMonero, so that tx proofs can then be generated.

@@ -8583,6 +8583,52 @@ bool wallet2::get_tx_key(const crypto::hash &txid, crypto::secret_key &tx_key, s
return true;
}
//----------------------------------------------------------------------------------------------------
bool wallet2::set_tx_key(const crypto::hash &txid, const crypto::secret_key &tx_key, const std::vector<crypto::secret_key> &additional_tx_keys)
{
THROW_WALLET_EXCEPTION_IF(m_tx_keys.find(txid) != m_tx_keys.end(), error::wallet_internal_error, "Tx key for is already stored in the cache");
Copy link
Collaborator

Choose a reason for hiding this comment

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

That means if you make a mistake, you can't correct it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

std::vector<tx_extra_field> tx_extra_fields;
THROW_WALLET_EXCEPTION_IF(!parse_tx_extra(tx.extra, tx_extra_fields), error::wallet_internal_error, "Transaction extra has unsupported format");
tx_extra_pub_key pub_key_field;
for (size_t i = 0; i < 2; ++i)
Copy link
Collaborator

Choose a reason for hiding this comment

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

That seems wrong. Either assume only one, or check all of them. Besides, it looks like it'll never get to try the second since it breaks if it succeeds on the first one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

}

crypto::secret_key tx_key;
if (!epee::string_tools::hex_to_pod(local_args[1].substr(0, 64), tx_key))
Copy link
Collaborator

Choose a reason for hiding this comment

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

That throws if the string is less than 64: need to catch.

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 don't see hex_to_pod throwing; it just returns false when the string size is wrong:

https://github.com/monero-project/monero/blob/master/contrib/epee/include/string_tools.h#L343

  template<class t_pod_type>
  bool hex_to_pod(const std::string& hex_str, t_pod_type& s)
  {
    ...
    if(sizeof(s)*2 != hex_str.size())
      return false;
    ...
  }

Copy link
Collaborator

Choose a reason for hiding this comment

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

It doesn't. substr does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, thanks.

if (local_args[1].empty())
break;
additional_tx_keys.resize(additional_tx_keys.size() + 1);
if (!epee::string_tools::hex_to_pod(local_args[1].substr(0, 64), additional_tx_keys.back()))
Copy link
Collaborator

Choose a reason for hiding this comment

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

That also throws.

m_tx_keys.insert(std::make_pair(txid, tx_key));
m_additional_tx_keys.insert(std::make_pair(txid, additional_tx_keys));
return true;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe remove the bool since all the errors are exceptions, and the caller doesn't check the return value, makes it less likely a future chance will return false on error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@luigi1111 luigi1111 merged commit a3fe1c5 into monero-project:master Aug 15, 2018
luigi1111 added a commit that referenced this pull request Aug 15, 2018
a3fe1c5 simplewallet: add set_tx_key for importing tx keys from 3rd party wallets (stoffu)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants