Remove ed25519 address generation from wallet (#1586)

* rm ed25519 address generation from wallet * rename fn * rm with_address * update bindings * handle builder address cases * unused import * re-add address to wallet options * fix tests * re-enable placeholder test * move tests * nodejs: address to wallet options * revert going through json file for address * revert unrelated change * Update sdk/tests/client/secret_manager/address_generation.rs Co-authored-by: Thoralf-M <[email protected]> * recipient Co-authored-by: DaughterOfMars <[email protected]> * bad wildcards * PR suggestion * import * example doc Co-authored-by: DaughterOfMars <[email protected]> * fix example * rename generate methods * Core: add SecretManager:GenerateEd25519AddressAsBech32 method * update examples and tests * Core: simplify SecretManager:GenerateEd25519AddressAsBech32 method * Python: update binding * nit * core: undo rename; update docs * add todo * Python: nits * NodeJs: add binding method * nit * sdk: undo rename; format * core: import * NodeJs: method suffix * fix test * nit * remove single address generation binding * Python: small fix --------- Co-authored-by: Thoralf-M <[email protected]> Co-authored-by: Thibault Martinez <[email protected]> Co-authored-by: DaughterOfMars <[email protected]> Co-authored-by: Thibault Martinez <[email protected]>
iotaledger · Mar 7, 2024 · 85d30ef · 85d30ef
1 parent ee51d3e
commit 85d30ef
Show file tree

Hide file tree

Showing 37 changed files with 380 additions and 463 deletions.
diff --git a/bindings/core/src/method/secret_manager.rs b/bindings/core/src/method/secret_manager.rs
@@ -19,7 +19,7 @@ use crate::OmittedDebug;
 #[serde(tag = "name", content = "data", rename_all = "camelCase")]
 #[non_exhaustive]
 pub enum SecretManagerMethod {
-    /// Generate Ed25519 addresses.
+    /// Generate multiple Ed25519 addresses at once.
     GenerateEd25519Addresses {
         /// Addresses generation options
         options: GetAddressesOptions,

diff --git a/bindings/core/src/method/wallet.rs b/bindings/core/src/method/wallet.rs
@@ -19,7 +19,6 @@ use iota_sdk::{
     client::{
         api::{transaction_builder::Burn, PreparedTransactionDataDto, SignedTransactionDataDto},
         node_manager::node::NodeAuth,
-        secret::GenerateAddressOptions,
     },
     types::block::{
         address::Hrp,
@@ -456,27 +455,11 @@ pub enum WalletMethod {
     /// Expected response: [`OutputsData`](crate::Response::OutputsData)
     #[serde(rename_all = "camelCase")]
     UnspentOutputs { filter_options: Option<FilterOptions> },
-
     /// Emits an event for testing if the event system is working
     /// Expected response: [`Ok`](crate::Response::Ok)
     #[cfg(feature = "events")]
     #[cfg_attr(docsrs, doc(cfg(feature = "events")))]
     EmitTestEvent { event: WalletEvent },
-
-    // TODO: reconsider whether to have the following methods on the wallet
-    /// Generate an address without storing it
-    /// Expected response: [`Bech32Address`](crate::Response::Bech32Address)
-    #[serde(rename_all = "camelCase")]
-    GenerateEd25519Address {
-        /// Account index
-        account_index: u32,
-        /// Account index
-        address_index: u32,
-        /// Options
-        options: Option<GenerateAddressOptions>,
-        /// Bech32 HRP
-        bech32_hrp: Option<Hrp>,
-    },
     /// Get the ledger nano status
     /// Expected response: [`LedgerNanoStatus`](crate::Response::LedgerNanoStatus)
     #[cfg(feature = "ledger_nano")]

diff --git a/bindings/core/src/method_handler/wallet.rs b/bindings/core/src/method_handler/wallet.rs
@@ -6,10 +6,7 @@ use std::time::Duration;
 use crypto::signatures::ed25519::PublicKey;
 use iota_sdk::{
     client::api::{PreparedTransactionData, SignedTransactionData, SignedTransactionDataDto},
-    types::{
-        block::{address::ToBech32Ext, output::feature::BlockIssuerKeySource},
-        TryFromDto,
-    },
+    types::{block::output::feature::BlockIssuerKeySource, TryFromDto},
     wallet::{types::TransactionWithMetadataDto, Wallet},
 };
 
@@ -73,23 +70,6 @@ pub(crate) async fn call_wallet_method_internal(
             let ledger_nano_status = wallet.get_ledger_nano_status().await?;
             Response::LedgerNanoStatus(ledger_nano_status)
         }
-        WalletMethod::GenerateEd25519Address {
-            account_index,
-            address_index,
-            options,
-            bech32_hrp,
-        } => {
-            let address = wallet
-                .generate_ed25519_address(account_index, address_index, options)
-                .await?;
-
-            let bech32_hrp = match bech32_hrp {
-                Some(bech32_hrp) => bech32_hrp,
-                None => *wallet.address().await.hrp(),
-            };
-
-            Response::Bech32Address(address.to_bech32(bech32_hrp))
-        }
         #[cfg(feature = "stronghold")]
         WalletMethod::SetStrongholdPassword { password } => {
             wallet.set_stronghold_password(password).await?;

diff --git a/bindings/nodejs/examples/how_tos/wallet/create-wallet.ts b/bindings/nodejs/examples/how_tos/wallet/create-wallet.ts
@@ -44,18 +44,7 @@ async function run() {
         // The mnemonic can't be retrieved from the Stronghold file, so make a backup in a secure place!
         await secretManager.storeMnemonic(process.env.MNEMONIC as string);
 
-        const walletAddress = await secretManager.generateEd25519Addresses({
-            coinType: CoinType.IOTA,
-            accountIndex: 0,
-            range: {
-                start: 0,
-                end: 1,
-            },
-            bech32Hrp: 'tst',
-        });
-
         const walletOptions: WalletOptions = {
-            address: walletAddress[0],
             storagePath: process.env.WALLET_DB_PATH,
             clientOptions: {
                 nodes: [process.env.NODE_URL as string],

diff --git a/bindings/nodejs/examples/wallet/getting-started.ts b/bindings/nodejs/examples/wallet/getting-started.ts
@@ -44,18 +44,7 @@ async function run() {
     // The mnemonic can't be retrieved from the Stronghold file, so make a backup in a secure place!
     await secretManager.storeMnemonic(mnemonic);
 
-    const wallet_address = await secretManager.generateEd25519Addresses({
-        coinType: CoinType.IOTA,
-        accountIndex: 0,
-        range: {
-            start: 0,
-            end: 1,
-        },
-        bech32Hrp: 'tst',
-    });
-
     const walletOptions: WalletOptions = {
-        address: wallet_address[0],
         storagePath: WALLET_DB_PATH,
         clientOptions: {
             nodes: [NODE_URL as string],

diff --git a/bindings/nodejs/lib/secret_manager/secret-manager.ts b/bindings/nodejs/lib/secret_manager/secret-manager.ts
@@ -21,6 +21,7 @@ import {
     UnsignedBlock,
     Block,
     parseBlock,
+    Bech32Address,
 } from '../types';
 
 import { plainToInstance } from 'class-transformer';
@@ -44,14 +45,14 @@ export class SecretManager {
     }
 
     /**
-     * Generate Ed25519 addresses.
+     * Generate multiple Ed25519 addresses at once.
      *
      * @param generateAddressesOptions Options to generate addresses.
      * @returns An array of generated addresses.
      */
     async generateEd25519Addresses(
         generateAddressesOptions: GenerateAddressesOptions,
-    ): Promise<string[]> {
+    ): Promise<Bech32Address[]> {
         const response = await this.methodHandler.callMethod({
             name: 'generateEd25519Addresses',
             data: {

diff --git a/bindings/nodejs/lib/types/client/generate-addresses-options.ts b/bindings/nodejs/lib/types/client/generate-addresses-options.ts
@@ -3,6 +3,8 @@
 import type { CoinType } from './constants';
 import type { Range } from './range';
 
+// TODO: Rename (to GetAddressOptions) and refactor (move out range field),
+// so we can use it for the single address generation method as well?
 /**
  * Input options for GenerateAddresses
  */

diff --git a/bindings/nodejs/src/wallet.rs b/bindings/nodejs/src/wallet.rs
@@ -121,7 +121,7 @@ pub async fn get_client(wallet: External<WalletMethodHandler>) -> Result<Externa
 #[napi(js_name = "getSecretManager")]
 pub async fn get_secret_manager(wallet: External<WalletMethodHandler>) -> Result<External<SecretManagerMethodHandler>> {
     if let Some(wallet) = &**wallet.as_ref().read().await {
-        Ok(External::new(wallet.get_secret_manager().clone()))
+        Ok(External::new(wallet.secret_manager().clone()))
     } else {
         Err(destroyed_err("Wallet"))
     }

diff --git a/bindings/nodejs/tests/secret_manager/secret_manager.spec.ts b/bindings/nodejs/tests/secret_manager/secret_manager.spec.ts
@@ -0,0 +1,48 @@
+// Copyright 2023 IOTA Stiftung
+// SPDX-License-Identifier: Apache-2.0
+
+import 'reflect-metadata';
+
+import { describe, it, expect } from '@jest/globals';
+import {
+    CoinType,
+    SecretManager,
+    Utils,
+} from '../../lib/';
+
+describe('SecretManager', () => {
+    it('generate IOTA Ed25519 address', async () => {
+        const mnemonicSecretManager = {
+            mnemonic: "acoustic trophy damage hint search taste love bicycle foster cradle brown govern endless depend situate athlete pudding blame question genius transfer van random vast"
+        };
+
+        let bech32_hrp = Utils.iotaMainnetProtocolParameters().bech32Hrp;
+
+        const secretManager = SecretManager.create(mnemonicSecretManager);
+        const addresses = await secretManager.generateEd25519Addresses({
+            coinType: CoinType.IOTA,
+            bech32Hrp: bech32_hrp,
+        });
+
+        expect(addresses[0]).toEqual('iota1qpg2xkj66wwgn8p2ggnp7p582gj8g6p79us5hve2tsudzpsr2ap4skprwjg');
+
+    }, 20000);
+
+    it('generate Shimmer Ed25519 address', async () => {
+        const mnemonicSecretManager = {
+            mnemonic: "acoustic trophy damage hint search taste love bicycle foster cradle brown govern endless depend situate athlete pudding blame question genius transfer van random vast"
+        };
+
+        let bech32_hrp = Utils.shimmerMainnetProtocolParameters().bech32Hrp;
+
+        const secretManager = SecretManager.create(mnemonicSecretManager);
+        const addresses = await secretManager.generateEd25519Addresses({
+            coinType: CoinType.Shimmer,
+            bech32Hrp: bech32_hrp,
+            range: { start: 0, end: 1 },
+        });
+
+        expect(addresses[0]).toEqual('smr1qzev36lk0gzld0k28fd2fauz26qqzh4hd4cwymlqlv96x7phjxcw6ckj80y');
+
+    }, 20000);
+});
diff --git a/bindings/python/iota_sdk/secret_manager/secret_manager.py b/bindings/python/iota_sdk/secret_manager/secret_manager.py
@@ -125,24 +125,69 @@ def _call_method(self, name, data=None):
             return json_response['payload']
         return response
 
+    # TODO: Should we include `bech32` in the method name?
+    def generate_ed25519_address(self,
+                                 coin_type: int,
+                                 bech32_hrp: str,
+                                 account_index: Optional[int] = None,
+                                 address_index: Optional[int] = None,
+                                 internal: Optional[bool] = None,
+                                 legder_nano_prompt: Optional[bool] = None):
+        """Generate a single Ed25519 address.
+
+        Args:
+            coin_type: The coin type to generate the address for.
+            bech32_hrp: The bech32 HRP (human readable part) to use.
+            account_index: An account index.
+            address_index: An address index.
+            internal: Whether the generated address should be internal.
+            ledger_nano_prompt: Whether to display the address on Ledger Nano devices.
+
+        Returns:
+            The generated Ed25519 address.
+        """
+
+        options = {}
+        options['coinType'] = coin_type
+        options['bech32Hrp'] = bech32_hrp
+        if address_index is not None:
+            options['range'] = {}
+            options['range']['start'] = address_index
+            options['range']['end'] = address_index + 1
+        if account_index is not None:
+            options['accountIndex'] = account_index
+        if internal is not None or legder_nano_prompt is not None:
+            options['options'] = {}
+            if internal is not None:
+                options['options']['internal'] = internal
+            if legder_nano_prompt is not None:
+                options['options']['ledgerNanoPrompot'] = legder_nano_prompt
+
+        return self._call_method('generateEd25519Addresses', {
+            'options': options
+        })[0]
+
     # pylint: disable=unused-argument
+
+    # TODO: Should `coin_type` and `bech32_hrp` be mandatory to provide?
+    # TODO: Should we include `bech32` in the method name?
     def generate_ed25519_addresses(self,
+                                   coin_type: Optional[int] = None,
+                                   bech32_hrp: Optional[str] = None,
                                    account_index: Optional[int] = None,
                                    start: Optional[int] = None,
                                    end: Optional[int] = None,
                                    internal: Optional[bool] = None,
-                                   coin_type: Optional[int] = None,
-                                   bech32_hrp: Optional[str] = None,
                                    ledger_nano_prompt: Optional[bool] = None):
-        """Generate Ed25519 addresses.
+        """Generate multiple Ed25519 addresses at once.
 
         Args:
+            coin_type: The coin type to generate addresses for.
+            bech32_hrp: The bech32 HRP (human readable part) to use.
             account_index: An account index.
             start: The start index of the addresses to generate.
             end: The end index of the addresses to generate.
             internal: Whether the generated addresses should be internal.
-            coin_type: The coin type to generate addresses for.
-            bech32_hrp: The bech32 HRP (human readable part) to use.
             ledger_nano_prompt: Whether to display the address on Ledger Nano devices.
 
         Returns:

diff --git a/bindings/python/src/wallet.rs b/bindings/python/src/wallet.rs
@@ -116,7 +116,7 @@ pub fn get_secret_manager_from_wallet(wallet: &Wallet) -> Result<SecretManager,
             .read()
             .await
             .as_ref()
-            .map(|w| w.get_secret_manager().clone())
+            .map(|w| w.secret_manager().clone())
             .ok_or_else(|| {
                 Error::from(
                     serde_json::to_string(&Response::Panic("wallet was destroyed".into()))

diff --git a/bindings/python/tests/test_secret_manager.py b/bindings/python/tests/test_secret_manager.py
@@ -0,0 +1,24 @@
+# Copyright 2023 IOTA Stiftung
+# SPDX-License-Identifier: Apache-2.0
+
+from iota_sdk import MnemonicSecretManager, SecretManager, CoinType, Utils
+
+
+def test_secret_manager_address_generation_iota():
+    secret_manager = SecretManager(MnemonicSecretManager(
+        "acoustic trophy damage hint search taste love bicycle foster cradle brown govern endless depend situate athlete pudding blame question genius transfer van random vast"))
+
+    bech32_hrp = Utils.iota_mainnet_protocol_parameters().bech32_hrp
+    address = secret_manager.generate_ed25519_address(CoinType.IOTA, bech32_hrp)
+
+    assert 'iota1qpg2xkj66wwgn8p2ggnp7p582gj8g6p79us5hve2tsudzpsr2ap4skprwjg' == address
+
+
+def test_secret_manager_address_generation_shimmer():
+    secret_manager = SecretManager(MnemonicSecretManager(
+        "acoustic trophy damage hint search taste love bicycle foster cradle brown govern endless depend situate athlete pudding blame question genius transfer van random vast"))
+
+    bech32_hrp = Utils.shimmer_mainnet_protocol_parameters().bech32_hrp
+    address = secret_manager.generate_ed25519_address(CoinType.SHIMMER, bech32_hrp)
+
+    assert 'smr1qzev36lk0gzld0k28fd2fauz26qqzh4hd4cwymlqlv96x7phjxcw6ckj80y' == address
diff --git a/...s/python/tests/address_generation_test.py → bindings/python/tests/test_wallet_address.py b/...s/python/tests/address_generation_test.py → bindings/python/tests/test_wallet_address.py
@@ -5,8 +5,8 @@
 from iota_sdk import Wallet, MnemonicSecretManager, CoinType, ClientOptions, WalletOptions, Bip44, Utils
 
 
-def test_address_generation_iota():
-    db_path = './test_address_generation_iota'
+def test_wallet_address_iota():
+    db_path = './test_wallet_address_iota'
     shutil.rmtree(db_path, ignore_errors=True)
 
     client_options = ClientOptions(
@@ -33,8 +33,8 @@ def test_address_generation_iota():
     shutil.rmtree(db_path, ignore_errors=True)
 
 
-def test_address_generation_shimmer():
-    db_path = './test_address_generation_shimmer'
+def test_wallet_address_shimmer():
+    db_path = './test_wallet_address_shimmer'
     shutil.rmtree(db_path, ignore_errors=True)
 
     client_options = ClientOptions(

diff --git a/bindings/wasm/src/wallet.rs b/bindings/wasm/src/wallet.rs
@@ -52,7 +52,7 @@ pub async fn get_client(method_handler: &WalletMethodHandler) -> Result<ClientMe
 #[wasm_bindgen(js_name = getSecretManager)]
 pub async fn get_secret_manager(method_handler: &WalletMethodHandler) -> Result<SecretManagerMethodHandler, JsError> {
     if let Some(wallet) = &*method_handler.0.read().await {
-        Ok(SecretManagerMethodHandler::new(wallet.get_secret_manager().clone()))
+        Ok(SecretManagerMethodHandler::new(wallet.secret_manager().clone()))
     } else {
         // Notify that the wallet was destroyed
         Err(destroyed_err("Wallet"))

diff --git a/cli/src/cli.rs b/cli/src/cli.rs
@@ -169,7 +169,7 @@ pub async fn new_wallet(cli: Cli) -> Result<Option<Wallet>, Error> {
         if storage_path.is_dir() {
             match Wallet::builder().with_storage_path(storage_path).finish().await {
                 Ok(wallet) => {
-                    let linked_secret_manager = match &mut *wallet.get_secret_manager().write().await {
+                    let linked_secret_manager = match &mut *wallet.secret_manager().write().await {
                         SecretManager::Stronghold(stronghold) => {
                             let snapshot_path = stronghold.snapshot_path().to_path_buf();
                             let snapshot_exists = snapshot_path.exists();

diff --git a/cli/src/wallet_cli/mod.rs b/cli/src/wallet_cli/mod.rs
@@ -1377,7 +1377,7 @@ pub enum PromptResponse {
 }
 
 async fn ensure_password(wallet: &Wallet) -> Result<(), Error> {
-    if matches!(*wallet.get_secret_manager().read().await, SecretManager::Stronghold(_))
+    if matches!(*wallet.secret_manager().read().await, SecretManager::Stronghold(_))
         && !wallet.is_stronghold_password_available().await?
     {
         let password = get_password("Stronghold password", false)?;

diff --git a/sdk/examples/client/02_address_balance.rs b/sdk/examples/client/02_address_balance.rs
@@ -11,8 +11,8 @@
 
 use iota_sdk::{
     client::{
-        api::GetAddressesOptions, node_api::indexer::query_parameters::BasicOutputQueryParameters,
-        secret::SecretManager, Client,
+        api::GetAddressesOptions, constants::SHIMMER_COIN_TYPE,
+        node_api::indexer::query_parameters::BasicOutputQueryParameters, secret::SecretManager, Client,
     },
     types::block::output::NativeTokensBuilder,
 };
@@ -36,14 +36,8 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
 
     // Generate the first address
     let first_address = secret_manager
-        .generate_ed25519_addresses(
-            GetAddressesOptions::from_client(&client)
-                .await?
-                .with_account_index(0)
-                .with_range(0..1),
-        )
-        .await?[0]
-        .clone();
+        .generate_ed25519_address(SHIMMER_COIN_TYPE, 0, 0, client.get_bech32_hrp().await?, None)
+        .await?;
 
     // Get output ids of outputs that can be controlled by this address without further unlock constraints
     let output_ids_response = client