From 9f8ef275a49f9fd262e953162bed5b7d7f6ddaa4 Mon Sep 17 00:00:00 2001
From: Shanin Roman <shanin1000@yandex.ru>
Date: Mon, 26 Dec 2022 18:51:31 +0300
Subject: [PATCH] [fix] #3038: Re-enable multisigs

Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
---
 .../integration/multisignature_transaction.rs |  1 -
 core/src/queue.rs                             | 38 +++++++++----------
 core/src/sumeragi/main_loop.rs                | 11 +++---
 3 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/client/tests/integration/multisignature_transaction.rs b/client/tests/integration/multisignature_transaction.rs
index 3c8c204d458..426ecd9eeab 100644
--- a/client/tests/integration/multisignature_transaction.rs
+++ b/client/tests/integration/multisignature_transaction.rs
@@ -12,7 +12,6 @@ use test_network::*;
 use super::Configuration;
 
 #[allow(clippy::too_many_lines)]
-#[ignore = "Multisignature is not working for now. See #2595"]
 #[test]
 fn multisignature_transactions_should_wait_for_all_signatures() {
     let (_rt, network, _) = <Network>::start_test_with_runtime(4, 1, None);
diff --git a/core/src/queue.rs b/core/src/queue.rs
index 85310578b63..8d203afc051 100644
--- a/core/src/queue.rs
+++ b/core/src/queue.rs
@@ -12,10 +12,11 @@ use std::collections::HashSet;
 
 use crossbeam_queue::ArrayQueue;
 use dashmap::{mapref::entry::Entry, DashMap};
-use eyre::{eyre, Report, Result};
+use eyre::{Report, Result};
 use iroha_config::queue::Configuration;
 use iroha_crypto::HashOf;
 use iroha_data_model::transaction::prelude::*;
+use iroha_primitives::must_use::MustUse;
 use rand::seq::IteratorRandom;
 use thiserror::Error;
 
@@ -108,24 +109,17 @@ impl Queue {
             )
     }
 
-    // FIXME: Currently it is impossible to distinguish if signature check condition failed or if there is just not enough signatures (#2595).
     fn check_tx(
         &self,
         tx: &VersionedAcceptedTransaction,
         wsv: &WorldStateView,
-    ) -> Result<(), Error> {
+    ) -> Result<MustUse<bool>, Error> {
         if tx.is_expired(self.tx_time_to_live) {
             Err(Error::Expired)
         } else if tx.is_in_blockchain(wsv) {
             Err(Error::InBlockchain)
         } else {
             tx.check_signature_condition(wsv)
-                .and_then(|success| {
-                    success
-                        .into_inner()
-                        .then_some(())
-                        .ok_or_else(|| eyre!("Signature condition check failed"))
-                })
                 .map_err(|reason| Error::SignatureCondition {
                     tx_hash: tx.hash(),
                     reason,
@@ -207,14 +201,18 @@ impl Queue {
                 continue;
             }
 
-            // Transactions are not removed from the queue until expired or committed
-            seen.push(hash);
-            if *entry
-                .get()
-                .check_signature_condition(wsv)
-                .expect("Checked in `check_tx` just above")
-            {
-                return Some(entry.get().clone());
+            match self.check_tx(entry.get(), wsv) {
+                Err(_) => {
+                    entry.remove_entry();
+                    continue;
+                }
+                Ok(MustUse(signature_check)) => {
+                    // Transactions are not removed from the queue until expired or committed
+                    seen.push(hash);
+                    if signature_check {
+                        return Some(entry.get().clone());
+                    }
+                }
             }
         }
     }
@@ -388,7 +386,10 @@ mod tests {
             let mut domain = Domain::new(domain_id.clone()).build();
             let account_id = AccountId::from_str("alice@wonderland").expect("Valid");
             let mut account = Account::new(account_id, [key_pair.public_key().clone()]).build();
-            account.set_signature_check_condition(SignatureCheckCondition(false.into()));
+            // Cause `check_siganture_condition` failure by trying to convert `u32` to `bool`
+            account.set_signature_check_condition(SignatureCheckCondition(
+                EvaluatesTo::new_unchecked(0u32.into()),
+            ));
             assert!(domain.add_account(account).is_none());
 
             let kura = Kura::blank_kura_for_testing();
@@ -414,7 +415,6 @@ mod tests {
     }
 
     #[test]
-    #[ignore = "Multisignature is not working for now. See #2595"]
     fn push_multisignature_tx() {
         let key_pairs = [KeyPair::generate().unwrap(), KeyPair::generate().unwrap()];
         let kura = Kura::blank_kura_for_testing();
diff --git a/core/src/sumeragi/main_loop.rs b/core/src/sumeragi/main_loop.rs
index 9f0d0b42756..f72659cff44 100644
--- a/core/src/sumeragi/main_loop.rs
+++ b/core/src/sumeragi/main_loop.rs
@@ -189,12 +189,11 @@ impl<F: FaultInjection> SumeragiWithFault<F> {
         let current_topology = &state.current_topology;
         let role = current_topology.role(&self.peer_id);
 
-        let txs = state
-            .transaction_cache
-            .iter()
-            .take(self.gossip_batch_size as usize)
-            .cloned()
-            .collect::<Vec<_>>();
+        // Transactions are intentionally taken from the queue instead of the cache
+        // to gossip multisignature transactions too
+        let txs = self
+            .queue
+            .n_random_transactions(self.gossip_batch_size, &state.wsv);
 
         if !txs.is_empty() {
             debug!(%role, tx_count = txs.len(), "Gossiping transactions");