Skip to content

NoteInputs's trial unhashing produces incorrect results #2065

@igamigo

Description

@igamigo

In the client (0xMiden/miden-client#1454), a couple of tests were failing with some checks related to the NoteScreener. Specifically, P2IDE inputs were not matching. When looking at why, we saw the note inputs length for the P2IDE note was 3. This had happened before so we looked at the recent changes regarding note inputs and found the following:

  • The values stored in the advice map are inputs.to_elements(), which pads the note inputs to the next multiple of WORD_SIZE * 2. For a P2IDE note, that's 8.
  • In this test, the code was first creating a P2IDE note with reclaim height but no timelock, the inputs of which would be [suffix, prefix, reclaim_height, 0], padding produces [suffix, prefix, reclaim_height, 0, 0, 0, 0, 0].
  • When the note is reconstructed, read_note_inputs_from_adv_map looks at that padded vector and picks the last non-zero entry to guess the original length. Because the 4th element is zero, the “last non-zero” is the third element, so it tries [suffix, prefix, reclaim_height].
  • NoteInputs::new pads that 3‑element list to eight elements before hashing, resulting in [suffix, prefix, reclaim_height, 0, 0, 0, 0, 0] again. The commitment therefore matches and the shorter vector is accepted.

Unless I'm missing something, this approach of unhashing would not work unless we were able to persist the non-padded inputs commitment all the way through to an output note that another user can use as an input note.

Related to the above, I think if we had had a test like the one we outlined in #1363 this might have been caught here; although it depends on the last note input(s) being purposefully 0. We caught it because we left some static checks on the NoteScreener (that we were plannign to remove with the changes done on the NoteConsumptionChecker).

Curious about why the previous approach of passing the length as the first element was not enough. Maybe we can roll back to that version until a more permanent solution lands.

Metadata

Metadata

Labels

No labels
No labels

Type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions