0xPolygonMiden / miden-node

Reference implementation of the node for the Polygon Miden rollup
MIT License
52 stars 37 forks source link

Implemented account state deltas storage and endpoint for getting account state deltas #418

Closed polydez closed 2 months ago

polydez commented 2 months ago

Resolves https://github.com/0xPolygonMiden/miden-node/issues/290

We decided to implement delta-based storage for public accounts (full account state for the latest state and deltas for all updates except creation of new account). This is needed for GetAccountStateDelta endpoint which is also implemented in this PR.

The only question I still have, if it enough to use block numbers for tracking account state updates on client or we need to use account nonces for that.

polydez commented 2 months ago

In general, I think using block_num to define the range works better than using nonces for this - but would be good for @igamigo to confirm if this is fine on the client side.

I want to add, that we might want to use nonces in order to enforce limits to endpoint for current implementation. Our current implementation is great in terms of space consumption and it's pretty fast to get needed deltas, but for some accounts it might be needed to get enormous number of deltas in order to sync to the latest state. Obvious solution is to limit number of deltas processed by one single call to the endpoint. For cases, when deltas number is too hight it might be better to just request account's full state.

I see two solutions for it:

  1. Limit maximum number of deltas processed in current solution. This will require adding block number to the response in order for the client to be aware that not all requested deltas were returned and make next requests starting from the last returned block number until it get all needed deltas. Another solution is to just return an error, if number of deltas is too high and client will request account's full latest state.
  2. Limit maximum number of account changes requested (by nonce). Return an error, if number of nonces (to_nonce - from_nonce) requested is higher than the limit. In this case client should request account's full latest state.
bobbinth commented 2 months ago

I want to add, that we might want to use nonces in order to enforce limits to endpoint for current implementation. Our current implementation is great in terms of space consumption and it's pretty fast to get needed deltas, but for some accounts it might be needed to get enormous number of deltas in order to sync to the latest state. Obvious solution is to limit number of deltas processed by one single call to the endpoint. For cases, when deltas number is too hight it might be better to just request account's full state.

I see two solutions for it:

  1. Limit maximum number of deltas processed in current solution. This will require adding block number to the response in order for the client to be aware that not all requested deltas were returned and make next requests starting from the last returned block number until it get all needed deltas. Another solution is to just return an error, if number of deltas is too high and client will request account's full latest state.
  2. Limit maximum number of account changes requested (by nonce). Return an error, if number of nonces (to_nonce - from_nonce) requested is higher than the limit. In this case client should request account's full latest state.

I'm leaning toward the first approach. In general, I think we could introduce "epoch-based pagination" I described in https://github.com/0xPolygonMiden/miden-node/issues/404#issuecomment-2226192376 to a couple of endpoints and this would be one of them. We should also "stress-test" at some point to see what happens if the response is too big. The default Tonic message size is 4MB. Would be good to find out what happens if we exceed this limit (i.e., what will break and what kind of an error message we'll get). Let's create an issue for this.

polydez commented 2 months ago

Added issue: https://github.com/0xPolygonMiden/miden-node/issues/422