hot-stuff / libhotstuff

A general-purpose BFT state machine replication library with modularity and simplicity, suitable for building hybrid consensus cryptocurrencies.
Apache License 2.0
239 stars 81 forks source link

Validation of Quorum Certificates #21

Open gramseyer opened 3 years ago

gramseyer commented 3 years ago

Hello,

I've been considering using HotStuff in a research project (which means reimplementing parts of the codebase) and had one question about this implementation. When must quorum certificates be checked?

My understanding from reading the HotStuff paper was that a proposal's QC should point to a previous block (hash or block.justify), and that the associated signatures need to be on the previous hash and need to be valid. It seemed as though this means that QCs should be validated upon receipt of a new block proposal.

The Diem consensus implementation appears to follow this behavior, i.e. verify the QC immediately upon receipt of a block proposal.

This implementation, however, appears to not verify the QCs of new proposals. Particularly, I do not see a call site for QuorumSet::verify other than in Block::verify, and I do not see a call to Block::verify other than in HotStuffBase::async_deliver_block (hotstuff.cpp line 188), where the result of checking the QC on a newly received block appears to be used to print a warning and is then discarded.

        std::vector<promise_t> pms;
        const auto &qc = blk->get_qc();
        assert(qc);
        if (blk == get_genesis())
            pms.push_back(promise_t([](promise_t &pm){ pm.resolve(true); }));
        else
            pms.push_back(blk->verify(this, vpool));
        pms.push_back(async_fetch_blk(qc->get_obj_hash(), &replica));
        /* the parents should be delivered */
        for (const auto &phash: blk->get_parent_hashes())
            pms.push_back(async_deliver_blk(phash, replica));
        promise::all(pms).then([this, blk](const promise::values_t values) {
            auto ret = promise::any_cast<bool>(values[0]) && this->on_deliver_blk(blk);
            if (!ret)
                HOTSTUFF_LOG_WARN("verification failed during async delivery");
        });

This would seem to allow delivery of blocks with invalid QCs.

Am I misunderstanding the codebase, or am I misunderstanding the HotStuff paper?

Thanks!