Improve builder interface:

- Introduce unsigned proto object - Return unsigned object from builder - Move SignatureType to types.hpp - Signing and adding signature perform by one method call now - Improve builder test to be aware about signatures - Adapt Swig interface for changes above Signed-off-by: luckychess <[email protected]>
neo4reo · Dec 19, 2017 · bb6aea6 · bb6aea6
1 parent b46159a
commit bb6aea6
Show file tree

Hide file tree

Showing 14 changed files with 106 additions and 32 deletions.
diff --git a/shared_model/backend/protobuf/transaction.hpp b/shared_model/backend/protobuf/transaction.hpp
@@ -88,9 +88,7 @@ namespace shared_model {
       }
 
       bool addSignature(
-          const shared_model::interface::Signable<
-              shared_model::interface::Transaction,
-              iroha::model::Transaction>::SignatureType &signature) override {
+          const interface::types::SignatureType &signature) override {
         if (signatures_->count(signature) > 0) {
           return false;
         }

diff --git a/shared_model/bindings/bindings.i b/shared_model/bindings/bindings.i
@@ -32,6 +32,7 @@
 #include "bindings/simple_builder.hpp"
 #include "bindings/simple_signer.hpp"
 #include "bindings/simple_transaction_proto.hpp"
+#include "builders/protobuf/unsigned_proto.hpp"
 %}
 
 %include "interfaces/common_objects/types.hpp"
@@ -43,10 +44,12 @@
 %include "cryptography/signed.hpp"
 %include "backend/protobuf/transaction.hpp"
 %include "builders/protobuf/proto_transaction_builder.hpp"
+%include "builders/protobuf/unsigned_proto.hpp"
 %include "bindings/simple_builder.hpp"
 %include "bindings/simple_signer.hpp"
 %include "bindings/simple_transaction_proto.hpp"
 
+%template (unsignedTx) shared_model::proto::UnsignedWrapper<shared_model::proto::Transaction>;
 
 namespace shared_model {
   namespace proto {
@@ -60,7 +63,7 @@ namespace iroha {
   namespace protocol {
     class Transaction {
     public:
-        size_t ByteSizeLong();
+      size_t ByteSizeLong();
     };
   }
 }
diff --git a/shared_model/bindings/simple_builder.cpp b/shared_model/bindings/simple_builder.cpp
@@ -37,6 +37,8 @@ namespace shared_model {
       return builder_.addAssetQuantity(account_id, asset_id, amount);
     }
 
-    Transaction SimpleBuilder::build() { return builder_.build(); }
+    UnsignedWrapper<Transaction> SimpleBuilder::build() {
+      return builder_.build();
+    }
   }  // namespace proto
 }  // namespace shared_model
diff --git a/shared_model/bindings/simple_builder.hpp b/shared_model/bindings/simple_builder.hpp
@@ -19,6 +19,7 @@
 #define IROHA_SIMPLE_BUILDER_HPP
 
 #include "builders/protobuf/proto_transaction_builder.hpp"
+#include "builders/protobuf/unsigned_proto.hpp"
 
 namespace shared_model {
   namespace proto {
@@ -38,7 +39,7 @@ namespace shared_model {
           const interface::types::AssetIdType &asset_id,
           const std::string &amount);
 
-      Transaction build();
+      UnsignedWrapper<Transaction> build();
 
      private:
       TransactionBuilder builder_;

diff --git a/shared_model/bindings/simple_signer.cpp b/shared_model/bindings/simple_signer.cpp
@@ -16,7 +16,6 @@
  */
 
 #include "bindings/simple_signer.hpp"
-#include "cryptography/crypto_provider/crypto_signer.hpp"
 #include "cryptography/ed25519_sha3_impl/crypto_provider.hpp"
 #include "cryptography/seed.hpp"
 
@@ -25,10 +24,5 @@ namespace shared_model {
     crypto::Keypair SimpleSigner::generateKeypair() {
       return crypto::CryptoProviderEd25519Sha3::generateKeypair();
     }
-
-    crypto::Signed SimpleSigner::sign(const crypto::Blob &blob,
-                                      crypto::Keypair &keypair) {
-      return crypto::CryptoSigner<>::sign(blob, keypair);
-    }
   }  // namespace proto
 }  // namespace shared_model
diff --git a/shared_model/bindings/simple_signer.hpp b/shared_model/bindings/simple_signer.hpp
@@ -27,8 +27,6 @@ namespace shared_model {
     class SimpleSigner {
      public:
       crypto::Keypair generateKeypair();
-
-      crypto::Signed sign(const crypto::Blob &blob, crypto::Keypair &keypair);
     };
   }  // namespace proto
 }  // namespace shared_model

diff --git a/shared_model/bindings/simple_transaction_proto.cpp b/shared_model/bindings/simple_transaction_proto.cpp
@@ -21,17 +21,12 @@
 
 namespace shared_model {
   namespace proto {
-    bool SimpleTransactionProto::addSignature(Transaction &tx,
-                                              crypto::Signed sign,
-                                              crypto::PublicKey publicKey) {
-      iroha::protocol::Signature protosig;
-      protosig.set_pubkey(publicKey.blob());
-      protosig.set_signature(sign.blob());
-      Signature *s1 = new Signature(protosig);
-      return tx.addSignature(detail::PolymorphicWrapper<Signature>(s1));
+    Transaction SimpleTransactionProto::signAndAddSignature(
+        UnsignedWrapper<Transaction> &tx, const crypto::Keypair &keypair) {
+      return tx.signAndAddSignature(keypair);
     }
 
-    iroha::protocol::Transaction SimpleTransactionProto::getTransport(
+    const iroha::protocol::Transaction &SimpleTransactionProto::getTransport(
         const Transaction &tx) {
       return tx.getTransport();
     }

diff --git a/shared_model/bindings/simple_transaction_proto.hpp b/shared_model/bindings/simple_transaction_proto.hpp
@@ -19,14 +19,16 @@
 #define IROHA_SIMPLE_TRANSACTION_PROTO_HPP
 
 #include "builders/protobuf/proto_transaction_builder.hpp"
+#include "builders/protobuf/unsigned_proto.hpp"
 
 namespace shared_model {
   namespace proto {
     class SimpleTransactionProto {
      public:
-      bool addSignature(Transaction &tx, crypto::Signed sign, crypto::PublicKey publicKey);
+      Transaction signAndAddSignature(UnsignedWrapper<Transaction> &tx,
+                                      const crypto::Keypair &keypair);
 
-      iroha::protocol::Transaction getTransport(const Transaction &tx);
+      const iroha::protocol::Transaction &getTransport(const Transaction &tx);
     };
   }  // namespace proto
 }  // namespace shared_model

diff --git a/shared_model/builders/protobuf/unsigned_proto.hpp b/shared_model/builders/protobuf/unsigned_proto.hpp
@@ -0,0 +1,63 @@
+/**
+ * Copyright Soramitsu Co., Ltd. 2017 All Rights Reserved.
+ * http://soramitsu.co.jp
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef IROHA_UNSIGNED_PROTO_HPP
+#define IROHA_UNSIGNED_PROTO_HPP
+
+#include "backend/protobuf/common_objects/signature.hpp"
+#include "cryptography/crypto_provider/crypto_signer.hpp"
+#include "cryptography/keypair.hpp"
+
+namespace shared_model {
+  namespace proto {
+    /**
+     * Class for holding built but still unsigned objects
+     * @tparam T - type of object received from builder
+     */
+    template <typename T>
+    class UnsignedWrapper {
+     public:
+      /**
+       * Constructs new unsigned object instance
+       * @param o - object received from builder
+       */
+      UnsignedWrapper(const T &o) : unsigned_(o) {}
+
+      /**
+       * Add signature and retrieve signed result
+       * @param signature - signature to add
+       * @return signed object
+       */
+      T signAndAddSignature(const crypto::Keypair &keypair) {
+        auto signedBlob = shared_model::crypto::CryptoSigner<>::sign(
+            shared_model::crypto::Blob(unsigned_.blob()), keypair);
+        iroha::protocol::Signature protosig;
+        protosig.set_pubkey(keypair.publicKey().blob());
+        protosig.set_signature(signedBlob.blob());
+        auto *s1 = new Signature(protosig);
+        unsigned_.addSignature(detail::PolymorphicWrapper<Signature>(
+            s1));  // TODO: 05.12.2017 luckychess think about false case
+        return unsigned_;
+      }
+
+     private:
+      T unsigned_;
+    };
+  }  // namespace proto
+}  // namespace shared_model
+
+#endif  // IROHA_UNSIGNED_PROTO_HPP
diff --git a/shared_model/cryptography/blob.hpp b/shared_model/cryptography/blob.hpp
@@ -42,7 +42,7 @@ namespace shared_model {
         std::stringstream ss;
         ss << std::hex << std::setfill('0');
         for (const auto &c : blob_) {
-          ss << std::setw(2) << static_cast<int>(c);
+          ss << std::setw(2) << (static_cast<int>(c) & 0xff);
         }
         hex_ = ss.str();
       }

diff --git a/shared_model/interfaces/common_objects/types.hpp b/shared_model/interfaces/common_objects/types.hpp
@@ -23,6 +23,7 @@
 #include <vector>
 #include "common/types.hpp"
 #include "cryptography/public_key.hpp"
+#include "interfaces/common_objects/signature.hpp"
 #include "interfaces/polymorphic_wrapper.hpp"
 
 namespace shared_model {
@@ -50,6 +51,8 @@ namespace shared_model {
       using PermissionNameType = std::string;
       /// Type of Quorum used in transaction and set quorum
       using QuorumType = uint32_t;
+      /// Type of transaction signature
+      using SignatureType = detail::PolymorphicWrapper<Signature>;
       /// Type of peer address
       using AddressType = std::string;
     }  // namespace types

diff --git a/shared_model/interfaces/signable.hpp b/shared_model/interfaces/signable.hpp
@@ -21,6 +21,7 @@
 #include <boost/functional/hash.hpp>
 #include <unordered_set>
 #include "interfaces/common_objects/signature.hpp"
+#include "interfaces/common_objects/types.hpp"
 #include "interfaces/hashable.hpp"
 #include "interfaces/polymorphic_wrapper.hpp"
 #include "utils/string_builder.hpp"
@@ -37,9 +38,6 @@ namespace shared_model {
     template <typename Model, typename OldModel>
     class Signable : public Hashable<Model, OldModel> {
      public:
-      /// Type of transaction signature
-      using SignatureType = detail::PolymorphicWrapper<Signature>;
-
       /**
        * Hash class for SigWrapper type. It's required since std::unordered_set
        * uses hash inside and it should be declared explicitly for user-defined
@@ -53,7 +51,7 @@ namespace shared_model {
          * @param sig - item to find hash from
          * @return calculated hash
          */
-        size_t operator()(const SignatureType &sig) const {
+        size_t operator()(const types::SignatureType &sig) const {
           std::size_t seed = 0;
           boost::hash_combine(seed, sig->publicKey().blob());
           boost::hash_combine(seed, sig->signedData().blob());
@@ -68,7 +66,8 @@ namespace shared_model {
        * limitations: it requires to have write access for elements for some
        * internal operations.
        */
-      using SignatureSetType = std::unordered_set<SignatureType, SignableHash>;
+      using SignatureSetType =
+          std::unordered_set<types::SignatureType, SignableHash>;
 
       /**
        * @return attached signatures
@@ -80,7 +79,7 @@ namespace shared_model {
        * @param signature - signature object for insertion
        * @return true, if signature was added
        */
-      virtual bool addSignature(const SignatureType &signature) = 0;
+      virtual bool addSignature(const types::SignatureType &signature) = 0;
 
       /// Type of timestamp
       using TimestampType = uint64_t;

diff --git a/test/module/shared_model/backend_proto/CMakeLists.txt b/test/module/shared_model/backend_proto/CMakeLists.txt
@@ -34,6 +34,7 @@ addtest(shared_proto_transaction_test
     )
 target_link_libraries(shared_proto_transaction_test
     shared_model_proto_backend
+    shared_model_ed25519_sha3
     )
 
 addtest(shared_proto_queries_test

diff --git a/test/module/shared_model/backend_proto/shared_proto_transaction_test.cpp b/test/module/shared_model/backend_proto/shared_proto_transaction_test.cpp
@@ -16,6 +16,10 @@
  */
 
 #include "backend/protobuf/transaction.hpp"
+#include "builders/protobuf/proto_transaction_builder.hpp"
+#include "cryptography/crypto_provider/crypto_signer.hpp"
+#include "cryptography/ed25519_sha3_impl/crypto_provider.hpp"
+#include "interfaces/polymorphic_wrapper.hpp"
 #include "builders/protobuf/transaction.hpp"
 
 #include <gtest/gtest.h>
@@ -54,12 +58,23 @@ TEST(ProtoTransaction, Builder) {
   command.mutable_amount()->mutable_value()->set_fourth(1000);
   command.mutable_amount()->set_precision(2);
 
+  auto keypair =
+      shared_model::crypto::CryptoProviderEd25519Sha3::generateKeypair();
+  auto signedProto = shared_model::crypto::CryptoSigner<>::sign(
+      shared_model::crypto::Blob(proto_tx.SerializeAsString()), keypair);
+
+  auto sig = proto_tx.add_signature();
+  sig->set_pubkey(keypair.publicKey().blob());
+  sig->set_signature(signedProto.blob());
+
   auto tx = shared_model::proto::TransactionBuilder()
                 .txCounter(tx_counter)
                 .creatorAccountId(account_id)
                 .assetQuantity(account_id, asset_id, amount)
                 .createdTime(created_time)
                 .build();
+
+  auto signedTx = tx.signAndAddSignature(keypair);
   auto &proto = tx.getTransport();
 
   ASSERT_EQ(proto_tx.SerializeAsString(), proto.SerializeAsString());