feat: add GetNoteError endpoint in ntx builder#1792
feat: add GetNoteError endpoint in ntx builder#1792SantiagoPittella wants to merge 3 commits intonextfrom
Conversation
crates/ntx-builder/src/actor/mod.rs
Outdated
| per_note | ||
| .into_iter() | ||
| .map(|f| { | ||
| let note_error = f.error.as_report(); | ||
| tracing::info!( | ||
| note.id = %f.note.id(), | ||
| nullifier = %f.note.nullifier(), | ||
| err = %note_error, | ||
| "note failed: consumability check", | ||
| ); | ||
| (f.note.nullifier(), note_error) | ||
| }) | ||
| .collect() |
There was a problem hiding this comment.
nit: might be worth deduping this and its use above. Maybe not though
There was a problem hiding this comment.
The match statement is pretty long in general. If we could reduce it that would be good.
There was a problem hiding this comment.
Addressed the dedup in 25aee9d
The match is still present, but it doesn't look as large now, we can still try to reduce it further tho.
crates/ntx-builder/src/actor/mod.rs
Outdated
| async fn mark_notes_failed(&self, nullifiers: &[Nullifier], block_num: BlockNumber) { | ||
| async fn mark_notes_failed( | ||
| &self, | ||
| failed_notes: &[(Nullifier, String)], |
There was a problem hiding this comment.
Would maybe be nice to replace String with Box<dyn ErrorReport> but the problem is that one of the branches doesn't provide an Error: (note.nullifier(), error_msg.clone()).
There was a problem hiding this comment.
It's doable, but instead of using Box, we need to wrap with Arc, and also restrain to Sync + Send, creating the following type:
type NoteError = Arc<dyn ErrorReport + Send + Sync>Which may be an overcomplicated solution just to get rid of the String, so we can roll it back if prefered.
| // | ||
| // This is useful for debugging notes that are failing to be consumed by | ||
| // the network transaction builder. | ||
| rpc GetNoteError(note.NoteId) returns (GetNoteErrorResponse) {} |
There was a problem hiding this comment.
Its starting to feel like we are implementing a manual gateway/reverse proxy. But maybe thats not a bad thing.
There was a problem hiding this comment.
Do you mean the RPC proxy into the "internal" components? It is essentially a gateway yeah.. If this was non gRPC we could probably use some off the shelf implementation for this, though maybe not, because there are some non-trivial validation steps happening.
docs/internal/src/ntx-builder.md
Outdated
|
|
||
| ## gRPC Server | ||
|
|
||
| The NTB exposes an internal gRPC server for querying its state. The RPC component proxies public |
There was a problem hiding this comment.
nit: NTX Builder, not NTB
|
With Santiago being out of office for the duration of this week, I'm taking over any remaining work he has on the node. |
4 participants
closes #1758