refactor: add client specific rpc functions #616
Conversation
tomyrd
force-pushed
the
tomyrd-rpc-validations
branch
from
22803ea to
798ac92
Compare
tomyrd
force-pushed
the
tomyrd-rpc-validations
branch
from
bcbab4d to
92a8d0e
Compare
| // ================================================================================================ | ||
|
|
||
| /// Describes the possible responses from the `GetNotesById` endpoint for a single note. | ||
| #[allow(clippy::large_enum_variant)] | ||
| pub enum NoteDetails { | ||
| /// Details for a private note only include its [NoteMetadata] and [NoteInclusionDetails]. | ||
| pub enum NodeNote { |
There was a problem hiding this comment.
I think I'd prefer NetworkNote, what do you think?
crates/rust-client/src/rpc/mod.rs
Outdated
| account_ids: &BTreeSet<AccountId>, | ||
| code_commitments: &[Digest], | ||
| ) -> Result<(u32, Vec<FpiAccountData>), RpcError> { | ||
| let response = self.get_account_proofs(account_ids, code_commitments, true).await?; |
There was a problem hiding this comment.
nit: for readability, we could destructure the response here (let (block_num, account_proofs) =...)
|
Also, I believe this closes #511 so I will link it |
tomyrd
force-pushed
the
tomyrd-rpc-validations
branch
from
4bfcb4c to
8fd0e1c
Compare
CHANGELOG.md
Outdated
| @@ -19,6 +19,7 @@ | |||
|
|
|||
| ### Changes | |||
|
|
|||
| * Refactored rpc functions and structs to improve code quality (#616). | |||
There was a problem hiding this comment.
nit:
| * Refactored rpc functions and structs to improve code quality (#616). | |
| * Refactored RPC functions and structs to improve code quality (#616). |
crates/rust-client/src/rpc/mod.rs
Outdated
| /// Fetches the account data needed to perform a Foreign Procedure Invocation (FPI) on the | ||
| /// specified foreign accounts. The `code_commitments` parameter is a list of known code hashes | ||
| /// to prevent unnecessary data fetching. Returns the block number and the FPI account data. | ||
| /// | ||
| /// The default implementation of this method uses [NodeRpcClient::get_account_proofs]. |
There was a problem hiding this comment.
We should mention here what the behavior is for non-returned accounts. I believe the endpoint might return an error (but not sure).
crates/rust-client/src/rpc/mod.rs
Outdated
| /// The default implementation of this method uses [NodeRpcClient::get_account_update]. | ||
| async fn get_updated_public_accounts( | ||
| &mut self, | ||
| local_accounts: Vec<&AccountHeader>, |
There was a problem hiding this comment.
Could this be &[AccountHeader]? Or maybe an iterator so that it doesn't take ownership of the vector.
crates/rust-client/src/sync/mod.rs
Outdated
| @@ -567,30 +535,22 @@ impl<R: FeltRng> Client<R> { | |||
| account_updates: &[(AccountId, Digest)], | |||
| current_onchain_accounts: &[AccountHeader], | |||
There was a problem hiding this comment.
Not directly related to this PR, but could we rename onchain to public? Both in the parameter and function name, to reflect the newer terminology.
The problem is that version of the client that supports the new account ID is still in review. Once it gets merged these CI errors should be fixed. |
closes #573
This PR refactors the functions/structs of the
NodeRpcClient. It adds some functions that are better suited for the Miden Client and improves its usability. Some of the structs were also changed, specifically theNoteInclusionDetailswas removed and theNoteDetailswas renamed toNodeNote(I'm not sure about the name here) as we already had a different struct with the same name.Some of the new rpc functions may have too specific of a use. I'm open to removing/changing them if we feel like it would be better.