fix signature verification bug

11 files changed, 39 insertions, 30 deletions

diff --git a/include/hotstuff/consensus.h b/include/hotstuff/consensus.h
index 6de6bd3..cb86bf6 100644
--- a/include/hotstuff/consensus.h
+++ b/include/hotstuff/consensus.h
@@ -166,7 +166,7 @@ struct Proposal: public Serializable {
           >> bqc_hash;
         Block _blk;
         _blk.unserialize(s, hsc);
-        blk = hsc->storage->add_blk(std::move(_blk));
+        blk = hsc->storage->add_blk(std::move(_blk), hsc->get_config());
     }
 
     operator std::string () const {
diff --git a/include/hotstuff/crypto.h b/include/hotstuff/crypto.h
index 32997c8..40c9140 100644
--- a/include/hotstuff/crypto.h
+++ b/include/hotstuff/crypto.h
@@ -340,7 +340,7 @@ class PartCertSecp256k1: public SigSecp256k1, public PartCert {
 class QuorumCertSecp256k1: public QuorumCert {
     uint256_t blk_hash;
     salticidae::Bits rids;
-    std::vector<SigSecp256k1> sigs;
+    std::unordered_map<ReplicaID, SigSecp256k1> sigs;
 
     public:
     QuorumCertSecp256k1() = default;
@@ -349,11 +349,9 @@ class QuorumCertSecp256k1: public QuorumCert {
     void add_part(ReplicaID rid, const PartCert &pc) override {
         if (pc.get_blk_hash() != blk_hash)
             throw std::invalid_argument("PartCert does match the block hash");
-        if (!rids.get(rid))
-        {
-            rids.set(rid);
-            sigs.push_back(static_cast<const PartCertSecp256k1 &>(pc));
-        }
+        sigs.insert(std::make_pair(
+            rid, static_cast<const PartCertSecp256k1 &>(pc)));
+        rids.set(rid);
     }
 
     void compute() override {}
@@ -368,13 +366,14 @@ class QuorumCertSecp256k1: public QuorumCert {
 
     void serialize(DataStream &s) const override {
         s << blk_hash << rids;
-        for (const auto &sig: sigs) s << sig;
+        for (size_t i = 0; i < rids.size(); i++)
+            if (rids.get(i)) s << sigs.at(i);
     }
 
     void unserialize(DataStream &s) override {
         s >> blk_hash >> rids;
-        sigs.resize(rids.size());
-        for (auto &sig: sigs) s >> sig;
+        for (size_t i = 0; i < rids.size(); i++)
+            if (rids.get(i)) s >> sigs[i];
     }
 };
 
diff --git a/include/hotstuff/entity.h b/include/hotstuff/entity.h
index 1fca18c..ba3906b 100644
--- a/include/hotstuff/entity.h
+++ b/include/hotstuff/entity.h
@@ -44,10 +44,14 @@ class ReplicaConfig {
     std::unordered_map<ReplicaID, ReplicaInfo> replica_map;
 
     public:
+    size_t nreplicas;
     size_t nmajority;
 
+    ReplicaConfig(): nreplicas(0), nmajority(0) {}
+
     void add_replica(ReplicaID rid, const ReplicaInfo &info) {
         replica_map.insert(std::make_pair(rid, info));
+        nreplicas++;
     }
 
     const ReplicaInfo &get_info(ReplicaID rid) const {
@@ -209,7 +213,12 @@ class EntityStorage {
         return blk_cache.count(blk_hash);
     }
 
-    const block_t &add_blk(Block &&_blk) {
+    block_t add_blk(Block &&_blk, const ReplicaConfig &config) {
+        if (!_blk.verify(config))
+        {
+            HOTSTUFF_LOG_WARN("block is invalid");
+            return nullptr;
+        }
         block_t blk = new Block(std::move(_blk));
         return blk_cache.insert(std::make_pair(blk->get_hash(), blk)).first->second;
     }
diff --git a/include/hotstuff/hotstuff.h b/include/hotstuff/hotstuff.h
index b6bebb9..4e7332f 100644
--- a/include/hotstuff/hotstuff.h
+++ b/include/hotstuff/hotstuff.h
@@ -223,6 +223,8 @@ class HotStuff: public HotStuffBase {
     protected:
 
     part_cert_bt create_part_cert(const PrivKey &priv_key, const uint256_t &blk_hash) override {
+        HOTSTUFF_LOG_DEBUG("create part cert with priv=%s, blk_hash=%s",
+                            get_hex10(priv_key).c_str(), get_hex10(blk_hash).c_str());
         return new PartCertType(
                     static_cast<const PrivKeyType &>(priv_key),
                     blk_hash);
diff --git a/include/hotstuff/type.h b/include/hotstuff/type.h
index 6014dab..0897956 100644
--- a/include/hotstuff/type.h
+++ b/include/hotstuff/type.h
@@ -28,7 +28,8 @@ using salticidae::Event;
 using salticidae::EventContext;
 using promise::promise_t;
 
-inline std::string get_hex10(const uint256_t &x) {
+template<typename SerialType>
+inline std::string get_hex10(const SerialType &x) {
     return get_hex(x).substr(0, 10);
 }
 
diff --git a/run_replicas.sh b/run_replicas.sh
index f053701..5f54787 100755
--- a/run_replicas.sh
+++ b/run_replicas.sh
@@ -5,8 +5,8 @@ if [[ $# -gt 0 ]]; then
 fi
 for i in "${rep[@]}"; do
     echo "starting replica $i"
-    valgrind --leak-check=full ./hotstuff-app --conf hotstuff-sec${i}.conf > log${i} 2>&1 &
+    #valgrind --leak-check=full ./hotstuff-app --conf hotstuff-sec${i}.conf > log${i} 2>&1 &
     #gdb -ex r -ex bt -ex q --args ./hotstuff-app --conf hotstuff-sec${i}.conf > log${i} 2>&1 &
-    #./hotstuff-app --conf hotstuff-sec${i}.conf > log${i} 2>&1 &
+    ./hotstuff-app --conf hotstuff-sec${i}.conf > log${i} 2>&1 &
 done
 wait
diff --git a/salticidae b/salticidae
-Subproject 473551d760bb52d3c8d12b2096c10210e938738
+Subproject d2de1d9a704fc4b23e9a9fb8d610620d6d4752c
diff --git a/src/consensus.cpp b/src/consensus.cpp
index 57152f0..ac7e56a 100644
--- a/src/consensus.cpp
+++ b/src/consensus.cpp
@@ -125,7 +125,7 @@ void HotStuffCore::on_propose(const std::vector<command_t> &cmds,
     }
     /* create the new block */
     block_t bnew = storage->add_blk(
-        Block(
+        new Block(
             parents,
             cmds,
             p->height + 1,
diff --git a/src/crypto.cpp b/src/crypto.cpp
index 560a8ac..88c2f57 100644
--- a/src/crypto.cpp
+++ b/src/crypto.cpp
@@ -8,19 +8,22 @@ secp256k1_context_t secp256k1_default_verify_ctx = new Secp256k1Context(false);
 
 QuorumCertSecp256k1::QuorumCertSecp256k1(
         const ReplicaConfig &config, const uint256_t &blk_hash):
-            QuorumCert(), blk_hash(blk_hash), rids(config.nmajority) {
+            QuorumCert(), blk_hash(blk_hash), rids(config.nreplicas) {
     rids.clear();
 }
    
 bool QuorumCertSecp256k1::verify(const ReplicaConfig &config) {
-    bytearray_t _blk_hash(blk_hash);
-    if (rids.size() < config.nmajority) return false;
+    if (sigs.size() < config.nmajority) return false;
     for (size_t i = 0; i < rids.size(); i++)
-        if (!sigs[i].verify(_blk_hash,
-                            static_cast<const PubKeySecp256k1 &>(config.get_pubkey(rids.get(i))),
-                            secp256k1_default_verify_ctx
-                            ))
+        if (rids.get(i))
+        {
+            HOTSTUFF_LOG_DEBUG("checking cert(%d), blk_hash=%s",
+                                i, get_hex10(blk_hash).c_str());
+            if (!sigs[i].verify(blk_hash,
+                            static_cast<const PubKeySecp256k1 &>(config.get_pubkey(i)),
+                            secp256k1_default_verify_ctx))
             return false;
+        }
     return true;
 }
 
diff --git a/src/hotstuff.cpp b/src/hotstuff.cpp
index 43da8a8..c01f2f3 100644
--- a/src/hotstuff.cpp
+++ b/src/hotstuff.cpp
@@ -53,13 +53,7 @@ void MsgRespBlock::postponed_parse(HotStuffCore *hsc) {
     {
         Block _blk;
         _blk.unserialize(serialized, hsc);
-        if (!_blk.verify(hsc->get_config()))
-            blk = hsc->storage->add_blk(std::move(_blk));
-        else
-        {
-            blk = nullptr;
-            LOG_WARN("block is invalid");
-        }
+        blk = hsc->storage->add_blk(std::move(_blk), hsc->get_config());
     }
 }
 
@@ -244,6 +238,7 @@ void HotStuffBase::propose_handler(MsgPropose &&msg, conn_t conn) {
     msg.postponed_parse(this);
     auto &prop = msg.proposal;
     block_t blk = prop.blk;
+    if (!blk) return;
     promise::all(std::vector<promise_t>{
         async_deliver_blk(prop.bqc_hash, peer),
         async_deliver_blk(blk->get_hash(), peer),
diff --git a/src/hotstuff_client.cpp b/src/hotstuff_client.cpp
index c1e7ea4..e213e78 100644
--- a/src/hotstuff_client.cpp
+++ b/src/hotstuff_client.cpp
@@ -67,7 +67,7 @@ void try_send() {
 
 void client_resp_cmd_handler(MsgRespCmd &&msg, MsgNetwork<opcode_t>::conn_t) {
     auto &fin = msg.fin;
-    HOTSTUFF_LOG_DEBUG("got %s", std::string(msg).c_str());
+    HOTSTUFF_LOG_DEBUG("got %s", std::string(msg.fin).c_str());
     const uint256_t &cmd_hash = fin.cmd_hash;
     auto it = waiting.find(cmd_hash);
     if (fin.rid != proposer)