-
br-m
<r4v3r23> @rbrunner7:monero.socialyoure handling polyseed implementation?
-
br-m
<rbrunner7> @r4v3r23:monero.social: Yes
-
br-m
<rbrunner7> In the Monero core software, the CLI wallet and hopefully later also in the GUI wallet, though I am not sure my Qt knowledge will be up to the task there
-
br-m
<r4v3r23> @rbrunner7: is there a PR?
-
br-m
<r4v3r23> tobtoht wrote a WIP patch a couple of years ago that works
-
br-m
<rbrunner7> No, not yet progressed far enough to PR. Standing maybe at 70%
-
br-m
<rbrunner7> Yes, after consulting with tobtotht I took that patch (which is now alive as code in monero-c) and modified it gently
-
br-m
<r4v3r23> @rbrunner7: ok great. is the plan to add it during the hardfork or is it coming before?
-
br-m
<rbrunner7> Not sure. I don't think something will prevent this PR going in before the hardfork to FCMP++
-
br-m
<rbrunner7> Didn't yet discuss any "plan" with selsta however
-
br-m
<r4v3r23> has there been a decision on seed encryption? legacy offset vs PS's built in passphrase?
-
br-m
<rbrunner7> Well, I have a proposal that I now implement, and when I PR, people can certainly comment: Support encrypted Polyseeds when restoring, but not producing any encrypted Polyseeds and working only with seed offsets
-
br-m
<r4v3r23> not sure if polyseed's built in encryption was even shipped in the wild
-
br-m
<r4v3r23> but that seems like a good balance
-
br-m
<rbrunner7> It's active in Cake Wallet: If you create a wallet and give a passphrase, it generates an encrypted Polyseed. But I think that was the only source of such seeds that I ever saw
-
br-m
<r4v3r23> lol
-
br-m
<r4v3r23> yeah
-
br-m
<rbrunner7> Yeah, why not :)
-
br-m
<r4v3r23> heads up: getSeed() doesnt play nice after stopBackgroundSync(), spoke the jberman about it
-
br-m
<rbrunner7> You mean that looks like a bug in the current release code?
-
br-m
<r4v3r23> in the current version of tobtohts patch + release bg sync code
-
br-m
<r4v3r23> theres a hotfix but well have it reviewed with your PR i guess
-
br-m
<rbrunner7> Ah, I see. Then it's probably a good idea that I have a look. Sounds strange that these 2 things should be connected somehow, but everything is possible
-
br-m
<r4v3r23> ah wait
-
br-m
<rbrunner7> A hotfix against what?
-
br-m
<r4v3r23> thats on the bg sync side
-
br-m
<r4v3r23> but itll need the polyseed PR to get a proper fix in
-
br-m
<rbrunner7> Maybe @jberman:monero.social knows already more details and can chime in
-
br-m
<r4v3r23> i mixed up, the bug is on bg sync since it was obviosuly written without PS in mind
-
br-m
<r4v3r23> its only present on polyseed wallets tho
-
br-m
<rbrunner7> I wonder a bit how monero-c's story will continue after we have Polyseed support as regular part in the Monero core code. That library supports 3 coins, Monero, Wownero and Zano. Now supporting those 3 coins work in pretty similar ways, however after my addition differences will grow
-
br-m
<rbrunner7> It's based anyway on a pretty old version of the Monero core code, that's not a very good story if you ask me
-
br-m
<rbrunner7> It probably does not matter too much, because it's only for wallet apps, and not that much happened in wallet2 and nearby code, but still
-
br-m
<jberman> Basically just need to be sure to properly bring the polyseed back in memory when stopping background sync > <@rbrunner7> Maybe @jberman:monero.social knows already more details and can chime in
-
br-m
<rbrunner7> The Polyseed? Not just the spend private key?
-
br-m
<rbrunner7> Polyseed or not Polyseed, there is only ever a single spend private key, no?
-
br-m
<jberman> I guess depends on the impl, but presumably yes. The bug was that starting background sync clears spend key + polyseed, and then only spend key was brought back properly but not the polyseed
-
br-m
<rbrunner7> You mean the Polyseed as it's part of the Account object as instantiated by wallet2?
-
br-m
<r4v3r23> @rbrunner7: private key is fine, yo ucan still spend after, its just displaying the actual seed phrase that crasehs it
-
br-m
<jberman> The bug in that patch was that forget_spend_key would also clear where the polyseed was kept in memory
-
br-m
<rbrunner7> Ah, I see, the Polyseed has to go in the interest of security
-
br-m
<jberman> And then it wasn't properly brough back into memory
-
br-m
<rbrunner7> I see. Is there a "hotfix" from you that I could incorporate?
-
br-m
<jberman> Probably does make sense to clear while background syncing. Extra info not exactly critical for background sync to function
-
br-m
-
br-m
<jberman> I don't know if you're keeping the polyseed in the same place in memory as the other polyseed patch is, so I'm not sure if it would apply to your code. The other code was keeping it stored in the account keys same place as private spend key, so it's also stored in the .keys file
-
br-m
<rbrunner7> Yes, that's the approach. I did not deviate from that
-
br-m
<jberman> This was my proposed fix (the hotfix won't work for all cases, should only work for anonero / "unattended" walles):
-
br-m
<rbrunner7> Thanks to you both, will certainly have a look.
-
br-m
<jberman> one sec
-
br-m
<rbrunner7> One more thing? :)
-
br-m
<jberman> the more optimal fix is:
-
br-m
<jberman> A) recover cryptonote::account_keys from verify_password rather than the secret spend key alone (so that you also recover the polyseed)
-
br-m
<jberman> B) if m_polyseed, verify the recovered polyseed is correct (probably by verifying that it corresponds to the priv spend key) in addition to the checks already done that the priv spend key corresponds to the pub spend key [I actually don't know if this is possible to do, probably isn't and I wrongly assumed so]
-
br-m
<jberman> C) have m_account.set_spend_key also take the polyseed as an optional param where if provided, set polyseed also
-
br-m
<jberman> "in addition to the checks already done that the priv spend key corresponds to the pub spend key" -> this is referring to the checks already done in stop_background_sync
-
br-m
<rbrunner7> Ok, might take some work for me to read into everything, but I am optimistic that I will find a good. Man, it's high time we solve Polyseed once and for all as core part of the regular Monero code ...
-
br-m
<rbrunner7> *good solution
-
br-m
<r4v3r23> @rbrunner7: definitely looking forward to this
-
br-m
<rbrunner7> I hope that MrCyjaneK will come around to modify monero-c to use it. The solution for wallet apps that are based on that library is of course not complete until then.
-
br-m
<rbrunner7> Maybe it won't be too hard - continue to use the "patch approach" for Wownero and Zano and stop to use any patch for the Monero part?
-
br-m
<rbrunner7> Hey, I feel pressured by all those thanks and upvotes emojis :)
-
br-m
<rbrunner7> I guess now I have to deliver
-
br-m
-
br-m
<jpk68:matrix.org> I didn't know monerod had gRPC :P
-
br-m
<rbrunner7> They are talking about using Cuprate instead
-
br-m
<jpk68:matrix.org> Ah, my bad
-
br-m
<rbrunner7> But I think that hasn't gRPC for wallet sync either out of the box?
-
br-m
<rbrunner7> Maybe they vibe-coded it ...
-
br-m
<boog900> Yes they AI'd the rest of the RPC endpoints and then added gRPC
-
br-m
<boog900> On our old too DB btw
-
br-m
<jpk68:matrix.org> Would be cool to have CBOR or something. gRPC is kind of bloated
-
br-m
<rbrunner7> @boog900:monero.social: Are you in contact with them?
-
br-m
<boog900> no I think this is their fork
-
br-m
-
br-m
<boog900> I saw it a week or so ago
-
br-m
-
selsta
from what I remember out current wallet sync code is quite slow so it seems plausible they got a large speedup
-
br-m
-
br-m
<jpk68:matrix.org> Lmao, they wrote tests in C++ > <@boog900>
github.com/tex8com/cuprate/tree/fast-rpc
-
plowsof
cover all bases
-
br-m
<rbrunner7> Hmm. Let's see where this all leads us.
-
br-m
<boog900> I am surprised the AI managed to get it working tbf
-
br-m
<rbrunner7> The truth is in solid, working, production quality code, anyway.
-
br-m
<boog900> it looks like it was just allowed to rip with little input
-
br-m
<rbrunner7> I wonder whether it really works. I mean, whether it syncs correctly if there are received enotes.
-
br-m
<rbrunner7> Not only that it munches through all the blocks in record time.
-
br-m
<boog900> Yeah I wouldn't be surprised if there are many bugs
-
br-m
<rbrunner7> Might be these people behind that Cuprate fork:
tex8.com
-
br-m
<rbrunner7> "In days, not months" :)
-
br-m
<rbrunner7> Sometimes I wish I was a startup guy for just a few weeks. Promise the sky, get noticed, get people raving about what you will build, collect some millions from venture capitalist, burn through that, vanish ...
-
br-m
<boog900> I think I am most annoyed about them using our old DB, the new one is so much better
-
br-m
<rbrunner7> Lol
-
br-m
-
br-m
<rbrunner7> I understand, but frankly, that would be the last thing on my mind :)
-
br-m
<rbrunner7> Confronted with these claims
-
br-m
<boog900> what claims?
-
br-m
<rbrunner7> The speedup claims, like 46x for some steps of syncing
-
br-m
<rbrunner7> But hey, maybe we really should explore things like pruning txs on the fly and zipping RPC traffic on the fly seriously
-
br-m
<rbrunner7> And aggressive batching of things
-
br-m
<jpk68:matrix.org> Cap'n Proto (which I think bitcoind uses for some things) is supposedly ~75% smaller and up to 10x faster
-
br-m
<jpk68:matrix.org> Also a lot more memory-efficient, of course
-
br-m
<rbrunner7> Yes, there certainly is no lack of RPC protocols and libraries. Nobody really takes you seriously as a dev until you designed and built one as well
-
br-m
<boog900> we should have them beat daemon side with the new DB for normal RPC. That one 46x speed up by making 6,600 calls 1 isn't even a different call for us now, it's joined to the block blobs request, so it's all 1 single DB request now. > <@rbrunner7> The speedup claims, like 46x for some steps of syncing
-
br-m
<boog900> The bugs they mentioned that slowed sync I had already fixed
-
br-m
<boog900> If they just used the new DB the only speed up there would be gRPC I think
-
br-m
<boog900> The pruning on the fly thing was just because the old DB did no pruning at all and always returned full blobs
-
br-m
<boog900> The new does separate pruned/prunable blobs and supports returning just pruned blobs