0xPolygonMiden / miden-client

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

Revisit transactions to commit check #252

Closed mFragaBA closed 5 months ago

mFragaBA commented 5 months ago

Now that we have onchain notes, because we receive note IDs based on account ID tags we cannot base the status change on output notes alone, since we might not be receiving all the output notes of a transaction. Perhaps we can re-use the /GetNotesById endpoint

          Yes, let's create an issue to follow up

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

igamigo commented 5 months ago

@bobbinth do you have any comments on this? Because the client does not get updates for notes that are not for accounts that it is tracking (ie they are not kept on the input notes table), I might not know when a P2ID transaction gets committed (for example, because I sent you a note and so it's for an account I'm not tracking, so I'm not able to tell if the note was included and hence the transaction committed). There's at least 2 solutions for this:

bobbinth commented 5 months ago

A couple of thoughts:

  1. If note sender is one of the accounts managed by the client, we should already be getting all required data back during the state sync requests. This is because, the notes returned by from the state sync request are matched based on tags or sender (see here). If this is not currently working, it is possible that there is a bug somewhere in the node, or maybe the client is not making use of this info?
  2. The network always knows the sender of a specific note (in the future, it will be possible to hide this via proxy transactions, but we don't have these implemented yet). This info is recorded in note metadata which is always public. So, using GetNotesById for such notes should not leak too much info (i.e., not more info than is being leaked during state sync requests where we specify the set of accounts we are interested in).

So, basically, I think we should already have all required data (but may need to make sure there are no bugs either on the node or client side), but also that using GetNotesById should be fine, if needed.

mFragaBA commented 5 months ago

closing as #313 got merged