0xMiden / 0xMiden/node

Unused error variant

Open
#2,150 2 comments 0 reactions 0 assignees View on GitHub
rpc
Dominant language
Rust
Stars
104
Forks
138
Avg merge
1d 13h
Merged PRs (30d)
56

Description

`GetNoteScriptByRoot` currently reports a missing script by returning `Ok(MaybeNoteScript { script: None })` ([link](https://github.com/0xMiden/node/blob/143cf7fe8511b55f28b32038b72dbf903f975663/crates/store/src/server/rpc_api.rs#L358-L376)). However, `GetNoteScriptByRootError::ScriptNotFound` is still declared with error code `2`:

https://github.com/0xMiden/node/blob/143cf7fe8511b55f28b32038b72dbf903f975663/crates/store/src/errors.rs#L460

As far as I can tell, that variant is never constructed anywhere in the codebase. This is a bit inconsistent with other lookup endpoints in the same crate. For example, `get_account` reports missing accounts via a typed `GetAccountError::AccountNotFound` error; though here it might make sense since not finding a script is not necessarily an error state.

We hit this in the client because we were parsing the error but instead we always got a response. That broke a flow that probes the registry before deciding whether to register a script.
Not sure there's much to do, but we probably want one of the following:

- keep the current `Ok(None)` / `MaybeNoteScript` behavior, and remove the unused `ScriptNotFound` variant;
- return `ScriptNotFound` when the script is missing, matching endpoints like `get_account` and removing the `Option<>` out of the response type

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.