Feature/rejected txs index (#1846)

* Add rejected tx index

Signed-off-by: kamilsa <[email protected]>

irohad/ametsuchi/block_query.hpp

@@ -22,6 +22,7 @@
 #include <cmath>
 #include <rxcpp/rx.hpp>
 
+#include "ametsuchi/tx_cache_response.hpp"
 #include "common/result.hpp"
 #include "interfaces/iroha_internal/block.hpp"
 #include "interfaces/transaction.hpp"
@@ -106,12 +107,14 @@ namespace iroha {
           const shared_model::crypto::Hash &hash) = 0;
 
       /**
-       * Synchronously checks whether transaction
-       * with given hash is present in any block
-       * @param hash - transaction hash
-       * @return true if transaction exists, false otherwise
+       * Synchronously checks whether transaction with given hash is present in
+       * any block
+       * @param hash - rejected transaction's hash
+       * @return TxCacheStatusType which returns status of transaction:
+       * Committed, Rejected or Missing
        */
-      virtual bool hasTxWithHash(const shared_model::crypto::Hash &hash) = 0;
+      virtual TxCacheStatusType checkTxPresence(
+          const shared_model::crypto::Hash &hash) = 0;
 
       /**
        * Get the top-most block

irohad/ametsuchi/impl/postgres_block_index.cpp

@@ -39,6 +39,15 @@ namespace {
     return (base % hash.hex() % height).str();
   }
 
+  std::string makeRejectedTxHashIndex(
+      const shared_model::interface::types::HashType &rejected_tx_hash,
+      shared_model::interface::types::HeightType height) {
+    boost::format base(
+        "INSERT INTO height_by_rejected_hash(hash, height) VALUES ('%s', "
+        "'%s');");
+    return (base % rejected_tx_hash.hex() % height).str();
+  }
+
   // make index account_id:height -> list of tx indexes
   // (where tx is placed in the block)
   std::string makeCreatorHeightIndex(
@@ -110,7 +119,8 @@ namespace iroha {
         const shared_model::interface::Block &block) {
       auto height = block.height();
       auto indexed_txs = block.transactions() | boost::adaptors::indexed(0);
-      std::string index_query = std::accumulate(
+      auto rejected_txs_hashes = block.rejected_transactions_hashes();
+      std::string tx_index_query = std::accumulate(
           indexed_txs.begin(),
           indexed_txs.end(),
           std::string{},
@@ -125,6 +135,17 @@ namespace iroha {
             query += makeCreatorHeightIndex(creator_id, height, index);
             return query;
           });
+
+      std::string rejected_tx_index_query = std::accumulate(
+          rejected_txs_hashes.begin(),
+          rejected_txs_hashes.end(),
+          std::string{},
+          [height](auto query, const auto &rejected_tx_hash) {
+            query += makeRejectedTxHashIndex(rejected_tx_hash, height);
+            return query;
+          });
+
+      auto index_query = tx_index_query + rejected_tx_index_query;
       try {
         sql_ << index_query;
       } catch (const std::exception &e) {

irohad/ametsuchi/impl/postgres_block_query.cpp

@@ -242,11 +242,38 @@ namespace iroha {
         };
     }
 
-    bool PostgresBlockQuery::hasTxWithHash(
+    TxCacheStatusType PostgresBlockQuery::checkTxPresence(
+        const shared_model::crypto::Hash &hash) {
+      if (hasCommittedTxWithHash(hash)) {
+        return tx_cache_status_responses::Committed();
+      }
+      if (hasRejectedTxWithHash(hash)) {
+        return tx_cache_status_responses::Rejected();
+      }
+      return tx_cache_status_responses::Missing();
+    }
+
+    bool PostgresBlockQuery::hasCommittedTxWithHash(
         const shared_model::crypto::Hash &hash) {
       return getBlockId(hash) != boost::none;
     }
 
+    bool PostgresBlockQuery::hasRejectedTxWithHash(
+        const shared_model::crypto::Hash &hash) {
+      boost::optional<uint64_t> blockId = boost::none;
+      boost::optional<std::string> block_str;
+      const auto &hash_str = hash.hex();
+
+      sql_ << "SELECT height FROM height_by_rejected_hash WHERE hash = :hash",
+          soci::into(block_str), soci::use(hash_str);
+      if (block_str) {
+        blockId = std::stoull(block_str.get());
+      } else {
+        log_->info("No block with rejected transaction {}", hash.toString());
+      }
+      return (bool)blockId;
+    }
+
     uint32_t PostgresBlockQuery::getTopBlockHeight() {
       return block_store_.last_id();
     }

irohad/ametsuchi/impl/postgres_block_query.hpp

@@ -61,7 +61,8 @@ namespace iroha {
 
       uint32_t getTopBlockHeight() override;
 
-      bool hasTxWithHash(const shared_model::crypto::Hash &hash) override;
+      TxCacheStatusType checkTxPresence(
+          const shared_model::crypto::Hash &hash) override;
 
       expected::Result<wBlock, std::string> getTopBlock() override;
 
@@ -73,7 +74,6 @@ namespace iroha {
        */
       std::vector<shared_model::interface::types::HeightType> getBlockIds(
           const shared_model::interface::types::AccountIdType &account_id);
-
       /**
        * Returns block id which contains transaction with a given hash
        * @param hash - hash of transaction
@@ -101,6 +101,10 @@ namespace iroha {
                        std::string>
       getBlock(shared_model::interface::types::HeightType id) const;
 
+      bool hasCommittedTxWithHash(const shared_model::crypto::Hash &hash);
+
+      bool hasRejectedTxWithHash(const shared_model::crypto::Hash &hash);
+
       std::unique_ptr<soci::session> psql_;
       soci::session &sql_;
 

irohad/ametsuchi/impl/storage_impl.cpp

@@ -508,6 +508,12 @@ CREATE TABLE IF NOT EXISTS height_by_hash (
     hash varchar,
     height text
 );
+
+CREATE TABLE IF NOT EXISTS height_by_rejected_hash (
+  hash varchar,
+  height text
+);
+
 CREATE TABLE IF NOT EXISTS height_by_account_set (
     account_id text,
     height text

irohad/torii/impl/command_service_impl.cpp

@@ -54,17 +54,20 @@ namespace torii {
       return cached.value();
     }
 
-    const bool is_present = storage_->getBlockQuery()->hasTxWithHash(request);
-
-    if (is_present) {
-      std::shared_ptr<shared_model::interface::TransactionResponse> response =
-          status_factory_->makeCommitted(request, "");
-      cache_->addItem(request, response);
-      return response;
-    } else {
-      log_->warn("Asked non-existing tx: {}", request.hex());
-      return status_factory_->makeNotReceived(request, "");
-    }
+    return iroha::visit_in_place(
+        storage_->getBlockQuery()->checkTxPresence(request),
+        [this,
+         &request](const iroha::ametsuchi::tx_cache_status_responses::Missing &)
+            -> std::shared_ptr<shared_model::interface::TransactionResponse> {
+          log_->warn("Asked non-existing tx: {}", request.hex());
+          return status_factory_->makeNotReceived(request, "");
+        },
+        [this, &request](const auto &) {
+          std::shared_ptr<shared_model::interface::TransactionResponse>
+              response = status_factory_->makeCommitted(request, "");
+          cache_->addItem(request, response);
+          return response;
+        });
   }
 
   /**

test/module/irohad/ametsuchi/ametsuchi_mocks.hpp

@@ -184,7 +184,8 @@ namespace iroha {
                        shared_model::interface::types::HeightType));
       MOCK_METHOD1(getTopBlocks, std::vector<BlockQuery::wBlock>(uint32_t));
       MOCK_METHOD0(getTopBlock, expected::Result<wBlock, std::string>(void));
-      MOCK_METHOD1(hasTxWithHash, bool(const shared_model::crypto::Hash &hash));
+      MOCK_METHOD1(checkTxPresence,
+                   TxCacheStatusType(const shared_model::crypto::Hash &));
       MOCK_METHOD0(getTopBlockHeight, uint32_t(void));
     };
 

test/module/irohad/ametsuchi/block_query_test.cpp

@@ -17,6 +17,7 @@
 
 #include <boost/filesystem.hpp>
 #include <boost/optional.hpp>
+#include <framework/specified_visitor.hpp>
 
 #include "ametsuchi/impl/postgres_block_index.hpp"
 #include "ametsuchi/impl/postgres_block_query.hpp"
@@ -67,6 +68,8 @@ class BlockQueryTest : public AmetsuchiTest {
             .transactions(
                 std::vector<shared_model::proto::Transaction>({txn1_1, txn1_2}))
             .prevHash(shared_model::crypto::Hash(zero_string))
+            .rejectedTransactions(
+                std::vector<shared_model::crypto::Hash>{rejected_hash})
             .build();
 
     // First tx in block 1
@@ -110,6 +113,7 @@ class BlockQueryTest : public AmetsuchiTest {
   std::string creator2 = "user2@test";
   std::size_t blocks_total{0};
   std::string zero_string = std::string(32, '0');
+  shared_model::crypto::Hash rejected_hash{"rejected_tx_hash"};
 };
 
 /**
@@ -347,24 +351,40 @@ TEST_F(BlockQueryTest, GetTop2Blocks) {
 
 /**
  * @given block store with preinserted blocks
- * @when hasTxWithHash is invoked on existing transaction hash
- * @then True is returned
+ * @when checkTxPresence is invoked on existing transaction hash
+ * @then Committed status is returned
  */
 TEST_F(BlockQueryTest, HasTxWithExistingHash) {
   for (const auto &hash : tx_hashes) {
-    EXPECT_TRUE(blocks->hasTxWithHash(hash));
+    ASSERT_NO_THROW(boost::apply_visitor(
+        framework::SpecifiedVisitor<tx_cache_status_responses::Committed>(),
+        blocks->checkTxPresence(hash)));
   }
 }
 
 /**
  * @given block store with preinserted blocks
  * user1@test AND 1 tx created by user2@test
- * @when hasTxWithHash is invoked on non-existing hash
- * @then False is returned
+ * @when checkTxPresence is invoked on non-existing hash
+ * @then Missing status is returned
  */
-TEST_F(BlockQueryTest, HasTxWithInvalidHash) {
-  shared_model::crypto::Hash invalid_tx_hash(zero_string);
-  EXPECT_FALSE(blocks->hasTxWithHash(invalid_tx_hash));
+TEST_F(BlockQueryTest, HasTxWithMissingHash) {
+  shared_model::crypto::Hash missing_tx_hash(zero_string);
+  ASSERT_NO_THROW(boost::apply_visitor(
+      framework::SpecifiedVisitor<tx_cache_status_responses::Missing>(),
+      blocks->checkTxPresence(missing_tx_hash)));
+}
+
+/**
+ * @given block store with preinserted blocks containing rejected_hash1 in one
+ * of the block
+ * @when checkTxPresence is invoked on existing rejected hash
+ * @then Rejected is returned
+ */
+TEST_F(BlockQueryTest, HasTxWithRejectedHash) {
+  ASSERT_NO_THROW(boost::apply_visitor(
+      framework::SpecifiedVisitor<tx_cache_status_responses::Rejected>(),
+      blocks->checkTxPresence(rejected_hash)));
 }
 
 /**

test/module/irohad/torii/torii_service_test.cpp

@@ -216,14 +216,15 @@ TEST_F(ToriiServiceTest, CommandClient) {
  * @then ensure those are not received
  */
 TEST_F(ToriiServiceTest, StatusWhenTxWasNotReceivedBlocking) {
-  std::vector<shared_model::proto::Transaction> txs;
   std::vector<shared_model::interface::types::HashType> tx_hashes;
 
+  ON_CALL(*block_query, checkTxPresence(_))
+      .WillByDefault(
+          Return(iroha::ametsuchi::tx_cache_status_responses::Missing()));
+
   // create transactions, but do not send them
   for (size_t i = 0; i < TimesToriiBlocking; ++i) {
-    auto tx = TestTransactionBuilder().creatorAccountId("accountA").build();
-    txs.push_back(tx);
-    tx_hashes.push_back(tx.hash());
+    tx_hashes.push_back(shared_model::crypto::Hash{std::to_string(i)});
   }
 
   // get statuses of unsent transactions
@@ -323,11 +324,11 @@ TEST_F(ToriiServiceTest, StatusWhenBlocking) {
       std::make_shared<iroha::validation::VerifiedProposalAndErrors>();
   validation_result->verified_proposal =
       std::make_unique<shared_model::proto::Proposal>(
-      TestProposalBuilder()
-          .height(1)
-          .createdTime(iroha::time::now())
-          .transactions(txs)
-          .build());
+          TestProposalBuilder()
+              .height(1)
+              .createdTime(iroha::time::now())
+              .transactions(txs)
+              .build());
   validation_result->rejected_transactions.emplace(
       failed_tx_hash,
       iroha::validation::CommandError{