Skip to content

Commit d55f255

Browse files
committed
Fix a bug in the echo round where the guilty party was assigned incorrectly
1 parent 876cd19 commit d55f255

File tree

3 files changed

+29
-10
lines changed

3 files changed

+29
-10
lines changed

manul/src/session/echo.rs

+22-5
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,12 @@ pub(crate) enum EchoRoundError<Id> {
3434
///
3535
/// The attached identifier points out the sender for whom the echoed message was invalid,
3636
/// to speed up the verification process.
37-
InvalidEcho(Id),
37+
InvalidEcho {
38+
// Even though this will be the same as the message sender, it is convenient to record it here
39+
// because of the way this error will be processed.
40+
guilty_party: Id,
41+
failed_for: Id,
42+
},
3843
/// The originally received message and the one received in the echo pack were both valid,
3944
/// but different.
4045
///
@@ -49,7 +54,7 @@ pub(crate) enum EchoRoundError<Id> {
4954
impl<Id> EchoRoundError<Id> {
5055
pub(crate) fn description(&self) -> String {
5156
match self {
52-
Self::InvalidEcho(_) => "Invalid message received among the ones echoed".into(),
57+
Self::InvalidEcho { .. } => "Invalid message received among the ones echoed".into(),
5358
Self::MismatchedBroadcasts { .. } => {
5459
"The echoed message is different from the originally received one".into()
5560
}
@@ -236,17 +241,29 @@ where
236241
// This means `from` sent us an incorrectly signed message.
237242
// Provable fault of `from`.
238243
Err(MessageVerificationError::InvalidSignature) => {
239-
return Err(EchoRoundError::InvalidEcho(sender.clone()).into())
244+
return Err(EchoRoundError::InvalidEcho {
245+
guilty_party: from.clone(),
246+
failed_for: sender.clone(),
247+
}
248+
.into())
240249
}
241250
Err(MessageVerificationError::SignatureMismatch) => {
242-
return Err(EchoRoundError::InvalidEcho(sender.clone()).into())
251+
return Err(EchoRoundError::InvalidEcho {
252+
guilty_party: from.clone(),
253+
failed_for: sender.clone(),
254+
}
255+
.into())
243256
}
244257
};
245258

246259
// `from` sent us a correctly signed message but from another round or another session.
247260
// Provable fault of `from`.
248261
if verified_echo.metadata() != previously_received_echo.metadata() {
249-
return Err(EchoRoundError::InvalidEcho(sender.clone()).into());
262+
return Err(EchoRoundError::InvalidEcho {
263+
guilty_party: from.clone(),
264+
failed_for: sender.clone(),
265+
}
266+
.into());
250267
}
251268

252269
// `sender` sent us and `from` messages with different payloads,

manul/src/session/evidence.rs

+6-4
Original file line numberDiff line numberDiff line change
@@ -164,18 +164,20 @@ where
164164
}
165165

166166
pub(crate) fn new_echo_round_error(
167-
verifier: &SP::Verifier,
168167
normal_broadcast: SignedMessagePart<NormalBroadcast>,
169168
error: EchoRoundError<SP::Verifier>,
170169
) -> Result<Self, LocalError> {
171170
let description = format!("Echo round error: {}", error.description());
172171
match error {
173-
EchoRoundError::InvalidEcho(from) => Ok(Self {
174-
guilty_party: verifier.clone(),
172+
EchoRoundError::InvalidEcho {
173+
guilty_party,
174+
failed_for,
175+
} => Ok(Self {
176+
guilty_party,
175177
description,
176178
evidence: EvidenceEnum::InvalidEchoPack(InvalidEchoPackEvidence {
177179
normal_broadcast,
178-
invalid_echo_sender: from,
180+
invalid_echo_sender: failed_for,
179181
}),
180182
}),
181183
EchoRoundError::MismatchedBroadcasts {

manul/src/session/session.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -738,7 +738,7 @@ where
738738
ReceiveErrorType::Unprovable(error) => self.register_unprovable_error(&from, error),
739739
ReceiveErrorType::Echo(error) => {
740740
let (_echo_broadcast, normal_broadcast, _direct_message) = processed.message.into_parts();
741-
let evidence = Evidence::new_echo_round_error(&from, normal_broadcast, *error)?;
741+
let evidence = Evidence::new_echo_round_error(normal_broadcast, *error)?;
742742
self.register_provable_error(&from, evidence)
743743
}
744744
ReceiveErrorType::Local(error) => Err(error),

0 commit comments

Comments
 (0)