-
Notifications
You must be signed in to change notification settings - Fork 59
fix: Transaction task use quorum exchange instead of committee exchange #1864
Conversation
…n fix all the build breaks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for adding those useful comments!
crates/types/src/traits/election.rs
Outdated
// TODO GG why create a VoteData::DA only to discard it immediately? | ||
// Why not just have a `sign_commitment()` member? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we want to distinguish the vote type when signing, so a signature on a no vote can't be used to verify a yes vote.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But get_commit()
just returns the underlying commitment regardless of the enum variant:
HotShot/crates/types/src/traits/election.rs
Lines 103 to 111 in 42d19ba
pub fn get_commit(&self) -> COMMITMENT { | |
#[allow(clippy::enum_glob_use)] | |
use VoteData::*; | |
match self { | |
DA(c) | Yes(c) | No(c) | Timeout(c) | ViewSyncPreCommit(c) | ViewSyncCommit(c) | |
| ViewSyncFinalize(c) => *c, | |
} | |
} | |
} |
Hmm. Seems it was me who made that change in #1777: https://github.com/EspressoSystems/HotShot/pull/1777/files
Ok, I need to fix this. I'll push more commits to this PR. Thanks for explaining!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 1fdc06a
close #1693
Change the transactions task to use
quorum_exchange
instead ofcommittee_exchange
so that VID can retrieve the number of storage nodes.The VID erasure code rate is still hard-coded. That's a separate issue #1734 .
Other things in this PR beyond #1693
Committable
impl forVIDBlockPayload
to re-use the commitment bytes instead of computing another commitment of the commitment.test_memory_network
to justfile.TODO GG
.