Import and Sync notes

[tomyrd]

The client's import and sync functions changed considerably in the last weeks, the idea of this post is to document and discuss the current client behavior in each case. We can use this as a base point to see if this is the desired state or if we want to make some changes.

Current behavior

Import

The import behavior depends mainly on the NoteFile variant being imported.

NoteFile::NoteId(id)

The client will use the GetNotesById endpoint to request information about the note the note with the given ID. If the note is public and is tracked by the node, the client will import it, otherwise it will return an error.
If the note imported is already tracked by the client, the client will update the note's inclusion proof and metadata.
The client then uses the CheckNullifiers endpoint to determine if the note is consumed or not. If the note is consumed, the client will import it as Consumed, otherwise it will be imported as Committed along with its inclusion proof and metadata.
If the note is imported as Committed and its in the past relative to the client, the block headers and MMR data for the note is also stored.

NoteFile::NoteWithProof(note, inclusion_proof)

The client then uses the CheckNullifiers endpoint to determine if the note is consumed or not. If the note is consumed, the client will import the note as Consumed, otherwise it will be imported as Committed along with its inclusion proof and metadata.
If the note is imported as Committed and its in the past relative to the client, the block headers and MMR data for the note is also stored.

NoteFile::NoteDetails{details, after_block_num, tag}

The imported note will be marked as ignored if the tag is None or if it's Some(tag) and the tag is not currently tracked by the client (ie, it's stored as a persistent tag).
The import process uses the SyncNotes endpoint to determine was committed in the past relative to the client's current block number. If the note is tracked by the node and it was committed in the past, the note is imported as Committed. If this doesn't happen or if the tag is None, the note is imported as Expected.
If the note is imported as Committed, its inclusion proof and metadata is also stored, with the addition of the block headers and MMR data for the block in which the note was committed.

Sync

The way a note is updated on sync depends on two factors:

1. The note's status (expected, committed or consumed).
2. Whether the note is ignored or not.

Expected + Tracked Notes

The tags used in the sync request to update expected notes are:

• account tags: built from the account ids tracked by the client.
• stored tags: stored in the client's database, manually added by the user.
• expected note tags: tags from the expected notes tracked by the client that have metadata and aren't ignored.
• imported tags: tags from the notes that were imported and aren't ignored. These tags may be unnecessary.
  The node returns a list of note inclusions which are notes tracked by the node that match the provided tags. Expected notes that are included in this list are turned into committed notes without changing its ignored status. The block headers and MMR changes for these notes are also added to the client.

Expected + Ignored Notes

As mentioned, only tags from non-ignored notes are added to the sync request. This means that ignored notes are not returned in the note inclusions list and will not be updated through the sync process.
The client offers a separate update_ignored_notes method that uses the GetNotesById node endpoint to specifically request information about ignored notes. Returned notes are known to be tracked by the node and can be changed from expected to committed. The client also updates the note's inclusion proof and metadata.
The update_ignored_notes method is not called during the sync process, it must be called manually by the user.

Committed/Processing + Tracked Notes

The client needs to know which committed notes have been consumed. On every sync, a list of nullifiers is sent. The requested nullifiers are:

• committed note nullifiers: nullifiers from the committed notes tracked by the client aren't ignored.
• processing note nullifiers: nullifiers from the processing notes tracked by the client aren't ignored.
  The node returns a list of nullifiers that have been recorded. This means that notes that match these nullifiers need to be updated to consumed.
  Additionally, after every sync, the client will request the block headers and MMR data for all committed notes that don't have them tracked.

Processing + Tracked Notes

Just like committed notes, these notes will be updated to consumed if they are included in the list of nullifiers returned by the node.

Processing + Ignored Notes

This case is currently not handled.

Consumed Notes

Once a note is set as consumed it is no longer updated.

[bobbinth]

This is great! Thank you for putting it together!

I think this is kind of related to #489 in that we need to understand what states a note can be in and which actions can move a note from one state to another.

I'm trying to think about this as if it was a finite-state machine. For example, Let's say we get a nullifier for a note during state sync. The action here would be "note nullified" (or something like that) and then almost any state we apply this action to would mean that the note moves into "consumed" state (unless it is already in the "consumed" state, and then we get an error?).

[tomyrd]

I've made a couple of diagrams. They are not exactly finite-state machines as they don't show all the combinations of state+action to make it more readable. If the transition is not shown it's because it probably doesn't make sense, it would throw an error or do nothing (like trying to "locally consume" a "consumed" note).

Only note status

This is a simplified diagram, only showing the transitions between the possible NoteStatus

With "ignored" flag

This version adds the states for ignored notes. It's harder to follow, the number of states is almost double as now each one has an "ignored" variant (except for the consumed notes where it doesn't matter if they are ignored).

While doing this version I found a couple of missing transitions that should be added.

[bobbinth]

Thinking more about it, maybe using NoteRecord wrapper approach could work well here:

pub struct InputNoteRecord {
    state: NoteState,
    created_at: u64,
}

One other thought, maybe it does make sense to split the NoteDetails out - e.g., something like:

pub struct InputNoteRecord {
    details: NoteDetails,
    state: NoteState,
    created_at: u64,
}

This will work fine for most variants except for the Processing variant where splitting the InputNote may be a bit tricky.

But the main advantage of this is that we can actually store details as multiple fields in the database. This gives us flexibility to, for example, have a separate table for note scripts so that we don't have to duplicate note scripts for every note.

But maybe I'm overthinking this.

[tomyrd]

Do we also need to keep track of the note tag here? I guess this depends on how we refactor note tag handling.

I think so, the tag was stored in the metadata before so we need a way to add it to the tag list on sync.

Do we not need to track the transaction ID of the transaction which is consuming the note?

Yes, we stored it before but we didn't add it to the InputNoteRecord. I can add it to return some extra information that may be useful (for cancelled transactions for example).

One other thought, maybe it does make sense to split the NoteDetails out

I don't think this would be too difficult to implement, it's how we do it now, we store the details decomposed and build it on every query.

[bobbinth]

One other thought, maybe it does make sense to split the NoteDetails out

I don't think this would be too difficult to implement, it's how we do it now, we store the details decomposed and build it on every query.

I wonder if it would make sense to split Processing into two variants: ProcessingAuthenticated and ProcessingUnauthenticated (or maybe some other names but with the similar meaning). This way, we could probably include only relevant fields into each.

[tomyrd]

Something it's not clear to me (at least at first sight) is how these changes impact on output notes. I feel like all of these states only make sense for input notes. We could take this oportunity to better differentiate between InputNoteRecord and OutputNoteRecord. The question that comes to mind here is, do we need to track the state of output notes? Maybe we can have a more simplifed approach like:

pub struct OutputNoteRecord {
    note: Note,
    inclusion_proof: Option<NoteInclusionProof>,
}

This let's us mantain the logic for the 3 export types. Here the option acts as a binary "state", whether we know it's committed or not.

One possible problem here is that we can't represent case 2a from this comment.

[bobbinth]

I was thinking we'd handle output notes separately. They should be simpler than input notes, but probably not as simple as what you have above. For example:

1. We do need to keep track of cases 2a and 2b. Maybe we could get rid of 2c - but not sure.
2. We may also need to keep track of different states - though here there should be just a few of them: (1) processing, (2) committed, and (3) consumed.

To summarize: I think it would probably make sense to tackle them as a separate issue.

[tomyrd]

Once #520 gets merged we should continue working on how output notes will be reworked. Will they use the same state as input notes? If this is the case should we add separate transitions for output notes?

Some other options I can think of:

• Something mentioned in the discussion above is to have a separate state enum for output notes. This would mean that we would have some duplicate states which wouldn't be ideal.
• If the logic for output notes is simpler we could forgo the state/transitions and use a struct like:

  pub struct OutputNoteRecord {
      header: NoteHeader,
      inclusion_proof: Option<NoteInclusionProof>,
      details: Option<NoteDetails>,
  }
  

  Unfortunately this would mean that it would be harder to extend (like what happened with InputNoteRecord before the rework).

[tomyrd]

I've been thinking about the best way to solve this problem. If we want to mantain consistency and also treat the output note records as state machine we should use the same approach as the one used for the input note records. The problem with this is that we wouldn't be able to reuse the states/transitions from the input note records because the output note records have different fields and rules. In this case we would probably need a separate state machine for the output note records, we could have a module structure like this:

/note_record
  /input_note_record
    - mod.rs
    /states
      - ...
  /output_note_record
    - mod.rs
    /states
      - ...
  - mod.rs

Both would work similarly but would have their own InputNoteState and OutputNoteState with their own states and transitions. This would allow for more extensibility for both but also creates a lot of similar code which is not ideal.

If we expect both to grow in complexity this would be the best approach. On the other hand, if we don't expect the output notes to change much, maybe another state machine for them is overkill and a simpler approach would be better.

cc @igamigo @bobbinth let me know what do you think, I can build a PR for the first approach and discard it if we think is too much

[tomyrd]

For output notes we wouldn't need to check if they are being processed or if they were consumed internally or externally, we only use them for exporting.

For the first approach I think some possible states and transitions would be:

• States:

  • Expected: Would contain an after_block_num (needed for exporting) and an optional NoteDetails (this optional field could be splitted into separate states if needed).
  • Committed: Would contain an InclusionProof and an optional NoteDetails.
  • Consumed: Would contain an InclusionProof and an optional NoteDetails along with a nullifier_height.

• I think the transitions would be:

  • note_details_received: This would be a new transition that only changed the state from not having a NoteDetails to having one.
  • inclusion_proof_received: Would work the same as the one for the input note records.
  • nullifier_received: Would work the same as the one for the input note records.

For the second, simpler approach we could have the optional values as fields in the struct:

pub struct OutputNoteRecord {
    id: NoteId,
    assets: NoteAssets,
    recipient: Digest,
    metadata: NoteMetadata,
    after_block_num: u32,
    details: Option<NoteDetails>,
    inclusion_proof: Option<NoteInclusionProof>,
    nullifier_height: Option<u32>,
}

We could have the transitions mentioned above as methods for the struct so as not to allow for invalid states. This would be simpler but would not be as extensible as the first approach (maybe this is a requirement for output notes). Even in this approach we would be removing the old NoteStatus which was mostly used for input notes.

[bobbinth]

note_details_received: This would be a new transition that only changed the state from not having a NoteDetails to having one.

When would this happen? I thought that we either get the details right a way or not at all - but maybe I am forgetting something.

For the second, simpler approach we could have the optional values as fields in the struct:

pub struct OutputNoteRecord {
    id: NoteId,
    assets: NoteAssets,
    recipient: Digest,
    metadata: NoteMetadata,
    after_block_num: u32,
    details: Option<NoteDetails>,
    inclusion_proof: Option<NoteInclusionProof>,
    nullifier_height: Option<u32>,
}

We could have the transitions mentioned above as methods for the struct so as not to allow for invalid states. This would be simpler but would not be as extensible as the first approach (maybe this is a requirement for output notes). Even in this approach we would be removing the old NoteStatus which was mostly used for input notes.

This doesn't seems as clean as the enum-based approach. For example, assets/recipient fields would be redundant when details is present.

[tomyrd]

When would this happen? I thought that we either get the details right a way or not at all - but maybe I am forgetting something.

Yes! That's something I realized when I started developing these changes. The partial/full note states don't mix so we wouldn't need this transition

I ended up going with the enum approach with a similar note record interface as the input notes but with a more simple implenetation (without a state machine design).