Refactor demo and block payload implementations #1770
Conversation
shenkeyao
requested review from
bfish713,
ggutoski,
jbearer and
elliedavidson
crates/hotshot/src/block_impl.rs
Outdated
| // TODO: Use use VID block commitment. | ||
| // <https://github.com/EspressoSystems/HotShot/issues/1730> | ||
| commit::RawCommitmentBuilder::new("Txn Comm").finalize() |
There was a problem hiding this comment.
How does the transaction commitment relate to the block commitment? Does it at all? My understanding is VID just treats the whole block as a blob, without regard for transaction boundaries. Then I think the only use of the transaction hash would be as a unique index, say for filtering duplicate transactions and stuff, peripheral to the core of consensus? In that case I think this can just be a regular Keccak commitment (ie using the commit library as we normally do)
There was a problem hiding this comment.
Agree! Ideally, this function could just return the keccak hash, but this would need updates to the Committable trait and some other structs that implement Committable, so as a short-term fix, I calculated the Keccak256 hash and then converted it into a Commitment.
There was a problem hiding this comment.
Made an issue for commitment refactoring: #1774.
vdemo(for validating consensus).sdemoto the upper crate, and renames it todemosince it's now the only demo.demotoblock_impl.Closes #1673.