diff --git a/fedimint-cli/src/lib.rs b/fedimint-cli/src/lib.rs index 369e6b7affb..e356350373a 100644 --- a/fedimint-cli/src/lib.rs +++ b/fedimint-cli/src/lib.rs @@ -568,7 +568,7 @@ impl FedimintCli { let client_builder = self.make_client_builder(cli).await?; - let mnemonic = load_or_generate_mnemonic(client_builder.db_no_encoders()).await?; + let mnemonic = load_or_generate_mnemonic(client_builder.db_no_decoders()).await?; client_builder .join( @@ -594,7 +594,7 @@ impl FedimintCli { } let mnemonic = Mnemonic::from_entropy( - &Client::load_decodable_client_secret::>(client_builder.db_no_encoders()) + &Client::load_decodable_client_secret::>(client_builder.db_no_decoders()) .await .map_err_cli()?, ) @@ -626,7 +626,7 @@ impl FedimintCli { .await .map_err_cli()?; - match Client::load_decodable_client_secret_opt::>(builder.db_no_encoders()) + match Client::load_decodable_client_secret_opt::>(builder.db_no_decoders()) .await .map_err_cli()? { @@ -637,7 +637,7 @@ impl FedimintCli { } None => { Client::store_encodable_client_secret( - builder.db_no_encoders(), + builder.db_no_decoders(), mnemonic.to_entropy(), ) .await diff --git a/fedimint-client/src/lib.rs b/fedimint-client/src/lib.rs index bf34f9e3553..d302235a4e0 100644 --- a/fedimint-client/src/lib.rs +++ b/fedimint-client/src/lib.rs @@ -1756,7 +1756,7 @@ pub struct ClientBuilder { module_inits: ClientModuleInitRegistry, primary_module_instance: Option, admin_creds: Option, - db_no_encoders: Database, + db_no_decoders: Database, stopped: bool, } @@ -1766,7 +1766,7 @@ impl ClientBuilder { module_inits: Default::default(), primary_module_instance: Default::default(), admin_creds: None, - db_no_encoders: db, + db_no_decoders: db, stopped: false, } } @@ -1776,7 +1776,7 @@ impl ClientBuilder { module_inits: client.module_inits.clone(), primary_module_instance: Some(client.primary_module_instance), admin_creds: None, - db_no_encoders: client.db.with_decoders(Default::default()), + db_no_decoders: client.db.with_decoders(Default::default()), stopped: false, } } @@ -1840,12 +1840,12 @@ impl ClientBuilder { Ok(()) } - pub fn db_no_encoders(&self) -> &Database { - &self.db_no_encoders + pub fn db_no_decoders(&self) -> &Database { + &self.db_no_decoders } pub async fn load_existing_config(&self) -> anyhow::Result { - let Some(config) = Client::get_config_from_db(&self.db_no_encoders).await else { + let Some(config) = Client::get_config_from_db(&self.db_no_decoders).await else { bail!("Client database not initialized") }; @@ -1862,7 +1862,7 @@ impl ClientBuilder { config: ClientConfig, init_mode: InitMode, ) -> anyhow::Result { - if Client::is_initialized(&self.db_no_encoders).await { + if Client::is_initialized(&self.db_no_decoders).await { bail!("Client database already initialized") } @@ -1870,7 +1870,7 @@ impl ClientBuilder { // transaction to avoid half-initialized client state. { debug!(target: LOG_CLIENT, "Initializing client database"); - let mut dbtx = self.db_no_encoders.begin_transaction().await; + let mut dbtx = self.db_no_decoders.begin_transaction().await; // Save config to DB dbtx.insert_new_entry( &ClientConfigKey { @@ -2024,7 +2024,7 @@ impl ClientBuilder { } pub async fn open(self, root_secret: DerivableSecret) -> anyhow::Result { - let Some(config) = Client::get_config_from_db(&self.db_no_encoders).await else { + let Some(config) = Client::get_config_from_db(&self.db_no_decoders).await else { bail!("Client database not initialized") }; let stopped = self.stopped; @@ -2071,7 +2071,7 @@ impl ClientBuilder { let decoders = self.decoders(&config); let config = Self::config_decoded(config, &decoders)?; let fed_id = config.calculate_federation_id(); - let db = self.db_no_encoders.with_decoders(decoders.clone()); + let db = self.db_no_decoders.with_decoders(decoders.clone()); let api = if let Some(admin_creds) = self.admin_creds.as_ref() { Self::admin_api_from_id(admin_creds.peer_id, &config)? } else { diff --git a/fedimint-client/src/sm/state.rs b/fedimint-client/src/sm/state.rs index 976ebbd5b54..062df95c448 100644 --- a/fedimint-client/src/sm/state.rs +++ b/fedimint-client/src/sm/state.rs @@ -319,10 +319,12 @@ impl Encodable for DynState { impl Decodable for DynState { fn consensus_decode( reader: &mut R, - modules: &::fedimint_core::module::registry::ModuleDecoderRegistry, + decoders: &::fedimint_core::module::registry::ModuleDecoderRegistry, ) -> Result { - let key = fedimint_core::core::ModuleInstanceId::consensus_decode(reader, modules)?; - modules.get_expect(key).decode(reader, key, modules) + let module_id = fedimint_core::core::ModuleInstanceId::consensus_decode(reader, decoders)?; + decoders + .get_expect(module_id) + .decode_partial(reader, module_id, decoders) } } diff --git a/fedimint-client/src/transaction/sm.rs b/fedimint-client/src/transaction/sm.rs index bd32c5fa6c3..47f5d0de2a5 100644 --- a/fedimint-client/src/transaction/sm.rs +++ b/fedimint-client/src/transaction/sm.rs @@ -148,7 +148,7 @@ impl IntoDynInstance for TxSubmissionStates { } pub fn tx_submission_sm_decoder() -> Decoder { - let mut decoder_builder = Decoder::builder(); + let mut decoder_builder = Decoder::builder_system(); decoder_builder.with_decodable_type::>(); decoder_builder.build() } diff --git a/fedimint-core/src/core.rs b/fedimint-core/src/core.rs index 8ceda224acc..e797bc2fd2f 100644 --- a/fedimint-core/src/core.rs +++ b/fedimint-core/src/core.rs @@ -258,15 +258,20 @@ where } } -type DecodeFn = for<'a> fn( - Box, - ModuleInstanceId, - &ModuleDecoderRegistry, -) -> Result, DecodeError>; +type DecodeFn = Box< + dyn for<'a> Fn( + Box, + ModuleInstanceId, + &ModuleDecoderRegistry, + ) -> Result, DecodeError> + + Send + + Sync, +>; #[derive(Default)] pub struct DecoderBuilder { decode_fns: BTreeMap, + transparent: bool, } impl DecoderBuilder { @@ -291,14 +296,30 @@ impl DecoderBuilder { where Type: IntoDynInstance + Decodable, { + let is_transparent_decoder = self.transparent; // TODO: enforce that all decoders are for the same module kind (+fix docs // after) - let decode_fn: DecodeFn = |mut reader, instance, modules| { - let typed_val = Type::consensus_decode(&mut reader, modules)?; - let dyn_val = typed_val.into_dyn(instance); - let any_val: Box = Box::new(dyn_val); - Ok(any_val) - }; + let decode_fn: DecodeFn = Box::new( + move |mut reader, instance, decoders: &ModuleDecoderRegistry| { + // TODO: Ideally `DynTypes` decoding couldn't ever be nested, so we could just + // pass empty `decoders`. But the client context uses nested `DynTypes` in + // `DynState`, so we special-case it with a flag. + let decoders = if is_transparent_decoder { + Cow::Borrowed(decoders) + } else { + Cow::Owned(Default::default()) + }; + let typed_val = Type::consensus_decode(&mut reader, &decoders).map_err(|err| { + let err: anyhow::Error = err.into(); + DecodeError::new_custom( + err.context(format!("while decoding Dyn type module_id={instance}")), + ) + })?; + let dyn_val = typed_val.into_dyn(instance); + let any_val: Box = Box::new(dyn_val); + Ok(any_val) + }, + ); if self .decode_fns .insert(TypeId::of::(), decode_fn) @@ -322,15 +343,53 @@ impl Decoder { DecoderBuilder::default() } + /// System Dyn-type, don't use. + #[doc(hidden)] + pub fn builder_system() -> DecoderBuilder { + DecoderBuilder { + transparent: true, + ..DecoderBuilder::default() + } + } + /// Decodes a specific `DynType` from the `reader` byte stream. /// /// # Panics /// * If no decoder is registered for the `DynType` - pub fn decode( + pub fn decode_complete( + &self, + reader: &mut dyn Read, + total_len: u64, + module_id: ModuleInstanceId, + decoders: &ModuleDecoderRegistry, + ) -> Result { + let mut reader = reader.take(total_len); + + let val = self.decode_partial(&mut reader, module_id, decoders)?; + let left = reader.limit(); + + if left != 0 { + return Err(fedimint_core::encoding::DecodeError::new_custom( + anyhow::anyhow!( + "Dyn type did not consume all bytes during decoding; module_id={}; expected={}; left={}; type={}", + module_id, + total_len, + left, + std::any::type_name::(), + ), + )); + } + + Ok(val) + } + + /// Like [`Self::decode_complete`] but does not verify that all bytes were + /// consumed + pub fn decode_partial( &self, reader: &mut dyn Read, - instance_id: ModuleInstanceId, - modules: &ModuleDecoderRegistry, + module_id: ModuleInstanceId, + decoders: &ModuleDecoderRegistry, ) -> Result { let decode_fn = self .decode_fns @@ -343,7 +402,7 @@ impl Decoder { ) }) .expect("Types being decoded must be registered"); - Ok(*decode_fn(Box::new(reader), instance_id, modules)? + Ok(*decode_fn(Box::new(reader), module_id, decoders)? .downcast::() .expect("Decode fn returned wrong type, can't happen due to with_decodable_type")) } diff --git a/fedimint-core/src/encoding/mod.rs b/fedimint-core/src/encoding/mod.rs index a03a035d487..1a7cea80c0e 100644 --- a/fedimint-core/src/encoding/mod.rs +++ b/fedimint-core/src/encoding/mod.rs @@ -1042,8 +1042,9 @@ where module_instance_id, raw, } => match decoders.get(module_instance_id) { - Some(decoder) => DynRawFallback::Decoded(decoder.decode( + Some(decoder) => DynRawFallback::Decoded(decoder.decode_complete( &mut &raw[..], + raw.len() as u64, module_instance_id, decoders, )?), @@ -1078,16 +1079,12 @@ where Ok(match decoders.get(module_instance_id) { Some(decoder) => { let total_len_u64 = u64::consensus_decode_from_finite_reader(reader, decoders)?; - let mut reader = reader.take(total_len_u64); - let v: T = decoder.decode(&mut reader, module_instance_id, decoders)?; - - if reader.limit() != 0 { - return Err(fedimint_core::encoding::DecodeError::new_custom( - anyhow::anyhow!("Dyn type did not consume all bytes during decoding"), - )); - } - - DynRawFallback::Decoded(v) + DynRawFallback::Decoded(decoder.decode_complete( + reader, + total_len_u64, + module_instance_id, + decoders, + )?) } None => { // since the decoder is not available, just read the raw data diff --git a/fedimint-core/src/macros.rs b/fedimint-core/src/macros.rs index 9076b0d4d12..d053e6bcb7d 100644 --- a/fedimint-core/src/macros.rs +++ b/fedimint-core/src/macros.rs @@ -313,34 +313,28 @@ macro_rules! module_plugin_dyn_newtype_encode_decode { impl Decodable for $name { fn consensus_decode_from_finite_reader( reader: &mut R, - modules: &$crate::module::registry::ModuleDecoderRegistry, + decoders: &$crate::module::registry::ModuleDecoderRegistry, ) -> Result { let module_instance_id = fedimint_core::core::ModuleInstanceId::consensus_decode_from_finite_reader( - reader, modules, + reader, decoders, )?; - let val = match modules.get(module_instance_id) { + let val = match decoders.get(module_instance_id) { Some(decoder) => { let total_len_u64 = - u64::consensus_decode_from_finite_reader(reader, modules)?; - let mut reader = std::io::Read::take(reader, total_len_u64); - let v = decoder.decode(&mut reader, module_instance_id, modules)?; - - if reader.limit() != 0 { - return Err(fedimint_core::encoding::DecodeError::new_custom( - anyhow::anyhow!( - "Dyn type did not consume all bytes during decoding" - ), - )); - } - - v + u64::consensus_decode_from_finite_reader(reader, decoders)?; + decoder.decode_complete( + reader, + total_len_u64, + module_instance_id, + decoders, + )? } - None => match modules.decoding_mode() { + None => match decoders.decoding_mode() { $crate::module::registry::DecodingMode::Reject => { return Err(fedimint_core::encoding::DecodeError::new_custom( anyhow::anyhow!( - "Module decoder not available: {module_instance_id}" + "Module decoder not available: {module_instance_id} when decoding {}", std::any::type_name::() ), )); } diff --git a/fedimint-core/src/module/mod.rs b/fedimint-core/src/module/mod.rs index 034551fae36..e465eab20de 100644 --- a/fedimint-core/src/module/mod.rs +++ b/fedimint-core/src/module/mod.rs @@ -16,7 +16,6 @@ pub mod registry; use std::collections::BTreeMap; use std::fmt::{self, Debug, Formatter}; -use std::io::Read; use std::marker::{self, PhantomData}; use std::pin::Pin; use std::sync::Arc; @@ -926,23 +925,9 @@ impl SerdeModuleEncoding { let total_len = u64::consensus_decode(&mut reader, &ModuleDecoderRegistry::default())?; - let mut reader = reader.take(total_len); - // No recursive module decoding is supported since we give an empty decoder // registry to the decode function - let val = decoder.decode( - &mut reader, - module_instance, - &ModuleDecoderRegistry::default(), - )?; - - if reader.limit() != 0 { - return Err(fedimint_core::encoding::DecodeError::new_custom( - anyhow::anyhow!("Dyn type did not consume all bytes during decoding"), - )); - } - - Ok(val) + decoder.decode_complete(&mut reader, total_len, module_instance, &Default::default()) } } diff --git a/fedimint-load-test-tool/src/common.rs b/fedimint-load-test-tool/src/common.rs index b476b3f2c8a..1026d531e21 100644 --- a/fedimint-load-test-tool/src/common.rs +++ b/fedimint-load-test-tool/src/common.rs @@ -149,10 +149,10 @@ pub async fn build_client( client_builder.with_module(WalletClientInit::default()); client_builder.with_primary_module(1); let client_secret = - Client::load_or_generate_client_secret(client_builder.db_no_encoders()).await?; + Client::load_or_generate_client_secret(client_builder.db_no_decoders()).await?; let root_secret = PlainRootSecretStrategy::to_root_secret(&client_secret); - let client = if !Client::is_initialized(client_builder.db_no_encoders()).await { + let client = if !Client::is_initialized(client_builder.db_no_decoders()).await { if let Some(invite_code) = &invite_code { let client_config = ClientConfig::download_from_invite_code(invite_code).await?; client_builder diff --git a/fedimint-testing/src/federation.rs b/fedimint-testing/src/federation.rs index a76b9981e21..2550d01cc75 100644 --- a/fedimint-testing/src/federation.rs +++ b/fedimint-testing/src/federation.rs @@ -80,7 +80,7 @@ impl FederationTest { let mut client_builder = Client::builder(db); client_builder.with_module_inits(self.client_init.clone()); client_builder.with_primary_module(self.primary_client); - let client_secret = Client::load_or_generate_client_secret(client_builder.db_no_encoders()) + let client_secret = Client::load_or_generate_client_secret(client_builder.db_no_decoders()) .await .unwrap(); client_builder diff --git a/fedimint-wasm-tests/src/lib.rs b/fedimint-wasm-tests/src/lib.rs index bf90f82b5f0..fb845cb6fe2 100644 --- a/fedimint-wasm-tests/src/lib.rs +++ b/fedimint-wasm-tests/src/lib.rs @@ -38,7 +38,7 @@ fn make_client_builder() -> fedimint_client::ClientBuilder { async fn client(invite_code: &InviteCode) -> Result { let client_config = ClientConfig::download_from_invite_code(invite_code).await?; let mut builder = make_client_builder(); - let client_secret = load_or_generate_mnemonic(builder.db_no_encoders()).await?; + let client_secret = load_or_generate_mnemonic(builder.db_no_decoders()).await?; builder.stopped(); builder .join( diff --git a/gateway/ln-gateway/src/client.rs b/gateway/ln-gateway/src/client.rs index 386705e3414..45aac8b0639 100644 --- a/gateway/ln-gateway/src/client.rs +++ b/gateway/ln-gateway/src/client.rs @@ -74,14 +74,14 @@ impl GatewayClientBuilder { client_builder.with_primary_module(self.primary_module); let client_secret = - match Client::load_decodable_client_secret::<[u8; 64]>(client_builder.db_no_encoders()) + match Client::load_decodable_client_secret::<[u8; 64]>(client_builder.db_no_decoders()) .await { Ok(secret) => secret, Err(_) => { info!("Generating secret and writing to client storage"); let secret = PlainRootSecretStrategy::random(&mut thread_rng()); - Client::store_encodable_client_secret(client_builder.db_no_encoders(), secret) + Client::store_encodable_client_secret(client_builder.db_no_decoders(), secret) .await .map_err(GatewayError::ClientStateMachineError)?; secret @@ -89,7 +89,7 @@ impl GatewayClientBuilder { }; let root_secret = PlainRootSecretStrategy::to_root_secret(&client_secret); - if Client::is_initialized(client_builder.db_no_encoders()).await { + if Client::is_initialized(client_builder.db_no_decoders()).await { client_builder // TODO: make this configurable? .open(root_secret)