From 24effd1a7c1d867377005ce7ad65534844081a5f Mon Sep 17 00:00:00 2001 From: Thoralf-M <46689931+Thoralf-M@users.noreply.github.com> Date: Fri, 18 Aug 2023 10:20:34 +0200 Subject: [PATCH] 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 | 5 + bindings/nodejs/CHANGELOG.md | 4 + sdk/CHANGELOG.md | 1 + .../operations/transaction/prepare_output.rs | 8 +- sdk/tests/wallet/output_preparation.rs | 102 +++++++++++++++++- 5 files changed, 116 insertions(+), 4 deletions(-) create mode 100644 .changes/prepare-output.md diff --git a/.changes/prepare-output.md b/.changes/prepare-output.md new file mode 100644 index 0000000000..66cd176c8e --- /dev/null +++ b/.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; \ No newline at end of file diff --git a/bindings/nodejs/CHANGELOG.md b/bindings/nodejs/CHANGELOG.md index d1b74c62f7..e566a3354d 100644 --- a/bindings/nodejs/CHANGELOG.md +++ b/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 diff --git a/sdk/CHANGELOG.md b/sdk/CHANGELOG.md index e2c1c8f462..d14d2a9ea5 100644 --- a/sdk/CHANGELOG.md +++ b/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 diff --git a/sdk/src/wallet/account/operations/transaction/prepare_output.rs b/sdk/src/wallet/account/operations/transaction/prepare_output.rs index 9550ac997c..7caccaec33 100644 --- a/sdk/src/wallet/account/operations/transaction/prepare_output.rs +++ b/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 = diff --git a/sdk/tests/wallet/output_preparation.rs b/sdk/tests/wallet/output_preparation.rs index 4d91794200..7037eaffa9 100644 --- a/sdk/tests/wallet/output_preparation.rs +++ b/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) +}