Remove overzealous clippy overrides

We have overridden a number of clippy warnings such as
"large enum variant".

Considering that we have a number of issues
 with the stack size in CI, it is more prudent to follow clippy's advice
 and box larger items so that the enum does not take larger space.

Do note that an instance of the enum always takes as much space as its
largest variant.
This commit is contained in:
Franck Royer 2020-12-23 15:06:43 +11:00
parent cdf2800fa5
commit 2a778f5644
No known key found for this signature in database
GPG Key ID: A82ED75A8DFC50A4
15 changed files with 103 additions and 71 deletions

View File

@ -53,15 +53,15 @@ pub fn new_swarm(
Ok(swarm) Ok(swarm)
} }
#[allow(clippy::large_enum_variant)]
#[derive(Debug)] #[derive(Debug)]
pub enum OutEvent { pub enum OutEvent {
ConnectionEstablished(PeerId), ConnectionEstablished(PeerId),
// TODO (Franck): Change this to get both amounts so parties can verify the amounts are // TODO (Franck): Change this to get both amounts so parties can verify the amounts are
// expected early on. // expected early on.
Request(amounts::OutEvent), // Not-uniform with Bob on purpose, ready for adding Xmr event. Request(Box<amounts::OutEvent>), /* Not-uniform with Bob on purpose, ready for adding Xmr
* event. */
Message0 { Message0 {
msg: bob::Message0, msg: Box<bob::Message0>,
channel: ResponseChannel<AliceToBob>, channel: ResponseChannel<AliceToBob>,
}, },
Message1 { Message1 {
@ -87,14 +87,17 @@ impl From<peer_tracker::OutEvent> for OutEvent {
impl From<amounts::OutEvent> for OutEvent { impl From<amounts::OutEvent> for OutEvent {
fn from(event: amounts::OutEvent) -> Self { fn from(event: amounts::OutEvent) -> Self {
OutEvent::Request(event) OutEvent::Request(Box::new(event))
} }
} }
impl From<message0::OutEvent> for OutEvent { impl From<message0::OutEvent> for OutEvent {
fn from(event: message0::OutEvent) -> Self { fn from(event: message0::OutEvent) -> Self {
match event { match event {
message0::OutEvent::Msg { channel, msg } => OutEvent::Message0 { msg, channel }, message0::OutEvent::Msg { channel, msg } => OutEvent::Message0 {
msg: Box::new(msg),
channel,
},
} }
} }
} }

View File

@ -206,7 +206,7 @@ impl EventLoop {
let _ = self.conn_established.send(alice).await; let _ = self.conn_established.send(alice).await;
} }
OutEvent::Message0 { msg, channel } => { OutEvent::Message0 { msg, channel } => {
let _ = self.msg0.send((msg, channel)).await; let _ = self.msg0.send((*msg, channel)).await;
} }
OutEvent::Message1 { msg, channel } => { OutEvent::Message1 { msg, channel } => {
let _ = self.msg1.send((msg, channel)).await; let _ = self.msg1.send((msg, channel)).await;
@ -218,7 +218,7 @@ impl EventLoop {
let _ = self.msg3.send(msg).await; let _ = self.msg3.send(msg).await;
} }
OutEvent::Request(event) => { OutEvent::Request(event) => {
let _ = self.request.send(event).await; let _ = self.request.send(*event).await;
} }
} }
}, },

View File

@ -37,7 +37,7 @@ pub struct Message0 {
impl Message0 { impl Message0 {
pub fn send(&mut self, channel: ResponseChannel<AliceToBob>, msg: xmr_btc::alice::Message0) { pub fn send(&mut self, channel: ResponseChannel<AliceToBob>, msg: xmr_btc::alice::Message0) {
let msg = AliceToBob::Message0(msg); let msg = AliceToBob::Message0(Box::new(msg));
self.rr.send_response(channel, msg); self.rr.send_response(channel, msg);
} }
fn poll( fn poll(
@ -82,7 +82,7 @@ impl NetworkBehaviourEventProcess<RequestResponseEvent<BobToAlice, AliceToBob>>
} => { } => {
if let BobToAlice::Message0(msg) = request { if let BobToAlice::Message0(msg) = request {
debug!("Received Message0"); debug!("Received Message0");
self.events.push_back(OutEvent::Msg { msg, channel }); self.events.push_back(OutEvent::Msg { msg: *msg, channel });
} }
} }
RequestResponseEvent::Message { RequestResponseEvent::Message {

View File

@ -38,7 +38,7 @@ pub struct Message1 {
impl Message1 { impl Message1 {
pub fn send(&mut self, channel: ResponseChannel<AliceToBob>, msg: xmr_btc::alice::Message1) { pub fn send(&mut self, channel: ResponseChannel<AliceToBob>, msg: xmr_btc::alice::Message1) {
let msg = AliceToBob::Message1(msg); let msg = AliceToBob::Message1(Box::new(msg));
self.rr.send_response(channel, msg); self.rr.send_response(channel, msg);
} }

View File

@ -42,7 +42,6 @@ trait Rng: RngCore + CryptoRng + Send {}
impl<T> Rng for T where T: RngCore + CryptoRng + Send {} impl<T> Rng for T where T: RngCore + CryptoRng + Send {}
#[derive(Debug)] #[derive(Debug)]
#[allow(clippy::large_enum_variant)]
pub enum AliceState { pub enum AliceState {
Started { Started {
amounts: SwapAmounts, amounts: SwapAmounts,
@ -51,36 +50,36 @@ pub enum AliceState {
Negotiated { Negotiated {
channel: Option<ResponseChannel<AliceToBob>>, channel: Option<ResponseChannel<AliceToBob>>,
amounts: SwapAmounts, amounts: SwapAmounts,
state3: State3, state3: Box<State3>,
}, },
BtcLocked { BtcLocked {
channel: Option<ResponseChannel<AliceToBob>>, channel: Option<ResponseChannel<AliceToBob>>,
amounts: SwapAmounts, amounts: SwapAmounts,
state3: State3, state3: Box<State3>,
}, },
XmrLocked { XmrLocked {
state3: State3, state3: Box<State3>,
}, },
EncSigLearned { EncSigLearned {
state3: State3,
encrypted_signature: EncryptedSignature, encrypted_signature: EncryptedSignature,
state3: Box<State3>,
}, },
BtcRedeemed, BtcRedeemed,
BtcCancelled { BtcCancelled {
state3: State3,
tx_cancel: TxCancel, tx_cancel: TxCancel,
state3: Box<State3>,
}, },
BtcRefunded { BtcRefunded {
spend_key: monero::PrivateKey, spend_key: monero::PrivateKey,
state3: State3, state3: Box<State3>,
}, },
BtcPunishable { BtcPunishable {
tx_refund: TxRefund, tx_refund: TxRefund,
state3: State3, state3: Box<State3>,
}, },
XmrRefunded, XmrRefunded,
CancelTimelockExpired { CancelTimelockExpired {
state3: State3, state3: Box<State3>,
}, },
BtcPunished, BtcPunished,
SafelyAborted, SafelyAborted,
@ -109,26 +108,28 @@ impl fmt::Display for AliceState {
impl From<&AliceState> for state::Alice { impl From<&AliceState> for state::Alice {
fn from(alice_state: &AliceState) -> Self { fn from(alice_state: &AliceState) -> Self {
match alice_state { match alice_state {
AliceState::Negotiated { state3, .. } => Alice::Negotiated(state3.clone()), AliceState::Negotiated { state3, .. } => Alice::Negotiated(state3.as_ref().clone()),
AliceState::BtcLocked { state3, .. } => Alice::BtcLocked(state3.clone()), AliceState::BtcLocked { state3, .. } => Alice::BtcLocked(state3.as_ref().clone()),
AliceState::XmrLocked { state3 } => Alice::XmrLocked(state3.clone()), AliceState::XmrLocked { state3 } => Alice::XmrLocked(state3.as_ref().clone()),
AliceState::EncSigLearned { AliceState::EncSigLearned {
state3, state3,
encrypted_signature, encrypted_signature,
} => Alice::EncSigLearned { } => Alice::EncSigLearned {
state: state3.clone(), state3: state3.as_ref().clone(),
encrypted_signature: encrypted_signature.clone(), encrypted_signature: encrypted_signature.clone(),
}, },
AliceState::BtcRedeemed => Alice::Done(AliceEndState::BtcRedeemed), AliceState::BtcRedeemed => Alice::Done(AliceEndState::BtcRedeemed),
AliceState::BtcCancelled { state3, .. } => Alice::BtcCancelled(state3.clone()), AliceState::BtcCancelled { state3, .. } => Alice::BtcCancelled(state3.as_ref().clone()),
AliceState::BtcRefunded { spend_key, state3 } => Alice::BtcRefunded { AliceState::BtcRefunded { spend_key, state3 } => Alice::BtcRefunded {
spend_key: *spend_key, spend_key: *spend_key,
state3: state3.clone(), state3: state3.as_ref().clone(),
}, },
AliceState::BtcPunishable { state3, .. } => Alice::BtcPunishable(state3.clone()), AliceState::BtcPunishable { state3, .. } => {
Alice::BtcPunishable(state3.as_ref().clone())
}
AliceState::XmrRefunded => Alice::Done(AliceEndState::XmrRefunded), AliceState::XmrRefunded => Alice::Done(AliceEndState::XmrRefunded),
AliceState::CancelTimelockExpired { state3 } => { AliceState::CancelTimelockExpired { state3 } => {
Alice::CancelTimelockExpired(state3.clone()) Alice::CancelTimelockExpired(state3.as_ref().clone())
} }
AliceState::BtcPunished => Alice::Done(AliceEndState::BtcPunished), AliceState::BtcPunished => Alice::Done(AliceEndState::BtcPunished),
AliceState::SafelyAborted => Alice::Done(AliceEndState::SafelyAborted), AliceState::SafelyAborted => Alice::Done(AliceEndState::SafelyAborted),
@ -151,7 +152,7 @@ impl From<state::Alice> for AliceState {
btc: state3.btc, btc: state3.btc,
xmr: state3.xmr, xmr: state3.xmr,
}, },
state3, state3: Box::new(state3),
}, },
Alice::BtcLocked(state3) => BtcLocked { Alice::BtcLocked(state3) => BtcLocked {
channel: None, channel: None,
@ -159,17 +160,21 @@ impl From<state::Alice> for AliceState {
btc: state3.btc, btc: state3.btc,
xmr: state3.xmr, xmr: state3.xmr,
}, },
state3, state3: Box::new(state3),
},
Alice::XmrLocked(state3) => XmrLocked {
state3: Box::new(state3),
}, },
Alice::XmrLocked(state3) => XmrLocked { state3 },
Alice::EncSigLearned { Alice::EncSigLearned {
state,
encrypted_signature,
} => EncSigLearned {
state3: state, state3: state,
encrypted_signature, encrypted_signature,
} => EncSigLearned {
state3: Box::new(state),
encrypted_signature,
},
Alice::CancelTimelockExpired(state3) => AliceState::CancelTimelockExpired {
state3: Box::new(state3),
}, },
Alice::CancelTimelockExpired(state3) => AliceState::CancelTimelockExpired { state3 },
Alice::BtcCancelled(state) => { Alice::BtcCancelled(state) => {
let tx_cancel = bitcoin::TxCancel::new( let tx_cancel = bitcoin::TxCancel::new(
&state.tx_lock, &state.tx_lock,
@ -179,30 +184,28 @@ impl From<state::Alice> for AliceState {
); );
BtcCancelled { BtcCancelled {
state3: state, state3: Box::new(state),
tx_cancel, tx_cancel,
} }
} }
Alice::BtcPunishable(state) => { Alice::BtcPunishable(state3) => {
let tx_cancel = bitcoin::TxCancel::new( let tx_cancel = bitcoin::TxCancel::new(
&state.tx_lock, &state3.tx_lock,
state.cancel_timelock, state3.cancel_timelock,
state.a.public(), state3.a.public(),
state.B, state3.B,
); );
let tx_refund = bitcoin::TxRefund::new(&tx_cancel, &state.refund_address); let tx_refund = bitcoin::TxRefund::new(&tx_cancel, &state3.refund_address);
BtcPunishable { BtcPunishable {
tx_refund, tx_refund,
state3: state, state3: Box::new(state3),
} }
} }
Alice::BtcRefunded { Alice::BtcRefunded {
state3: state, state3, spend_key, ..
spend_key,
..
} => BtcRefunded { } => BtcRefunded {
spend_key, spend_key,
state3: state, state3: Box::new(state3),
}, },
Alice::Done(end_state) => match end_state { Alice::Done(end_state) => match end_state {
AliceEndState::SafelyAborted => SafelyAborted, AliceEndState::SafelyAborted => SafelyAborted,
@ -286,7 +289,7 @@ pub async fn run_until(
let state = AliceState::Negotiated { let state = AliceState::Negotiated {
channel: Some(channel), channel: Some(channel),
amounts, amounts,
state3, state3: Box::new(state3),
}; };
let db_state = (&state).into(); let db_state = (&state).into();
@ -357,7 +360,7 @@ pub async fn run_until(
lock_xmr( lock_xmr(
channel, channel,
amounts, amounts,
state3.clone(), *state3.clone(),
&mut event_loop_handle, &mut event_loop_handle,
monero_wallet.clone(), monero_wallet.clone(),
) )

View File

@ -45,13 +45,12 @@ pub fn new_swarm(transport: SwapTransport, behaviour: Behaviour) -> Result<Swarm
Ok(swarm) Ok(swarm)
} }
#[allow(clippy::large_enum_variant)]
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub enum OutEvent { pub enum OutEvent {
ConnectionEstablished(PeerId), ConnectionEstablished(PeerId),
Amounts(SwapAmounts), Amounts(SwapAmounts),
Message0(alice::Message0), Message0(Box<alice::Message0>),
Message1(alice::Message1), Message1(Box<alice::Message1>),
Message2(alice::Message2), Message2(alice::Message2),
Message3, Message3,
} }
@ -77,7 +76,7 @@ impl From<amounts::OutEvent> for OutEvent {
impl From<message0::OutEvent> for OutEvent { impl From<message0::OutEvent> for OutEvent {
fn from(event: message0::OutEvent) -> Self { fn from(event: message0::OutEvent) -> Self {
match event { match event {
message0::OutEvent::Msg(msg) => OutEvent::Message0(msg), message0::OutEvent::Msg(msg) => OutEvent::Message0(Box::new(msg)),
} }
} }
} }
@ -85,7 +84,7 @@ impl From<message0::OutEvent> for OutEvent {
impl From<message1::OutEvent> for OutEvent { impl From<message1::OutEvent> for OutEvent {
fn from(event: message1::OutEvent) -> Self { fn from(event: message1::OutEvent) -> Self {
match event { match event {
message1::OutEvent::Msg(msg) => OutEvent::Message1(msg), message1::OutEvent::Msg(msg) => OutEvent::Message1(Box::new(msg)),
} }
} }
} }

View File

@ -192,10 +192,10 @@ impl EventLoop {
} }
OutEvent::Amounts(_amounts) => info!("Amounts received from Alice"), OutEvent::Amounts(_amounts) => info!("Amounts received from Alice"),
OutEvent::Message0(msg) => { OutEvent::Message0(msg) => {
let _ = self.msg0.send(msg).await; let _ = self.msg0.send(*msg).await;
} }
OutEvent::Message1(msg) => { OutEvent::Message1(msg) => {
let _ = self.msg1.send(msg).await; let _ = self.msg1.send(*msg).await;
} }
OutEvent::Message2(msg) => { OutEvent::Message2(msg) => {
let _ = self.msg2.send(msg).await; let _ = self.msg2.send(msg).await;

View File

@ -33,7 +33,7 @@ pub struct Message0 {
impl Message0 { impl Message0 {
pub fn send(&mut self, alice: PeerId, msg: bob::Message0) { pub fn send(&mut self, alice: PeerId, msg: bob::Message0) {
let msg = BobToAlice::Message0(msg); let msg = BobToAlice::Message0(Box::new(msg));
let _id = self.rr.send_request(&alice, msg); let _id = self.rr.send_request(&alice, msg);
} }
@ -80,7 +80,7 @@ impl NetworkBehaviourEventProcess<RequestResponseEvent<BobToAlice, AliceToBob>>
} => { } => {
if let AliceToBob::Message0(msg) = response { if let AliceToBob::Message0(msg) = response {
debug!("Received Message0"); debug!("Received Message0");
self.events.push_back(OutEvent::Msg(msg)); self.events.push_back(OutEvent::Msg(*msg));
} }
} }
RequestResponseEvent::InboundFailure { error, .. } => { RequestResponseEvent::InboundFailure { error, .. } => {

View File

@ -80,7 +80,7 @@ impl NetworkBehaviourEventProcess<RequestResponseEvent<BobToAlice, AliceToBob>>
} => { } => {
if let AliceToBob::Message1(msg) = response { if let AliceToBob::Message1(msg) = response {
debug!("Received Message1"); debug!("Received Message1");
self.events.push_back(OutEvent::Msg(msg)); self.events.push_back(OutEvent::Msg(*msg));
} }
} }
RequestResponseEvent::InboundFailure { error, .. } => { RequestResponseEvent::InboundFailure { error, .. } => {

View File

@ -22,12 +22,11 @@ const BUF_SIZE: usize = 1024 * 1024;
/// Messages Bob sends to Alice. /// Messages Bob sends to Alice.
#[derive(Clone, Debug, Serialize, Deserialize)] #[derive(Clone, Debug, Serialize, Deserialize)]
#[allow(clippy::large_enum_variant)]
pub enum BobToAlice { pub enum BobToAlice {
#[serde(with = "::bitcoin::util::amount::serde::as_sat")] #[serde(with = "::bitcoin::util::amount::serde::as_sat")]
AmountsFromBtc(::bitcoin::Amount), AmountsFromBtc(::bitcoin::Amount),
AmountsFromXmr(monero::Amount), AmountsFromXmr(monero::Amount),
Message0(bob::Message0), Message0(Box<bob::Message0>),
Message1(bob::Message1), Message1(bob::Message1),
Message2(bob::Message2), Message2(bob::Message2),
Message3(bob::Message3), Message3(bob::Message3),
@ -35,11 +34,10 @@ pub enum BobToAlice {
/// Messages Alice sends to Bob. /// Messages Alice sends to Bob.
#[derive(Clone, Debug, Serialize, Deserialize)] #[derive(Clone, Debug, Serialize, Deserialize)]
#[allow(clippy::large_enum_variant)]
pub enum AliceToBob { pub enum AliceToBob {
Amounts(SwapAmounts), Amounts(SwapAmounts),
Message0(alice::Message0), Message0(Box<alice::Message0>),
Message1(alice::Message1), Message1(Box<alice::Message1>),
Message2(alice::Message2), Message2(alice::Message2),
Message3, // empty response Message3, // empty response
} }

View File

@ -3,13 +3,14 @@ use serde::{Deserialize, Serialize};
use std::fmt::Display; use std::fmt::Display;
use xmr_btc::{alice, bitcoin::EncryptedSignature, bob, monero, serde::monero_private_key}; use xmr_btc::{alice, bitcoin::EncryptedSignature, bob, monero, serde::monero_private_key};
#[allow(clippy::large_enum_variant)]
#[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)]
pub enum Swap { pub enum Swap {
Alice(Alice), Alice(Alice),
Bob(Bob), Bob(Bob),
} }
// Large enum variant is fine because this is only used for storage
// and is dropped once written in DB.
#[allow(clippy::large_enum_variant)] #[allow(clippy::large_enum_variant)]
#[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)]
pub enum Alice { pub enum Alice {
@ -21,8 +22,8 @@ pub enum Alice {
BtcLocked(alice::State3), BtcLocked(alice::State3),
XmrLocked(alice::State3), XmrLocked(alice::State3),
EncSigLearned { EncSigLearned {
state: alice::State3,
encrypted_signature: EncryptedSignature, encrypted_signature: EncryptedSignature,
state3: alice::State3,
}, },
CancelTimelockExpired(alice::State3), CancelTimelockExpired(alice::State3),
BtcCancelled(alice::State3), BtcCancelled(alice::State3),

View File

@ -375,11 +375,10 @@ pub async fn next_state<
state6.redeem_btc(bitcoin_wallet).await?; state6.redeem_btc(bitcoin_wallet).await?;
Ok(state6.into()) Ok(state6.into())
} }
State::State6(state6) => Ok(state6.into()), State::State6(state6) => Ok((*state6).into()),
} }
} }
#[allow(clippy::large_enum_variant)]
#[derive(Debug, Deserialize, Serialize)] #[derive(Debug, Deserialize, Serialize)]
pub enum State { pub enum State {
State0(State0), State0(State0),
@ -388,7 +387,7 @@ pub enum State {
State3(State3), State3(State3),
State4(State4), State4(State4),
State5(State5), State5(State5),
State6(State6), State6(Box<State6>),
} }
impl_try_from_parent_enum!(State0, State); impl_try_from_parent_enum!(State0, State);
@ -397,7 +396,7 @@ impl_try_from_parent_enum!(State2, State);
impl_try_from_parent_enum!(State3, State); impl_try_from_parent_enum!(State3, State);
impl_try_from_parent_enum!(State4, State); impl_try_from_parent_enum!(State4, State);
impl_try_from_parent_enum!(State5, State); impl_try_from_parent_enum!(State5, State);
impl_try_from_parent_enum!(State6, State); impl_try_from_parent_enum_for_boxed!(State6, State);
impl_from_child_enum!(State0, State); impl_from_child_enum!(State0, State);
impl_from_child_enum!(State1, State); impl_from_child_enum!(State1, State);
@ -405,7 +404,7 @@ impl_from_child_enum!(State2, State);
impl_from_child_enum!(State3, State); impl_from_child_enum!(State3, State);
impl_from_child_enum!(State4, State); impl_from_child_enum!(State4, State);
impl_from_child_enum!(State5, State); impl_from_child_enum!(State5, State);
impl_from_child_enum!(State6, State); impl_from_child_enum_for_boxed!(State6, State);
impl State { impl State {
pub fn new<R: RngCore + CryptoRng>( pub fn new<R: RngCore + CryptoRng>(

View File

@ -50,6 +50,8 @@ impl TxLock {
} }
pub fn as_outpoint(&self) -> OutPoint { pub fn as_outpoint(&self) -> OutPoint {
// This is fine because a transaction that has that many outputs is not
// realistic
#[allow(clippy::cast_possible_truncation)] #[allow(clippy::cast_possible_truncation)]
OutPoint::new(self.inner.txid(), self.lock_output_vout() as u32) OutPoint::new(self.inner.txid(), self.lock_output_vout() as u32)
} }

View File

@ -43,7 +43,6 @@ use crate::{
use ::bitcoin::{Transaction, Txid}; use ::bitcoin::{Transaction, Txid};
pub use message::{Message, Message0, Message1, Message2, Message3}; pub use message::{Message, Message0, Message1, Message2, Message3};
#[allow(clippy::large_enum_variant)]
#[derive(Debug)] #[derive(Debug)]
pub enum Action { pub enum Action {
LockBtc(bitcoin::TxLock), LockBtc(bitcoin::TxLock),

View File

@ -41,6 +41,24 @@ mod utils {
}; };
} }
macro_rules! impl_try_from_parent_enum_for_boxed {
($type:ident, $parent:ident) => {
impl TryFrom<$parent> for $type {
type Error = anyhow::Error;
fn try_from(from: $parent) -> Result<Self> {
if let $parent::$type(inner) = from {
Ok(*inner)
} else {
Err(anyhow::anyhow!(
"Failed to convert parent state to child state"
))
}
}
}
};
}
macro_rules! impl_from_child_enum { macro_rules! impl_from_child_enum {
($type:ident, $parent:ident) => { ($type:ident, $parent:ident) => {
impl From<$type> for $parent { impl From<$type> for $parent {
@ -50,6 +68,16 @@ mod utils {
} }
}; };
} }
macro_rules! impl_from_child_enum_for_boxed {
($type:ident, $parent:ident) => {
impl From<$type> for $parent {
fn from(from: $type) -> Self {
$parent::$type(Box::new(from))
}
}
};
}
} }
pub mod alice; pub mod alice;