MrCyjaneK / monero_c

GNU Lesser General Public License v3.0
4 stars 4 forks source link

A Faster Encryption Key Hash #86

Open Charon-Fan opened 3 weeks ago

Charon-Fan commented 3 weeks ago

Hi @MrCyjaneK , I noticed that Monero supports four types of UR and uses encrypt_with_view_secret_key and decrypt_with_view_secret_key to keep things safe when sending data. Sadly, cn_slow_hash is too hard on many devices. Can we switch to cn_fast_hash instead? I'd like to help by adding code and want to talk more about it.

MrCyjaneK commented 3 weeks ago

Hey! I don't have anything against supporting other formats - however I didn't notice any issue with it currently. By some devices did you mean low end phones or something closer to embedded?

Charon-Fan commented 3 weeks ago

I'm talking about some small devices where the slow hash needs a few M of memory, which is often hard to do on these devices since most small hardware doesn't have much memory. I think if the private view key is the same, it doesn't really matter what hash you use. What do you think? Can't wait to hear back from you! @MrCyjaneK

MrCyjaneK commented 2 weeks ago

Hey! Sorry for delayed reply - I'm up to having that kind of feature, however it should maintain backwards compatibility with other wallets. Do you plan on opening a PR to have this feature in monero_c?

Charon-Fan commented 2 weeks ago

Yes, I hope to open a PR to monero_c, either as a patch or another method. If you're okay with that, I'll start asap. By the way, what approach do you think is better? I think adding a new UR type could ensure compatibility with older versions, but it might add extra maintenance work.

MrCyjaneK commented 2 weeks ago

@Charon-Fan I see two options

MrCyjaneK commented 2 weeks ago

If you need any help, support or assistance I'm up to guide you through the steps of adding a patch, easiest way is to

  1. clone the repo, run apply-patches.sh script
  2. Make you change and commit it (in monero submodule)
  3. $ git format-patch -1
  4. Move it to patches directory

There is a pending cleanup PR going on currently so a new patch is fine, ideally I would like it to be in the UR patch but.. don't bother with it, I have to cleanup all of the patches anyway.

Charon-Fan commented 1 week ago

Hey bro @MrCyjaneK , I want to check something with you. I noticed that when unsigned data is sent to an offline device, it seems like the minor subaddress is missing. The current method is to search for it by going through a subaddress map. Am I understanding this right? https://github.com/monero-project/monero/blob/b089f9ee69924882c5d14dd1a6991deb05d9d1cd/src/cryptonote_basic/cryptonote_format_utils.cpp#L314

MrCyjaneK commented 1 week ago
From f49bc7059762210dc0d00c4ad0ab2461f81c8119 Mon Sep 17 00:00:00 2001
From: ANONERO <anonero@dnmx.org>
Date: Mon, 20 May 2024 16:11:57 +0000
Subject: [PATCH] Offline-signing: show actual address when spending to
 integrated address

fix by tobtoht
---
 src/wallet/api/unsigned_transaction.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/wallet/api/unsigned_transaction.cpp b/src/wallet/api/unsigned_transaction.cpp
index 6165a2240..2202f06bb 100644
--- a/src/wallet/api/unsigned_transaction.cpp
+++ b/src/wallet/api/unsigned_transaction.cpp
@@ -297,7 +297,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(utx.dests[0].address(m_wallet.m_wallet->nettype(), {}));
     }
     return result;
 }

This may be the fix, if I understand the issue correctly @Charon-Fan

Charon-Fan commented 1 week ago

Sorry, I didn't explain it clearly. The specific situation is that offline signing needs to generate a key image, but the data is missing the minor of the output I need (I only have the unsigned_tx_set). I want to know if I need to find it myself by going through the list.