From 5796fb878e620f37151a09aa1d41b1d78657ddc8 Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Tue, 23 Jan 2024 14:22:15 +0100 Subject: [PATCH 1/3] Context inputs lexical ordering --- Cargo.lock | 10 ++++- sdk/Cargo.toml | 2 +- .../context_input/block_issuance_credit.rs | 6 +-- .../payload/signed_transaction/transaction.rs | 43 ++++++++++++------- 4 files changed, 40 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a05681800f..e17b07094f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1574,7 +1574,7 @@ dependencies = [ "getrandom", "hkdf", "hmac", - "iterator-sorted", + "iterator-sorted 0.1.0", "k256", "pbkdf2", "rand", @@ -1626,7 +1626,7 @@ dependencies = [ "iota-ledger-nano", "iota-sdk", "iota_stronghold", - "iterator-sorted", + "iterator-sorted 0.2.0", "lazy_static", "log", "num_cpus", @@ -1761,6 +1761,12 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d101775d2bc8f99f4ac18bf29b9ed70c0dd138b9a1e88d7b80179470cbbe8bd2" +[[package]] +name = "iterator-sorted" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed3c1d66191fc266439b989dc1a9a69d9c4156e803ce456221231398b84c35d1" + [[package]] name = "itoa" version = "1.0.10" diff --git a/sdk/Cargo.toml b/sdk/Cargo.toml index f72ad700df..e8e20d6ca6 100644 --- a/sdk/Cargo.toml +++ b/sdk/Cargo.toml @@ -44,7 +44,7 @@ iota-crypto = { version = "0.23.0", default-features = false, features = [ "ed25519", "secp256k1", ] } -iterator-sorted = { version = "0.1.0", default-features = false } +iterator-sorted = { version = "0.2.0", default-features = false } packable = { version = "0.10.1", default-features = false, features = [ "primitive-types", ] } diff --git a/sdk/src/types/block/context_input/block_issuance_credit.rs b/sdk/src/types/block/context_input/block_issuance_credit.rs index fececf43f9..ce5f7a2283 100644 --- a/sdk/src/types/block/context_input/block_issuance_credit.rs +++ b/sdk/src/types/block/context_input/block_issuance_credit.rs @@ -23,8 +23,8 @@ impl BlockIssuanceCreditContextInput { } /// Returns the account id of the [`BlockIssuanceCreditContextInput`]. - pub fn account_id(&self) -> AccountId { - self.0 + pub fn account_id(&self) -> &AccountId { + &self.0 } } @@ -54,7 +54,7 @@ mod dto { fn from(value: &BlockIssuanceCreditContextInput) -> Self { Self { kind: BlockIssuanceCreditContextInput::KIND, - account_id: value.account_id(), + account_id: *value.account_id(), } } } diff --git a/sdk/src/types/block/payload/signed_transaction/transaction.rs b/sdk/src/types/block/payload/signed_transaction/transaction.rs index d0ca90259e..4e58e8d1b4 100644 --- a/sdk/src/types/block/payload/signed_transaction/transaction.rs +++ b/sdk/src/types/block/payload/signed_transaction/transaction.rs @@ -5,6 +5,7 @@ use alloc::{collections::BTreeSet, vec::Vec}; use crypto::hashes::{blake2b::Blake2b256, Digest}; use hashbrown::HashSet; +use iterator_sorted::is_unique_sorted_by; use packable::{bounded::BoundedU16, prefix::BoxedSlicePrefix, Packable, PackableExt}; use crate::{ @@ -366,36 +367,48 @@ fn verify_context_inputs_packable( } fn verify_context_inputs(context_inputs: &[ContextInput]) -> Result<(), Error> { - // There must be zero or one Commitment Input. - if context_inputs - .iter() - .filter(|i| matches!(i, ContextInput::Commitment(_))) - .count() - > 1 - { - return Err(Error::TooManyCommitmentInputs); - } + is_unique_sorted_by(context_inputs.iter(), |a, b| { + a.kind().cmp(&b.kind()).then_with(|| match (a, b) { + (ContextInput::Commitment(_), ContextInput::Commitment(_)) => core::cmp::Ordering::Equal, + (ContextInput::BlockIssuanceCredit(a), ContextInput::BlockIssuanceCredit(b)) => { + a.account_id().cmp(b.account_id()) + } + (ContextInput::Reward(a), ContextInput::Reward(b)) => a.index().cmp(&b.index()), - let mut reward_index_set = HashSet::new(); + // No need to evaluate all combinations as `then_with` is only called if the first cmp is Equal. + _ => unreachable!(), + }) + }); + + let mut commitment = false; let mut bic_account_id_set = HashSet::new(); + let mut reward_index_set = HashSet::new(); for input in context_inputs.iter() { match input { + ContextInput::Commitment(_) => { + // There must be zero or one Commitment Input. + if commitment { + return Err(Error::TooManyCommitmentInputs); + } + commitment = true; + } ContextInput::BlockIssuanceCredit(bic) => { let account_id = bic.account_id(); + // All Block Issuance Credit Inputs must reference a different Account ID. if !bic_account_id_set.insert(account_id) { - return Err(Error::DuplicateBicAccountId(account_id)); + return Err(Error::DuplicateBicAccountId(*account_id)); } } ContextInput::Reward(r) => { - let idx = r.index(); + let index = r.index(); + // All Rewards Inputs must reference a different Index - if !reward_index_set.insert(idx) { - return Err(Error::DuplicateRewardInputIndex(idx)); + if !reward_index_set.insert(index) { + return Err(Error::DuplicateRewardInputIndex(index)); } } - _ => (), } } From 7e96a19ba75d85192abe2040127e862eeaf371f3 Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Tue, 23 Jan 2024 16:20:00 +0100 Subject: [PATCH 2/3] Simplify --- sdk/src/types/block/error.rs | 8 +--- .../payload/signed_transaction/transaction.rs | 39 ++----------------- 2 files changed, 6 insertions(+), 41 deletions(-) diff --git a/sdk/src/types/block/error.rs b/sdk/src/types/block/error.rs index 751270f18a..ad9174341c 100644 --- a/sdk/src/types/block/error.rs +++ b/sdk/src/types/block/error.rs @@ -41,12 +41,11 @@ pub enum Error { ConsumedAmountOverflow, ConsumedManaOverflow, ConsumedNativeTokensAmountOverflow, + ContextInputsNotUniqueSorted, CreatedAmountOverflow, CreatedManaOverflow, CreatedNativeTokensAmountOverflow, Crypto(CryptoError), - DuplicateBicAccountId(AccountId), - DuplicateRewardInputIndex(u16), DuplicateSignatureUnlock(u16), DuplicateUtxo(UtxoInput), ExpirationUnlockConditionZero, @@ -188,7 +187,6 @@ pub enum Error { }, StorageDepositReturnOverflow, TimelockUnlockConditionZero, - TooManyCommitmentInputs, UnallowedFeature { index: usize, kind: u8, @@ -222,12 +220,11 @@ impl fmt::Display for Error { Self::ConsumedAmountOverflow => write!(f, "consumed amount overflow"), Self::ConsumedManaOverflow => write!(f, "consumed mana overflow"), Self::ConsumedNativeTokensAmountOverflow => write!(f, "consumed native tokens amount overflow"), + Self::ContextInputsNotUniqueSorted => write!(f, "context inputs are not unique and/or sorted"), Self::CreatedAmountOverflow => write!(f, "created amount overflow"), Self::CreatedManaOverflow => write!(f, "created mana overflow"), Self::CreatedNativeTokensAmountOverflow => write!(f, "created native tokens amount overflow"), Self::Crypto(e) => write!(f, "cryptographic error: {e}"), - Self::DuplicateBicAccountId(account_id) => write!(f, "duplicate BIC account id: {account_id}"), - Self::DuplicateRewardInputIndex(idx) => write!(f, "duplicate reward input index: {idx}"), Self::DuplicateSignatureUnlock(index) => { write!(f, "duplicate signature unlock at index: {index}") } @@ -434,7 +431,6 @@ impl fmt::Display for Error { "timelock unlock condition with milestone index and timestamp set to 0", ) } - Self::TooManyCommitmentInputs => write!(f, "too many commitment inputs"), Self::UnallowedFeature { index, kind } => { write!(f, "unallowed feature at index {index} with kind {kind}") } diff --git a/sdk/src/types/block/payload/signed_transaction/transaction.rs b/sdk/src/types/block/payload/signed_transaction/transaction.rs index 4e58e8d1b4..b03405fa30 100644 --- a/sdk/src/types/block/payload/signed_transaction/transaction.rs +++ b/sdk/src/types/block/payload/signed_transaction/transaction.rs @@ -367,49 +367,18 @@ fn verify_context_inputs_packable( } fn verify_context_inputs(context_inputs: &[ContextInput]) -> Result<(), Error> { - is_unique_sorted_by(context_inputs.iter(), |a, b| { + if !is_unique_sorted_by(context_inputs.iter(), |a, b| { a.kind().cmp(&b.kind()).then_with(|| match (a, b) { (ContextInput::Commitment(_), ContextInput::Commitment(_)) => core::cmp::Ordering::Equal, (ContextInput::BlockIssuanceCredit(a), ContextInput::BlockIssuanceCredit(b)) => { a.account_id().cmp(b.account_id()) } (ContextInput::Reward(a), ContextInput::Reward(b)) => a.index().cmp(&b.index()), - - // No need to evaluate all combinations as `then_with` is only called if the first cmp is Equal. + // No need to evaluate all combinations as `then_with` is only called on Equal. _ => unreachable!(), }) - }); - - let mut commitment = false; - let mut bic_account_id_set = HashSet::new(); - let mut reward_index_set = HashSet::new(); - - for input in context_inputs.iter() { - match input { - ContextInput::Commitment(_) => { - // There must be zero or one Commitment Input. - if commitment { - return Err(Error::TooManyCommitmentInputs); - } - commitment = true; - } - ContextInput::BlockIssuanceCredit(bic) => { - let account_id = bic.account_id(); - - // All Block Issuance Credit Inputs must reference a different Account ID. - if !bic_account_id_set.insert(account_id) { - return Err(Error::DuplicateBicAccountId(*account_id)); - } - } - ContextInput::Reward(r) => { - let index = r.index(); - - // All Rewards Inputs must reference a different Index - if !reward_index_set.insert(index) { - return Err(Error::DuplicateRewardInputIndex(index)); - } - } - } + }) { + return Err(Error::ContextInputsNotUniqueSorted); } Ok(()) From a502833118e8c7d787089574db2370385d125a57 Mon Sep 17 00:00:00 2001 From: Thibault Martinez Date: Tue, 23 Jan 2024 16:28:29 +0100 Subject: [PATCH 3/3] sort --- .../payload/signed_transaction/transaction.rs | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/sdk/src/types/block/payload/signed_transaction/transaction.rs b/sdk/src/types/block/payload/signed_transaction/transaction.rs index b03405fa30..c66e5d2d81 100644 --- a/sdk/src/types/block/payload/signed_transaction/transaction.rs +++ b/sdk/src/types/block/payload/signed_transaction/transaction.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use alloc::{collections::BTreeSet, vec::Vec}; +use core::cmp::Ordering; use crypto::hashes::{blake2b::Blake2b256, Digest}; use hashbrown::HashSet; @@ -127,7 +128,7 @@ impl TransactionBuilder { /// Finishes a [`TransactionBuilder`] into a [`Transaction`]. pub fn finish_with_params<'a>( - self, + mut self, params: impl Into>, ) -> Result { let params = params.into(); @@ -159,6 +160,8 @@ impl TransactionBuilder { }) .ok_or(Error::InvalidField("creation slot"))?; + self.context_inputs.sort_by(context_inputs_cmp); + let context_inputs: BoxedSlicePrefix = self .context_inputs .into_boxed_slice() @@ -366,18 +369,20 @@ fn verify_context_inputs_packable( Ok(()) } +fn context_inputs_cmp(a: &ContextInput, b: &ContextInput) -> Ordering { + a.kind().cmp(&b.kind()).then_with(|| match (a, b) { + (ContextInput::Commitment(_), ContextInput::Commitment(_)) => Ordering::Equal, + (ContextInput::BlockIssuanceCredit(a), ContextInput::BlockIssuanceCredit(b)) => { + a.account_id().cmp(b.account_id()) + } + (ContextInput::Reward(a), ContextInput::Reward(b)) => a.index().cmp(&b.index()), + // No need to evaluate all combinations as `then_with` is only called on Equal. + _ => unreachable!(), + }) +} + fn verify_context_inputs(context_inputs: &[ContextInput]) -> Result<(), Error> { - if !is_unique_sorted_by(context_inputs.iter(), |a, b| { - a.kind().cmp(&b.kind()).then_with(|| match (a, b) { - (ContextInput::Commitment(_), ContextInput::Commitment(_)) => core::cmp::Ordering::Equal, - (ContextInput::BlockIssuanceCredit(a), ContextInput::BlockIssuanceCredit(b)) => { - a.account_id().cmp(b.account_id()) - } - (ContextInput::Reward(a), ContextInput::Reward(b)) => a.index().cmp(&b.index()), - // No need to evaluate all combinations as `then_with` is only called on Equal. - _ => unreachable!(), - }) - }) { + if !is_unique_sorted_by(context_inputs.iter(), |a, b| context_inputs_cmp(a, b)) { return Err(Error::ContextInputsNotUniqueSorted); }