-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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
.