Notes don't get a commit height

[Dominik1999]

Sometimes the notes don't get commit height even if we don't do a sync first, it doesn't happen consistently (e.g., i created 2 notes and it worked fine). But we just did this from scratch on a different machine, and even if all steps were followed properly, the notes don't get commit height.

It might be related to this explanation:

If you did a sync before importing the node, you might bring the client to the chain tip (which could have passed the note's inclusion). If you then import the note and sync, the origin would not be built because you are past the point of inclusion (the node filters to only include notes after the request's block number).

[igamigo]

I spent some time trying to reproduce this and was not able to. I tried the script used in the workshops and different permutations of commands, using both the remote node and a local one.
When not passing the chain tip before importing the note (this is a separate issue), I have not been able to not get anchor information on the note.

Without a way to reproduce it's hard to investigate from the client.

If there are still clients that we have with note IDs that did not get inclusion data on a sync, I can think of a couple of ways of moving forward with the investigation:

• Using the node store's ListNotes endpoint to see if the note was included properly. This is not currently accessible on the remote node (from remote networks at least).
  • Also investigating what tag it has and why it did not match on sync requests
• Manually moving back the client's chain information in order to reproduce and understand why the data is not retrieved.

[Dominik1999]

Got it, I suggest we keep the issue open for a while and see if it happens again. If we can't reproduce it anymore, the bug might be gone.

[igamigo]

So it seems that while attempting to make integration tests concurrent as part of #215, @mFragaBA might have stumbled upon this issue. The problem is on the node when creating more than one note per block. Here is the error that is logged:

2024-03-11T20:53:52.445068Z ERROR store:apply_block:apply_block: miden-store: store/src/state.rs:102: error: Failed to create notes tree: multiple values provided for key 1

And the culprit code:

    for note in notes.iter() {
        let note_metadata = NoteMetadata::new(
            note.sender.try_into()?,
            note.tag
                .try_into()
                .expect("tag value is greater than or equal to the field modulus"),
        );
        let index = note.note_index as u64;
        entries.push((index, note.note_id.into()));
        entries.push((index + 1, note_metadata.into()));
    }

When there is more than one note, index 0 and index 1 of the tree are occupied by the first note and its metadata respectively, but then the second note occupies index 1 as well, which results in the above error. As a consequence of this, the block is built without the correct tree and after syncing there is no new note data (although blocks continue to be built)

It feels like this is the fix to the error, but this might have side implications (it doesn't seem like it after a few quick tests) cc @hackaugusto @bobbinth @polydez. As an additional problem the node might have to handle node tree creation failures (and anything similar) differently since it does seem that the transactions get included anyway (could be wrong about this).

[hackaugusto]

I added a comment on the commit: 0xPolygonMiden/miden-node@3c202e7

[hackaugusto]

I created an issue in the node to tackle the bigger problem: 0xPolygonMiden/miden-node#276

[Dominik1999]

Is this actively being worked on again?

[igamigo]

Is this actively being worked on again?

I'm pretty sure this is the problem you were alluding to when creating this issue. I think the code I proposed fixes it but according to 0xPolygonMiden/miden-node#276 it seems it might be solved as part of the bigger refactor. In any case it's an issue that is to be fixed on the node and so I believe we could close this now (or keep it linked to issue 276 on the node to track advancements).

[Dominik1999]

Then, let's close it, since it is an issue in the Miden node