From 20f6454407acc1de75832f6a2c8334c09ec11bbb Mon Sep 17 00:00:00 2001 From: elsirion Date: Sat, 7 Dec 2024 17:29:52 +0100 Subject: [PATCH 1/3] feat: allow specifying primary module kind The primary module instance id cannot be known upfront, it may differ from federation to federation. --- fedimint-client/src/lib.rs | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/fedimint-client/src/lib.rs b/fedimint-client/src/lib.rs index de7d69ed9b9..75a598aaebe 100644 --- a/fedimint-client/src/lib.rs +++ b/fedimint-client/src/lib.rs @@ -2300,6 +2300,7 @@ pub struct AdminCreds { pub struct ClientBuilder { module_inits: ClientModuleInitRegistry, primary_module_instance: Option, + primary_module_kind: Option, admin_creds: Option, db_no_decoders: Database, meta_service: Arc, @@ -2358,6 +2359,10 @@ impl ClientBuilder { /// /// ## Panics /// If there was a primary module specified previously + #[deprecated( + since = "0.6.0", + note = "Use `with_primary_module_kind` instead, as the instance id can't be known upfront" + )] pub fn with_primary_module(&mut self, primary_module_instance: ModuleInstanceId) { let was_replaced = self .primary_module_instance @@ -2369,6 +2374,22 @@ impl ClientBuilder { ); } + /// Uses this module kind as the primary module if present in the config. + /// See [`ClientModule::supports_being_primary`] for more information. + /// + /// ## Panics + /// If there was a primary module kind specified previously + pub fn with_primary_module_kind(&mut self, primary_module_kind: ModuleKind) { + let was_replaced = self + .primary_module_kind + .replace(primary_module_kind) + .is_some(); + assert!( + !was_replaced, + "Only one primary module kind can be given to the builder." + ); + } + pub fn with_meta_service(&mut self, meta_service: Arc) { self.meta_service = meta_service; } @@ -2726,7 +2747,17 @@ impl ClientBuilder { let primary_module_instance = self .primary_module_instance - .ok_or(anyhow!("No primary module instance id was provided"))?; + .or_else(|| { + let primary_module_kind = self.primary_module_kind?; + config + .modules + .iter() + .find_map(|(module_instance_id, module_config)| { + (module_config.kind() == &primary_module_kind) + .then_some(*module_instance_id) + }) + }) + .ok_or(anyhow!("No primary module set or found"))?; let notifier = Notifier::new(db.clone()); From 81c82a4aab93e8e7b3cc3d31a0dd5973df7e1633 Mon Sep 17 00:00:00 2001 From: elsirion Date: Sat, 7 Dec 2024 18:04:22 +0100 Subject: [PATCH 2/3] chore: replace usage of deprecated `with_primary_module` --- fedimint-cli/src/lib.rs | 2 +- fedimint-client/src/lib.rs | 2 ++ fedimint-load-test-tool/src/common.rs | 2 +- fedimint-testing/src/federation.rs | 17 +++++++++-------- fedimint-testing/src/fixtures.rs | 5 ++++- fedimint-wasm-tests/src/lib.rs | 2 +- gateway/ln-gateway/src/client.rs | 10 +++++----- gateway/ln-gateway/src/lib.rs | 7 ++----- 8 files changed, 25 insertions(+), 22 deletions(-) diff --git a/fedimint-cli/src/lib.rs b/fedimint-cli/src/lib.rs index 6c140dfb198..a91e21d26fd 100644 --- a/fedimint-cli/src/lib.rs +++ b/fedimint-cli/src/lib.rs @@ -654,7 +654,7 @@ impl FedimintCli { let db = cli.load_rocks_db().await?; let mut client_builder = Client::builder(db).await.map_err_cli()?; client_builder.with_module_inits(self.module_inits.clone()); - client_builder.with_primary_module(1); + client_builder.with_primary_module_kind(fedimint_mint_client::KIND); #[cfg(feature = "tor")] if cli.use_tor { diff --git a/fedimint-client/src/lib.rs b/fedimint-client/src/lib.rs index 75a598aaebe..86eb1ce5a07 100644 --- a/fedimint-client/src/lib.rs +++ b/fedimint-client/src/lib.rs @@ -2317,6 +2317,7 @@ impl ClientBuilder { ClientBuilder { module_inits: ModuleInitRegistry::new(), primary_module_instance: None, + primary_module_kind: None, connector: Connector::default(), admin_creds: None, db_no_decoders: db, @@ -2330,6 +2331,7 @@ impl ClientBuilder { ClientBuilder { module_inits: client.module_inits.clone(), primary_module_instance: Some(client.primary_module_instance), + primary_module_kind: None, admin_creds: None, db_no_decoders: client.db.with_decoders(ModuleRegistry::default()), stopped: false, diff --git a/fedimint-load-test-tool/src/common.rs b/fedimint-load-test-tool/src/common.rs index 4a6f1e43b62..b997134fd7c 100644 --- a/fedimint-load-test-tool/src/common.rs +++ b/fedimint-load-test-tool/src/common.rs @@ -153,7 +153,7 @@ pub async fn build_client( client_builder.with_module(MintClientInit); client_builder.with_module(LightningClientInit::default()); client_builder.with_module(WalletClientInit::default()); - client_builder.with_primary_module(1); + client_builder.with_primary_module_kind(fedimint_mint_client::KIND); let client_secret = Client::load_or_generate_client_secret(client_builder.db_no_decoders()).await?; let root_secret = PlainRootSecretStrategy::to_root_secret(&client_secret); diff --git a/fedimint-testing/src/federation.rs b/fedimint-testing/src/federation.rs index df98b20aed2..9b5fe2a5a4e 100644 --- a/fedimint-testing/src/federation.rs +++ b/fedimint-testing/src/federation.rs @@ -12,7 +12,7 @@ use fedimint_core::config::{ ClientConfig, FederationId, ServerModuleConfigGenParamsRegistry, ServerModuleInitRegistry, META_FEDERATION_NAME_KEY, }; -use fedimint_core::core::ModuleInstanceId; +use fedimint_core::core::ModuleKind; use fedimint_core::db::mem_impl::MemDatabase; use fedimint_core::db::Database; use fedimint_core::endpoint_constants::SESSION_COUNT_ENDPOINT; @@ -37,7 +37,7 @@ pub struct FederationTest { configs: BTreeMap, server_init: ServerModuleInitRegistry, client_init: ClientModuleInitRegistry, - primary_client: ModuleInstanceId, + primary_module_kind: ModuleKind, _task: TaskGroup, } @@ -97,7 +97,7 @@ impl FederationTest { info!(target: LOG_TEST, "Setting new client with config"); let mut client_builder = Client::builder(db).await.expect("Failed to build client"); client_builder.with_module_inits(self.client_init.clone()); - client_builder.with_primary_module(self.primary_client); + client_builder.with_primary_module_kind(self.primary_module_kind.clone()); if let Some(admin_creds) = admin_creds { client_builder.set_admin_creds(admin_creds); } @@ -149,7 +149,7 @@ pub struct FederationTestBuilder { num_peers: u16, num_offline: u16, base_port: u16, - primary_client: ModuleInstanceId, + primary_module_kind: ModuleKind, version_hash: String, params: ServerModuleConfigGenParamsRegistry, server_init: ServerModuleInitRegistry, @@ -161,6 +161,7 @@ impl FederationTestBuilder { params: ServerModuleConfigGenParamsRegistry, server_init: ServerModuleInitRegistry, client_init: ClientModuleInitRegistry, + primary_module_kind: ModuleKind, ) -> FederationTestBuilder { let num_peers = 4; Self { @@ -168,7 +169,7 @@ impl FederationTestBuilder { num_offline: 1, base_port: block_in_place(|| fedimint_portalloc::port_alloc(num_peers * 2)) .expect("Failed to allocate a port range"), - primary_client: 0, + primary_module_kind, version_hash: "fedimint-testing-dummy-version-hash".to_owned(), params, server_init, @@ -191,8 +192,8 @@ impl FederationTestBuilder { self } - pub fn primary_client(mut self, primary_client: ModuleInstanceId) -> FederationTestBuilder { - self.primary_client = primary_client; + pub fn primary_module_kind(mut self, primary_module_kind: ModuleKind) -> FederationTestBuilder { + self.primary_module_kind = primary_module_kind; self } @@ -277,7 +278,7 @@ impl FederationTestBuilder { configs, server_init: self.server_init, client_init: self.client_init, - primary_client: self.primary_client, + primary_module_kind: self.primary_module_kind, _task: task_group, } } diff --git a/fedimint-testing/src/fixtures.rs b/fedimint-testing/src/fixtures.rs index 6970fcec39d..c8dfdd52ddb 100644 --- a/fedimint-testing/src/fixtures.rs +++ b/fedimint-testing/src/fixtures.rs @@ -48,6 +48,7 @@ pub struct Fixtures { bitcoin_rpc: BitcoinRpcConfig, bitcoin: Arc, dyn_bitcoin_rpc: DynBitcoindRpc, + primary_module_kind: ModuleKind, id: ModuleInstanceId, } @@ -101,6 +102,7 @@ impl Fixtures { bitcoin_rpc: config, bitcoin, dyn_bitcoin_rpc, + primary_module_kind: IClientModuleInit::module_kind(&client), id: 0, } .with_module(client, server, params) @@ -152,6 +154,7 @@ impl Fixtures { self.params.clone(), ServerModuleInitRegistry::from(self.servers.clone()), ClientModuleInitRegistry::from(self.clients.clone()), + self.primary_module_kind.clone(), ) } @@ -178,7 +181,7 @@ impl Fixtures { // Create federation client builder for the gateway let client_builder: GatewayClientBuilder = - GatewayClientBuilder::new(path.clone(), registry, 0); + GatewayClientBuilder::new(path.clone(), registry, ModuleKind::from_static_str("dummy")); let lightning_builder: Arc = Arc::new(FakeLightningBuilder); diff --git a/fedimint-wasm-tests/src/lib.rs b/fedimint-wasm-tests/src/lib.rs index b5a3073f88b..818c0f07bc8 100644 --- a/fedimint-wasm-tests/src/lib.rs +++ b/fedimint-wasm-tests/src/lib.rs @@ -31,7 +31,7 @@ async fn make_client_builder() -> Result { builder.with_module(LightningClientInit::default()); builder.with_module(MintClientInit); builder.with_module(WalletClientInit::default()); - builder.with_primary_module(1); + builder.with_primary_module_kind(fedimint_mint_client::KIND); Ok(builder) } diff --git a/gateway/ln-gateway/src/client.rs b/gateway/ln-gateway/src/client.rs index bb96de1f895..b57b89553bd 100644 --- a/gateway/ln-gateway/src/client.rs +++ b/gateway/ln-gateway/src/client.rs @@ -10,7 +10,7 @@ use fedimint_client::module::init::ClientModuleInitRegistry; use fedimint_client::secret::{PlainRootSecretStrategy, RootSecretStrategy}; use fedimint_client::{Client, ClientBuilder}; use fedimint_core::config::FederationId; -use fedimint_core::core::ModuleInstanceId; +use fedimint_core::core::ModuleKind; use fedimint_core::db::{Database, IDatabaseTransactionOpsCoreTyped}; use fedimint_core::encoding::Encodable; use fedimint_core::module::registry::ModuleDecoderRegistry; @@ -25,19 +25,19 @@ use crate::{AdminResult, Gateway}; pub struct GatewayClientBuilder { work_dir: PathBuf, registry: ClientModuleInitRegistry, - primary_module: ModuleInstanceId, + primary_module_kind: ModuleKind, } impl GatewayClientBuilder { pub fn new( work_dir: PathBuf, registry: ClientModuleInitRegistry, - primary_module: ModuleInstanceId, + primary_module_kind: ModuleKind, ) -> Self { Self { work_dir, registry, - primary_module, + primary_module_kind, } } @@ -85,7 +85,7 @@ impl GatewayClientBuilder { .await .map_err(AdminGatewayError::ClientCreationError)?; client_builder.with_module_inits(registry); - client_builder.with_primary_module(self.primary_module); + client_builder.with_primary_module_kind(self.primary_module_kind.clone()); client_builder.with_connector(connector); Ok(client_builder) } diff --git a/gateway/ln-gateway/src/lib.rs b/gateway/ln-gateway/src/lib.rs index 98022ae1402..9bd12a8d1b7 100644 --- a/gateway/ln-gateway/src/lib.rs +++ b/gateway/ln-gateway/src/lib.rs @@ -306,11 +306,8 @@ impl Gateway { decoders, ); - let client_builder = GatewayClientBuilder::new( - opts.data_dir.clone(), - registry, - LEGACY_HARDCODED_INSTANCE_ID_MINT, - ); + let client_builder = + GatewayClientBuilder::new(opts.data_dir.clone(), registry, fedimint_mint_client::KIND); info!( "Starting gatewayd (version: {})", From 95c21ec499c0f63bb26b0726a169b95a93d2fa3e Mon Sep 17 00:00:00 2001 From: elsirion Date: Mon, 9 Dec 2024 22:10:27 +0100 Subject: [PATCH 3/3] chore: introduce non-deprecated version of primary module by instance id selection --- fedimint-client/src/lib.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/fedimint-client/src/lib.rs b/fedimint-client/src/lib.rs index 86eb1ce5a07..c246cd05f5e 100644 --- a/fedimint-client/src/lib.rs +++ b/fedimint-client/src/lib.rs @@ -2363,9 +2363,26 @@ impl ClientBuilder { /// If there was a primary module specified previously #[deprecated( since = "0.6.0", - note = "Use `with_primary_module_kind` instead, as the instance id can't be known upfront" + note = "Use `with_primary_module_kind` instead, as the instance id can't be known upfront. If you *really* need the old behavior you can use `with_primary_module_instance_id`." )] pub fn with_primary_module(&mut self, primary_module_instance: ModuleInstanceId) { + self.with_primary_module_instance_id(primary_module_instance); + } + + /// **You are likely looking for + /// [`ClientBuilder::with_primary_module_kind`]. This function is rarely + /// useful and often dangerous, handle with care.** + /// + /// Uses this module with the given instance id as the primary module. See + /// [`ClientModule::supports_being_primary`] for more information. Since the + /// module instance id of modules of a specific kind may differ between + /// different federations it is generally not recommended to specify it, but + /// rather to specify the module kind that should be used as primary. See + /// [`ClientBuilder::with_primary_module_kind`]. + /// + /// ## Panics + /// If there was a primary module specified previously + pub fn with_primary_module_instance_id(&mut self, primary_module_instance: ModuleInstanceId) { let was_replaced = self .primary_module_instance .replace(primary_module_instance)