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

Return a list of committed transactions from the state sync endpoint #371

Closed
bobbinth opened this issue May 31, 2024 · 4 comments
Closed
Assignees
Labels
enhancement New feature or request rpc Related to the RPC component store Related to the store component
Milestone

Comments

@bobbinth
Copy link
Contributor

To address the issues discussed in 0xPolygonMiden/miden-client#363 we should modify the state sync endpoint to return a list of transactions committed to the chain for the accounts the client is interested in.

The set of accounts is specified via account_ids field in the SyncStateRequest message. For every such account, we should return a list of transaction IDs for transactions executed against this account between blocks specified by request.block_num and response.block_header.block_num.

Impact on SyncStateResponse

One question is how exactly we should return the transaction IDs. A couple options come to mind:

  1. We can add a new field to the SyncStateResponse - e.g., transactions. This field would contain a simple list transaction IDs (i.e., repeat TransactionId) for all of the requested account IDs within the relevant time interval.
  2. We could modify the AccountSummary message to contain the list of transaction IDs affecting this specific account.

A downside of the first approach is that we don't explicitly return info about which transaction affected which account - but I'm not sure how important this info is (i.e., the client should have this info already).

A downside of the second approach is that it is not clear what to do for transactions which do not change account state (e.g., a transaction may consume and produce some notes, but leave the account state unchanged; in the current structure, we would not have an account summary for such account).

There could also be another way to modify the SyncStateResponse which does not have any of these downsides.

cc @igamigo and @mFragaBA to see if they have any thoughts on what would be most useful for the client.

Impact on the Store component

To support the above functionality, we would need to store additional info in the store. Perhaps the simplest way to do this would be to add something like transactions table which would store (transaction_id, account_id, block_num) tuples where transaction_id would be the primary key and we'd also have an index set up on the block_num field.

We could do something more sophisticated (e.g., store a list of note IDs produced in each transaction) - but I'm not sure we need to do this. So, probably makes sense to start with something simple.

One open question is how would the store learn about the above relationship. To support this, we'll need to modify the Block struct in miden-base. I'll create a separate issue for this there.

@bobbinth bobbinth added enhancement New feature or request store Related to the store component rpc Related to the RPC component labels May 31, 2024
@bobbinth bobbinth added this to the v0.4 milestone May 31, 2024
@mFragaBA
Copy link
Contributor

cc @igamigo and @mFragaBA to see if they have any thoughts on what would be most useful for the client.

I think returning the transaction IDs is more than fine as the client already tracks for each transaction which account it modifies,which notes it consumes and which ones it produces.

@igamigo
Copy link
Collaborator

igamigo commented May 31, 2024

A downside of the first approach is that we don't explicitly return info about which transaction affected which account - but I'm not sure how important this info is (i.e., the client should have this info already).

For public accounts, the client might not have all transaction-related data. Although I'm not sure this would be interesting to clients anyway (ie, there might not be much value in knowing or storing Transaction IDs without the actual private transaction info), so I think this is a good first approach. Could we also not modify the sync response so that the AccountSummary is present if there is an account update or transactions included?

Is there any relevant impact to privacy in returning the exhaustive list of transaction IDs from an account considering anyone can know about it? I think it's marginal and I'm guessing you thought about this but just in case.

@bobbinth
Copy link
Contributor Author

Could we also not modify the sync response so that the AccountSummary is present if there is an account update or transactions included?

That's definitely an option. We'd basically wrap AccountSummary in another message which would consist of optional AccountSummary and a list of transaction IDs.

Is there any relevant impact to privacy in returning the exhaustive list of transaction IDs from an account considering anyone can know about it? I think it's marginal and I'm guessing you thought about this but just in case.

I don't think this should affect privacy in a meaningful way because we already assume that users will broadcast their proven transactions to the network and this ties account and transaction IDs. So, an entity monitoring the network would be able to build a list of transaction IDs for an account regardless of the changes discussed here.

@bobbinth
Copy link
Contributor Author

Closed by #377.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request rpc Related to the RPC component store Related to the store component
Projects
Status: Done
Development

No branches or pull requests

4 participants