Merge pull request fedimint#5365 from elsirion/2024-05-remove-json-co…

…nsensus-cfg

Remove json consensus config from server config only kept around for a UI

fedimint-client/src/lib.rs

@@ -85,10 +85,7 @@ use db::{
 use fedimint_api_client::api::{
     ApiVersionSet, DynGlobalApi, DynModuleApi, FederationApiExt, IGlobalFederationApi,
 };
-use fedimint_core::config::{
-    ClientConfig, ClientModuleConfig, FederationId, JsonClientConfig, JsonWithKind,
-    ModuleInitRegistry,
-};
+use fedimint_core::config::{ClientConfig, FederationId, JsonClientConfig, ModuleInitRegistry};
 use fedimint_core::core::{
     DynInput, DynOutput, IInput, IOutput, ModuleInstanceId, ModuleKind, OperationId,
 };
@@ -1303,28 +1300,7 @@ impl Client {
     /// encoded this format cannot be cryptographically verified but is easier
     /// to consume and to some degree human-readable.
     pub fn get_config_json(&self) -> JsonClientConfig {
-        JsonClientConfig {
-            global: self.get_config().global.clone(),
-            modules: self
-                .get_config()
-                .modules
-                .iter()
-                .map(|(instance_id, ClientModuleConfig { kind, config, .. })| {
-                    (
-                        *instance_id,
-                        JsonWithKind::new(
-                            kind.clone(),
-                            config
-                                .clone()
-                                .decoded()
-                                .map_or(serde_json::Value::Null, |decoded| {
-                                    decoded.to_json().into()
-                                }),
-                        ),
-                    )
-                })
-                .collect(),
-        }
+        self.get_config().to_json()
     }
 
     /// Get the primary module

fedimint-core/src/config.rs

@@ -21,6 +21,7 @@ use hex::FromHex;
 use serde::de::DeserializeOwned;
 use serde::ser::SerializeMap;
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
+use serde_json::json;
 use thiserror::Error;
 use threshold_crypto::group::{Curve, Group, GroupEncoding};
 use threshold_crypto::{G1Projective, G2Projective};
@@ -238,6 +239,34 @@ impl ClientConfig {
             )
         })
     }
+
+    /// Converts a consensus-encoded client config struct to a client config
+    /// struct that when encoded as JSON shows the fields of module configs
+    /// instead of a consensus-encoded hex string.
+    ///
+    /// In case of unknown module the config value is a hex string.
+    pub fn to_json(&self) -> JsonClientConfig {
+        JsonClientConfig {
+            global: self.global.clone(),
+            modules: self
+                .modules
+                .iter()
+                .map(|(&module_instance_id, module_config)| {
+                    let module_config_json = JsonWithKind {
+                    kind: module_config.kind.clone(),
+                    value: module_config.config
+                        .clone()
+                        .decoded()
+                        .and_then(|dyn_cfg| dyn_cfg.to_json())
+                        .unwrap_or_else(|| json!({
+                            "unknown_module_hex": module_config.config.consensus_encode_to_hex()
+                        })),
+                };
+                    (module_instance_id, module_config_json)
+                })
+                .collect(),
+        }
+    }
 }
 
 /// The federation id is a copy of the authentication threshold public key of
@@ -721,21 +750,18 @@ pub struct ServerModuleConfig {
     pub local: JsonWithKind,
     pub private: JsonWithKind,
     pub consensus: ServerModuleConsensusConfig,
-    pub consensus_json: JsonWithKind,
 }
 
 impl ServerModuleConfig {
     pub fn from(
         local: JsonWithKind,
         private: JsonWithKind,
         consensus: ServerModuleConsensusConfig,
-        consensus_json: JsonWithKind,
     ) -> Self {
         Self {
             local,
             private,
             consensus,
-            consensus_json,
         }
     }
 
@@ -801,10 +827,6 @@ pub trait TypedServerModuleConfig: DeserializeOwned + Serialize {
                 version: consensus.version(),
                 config: consensus.consensus_encode_to_vec(),
             },
-            consensus_json: JsonWithKind::new(
-                kind,
-                serde_json::to_value(consensus).expect("serialization can't fail"),
-            ),
         }
     }
 }

fedimint-core/src/endpoint_constants.rs

@@ -5,6 +5,7 @@ pub const AUTH_ENDPOINT: &str = "auth";
 pub const AWAIT_OUTPUT_OUTCOME_ENDPOINT: &str = "await_output_outcome";
 pub const BACKUP_ENDPOINT: &str = "backup";
 pub const CLIENT_CONFIG_ENDPOINT: &str = "client_config";
+pub const CLIENT_CONFIG_JSON_ENDPOINT: &str = "client_config_json";
 pub const SERVER_CONFIG_CONSENSUS_HASH_ENDPOINT: &str = "server_config_consensus_hash";
 pub const SESSION_COUNT_ENDPOINT: &str = "session_count";
 pub const AWAIT_SESSION_OUTCOME_ENDPOINT: &str = "await_session_outcome";
@@ -15,7 +16,6 @@ pub const CONFIG_GEN_PEERS_ENDPOINT: &str = "config_gen_peers";
 pub const CONSENSUS_CONFIG_GEN_PARAMS_ENDPOINT: &str = "consensus_config_gen_params";
 pub const DEFAULT_CONFIG_GEN_PARAMS_ENDPOINT: &str = "default_config_gen_params";
 pub const VERIFY_CONFIG_HASH_ENDPOINT: &str = "verify_config_hash";
-pub const MODULES_CONFIG_JSON_ENDPOINT: &str = "modules_config_json";
 pub const RECOVER_ENDPOINT: &str = "recover";
 pub const REGISTER_GATEWAY_ENDPOINT: &str = "register_gateway";
 pub const RUN_DKG_ENDPOINT: &str = "run_dkg";

fedimint-derive/src/lib.rs

@@ -7,25 +7,10 @@ use quote::{format_ident, quote};
 use syn::punctuated::Punctuated;
 use syn::token::Comma;
 use syn::{
-    parse_macro_input, Attribute, Data, DataEnum, DataStruct, DeriveInput, Field, Fields, Index,
-    Lit, Token, Variant,
+    parse_macro_input, Attribute, Data, DataEnum, DataStruct, DeriveInput, Fields, Index, Lit,
+    Token, Variant,
 };
 
-fn do_not_ignore(field: &Field) -> bool {
-    !field
-        .attrs
-        .iter()
-        .any(|attr| attr.path().is_ident("encodable_ignore"))
-}
-
-fn panic_if_ignored(field: &Field) -> bool {
-    assert!(
-        do_not_ignore(field),
-        "Trying to derive decodable from a struct with ignored fields"
-    );
-    true
-}
-
 fn is_default_variant_enforce_valid(variant: &Variant) -> bool {
     let is_default = variant
         .attrs
@@ -53,9 +38,9 @@ fn is_default_variant_enforce_valid(variant: &Variant) -> bool {
     is_default
 }
 
-// TODO: use encodable attr for everything: #[encodable(ignore)],
-// #[encodable(index = 42)], …
-#[proc_macro_derive(Encodable, attributes(encodable_ignore, encodable_default, encodable))]
+// TODO: use encodable attr for everything: #[encodable(index = 42)],
+// #[encodable(default)], …
+#[proc_macro_derive(Encodable, attributes(encodable_default, encodable))]
 pub fn derive_encodable(input: TokenStream) -> TokenStream {
     let DeriveInput {
         ident,
@@ -88,7 +73,6 @@ fn derive_struct_encode(fields: &Fields) -> TokenStream2 {
         let field_names = fields
             .iter()
             .enumerate()
-            .filter(|(_, f)| do_not_ignore(f))
             .map(|(idx, _)| Index::from(idx))
             .collect::<Vec<_>>();
         quote! {
@@ -100,7 +84,6 @@ fn derive_struct_encode(fields: &Fields) -> TokenStream2 {
         // Named struct
         let field_names = fields
             .iter()
-            .filter(|f| do_not_ignore(f))
             .map(|field| field.ident.clone().unwrap())
             .collect::<Vec<_>>();
         quote! {
@@ -193,7 +176,6 @@ fn derive_enum_encode(ident: &Ident, variants: &Punctuated<Variant, Comma>) -> T
                         .fields
                         .iter()
                         .enumerate()
-                        .filter(|(_, f)| do_not_ignore(f))
                         .map(|(idx, _)| format_ident!("bound_{}", idx))
                         .collect::<Vec<_>>();
                     let variant_encode_block =
@@ -207,7 +189,6 @@ fn derive_enum_encode(ident: &Ident, variants: &Punctuated<Variant, Comma>) -> T
                     let variant_fields = variant
                         .fields
                         .iter()
-                        .filter(|f| do_not_ignore(f))
                         .map(|field| field.ident.clone().unwrap())
                         .collect::<Vec<_>>();
                     let variant_encode_block =
@@ -410,7 +391,6 @@ fn derive_tuple_decode_block(
 ) -> TokenStream2 {
     let field_names = fields
         .iter()
-        .filter(|f| panic_if_ignored(f))
         .enumerate()
         .map(|(idx, _)| format_ident!("field_{}", idx))
         .collect::<Vec<_>>();
@@ -438,7 +418,6 @@ fn derive_named_decode_block(
 ) -> TokenStream2 {
     let variant_fields = fields
         .iter()
-        .filter(|f| panic_if_ignored(f))
         .map(|field| field.ident.clone().unwrap())
         .collect::<Vec<_>>();
     quote! {

fedimint-server/src/config/mod.rs

@@ -134,10 +134,6 @@ pub struct ServerConfigConsensus {
     pub tls_certs: BTreeMap<PeerId, rustls::Certificate>,
     /// All configuration that needs to be the same for modules
     pub modules: BTreeMap<ModuleInstanceId, ServerModuleConsensusConfig>,
-    #[encodable_ignore]
-    // FIXME: Make modules encodable or we will not check module keys
-    /// Human readable representation of [`Self::modules`]
-    pub modules_json: BTreeMap<ModuleInstanceId, JsonWithKind>,
     /// Additional config the federation wants to transmit to the clients
     pub meta: BTreeMap<String, String>,
 }
@@ -264,7 +260,6 @@ impl ServerConfig {
             api_endpoints: params.api_urls(),
             tls_certs: params.tls_certs(),
             modules: Default::default(),
-            modules_json: Default::default(),
             meta: params.consensus.meta,
         };
         let mut cfg = Self {
@@ -297,13 +292,11 @@ impl ServerConfig {
                 local,
                 private,
                 consensus,
-                consensus_json,
             } = config;
 
             self.local.modules.insert(name, local);
             self.private.modules.insert(name, private);
             self.consensus.modules.insert(name, consensus);
-            self.consensus.modules_json.insert(name, consensus_json);
         }
     }
 
@@ -320,8 +313,7 @@ impl ServerConfig {
             .get(&id)
             .ok_or_else(|| format_err!("Module {id} not found"))?
             .clone();
-        let consensus_json = Self::get_module_cfg_by_instance_id(&self.consensus.modules_json, id)?;
-        let module = ServerModuleConfig::from(local, private, consensus, consensus_json);
+        let module = ServerModuleConfig::from(local, private, consensus);
 
         module.to_typed()
     }
@@ -349,13 +341,7 @@ impl ServerConfig {
             .get(&id)
             .ok_or_else(|| format_err!("Module {id} not found"))?
             .clone();
-        let consensus_json = Self::get_module_cfg_by_instance_id(&self.consensus.modules_json, id)?;
-        Ok(ServerModuleConfig::from(
-            local,
-            private,
-            consensus,
-            consensus_json,
-        ))
+        Ok(ServerModuleConfig::from(local, private, consensus))
     }
 
     fn get_module_cfg_by_instance_id(
@@ -526,7 +512,7 @@ impl ServerConfig {
         });
         for (module_instance_id, config) in join_all(modules_runner).await {
             let config = config?;
-            registered_modules.remove(config.consensus_json.kind());
+            registered_modules.remove(&config.consensus.kind);
             module_cfgs.insert(module_instance_id, config);
         }
         if !registered_modules.is_empty() {

fedimint-server/src/consensus/api.rs

@@ -13,7 +13,7 @@ use fedimint_api_client::api::{
 };
 use fedimint_core::admin_client::ServerStatus;
 use fedimint_core::backup::{ClientBackupKey, ClientBackupSnapshot};
-use fedimint_core::config::ClientConfig;
+use fedimint_core::config::{ClientConfig, JsonClientConfig};
 use fedimint_core::core::backup::{SignedBackupRequest, BACKUP_REQUEST_MAX_PAYLOAD_SIZE_BYTES};
 use fedimint_core::core::{DynOutputOutcome, ModuleInstanceId};
 use fedimint_core::db::{
@@ -22,8 +22,8 @@ use fedimint_core::db::{
 use fedimint_core::endpoint_constants::{
     AUDIT_ENDPOINT, AUTH_ENDPOINT, AWAIT_OUTPUT_OUTCOME_ENDPOINT, AWAIT_SESSION_OUTCOME_ENDPOINT,
     AWAIT_SIGNED_SESSION_OUTCOME_ENDPOINT, AWAIT_TRANSACTION_ENDPOINT, BACKUP_ENDPOINT,
-    CLIENT_CONFIG_ENDPOINT, FEDERATION_ID_ENDPOINT, GUARDIAN_CONFIG_BACKUP_ENDPOINT,
-    INVITE_CODE_ENDPOINT, MODULES_CONFIG_JSON_ENDPOINT, RECOVER_ENDPOINT,
+    CLIENT_CONFIG_ENDPOINT, CLIENT_CONFIG_JSON_ENDPOINT, FEDERATION_ID_ENDPOINT,
+    GUARDIAN_CONFIG_BACKUP_ENDPOINT, INVITE_CODE_ENDPOINT, RECOVER_ENDPOINT,
     SERVER_CONFIG_CONSENSUS_HASH_ENDPOINT, SESSION_COUNT_ENDPOINT, SESSION_STATUS_ENDPOINT,
     SHUTDOWN_ENDPOINT, STATUS_ENDPOINT, SUBMIT_TRANSACTION_ENDPOINT, VERSION_ENDPOINT,
 };
@@ -49,7 +49,7 @@ use tracing::{debug, info};
 use crate::config::io::{
     CONSENSUS_CONFIG, ENCRYPTED_EXT, JSON_EXT, LOCAL_CONFIG, PRIVATE_CONFIG, SALT_FILE,
 };
-use crate::config::{JsonWithKind, ServerConfig};
+use crate::config::ServerConfig;
 use crate::consensus::db::{AcceptedItemPrefix, AcceptedTransactionKey, SignedSessionOutcomeKey};
 use crate::consensus::engine::get_finished_session_count_static;
 use crate::consensus::transaction::process_transaction_with_dbtx;
@@ -473,6 +473,14 @@ pub fn server_endpoints() -> Vec<ApiEndpoint<ConsensusApi>> {
                 Ok(fedimint.client_cfg.clone())
             }
         },
+        // Helper endpoint for Admin UI that can't parse consensus encoding
+        api_endpoint! {
+            CLIENT_CONFIG_JSON_ENDPOINT,
+            ApiVersion::new(0, 0),
+            async |fedimint: &ConsensusApi, _context, _v: ()| -> JsonClientConfig {
+                Ok(fedimint.client_cfg.to_json())
+            }
+        },
         api_endpoint! {
             SERVER_CONFIG_CONSENSUS_HASH_ENDPOINT,
             ApiVersion::new(0, 0),
@@ -570,12 +578,5 @@ pub fn server_endpoints() -> Vec<ApiEndpoint<ConsensusApi>> {
                 Ok(())
             }
         },
-        api_endpoint! {
-            MODULES_CONFIG_JSON_ENDPOINT,
-            ApiVersion::new(0, 0),
-            async |fedimint: &ConsensusApi, _context, _v: ()| -> BTreeMap<ModuleInstanceId, JsonWithKind> {
-                Ok(fedimint.cfg.consensus.modules_json.clone())
-            }
-        },
     ]
 }