fix: handle missing nullifiers in nonce discovery

[nchamo]

Summary

• compute_note_hash_and_nullifier now returns Option instead of panicking when note hash computation fails (e.g. unknown note type)
• Notes with matching hashes but missing nullifiers (e.g. unavailable nullifier secret key) are skipped with a warning log instead of panicking

Closes #11157
Closes F-344

[nventuro]

This will have conflicts with #21438. Sorry about that.

[nventuro]

It's not the computation that fails, it's that the contract may not know what to do. Examples include it not recognizing the note_type_id, or the packed_note not having the correct length.

We should log a warning mentioning that we attempted to process note data during discovery but failed to compute its hash.

[nventuro]

This TODO should go on the else

[benesjan]

Nico's comments are not addressed yet and there are conflicts so not reviewing again for now.

[nchamo]

Like @benesjan mentioned before, we were emitting warnings inside the loop. The problem is that Option::none could be returned for reasons unrelated to the nonce, so we would end up logging the warning for every nonce. So I moved this here

[nventuro]

Hm, not sure. This is the second time we run into this - might as well do it properly if we're going to be doing so much error handling.

We could have a local implementation of Result (Noir doesn't have it yet), which is the same as Option except the none case has a type:

struct Result<T, E> {
  value: T;
  error: E;
  is_ok: bool;
}

impl<T, E> Result<T, E> {
   fn ok(value: T) -> Self {
       Self { value, error: std::mem::zeroed(), is_ok: true }
   }

   fn err(error: E) -> Self {
       Self { value: std::mem::zeroed(), error,  is_ok: false }
   }
  
   fn is_ok(self) -> bool {
     ..
   }

   fn is_err(self) -> bool {
     ..
   }
  
    fn and_then(self) -> bool {
     ..
   }
}

See https://doc.rust-lang.org/std/result/ and https://doc.rust-lang.org/std/result/enum.Result.html.

wdyt?

[benesjan]

I like it but seems like something that should be in noir::std. Shall we ping Noir people if they are willing to add it?

@nchamo once this discussion gets resolved feel free to re-request review from me (just trying to keep my review work clean by un-requesting when it doesn't make sense for me to imminently review). Thanks

[nventuro]

#21639 now accidentally also overlaps with this.

[nchamo]

@nventuro , closing this since it will be handled in #21639