From 0910c0a231b4e0ef48b6cf0b9d7d48a44cc557a7 Mon Sep 17 00:00:00 2001 From: hinto-janai Date: Fri, 12 Jul 2024 20:26:11 -0400 Subject: [PATCH] rpc: use `ByteArrayVec` and `ContainerAsBlob` (#227) * fixed-bytes: add `serde`, document feature flags * fixed-bytes: add derives * rpc: add `as _` syntax to macro * rpc: use `ByteArrayVec` and `ContainerAsBlob` for binary types * fixed-bytes: re-add derives * rpc-types: dedup default value within macro * readme: fixed bytes section --- Cargo.lock | 1 + net/fixed-bytes/Cargo.toml | 2 +- net/fixed-bytes/src/lib.rs | 5 ++- rpc/types/Cargo.toml | 3 +- rpc/types/README.md | 14 +++++++ rpc/types/src/bin.rs | 43 +++++++++++++------- rpc/types/src/json.rs | 80 ++++++++++++++++++++++---------------- rpc/types/src/macros.rs | 47 ++++++++++++++++------ rpc/types/src/other.rs | 39 +++++++------------ 9 files changed, 146 insertions(+), 88 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fbb68b06..01d5329d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -762,6 +762,7 @@ name = "cuprate-rpc-types" version = "0.0.0" dependencies = [ "cuprate-epee-encoding", + "cuprate-fixed-bytes", "monero-serai", "paste", "serde", diff --git a/net/fixed-bytes/Cargo.toml b/net/fixed-bytes/Cargo.toml index e9985e81..4c5a1afb 100644 --- a/net/fixed-bytes/Cargo.toml +++ b/net/fixed-bytes/Cargo.toml @@ -16,4 +16,4 @@ bytes = { workspace = true } serde = { workspace = true, features = ["derive"], optional = true } [dev-dependencies] -serde_json = { workspace = true, features = ["std"] } \ No newline at end of file +serde_json = { workspace = true, features = ["std"] } diff --git a/net/fixed-bytes/src/lib.rs b/net/fixed-bytes/src/lib.rs index 370b8817..2e8f1bc5 100644 --- a/net/fixed-bytes/src/lib.rs +++ b/net/fixed-bytes/src/lib.rs @@ -12,6 +12,7 @@ use serde::{Deserialize, Deserializer, Serialize}; #[cfg_attr(feature = "std", derive(thiserror::Error))] #[cfg_attr(feature = "serde", derive(Deserialize, Serialize))] +#[derive(Clone, Eq, PartialEq, PartialOrd, Ord, Hash)] pub enum FixedByteError { #[cfg_attr( feature = "std", @@ -48,7 +49,7 @@ impl Debug for FixedByteError { /// /// Internally this is just a wrapper around [`Bytes`], with the constructors checking that the length is equal to `N`. /// This implements [`Deref`] with the target being `[u8; N]`. -#[derive(Debug, Clone, Eq, PartialEq)] +#[derive(Debug, Default, Clone, Eq, PartialEq, PartialOrd, Ord, Hash)] #[cfg_attr(feature = "serde", derive(Serialize))] #[cfg_attr(feature = "serde", serde(transparent))] #[repr(transparent)] @@ -115,7 +116,7 @@ impl TryFrom> for ByteArray { } } -#[derive(Debug, Clone, Eq, PartialEq)] +#[derive(Debug, Default, Clone, Eq, PartialEq, PartialOrd, Ord, Hash)] #[cfg_attr(feature = "serde", derive(Serialize))] #[cfg_attr(feature = "serde", serde(transparent))] #[repr(transparent)] diff --git a/rpc/types/Cargo.toml b/rpc/types/Cargo.toml index c088e4df..1176526a 100644 --- a/rpc/types/Cargo.toml +++ b/rpc/types/Cargo.toml @@ -10,11 +10,12 @@ keywords = ["cuprate", "rpc", "types", "monero"] [features] default = ["serde", "epee"] -serde = ["dep:serde"] +serde = ["dep:serde", "cuprate-fixed-bytes/serde"] epee = ["dep:cuprate-epee-encoding"] [dependencies] cuprate-epee-encoding = { path = "../../net/epee-encoding", optional = true } +cuprate-fixed-bytes = { path = "../../net/fixed-bytes" } monero-serai = { workspace = true } paste = { workspace = true } diff --git a/rpc/types/README.md b/rpc/types/README.md index 21905fab..eb8da013 100644 --- a/rpc/types/README.md +++ b/rpc/types/README.md @@ -64,6 +64,20 @@ These mixed types are: TODO: we need to figure out a type that (de)serializes correctly, `String` errors with `serde_json` +# Fixed byte containers +TODO + + + # Feature flags List of feature flags for `cuprate-rpc-types`. diff --git a/rpc/types/src/bin.rs b/rpc/types/src/bin.rs index 02be1938..3dcfb967 100644 --- a/rpc/types/src/bin.rs +++ b/rpc/types/src/bin.rs @@ -3,6 +3,11 @@ //! All types are originally defined in [`rpc/core_rpc_server_commands_defs.h`](https://github.com/monero-project/monero/blob/cc73fe71162d564ffda8e549b79a350bca53c454/src/rpc/core_rpc_server_commands_defs.h). //---------------------------------------------------------------------------------------------------- Import +use cuprate_fixed_bytes::ByteArrayVec; + +#[cfg(feature = "epee")] +use cuprate_epee_encoding::container_as_blob::ContainerAsBlob; + use crate::{ base::{AccessResponseBase, ResponseBase}, defaults::{default_false, default_height, default_string, default_vec, default_zero}, @@ -22,16 +27,13 @@ define_request_and_response! { core_rpc_server_commands_defs.h => 162..=262, GetBlocks, Request { - #[cfg_attr(feature = "serde", serde(default = "default_zero"))] - requested_info: u8 = default_zero(), - // TODO: This is a `std::list` in `monerod` because...? - block_ids: Vec<[u8; 32]>, + requested_info: u8 = default_zero(), "default_zero", + // FIXME: This is a `std::list` in `monerod` because...? + block_ids: ByteArrayVec<32>, start_height: u64, prune: bool, - #[cfg_attr(feature = "serde", serde(default = "default_false"))] - no_miner_tx: bool = default_false(), - #[cfg_attr(feature = "serde", serde(default = "default_zero"))] - pool_info_since: u64 = default_zero(), + no_miner_tx: bool = default_false(), "default_false", + pool_info_since: u64 = default_zero(), "default_zero", }, // TODO: this has custom epee (de)serialization. // @@ -67,16 +69,17 @@ define_request_and_response! { core_rpc_server_commands_defs.h => 309..=338, GetHashes, Request { - block_ids: Vec<[u8; 32]>, + block_ids: ByteArrayVec<32>, start_height: u64, }, AccessResponseBase { - m_blocks_ids: Vec<[u8; 32]>, + m_blocks_ids: ByteArrayVec<32>, start_height: u64, current_height: u64, } } +#[cfg(not(feature = "epee"))] define_request_and_response! { get_o_indexesbin, cc73fe71162d564ffda8e549b79a350bca53c454 => @@ -91,6 +94,21 @@ define_request_and_response! { } } +#[cfg(feature = "epee")] +define_request_and_response! { + get_o_indexesbin, + cc73fe71162d564ffda8e549b79a350bca53c454 => + core_rpc_server_commands_defs.h => 487..=510, + GetOutputIndexes, + #[derive(Copy)] + Request { + txid: [u8; 32], + }, + AccessResponseBase { + o_indexes: Vec as ContainerAsBlob, + } +} + define_request_and_response! { get_outsbin, cc73fe71162d564ffda8e549b79a350bca53c454 => @@ -98,8 +116,7 @@ define_request_and_response! { GetOuts, Request { outputs: Vec, - #[cfg_attr(feature = "serde", serde(default = "default_false"))] - get_txid: bool = default_false(), + get_txid: bool = default_false(), "default_false", }, AccessResponseBase { outs: Vec, @@ -113,7 +130,7 @@ define_request_and_response! { GetTransactionPoolHashes, Request {}, AccessResponseBase { - tx_hashes: Vec<[u8; 32]>, + tx_hashes: ByteArrayVec<32>, } } diff --git a/rpc/types/src/json.rs b/rpc/types/src/json.rs index b5b53c9a..2e7aa82a 100644 --- a/rpc/types/src/json.rs +++ b/rpc/types/src/json.rs @@ -49,8 +49,33 @@ define_request_and_response! { // $FIELD_NAME: $FIELD_TYPE, // ``` // The struct generated and all fields are `pub`. - extra_nonce: String, - prev_block: String, + + // This optional expression can be placed after + // a `field: field_type`. this indicates to the + // macro to (de)serialize this field using this + // default expression if it doesn't exist in epee. + // + // See `cuprate_epee_encoding::epee_object` for info. + // + // The default function must be specified twice: + // + // 1. As an expression + // 2. As a string literal + // + // For example: `extra_nonce: String /* = default_string(), "default_string" */,` + // + // This is a HACK since `serde`'s default attribute only takes in + // string literals and macros (stringify) within attributes do not work. + extra_nonce: String /* = default_expression, "default_literal" */, + + // Another optional expression: + // This indicates to the macro to (de)serialize + // this field as another type in epee. + // + // See `cuprate_epee_encoding::epee_object` for info. + prev_block: String /* as Type */, + + // Regular fields. reserve_size: u64, wallet_address: String, }, @@ -197,8 +222,7 @@ define_request_and_response! { GetLastBlockHeader, #[derive(Copy)] Request { - #[cfg_attr(feature = "serde", serde(default = "default_false"))] - fill_pow_hash: bool = default_false(), + fill_pow_hash: bool = default_false(), "default_false", }, AccessResponseBase { block_header: BlockHeader, @@ -213,8 +237,7 @@ define_request_and_response! { Request { hash: String, hashes: Vec, - #[cfg_attr(feature = "serde", serde(default = "default_false"))] - fill_pow_hash: bool = default_false(), + fill_pow_hash: bool = default_false(), "default_false", }, AccessResponseBase { block_header: BlockHeader, @@ -230,8 +253,7 @@ define_request_and_response! { #[derive(Copy)] Request { height: u64, - #[cfg_attr(feature = "serde", serde(default = "default_false"))] - fill_pow_hash: bool = default_false(), + fill_pow_hash: bool = default_false(), "default_false", }, AccessResponseBase { block_header: BlockHeader, @@ -247,8 +269,7 @@ define_request_and_response! { Request { start_height: u64, end_height: u64, - #[cfg_attr(feature = "serde", serde(default = "default_false"))] - fill_pow_hash: bool = default_false(), + fill_pow_hash: bool = default_false(), "default_false", }, AccessResponseBase { headers: Vec, @@ -264,12 +285,9 @@ define_request_and_response! { // `monerod` has both `hash` and `height` fields. // In the RPC handler, if `hash.is_empty()`, it will use it, else, it uses `height`. // - #[cfg_attr(feature = "serde", serde(default = "default_string"))] - hash: String = default_string(), - #[cfg_attr(feature = "serde", serde(default = "default_height"))] - height: u64 = default_height(), - #[cfg_attr(feature = "serde", serde(default = "default_false"))] - fill_pow_hash: bool = default_false(), + hash: String = default_string(), "default_string", + height: u64 = default_height(), "default_height", + fill_pow_hash: bool = default_false(), "default_false", }, AccessResponseBase { blob: String, @@ -287,7 +305,7 @@ define_request_and_response! { GetConnections, Request {}, ResponseBase { - // TODO: This is a `std::list` in `monerod` because...? + // FIXME: This is a `std::list` in `monerod` because...? connections: Vec, } } @@ -405,8 +423,7 @@ define_request_and_response! { core_rpc_server_commands_defs.h => 2096..=2116, FlushTransactionPool, Request { - #[cfg_attr(feature = "serde", serde(default = "default_vec"))] - txids: Vec = default_vec::(), + txids: Vec = default_vec::(), "default_vec", }, #[derive(Copy)] #[cfg_attr(feature = "serde", serde(transparent))] @@ -461,12 +478,12 @@ define_request_and_response! { ResponseBase { version: u32, release: bool, - #[serde(skip_serializing_if = "is_zero", default = "default_zero")] - current_height: u64 = default_zero(), - #[serde(skip_serializing_if = "is_zero", default = "default_zero")] - target_height: u64 = default_zero(), - #[serde(skip_serializing_if = "Vec::is_empty", default = "default_vec")] - hard_forks: Vec = default_vec(), + #[serde(skip_serializing_if = "is_zero")] + current_height: u64 = default_zero(), "default_zero", + #[serde(skip_serializing_if = "is_zero")] + target_height: u64 = default_zero(), "default_zero", + #[serde(skip_serializing_if = "Vec::is_empty")] + hard_forks: Vec = default_vec(), "default_vec", } } @@ -521,9 +538,9 @@ define_request_and_response! { height: u64, next_needed_pruning_seed: u32, overview: String, - // TODO: This is a `std::list` in `monerod` because...? + // FIXME: This is a `std::list` in `monerod` because...? peers: Vec, - // TODO: This is a `std::list` in `monerod` because...? + // FIXME: This is a `std::list` in `monerod` because...? spans: Vec, target_height: u64, } @@ -588,8 +605,7 @@ define_request_and_response! { PruneBlockchain, #[derive(Copy)] Request { - #[cfg_attr(feature = "serde", serde(default = "default_false"))] - check: bool = default_false(), + check: bool = default_false(), "default_false", }, #[derive(Copy)] ResponseBase { @@ -623,10 +639,8 @@ define_request_and_response! { FlushCache, #[derive(Copy)] Request { - #[cfg_attr(feature = "serde", serde(default = "default_false"))] - bad_txs: bool = default_false(), - #[cfg_attr(feature = "serde", serde(default = "default_false"))] - bad_blocks: bool = default_false(), + bad_txs: bool = default_false(), "default_false", + bad_blocks: bool = default_false(), "default_false", }, ResponseBase {} } diff --git a/rpc/types/src/macros.rs b/rpc/types/src/macros.rs index 31bc6bed..e1301387 100644 --- a/rpc/types/src/macros.rs +++ b/rpc/types/src/macros.rs @@ -45,7 +45,7 @@ /// would trigger the different branches. macro_rules! define_request_and_response { ( - // The markdown tag for Monero RPC documentation. Not necessarily the endpoint. + // The markdown tag for Monero daemon RPC documentation. Not necessarily the endpoint. $monero_daemon_rpc_doc_link:ident, // The commit hash and `$file.$extension` in which this type is defined in @@ -67,8 +67,10 @@ macro_rules! define_request_and_response { Request { // And any fields. $( - $( #[$request_field_attr:meta] )* - $request_field:ident: $request_field_type:ty $(= $request_field_type_default:expr)?, + $( #[$request_field_attr:meta] )* // Field attribute. + $request_field:ident: $request_field_type:ty // field_name: field type + $(as $request_field_type_as:ty)? // (optional) alternative type (de)serialization + $(= $request_field_type_default:expr, $request_field_type_default_string:literal)?, // (optional) default value )* }, @@ -78,7 +80,9 @@ macro_rules! define_request_and_response { // And any fields. $( $( #[$response_field_attr:meta] )* - $response_field:ident: $response_field_type:ty $(= $response_field_type_default:expr)?, + $response_field:ident: $response_field_type:ty + $(as $response_field_type_as:ty)? + $(= $response_field_type_default:expr, $response_field_type_default_string:literal)?, )* } ) => { paste::paste! { @@ -99,7 +103,9 @@ macro_rules! define_request_and_response { [<$type_name Request>] { $( $( #[$request_field_attr] )* - $request_field: $request_field_type $(= $request_field_type_default)?, + $request_field: $request_field_type + $(as $request_field_type_as)? + $(= $request_field_type_default, $request_field_type_default_string)?, )* } } @@ -125,7 +131,9 @@ macro_rules! define_request_and_response { $response_base_type => [<$type_name Response>] { $( $( #[$response_field_attr] )* - $response_field: $response_field_type $(= $response_field_type_default)?, + $response_field: $response_field_type + $(as $response_field_type_as)? + $(= $response_field_type_default, $response_field_type_default_string)?, )* } } @@ -166,7 +174,9 @@ macro_rules! __define_request { $( $( #[$field_attr:meta] )* // field attributes // field_name: FieldType - $field:ident: $field_type:ty $(= $field_default:expr)?, + $field:ident: $field_type:ty + $(as $field_as:ty)? + $(= $field_default:expr, $field_default_string:literal)?, // The $field_default is an optional extra token that represents // a default value to pass to [`cuprate_epee_encoding::epee_object`], // see it for usage. @@ -180,6 +190,7 @@ macro_rules! __define_request { pub struct $t { $( $( #[$field_attr] )* + $(#[cfg_attr(feature = "serde", serde(default = $field_default_string))])? pub $field: $field_type, )* } @@ -188,7 +199,9 @@ macro_rules! __define_request { ::cuprate_epee_encoding::epee_object! { $t, $( - $field: $field_type $(= $field_default)?, + $field: $field_type + $(as $field_as)? + $(= $field_default)?, )* } }; @@ -218,7 +231,9 @@ macro_rules! __define_response { // See [`__define_request`] for docs, this does the same thing. $( $( #[$field_attr:meta] )* - $field:ident: $field_type:ty $(= $field_default:expr)?, + $field:ident: $field_type:ty + $(as $field_as:ty)? + $(= $field_default:expr, $field_default_string:literal)?, )* } ) => { @@ -226,6 +241,7 @@ macro_rules! __define_response { pub struct $t { $( $( #[$field_attr] )* + $(#[cfg_attr(feature = "serde", serde(default = $field_default_string))])? pub $field: $field_type, )* } @@ -234,7 +250,9 @@ macro_rules! __define_response { ::cuprate_epee_encoding::epee_object! { $t, $( - $field: $field_type $($field_default)?, + $field: $field_type + $(as $field_as)? + $(= $field_default)?, )* } }; @@ -250,7 +268,9 @@ macro_rules! __define_response { // See [`__define_request`] for docs, this does the same thing. $( $( #[$field_attr:meta] )* - $field:ident: $field_type:ty $(= $field_default:expr)?, + $field:ident: $field_type:ty + $(as $field_as:ty)? + $(= $field_default:expr, $field_default_string:literal)?, )* } ) => { @@ -261,6 +281,7 @@ macro_rules! __define_response { $( $( #[$field_attr] )* + $(#[cfg_attr(feature = "serde", serde(default = $field_default_string))])? pub $field: $field_type, )* } @@ -269,7 +290,9 @@ macro_rules! __define_response { ::cuprate_epee_encoding::epee_object! { $t, $( - $field: $field_type $(= $field_default)?, + $field: $field_type + $(as $field_as)? + $(= $field_default)?, )* !flatten: base: $base, } diff --git a/rpc/types/src/other.rs b/rpc/types/src/other.rs index 03cb05dd..5ad2caac 100644 --- a/rpc/types/src/other.rs +++ b/rpc/types/src/other.rs @@ -36,12 +36,9 @@ define_request_and_response! { // FIXME: this is documented as optional but it isn't serialized as an optional // but it is set _somewhere_ to false in `monerod` // - #[cfg_attr(feature = "serde", serde(default = "default_false"))] - decode_as_json: bool = default_false(), - #[cfg_attr(feature = "serde", serde(default = "default_false"))] - prune: bool = default_false(), - #[cfg_attr(feature = "serde", serde(default = "default_false"))] - split: bool = default_false(), + decode_as_json: bool = default_false(), "default_false", + prune: bool = default_false(), "default_false", + split: bool = default_false(), "default_false", }, AccessResponseBase { txs_as_hex: Vec, @@ -82,10 +79,8 @@ define_request_and_response! { SendRawTransaction, Request { tx_as_hex: String, - #[cfg_attr(feature = "serde", serde(default = "default_false"))] - do_not_relay: bool = default_false(), - #[cfg_attr(feature = "serde", serde(default = "default_true"))] - do_sanity_checks: bool = default_true(), + do_not_relay: bool = default_false(), "default_false", + do_sanity_checks: bool = default_true(), "default_true", }, AccessResponseBase { double_spend: bool, @@ -167,10 +162,8 @@ define_request_and_response! { core_rpc_server_commands_defs.h => 1369..=1417, GetPeerList, Request { - #[cfg_attr(feature = "serde", serde(default = "default_true"))] - public_only: bool = default_true(), - #[cfg_attr(feature = "serde", serde(default = "default_false"))] - include_blocked: bool = default_false(), + public_only: bool = default_true(), "default_true", + include_blocked: bool = default_false(), "default_false", }, ResponseBase { white_list: Vec, @@ -208,8 +201,7 @@ define_request_and_response! { core_rpc_server_commands_defs.h => 1494..=1517, SetLogCategories, Request { - #[cfg_attr(feature = "serde", serde(default = "default_string"))] - categories: String = default_string(), + categories: String = default_string(), "default_string", }, ResponseBase { categories: String, @@ -300,8 +292,7 @@ define_request_and_response! { core_rpc_server_commands_defs.h => 1876..=1903, OutPeers, Request { - #[cfg_attr(feature = "serde", serde(default = "default_true"))] - set: bool = default_true(), + set: bool = default_true(), "default_true", out_peers: u32, }, ResponseBase { @@ -345,8 +336,7 @@ define_request_and_response! { Update, Request { command: String, - #[cfg_attr(feature = "serde", serde(default = "default_string"))] - path: String = default_string(), + path: String = default_string(), "default_string", }, ResponseBase { auto_uri: String, @@ -402,12 +392,9 @@ define_request_and_response! { core_rpc_server_commands_defs.h => 1419..=1448, GetPublicNodes, Request { - #[cfg_attr(feature = "serde", serde(default = "default_false"))] - gray: bool = default_false(), - #[cfg_attr(feature = "serde", serde(default = "default_true"))] - white: bool = default_true(), - #[cfg_attr(feature = "serde", serde(default = "default_false"))] - include_blocked: bool = default_false(), + gray: bool = default_false(), "default_false", + white: bool = default_true(), "default_true", + include_blocked: bool = default_false(), "default_false", }, ResponseBase { gray: Vec,