Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement support for on-chain accounts #142

Closed
bobbinth opened this issue Jan 11, 2024 · 5 comments · Fixed by #294
Closed

Implement support for on-chain accounts #142

bobbinth opened this issue Jan 11, 2024 · 5 comments · Fixed by #294
Assignees
Labels
block-producer Related to the block producer component rpc Related to the RPC component store Related to the store component
Milestone

Comments

@bobbinth
Copy link
Contributor

Currently, the node can handle only off-chain accounts. To support on-chain accounts we'd need to do the following:

  1. Add necessary tables to the store database to store on-chain account info.
  2. Update block producer to make sure it extracts on-chain account data from proven transactions and send this data to the store via apply_block.
  3. Update the store to record the on-chain account data in the DB.
  4. Update the store and the RPC to provide an endpoint for users to retrieve on-chain account details.

This work requires 0xPolygonMiden/miden-base#403 to be completed.

@bobbinth bobbinth added store Related to the store component rpc Related to the RPC component block-producer Related to the block producer component labels Jan 11, 2024
@bobbinth bobbinth added this to the v0.2 milestone Jan 11, 2024
@hackaugusto
Copy link
Contributor

For some context:

related issues in miden-base: 0xPolygonMiden/miden-base#403 0xPolygonMiden/miden-base#402
related PRs so far: 0xPolygonMiden/miden-base#481 0xPolygonMiden/miden-base#485

@polydez polydez self-assigned this Mar 8, 2024
@polydez polydez moved this from Todo to In Progress in Builder's testnet Mar 8, 2024
@polydez polydez linked a pull request Mar 26, 2024 that will close this issue
@polydez
Copy link
Contributor

polydez commented Mar 27, 2024

@bobbinth @hackaugusto @phklive, how do you think, should we add on-chain account details into the response of sync_state endpoint? Now it returns just account hashes. As an alternative, we can implement different endpoint for account full state.

@bobbinth
Copy link
Contributor Author

bobbinth commented Mar 27, 2024

I don't have a great answer to this yet - but maybe this is the right time to start thinking this through. So, a few preliminary thoughts/questions:

  1. For public account, we should have an endpoint that would return the full account state. A few questions about how this endpoint should work:
    a. Should it return only the latest sate? Or should we try to return the state at some prior point in time? The former is going to be much easier but not sure if that will be sufficient.
    b. Should this endpoint allow returning only some portions of the account state? For example, should we be able to request just the storage or just the vault etc.?
  2. It would be cool if we somehow were send only the minimum set of changes required to bring the account state up to the latest point. So, if the public account was updated, I should be able to download only the delta to get it up to date. This means that for public accounts we'd need to store all historical deltas.
  3. How would the client get either the full account state or the delta? And how could we determine what is better to send in a given situation? (i.e., in some cases sending the full account state may be cheaper than sending a set of all deltas).

One potential approach is as follows:

  1. We don't change the state sync endpoint at all - or maybe change it very minimally. Some options here include:
    a. For public accounts, in addition to the latest state hash we send also the latest nonce.
    b. Or maybe we send all underlying hashes of the latest state - i.e., code root, vault root, storage root, and nonce - but nothing more than that.
  2. The client would be able to use the above info to determine that the state of the account hash changed and would use a separate endpoint to get the actual changes.
  3. This separate endpoint could be something like get_account_state_delta. The arguments would be account_id, from_nonce and to_nonce. This endpoint would return a single delta describing the changes which happened in the account between the two specified nonces. The big question here is if we can implement this efficiently. Doing something like this would require:
    a. Storing deltas for all accounts in the node (could be something like account_state_deltas table).
    b. Merging all deltas between the to/from nonces into a single delta on the fly. It would be awesome if we could pre-compute this somehow - but not sure that's possible.

@polydez
Copy link
Contributor

polydez commented Mar 27, 2024

I've implemented one single endpoint for getting the latest details for public account. I think, it's enough for now, but we should create separate issue(s) for storage/endpoints improvements.

@bobbinth
Copy link
Contributor Author

I've implemented one single endpoint for getting the latest details for public account. I think, it's enough for now, but we should create separate issue(s) for storage/endpoints improvements.

I think this is fine for now - but let's create a separate issue specifically for implementing the delta-based approach.

@Dominik1999 Dominik1999 moved this from In Progress to In Review in Builder's testnet Apr 8, 2024
@polydez polydez moved this from In Review to Done in Builder's testnet Apr 9, 2024
@polydez polydez closed this as completed Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
block-producer Related to the block producer component rpc Related to the RPC component store Related to the store component
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants