Fix prepare_output() with ReturnStrategy::Gift (#1029)

* Fix `Account::prepare_output()` when `ReturnStrategy::Gift` is used with an existing NFT output

* Use correct min_required_storage_deposit for each type

* Simplify and compare against correct value

* Add .changes file

.changes/prepare-output.md

@@ -0,0 +1,5 @@
+---
+"wallet-nodejs-binding": patch
+---
+
+Fixed `Account::prepareOutput()` when `ReturnStrategy::Gift` is used with an existing NFT output;

bindings/nodejs/CHANGELOG.md

@@ -30,6 +30,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Deprecate `Account::prepareVotingPower`;
 
+### Fixed
+
+- `Account::prepareOutput()` when `ReturnStrategy::Gift` is used with an existing NFT output;
+
 ## 1.0.4 - 2023-08-08
 
 ### Fixed

sdk/CHANGELOG.md

@@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - `Clients` returning the default protocol parameters when multiple `Client` instances are used;
 - Ledger Nano events properly created when preparing transactions using a `SecretManager`;
+- `Account::prepare_output()` when `ReturnStrategy::Gift` is used with an existing NFT output;
 
 ## 1.0.2 - 2023-07-28
 

sdk/src/wallet/account/operations/transaction/prepare_output.rs

@@ -110,7 +110,9 @@ where
         let min_storage_deposit_basic_output =
             MinimumStorageDepositBasicOutput::new(rent_structure, token_supply).finish()?;
 
-        if params.amount > min_storage_deposit_basic_output {
+        let min_required_storage_deposit = first_output.rent_cost(&rent_structure);
+
+        if params.amount > min_required_storage_deposit {
             second_output_builder = second_output_builder.with_amount(params.amount);
         }
 
@@ -121,9 +123,9 @@ where
             .return_strategy
             .unwrap_or_default();
         let remainder_address = self.get_remainder_address(transaction_options).await?;
-        if params.amount < min_storage_deposit_basic_output {
+        if params.amount < min_required_storage_deposit {
             if return_strategy == ReturnStrategy::Gift {
-                second_output_builder = second_output_builder.with_amount(min_storage_deposit_basic_output);
+                second_output_builder = second_output_builder.with_amount(min_required_storage_deposit);
             }
             if return_strategy == ReturnStrategy::Return {
                 second_output_builder =

sdk/tests/wallet/output_preparation.rs

@@ -6,7 +6,7 @@ use std::str::FromStr;
 use iota_sdk::{
     types::block::{
         address::{Address, Bech32Address, ToBech32Ext},
-        output::{MinimumStorageDepositBasicOutput, NativeToken, NftId, TokenId},
+        output::{MinimumStorageDepositBasicOutput, NativeToken, NftId, Output, Rent, TokenId},
     },
     wallet::{
         account::{Assets, Features, OutputParams, ReturnStrategy, StorageDeposit, Unlocks},
@@ -386,6 +386,39 @@ async fn output_preparation() -> Result<()> {
     // address, sdr, expiration
     assert_eq!(output.unlock_conditions().unwrap().len(), 3);
 
+    let output = account
+        .prepare_output(
+            OutputParams {
+                recipient_address,
+                amount: 42600,
+                assets: None,
+                features: Some(Features {
+                    metadata: Some(prefix_hex::encode(b"Large metadata".repeat(100))),
+                    tag: Some(prefix_hex::encode(b"My Tag")),
+                    issuer: None,
+                    sender: None,
+                }),
+                unlocks: None,
+                storage_deposit: Some(StorageDeposit {
+                    return_strategy: Some(ReturnStrategy::Return),
+                    use_excess_if_low: None,
+                }),
+            },
+            None,
+        )
+        .await?;
+    let rent_structure = wallet.client().get_rent_structure().await?;
+    let minimum_storage_deposit = output.rent_cost(&rent_structure);
+    assert_eq!(output.amount(), minimum_storage_deposit);
+    assert_eq!(output.amount(), 187900);
+    let sdr = output.unlock_conditions().unwrap().storage_deposit_return().unwrap();
+    assert_eq!(sdr.amount(), 145300);
+    // address and storage deposit unlock condition, because of the metadata feature block, 42600 is not enough for the
+    // required storage deposit
+    assert_eq!(output.unlock_conditions().unwrap().len(), 2);
+    // metadata and tag features
+    assert_eq!(output.features().unwrap().len(), 2);
+
     tear_down(storage_path)
 }
 
@@ -766,3 +799,70 @@ async fn prepare_output_only_single_nft() -> Result<()> {
 
     tear_down(storage_path)
 }
+
+#[ignore]
+#[tokio::test]
+async fn prepare_existing_nft_output_gift() -> Result<()> {
+    let storage_path = "test-storage/prepare_existing_nft_output_gift";
+    setup(storage_path)?;
+
+    let wallet = make_wallet(storage_path, None, None).await?;
+    let accounts = &create_accounts_with_funds(&wallet, 1).await?;
+    let addresses = accounts[0].addresses().await?;
+    let address = addresses[0].address();
+
+    let nft_options = [MintNftParams::new()
+        .with_address(*address)
+        .with_sender(*address)
+        .with_metadata(b"some nft metadata".to_vec())
+        .with_tag(b"some nft tag".to_vec())
+        .with_issuer(*address)
+        .with_immutable_metadata(b"some immutable nft metadata".to_vec())];
+
+    let transaction = accounts[0].mint_nfts(nft_options, None).await.unwrap();
+    accounts[0]
+        .retry_transaction_until_included(&transaction.transaction_id, None, None)
+        .await?;
+
+    let nft_id = *accounts[0].sync(None).await?.nfts().first().unwrap();
+
+    let nft = accounts[0]
+        .prepare_output(
+            OutputParams {
+                recipient_address: *address,
+                amount: 0,
+                assets: Some(Assets {
+                    native_tokens: None,
+                    nft_id: Some(nft_id),
+                }),
+                features: None,
+                unlocks: None,
+                storage_deposit: Some(StorageDeposit {
+                    return_strategy: Some(ReturnStrategy::Gift),
+                    use_excess_if_low: None,
+                }),
+            },
+            None,
+        )
+        .await?
+        .as_nft()
+        .clone();
+
+    let rent_structure = wallet.client().get_rent_structure().await?;
+    let minimum_storage_deposit = Output::Nft(nft.clone()).rent_cost(&rent_structure);
+    assert_eq!(nft.amount(), minimum_storage_deposit);
+
+    assert_eq!(nft.amount(), 52300);
+    assert_eq!(nft.address(), accounts[0].addresses().await?[0].address().as_ref());
+    assert!(nft.features().is_empty());
+    assert_eq!(
+        nft.immutable_features().metadata().unwrap().data(),
+        b"some immutable nft metadata"
+    );
+    assert_eq!(
+        nft.immutable_features().issuer().unwrap().address(),
+        accounts[0].addresses().await?[0].address().as_ref()
+    );
+
+    tear_down(storage_path)
+}