seraphis-migration / wallet3

Info and discussions about a hypothetical full 'wallet2' rewrite from scratch
MIT License
14 stars 0 forks source link

Spend_proof and node connection #56

Open DangerousFreedom1984 opened 1 year ago

DangerousFreedom1984 commented 1 year ago

I'm writing the generate_legacy_spend_proof and I'm stuck to get the public keys of the ring members because there are two design choices in my opinion.

1) Do like it is done in wallet2. The wallet queries the node using the tx_id to get the public keys of the ring members. 2) Store this information on the TransactionHistory component and update this component whenever the user wants or when the wallet is idle.

I believe that the best option is the second because the prover could also generate spend proofs without being connected to a node (if he has an updated TransactionHistory component) for example avoiding this issue: https://github.com/monero-project/monero/issues/8863.

But I think it is worth the discussion since seraphis_wallet appears to be going in the direction of avoiding having functions to connect directly to the node but instead use objects (that were previously updated through a node) whenever possible.

UkoeHB commented 1 year ago

I recommend doing what wallet2 does. A) this is much simpler, B) if you cannot guarantee that the information is always available for offline wallets ("update this component whenever the user wants or when the wallet is idle." implies a race condition in the gap between balance recovery and going offline) then it's not worthwhile.

DangerousFreedom1984 commented 1 year ago

I don't agree. A) It looks simpler to have everything in one function but modularity pays off (for example solving the problem of being capable of generating these proofs offline). B) There is no race condition. If the object doesnt have the info, then it is not able to provide it. Simple as that. The same way as if your node doesnt have the block to provide the info. But yeah, if you want to make sure that you have the info (in this case the public keys of the ring members of a tx) then you have to make sure that your wallet is updated when you go offline. Probably by exiting the program correctly.

UkoeHB commented 1 year ago

There is no race condition. If the object doesnt have the info, then it is not able to provide it. Simple as that.

There is a race condition to whether or not you have the info. It is not a valid UX to say 'this feature is available at random'.

DangerousFreedom1984 commented 1 year ago

There is a race condition to whether or not you have the info. It is not a valid UX to say 'this feature is available at random'.

You could give the same argument to the EnoteStore and then we would end up with wallet2 again. Of course there are ways of preventing that and the wallet will have to work with some kind of "seraphis engine" anyway.

UkoeHB commented 1 year ago

You could give the same argument to the EnoteStore

Info in the enote store is not available at random. It is available up the the nth scanned block.

DangerousFreedom1984 commented 1 year ago

Yeah, I know but the idea is the same. It is available after some time (due to scanning/processing) but not immediately.