search

0xPolygonMiden / miden-client

Client library that facilitates interaction with the Miden rollup
MIT License
33 stars 29 forks source link

feat: handle nullified note import corner case #547

Open tomyrd opened 2 weeks ago

tomyrd commented 2 weeks ago

Are we sure this always means it was consumed externally? What would happen if the client already had the note and it was consumed by a local account? Probably something that would not occur in the real world (you would have to consume the note and then import it again) but just wondering.

_Originally posted by @igamigo in https://github.com/0xPolygonMiden/miden-client/pull/531#discussion_r1785114348_

This is a very specific corner case where a user imports a note that already tracked by the client and is in a processing state. This means that when it gets imported, we will probably get a nullifier but we wouldn't know if it was consumed by the current tracked transaction or a different one.

Currently we are marking it as ConsumedExternally as it handles the most common case.

tomyrd commented 2 weeks ago

One possible solution is to not allow imports that update tracked processing notes as they are in an "unstable" state that will probably change in the next sync.

bobbinth commented 2 weeks ago

What additional info could be added to the processing note as a part of the import? It seems like if it is processing, then it already has metadata, authentication path etc. So, importing it shouldn't add any new info and should result in no transition. Or am I missing something?

tomyrd commented 1 week ago

With NoteFile::NoteId and NoteFile::NoteWithProof imports we also check if the imported note is already consumed (by using the CheckNullifiersByPrefix endpoint). In the cases where we detect that the note is already consumed in the node then the note is updated with the consumed_externally transition which changes processing notes into ConsumedExternally which could be wrong if the note was consumed by the local transaction.

bobbinth commented 1 week ago

Makes sense. It seems reasonable to return an error if we are trying to import a note that is currently being processed.

One thing that it does make me wonder about is whether our import process is "overloaded" too much and if there is a way to simplify it. But I don't have concrete thoughts on this yet.