0xPolygonMiden / miden-client

Client library that facilitates interaction with the Miden rollup
MIT License
27 stars 23 forks source link

feat: handle old account updates in sync #412

Open mFragaBA opened 1 week ago

mFragaBA commented 1 week ago

addresses #360. (The PR is already reviewable but I think this will be included in the following release)

The issue is that after we submit the proven transaction we update our local account state. This means that when we receive the sync state update, we already have that update. In order to ensure integrity in private notes and to ensure we have the most up-to-date version of a public account, we check the received account hash against the current state for that account.

This generally works fine, but then in might happen the following:

  1. you execute and submit a TX
  2. the TX gets included in a block
  3. you execute and submit another TX with the same account (without previously syncing)
  4. you sync before the second transaction gets included in the block
  5. you sync

In this situation, the node will send an update of the account after the first TX but not after the second. With the current client this would mean:

In this PR the approach taken (I'm open to trying other alternatives) is that in both cases we should also check whether the update is "stale" or not.

igamigo commented 11 hours ago

The code for this looks good, but I wonder if we might want to do a different approach. Getting all old account states is probably not very future-proof. An alternative could be to store account the account hash in the accounts table, and when we receive an update we search by account hash to see if we had that state already (I think this is what you are suggesting as well).

instead of handling the corner case, just changing the error so that we request the user to retry in a few minutes (the problem with that is that it doesn't provide useful info to the user and if we wanted to do so we'd probably still need to handle the error and use a function akin to get_account_stub_history or its alternative approaches)

I think this could work but it's possible that the state got corrupted somehow and it would not be detectable.

tomyrd commented 7 hours ago

Just changed the get_account_stub_history to get_account_stub_by_hash with the addition of an account_hash column in the store.