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

Usability: Add an easy way for a user to retrieve an inclusion proof after having synced over its block #256

Closed
hackaugusto opened this issue Feb 29, 2024 · 10 comments
Assignees
Milestone

Comments

@hackaugusto
Copy link
Contributor

Given the following scenario:

  • Note N is created and included in block X
  • Current block height is X+N
  • User syncs with a block higher than X
  • User learns about note N and wants to consume it

The sync endpoint, as it is, doesn't cover the situation above. So a user may never see the inclusion proof for the block N, and therefor never be able to consume it.

@hackaugusto
Copy link
Contributor Author

hackaugusto commented Feb 29, 2024

We have some workarounds for the issue above:

  • Responsibility to the sender: The note creator can send the inclusion proof to the user.
    • Tradeoff/Notes: easiest to implement, at the cost of increased latency. The sender now needs to wait for its transaction confirmation
  • Responsibility to the receiver: The receiver calls sync for the new note alone starting at the genesis.
    • Tradeoff/Notes: This needs handling for epochs, so the genesis can't always be used. The cost to the receiver will increase depending on how many tag collisions there are. It will be a bit annoying getting this to align with block height for the current sync request
    • One optimization here is for the sender to give the lower bound to use, so the receiver can start closer to the actual note creation.
  • Responsibility to the server: Change the semantics of sync and move the responsibility to the server to look in the past.
    • Tradeoff/Notes: Should be the best latency wise, since the sender doesn't have to wait. But has the same issue of increasing the sync cost as the one above.
  • Responsibility shared with sender & server: We could change the sync endpoint to have a block height per tag, instead of a block height per request. And the sender would give the lower bound to the receiver.
    • Removes the block height / sync alignment from the client. Since one single request can handle both circumstances. Reduce the data cost of downloading collisions by reducing the block range

@bobbinth
Copy link
Contributor

bobbinth commented Mar 3, 2024

Responsibility to the sender: The note creator can send the inclusion proof to the user.

I think this may be independently interesting as this may remove the source of ambiguity on whether the transactions is already included or not. I think our note serialization format should be able to handle notes of different types - specifically:

  • A note without inclusion path (same as now).
  • A note with inclusion path (and here, this could be just the path from the note root of a block header, and the client would be responsible for fetching the block header from the network).

On the negative side, in additional to extra latency, it does introduce a potential attack vector where a malicious sender could send an invalid inclusion path - so, the client would need to handle such situations.

Responsibility shared with sender & server:

I think one other approach that falls into this category could be to add a new endpoint to the node - something like get_notes_by_id. This endpoint would accept a list note IDs and return note data together with note inclusion paths. This endpoint could also be used for retrieving note details which we'll need for public notes.

The main downside of this approach is reduced privacy - i.e., the node may learn additional info about the notes the client is interested in.

@hackaugusto
Copy link
Contributor Author

On the negative side, in additional to extra latency, it does introduce a potential attack vector where a malicious sender could send an invalid inclusion path - so, the client would need to handle such situations.

I was thinking the authentication path would be anchored in the MMR. That is to say, not only the path from the note to the block, but also the path from the block to the current MMR. There are MMR race conditions the client needs to handle, but the sender would not be allowed to lie.

I think one other approach that falls into this category could be to add a new endpoint to the node - something like get_notes_by_id. This endpoint would accept a list note IDs and return note data together with note inclusion paths. This endpoint could also be used for retrieving note details which we'll need for public notes.

Agree, that is what I tried to propose here

@igamigo
Copy link
Collaborator

igamigo commented Mar 4, 2024

I think this may be independently interesting as this may remove the source of ambiguity on whether the transactions is already included or not. I think our note serialization format should be able to handle notes of different types - specifically:

  • A note without inclusion path (same as now).
  • A note with inclusion path (and here, this could be just the path from the note root of a block header, and the client would be responsible for fetching the block header from the network).
    On the negative side, in additional to extra latency, it does introduce a potential attack vector where a malicious sender could send an invalid inclusion path - so, the client would need to handle such situations.

This is supported from the client (eventually this struct should be moved to miden-base as has been discussed). Although there is no current way to retrieve a block header on demand to build the reset of the required proof.

The main downside of this approach is reduced privacy - i.e., the node may learn additional info about the notes the client is interested in.

Is this the case? It seems there is no more information than when getting information with a sync.

@hackaugusto
Copy link
Contributor Author

Is this the case? It seems there is no more information than when getting information with a sync.

I think the description here was to return only the data for an specific note. Whereas the sync returns data for any note matching a given prefix. IMO we should make this a bit more flexible, as in the user can choose how much data she receives by having a variable length prefix, so privacy would be preserved, and we would still have the possibility of a fast path for things like on-chain notes.

@bobbinth
Copy link
Contributor

bobbinth commented Mar 5, 2024

This is supported from the client (eventually this struct should be moved to miden-base as has been discussed). Although there is no current way to retrieve a block header on demand to build the reset of the required proof.

I'll think we'll need to add an endpoint to miden node to retrieve a block header + the inclusion path for that block against a given MMR forest.

Is this the case? It seems there is no more information than when getting information with a sync.

I think the description here was to return only the data for an specific note. Whereas the sync returns data for any note matching a given prefix. IMO we should make this a bit more flexible, as in the user can choose how much data she receives by having a variable length prefix, so privacy would be preserved, and we would still have the possibility of a fast path for things like on-chain notes.

Generally, I agree - but I would probably start with the simpler endpoint which works with Node IDs, and then add a privacy-preserving one later.

@bobbinth bobbinth added this to the v0.3 milestone Apr 12, 2024
@bobbinth
Copy link
Contributor

We already have a part of this issue done - i.e., we have GetNotesById endpoint. What is missing is the ability to retrieve an MMR authentication for an arbitrary block. I think we have 2 options here:

  1. Modify the existing GetBlockHeaderByNumber endpoint to also return the authentication path.
  2. Add a new endpoint - something like GetBlockHeaderWithAuthPath

In either case, we could return the authentication path against the latest MMR state, and the client will be able to truncate it accordingly.

@igamigo
Copy link
Collaborator

igamigo commented May 1, 2024

In terms of what a user would need in order to be able to use a note for which it has not received sync-related data, it seems that adding auth path to the GetBlockHeaderByNumber should be enough and the simpler option.

However, there will likely be other use-cases where it would be unnecessary to return the MMR data alongside the block header data (explorer?), so it feels like adding a new endpoint would be a better way to go. Another more mixed approach could be to add a flag to the GetBlockHeaderByNumber request that defines whether the response contains MMR data or not.

@bobbinth
Copy link
Contributor

bobbinth commented May 3, 2024

I'm leaning toward using a single GetBlockHeaderByNumber endpoint where by default we return both the block header and the MMR path. We can also add an option to "omit" the MMR path - but this can also be done later. The assumptions are:

  • Adding MMR path to the response should not impose too much of a penalty as the MMR is already in memory and so, getting relevant data from it would be super quick.
  • It's not going to increase the response size too much. I'm thinking this will add 2KB - 4KB to the response.
  • Getting a block header by itself, is not a super useful. For entities that need to build up a complete view of the chain, we have GetBlockByNumber which returns the full block.
  • In case someone wants to download a bunch of block headers without the paths, we can add the option to omit the paths from the responses.

But I'm also fine with not returning the path by default and use an additional parameter to specify whether the path should be included.

@bobbinth
Copy link
Contributor

bobbinth commented May 8, 2024

Closed by #345.

@bobbinth bobbinth closed this as completed May 8, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in User's testnet May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

3 participants