Skip to content

Commit c680543

Browse files
DCNick36r1d
DCNick3
authored and
6r1d
committed
[refactor] #3526: Allow only a fixed set of signature verification conditions
Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>
1 parent 3ffa799 commit c680543

File tree

9 files changed

+98
-150
lines changed

9 files changed

+98
-150
lines changed

‎client/tests/integration/multisignature_transaction.rs

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,8 @@ use iroha_client::client::{self, Client, QueryResult};
77
use iroha_config::client::Configuration as ClientConfiguration;
88
use iroha_crypto::KeyPair;
99
use iroha_data_model::{
10-
account::TRANSACTION_SIGNATORIES_VALUE,
1110
parameter::{default::MAX_TRANSACTIONS_IN_BLOCK, ParametersBuilder},
1211
prelude::*,
13-
val_vec,
1412
};
1513
use test_network::*;
1614

@@ -35,15 +33,9 @@ fn multisignature_transactions_should_wait_for_all_signatures() -> Result<()> {
3533
let asset_definition_id = AssetDefinitionId::from_str("camomile#wonderland")?;
3634
let create_asset = RegisterBox::new(AssetDefinition::quantity(asset_definition_id.clone()));
3735
let set_signature_condition = MintBox::new(
38-
SignatureCheckCondition::new(EvaluatesTo::new_unchecked(ContainsAll::new(
39-
EvaluatesTo::new_unchecked(ContextValue::new(Name::from_str(
40-
TRANSACTION_SIGNATORIES_VALUE,
41-
)?)),
42-
val_vec![
43-
alice_key_pair.public_key().clone(),
44-
key_pair_2.public_key().clone(),
45-
],
46-
))),
36+
SignatureCheckCondition::AllAccountSignaturesAnd(
37+
vec![key_pair_2.public_key().clone()].into(),
38+
),
4739
IdBox::AccountId(alice_id.clone()),
4840
);
4941

@@ -84,7 +76,8 @@ fn multisignature_transactions_should_wait_for_all_signatures() -> Result<()> {
8476
.collect::<QueryResult<Vec<_>>>()?;
8577
assert_eq!(
8678
assets.len(),
87-
2 // Alice has roses and cabbage from Genesis
79+
2, // Alice has roses and cabbage from Genesis, but doesn't yet have camomile
80+
"Multisignature transaction was committed before all required signatures were added"
8881
);
8982
let (public_key2, private_key2) = key_pair_2.into();
9083
client_configuration.public_key = public_key2;

‎client_cli/src/main.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -519,8 +519,9 @@ mod account {
519519
let deser_err_msg =
520520
format!("Failed to deserialize signature condition from file {}", &s);
521521
let content = fs::read_to_string(s).wrap_err(err_msg)?;
522-
let condition: EvaluatesTo<bool> = json5::from_str(&content).wrap_err(deser_err_msg)?;
523-
Ok(Self(SignatureCheckCondition::new(condition)))
522+
let condition: SignatureCheckCondition =
523+
json5::from_str(&content).wrap_err(deser_err_msg)?;
524+
Ok(Self(condition))
524525
}
525526
}
526527

‎configs/peer/validator.wasm

-502 Bytes
Binary file not shown.

‎core/src/queue.rs

Lines changed: 9 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,7 @@ use dashmap::{mapref::entry::Entry, DashMap};
1414
use eyre::{Report, Result};
1515
use iroha_config::queue::Configuration;
1616
use iroha_crypto::HashOf;
17-
use iroha_data_model::{
18-
account::{Account, AccountId},
19-
evaluate::ExpressionEvaluator as _,
20-
expression::{EvaluatesTo, Where},
21-
transaction::prelude::*,
22-
};
17+
use iroha_data_model::{account::AccountId, transaction::prelude::*};
2318
use iroha_logger::{debug, info, trace, warn};
2419
use iroha_primitives::must_use::MustUse;
2520
use rand::seq::IteratorRandom;
@@ -32,16 +27,17 @@ impl AcceptedTransaction {
3227
fn check_signature_condition(&self, wsv: &WorldStateView) -> Result<MustUse<bool>> {
3328
let authority = &self.payload().authority;
3429

35-
let signatories = self
30+
let transaction_signatories = self
3631
.signatures()
3732
.iter()
3833
.map(|signature| signature.public_key())
39-
.cloned();
34+
.cloned()
35+
.collect();
4036

4137
wsv.map_account(authority, |account| {
42-
wsv.evaluate(&check_signature_condition(account, signatories))
43-
.map(MustUse::new)
44-
.map_err(Into::into)
38+
Ok(account
39+
.signature_check_condition
40+
.check(&account.signatories, &transaction_signatories))
4541
})?
4642
}
4743

@@ -51,29 +47,6 @@ impl AcceptedTransaction {
5147
}
5248
}
5349

54-
fn check_signature_condition(
55-
account: &Account,
56-
signatories: impl IntoIterator<Item = PublicKey>,
57-
) -> EvaluatesTo<bool> {
58-
let where_expr = Where::new(EvaluatesTo::new_evaluates_to_value(
59-
*account.signature_check_condition.0.expression.clone(),
60-
))
61-
.with_value(
62-
iroha_data_model::account::ACCOUNT_SIGNATORIES_VALUE
63-
.parse()
64-
.expect("ACCOUNT_SIGNATORIES_VALUE should be valid."),
65-
account.signatories.iter().cloned().collect::<Vec<_>>(),
66-
)
67-
.with_value(
68-
iroha_data_model::account::TRANSACTION_SIGNATORIES_VALUE
69-
.parse()
70-
.expect("TRANSACTION_SIGNATORIES_VALUE should be valid."),
71-
signatories.into_iter().collect::<Vec<_>>(),
72-
);
73-
74-
EvaluatesTo::new_unchecked(where_expr)
75-
}
76-
7750
/// Lockfree queue for transactions
7851
///
7952
/// Multiple producers, single consumer
@@ -414,11 +387,7 @@ mod tests {
414387
use std::{str::FromStr, sync::Arc, thread, time::Duration};
415388

416389
use iroha_config::{base::proxy::Builder, queue::ConfigurationProxy};
417-
use iroha_data_model::{
418-
account::{ACCOUNT_SIGNATORIES_VALUE, TRANSACTION_SIGNATORIES_VALUE},
419-
prelude::*,
420-
transaction::TransactionLimits,
421-
};
390+
use iroha_data_model::{prelude::*, transaction::TransactionLimits};
422391
use iroha_primitives::must_use::MustUse;
423392
use rand::Rng as _;
424393

@@ -509,46 +478,6 @@ mod tests {
509478
));
510479
}
511480

512-
#[test]
513-
fn push_tx_signature_condition_failure() {
514-
let max_txs_in_queue = 10;
515-
let key_pair = KeyPair::generate().unwrap();
516-
517-
let wsv = {
518-
let domain_id = DomainId::from_str("wonderland").expect("Valid");
519-
let account_id = AccountId::from_str("alice@wonderland").expect("Valid");
520-
let mut domain = Domain::new(domain_id.clone()).build(&account_id);
521-
let mut account = Account::new(account_id.clone(), [key_pair.public_key().clone()])
522-
.build(&account_id);
523-
// Cause `check_siganture_condition` failure by trying to convert `u32` to `bool`
524-
account.signature_check_condition =
525-
SignatureCheckCondition(EvaluatesTo::new_unchecked(0u32));
526-
assert!(domain.add_account(account).is_none());
527-
528-
let kura = Kura::blank_kura_for_testing();
529-
Arc::new(WorldStateView::new(
530-
World::with([domain], PeersIds::new()),
531-
kura.clone(),
532-
))
533-
};
534-
535-
let queue = Queue::from_configuration(&Configuration {
536-
transaction_time_to_live_ms: 100_000,
537-
max_transactions_in_queue: max_txs_in_queue,
538-
..ConfigurationProxy::default()
539-
.build()
540-
.expect("Default queue config should always build")
541-
});
542-
543-
assert!(matches!(
544-
queue.push(accepted_tx("alice@wonderland", key_pair), &wsv),
545-
Err(Failure {
546-
err: Error::SignatureCondition { .. },
547-
..
548-
})
549-
));
550-
}
551-
552481
#[test]
553482
fn push_multisignature_tx() {
554483
let max_txs_in_block = 2;
@@ -563,19 +492,7 @@ mod tests {
563492
key_pairs.iter().map(KeyPair::public_key).cloned(),
564493
)
565494
.build(&account_id);
566-
account.signature_check_condition = SignatureCheckCondition(
567-
ContainsAll::new(
568-
EvaluatesTo::new_unchecked(ContextValue::new(
569-
Name::from_str(TRANSACTION_SIGNATORIES_VALUE)
570-
.expect("TRANSACTION_SIGNATORIES_VALUE should be valid."),
571-
)),
572-
EvaluatesTo::new_unchecked(ContextValue::new(
573-
Name::from_str(ACCOUNT_SIGNATORIES_VALUE)
574-
.expect("ACCOUNT_SIGNATORIES_VALUE should be valid."),
575-
)),
576-
)
577-
.into(),
578-
);
495+
account.signature_check_condition = SignatureCheckCondition::all_account_signatures();
579496
assert!(domain.add_account(account).is_none());
580497
Arc::new(WorldStateView::new(
581498
World::with([domain], PeersIds::new()),

‎data_model/src/account.rs

Lines changed: 53 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use std::collections::{btree_map, btree_set};
1515
use derive_more::{Constructor, DebugCustom, Display};
1616
use getset::Getters;
1717
use iroha_data_model_derive::{model, IdEqOrdHash};
18+
use iroha_primitives::{const_vec::ConstVec, must_use::MustUse};
1819
use iroha_schema::IntoSchema;
1920
use parity_scale_codec::{Decode, Encode};
2021
use serde::{Deserialize, Serialize};
@@ -27,7 +28,6 @@ use crate::{
2728
AssetsMap,
2829
},
2930
domain::prelude::*,
30-
expression::{ContainsAny, ContextValue, EvaluatesTo},
3131
metadata::Metadata,
3232
role::{prelude::RoleId, RoleIds},
3333
HasMetadata, Identifiable, Name, ParseError, PublicKey, Registered,
@@ -44,18 +44,6 @@ pub type AccountsMap = btree_map::BTreeMap<AccountId, Account>;
4444
// of space, over `Vec`.
4545
type Signatories = btree_set::BTreeSet<PublicKey>;
4646

47-
/// The context value name for transaction signatories.
48-
#[cfg(feature = "transparent_api")]
49-
pub const TRANSACTION_SIGNATORIES_VALUE: &str = "transaction_signatories";
50-
#[cfg(not(feature = "transparent_api"))]
51-
const TRANSACTION_SIGNATORIES_VALUE: &str = "transaction_signatories";
52-
53-
/// The context value name for account signatories.
54-
#[cfg(feature = "transparent_api")]
55-
pub const ACCOUNT_SIGNATORIES_VALUE: &str = "account_signatories";
56-
#[cfg(not(feature = "transparent_api"))]
57-
const ACCOUNT_SIGNATORIES_VALUE: &str = "account_signatories";
58-
5947
#[model]
6048
pub mod model {
6149
use super::*;
@@ -154,18 +142,20 @@ pub mod model {
154142
Eq,
155143
PartialOrd,
156144
Ord,
157-
Constructor,
158145
Decode,
159146
Encode,
160147
Deserialize,
161148
Serialize,
162149
IntoSchema,
163150
)]
164-
#[serde(transparent)]
165-
#[repr(transparent)]
166-
// SAFETY: `SignatureCheckCondition` has no trap representation in `EvalueatesTo<bool>`
167-
#[ffi_type(unsafe {robust})]
168-
pub struct SignatureCheckCondition(pub EvaluatesTo<bool>);
151+
#[ffi_type(opaque)]
152+
#[allow(clippy::enum_variant_names)]
153+
pub enum SignatureCheckCondition {
154+
#[display(fmt = "AnyAccountSignatureOr({_0:?})")]
155+
AnyAccountSignatureOr(ConstVec<PublicKey>),
156+
#[display(fmt = "AllAccountSignaturesAnd({_0:?})")]
157+
AllAccountSignaturesAnd(ConstVec<PublicKey>),
158+
}
169159
}
170160

171161
impl Account {
@@ -278,27 +268,6 @@ impl NewAccount {
278268
}
279269
}
280270

281-
/// Default signature condition check for accounts.
282-
/// Returns true if any of the signatories have signed the transaction.
283-
impl Default for SignatureCheckCondition {
284-
#[inline]
285-
fn default() -> Self {
286-
Self(
287-
ContainsAny::new(
288-
EvaluatesTo::new_unchecked(ContextValue::new(
289-
Name::from_str(TRANSACTION_SIGNATORIES_VALUE)
290-
.expect("TRANSACTION_SIGNATORIES_VALUE should be valid."),
291-
)),
292-
EvaluatesTo::new_unchecked(ContextValue::new(
293-
Name::from_str(ACCOUNT_SIGNATORIES_VALUE)
294-
.expect("ACCOUNT_SIGNATORIES_VALUE should be valid."),
295-
)),
296-
)
297-
.into(),
298-
)
299-
}
300-
}
301-
302271
impl HasMetadata for NewAccount {
303272
fn metadata(&self) -> &Metadata {
304273
&self.metadata
@@ -345,6 +314,50 @@ impl FromStr for AccountId {
345314
}
346315
}
347316

317+
impl Default for SignatureCheckCondition {
318+
fn default() -> Self {
319+
Self::AnyAccountSignatureOr(ConstVec::new_empty())
320+
}
321+
}
322+
323+
impl SignatureCheckCondition {
324+
/// Shorthand to create a [`SignatureCheckCondition::AnyAccountSignatureOr`] variant without additional allowed signatures.
325+
#[inline]
326+
pub fn any_account_signature() -> Self {
327+
Self::AnyAccountSignatureOr(ConstVec::new_empty())
328+
}
329+
330+
/// Shorthand to create a [`SignatureCheckCondition::AllAccountSignaturesAnd`] variant without additional required signatures.
331+
#[inline]
332+
pub fn all_account_signatures() -> Self {
333+
Self::AllAccountSignaturesAnd(ConstVec::new_empty())
334+
}
335+
336+
/// Checks whether the transaction contains all the signatures required by the `SignatureCheckCondition`.
337+
pub fn check(
338+
&self,
339+
account_signatories: &btree_set::BTreeSet<PublicKey>,
340+
transaction_signatories: &btree_set::BTreeSet<PublicKey>,
341+
) -> MustUse<bool> {
342+
let result = match &self {
343+
SignatureCheckCondition::AnyAccountSignatureOr(additional_allowed_signatures) => {
344+
account_signatories
345+
.iter()
346+
.chain(additional_allowed_signatures.as_ref())
347+
.any(|allowed| transaction_signatories.contains(allowed))
348+
}
349+
SignatureCheckCondition::AllAccountSignaturesAnd(additional_required_signatures) => {
350+
account_signatories
351+
.iter()
352+
.chain(additional_required_signatures.as_ref())
353+
.all(|required_signature| transaction_signatories.contains(required_signature))
354+
}
355+
};
356+
357+
MustUse::new(result)
358+
}
359+
}
360+
348361
/// The prelude re-exports most commonly used traits, structs and macros from this crate.
349362
pub mod prelude {
350363
pub use super::{Account, AccountId, SignatureCheckCondition};

‎data_model/src/lib.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ use block::VersionedCommittedBlock;
3434
#[cfg(not(target_arch = "aarch64"))]
3535
use derive_more::Into;
3636
use derive_more::{AsRef, DebugCustom, Deref, Display, From, FromStr};
37-
use evaluate::Evaluate;
3837
use events::TriggeringFilterBox;
3938
use getset::Getters;
4039
use iroha_crypto::{HashOf, PublicKey};
@@ -1126,10 +1125,10 @@ impl Value {
11261125
| LengthLimits(_)
11271126
| Numeric(_)
11281127
| Validator(_)
1129-
| LogLevel(_) => 1_usize,
1128+
| LogLevel(_)
1129+
| SignatureCheckCondition(_) => 1_usize,
11301130
Vec(v) => v.iter().map(Self::len).sum::<usize>() + 1_usize,
11311131
LimitedMetadata(data) => data.nested_len() + 1_usize,
1132-
SignatureCheckCondition(s) => Evaluate::len(&s.0),
11331132
}
11341133
}
11351134
}

‎docs/source/references/schema.json

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3940,7 +3940,20 @@
39403940
}
39413941
]
39423942
},
3943-
"SignatureCheckCondition": "EvaluatesTo<bool>",
3943+
"SignatureCheckCondition": {
3944+
"Enum": [
3945+
{
3946+
"tag": "AnyAccountSignatureOr",
3947+
"discriminant": 0,
3948+
"type": "Vec<PublicKey>"
3949+
},
3950+
{
3951+
"tag": "AllAccountSignaturesAnd",
3952+
"discriminant": 1,
3953+
"type": "Vec<PublicKey>"
3954+
}
3955+
]
3956+
},
39443957
"SignatureOf<CommittedBlock>": "Signature",
39453958
"SignatureOf<QueryPayload>": "Signature",
39463959
"SignatureOf<TransactionPayload>": "Signature",
@@ -4771,6 +4784,9 @@
47714784
"Vec<PeerId>": {
47724785
"Vec": "PeerId"
47734786
},
4787+
"Vec<PublicKey>": {
4788+
"Vec": "PublicKey"
4789+
},
47744790
"Vec<TransactionValue>": {
47754791
"Vec": "TransactionValue"
47764792
},

0 commit comments

Comments
 (0)