-
Notifications
You must be signed in to change notification settings - Fork 32
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
Lr/eqc voting #3851
Lr/eqc voting #3851
Changes from all commits
d941ba9
0ea759a
e6c6492
0db0add
d074292
8505870
cff7d76
5304ca5
c6603af
de41a5a
680bab3
bca2f2b
4ac1db5
4073351
7f7986d
2f3b23b
eecb9f8
e733582
8a03eeb
4a51e13
1c8daf0
1489fa0
fab49f2
073cca6
8afbf58
615d14e
224748b
bd0d841
6cec489
687c5c7
8cde052
7593cc6
5b1be9f
884a445
cd4d315
3ded97f
744ac1a
f8b3a4a
7dc1ca6
11bd731
42672fd
c19e706
6b3f9cb
cc3a2a6
d8640b2
df0730f
a3842e8
c5173fd
50d9306
33e78e5
307e2c5
4a764ee
6db7096
1bd66cb
c76f674
e1c5a1d
5294a83
8e179fd
e150806
34b271a
4a1651d
e09dd6f
a653525
de5a488
ab4293a
4075e5d
7c46afc
0330ca4
4ad900f
8928fb0
0c70cc6
3abcbc4
bda9546
5f181c3
f022612
355f5a6
c7fb70e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,7 @@ use async_lock::RwLock; | |
use async_trait::async_trait; | ||
use hotshot_task::task::TaskState; | ||
use hotshot_types::{ | ||
consensus::Consensus, | ||
consensus::OuterConsensus, | ||
data::{VidDisperse, VidDisperseShare}, | ||
event::{Event, EventType, HotShotAction}, | ||
message::{ | ||
|
@@ -212,7 +212,7 @@ pub struct NetworkEventTaskState< | |
/// Storage to store actionable events | ||
pub storage: Arc<RwLock<S>>, | ||
/// Shared consensus state | ||
pub consensus: Arc<RwLock<Consensus<TYPES>>>, | ||
pub consensus: OuterConsensus<TYPES>, | ||
/// Lock for a decided upgrade | ||
pub upgrade_lock: UpgradeLock<TYPES, V>, | ||
/// map view number to transmit tasks | ||
|
@@ -294,7 +294,7 @@ impl< | |
|
||
let net = Arc::clone(&self.network); | ||
let storage = Arc::clone(&self.storage); | ||
let consensus = Arc::clone(&self.consensus); | ||
let consensus = OuterConsensus::new(Arc::clone(&self.consensus.inner_consensus)); | ||
spawn(async move { | ||
if NetworkEventTaskState::<TYPES, V, NET, S>::maybe_record_action( | ||
Some(HotShotAction::VidDisperse), | ||
|
@@ -320,7 +320,7 @@ impl< | |
async fn maybe_record_action( | ||
maybe_action: Option<HotShotAction>, | ||
storage: Arc<RwLock<S>>, | ||
consensus: Arc<RwLock<Consensus<TYPES>>>, | ||
consensus: OuterConsensus<TYPES>, | ||
view: <TYPES as NodeType>::View, | ||
) -> std::result::Result<(), ()> { | ||
if let Some(mut action) = maybe_action { | ||
|
@@ -408,6 +408,16 @@ impl< | |
TransmitType::Direct(leader), | ||
)) | ||
} | ||
HotShotEvent::ExtendedQuorumVoteSend(vote) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is there a reason we don't want to just use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
*maybe_action = Some(HotShotAction::Vote); | ||
Some(( | ||
vote.signing_key(), | ||
MessageKind::<TYPES>::from_consensus_message(SequencingMessage::General( | ||
GeneralConsensusMessage::Vote(vote.clone()), | ||
)), | ||
TransmitType::Broadcast, | ||
)) | ||
} | ||
HotShotEvent::QuorumProposalRequestSend(req, signature) => Some(( | ||
req.key.clone(), | ||
MessageKind::<TYPES>::from_consensus_message(SequencingMessage::General( | ||
|
@@ -669,7 +679,7 @@ impl< | |
.committee_members(view_number, self.epoch); | ||
let network = Arc::clone(&self.network); | ||
let storage = Arc::clone(&self.storage); | ||
let consensus = Arc::clone(&self.consensus); | ||
let consensus = OuterConsensus::new(Arc::clone(&self.consensus.inner_consensus)); | ||
let upgrade_lock = self.upgrade_lock.clone(); | ||
let handle = spawn(async move { | ||
if NetworkEventTaskState::<TYPES, V, NET, S>::maybe_record_action( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -283,6 +283,7 @@ pub(crate) async fn submit_vote<TYPES: NodeType, I: NodeImplementation<TYPES>, V | |
storage: Arc<RwLock<I::Storage>>, | ||
leaf: Leaf<TYPES>, | ||
vid_share: Proposal<TYPES, VidDisperseShare<TYPES>>, | ||
extended_vote: bool, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I understand the reason for this either -- are we planning for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only difference is that the last vote in 3-chain is broadcast instead of just sent to the next leader. To differentiate between the last vote in the 3-chain against any other vote, we need to check the two leaves up the chain. That logic seems to belong to the voting part instead of networking part. We now use the new |
||
) -> Result<()> { | ||
ensure!( | ||
quorum_membership.has_stake(&public_key, epoch_number), | ||
|
@@ -317,7 +318,16 @@ pub(crate) async fn submit_vote<TYPES: NodeType, I: NodeImplementation<TYPES>, V | |
.await | ||
.wrap() | ||
.context(error!("Failed to store VID share"))?; | ||
broadcast_event(Arc::new(HotShotEvent::QuorumVoteSend(vote)), &sender).await; | ||
|
||
if extended_vote { | ||
broadcast_event( | ||
Arc::new(HotShotEvent::ExtendedQuorumVoteSend(vote)), | ||
&sender, | ||
) | ||
.await; | ||
} else { | ||
broadcast_event(Arc::new(HotShotEvent::QuorumVoteSend(vote)), &sender).await; | ||
} | ||
|
||
Ok(()) | ||
} |
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.
Do we also need all this logic in the
validate_proposal_liveness
path, I suppose we can't actually do it there because we are missing the previous proposal but don't we need something to check the epochs?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.
I think we cannot check the epochs there. We can only calculate the epoch based on the block number. I don't think we have an access to the parent's block number if we don't have the parent leaf. Did I miss something?
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.
I don't think so, I was just trying to be sure we didn't. I also don't think we can do the check in the liveness path, and we won't vote in that case either