From 421887ac6edcee12aede3b880ad3fe400c93771e Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Wed, 27 Sep 2023 15:37:25 +0200 Subject: [PATCH 01/17] Timelock/Expiration validation with a CommitmentInput --- sdk/src/types/block/slot/commitment_id.rs | 14 ++++++++++++++ sdk/tests/types/slot.rs | 7 ++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/sdk/src/types/block/slot/commitment_id.rs b/sdk/src/types/block/slot/commitment_id.rs index 9c8380e2b7..3a3a677fb5 100644 --- a/sdk/src/types/block/slot/commitment_id.rs +++ b/sdk/src/types/block/slot/commitment_id.rs @@ -1,7 +1,21 @@ // Copyright 2023 IOTA Stiftung // SPDX-License-Identifier: Apache-2.0 +use crate::types::block::slot::SlotIndex; + impl_id!(pub SlotCommitmentId, 40, "Identifier of a slot commitment."); +impl SlotCommitmentId { + pub fn index(&self) -> SlotIndex { + // PANIC: taking the last 8 bytes of 40 bytes is safe. + u64::from_le_bytes( + self.0[Self::LENGTH - core::mem::size_of::()..] + .try_into() + .unwrap(), + ) + .into() + } +} + #[cfg(feature = "serde")] string_serde_impl!(SlotCommitmentId); diff --git a/sdk/tests/types/slot.rs b/sdk/tests/types/slot.rs index b477e298d0..069411da21 100644 --- a/sdk/tests/types/slot.rs +++ b/sdk/tests/types/slot.rs @@ -5,7 +5,7 @@ use iota_sdk::types::block::slot::SlotCommitment; use packable::PackableExt; #[test] -fn slot_commitment_id() { +fn slot_commitment_id_index() { // Test from https://github.com/iotaledger/tips-draft/blob/tip46/tips/TIP-0046/tip-0046.md#slot-commitment-id-1 let slot_commitment_json = serde_json::json!({ @@ -30,10 +30,11 @@ fn slot_commitment_id() { ] ); - let slot_commitment_id = slot_commitment.id().to_string(); + let slot_commitment_id = slot_commitment.id(); assert_eq!( - slot_commitment_id, + slot_commitment_id.to_string(), "0x3a73079f3dbf8c1744ae0b020b9767546e32f5bbbf4c6f0233da7b64f16581f80a00000000000000" ); + assert_eq!(slot_commitment_id.index(), 10); } From 398e212ec59093f47fd6eae7ff6ebd729ef859a2 Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Wed, 27 Sep 2023 16:11:23 +0200 Subject: [PATCH 02/17] Oh no --- bindings/core/src/method/secret_manager.rs | 2 ++ .../core/src/method_handler/secret_manager.rs | 6 ++++- .../client/api/block_builder/transaction.rs | 3 +++ sdk/src/client/secret/ledger_nano.rs | 4 ++- sdk/src/client/secret/mnemonic.rs | 6 +++-- sdk/src/client/secret/mod.rs | 26 +++++++++++++++---- sdk/src/client/secret/private_key.rs | 6 +++-- sdk/src/client/stronghold/secret.rs | 6 +++-- .../types/block/context_input/commitment.rs | 13 +++++++--- sdk/src/types/block/context_input/mod.rs | 2 +- .../block/output/unlock_condition/timelock.rs | 4 +++ .../payload/transaction/essence/regular.rs | 1 + sdk/src/types/block/semantic.rs | 23 ++++++++++++++-- .../account/operations/transaction/mod.rs | 1 + sdk/tests/client/signing/account.rs | 12 ++++----- sdk/tests/client/signing/basic.rs | 12 ++++----- sdk/tests/client/signing/mod.rs | 4 +-- sdk/tests/client/signing/nft.rs | 4 +-- 18 files changed, 99 insertions(+), 36 deletions(-) diff --git a/bindings/core/src/method/secret_manager.rs b/bindings/core/src/method/secret_manager.rs index e4d548d052..64be35e1fe 100644 --- a/bindings/core/src/method/secret_manager.rs +++ b/bindings/core/src/method/secret_manager.rs @@ -5,6 +5,7 @@ use crypto::keys::bip44::Bip44; use derivative::Derivative; use iota_sdk::{ client::api::{GetAddressesOptions, PreparedTransactionDataDto}, + types::block::protocol::ProtocolParameters, utils::serde::bip44::Bip44Def, }; use serde::{Deserialize, Serialize}; @@ -60,6 +61,7 @@ pub enum SecretManagerMethod { SignTransaction { /// Prepared transaction data prepared_transaction_data: PreparedTransactionDataDto, + protocol_parameters: ProtocolParameters, }, /// Store a mnemonic in the Stronghold vault #[cfg(feature = "stronghold")] diff --git a/bindings/core/src/method_handler/secret_manager.rs b/bindings/core/src/method_handler/secret_manager.rs index ff26572766..48d17b81a3 100644 --- a/bindings/core/src/method_handler/secret_manager.rs +++ b/bindings/core/src/method_handler/secret_manager.rs @@ -37,9 +37,13 @@ pub(crate) async fn call_secret_manager_method_internal( } SecretManagerMethod::SignTransaction { prepared_transaction_data, + protocol_parameters, } => { let transaction = &secret_manager - .sign_transaction(PreparedTransactionData::try_from_dto(prepared_transaction_data)?) + .sign_transaction( + PreparedTransactionData::try_from_dto(prepared_transaction_data)?, + protocol_parameters, + ) .await?; Response::SignedTransaction(transaction.into()) } diff --git a/sdk/src/client/api/block_builder/transaction.rs b/sdk/src/client/api/block_builder/transaction.rs index 60c45a77e3..bc84c297a9 100644 --- a/sdk/src/client/api/block_builder/transaction.rs +++ b/sdk/src/client/api/block_builder/transaction.rs @@ -10,6 +10,7 @@ use crate::{ types::block::{ output::{Output, OutputId}, payload::transaction::{RegularTransactionEssence, TransactionPayload}, + protocol::ProtocolParameters, semantic::{semantic_validation, TransactionFailureReason, ValidationContext}, signature::Ed25519Signature, BlockId, BlockWrapper, @@ -29,6 +30,7 @@ const REFERENCE_ACCOUNT_NFT_UNLOCK_LENGTH: usize = 1 + 2; pub fn verify_semantic( input_signing_data: &[InputSigningData], transaction: &TransactionPayload, + protocol_parameters: ProtocolParameters, ) -> crate::client::Result> { let transaction_id = transaction.id(); let inputs = input_signing_data @@ -37,6 +39,7 @@ pub fn verify_semantic( .collect::>(); let context = ValidationContext::new( + protocol_parameters, &transaction_id, transaction.essence(), inputs.iter().map(|(id, input)| (*id, *input)), diff --git a/sdk/src/client/secret/ledger_nano.rs b/sdk/src/client/secret/ledger_nano.rs index 409ab571a0..e06a83c94c 100644 --- a/sdk/src/client/secret/ledger_nano.rs +++ b/sdk/src/client/secret/ledger_nano.rs @@ -30,6 +30,7 @@ use crate::{ address::{AccountAddress, Address, Ed25519Address, NftAddress}, output::Output, payload::transaction::{TransactionEssence, TransactionPayload}, + protocol::ProtocolParameters, signature::{Ed25519Signature, Signature}, unlock::{AccountUnlock, NftUnlock, ReferenceUnlock, SignatureUnlock, Unlock, Unlocks}, }, @@ -405,8 +406,9 @@ impl SecretManage for LedgerSecretManager { async fn sign_transaction( &self, prepared_transaction_data: PreparedTransactionData, + protocol_parameters: ProtocolParameters, ) -> Result { - super::default_sign_transaction(self, prepared_transaction_data).await + super::default_sign_transaction(self, prepared_transaction_data, protocol_parameters).await } } diff --git a/sdk/src/client/secret/mnemonic.rs b/sdk/src/client/secret/mnemonic.rs index 69062278e0..4607ca42e6 100644 --- a/sdk/src/client/secret/mnemonic.rs +++ b/sdk/src/client/secret/mnemonic.rs @@ -20,7 +20,8 @@ use super::{GenerateAddressOptions, SecretManage}; use crate::{ client::{api::PreparedTransactionData, Client, Error}, types::block::{ - address::Ed25519Address, payload::transaction::TransactionPayload, signature::Ed25519Signature, unlock::Unlocks, + address::Ed25519Address, payload::transaction::TransactionPayload, protocol::ProtocolParameters, + signature::Ed25519Signature, unlock::Unlocks, }, }; @@ -131,8 +132,9 @@ impl SecretManage for MnemonicSecretManager { async fn sign_transaction( &self, prepared_transaction_data: PreparedTransactionData, + protocol_parameters: ProtocolParameters, ) -> Result { - super::default_sign_transaction(self, prepared_transaction_data).await + super::default_sign_transaction(self, prepared_transaction_data, protocol_parameters).await } } diff --git a/sdk/src/client/secret/mod.rs b/sdk/src/client/secret/mod.rs index 67c58865e8..5876e40b12 100644 --- a/sdk/src/client/secret/mod.rs +++ b/sdk/src/client/secret/mod.rs @@ -55,6 +55,7 @@ use crate::{ address::{Address, Ed25519Address}, output::Output, payload::{transaction::TransactionEssence, TransactionPayload}, + protocol::ProtocolParameters, signature::{Ed25519Signature, Signature}, unlock::{AccountUnlock, NftUnlock, ReferenceUnlock, SignatureUnlock, Unlock, Unlocks}, }, @@ -110,6 +111,7 @@ pub trait SecretManage: Send + Sync { async fn sign_transaction( &self, prepared_transaction_data: PreparedTransactionData, + protocol_parameters: ProtocolParameters, ) -> Result; } @@ -425,15 +427,28 @@ impl SecretManage for SecretManager { async fn sign_transaction( &self, prepared_transaction_data: PreparedTransactionData, + protocol_parameters: ProtocolParameters, ) -> Result { match self { #[cfg(feature = "stronghold")] - Self::Stronghold(secret_manager) => Ok(secret_manager.sign_transaction(prepared_transaction_data).await?), + Self::Stronghold(secret_manager) => Ok(secret_manager + .sign_transaction(prepared_transaction_data, protocol_parameters) + .await?), #[cfg(feature = "ledger_nano")] - Self::LedgerNano(secret_manager) => Ok(secret_manager.sign_transaction(prepared_transaction_data).await?), - Self::Mnemonic(secret_manager) => secret_manager.sign_transaction(prepared_transaction_data).await, + Self::LedgerNano(secret_manager) => Ok(secret_manager + .sign_transaction(prepared_transaction_data, protocol_parameters) + .await?), + Self::Mnemonic(secret_manager) => { + secret_manager + .sign_transaction(prepared_transaction_data, protocol_parameters) + .await + } #[cfg(feature = "private_key_secret_manager")] - Self::PrivateKey(secret_manager) => secret_manager.sign_transaction(prepared_transaction_data).await, + Self::PrivateKey(secret_manager) => { + secret_manager + .sign_transaction(prepared_transaction_data, protocol_parameters) + .await + } Self::Placeholder => Err(Error::PlaceholderSecretManager), } } @@ -574,6 +589,7 @@ where pub(crate) async fn default_sign_transaction( secret_manager: &M, prepared_transaction_data: PreparedTransactionData, + protocol_parameters: ProtocolParameters, ) -> crate::client::Result where crate::client::Error: From, @@ -593,7 +609,7 @@ where validate_transaction_payload_length(&tx_payload)?; - let conflict = verify_semantic(&inputs_data, &tx_payload)?; + let conflict = verify_semantic(&inputs_data, &tx_payload, protocol_parameters)?; if let Some(conflict) = conflict { log::debug!("[sign_transaction] conflict: {conflict:?} for {:#?}", tx_payload); diff --git a/sdk/src/client/secret/private_key.rs b/sdk/src/client/secret/private_key.rs index 9622d16d40..e94cea6b34 100644 --- a/sdk/src/client/secret/private_key.rs +++ b/sdk/src/client/secret/private_key.rs @@ -20,7 +20,8 @@ use super::{GenerateAddressOptions, SecretManage}; use crate::{ client::{api::PreparedTransactionData, Error}, types::block::{ - address::Ed25519Address, payload::transaction::TransactionPayload, signature::Ed25519Signature, unlock::Unlocks, + address::Ed25519Address, payload::transaction::TransactionPayload, protocol::ProtocolParameters, + signature::Ed25519Signature, unlock::Unlocks, }, }; @@ -91,8 +92,9 @@ impl SecretManage for PrivateKeySecretManager { async fn sign_transaction( &self, prepared_transaction_data: PreparedTransactionData, + protocol_parameters: ProtocolParameters, ) -> Result { - super::default_sign_transaction(self, prepared_transaction_data).await + super::default_sign_transaction(self, prepared_transaction_data, protocol_parameters).await } } diff --git a/sdk/src/client/stronghold/secret.rs b/sdk/src/client/stronghold/secret.rs index 9167d87f4c..3ebf21402b 100644 --- a/sdk/src/client/stronghold/secret.rs +++ b/sdk/src/client/stronghold/secret.rs @@ -36,7 +36,8 @@ use crate::{ stronghold::Error, }, types::block::{ - address::Ed25519Address, payload::transaction::TransactionPayload, signature::Ed25519Signature, unlock::Unlocks, + address::Ed25519Address, payload::transaction::TransactionPayload, protocol::ProtocolParameters, + signature::Ed25519Signature, unlock::Unlocks, }, }; @@ -290,8 +291,9 @@ impl SecretManage for StrongholdAdapter { async fn sign_transaction( &self, prepared_transaction_data: PreparedTransactionData, + protocol_parameters: ProtocolParameters, ) -> Result { - crate::client::secret::default_sign_transaction(self, prepared_transaction_data).await + crate::client::secret::default_sign_transaction(self, prepared_transaction_data, protocol_parameters).await } } diff --git a/sdk/src/types/block/context_input/commitment.rs b/sdk/src/types/block/context_input/commitment.rs index 62503b3cbb..d8e729bb92 100644 --- a/sdk/src/types/block/context_input/commitment.rs +++ b/sdk/src/types/block/context_input/commitment.rs @@ -3,7 +3,7 @@ use derive_more::{Display, From}; -use crate::types::block::slot::SlotCommitmentId; +use crate::types::block::slot::{SlotCommitmentId, SlotIndex}; /// A Commitment Context Input references a commitment to a certain slot. #[derive(Clone, Copy, Display, Debug, Eq, PartialEq, Hash, Ord, PartialOrd, From, packable::Packable)] @@ -18,10 +18,15 @@ impl CommitmentContextInput { Self(commitment_id) } - /// Returns the commitment id of the [`CommitmentContextInput`]. - pub fn commitment_id(&self) -> SlotCommitmentId { + /// Returns the slot commitment id of the [`CommitmentContextInput`]. + pub fn slot_commitment_id(&self) -> SlotCommitmentId { self.0 } + + /// Returns the slot index of the [`CommitmentContextInput`]. + pub fn slot_index(&self) -> SlotIndex { + self.0.index() + } } #[cfg(feature = "serde")] @@ -43,7 +48,7 @@ mod dto { fn from(value: &CommitmentContextInput) -> Self { Self { kind: CommitmentContextInput::KIND, - commitment_id: value.commitment_id(), + commitment_id: value.slot_commitment_id(), } } } diff --git a/sdk/src/types/block/context_input/mod.rs b/sdk/src/types/block/context_input/mod.rs index 09bfeaab55..d2f83c4607 100644 --- a/sdk/src/types/block/context_input/mod.rs +++ b/sdk/src/types/block/context_input/mod.rs @@ -123,7 +123,7 @@ mod tests { .unwrap(); assert!(commitment.is_commitment()); assert_eq!( - commitment.as_commitment().commitment_id().to_string(), + commitment.as_commitment().slot_commitment_id().to_string(), "0xedf5f572c58ddf4b4f9567d82bf96689cc68b730df796d822b4b9fb643f5efda4f9567d82bf96689" ); diff --git a/sdk/src/types/block/output/unlock_condition/timelock.rs b/sdk/src/types/block/output/unlock_condition/timelock.rs index 9a93d65ddc..80dd96069d 100644 --- a/sdk/src/types/block/output/unlock_condition/timelock.rs +++ b/sdk/src/types/block/output/unlock_condition/timelock.rs @@ -29,6 +29,10 @@ impl TimelockUnlockCondition { pub fn slot_index(&self) -> SlotIndex { self.0 } + + pub fn is_time_locked(&self, commitment_index: SlotIndex, min_committable_age: SlotIndex) -> bool { + (commitment_index + min_committable_age) >= self.0 + } } #[inline] diff --git a/sdk/src/types/block/payload/transaction/essence/regular.rs b/sdk/src/types/block/payload/transaction/essence/regular.rs index c21dfb0ddd..dcad1977ab 100644 --- a/sdk/src/types/block/payload/transaction/essence/regular.rs +++ b/sdk/src/types/block/payload/transaction/essence/regular.rs @@ -312,6 +312,7 @@ fn verify_context_inputs(context_inputs: &[ContextInput]) -> Result<(), Error> { let mut reward_index_set = HashSet::new(); let mut bic_account_id_set = HashSet::new(); + for input in context_inputs.iter() { match input { ContextInput::BlockIssuanceCredit(bic) => { diff --git a/sdk/src/types/block/semantic.rs b/sdk/src/types/block/semantic.rs index b98b916bcb..c663b85327 100644 --- a/sdk/src/types/block/semantic.rs +++ b/sdk/src/types/block/semantic.rs @@ -11,6 +11,7 @@ use crate::types::block::{ address::Address, output::{ChainId, FoundryId, InputsCommitment, NativeTokens, Output, OutputId, TokenId}, payload::transaction::{RegularTransactionEssence, TransactionEssence, TransactionId}, + protocol::ProtocolParameters, unlock::Unlocks, Error, }; @@ -152,6 +153,8 @@ impl TryFrom for TransactionFailureReason { /// pub struct ValidationContext<'a> { + /// + protocol_parameters: ProtocolParameters, /// pub essence: &'a RegularTransactionEssence, /// @@ -183,12 +186,14 @@ pub struct ValidationContext<'a> { impl<'a> ValidationContext<'a> { /// pub fn new( + protocol_parameters: ProtocolParameters, transaction_id: &TransactionId, essence: &'a RegularTransactionEssence, inputs: impl Iterator + Clone, unlocks: &'a Unlocks, ) -> Self { Self { + protocol_parameters, essence, unlocks, essence_hash: TransactionEssence::from(essence.clone()).hash(), @@ -274,10 +279,24 @@ pub fn semantic_validation( return Ok(Some(conflict)); } - if unlock_conditions.is_time_locked(context.essence.creation_slot()) { - return Ok(Some(TransactionFailureReason::TimelockNotExpired)); + if let Some(timelock) = unlock_conditions.timelock() { + if let Some(commitment) = context.essence.context_inputs().iter().find(|c| c.is_commitment()) { + if timelock.is_time_locked( + commitment.as_commitment().slot_index(), + context.protocol_parameters.min_committable_age(), + ) { + return Ok(Some(TransactionFailureReason::TimelockNotExpired)); + } + } else { + // TODO return an error + } } + // TODO remove the method? + // if unlock_conditions.is_time_locked(context.essence.creation_slot()) { + // return Ok(Some(TransactionFailureReason::TimelockNotExpired)); + // } + if !unlock_conditions.is_expired(context.essence.creation_slot()) { if let Some(storage_deposit_return) = unlock_conditions.storage_deposit_return() { let amount = context diff --git a/sdk/src/wallet/account/operations/transaction/mod.rs b/sdk/src/wallet/account/operations/transaction/mod.rs index c3003ef4f8..75c93068d1 100644 --- a/sdk/src/wallet/account/operations/transaction/mod.rs +++ b/sdk/src/wallet/account/operations/transaction/mod.rs @@ -131,6 +131,7 @@ where let conflict = verify_semantic( &signed_transaction_data.inputs_data, &signed_transaction_data.transaction_payload, + self.client().get_protocol_parameters().await?, )?; if let Some(conflict) = conflict { diff --git a/sdk/tests/client/signing/account.rs b/sdk/tests/client/signing/account.rs index 5dbc140e31..81b84c5055 100644 --- a/sdk/tests/client/signing/account.rs +++ b/sdk/tests/client/signing/account.rs @@ -95,7 +95,7 @@ async fn sign_account_state_transition() -> Result<()> { ) .with_outputs(outputs) .add_mana_allotment(rand_mana_allotment(&protocol_parameters)) - .finish_with_params(protocol_parameters)?, + .finish_with_params(protocol_parameters.clone())?, ); let prepared_transaction_data = PreparedTransactionData { @@ -115,7 +115,7 @@ async fn sign_account_state_transition() -> Result<()> { validate_transaction_payload_length(&tx_payload)?; - let conflict = verify_semantic(&prepared_transaction_data.inputs_data, &tx_payload)?; + let conflict = verify_semantic(&prepared_transaction_data.inputs_data, &tx_payload, protocol_parameters)?; if let Some(conflict) = conflict { panic!("{conflict:?}, with {tx_payload:#?}"); @@ -185,7 +185,7 @@ async fn sign_account_governance_transition() -> Result<()> { ) .with_outputs(outputs) .add_mana_allotment(rand_mana_allotment(&protocol_parameters)) - .finish_with_params(protocol_parameters)?, + .finish_with_params(protocol_parameters.clone())?, ); let prepared_transaction_data = PreparedTransactionData { @@ -205,7 +205,7 @@ async fn sign_account_governance_transition() -> Result<()> { validate_transaction_payload_length(&tx_payload)?; - let conflict = verify_semantic(&prepared_transaction_data.inputs_data, &tx_payload)?; + let conflict = verify_semantic(&prepared_transaction_data.inputs_data, &tx_payload, protocol_parameters)?; if let Some(conflict) = conflict { panic!("{conflict:?}, with {tx_payload:#?}"); @@ -315,7 +315,7 @@ async fn account_reference_unlocks() -> Result<()> { ) .with_outputs(outputs) .add_mana_allotment(rand_mana_allotment(&protocol_parameters)) - .finish_with_params(protocol_parameters)?, + .finish_with_params(protocol_parameters.clone())?, ); let prepared_transaction_data = PreparedTransactionData { @@ -347,7 +347,7 @@ async fn account_reference_unlocks() -> Result<()> { validate_transaction_payload_length(&tx_payload)?; - let conflict = verify_semantic(&prepared_transaction_data.inputs_data, &tx_payload)?; + let conflict = verify_semantic(&prepared_transaction_data.inputs_data, &tx_payload, protocol_parameters)?; if let Some(conflict) = conflict { panic!("{conflict:?}, with {tx_payload:#?}"); diff --git a/sdk/tests/client/signing/basic.rs b/sdk/tests/client/signing/basic.rs index 908d296179..44a4bfd1dd 100644 --- a/sdk/tests/client/signing/basic.rs +++ b/sdk/tests/client/signing/basic.rs @@ -78,7 +78,7 @@ async fn single_ed25519_unlock() -> Result<()> { ) .with_outputs(outputs) .add_mana_allotment(rand_mana_allotment(&protocol_parameters)) - .finish_with_params(protocol_parameters)?, + .finish_with_params(protocol_parameters.clone())?, ); let prepared_transaction_data = PreparedTransactionData { @@ -98,7 +98,7 @@ async fn single_ed25519_unlock() -> Result<()> { validate_transaction_payload_length(&tx_payload)?; - let conflict = verify_semantic(&prepared_transaction_data.inputs_data, &tx_payload)?; + let conflict = verify_semantic(&prepared_transaction_data.inputs_data, &tx_payload, protocol_parameters)?; if let Some(conflict) = conflict { panic!("{conflict:?}, with {tx_payload:#?}"); @@ -179,7 +179,7 @@ async fn ed25519_reference_unlocks() -> Result<()> { ) .with_outputs(outputs) .add_mana_allotment(rand_mana_allotment(&protocol_parameters)) - .finish_with_params(protocol_parameters)?, + .finish_with_params(protocol_parameters.clone())?, ); let prepared_transaction_data = PreparedTransactionData { @@ -211,7 +211,7 @@ async fn ed25519_reference_unlocks() -> Result<()> { validate_transaction_payload_length(&tx_payload)?; - let conflict = verify_semantic(&prepared_transaction_data.inputs_data, &tx_payload)?; + let conflict = verify_semantic(&prepared_transaction_data.inputs_data, &tx_payload, protocol_parameters)?; if let Some(conflict) = conflict { panic!("{conflict:?}, with {tx_payload:#?}"); @@ -290,7 +290,7 @@ async fn two_signature_unlocks() -> Result<()> { ) .with_outputs(outputs) .add_mana_allotment(rand_mana_allotment(&protocol_parameters)) - .finish_with_params(protocol_parameters)?, + .finish_with_params(protocol_parameters.clone())?, ); let prepared_transaction_data = PreparedTransactionData { @@ -311,7 +311,7 @@ async fn two_signature_unlocks() -> Result<()> { validate_transaction_payload_length(&tx_payload)?; - let conflict = verify_semantic(&prepared_transaction_data.inputs_data, &tx_payload)?; + let conflict = verify_semantic(&prepared_transaction_data.inputs_data, &tx_payload, protocol_parameters)?; if let Some(conflict) = conflict { panic!("{conflict:?}, with {tx_payload:#?}"); diff --git a/sdk/tests/client/signing/mod.rs b/sdk/tests/client/signing/mod.rs index 95e1497fbd..05b7a80927 100644 --- a/sdk/tests/client/signing/mod.rs +++ b/sdk/tests/client/signing/mod.rs @@ -385,7 +385,7 @@ async fn all_combined() -> Result<()> { .with_outputs(outputs) .with_creation_slot(slot_index) .add_mana_allotment(rand_mana_allotment(&protocol_parameters)) - .finish_with_params(protocol_parameters)?, + .finish_with_params(protocol_parameters.clone())?, ); let prepared_transaction_data = PreparedTransactionData { @@ -480,7 +480,7 @@ async fn all_combined() -> Result<()> { validate_transaction_payload_length(&tx_payload)?; - let conflict = verify_semantic(&prepared_transaction_data.inputs_data, &tx_payload)?; + let conflict = verify_semantic(&prepared_transaction_data.inputs_data, &tx_payload, protocol_parameters)?; if let Some(conflict) = conflict { panic!("{conflict:?}, with {tx_payload:#?}"); diff --git a/sdk/tests/client/signing/nft.rs b/sdk/tests/client/signing/nft.rs index 5473d5bed8..17c6f4ec7a 100644 --- a/sdk/tests/client/signing/nft.rs +++ b/sdk/tests/client/signing/nft.rs @@ -122,7 +122,7 @@ async fn nft_reference_unlocks() -> Result<()> { ) .with_outputs(outputs) .add_mana_allotment(rand_mana_allotment(&protocol_parameters)) - .finish_with_params(protocol_parameters)?, + .finish_with_params(protocol_parameters.clone())?, ); let prepared_transaction_data = PreparedTransactionData { @@ -154,7 +154,7 @@ async fn nft_reference_unlocks() -> Result<()> { validate_transaction_payload_length(&tx_payload)?; - let conflict = verify_semantic(&prepared_transaction_data.inputs_data, &tx_payload)?; + let conflict = verify_semantic(&prepared_transaction_data.inputs_data, &tx_payload, protocol_parameters)?; if let Some(conflict) = conflict { panic!("{conflict:?}, with {tx_payload:#?}"); From ba69a7d32acaa9c1fb843e973e74c2fc5321ef9a Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Wed, 27 Sep 2023 17:39:37 +0200 Subject: [PATCH 03/17] Correct is_expired --- .../output/unlock_condition/expiration.rs | 5 +++ .../block/output/unlock_condition/timelock.rs | 4 +- sdk/src/types/block/semantic.rs | 43 ++++++++++++++----- 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/sdk/src/types/block/output/unlock_condition/expiration.rs b/sdk/src/types/block/output/unlock_condition/expiration.rs index 185de6eb6a..26635f3449 100644 --- a/sdk/src/types/block/output/unlock_condition/expiration.rs +++ b/sdk/src/types/block/output/unlock_condition/expiration.rs @@ -45,7 +45,12 @@ impl ExpirationUnlockCondition { self.slot_index } + pub fn is_expired(&self, slot_index: SlotIndex, min_committable_age: SlotIndex) -> bool { + (slot_index + min_committable_age) >= self.slot_index + } + /// Returns the return address if the condition has expired. + /// TODO needs to be updated pub fn return_address_expired(&self, slot_index: SlotIndex) -> Option<&Address> { if slot_index >= self.slot_index() { Some(&self.return_address) diff --git a/sdk/src/types/block/output/unlock_condition/timelock.rs b/sdk/src/types/block/output/unlock_condition/timelock.rs index 80dd96069d..70764b7bd1 100644 --- a/sdk/src/types/block/output/unlock_condition/timelock.rs +++ b/sdk/src/types/block/output/unlock_condition/timelock.rs @@ -30,8 +30,8 @@ impl TimelockUnlockCondition { self.0 } - pub fn is_time_locked(&self, commitment_index: SlotIndex, min_committable_age: SlotIndex) -> bool { - (commitment_index + min_committable_age) >= self.0 + pub fn is_time_locked(&self, slot_index: SlotIndex, min_committable_age: SlotIndex) -> bool { + (slot_index + min_committable_age) < self.0 } } diff --git a/sdk/src/types/block/semantic.rs b/sdk/src/types/block/semantic.rs index c663b85327..a09961bc5d 100644 --- a/sdk/src/types/block/semantic.rs +++ b/sdk/src/types/block/semantic.rs @@ -297,19 +297,42 @@ pub fn semantic_validation( // return Ok(Some(TransactionFailureReason::TimelockNotExpired)); // } - if !unlock_conditions.is_expired(context.essence.creation_slot()) { - if let Some(storage_deposit_return) = unlock_conditions.storage_deposit_return() { - let amount = context - .storage_deposit_returns - .entry(*storage_deposit_return.return_address()) - .or_default(); - - *amount = amount - .checked_add(storage_deposit_return.amount()) - .ok_or(Error::StorageDepositReturnOverflow)?; + if let Some(expiration) = unlock_conditions.expiration() { + if let Some(commitment) = context.essence.context_inputs().iter().find(|c| c.is_commitment()) { + if !expiration.is_expired( + commitment.as_commitment().slot_index(), + context.protocol_parameters.min_committable_age(), + ) { + if let Some(storage_deposit_return) = unlock_conditions.storage_deposit_return() { + let amount = context + .storage_deposit_returns + .entry(*storage_deposit_return.return_address()) + .or_default(); + + *amount = amount + .checked_add(storage_deposit_return.amount()) + .ok_or(Error::StorageDepositReturnOverflow)?; + } + } + } else { + // TODO return an error } } + // TODO remove the method? + // if !unlock_conditions.is_expired(context.essence.creation_slot()) { + // if let Some(storage_deposit_return) = unlock_conditions.storage_deposit_return() { + // let amount = context + // .storage_deposit_returns + // .entry(*storage_deposit_return.return_address()) + // .or_default(); + + // *amount = amount + // .checked_add(storage_deposit_return.amount()) + // .ok_or(Error::StorageDepositReturnOverflow)?; + // } + // } + context.input_amount = context .input_amount .checked_add(amount) From 2f30bda348fe8079c1ef642097bf62c5057efb85 Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Wed, 27 Sep 2023 17:52:20 +0200 Subject: [PATCH 04/17] TODO --- sdk/src/types/block/output/unlock_condition/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/src/types/block/output/unlock_condition/mod.rs b/sdk/src/types/block/output/unlock_condition/mod.rs index de473e337f..e84a0f5b4d 100644 --- a/sdk/src/types/block/output/unlock_condition/mod.rs +++ b/sdk/src/types/block/output/unlock_condition/mod.rs @@ -426,6 +426,7 @@ impl UnlockConditions { } /// Returns whether a time lock exists and is still relevant. + /// TODO take expiration deadzone into account #[inline(always)] pub fn is_time_locked(&self, slot_index: impl Into) -> bool { let slot_index = slot_index.into(); From c5fd2fb67f1cc8df87a35bb474e4d22ccb4ffe5d Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Wed, 27 Sep 2023 17:54:43 +0200 Subject: [PATCH 05/17] meh --- sdk/src/types/block/output/unlock_condition/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/types/block/output/unlock_condition/mod.rs b/sdk/src/types/block/output/unlock_condition/mod.rs index e84a0f5b4d..078a3b2ae5 100644 --- a/sdk/src/types/block/output/unlock_condition/mod.rs +++ b/sdk/src/types/block/output/unlock_condition/mod.rs @@ -418,6 +418,7 @@ impl UnlockConditions { } /// Returns the address to be unlocked. + /// TODO take expiration deadzone into account #[inline(always)] pub fn locked_address<'a>(&'a self, address: &'a Address, slot_index: SlotIndex) -> &'a Address { self.expiration() @@ -426,7 +427,6 @@ impl UnlockConditions { } /// Returns whether a time lock exists and is still relevant. - /// TODO take expiration deadzone into account #[inline(always)] pub fn is_time_locked(&self, slot_index: impl Into) -> bool { let slot_index = slot_index.into(); From 3cb8b4680d689769c1547f3f2c4d8ffd72e05f09 Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Thu, 28 Sep 2023 09:38:35 +0200 Subject: [PATCH 06/17] TODO --- sdk/src/types/block/semantic.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/src/types/block/semantic.rs b/sdk/src/types/block/semantic.rs index a09961bc5d..f7bae5a691 100644 --- a/sdk/src/types/block/semantic.rs +++ b/sdk/src/types/block/semantic.rs @@ -299,6 +299,7 @@ pub fn semantic_validation( if let Some(expiration) = unlock_conditions.expiration() { if let Some(commitment) = context.essence.context_inputs().iter().find(|c| c.is_commitment()) { + // TODO check is_deadzoned ? if !expiration.is_expired( commitment.as_commitment().slot_index(), context.protocol_parameters.min_committable_age(), From d40937aee5c85d58291e8eb94f05a71d0e526e61 Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Thu, 28 Sep 2023 13:08:31 +0200 Subject: [PATCH 07/17] Nits --- .../api/block_builder/input_selection/mod.rs | 2 +- .../block/output/unlock_condition/mod.rs | 36 +++++++++---------- .../block/output/unlock_condition/timelock.rs | 4 +-- sdk/src/types/block/semantic.rs | 2 +- .../wallet/account/operations/helpers/time.rs | 4 +-- .../operations/output_consolidation.rs | 2 +- sdk/tests/wallet/output_preparation.rs | 2 +- 7 files changed, 26 insertions(+), 26 deletions(-) diff --git a/sdk/src/client/api/block_builder/input_selection/mod.rs b/sdk/src/client/api/block_builder/input_selection/mod.rs index 4baa80fc55..6489202ae4 100644 --- a/sdk/src/client/api/block_builder/input_selection/mod.rs +++ b/sdk/src/client/api/block_builder/input_selection/mod.rs @@ -238,7 +238,7 @@ impl InputSelection { // PANIC: safe to unwrap as non basic/account/foundry/nft outputs are already filtered out. let unlock_conditions = input.output.unlock_conditions().unwrap(); - if unlock_conditions.is_time_locked(self.slot_index) { + if unlock_conditions.is_timelocked(self.slot_index) { return false; } diff --git a/sdk/src/types/block/output/unlock_condition/mod.rs b/sdk/src/types/block/output/unlock_condition/mod.rs index 078a3b2ae5..e5404d68eb 100644 --- a/sdk/src/types/block/output/unlock_condition/mod.rs +++ b/sdk/src/types/block/output/unlock_condition/mod.rs @@ -389,6 +389,15 @@ impl UnlockConditions { .map(UnlockCondition::as_timelock) } + /// Returns whether a timelock exists and is still relevant. + #[inline(always)] + pub fn is_timelocked(&self, slot_index: impl Into) -> bool { + let slot_index = slot_index.into(); + + self.timelock() + .map_or(false, |timelock| slot_index < timelock.slot_index()) + } + /// Gets a reference to an [`ExpirationUnlockCondition`], if any. #[inline(always)] pub fn expiration(&self) -> Option<&ExpirationUnlockCondition> { @@ -396,6 +405,15 @@ impl UnlockConditions { .map(UnlockCondition::as_expiration) } + /// Returns whether an expiration exists and is expired. + #[inline(always)] + pub fn is_expired(&self, slot_index: impl Into) -> bool { + let slot_index = slot_index.into(); + + self.expiration() + .map_or(false, |expiration| slot_index >= expiration.slot_index()) + } + /// Gets a reference to a [`StateControllerAddressUnlockCondition`], if any. #[inline(always)] pub fn state_controller_address(&self) -> Option<&StateControllerAddressUnlockCondition> { @@ -425,24 +443,6 @@ impl UnlockConditions { .and_then(|e| e.return_address_expired(slot_index)) .unwrap_or(address) } - - /// Returns whether a time lock exists and is still relevant. - #[inline(always)] - pub fn is_time_locked(&self, slot_index: impl Into) -> bool { - let slot_index = slot_index.into(); - - self.timelock() - .map_or(false, |timelock| slot_index < timelock.slot_index()) - } - - /// Returns whether an expiration exists and is expired. - #[inline(always)] - pub fn is_expired(&self, slot_index: impl Into) -> bool { - let slot_index = slot_index.into(); - - self.expiration() - .map_or(false, |expiration| slot_index >= expiration.slot_index()) - } } #[inline] diff --git a/sdk/src/types/block/output/unlock_condition/timelock.rs b/sdk/src/types/block/output/unlock_condition/timelock.rs index 70764b7bd1..e4597623e7 100644 --- a/sdk/src/types/block/output/unlock_condition/timelock.rs +++ b/sdk/src/types/block/output/unlock_condition/timelock.rs @@ -30,8 +30,8 @@ impl TimelockUnlockCondition { self.0 } - pub fn is_time_locked(&self, slot_index: SlotIndex, min_committable_age: SlotIndex) -> bool { - (slot_index + min_committable_age) < self.0 + pub fn is_timelocked(&self, slot_index: impl Into, min_committable_age: impl Into) -> bool { + (slot_index.into() + min_committable_age.into()) < self.0 } } diff --git a/sdk/src/types/block/semantic.rs b/sdk/src/types/block/semantic.rs index f7bae5a691..3ef29f4eb9 100644 --- a/sdk/src/types/block/semantic.rs +++ b/sdk/src/types/block/semantic.rs @@ -281,7 +281,7 @@ pub fn semantic_validation( if let Some(timelock) = unlock_conditions.timelock() { if let Some(commitment) = context.essence.context_inputs().iter().find(|c| c.is_commitment()) { - if timelock.is_time_locked( + if timelock.is_timelocked( commitment.as_commitment().slot_index(), context.protocol_parameters.min_committable_age(), ) { diff --git a/sdk/src/wallet/account/operations/helpers/time.rs b/sdk/src/wallet/account/operations/helpers/time.rs index 271b42929d..185a6a1900 100644 --- a/sdk/src/wallet/account/operations/helpers/time.rs +++ b/sdk/src/wallet/account/operations/helpers/time.rs @@ -22,7 +22,7 @@ pub(crate) fn can_output_be_unlocked_now( account_transition: Option, ) -> crate::wallet::Result { if let Some(unlock_conditions) = output_data.output.unlock_conditions() { - if unlock_conditions.is_time_locked(slot_index) { + if unlock_conditions.is_timelocked(slot_index) { return Ok(false); } } @@ -47,7 +47,7 @@ pub(crate) fn can_output_be_unlocked_forever_from_now_on( slot_index: SlotIndex, ) -> bool { if let Some(unlock_conditions) = output.unlock_conditions() { - if unlock_conditions.is_time_locked(slot_index) { + if unlock_conditions.is_timelocked(slot_index) { return false; } diff --git a/sdk/src/wallet/account/operations/output_consolidation.rs b/sdk/src/wallet/account/operations/output_consolidation.rs index d27feae1ce..24e2095577 100644 --- a/sdk/src/wallet/account/operations/output_consolidation.rs +++ b/sdk/src/wallet/account/operations/output_consolidation.rs @@ -83,7 +83,7 @@ where Ok(if let Output::Basic(basic_output) = &output_data.output { let unlock_conditions = basic_output.unlock_conditions(); - let is_time_locked = unlock_conditions.is_time_locked(slot_index); + let is_time_locked = unlock_conditions.is_timelocked(slot_index); if is_time_locked { // If the output is timelocked, then it cannot be consolidated. return Ok(false); diff --git a/sdk/tests/wallet/output_preparation.rs b/sdk/tests/wallet/output_preparation.rs index 37e36ba2af..c0258b44b0 100644 --- a/sdk/tests/wallet/output_preparation.rs +++ b/sdk/tests/wallet/output_preparation.rs @@ -352,7 +352,7 @@ async fn output_preparation() -> Result<()> { assert_eq!(sender_feature.address(), issuer_and_sender_address.inner()); // Unlocks let conditions = output.unlock_conditions().unwrap(); - assert!(conditions.is_time_locked(0)); + assert!(conditions.is_timelocked(0)); assert!(conditions.is_expired(2)); // nft with expiration From da5a0a5c80db3fb75fc2ea83513c2003d6b252a7 Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Thu, 28 Sep 2023 13:21:34 +0200 Subject: [PATCH 08/17] Fix is_timelocked --- .../api/block_builder/input_selection/mod.rs | 2 +- .../block/output/unlock_condition/mod.rs | 11 +++++------ .../block/output/unlock_condition/timelock.rs | 1 + sdk/src/types/block/semantic.rs | 5 ----- sdk/src/wallet/account/operations/balance.rs | 6 ++++-- .../wallet/account/operations/helpers/time.rs | 6 ++++-- .../account/operations/output_claiming.rs | 2 ++ .../operations/output_consolidation.rs | 19 ++++++++++++++----- .../operations/transaction/input_selection.rs | 3 +++ sdk/tests/wallet/output_preparation.rs | 3 ++- 10 files changed, 36 insertions(+), 22 deletions(-) diff --git a/sdk/src/client/api/block_builder/input_selection/mod.rs b/sdk/src/client/api/block_builder/input_selection/mod.rs index 6489202ae4..ef7dde8e16 100644 --- a/sdk/src/client/api/block_builder/input_selection/mod.rs +++ b/sdk/src/client/api/block_builder/input_selection/mod.rs @@ -238,7 +238,7 @@ impl InputSelection { // PANIC: safe to unwrap as non basic/account/foundry/nft outputs are already filtered out. let unlock_conditions = input.output.unlock_conditions().unwrap(); - if unlock_conditions.is_timelocked(self.slot_index) { + if unlock_conditions.is_timelocked(self.slot_index, self.protocol_parameters.min_committable_age()) { return false; } diff --git a/sdk/src/types/block/output/unlock_condition/mod.rs b/sdk/src/types/block/output/unlock_condition/mod.rs index e5404d68eb..064f473a87 100644 --- a/sdk/src/types/block/output/unlock_condition/mod.rs +++ b/sdk/src/types/block/output/unlock_condition/mod.rs @@ -389,13 +389,12 @@ impl UnlockConditions { .map(UnlockCondition::as_timelock) } - /// Returns whether a timelock exists and is still relevant. + /// Checks whether a timelock exists and is still relevant. #[inline(always)] - pub fn is_timelocked(&self, slot_index: impl Into) -> bool { - let slot_index = slot_index.into(); - - self.timelock() - .map_or(false, |timelock| slot_index < timelock.slot_index()) + pub fn is_timelocked(&self, slot_index: impl Into, min_committable_age: impl Into) -> bool { + self.timelock().map_or(false, |timelock| { + timelock.is_timelocked(slot_index, min_committable_age) + }) } /// Gets a reference to an [`ExpirationUnlockCondition`], if any. diff --git a/sdk/src/types/block/output/unlock_condition/timelock.rs b/sdk/src/types/block/output/unlock_condition/timelock.rs index e4597623e7..db9619aeb6 100644 --- a/sdk/src/types/block/output/unlock_condition/timelock.rs +++ b/sdk/src/types/block/output/unlock_condition/timelock.rs @@ -30,6 +30,7 @@ impl TimelockUnlockCondition { self.0 } + /// Checks whether the timelock is still relevant. pub fn is_timelocked(&self, slot_index: impl Into, min_committable_age: impl Into) -> bool { (slot_index.into() + min_committable_age.into()) < self.0 } diff --git a/sdk/src/types/block/semantic.rs b/sdk/src/types/block/semantic.rs index 3ef29f4eb9..8d4f843223 100644 --- a/sdk/src/types/block/semantic.rs +++ b/sdk/src/types/block/semantic.rs @@ -292,11 +292,6 @@ pub fn semantic_validation( } } - // TODO remove the method? - // if unlock_conditions.is_time_locked(context.essence.creation_slot()) { - // return Ok(Some(TransactionFailureReason::TimelockNotExpired)); - // } - if let Some(expiration) = unlock_conditions.expiration() { if let Some(commitment) = context.essence.context_inputs().iter().find(|c| c.is_commitment()) { // TODO check is_deadzoned ? diff --git a/sdk/src/wallet/account/operations/balance.rs b/sdk/src/wallet/account/operations/balance.rs index 0a6d8fb026..8d57fe0496 100644 --- a/sdk/src/wallet/account/operations/balance.rs +++ b/sdk/src/wallet/account/operations/balance.rs @@ -61,8 +61,9 @@ where addresses_with_unspent_outputs: impl Iterator + Send, account_details: &AccountDetails, ) -> Result { - let network_id = self.client().get_network_id().await?; - let rent_structure = self.client().get_rent_structure().await?; + let protocol_parameters = self.client().get_protocol_parameters().await?; + let network_id = protocol_parameters.network_id(); + let rent_structure = protocol_parameters.rent_structure(); let mut balance = Balance::default(); let mut total_rent_amount = 0; let mut total_native_tokens = NativeTokensBuilder::default(); @@ -183,6 +184,7 @@ where &account_details.addresses_with_unspent_outputs, output, slot_index, + protocol_parameters.min_committable_age(), ); if output_can_be_unlocked_now_and_in_future { diff --git a/sdk/src/wallet/account/operations/helpers/time.rs b/sdk/src/wallet/account/operations/helpers/time.rs index 185a6a1900..181ce53182 100644 --- a/sdk/src/wallet/account/operations/helpers/time.rs +++ b/sdk/src/wallet/account/operations/helpers/time.rs @@ -19,10 +19,11 @@ pub(crate) fn can_output_be_unlocked_now( account_and_nft_addresses: &[Address], output_data: &OutputData, slot_index: SlotIndex, + min_committable_age: SlotIndex, account_transition: Option, ) -> crate::wallet::Result { if let Some(unlock_conditions) = output_data.output.unlock_conditions() { - if unlock_conditions.is_timelocked(slot_index) { + if unlock_conditions.is_timelocked(slot_index, min_committable_age) { return Ok(false); } } @@ -45,9 +46,10 @@ pub(crate) fn can_output_be_unlocked_forever_from_now_on( account_addresses: &[AddressWithUnspentOutputs], output: &Output, slot_index: SlotIndex, + min_committable_age: SlotIndex, ) -> bool { if let Some(unlock_conditions) = output.unlock_conditions() { - if unlock_conditions.is_timelocked(slot_index) { + if unlock_conditions.is_timelocked(slot_index, min_committable_age) { return false; } diff --git a/sdk/src/wallet/account/operations/output_claiming.rs b/sdk/src/wallet/account/operations/output_claiming.rs index 66192f018a..fce38afe23 100644 --- a/sdk/src/wallet/account/operations/output_claiming.rs +++ b/sdk/src/wallet/account/operations/output_claiming.rs @@ -48,6 +48,7 @@ where let account_details = self.details().await; let slot_index = self.client().get_slot_index().await?; + let protocol_parameters = self.client().get_protocol_parameters().await?; // Get outputs for the claim let mut output_ids_to_claim: HashSet = HashSet::new(); @@ -71,6 +72,7 @@ where &[], output_data, slot_index, + protocol_parameters.min_committable_age(), // Not relevant without account addresses None, )? diff --git a/sdk/src/wallet/account/operations/output_consolidation.rs b/sdk/src/wallet/account/operations/output_consolidation.rs index 24e2095577..349cb9e83e 100644 --- a/sdk/src/wallet/account/operations/output_consolidation.rs +++ b/sdk/src/wallet/account/operations/output_consolidation.rs @@ -74,16 +74,17 @@ impl Account where crate::wallet::Error: From, { - fn should_consolidate_output( + async fn should_consolidate_output( &self, output_data: &OutputData, slot_index: SlotIndex, account_addresses: &[AddressWithUnspentOutputs], ) -> Result { Ok(if let Output::Basic(basic_output) = &output_data.output { + let min_committable_age = self.client().get_protocol_parameters().await?.min_committable_age(); let unlock_conditions = basic_output.unlock_conditions(); - let is_time_locked = unlock_conditions.is_timelocked(slot_index); + let is_time_locked = unlock_conditions.is_timelocked(slot_index, min_committable_age); if is_time_locked { // If the output is timelocked, then it cannot be consolidated. return Ok(false); @@ -97,7 +98,14 @@ where return Ok(false); } - can_output_be_unlocked_now(account_addresses, &[], output_data, slot_index, None)? + can_output_be_unlocked_now( + account_addresses, + &[], + output_data, + slot_index, + min_committable_age, + None, + )? } else { false }) @@ -141,8 +149,9 @@ where } } let is_locked_output = account_details.locked_outputs.contains(output_id); - let should_consolidate_output = - self.should_consolidate_output(output_data, slot_index, account_addresses)?; + let should_consolidate_output = self + .should_consolidate_output(output_data, slot_index, account_addresses) + .await?; if !is_locked_output && should_consolidate_output { outputs_to_consolidate.push(output_data.clone()); } diff --git a/sdk/src/wallet/account/operations/transaction/input_selection.rs b/sdk/src/wallet/account/operations/transaction/input_selection.rs index 7674e1eeaf..532de85c7f 100644 --- a/sdk/src/wallet/account/operations/transaction/input_selection.rs +++ b/sdk/src/wallet/account/operations/transaction/input_selection.rs @@ -76,6 +76,7 @@ where &account_details, account_details.unspent_outputs.values(), slot_index, + protocol_parameters.min_committable_age(), &outputs, burn, custom_inputs.as_ref(), @@ -227,6 +228,7 @@ fn filter_inputs( account: &AccountDetails, available_outputs: Values<'_, OutputId, OutputData>, slot_index: SlotIndex, + min_committable_age: SlotIndex, outputs: &[Output], burn: Option<&Burn>, custom_inputs: Option<&HashSet>, @@ -248,6 +250,7 @@ fn filter_inputs( &account.addresses_with_unspent_outputs, &output_data.output, slot_index, + min_committable_age, ); // Outputs that could get unlocked in the future will not be included diff --git a/sdk/tests/wallet/output_preparation.rs b/sdk/tests/wallet/output_preparation.rs index c0258b44b0..a6d692ac8b 100644 --- a/sdk/tests/wallet/output_preparation.rs +++ b/sdk/tests/wallet/output_preparation.rs @@ -352,7 +352,8 @@ async fn output_preparation() -> Result<()> { assert_eq!(sender_feature.address(), issuer_and_sender_address.inner()); // Unlocks let conditions = output.unlock_conditions().unwrap(); - assert!(conditions.is_timelocked(0)); + // TODO double check + assert!(conditions.is_timelocked(0, 0)); assert!(conditions.is_expired(2)); // nft with expiration From df3a9a243396586d8dbcc429e78ac5d63ba07cdf Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Thu, 28 Sep 2023 13:27:21 +0200 Subject: [PATCH 09/17] Fix is_expired --- .../block/output/unlock_condition/expiration.rs | 5 +++-- sdk/src/types/block/output/unlock_condition/mod.rs | 11 +++++------ sdk/src/types/block/semantic.rs | 14 -------------- .../wallet/account/operations/output_claiming.rs | 6 ++++-- .../account/operations/output_consolidation.rs | 2 +- sdk/tests/wallet/output_preparation.rs | 3 ++- 6 files changed, 15 insertions(+), 26 deletions(-) diff --git a/sdk/src/types/block/output/unlock_condition/expiration.rs b/sdk/src/types/block/output/unlock_condition/expiration.rs index 26635f3449..6618ca60f3 100644 --- a/sdk/src/types/block/output/unlock_condition/expiration.rs +++ b/sdk/src/types/block/output/unlock_condition/expiration.rs @@ -45,8 +45,9 @@ impl ExpirationUnlockCondition { self.slot_index } - pub fn is_expired(&self, slot_index: SlotIndex, min_committable_age: SlotIndex) -> bool { - (slot_index + min_committable_age) >= self.slot_index + /// Checks whether the expiration is expired. + pub fn is_expired(&self, slot_index: impl Into, min_committable_age: impl Into) -> bool { + (slot_index.into() + min_committable_age.into()) >= self.slot_index } /// Returns the return address if the condition has expired. diff --git a/sdk/src/types/block/output/unlock_condition/mod.rs b/sdk/src/types/block/output/unlock_condition/mod.rs index 064f473a87..1cd44f09a9 100644 --- a/sdk/src/types/block/output/unlock_condition/mod.rs +++ b/sdk/src/types/block/output/unlock_condition/mod.rs @@ -404,13 +404,12 @@ impl UnlockConditions { .map(UnlockCondition::as_expiration) } - /// Returns whether an expiration exists and is expired. + /// Checks whether an expiration exists and is expired. #[inline(always)] - pub fn is_expired(&self, slot_index: impl Into) -> bool { - let slot_index = slot_index.into(); - - self.expiration() - .map_or(false, |expiration| slot_index >= expiration.slot_index()) + pub fn is_expired(&self, slot_index: impl Into, min_committable_age: impl Into) -> bool { + self.expiration().map_or(false, |expiration| { + expiration.is_expired(slot_index, min_committable_age) + }) } /// Gets a reference to a [`StateControllerAddressUnlockCondition`], if any. diff --git a/sdk/src/types/block/semantic.rs b/sdk/src/types/block/semantic.rs index 8d4f843223..7dcc4377f8 100644 --- a/sdk/src/types/block/semantic.rs +++ b/sdk/src/types/block/semantic.rs @@ -315,20 +315,6 @@ pub fn semantic_validation( } } - // TODO remove the method? - // if !unlock_conditions.is_expired(context.essence.creation_slot()) { - // if let Some(storage_deposit_return) = unlock_conditions.storage_deposit_return() { - // let amount = context - // .storage_deposit_returns - // .entry(*storage_deposit_return.return_address()) - // .or_default(); - - // *amount = amount - // .checked_add(storage_deposit_return.amount()) - // .ok_or(Error::StorageDepositReturnOverflow)?; - // } - // } - context.input_amount = context .input_amount .checked_add(amount) diff --git a/sdk/src/wallet/account/operations/output_claiming.rs b/sdk/src/wallet/account/operations/output_claiming.rs index fce38afe23..5c3571b2bb 100644 --- a/sdk/src/wallet/account/operations/output_claiming.rs +++ b/sdk/src/wallet/account/operations/output_claiming.rs @@ -81,7 +81,8 @@ where OutputsToClaim::MicroTransactions => { if let Some(sdr) = unlock_conditions.storage_deposit_return() { // If expired, it's not a micro transaction anymore - if unlock_conditions.is_expired(slot_index) { + if unlock_conditions.is_expired(slot_index, protocol_parameters.min_committable_age) + { continue; } // Only micro transaction if not the same @@ -102,7 +103,8 @@ where } OutputsToClaim::Amount => { let mut claimable_amount = output_data.output.amount(); - if !unlock_conditions.is_expired(slot_index) { + if !unlock_conditions.is_expired(slot_index, protocol_parameters.min_committable_age()) + { claimable_amount -= unlock_conditions .storage_deposit_return() .map(|s| s.amount()) diff --git a/sdk/src/wallet/account/operations/output_consolidation.rs b/sdk/src/wallet/account/operations/output_consolidation.rs index 349cb9e83e..b35a907d90 100644 --- a/sdk/src/wallet/account/operations/output_consolidation.rs +++ b/sdk/src/wallet/account/operations/output_consolidation.rs @@ -92,7 +92,7 @@ where let has_storage_deposit_return = unlock_conditions.storage_deposit_return().is_some(); let has_expiration = unlock_conditions.expiration().is_some(); - let is_expired = unlock_conditions.is_expired(slot_index); + let is_expired = unlock_conditions.is_expired(slot_index, min_committable_age); if has_storage_deposit_return && (!has_expiration || !is_expired) { // If the output has not expired and must return a storage deposit, then it cannot be consolidated. return Ok(false); diff --git a/sdk/tests/wallet/output_preparation.rs b/sdk/tests/wallet/output_preparation.rs index a6d692ac8b..244abcc722 100644 --- a/sdk/tests/wallet/output_preparation.rs +++ b/sdk/tests/wallet/output_preparation.rs @@ -354,7 +354,8 @@ async fn output_preparation() -> Result<()> { let conditions = output.unlock_conditions().unwrap(); // TODO double check assert!(conditions.is_timelocked(0, 0)); - assert!(conditions.is_expired(2)); + // TODO double check + assert!(conditions.is_expired(2, 0)); // nft with expiration let output = account From 9e0dc728e706d510063c890d59aa5dca7bf13df4 Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Thu, 28 Sep 2023 14:47:04 +0200 Subject: [PATCH 10/17] First batch of changes --- .../offline_signing/2_sign_transaction.rs | 8 ++- .../api/block_builder/input_selection/mod.rs | 53 +++++++++++++++---- .../input_selection/remainder.rs | 8 ++- .../input_selection/requirement/amount.rs | 18 ++++++- .../input_selection/requirement/ed25519.rs | 16 +++++- sdk/src/client/secret/ledger_nano.rs | 8 ++- sdk/src/client/secret/mnemonic.rs | 5 +- sdk/src/client/secret/mod.rs | 29 ++++++---- sdk/src/client/secret/private_key.rs | 5 +- sdk/src/client/stronghold/secret.rs | 6 ++- sdk/src/types/block/output/basic.rs | 9 +++- sdk/src/types/block/output/delegation.rs | 9 +++- sdk/src/types/block/output/mod.rs | 22 ++++++-- sdk/src/types/block/output/nft.rs | 9 +++- .../output/unlock_condition/expiration.rs | 15 ++++-- .../block/output/unlock_condition/mod.rs | 17 ++++-- sdk/src/types/block/semantic.rs | 2 +- sdk/src/wallet/account/operations/balance.rs | 1 + .../wallet/account/operations/helpers/time.rs | 38 ++++++++----- .../account/operations/output_claiming.rs | 1 + .../operations/output_consolidation.rs | 9 ++-- .../operations/transaction/input_selection.rs | 11 +++- .../transaction/sign_transaction.rs | 5 +- sdk/src/wallet/account/types/mod.rs | 12 +++-- 24 files changed, 244 insertions(+), 72 deletions(-) diff --git a/sdk/examples/wallet/offline_signing/2_sign_transaction.rs b/sdk/examples/wallet/offline_signing/2_sign_transaction.rs index 1c9c1113a1..cc5bd2ca0f 100644 --- a/sdk/examples/wallet/offline_signing/2_sign_transaction.rs +++ b/sdk/examples/wallet/offline_signing/2_sign_transaction.rs @@ -16,7 +16,10 @@ use iota_sdk::{ }, secret::{stronghold::StrongholdSecretManager, SecretManage, SecretManager}, }, - types::{block::payload::TransactionPayload, TryFromDto}, + types::{ + block::{payload::TransactionPayload, protocol::protocol_parameters}, + TryFromDto, + }, wallet::Result, }; @@ -38,7 +41,8 @@ async fn main() -> Result<()> { // Signs prepared transaction offline. let unlocks = SecretManager::Stronghold(secret_manager) - .sign_transaction_essence(&prepared_transaction_data) + // TODO meh + .sign_transaction_essence(&prepared_transaction_data, &protocol_parameters()) .await?; let signed_transaction = TransactionPayload::new(prepared_transaction_data.essence.as_regular().clone(), unlocks)?; diff --git a/sdk/src/client/api/block_builder/input_selection/mod.rs b/sdk/src/client/api/block_builder/input_selection/mod.rs index ef7dde8e16..531d48f565 100644 --- a/sdk/src/client/api/block_builder/input_selection/mod.rs +++ b/sdk/src/client/api/block_builder/input_selection/mod.rs @@ -63,7 +63,13 @@ impl InputSelection { is_account_transition(&input.output, *input.output_id(), &self.outputs, self.burn.as_ref()); let required_address = input .output - .required_and_unlocked_address(self.slot_index, input.output_id(), account_transition)? + .required_and_unlocked_address( + self.slot_index, + self.protocol_parameters.min_committable_age(), + self.protocol_parameters.max_committable_age(), + input.output_id(), + account_transition, + )? .0; match required_address { @@ -245,7 +251,13 @@ impl InputSelection { let required_address = input .output // Account transition is irrelevant here as we keep accounts anyway. - .required_and_unlocked_address(self.slot_index, input.output_id(), None) + .required_and_unlocked_address( + self.slot_index, + self.protocol_parameters.min_committable_age(), + self.protocol_parameters.max_committable_age(), + input.output_id(), + None, + ) // PANIC: safe to unwrap as non basic/account/foundry/nft outputs are already filtered out. .unwrap() .0; @@ -259,6 +271,8 @@ impl InputSelection { mut inputs: Vec, outputs: &[Output], slot_index: SlotIndex, + min_committable_age: SlotIndex, + max_committable_age: SlotIndex, ) -> Result, Error> { // initially sort by output to make it deterministic // TODO: rethink this, we only need it deterministic for tests, for the protocol it doesn't matter, also there @@ -275,7 +289,13 @@ impl InputSelection { ); let (input_address, _) = input_signing_data .output - .required_and_unlocked_address(slot_index, input_signing_data.output_id(), account_transition) + .required_and_unlocked_address( + slot_index, + min_committable_age, + max_committable_age, + input_signing_data.output_id(), + account_transition, + ) // PANIC: safe to unwrap, because we filtered irrelevant outputs out before .unwrap(); @@ -284,10 +304,13 @@ impl InputSelection { for input in account_nft_address_inputs { let account_transition = is_account_transition(&input.output, *input.output_id(), outputs, None); - let (input_address, _) = - input - .output - .required_and_unlocked_address(slot_index, input.output_id(), account_transition)?; + let (input_address, _) = input.output.required_and_unlocked_address( + slot_index, + min_committable_age, + max_committable_age, + input.output_id(), + account_transition, + )?; match sorted_inputs.iter().position(|input_signing_data| match input_address { Address::Account(unlock_address) => { @@ -334,7 +357,13 @@ impl InputSelection { ); let (input_address, _) = input_signing_data .output - .required_and_unlocked_address(slot_index, input.output_id(), account_transition) + .required_and_unlocked_address( + slot_index, + min_committable_age, + max_committable_age, + input.output_id(), + account_transition, + ) // PANIC: safe to unwrap, because we filtered irrelevant outputs out before .unwrap(); @@ -408,7 +437,13 @@ impl InputSelection { self.validate_transitions()?; Ok(Selected { - inputs: Self::sort_input_signing_data(self.selected_inputs, &self.outputs, self.slot_index)?, + inputs: Self::sort_input_signing_data( + self.selected_inputs, + &self.outputs, + self.slot_index, + self.protocol_parameters.min_committable_age(), + self.protocol_parameters.max_committable_age(), + )?, outputs: self.outputs, remainder, }) diff --git a/sdk/src/client/api/block_builder/input_selection/remainder.rs b/sdk/src/client/api/block_builder/input_selection/remainder.rs index 5b2e39caa9..996110cabd 100644 --- a/sdk/src/client/api/block_builder/input_selection/remainder.rs +++ b/sdk/src/client/api/block_builder/input_selection/remainder.rs @@ -36,7 +36,13 @@ impl InputSelection { // PANIC: safe to unwrap as outputs with no address have been filtered out already. let required_address = input .output - .required_and_unlocked_address(self.slot_index, input.output_id(), account_transition) + .required_and_unlocked_address( + self.slot_index, + self.protocol_parameters.min_committable_age(), + self.protocol_parameters.max_committable_age(), + input.output_id(), + account_transition, + ) .unwrap() .0; diff --git a/sdk/src/client/api/block_builder/input_selection/requirement/amount.rs b/sdk/src/client/api/block_builder/input_selection/requirement/amount.rs index 549bc43e62..00d1e2a59b 100644 --- a/sdk/src/client/api/block_builder/input_selection/requirement/amount.rs +++ b/sdk/src/client/api/block_builder/input_selection/requirement/amount.rs @@ -356,7 +356,14 @@ impl InputSelection { if let Output::Basic(output) = &input.output { output .unlock_conditions() - .locked_address(output.address(), self.slot_index) + .locked_address( + output.address(), + self.slot_index, + self.protocol_parameters.min_committable_age(), + self.protocol_parameters.max_committable_age(), + ) + // TODO + .unwrap() .is_ed25519() } else { false @@ -371,7 +378,14 @@ impl InputSelection { if let Output::Basic(output) = &input.output { !output .unlock_conditions() - .locked_address(output.address(), self.slot_index) + .locked_address( + output.address(), + self.slot_index, + self.protocol_parameters.min_committable_age(), + self.protocol_parameters.max_committable_age(), + ) + // TODO + .unwrap() .is_ed25519() } else { false diff --git a/sdk/src/client/api/block_builder/input_selection/requirement/ed25519.rs b/sdk/src/client/api/block_builder/input_selection/requirement/ed25519.rs index f9d56c91e1..1d3bf12e78 100644 --- a/sdk/src/client/api/block_builder/input_selection/requirement/ed25519.rs +++ b/sdk/src/client/api/block_builder/input_selection/requirement/ed25519.rs @@ -20,7 +20,13 @@ impl InputSelection { // PANIC: safe to unwrap as outputs with no address have been filtered out already. let required_address = input .output - .required_and_unlocked_address(self.slot_index, input.output_id(), account_transition) + .required_and_unlocked_address( + self.slot_index, + self.protocol_parameters.min_committable_age(), + self.protocol_parameters.max_committable_age(), + input.output_id(), + account_transition, + ) .unwrap() .0; @@ -58,7 +64,13 @@ impl InputSelection { } else { let (required_address, _) = input .output - .required_and_unlocked_address(self.slot_index, input.output_id(), None) + .required_and_unlocked_address( + self.slot_index, + self.protocol_parameters.min_committable_age(), + self.protocol_parameters.max_committable_age(), + input.output_id(), + None, + ) .unwrap(); (&required_address == address, None) diff --git a/sdk/src/client/secret/ledger_nano.rs b/sdk/src/client/secret/ledger_nano.rs index e06a83c94c..8b86455af6 100644 --- a/sdk/src/client/secret/ledger_nano.rs +++ b/sdk/src/client/secret/ledger_nano.rs @@ -241,6 +241,7 @@ impl SecretManage for LedgerSecretManager { async fn sign_transaction_essence( &self, prepared_transaction: &PreparedTransactionData, + protocol_parameters: &ProtocolParameters, ) -> Result::Error> { let mut input_bip32_indices = Vec::new(); let mut coin_type = None; @@ -397,7 +398,7 @@ impl SecretManage for LedgerSecretManager { // With blind signing the ledger only returns SignatureUnlocks, so we might have to merge them with // Account/Nft/Reference unlocks if blind_signing { - unlocks = merge_unlocks(prepared_transaction, unlocks.into_iter())?; + unlocks = merge_unlocks(prepared_transaction, unlocks.into_iter(), protocol_parameters)?; } Ok(Unlocks::new(unlocks)?) @@ -406,7 +407,7 @@ impl SecretManage for LedgerSecretManager { async fn sign_transaction( &self, prepared_transaction_data: PreparedTransactionData, - protocol_parameters: ProtocolParameters, + protocol_parameters: &ProtocolParameters, ) -> Result { super::default_sign_transaction(self, prepared_transaction_data, protocol_parameters).await } @@ -515,6 +516,7 @@ impl LedgerSecretManager { fn merge_unlocks( prepared_transaction_data: &PreparedTransactionData, mut unlocks: impl Iterator, + protocol_parameters: &ProtocolParameters, ) -> Result, Error> { let TransactionEssence::Regular(essence) = &prepared_transaction_data.essence; let slot_index = essence.creation_slot(); @@ -531,6 +533,8 @@ fn merge_unlocks( let account_transition = is_account_transition(&input.output, *input.output_id(), regular.outputs(), None); let (input_address, _) = input.output.required_and_unlocked_address( slot_index, + protocol_parameters.min_committable_age(), + protocol_parameters.max_committable_age(), input.output_metadata.output_id(), account_transition, )?; diff --git a/sdk/src/client/secret/mnemonic.rs b/sdk/src/client/secret/mnemonic.rs index 4607ca42e6..76b37c988f 100644 --- a/sdk/src/client/secret/mnemonic.rs +++ b/sdk/src/client/secret/mnemonic.rs @@ -125,14 +125,15 @@ impl SecretManage for MnemonicSecretManager { async fn sign_transaction_essence( &self, prepared_transaction_data: &PreparedTransactionData, + protocol_parameters: &ProtocolParameters, ) -> Result { - super::default_sign_transaction_essence(self, prepared_transaction_data).await + super::default_sign_transaction_essence(self, prepared_transaction_data, protocol_parameters).await } async fn sign_transaction( &self, prepared_transaction_data: PreparedTransactionData, - protocol_parameters: ProtocolParameters, + protocol_parameters: &ProtocolParameters, ) -> Result { super::default_sign_transaction(self, prepared_transaction_data, protocol_parameters).await } diff --git a/sdk/src/client/secret/mod.rs b/sdk/src/client/secret/mod.rs index 5876e40b12..eac35c43f8 100644 --- a/sdk/src/client/secret/mod.rs +++ b/sdk/src/client/secret/mod.rs @@ -106,12 +106,13 @@ pub trait SecretManage: Send + Sync { async fn sign_transaction_essence( &self, prepared_transaction_data: &PreparedTransactionData, + protocol_parameters: &ProtocolParameters, ) -> Result; async fn sign_transaction( &self, prepared_transaction_data: PreparedTransactionData, - protocol_parameters: ProtocolParameters, + protocol_parameters: &ProtocolParameters, ) -> Result; } @@ -405,20 +406,27 @@ impl SecretManage for SecretManager { async fn sign_transaction_essence( &self, prepared_transaction_data: &PreparedTransactionData, + protocol_parameters: &ProtocolParameters, ) -> Result { match self { #[cfg(feature = "stronghold")] Self::Stronghold(secret_manager) => Ok(secret_manager - .sign_transaction_essence(prepared_transaction_data) + .sign_transaction_essence(prepared_transaction_data, protocol_parameters) .await?), #[cfg(feature = "ledger_nano")] Self::LedgerNano(secret_manager) => Ok(secret_manager - .sign_transaction_essence(prepared_transaction_data) + .sign_transaction_essence(prepared_transaction_data, protocol_parameters) .await?), - Self::Mnemonic(secret_manager) => secret_manager.sign_transaction_essence(prepared_transaction_data).await, + Self::Mnemonic(secret_manager) => { + secret_manager + .sign_transaction_essence(prepared_transaction_data, protocol_parameters) + .await + } #[cfg(feature = "private_key_secret_manager")] Self::PrivateKey(secret_manager) => { - secret_manager.sign_transaction_essence(prepared_transaction_data).await + secret_manager + .sign_transaction_essence(prepared_transaction_data, protocol_parameters) + .await } Self::Placeholder => Err(Error::PlaceholderSecretManager), } @@ -427,7 +435,7 @@ impl SecretManage for SecretManager { async fn sign_transaction( &self, prepared_transaction_data: PreparedTransactionData, - protocol_parameters: ProtocolParameters, + protocol_parameters: &ProtocolParameters, ) -> Result { match self { #[cfg(feature = "stronghold")] @@ -516,6 +524,7 @@ impl SecretManager { pub(crate) async fn default_sign_transaction_essence( secret_manager: &M, prepared_transaction_data: &PreparedTransactionData, + protocol_parameters: &ProtocolParameters, ) -> crate::client::Result where crate::client::Error: From, @@ -534,6 +543,8 @@ where let account_transition = is_account_transition(&input.output, *input.output_id(), regular.outputs(), None); let (input_address, _) = input.output.required_and_unlocked_address( slot_index, + protocol_parameters.min_committable_age(), + protocol_parameters.max_committable_age(), input.output_metadata.output_id(), account_transition, )?; @@ -589,7 +600,7 @@ where pub(crate) async fn default_sign_transaction( secret_manager: &M, prepared_transaction_data: PreparedTransactionData, - protocol_parameters: ProtocolParameters, + protocol_parameters: &ProtocolParameters, ) -> crate::client::Result where crate::client::Error: From, @@ -597,7 +608,7 @@ where log::debug!("[sign_transaction] {:?}", prepared_transaction_data); let unlocks = secret_manager - .sign_transaction_essence(&prepared_transaction_data) + .sign_transaction_essence(&prepared_transaction_data, protocol_parameters) .await?; let PreparedTransactionData { @@ -609,7 +620,7 @@ where validate_transaction_payload_length(&tx_payload)?; - let conflict = verify_semantic(&inputs_data, &tx_payload, protocol_parameters)?; + let conflict = verify_semantic(&inputs_data, &tx_payload, protocol_parameters.clone())?; if let Some(conflict) = conflict { log::debug!("[sign_transaction] conflict: {conflict:?} for {:#?}", tx_payload); diff --git a/sdk/src/client/secret/private_key.rs b/sdk/src/client/secret/private_key.rs index e94cea6b34..61bd8bc6dd 100644 --- a/sdk/src/client/secret/private_key.rs +++ b/sdk/src/client/secret/private_key.rs @@ -85,14 +85,15 @@ impl SecretManage for PrivateKeySecretManager { async fn sign_transaction_essence( &self, prepared_transaction_data: &PreparedTransactionData, + protocol_parameters: &ProtocolParameters, ) -> Result { - super::default_sign_transaction_essence(self, prepared_transaction_data).await + super::default_sign_transaction_essence(self, prepared_transaction_data, protocol_parameters).await } async fn sign_transaction( &self, prepared_transaction_data: PreparedTransactionData, - protocol_parameters: ProtocolParameters, + protocol_parameters: &ProtocolParameters, ) -> Result { super::default_sign_transaction(self, prepared_transaction_data, protocol_parameters).await } diff --git a/sdk/src/client/stronghold/secret.rs b/sdk/src/client/stronghold/secret.rs index 3ebf21402b..72d9015776 100644 --- a/sdk/src/client/stronghold/secret.rs +++ b/sdk/src/client/stronghold/secret.rs @@ -284,14 +284,16 @@ impl SecretManage for StrongholdAdapter { async fn sign_transaction_essence( &self, prepared_transaction_data: &PreparedTransactionData, + protocol_parameters: &ProtocolParameters, ) -> Result { - crate::client::secret::default_sign_transaction_essence(self, prepared_transaction_data).await + crate::client::secret::default_sign_transaction_essence(self, prepared_transaction_data, protocol_parameters) + .await } async fn sign_transaction( &self, prepared_transaction_data: PreparedTransactionData, - protocol_parameters: ProtocolParameters, + protocol_parameters: &ProtocolParameters, ) -> Result { crate::client::secret::default_sign_transaction(self, prepared_transaction_data, protocol_parameters).await } diff --git a/sdk/src/types/block/output/basic.rs b/sdk/src/types/block/output/basic.rs index 7fed97f268..810c054b5c 100644 --- a/sdk/src/types/block/output/basic.rs +++ b/sdk/src/types/block/output/basic.rs @@ -305,7 +305,14 @@ impl BasicOutput { context: &mut ValidationContext<'_>, ) -> Result<(), TransactionFailureReason> { self.unlock_conditions() - .locked_address(self.address(), context.essence.creation_slot()) + .locked_address( + self.address(), + context.essence.creation_slot(), + context.protocol_parameters.min_committable_age(), + context.protocol_parameters.max_committable_age(), + ) + // TODO + .unwrap() .unlock(unlock, inputs, context) } diff --git a/sdk/src/types/block/output/delegation.rs b/sdk/src/types/block/output/delegation.rs index 5c926cd55f..cb8c1c2334 100644 --- a/sdk/src/types/block/output/delegation.rs +++ b/sdk/src/types/block/output/delegation.rs @@ -353,7 +353,14 @@ impl DelegationOutput { context: &mut ValidationContext<'_>, ) -> Result<(), TransactionFailureReason> { self.unlock_conditions() - .locked_address(self.address(), context.essence.creation_slot()) + .locked_address( + self.address(), + context.essence.creation_slot(), + context.protocol_parameters.min_committable_age(), + context.protocol_parameters.max_committable_age(), + ) + // TODO + .unwrap() .unlock(unlock, inputs, context) } diff --git a/sdk/src/types/block/output/mod.rs b/sdk/src/types/block/output/mod.rs index c9829eb5a0..a54e89c279 100644 --- a/sdk/src/types/block/output/mod.rs +++ b/sdk/src/types/block/output/mod.rs @@ -309,13 +309,19 @@ impl Output { /// If no `account_transition` has been provided, assumes a state transition. pub fn required_and_unlocked_address( &self, - slot_index: SlotIndex, + slot_index: impl Into, + min_committable_age: impl Into, + max_committable_age: impl Into, output_id: &OutputId, account_transition: Option, ) -> Result<(Address, Option
), Error> { match self { Self::Basic(output) => Ok(( - *output.unlock_conditions().locked_address(output.address(), slot_index), + *output + .unlock_conditions() + .locked_address(output.address(), slot_index, min_committable_age, max_committable_age) + // TODO + .unwrap(), None, )), Self::Account(output) => { @@ -331,11 +337,19 @@ impl Output { } Self::Foundry(output) => Ok((Address::Account(*output.account_address()), None)), Self::Nft(output) => Ok(( - *output.unlock_conditions().locked_address(output.address(), slot_index), + *output + .unlock_conditions() + .locked_address(output.address(), slot_index, min_committable_age, max_committable_age) + // TODO + .unwrap(), Some(Address::Nft(output.nft_address(output_id))), )), Self::Delegation(output) => Ok(( - *output.unlock_conditions().locked_address(output.address(), slot_index), + *output + .unlock_conditions() + .locked_address(output.address(), slot_index, min_committable_age, max_committable_age) + // TODO + .unwrap(), None, )), } diff --git a/sdk/src/types/block/output/nft.rs b/sdk/src/types/block/output/nft.rs index 957e384e51..d30a6854ac 100644 --- a/sdk/src/types/block/output/nft.rs +++ b/sdk/src/types/block/output/nft.rs @@ -408,7 +408,14 @@ impl NftOutput { context: &mut ValidationContext<'_>, ) -> Result<(), TransactionFailureReason> { self.unlock_conditions() - .locked_address(self.address(), context.essence.creation_slot()) + .locked_address( + self.address(), + context.essence.creation_slot(), + context.protocol_parameters.min_committable_age(), + context.protocol_parameters.max_committable_age(), + ) + // TODO + .unwrap() .unlock(unlock, inputs, context)?; let nft_id = if self.nft_id().is_null() { diff --git a/sdk/src/types/block/output/unlock_condition/expiration.rs b/sdk/src/types/block/output/unlock_condition/expiration.rs index 6618ca60f3..b78e0574d4 100644 --- a/sdk/src/types/block/output/unlock_condition/expiration.rs +++ b/sdk/src/types/block/output/unlock_condition/expiration.rs @@ -51,9 +51,18 @@ impl ExpirationUnlockCondition { } /// Returns the return address if the condition has expired. - /// TODO needs to be updated - pub fn return_address_expired(&self, slot_index: SlotIndex) -> Option<&Address> { - if slot_index >= self.slot_index() { + pub fn return_address_expired<'a>( + &'a self, + address: &'a Address, + slot_index: impl Into, + min_committable_age: impl Into, + max_committable_age: impl Into, + ) -> Option<&'a Address> { + let slot_index = slot_index.into(); + + if self.slot_index() > (slot_index + max_committable_age.into()) { + Some(address) + } else if self.slot_index() <= (slot_index + min_committable_age.into()) { Some(&self.return_address) } else { None diff --git a/sdk/src/types/block/output/unlock_condition/mod.rs b/sdk/src/types/block/output/unlock_condition/mod.rs index 1cd44f09a9..247ebc70c0 100644 --- a/sdk/src/types/block/output/unlock_condition/mod.rs +++ b/sdk/src/types/block/output/unlock_condition/mod.rs @@ -434,12 +434,19 @@ impl UnlockConditions { } /// Returns the address to be unlocked. - /// TODO take expiration deadzone into account #[inline(always)] - pub fn locked_address<'a>(&'a self, address: &'a Address, slot_index: SlotIndex) -> &'a Address { - self.expiration() - .and_then(|e| e.return_address_expired(slot_index)) - .unwrap_or(address) + pub fn locked_address<'a>( + &'a self, + address: &'a Address, + slot_index: impl Into, + min_committable_age: impl Into, + max_committable_age: impl Into, + ) -> Option<&'a Address> { + if let Some(expiration) = self.expiration() { + expiration.return_address_expired(address, slot_index, min_committable_age, max_committable_age) + } else { + Some(address) + } } } diff --git a/sdk/src/types/block/semantic.rs b/sdk/src/types/block/semantic.rs index 7dcc4377f8..9356753226 100644 --- a/sdk/src/types/block/semantic.rs +++ b/sdk/src/types/block/semantic.rs @@ -154,7 +154,7 @@ impl TryFrom for TransactionFailureReason { /// pub struct ValidationContext<'a> { /// - protocol_parameters: ProtocolParameters, + pub protocol_parameters: ProtocolParameters, /// pub essence: &'a RegularTransactionEssence, /// diff --git a/sdk/src/wallet/account/operations/balance.rs b/sdk/src/wallet/account/operations/balance.rs index 8d57fe0496..e261279590 100644 --- a/sdk/src/wallet/account/operations/balance.rs +++ b/sdk/src/wallet/account/operations/balance.rs @@ -185,6 +185,7 @@ where output, slot_index, protocol_parameters.min_committable_age(), + protocol_parameters.max_committable_age(), ); if output_can_be_unlocked_now_and_in_future { diff --git a/sdk/src/wallet/account/operations/helpers/time.rs b/sdk/src/wallet/account/operations/helpers/time.rs index 181ce53182..1363d35a23 100644 --- a/sdk/src/wallet/account/operations/helpers/time.rs +++ b/sdk/src/wallet/account/operations/helpers/time.rs @@ -20,6 +20,7 @@ pub(crate) fn can_output_be_unlocked_now( output_data: &OutputData, slot_index: SlotIndex, min_committable_age: SlotIndex, + max_committable_age: SlotIndex, account_transition: Option, ) -> crate::wallet::Result { if let Some(unlock_conditions) = output_data.output.unlock_conditions() { @@ -28,9 +29,14 @@ pub(crate) fn can_output_be_unlocked_now( } } - let (required_unlock_address, _unlocked_account_or_nft_address) = output_data - .output - .required_and_unlocked_address(slot_index, &output_data.output_id, account_transition)?; + let (required_unlock_address, _unlocked_account_or_nft_address) = + output_data.output.required_and_unlocked_address( + slot_index, + min_committable_age, + max_committable_age, + &output_data.output_id, + account_transition, + )?; Ok(account_addresses .iter() @@ -47,23 +53,27 @@ pub(crate) fn can_output_be_unlocked_forever_from_now_on( output: &Output, slot_index: SlotIndex, min_committable_age: SlotIndex, + max_committable_age: SlotIndex, ) -> bool { if let Some(unlock_conditions) = output.unlock_conditions() { if unlock_conditions.is_timelocked(slot_index, min_committable_age) { return false; } - // If there is an expiration unlock condition, we can only unlock it forever from now on, if it's expired and - // the return address belongs to the account - if let Some(expiration) = unlock_conditions.expiration() { - if let Some(return_address) = expiration.return_address_expired(slot_index) { - if !account_addresses.iter().any(|a| a.address.inner == *return_address) { - return false; - }; - } else { - return false; - } - } + // TODO HELP + // // If there is an expiration unlock condition, we can only unlock it forever from now on, if it's expired and + // // the return address belongs to the account + // if let Some(expiration) = unlock_conditions.expiration() { + // if let Some(return_address) = + // expiration.return_address_expired(slot_index, min_committable_age, max_committable_age) + // { + // if !account_addresses.iter().any(|a| a.address.inner == *return_address) { + // return false; + // }; + // } else { + // return false; + // } + // } true } else { diff --git a/sdk/src/wallet/account/operations/output_claiming.rs b/sdk/src/wallet/account/operations/output_claiming.rs index 5c3571b2bb..b5d7eba463 100644 --- a/sdk/src/wallet/account/operations/output_claiming.rs +++ b/sdk/src/wallet/account/operations/output_claiming.rs @@ -73,6 +73,7 @@ where output_data, slot_index, protocol_parameters.min_committable_age(), + protocol_parameters.max_committable_age(), // Not relevant without account addresses None, )? diff --git a/sdk/src/wallet/account/operations/output_consolidation.rs b/sdk/src/wallet/account/operations/output_consolidation.rs index b35a907d90..88b8fa0e11 100644 --- a/sdk/src/wallet/account/operations/output_consolidation.rs +++ b/sdk/src/wallet/account/operations/output_consolidation.rs @@ -81,10 +81,10 @@ where account_addresses: &[AddressWithUnspentOutputs], ) -> Result { Ok(if let Output::Basic(basic_output) = &output_data.output { - let min_committable_age = self.client().get_protocol_parameters().await?.min_committable_age(); + let protocol_parameters = self.client().get_protocol_parameters().await?; let unlock_conditions = basic_output.unlock_conditions(); - let is_time_locked = unlock_conditions.is_timelocked(slot_index, min_committable_age); + let is_time_locked = unlock_conditions.is_timelocked(slot_index, protocol_parameters.min_committable_age()); if is_time_locked { // If the output is timelocked, then it cannot be consolidated. return Ok(false); @@ -92,7 +92,7 @@ where let has_storage_deposit_return = unlock_conditions.storage_deposit_return().is_some(); let has_expiration = unlock_conditions.expiration().is_some(); - let is_expired = unlock_conditions.is_expired(slot_index, min_committable_age); + let is_expired = unlock_conditions.is_expired(slot_index, protocol_parameters.min_committable_age()); if has_storage_deposit_return && (!has_expiration || !is_expired) { // If the output has not expired and must return a storage deposit, then it cannot be consolidated. return Ok(false); @@ -103,7 +103,8 @@ where &[], output_data, slot_index, - min_committable_age, + protocol_parameters.min_committable_age(), + protocol_parameters.max_committable_age(), None, )? } else { diff --git a/sdk/src/wallet/account/operations/transaction/input_selection.rs b/sdk/src/wallet/account/operations/transaction/input_selection.rs index 532de85c7f..1914aa38c7 100644 --- a/sdk/src/wallet/account/operations/transaction/input_selection.rs +++ b/sdk/src/wallet/account/operations/transaction/input_selection.rs @@ -77,6 +77,7 @@ where account_details.unspent_outputs.values(), slot_index, protocol_parameters.min_committable_age(), + protocol_parameters.max_committable_age(), &outputs, burn, custom_inputs.as_ref(), @@ -229,6 +230,7 @@ fn filter_inputs( available_outputs: Values<'_, OutputId, OutputData>, slot_index: SlotIndex, min_committable_age: SlotIndex, + max_committable_age: SlotIndex, outputs: &[Output], burn: Option<&Burn>, custom_inputs: Option<&HashSet>, @@ -251,6 +253,7 @@ fn filter_inputs( &output_data.output, slot_index, min_committable_age, + max_committable_age, ); // Outputs that could get unlocked in the future will not be included @@ -262,7 +265,13 @@ fn filter_inputs( // Defaults to state transition if it is not explicitly a governance transition or a burn. let account_state_transition = is_account_transition(&output_data.output, output_data.output_id, outputs, burn); - if let Some(available_input) = output_data.input_signing_data(account, slot_index, account_state_transition)? { + if let Some(available_input) = output_data.input_signing_data( + account, + slot_index, + min_committable_age, + max_committable_age, + account_state_transition, + )? { available_outputs_signing_data.push(available_input); } } diff --git a/sdk/src/wallet/account/operations/transaction/sign_transaction.rs b/sdk/src/wallet/account/operations/transaction/sign_transaction.rs index 883d0631a4..bf7a821e55 100644 --- a/sdk/src/wallet/account/operations/transaction/sign_transaction.rs +++ b/sdk/src/wallet/account/operations/transaction/sign_transaction.rs @@ -79,7 +79,10 @@ where .secret_manager .read() .await - .sign_transaction_essence(prepared_transaction_data) + .sign_transaction_essence( + prepared_transaction_data, + &self.client().get_protocol_parameters().await?, + ) .await { Ok(res) => res, diff --git a/sdk/src/wallet/account/types/mod.rs b/sdk/src/wallet/account/types/mod.rs index fa22cd1b14..0f1143dd9b 100644 --- a/sdk/src/wallet/account/types/mod.rs +++ b/sdk/src/wallet/account/types/mod.rs @@ -57,11 +57,17 @@ impl OutputData { &self, account: &AccountDetails, slot_index: SlotIndex, + min_committable_age: SlotIndex, + max_committable_age: SlotIndex, account_transition: Option, ) -> crate::wallet::Result> { - let (unlock_address, _unlocked_account_or_nft_address) = - self.output - .required_and_unlocked_address(slot_index, &self.output_id, account_transition)?; + let (unlock_address, _unlocked_account_or_nft_address) = self.output.required_and_unlocked_address( + slot_index, + min_committable_age, + max_committable_age, + &self.output_id, + account_transition, + )?; let chain = if unlock_address == self.address { self.chain From c65a469b248cf61bac193acc362361a4320c9a94 Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Thu, 28 Sep 2023 14:59:38 +0200 Subject: [PATCH 11/17] Second batch of changes --- bindings/core/src/method_handler/secret_manager.rs | 2 +- cli/src/command/account.rs | 11 ++++++++--- sdk/tests/client/signing/account.rs | 6 +++--- sdk/tests/client/signing/basic.rs | 6 +++--- sdk/tests/client/signing/mod.rs | 2 +- sdk/tests/client/signing/nft.rs | 2 +- 6 files changed, 17 insertions(+), 12 deletions(-) diff --git a/bindings/core/src/method_handler/secret_manager.rs b/bindings/core/src/method_handler/secret_manager.rs index 48d17b81a3..25669789be 100644 --- a/bindings/core/src/method_handler/secret_manager.rs +++ b/bindings/core/src/method_handler/secret_manager.rs @@ -42,7 +42,7 @@ pub(crate) async fn call_secret_manager_method_internal( let transaction = &secret_manager .sign_transaction( PreparedTransactionData::try_from_dto(prepared_transaction_data)?, - protocol_parameters, + &protocol_parameters, ) .await?; Response::SignedTransaction(transaction.into()) diff --git a/cli/src/command/account.rs b/cli/src/command/account.rs index 5ea180908c..9779fe28d6 100644 --- a/cli/src/command/account.rs +++ b/cli/src/command/account.rs @@ -933,6 +933,7 @@ async fn print_address(account: &Account, address: &Bip44Address) -> Result<(), let addresses = account.addresses_with_unspent_outputs().await?; let slot_index = account.client().get_slot_index().await?; + let protocol_parameters = account.client().get_protocol_parameters().await?; let mut output_ids: &[OutputId] = &[]; let mut amount = 0; @@ -950,9 +951,13 @@ async fn print_address(account: &Account, address: &Bip44Address) -> Result<(), for output_id in output_ids { if let Some(output_data) = account.get_output(output_id).await { // Output might be associated with the address, but can't be unlocked by it, so we check that here. - let (required_address, _) = output_data - .output - .required_and_unlocked_address(slot_index, output_id, None)?; + let (required_address, _) = output_data.output.required_and_unlocked_address( + slot_index, + protocol_parameters.min_committable_age(), + protocol_parameters.max_committable_age(), + output_id, + None, + )?; if address.address().as_ref() == &required_address { if let Some(nts) = output_data.output.native_tokens() { diff --git a/sdk/tests/client/signing/account.rs b/sdk/tests/client/signing/account.rs index 81b84c5055..a41d821b7d 100644 --- a/sdk/tests/client/signing/account.rs +++ b/sdk/tests/client/signing/account.rs @@ -105,7 +105,7 @@ async fn sign_account_state_transition() -> Result<()> { }; let unlocks = secret_manager - .sign_transaction_essence(&prepared_transaction_data) + .sign_transaction_essence(&prepared_transaction_data, &protocol_parameters) .await?; assert_eq!(unlocks.len(), 1); @@ -195,7 +195,7 @@ async fn sign_account_governance_transition() -> Result<()> { }; let unlocks = secret_manager - .sign_transaction_essence(&prepared_transaction_data) + .sign_transaction_essence(&prepared_transaction_data, &protocol_parameters) .await?; assert_eq!(unlocks.len(), 1); @@ -325,7 +325,7 @@ async fn account_reference_unlocks() -> Result<()> { }; let unlocks = secret_manager - .sign_transaction_essence(&prepared_transaction_data) + .sign_transaction_essence(&prepared_transaction_data, &protocol_parameters) .await?; assert_eq!(unlocks.len(), 3); diff --git a/sdk/tests/client/signing/basic.rs b/sdk/tests/client/signing/basic.rs index 44a4bfd1dd..8d884ae06c 100644 --- a/sdk/tests/client/signing/basic.rs +++ b/sdk/tests/client/signing/basic.rs @@ -88,7 +88,7 @@ async fn single_ed25519_unlock() -> Result<()> { }; let unlocks = secret_manager - .sign_transaction_essence(&prepared_transaction_data) + .sign_transaction_essence(&prepared_transaction_data, &protocol_parameters) .await?; assert_eq!(unlocks.len(), 1); @@ -189,7 +189,7 @@ async fn ed25519_reference_unlocks() -> Result<()> { }; let unlocks = secret_manager - .sign_transaction_essence(&prepared_transaction_data) + .sign_transaction_essence(&prepared_transaction_data, &protocol_parameters) .await?; assert_eq!(unlocks.len(), 3); @@ -300,7 +300,7 @@ async fn two_signature_unlocks() -> Result<()> { }; let unlocks = secret_manager - .sign_transaction_essence(&prepared_transaction_data) + .sign_transaction_essence(&prepared_transaction_data, &protocol_parameters) .await?; assert_eq!(unlocks.len(), 2); diff --git a/sdk/tests/client/signing/mod.rs b/sdk/tests/client/signing/mod.rs index 05b7a80927..8e6b4ba53f 100644 --- a/sdk/tests/client/signing/mod.rs +++ b/sdk/tests/client/signing/mod.rs @@ -395,7 +395,7 @@ async fn all_combined() -> Result<()> { }; let unlocks = secret_manager - .sign_transaction_essence(&prepared_transaction_data) + .sign_transaction_essence(&prepared_transaction_data, &protocol_parameters) .await?; assert_eq!(unlocks.len(), 15); diff --git a/sdk/tests/client/signing/nft.rs b/sdk/tests/client/signing/nft.rs index 17c6f4ec7a..66175119f8 100644 --- a/sdk/tests/client/signing/nft.rs +++ b/sdk/tests/client/signing/nft.rs @@ -132,7 +132,7 @@ async fn nft_reference_unlocks() -> Result<()> { }; let unlocks = secret_manager - .sign_transaction_essence(&prepared_transaction_data) + .sign_transaction_essence(&prepared_transaction_data, &protocol_parameters) .await?; assert_eq!(unlocks.len(), 3); From 7c89d62ef5ca789fd4c17e8b173154fd3d1d23ee Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Fri, 29 Sep 2023 14:26:07 +0200 Subject: [PATCH 12/17] Fixes --- sdk/src/client/api/block_builder/input_selection/mod.rs | 4 ++-- sdk/src/wallet/account/operations/helpers/time.rs | 8 ++++---- .../account/operations/transaction/input_selection.rs | 4 ++-- sdk/src/wallet/account/types/mod.rs | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/sdk/src/client/api/block_builder/input_selection/mod.rs b/sdk/src/client/api/block_builder/input_selection/mod.rs index 531d48f565..305c1fc084 100644 --- a/sdk/src/client/api/block_builder/input_selection/mod.rs +++ b/sdk/src/client/api/block_builder/input_selection/mod.rs @@ -271,8 +271,8 @@ impl InputSelection { mut inputs: Vec, outputs: &[Output], slot_index: SlotIndex, - min_committable_age: SlotIndex, - max_committable_age: SlotIndex, + min_committable_age: u64, + max_committable_age: u64, ) -> Result, Error> { // initially sort by output to make it deterministic // TODO: rethink this, we only need it deterministic for tests, for the protocol it doesn't matter, also there diff --git a/sdk/src/wallet/account/operations/helpers/time.rs b/sdk/src/wallet/account/operations/helpers/time.rs index 1363d35a23..540b3d679e 100644 --- a/sdk/src/wallet/account/operations/helpers/time.rs +++ b/sdk/src/wallet/account/operations/helpers/time.rs @@ -19,8 +19,8 @@ pub(crate) fn can_output_be_unlocked_now( account_and_nft_addresses: &[Address], output_data: &OutputData, slot_index: SlotIndex, - min_committable_age: SlotIndex, - max_committable_age: SlotIndex, + min_committable_age: u64, + max_committable_age: u64, account_transition: Option, ) -> crate::wallet::Result { if let Some(unlock_conditions) = output_data.output.unlock_conditions() { @@ -52,8 +52,8 @@ pub(crate) fn can_output_be_unlocked_forever_from_now_on( account_addresses: &[AddressWithUnspentOutputs], output: &Output, slot_index: SlotIndex, - min_committable_age: SlotIndex, - max_committable_age: SlotIndex, + min_committable_age: u64, + max_committable_age: u64, ) -> bool { if let Some(unlock_conditions) = output.unlock_conditions() { if unlock_conditions.is_timelocked(slot_index, min_committable_age) { diff --git a/sdk/src/wallet/account/operations/transaction/input_selection.rs b/sdk/src/wallet/account/operations/transaction/input_selection.rs index 1914aa38c7..68e42eb352 100644 --- a/sdk/src/wallet/account/operations/transaction/input_selection.rs +++ b/sdk/src/wallet/account/operations/transaction/input_selection.rs @@ -229,8 +229,8 @@ fn filter_inputs( account: &AccountDetails, available_outputs: Values<'_, OutputId, OutputData>, slot_index: SlotIndex, - min_committable_age: SlotIndex, - max_committable_age: SlotIndex, + min_committable_age: u64, + max_committable_age: u64, outputs: &[Output], burn: Option<&Burn>, custom_inputs: Option<&HashSet>, diff --git a/sdk/src/wallet/account/types/mod.rs b/sdk/src/wallet/account/types/mod.rs index 0f1143dd9b..8274466220 100644 --- a/sdk/src/wallet/account/types/mod.rs +++ b/sdk/src/wallet/account/types/mod.rs @@ -57,8 +57,8 @@ impl OutputData { &self, account: &AccountDetails, slot_index: SlotIndex, - min_committable_age: SlotIndex, - max_committable_age: SlotIndex, + min_committable_age: u64, + max_committable_age: u64, account_transition: Option, ) -> crate::wallet::Result> { let (unlock_address, _unlocked_account_or_nft_address) = self.output.required_and_unlocked_address( From e82ef304c9fce501c6d7ad8645fecc97bdf36ea7 Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Fri, 29 Sep 2023 14:57:29 +0200 Subject: [PATCH 13/17] Simplified required_address --- cli/src/command/account.rs | 3 +- .../api/block_builder/input_selection/mod.rs | 76 ++++++++---------- .../input_selection/remainder.rs | 6 +- .../input_selection/requirement/ed25519.rs | 11 +-- sdk/src/client/secret/ledger_nano.rs | 13 ++- sdk/src/client/secret/mod.rs | 11 ++- sdk/src/types/block/output/mod.rs | 79 +++++++++++-------- .../wallet/account/operations/helpers/time.rs | 20 ++--- sdk/src/wallet/account/types/mod.rs | 16 ++-- 9 files changed, 108 insertions(+), 127 deletions(-) diff --git a/cli/src/command/account.rs b/cli/src/command/account.rs index 9779fe28d6..b3bd883d13 100644 --- a/cli/src/command/account.rs +++ b/cli/src/command/account.rs @@ -951,11 +951,10 @@ async fn print_address(account: &Account, address: &Bip44Address) -> Result<(), for output_id in output_ids { if let Some(output_data) = account.get_output(output_id).await { // Output might be associated with the address, but can't be unlocked by it, so we check that here. - let (required_address, _) = output_data.output.required_and_unlocked_address( + let required_address = output_data.output.required_address( slot_index, protocol_parameters.min_committable_age(), protocol_parameters.max_committable_age(), - output_id, None, )?; diff --git a/sdk/src/client/api/block_builder/input_selection/mod.rs b/sdk/src/client/api/block_builder/input_selection/mod.rs index 305c1fc084..a3def0400e 100644 --- a/sdk/src/client/api/block_builder/input_selection/mod.rs +++ b/sdk/src/client/api/block_builder/input_selection/mod.rs @@ -61,16 +61,12 @@ impl InputSelection { fn required_account_nft_addresses(&self, input: &InputSigningData) -> Result, Error> { let account_transition = is_account_transition(&input.output, *input.output_id(), &self.outputs, self.burn.as_ref()); - let required_address = input - .output - .required_and_unlocked_address( - self.slot_index, - self.protocol_parameters.min_committable_age(), - self.protocol_parameters.max_committable_age(), - input.output_id(), - account_transition, - )? - .0; + let required_address = input.output.required_address( + self.slot_index, + self.protocol_parameters.min_committable_age(), + self.protocol_parameters.max_committable_age(), + account_transition, + )?; match required_address { Address::Ed25519(_) => { @@ -251,16 +247,14 @@ impl InputSelection { let required_address = input .output // Account transition is irrelevant here as we keep accounts anyway. - .required_and_unlocked_address( + .required_address( self.slot_index, self.protocol_parameters.min_committable_age(), self.protocol_parameters.max_committable_age(), - input.output_id(), None, ) // PANIC: safe to unwrap as non basic/account/foundry/nft outputs are already filtered out. - .unwrap() - .0; + .unwrap(); self.addresses.contains(&required_address) }) @@ -287,15 +281,9 @@ impl InputSelection { outputs, None, ); - let (input_address, _) = input_signing_data + let input_address = input_signing_data .output - .required_and_unlocked_address( - slot_index, - min_committable_age, - max_committable_age, - input_signing_data.output_id(), - account_transition, - ) + .required_address(slot_index, min_committable_age, max_committable_age, account_transition) // PANIC: safe to unwrap, because we filtered irrelevant outputs out before .unwrap(); @@ -304,32 +292,33 @@ impl InputSelection { for input in account_nft_address_inputs { let account_transition = is_account_transition(&input.output, *input.output_id(), outputs, None); - let (input_address, _) = input.output.required_and_unlocked_address( + let required_address = input.output.required_address( slot_index, min_committable_age, max_committable_age, - input.output_id(), account_transition, )?; - match sorted_inputs.iter().position(|input_signing_data| match input_address { - Address::Account(unlock_address) => { - if let Output::Account(account_output) = &input_signing_data.output { - *unlock_address.account_id() - == account_output.account_id_non_null(input_signing_data.output_id()) - } else { - false + match sorted_inputs + .iter() + .position(|input_signing_data| match required_address { + Address::Account(unlock_address) => { + if let Output::Account(account_output) = &input_signing_data.output { + *unlock_address.account_id() + == account_output.account_id_non_null(input_signing_data.output_id()) + } else { + false + } } - } - Address::Nft(unlock_address) => { - if let Output::Nft(nft_output) = &input_signing_data.output { - *unlock_address.nft_id() == nft_output.nft_id_non_null(input_signing_data.output_id()) - } else { - false + Address::Nft(unlock_address) => { + if let Output::Nft(nft_output) = &input_signing_data.output { + *unlock_address.nft_id() == nft_output.nft_id_non_null(input_signing_data.output_id()) + } else { + false + } } - } - _ => false, - }) { + _ => false, + }) { Some(position) => { // Insert after the output we need sorted_inputs.insert(position + 1, input); @@ -355,19 +344,18 @@ impl InputSelection { outputs, None, ); - let (input_address, _) = input_signing_data + let required_address = input_signing_data .output - .required_and_unlocked_address( + .required_address( slot_index, min_committable_age, max_committable_age, - input.output_id(), account_transition, ) // PANIC: safe to unwrap, because we filtered irrelevant outputs out before .unwrap(); - input_address == account_or_nft_address + required_address == account_or_nft_address }) { Some(position) => { // Insert before the output with this address required for unlocking diff --git a/sdk/src/client/api/block_builder/input_selection/remainder.rs b/sdk/src/client/api/block_builder/input_selection/remainder.rs index 996110cabd..ad6e1ba6d4 100644 --- a/sdk/src/client/api/block_builder/input_selection/remainder.rs +++ b/sdk/src/client/api/block_builder/input_selection/remainder.rs @@ -36,15 +36,13 @@ impl InputSelection { // PANIC: safe to unwrap as outputs with no address have been filtered out already. let required_address = input .output - .required_and_unlocked_address( + .required_address( self.slot_index, self.protocol_parameters.min_committable_age(), self.protocol_parameters.max_committable_age(), - input.output_id(), account_transition, ) - .unwrap() - .0; + .unwrap(); if required_address.is_ed25519() { return Some((required_address, input.chain)); diff --git a/sdk/src/client/api/block_builder/input_selection/requirement/ed25519.rs b/sdk/src/client/api/block_builder/input_selection/requirement/ed25519.rs index 1d3bf12e78..4d7d6d2be3 100644 --- a/sdk/src/client/api/block_builder/input_selection/requirement/ed25519.rs +++ b/sdk/src/client/api/block_builder/input_selection/requirement/ed25519.rs @@ -20,15 +20,13 @@ impl InputSelection { // PANIC: safe to unwrap as outputs with no address have been filtered out already. let required_address = input .output - .required_and_unlocked_address( + .required_address( self.slot_index, self.protocol_parameters.min_committable_age(), self.protocol_parameters.max_committable_age(), - input.output_id(), account_transition, ) - .unwrap() - .0; + .unwrap(); if account_transition.is_some() { // Only check if we own the required address if the input is an account because other types of output have @@ -62,13 +60,12 @@ impl InputSelection { (false, None) } else { - let (required_address, _) = input + let required_address = input .output - .required_and_unlocked_address( + .required_address( self.slot_index, self.protocol_parameters.min_committable_age(), self.protocol_parameters.max_committable_age(), - input.output_id(), None, ) .unwrap(); diff --git a/sdk/src/client/secret/ledger_nano.rs b/sdk/src/client/secret/ledger_nano.rs index 8b86455af6..c0739b0bcf 100644 --- a/sdk/src/client/secret/ledger_nano.rs +++ b/sdk/src/client/secret/ledger_nano.rs @@ -531,18 +531,17 @@ fn merge_unlocks( // Get the address that is required to unlock the input let TransactionEssence::Regular(regular) = &prepared_transaction_data.essence; let account_transition = is_account_transition(&input.output, *input.output_id(), regular.outputs(), None); - let (input_address, _) = input.output.required_and_unlocked_address( + let required_address = input.output.required_address( slot_index, protocol_parameters.min_committable_age(), protocol_parameters.max_committable_age(), - input.output_metadata.output_id(), account_transition, )?; // Check if we already added an [Unlock] for this address - match block_indexes.get(&input_address) { + match block_indexes.get(&required_address) { // If we already have an [Unlock] for this address, add a [Unlock] based on the address type - Some(block_index) => match input_address { + Some(block_index) => match required_address { Address::Account(_account) => { merged_unlocks.push(Unlock::Account(AccountUnlock::new(*block_index as u16)?)) } @@ -555,7 +554,7 @@ fn merge_unlocks( // We can only sign ed25519 addresses and block_indexes needs to contain the account or nft // address already at this point, because the reference index needs to be lower // than the current block index - if !input_address.is_ed25519() { + if !required_address.is_ed25519() { return Err(Error::MissingInputWithEd25519Address); } @@ -563,7 +562,7 @@ fn merge_unlocks( if let Unlock::Signature(signature_unlock) = &unlock { let Signature::Ed25519(ed25519_signature) = signature_unlock.signature(); - let ed25519_address = match input_address { + let ed25519_address = match required_address { Address::Ed25519(ed25519_address) => ed25519_address, _ => return Err(Error::MissingInputWithEd25519Address), }; @@ -574,7 +573,7 @@ fn merge_unlocks( // Add the ed25519 address to the block_indexes, so it gets referenced if further inputs have // the same address in their unlock condition - block_indexes.insert(input_address, current_block_index); + block_indexes.insert(required_address, current_block_index); } } diff --git a/sdk/src/client/secret/mod.rs b/sdk/src/client/secret/mod.rs index eac35c43f8..f14b7aa1a5 100644 --- a/sdk/src/client/secret/mod.rs +++ b/sdk/src/client/secret/mod.rs @@ -541,18 +541,17 @@ where // Get the address that is required to unlock the input let TransactionEssence::Regular(regular) = &prepared_transaction_data.essence; let account_transition = is_account_transition(&input.output, *input.output_id(), regular.outputs(), None); - let (input_address, _) = input.output.required_and_unlocked_address( + let required_address = input.output.required_address( slot_index, protocol_parameters.min_committable_age(), protocol_parameters.max_committable_age(), - input.output_metadata.output_id(), account_transition, )?; // Check if we already added an [Unlock] for this address - match block_indexes.get(&input_address) { + match block_indexes.get(&required_address) { // If we already have an [Unlock] for this address, add a [Unlock] based on the address type - Some(block_index) => match input_address { + Some(block_index) => match required_address { Address::Account(_account) => blocks.push(Unlock::Account(AccountUnlock::new(*block_index as u16)?)), Address::Ed25519(_ed25519) => { blocks.push(Unlock::Reference(ReferenceUnlock::new(*block_index as u16)?)); @@ -563,7 +562,7 @@ where // We can only sign ed25519 addresses and block_indexes needs to contain the account or nft // address already at this point, because the reference index needs to be lower // than the current block index - if !input_address.is_ed25519() { + if !required_address.is_ed25519() { Err(InputSelectionError::MissingInputWithEd25519Address)?; } @@ -574,7 +573,7 @@ where // Add the ed25519 address to the block_indexes, so it gets referenced if further inputs have // the same address in their unlock condition - block_indexes.insert(input_address, current_block_index); + block_indexes.insert(required_address, current_block_index); } } diff --git a/sdk/src/types/block/output/mod.rs b/sdk/src/types/block/output/mod.rs index a54e89c279..0ce0ed16e2 100644 --- a/sdk/src/types/block/output/mod.rs +++ b/sdk/src/types/block/output/mod.rs @@ -304,57 +304,66 @@ impl Output { } } - /// Returns the address that is required to unlock this [`Output`] and the account or nft address that gets - /// unlocked by it, if it's an account or nft. + /// Returns the address that is required to unlock this [`Output`]. /// If no `account_transition` has been provided, assumes a state transition. - pub fn required_and_unlocked_address( + pub fn required_address( &self, slot_index: impl Into, min_committable_age: impl Into, max_committable_age: impl Into, - output_id: &OutputId, account_transition: Option, - ) -> Result<(Address, Option
), Error> { + ) -> Result { match self { - Self::Basic(output) => Ok(( - *output - .unlock_conditions() - .locked_address(output.address(), slot_index, min_committable_age, max_committable_age) - // TODO - .unwrap(), - None, - )), + Self::Basic(output) => Ok(*output + .unlock_conditions() + .locked_address(output.address(), slot_index, min_committable_age, max_committable_age) + // TODO + .unwrap()), Self::Account(output) => { if account_transition.unwrap_or(AccountTransition::State) == AccountTransition::State { // Account address is only unlocked if it's a state transition - Ok(( - *output.state_controller_address(), - Some(Address::Account(output.account_address(output_id))), - )) + Ok(*output.state_controller_address()) } else { - Ok((*output.governor_address(), None)) + Ok(*output.governor_address()) } } - Self::Foundry(output) => Ok((Address::Account(*output.account_address()), None)), - Self::Nft(output) => Ok(( - *output - .unlock_conditions() - .locked_address(output.address(), slot_index, min_committable_age, max_committable_age) - // TODO - .unwrap(), - Some(Address::Nft(output.nft_address(output_id))), - )), - Self::Delegation(output) => Ok(( - *output - .unlock_conditions() - .locked_address(output.address(), slot_index, min_committable_age, max_committable_age) - // TODO - .unwrap(), - None, - )), + Self::Foundry(output) => Ok(Address::Account(*output.account_address())), + Self::Nft(output) => Ok(*output + .unlock_conditions() + .locked_address(output.address(), slot_index, min_committable_age, max_committable_age) + // TODO + .unwrap()), + Self::Delegation(output) => Ok(*output + .unlock_conditions() + .locked_address(output.address(), slot_index, min_committable_age, max_committable_age) + // TODO + .unwrap()), } } + // /// Returns the address that is required to unlock this [`Output`] and the account or nft address that gets + // /// unlocked by it, if it's an account or nft. + // /// If no `account_transition` has been provided, assumes a state transition. + // pub fn required_and_unlocked_address( + // &self, + // slot_index: impl Into, + // min_committable_age: impl Into, + // max_committable_age: impl Into, + // output_id: &OutputId, + // account_transition: Option, + // ) -> Result<(Address, Option
), Error> { match self { Self::Basic(output) => Ok(( *output + // .unlock_conditions() .locked_address(output.address(), slot_index, min_committable_age, max_committable_age) // + // TODO .unwrap(), None, )), Self::Account(output) => { if account_transition.unwrap_or(AccountTransition::State) + // == AccountTransition::State { // Account address is only unlocked if it's a state transition Ok(( + // *output.state_controller_address(), Some(Address::Account(output.account_address(output_id))), )) } else { + // Ok((*output.governor_address(), None)) } } Self::Foundry(output) => + // Ok((Address::Account(*output.account_address()), None)), Self::Nft(output) => Ok(( *output .unlock_conditions() + // .locked_address(output.address(), slot_index, min_committable_age, max_committable_age) // TODO .unwrap(), + // Some(Address::Nft(output.nft_address(output_id))), )), Self::Delegation(output) => Ok(( *output + // .unlock_conditions() .locked_address(output.address(), slot_index, min_committable_age, max_committable_age) // + // TODO .unwrap(), None, )), } + // } + /// pub fn verify_state_transition( current_state: Option<&Self>, diff --git a/sdk/src/wallet/account/operations/helpers/time.rs b/sdk/src/wallet/account/operations/helpers/time.rs index 540b3d679e..cc8b7561fd 100644 --- a/sdk/src/wallet/account/operations/helpers/time.rs +++ b/sdk/src/wallet/account/operations/helpers/time.rs @@ -29,19 +29,15 @@ pub(crate) fn can_output_be_unlocked_now( } } - let (required_unlock_address, _unlocked_account_or_nft_address) = - output_data.output.required_and_unlocked_address( - slot_index, - min_committable_age, - max_committable_age, - &output_data.output_id, - account_transition, - )?; + let required_address = output_data.output.required_address( + slot_index, + min_committable_age, + max_committable_age, + account_transition, + )?; - Ok(account_addresses - .iter() - .any(|a| a.address.inner == required_unlock_address) - || account_and_nft_addresses.iter().any(|a| *a == required_unlock_address)) + Ok(account_addresses.iter().any(|a| a.address.inner == required_address) + || account_and_nft_addresses.iter().any(|a| *a == required_address)) } // Check if an output can be unlocked by one of the account addresses at the current time and at any diff --git a/sdk/src/wallet/account/types/mod.rs b/sdk/src/wallet/account/types/mod.rs index 8274466220..8dbb38b2b9 100644 --- a/sdk/src/wallet/account/types/mod.rs +++ b/sdk/src/wallet/account/types/mod.rs @@ -61,21 +61,17 @@ impl OutputData { max_committable_age: u64, account_transition: Option, ) -> crate::wallet::Result> { - let (unlock_address, _unlocked_account_or_nft_address) = self.output.required_and_unlocked_address( - slot_index, - min_committable_age, - max_committable_age, - &self.output_id, - account_transition, - )?; + let required_address = + self.output + .required_address(slot_index, min_committable_age, max_committable_age, account_transition)?; - let chain = if unlock_address == self.address { + let chain = if required_address == self.address { self.chain - } else if let Address::Ed25519(_) = unlock_address { + } else if let Address::Ed25519(_) = required_address { if let Some(address) = account .addresses_with_unspent_outputs .iter() - .find(|a| a.address.inner == unlock_address) + .find(|a| a.address.inner == required_address) { Some( Bip44::new(account.coin_type) From b7c671176ad330fc43397fdafa387afabd8be688 Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Fri, 29 Sep 2023 15:29:35 +0200 Subject: [PATCH 14/17] Infallible required_address --- cli/src/command/account.rs | 16 +++++---- .../api/block_builder/input_selection/mod.rs | 33 ++++++++++--------- .../input_selection/remainder.rs | 1 + .../input_selection/requirement/ed25519.rs | 2 ++ sdk/src/client/secret/ledger_nano.rs | 16 +++++---- sdk/src/client/secret/mod.rs | 16 +++++---- sdk/src/types/block/output/mod.rs | 33 +++++++++---------- .../wallet/account/operations/helpers/time.rs | 11 +++---- sdk/src/wallet/account/types/mod.rs | 8 +++-- 9 files changed, 77 insertions(+), 59 deletions(-) diff --git a/cli/src/command/account.rs b/cli/src/command/account.rs index b3bd883d13..0319370782 100644 --- a/cli/src/command/account.rs +++ b/cli/src/command/account.rs @@ -951,12 +951,16 @@ async fn print_address(account: &Account, address: &Bip44Address) -> Result<(), for output_id in output_ids { if let Some(output_data) = account.get_output(output_id).await { // Output might be associated with the address, but can't be unlocked by it, so we check that here. - let required_address = output_data.output.required_address( - slot_index, - protocol_parameters.min_committable_age(), - protocol_parameters.max_committable_age(), - None, - )?; + let required_address = output_data + .output + .required_address( + slot_index, + protocol_parameters.min_committable_age(), + protocol_parameters.max_committable_age(), + None, + ) + // TODO + .unwrap(); if address.address().as_ref() == &required_address { if let Some(nts) = output_data.output.native_tokens() { diff --git a/sdk/src/client/api/block_builder/input_selection/mod.rs b/sdk/src/client/api/block_builder/input_selection/mod.rs index a3def0400e..284142c471 100644 --- a/sdk/src/client/api/block_builder/input_selection/mod.rs +++ b/sdk/src/client/api/block_builder/input_selection/mod.rs @@ -61,12 +61,16 @@ impl InputSelection { fn required_account_nft_addresses(&self, input: &InputSigningData) -> Result, Error> { let account_transition = is_account_transition(&input.output, *input.output_id(), &self.outputs, self.burn.as_ref()); - let required_address = input.output.required_address( - self.slot_index, - self.protocol_parameters.min_committable_age(), - self.protocol_parameters.max_committable_age(), - account_transition, - )?; + let required_address = input + .output + .required_address( + self.slot_index, + self.protocol_parameters.min_committable_age(), + self.protocol_parameters.max_committable_age(), + account_transition, + ) + // TODO + .unwrap(); match required_address { Address::Ed25519(_) => { @@ -253,7 +257,7 @@ impl InputSelection { self.protocol_parameters.max_committable_age(), None, ) - // PANIC: safe to unwrap as non basic/account/foundry/nft outputs are already filtered out. + // TODO .unwrap(); self.addresses.contains(&required_address) @@ -284,7 +288,7 @@ impl InputSelection { let input_address = input_signing_data .output .required_address(slot_index, min_committable_age, max_committable_age, account_transition) - // PANIC: safe to unwrap, because we filtered irrelevant outputs out before + // TODO .unwrap(); input_address.is_ed25519() @@ -292,12 +296,11 @@ impl InputSelection { for input in account_nft_address_inputs { let account_transition = is_account_transition(&input.output, *input.output_id(), outputs, None); - let required_address = input.output.required_address( - slot_index, - min_committable_age, - max_committable_age, - account_transition, - )?; + let required_address = input + .output + .required_address(slot_index, min_committable_age, max_committable_age, account_transition) + // TODO + .unwrap(); match sorted_inputs .iter() @@ -352,7 +355,7 @@ impl InputSelection { max_committable_age, account_transition, ) - // PANIC: safe to unwrap, because we filtered irrelevant outputs out before + // TODO .unwrap(); required_address == account_or_nft_address diff --git a/sdk/src/client/api/block_builder/input_selection/remainder.rs b/sdk/src/client/api/block_builder/input_selection/remainder.rs index ad6e1ba6d4..7766769a14 100644 --- a/sdk/src/client/api/block_builder/input_selection/remainder.rs +++ b/sdk/src/client/api/block_builder/input_selection/remainder.rs @@ -42,6 +42,7 @@ impl InputSelection { self.protocol_parameters.max_committable_age(), account_transition, ) + // TODO .unwrap(); if required_address.is_ed25519() { diff --git a/sdk/src/client/api/block_builder/input_selection/requirement/ed25519.rs b/sdk/src/client/api/block_builder/input_selection/requirement/ed25519.rs index 4d7d6d2be3..3336e7a3ad 100644 --- a/sdk/src/client/api/block_builder/input_selection/requirement/ed25519.rs +++ b/sdk/src/client/api/block_builder/input_selection/requirement/ed25519.rs @@ -26,6 +26,7 @@ impl InputSelection { self.protocol_parameters.max_committable_age(), account_transition, ) + // TODO .unwrap(); if account_transition.is_some() { @@ -68,6 +69,7 @@ impl InputSelection { self.protocol_parameters.max_committable_age(), None, ) + // TODO .unwrap(); (&required_address == address, None) diff --git a/sdk/src/client/secret/ledger_nano.rs b/sdk/src/client/secret/ledger_nano.rs index c0739b0bcf..cc21869e3a 100644 --- a/sdk/src/client/secret/ledger_nano.rs +++ b/sdk/src/client/secret/ledger_nano.rs @@ -531,12 +531,16 @@ fn merge_unlocks( // Get the address that is required to unlock the input let TransactionEssence::Regular(regular) = &prepared_transaction_data.essence; let account_transition = is_account_transition(&input.output, *input.output_id(), regular.outputs(), None); - let required_address = input.output.required_address( - slot_index, - protocol_parameters.min_committable_age(), - protocol_parameters.max_committable_age(), - account_transition, - )?; + let required_address = input + .output + .required_address( + slot_index, + protocol_parameters.min_committable_age(), + protocol_parameters.max_committable_age(), + account_transition, + ) + // TODO + .unwrap(); // Check if we already added an [Unlock] for this address match block_indexes.get(&required_address) { diff --git a/sdk/src/client/secret/mod.rs b/sdk/src/client/secret/mod.rs index f14b7aa1a5..dfb25d880f 100644 --- a/sdk/src/client/secret/mod.rs +++ b/sdk/src/client/secret/mod.rs @@ -541,12 +541,16 @@ where // Get the address that is required to unlock the input let TransactionEssence::Regular(regular) = &prepared_transaction_data.essence; let account_transition = is_account_transition(&input.output, *input.output_id(), regular.outputs(), None); - let required_address = input.output.required_address( - slot_index, - protocol_parameters.min_committable_age(), - protocol_parameters.max_committable_age(), - account_transition, - )?; + let required_address = input + .output + .required_address( + slot_index, + protocol_parameters.min_committable_age(), + protocol_parameters.max_committable_age(), + account_transition, + ) + // TODO + .unwrap(); // Check if we already added an [Unlock] for this address match block_indexes.get(&required_address) { diff --git a/sdk/src/types/block/output/mod.rs b/sdk/src/types/block/output/mod.rs index 0ce0ed16e2..61abeed3d2 100644 --- a/sdk/src/types/block/output/mod.rs +++ b/sdk/src/types/block/output/mod.rs @@ -312,32 +312,31 @@ impl Output { min_committable_age: impl Into, max_committable_age: impl Into, account_transition: Option, - ) -> Result { + ) -> Option
{ match self { - Self::Basic(output) => Ok(*output + Self::Basic(output) => output .unlock_conditions() .locked_address(output.address(), slot_index, min_committable_age, max_committable_age) - // TODO - .unwrap()), + .copied(), Self::Account(output) => { - if account_transition.unwrap_or(AccountTransition::State) == AccountTransition::State { - // Account address is only unlocked if it's a state transition - Ok(*output.state_controller_address()) - } else { - Ok(*output.governor_address()) - } + Some( + if account_transition.unwrap_or(AccountTransition::State) == AccountTransition::State { + // Account address is only unlocked if it's a state transition + *output.state_controller_address() + } else { + *output.governor_address() + }, + ) } - Self::Foundry(output) => Ok(Address::Account(*output.account_address())), - Self::Nft(output) => Ok(*output + Self::Foundry(output) => Some(Address::Account(*output.account_address())), + Self::Nft(output) => output .unlock_conditions() .locked_address(output.address(), slot_index, min_committable_age, max_committable_age) - // TODO - .unwrap()), - Self::Delegation(output) => Ok(*output + .copied(), + Self::Delegation(output) => output .unlock_conditions() .locked_address(output.address(), slot_index, min_committable_age, max_committable_age) - // TODO - .unwrap()), + .copied(), } } diff --git a/sdk/src/wallet/account/operations/helpers/time.rs b/sdk/src/wallet/account/operations/helpers/time.rs index cc8b7561fd..657e67d3c8 100644 --- a/sdk/src/wallet/account/operations/helpers/time.rs +++ b/sdk/src/wallet/account/operations/helpers/time.rs @@ -29,12 +29,11 @@ pub(crate) fn can_output_be_unlocked_now( } } - let required_address = output_data.output.required_address( - slot_index, - min_committable_age, - max_committable_age, - account_transition, - )?; + let required_address = output_data + .output + .required_address(slot_index, min_committable_age, max_committable_age, account_transition) + // TODO + .unwrap(); Ok(account_addresses.iter().any(|a| a.address.inner == required_address) || account_and_nft_addresses.iter().any(|a| *a == required_address)) diff --git a/sdk/src/wallet/account/types/mod.rs b/sdk/src/wallet/account/types/mod.rs index 8dbb38b2b9..2608cab68c 100644 --- a/sdk/src/wallet/account/types/mod.rs +++ b/sdk/src/wallet/account/types/mod.rs @@ -61,9 +61,11 @@ impl OutputData { max_committable_age: u64, account_transition: Option, ) -> crate::wallet::Result> { - let required_address = - self.output - .required_address(slot_index, min_committable_age, max_committable_age, account_transition)?; + let required_address = self + .output + .required_address(slot_index, min_committable_age, max_committable_age, account_transition) + // TODO + .unwrap(); let chain = if required_address == self.address { self.chain From fe4cc97fe8bbe238899b5258d609fcbd168a97c8 Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Mon, 16 Oct 2023 18:05:44 +0200 Subject: [PATCH 15/17] Some fixes --- sdk/src/client/api/block_builder/input_selection/mod.rs | 4 ++-- sdk/src/types/block/output/mod.rs | 6 +++--- sdk/src/types/block/semantic.rs | 2 +- sdk/src/types/block/slot/commitment_id.rs | 2 +- sdk/src/wallet/account/operations/helpers/time.rs | 8 ++++---- .../account/operations/transaction/input_selection.rs | 4 ++-- sdk/src/wallet/account/types/mod.rs | 4 ++-- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/sdk/src/client/api/block_builder/input_selection/mod.rs b/sdk/src/client/api/block_builder/input_selection/mod.rs index 5ae36e0246..a7e4c1fd0b 100644 --- a/sdk/src/client/api/block_builder/input_selection/mod.rs +++ b/sdk/src/client/api/block_builder/input_selection/mod.rs @@ -270,8 +270,8 @@ impl InputSelection { mut inputs: Vec, outputs: &[Output], slot_index: SlotIndex, - min_committable_age: u64, - max_committable_age: u64, + min_committable_age: u32, + max_committable_age: u32, ) -> Result, Error> { // initially sort by output to make it deterministic // TODO: rethink this, we only need it deterministic for tests, for the protocol it doesn't matter, also there diff --git a/sdk/src/types/block/output/mod.rs b/sdk/src/types/block/output/mod.rs index 18c5498a4f..2e2e3751ca 100644 --- a/sdk/src/types/block/output/mod.rs +++ b/sdk/src/types/block/output/mod.rs @@ -322,9 +322,9 @@ impl Output { Some( if account_transition.unwrap_or(AccountTransition::State) == AccountTransition::State { // Account address is only unlocked if it's a state transition - *output.state_controller_address() + output.state_controller_address().clone() } else { - *output.governor_address() + output.governor_address().clone() }, ) } @@ -336,7 +336,7 @@ impl Output { Self::Delegation(output) => output .unlock_conditions() .locked_address(output.address(), slot_index, min_committable_age, max_committable_age) - .copied(), + .cloned(), } } diff --git a/sdk/src/types/block/semantic.rs b/sdk/src/types/block/semantic.rs index f8a2024c15..6448c1b590 100644 --- a/sdk/src/types/block/semantic.rs +++ b/sdk/src/types/block/semantic.rs @@ -302,7 +302,7 @@ pub fn semantic_validation( if let Some(storage_deposit_return) = unlock_conditions.storage_deposit_return() { let amount = context .storage_deposit_returns - .entry(*storage_deposit_return.return_address()) + .entry(storage_deposit_return.return_address().clone()) .or_default(); *amount = amount diff --git a/sdk/src/types/block/slot/commitment_id.rs b/sdk/src/types/block/slot/commitment_id.rs index 3a3a677fb5..f92774f957 100644 --- a/sdk/src/types/block/slot/commitment_id.rs +++ b/sdk/src/types/block/slot/commitment_id.rs @@ -8,7 +8,7 @@ impl_id!(pub SlotCommitmentId, 40, "Identifier of a slot commitment."); impl SlotCommitmentId { pub fn index(&self) -> SlotIndex { // PANIC: taking the last 8 bytes of 40 bytes is safe. - u64::from_le_bytes( + u32::from_le_bytes( self.0[Self::LENGTH - core::mem::size_of::()..] .try_into() .unwrap(), diff --git a/sdk/src/wallet/account/operations/helpers/time.rs b/sdk/src/wallet/account/operations/helpers/time.rs index 657e67d3c8..c342dc7f05 100644 --- a/sdk/src/wallet/account/operations/helpers/time.rs +++ b/sdk/src/wallet/account/operations/helpers/time.rs @@ -19,8 +19,8 @@ pub(crate) fn can_output_be_unlocked_now( account_and_nft_addresses: &[Address], output_data: &OutputData, slot_index: SlotIndex, - min_committable_age: u64, - max_committable_age: u64, + min_committable_age: u32, + max_committable_age: u32, account_transition: Option, ) -> crate::wallet::Result { if let Some(unlock_conditions) = output_data.output.unlock_conditions() { @@ -47,8 +47,8 @@ pub(crate) fn can_output_be_unlocked_forever_from_now_on( account_addresses: &[AddressWithUnspentOutputs], output: &Output, slot_index: SlotIndex, - min_committable_age: u64, - max_committable_age: u64, + min_committable_age: u32, + max_committable_age: u32, ) -> bool { if let Some(unlock_conditions) = output.unlock_conditions() { if unlock_conditions.is_timelocked(slot_index, min_committable_age) { diff --git a/sdk/src/wallet/account/operations/transaction/input_selection.rs b/sdk/src/wallet/account/operations/transaction/input_selection.rs index 8be7657506..514213a384 100644 --- a/sdk/src/wallet/account/operations/transaction/input_selection.rs +++ b/sdk/src/wallet/account/operations/transaction/input_selection.rs @@ -229,8 +229,8 @@ fn filter_inputs( account: &AccountDetails, available_outputs: Values<'_, OutputId, OutputData>, slot_index: SlotIndex, - min_committable_age: u64, - max_committable_age: u64, + min_committable_age: u32, + max_committable_age: u32, outputs: &[Output], burn: Option<&Burn>, custom_inputs: Option<&HashSet>, diff --git a/sdk/src/wallet/account/types/mod.rs b/sdk/src/wallet/account/types/mod.rs index 9429824083..e303d9f5e6 100644 --- a/sdk/src/wallet/account/types/mod.rs +++ b/sdk/src/wallet/account/types/mod.rs @@ -57,8 +57,8 @@ impl OutputData { &self, account: &AccountDetails, slot_index: SlotIndex, - min_committable_age: u64, - max_committable_age: u64, + min_committable_age: u32, + max_committable_age: u32, account_transition: Option, ) -> crate::wallet::Result> { let required_address = self From c06f0dd85914af577a8f8464bfb25543a3e7aa4b Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Tue, 17 Oct 2023 13:26:46 +0200 Subject: [PATCH 16/17] Nit --- sdk/src/types/block/slot/commitment_id.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/types/block/slot/commitment_id.rs b/sdk/src/types/block/slot/commitment_id.rs index f92774f957..b87835fdcf 100644 --- a/sdk/src/types/block/slot/commitment_id.rs +++ b/sdk/src/types/block/slot/commitment_id.rs @@ -7,7 +7,7 @@ impl_id!(pub SlotCommitmentId, 40, "Identifier of a slot commitment."); impl SlotCommitmentId { pub fn index(&self) -> SlotIndex { - // PANIC: taking the last 8 bytes of 40 bytes is safe. + // PANIC: taking the last 8 bytes of an array of more than 8 bytes is safe. u32::from_le_bytes( self.0[Self::LENGTH - core::mem::size_of::()..] .try_into() From a1e69da46c031c82ed758baa4550bc55a98e9614 Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Tue, 14 Nov 2023 12:27:09 +0100 Subject: [PATCH 17/17] Remove commented code --- sdk/src/types/block/output/mod.rs | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/sdk/src/types/block/output/mod.rs b/sdk/src/types/block/output/mod.rs index dde662812f..999245a7a4 100644 --- a/sdk/src/types/block/output/mod.rs +++ b/sdk/src/types/block/output/mod.rs @@ -293,29 +293,6 @@ impl Output { } } - // /// Returns the address that is required to unlock this [`Output`] and the account or nft address that gets - // /// unlocked by it, if it's an account or nft. - // /// If no `account_transition` has been provided, assumes a state transition. - // pub fn required_and_unlocked_address( - // &self, - // slot_index: impl Into, - // min_committable_age: impl Into, - // max_committable_age: impl Into, - // output_id: &OutputId, - // account_transition: Option, - // ) -> Result<(Address, Option
), Error> { match self { Self::Basic(output) => Ok(( *output - // .unlock_conditions() .locked_address(output.address(), slot_index, min_committable_age, max_committable_age) // - // TODO .unwrap(), None, )), Self::Account(output) => { if account_transition.unwrap_or(AccountTransition::State) - // == AccountTransition::State { // Account address is only unlocked if it's a state transition Ok(( - // *output.state_controller_address(), Some(Address::Account(output.account_address(output_id))), )) } else { - // Ok((*output.governor_address(), None)) } } Self::Foundry(output) => - // Ok((Address::Account(*output.account_address()), None)), Self::Nft(output) => Ok(( *output .unlock_conditions() - // .locked_address(output.address(), slot_index, min_committable_age, max_committable_age) // TODO .unwrap(), - // Some(Address::Nft(output.nft_address(output_id))), )), Self::Delegation(output) => Ok(( *output - // .unlock_conditions() .locked_address(output.address(), slot_index, min_committable_age, max_committable_age) // - // TODO .unwrap(), None, )), } - // } - /// pub fn verify_state_transition( current_state: Option<&Self>,