From 1276cc72de06a24ab7ce432f4662f2d954f3c1d0 Mon Sep 17 00:00:00 2001 From: Ryan Loomba Date: Tue, 6 Oct 2020 16:47:23 -0700 Subject: [PATCH 1/3] fix all clippy::redundant_field_names warnings --- lightning/src/chain/channelmonitor.rs | 4 +-- lightning/src/ln/channel.rs | 32 +++++++++++----------- lightning/src/ln/channelmanager.rs | 14 +++++----- lightning/src/ln/onion_utils.rs | 4 +-- lightning/src/ln/peer_channel_encryptor.rs | 16 +++++------ lightning/src/ln/peer_handler.rs | 4 +-- lightning/src/util/chacha20poly1305rfc.rs | 4 +-- 7 files changed, 39 insertions(+), 39 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 0a9cf6997c9..31e4565ecab 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -981,7 +981,7 @@ impl ChannelMonitor { counterparty_tx_cache, funding_redeemscript, - channel_value_satoshis: channel_value_satoshis, + channel_value_satoshis, their_cur_revocation_points: None, on_holder_tx_csv, @@ -1129,7 +1129,7 @@ impl ChannelMonitor { delayed_payment_key: commitment_tx.keys.broadcaster_delayed_payment_key, per_commitment_point: commitment_tx.keys.per_commitment_point, feerate_per_kw: commitment_tx.feerate_per_kw, - htlc_outputs: htlc_outputs, + htlc_outputs, }; self.onchain_tx_handler.provide_latest_holder_tx(commitment_tx); self.current_holder_commitment_number = 0xffff_ffff_ffff - ((((sequence & 0xffffff) << 3*8) | (locktime as u64 & 0xffffff)) ^ self.commitment_transaction_number_obscure_factor); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 934f155db2c..8782bb3ac40 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -487,14 +487,14 @@ impl Channel { let feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal); Ok(Channel { - user_id: user_id, + user_id, config: config.channel_options.clone(), channel_id: keys_provider.get_secure_random_bytes(), channel_state: ChannelState::OurInitSent as u32, channel_outbound: true, secp_ctx: Secp256k1::new(), - channel_value_satoshis: channel_value_satoshis, + channel_value_satoshis, latest_monitor_update_id: 0, @@ -715,7 +715,7 @@ impl Channel { } else { None }; let chan = Channel { - user_id: user_id, + user_id, config: local_config, channel_id: msg.temporary_channel_id, @@ -766,7 +766,7 @@ impl Channel { feerate_per_kw: msg.feerate_per_kw, channel_value_satoshis: msg.funding_satoshis, counterparty_dust_limit_satoshis: msg.dust_limit_satoshis, - holder_dust_limit_satoshis: holder_dust_limit_satoshis, + holder_dust_limit_satoshis, counterparty_max_htlc_value_in_flight_msat: cmp::min(msg.max_htlc_value_in_flight_msat, msg.funding_satoshis * 1000), counterparty_selected_channel_reserve_satoshis: msg.channel_reserve_satoshis, counterparty_htlc_minimum_msat: msg.htlc_minimum_msat, @@ -1579,7 +1579,7 @@ impl Channel { Ok((msgs::FundingSigned { channel_id: self.channel_id, - signature: signature + signature }, channel_monitor)) } @@ -2135,8 +2135,8 @@ impl Channel { Ok((msgs::RevokeAndACK { channel_id: self.channel_id, - per_commitment_secret: per_commitment_secret, - next_per_commitment_point: next_per_commitment_point, + per_commitment_secret, + next_per_commitment_point, }, commitment_signed, closing_signed, monitor_update)) } @@ -2240,7 +2240,7 @@ impl Channel { update_fulfill_htlcs, update_fail_htlcs, update_fail_malformed_htlcs: Vec::new(), - update_fee: update_fee, + update_fee, commitment_signed, }, monitor_update)), htlcs_to_fail)) } else { @@ -2497,7 +2497,7 @@ impl Channel { Some(msgs::UpdateFee { channel_id: self.channel_id, - feerate_per_kw: feerate_per_kw, + feerate_per_kw, }) } @@ -2623,7 +2623,7 @@ impl Channel { let next_per_commitment_point = self.holder_keys.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx); Some(msgs::FundingLocked { channel_id: self.channel_id(), - next_per_commitment_point: next_per_commitment_point, + next_per_commitment_point, }) } else { None }; @@ -2798,7 +2798,7 @@ impl Channel { let next_per_commitment_point = self.holder_keys.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx); return Ok((Some(msgs::FundingLocked { channel_id: self.channel_id(), - next_per_commitment_point: next_per_commitment_point, + next_per_commitment_point, }), None, None, None, RAACommitmentOrder::CommitmentFirst, shutdown_msg)); } @@ -2828,7 +2828,7 @@ impl Channel { let next_per_commitment_point = self.holder_keys.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx); Some(msgs::FundingLocked { channel_id: self.channel_id(), - next_per_commitment_point: next_per_commitment_point, + next_per_commitment_point, }) } else { None }; @@ -3443,7 +3443,7 @@ impl Channel { let next_per_commitment_point = self.holder_keys.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx); return Ok((Some(msgs::FundingLocked { channel_id: self.channel_id, - next_per_commitment_point: next_per_commitment_point, + next_per_commitment_point, }), timed_out_htlcs)); } else { self.monitor_pending_funding_locked = true; @@ -3492,7 +3492,7 @@ impl Channel { let keys = self.holder_keys.pubkeys(); msgs::OpenChannel { - chain_hash: chain_hash, + chain_hash, temporary_channel_id: self.channel_id, funding_satoshis: self.channel_value_satoshis, push_msat: self.channel_value_satoshis * 1000 - self.value_to_self_msat, @@ -3597,7 +3597,7 @@ impl Channel { temporary_channel_id, funding_txid: funding_txo.txid, funding_output_index: funding_txo.index, - signature: signature + signature }) } @@ -3624,7 +3624,7 @@ impl Channel { let msg = msgs::UnsignedChannelAnnouncement { features: ChannelFeatures::known(), - chain_hash: chain_hash, + chain_hash, short_channel_id: self.get_short_channel_id().unwrap(), node_id_1: if were_node_one { node_id } else { self.get_counterparty_node_id() }, node_id_2: if were_node_one { self.get_counterparty_node_id() } else { node_id }, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 6b2f50e9704..a1951d6bb62 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1126,7 +1126,7 @@ impl PendingHTLCStatus::Forward(PendingHTLCInfo { routing: PendingHTLCRouting::Forward { onion_packet: outgoing_packet, - short_channel_id: short_channel_id, + short_channel_id, }, payment_hash: msg.payment_hash.clone(), incoming_shared_secret: shared_secret, @@ -1221,7 +1221,7 @@ impl let unsigned = msgs::UnsignedChannelUpdate { chain_hash: self.genesis_hash, - short_channel_id: short_channel_id, + short_channel_id, timestamp: chan.get_update_time_counter(), flags: (!were_node_one) as u8 | ((!chan.is_live() as u8) << 1), cltv_expiry_delta: CLTV_EXPIRY_DELTA, @@ -1447,7 +1447,7 @@ impl let mut channel_state = self.channel_state.lock().unwrap(); channel_state.pending_msg_events.push(events::MessageSendEvent::SendFundingCreated { node_id: chan.get_counterparty_node_id(), - msg: msg, + msg, }); match channel_state.by_id.entry(chan.channel_id()) { hash_map::Entry::Occupied(_) => { @@ -1745,14 +1745,14 @@ impl } } else if total_value == data.total_msat { new_events.push(events::Event::PaymentReceived { - payment_hash: payment_hash, + payment_hash, payment_secret: Some(data.payment_secret), amt: total_value, }); } } else { new_events.push(events::Event::PaymentReceived { - payment_hash: payment_hash, + payment_hash, payment_secret: None, amt: amt_to_forward, }); @@ -2304,7 +2304,7 @@ impl pending_events.push(events::Event::FundingGenerationReady { temporary_channel_id: msg.temporary_channel_id, channel_value_satoshis: value, - output_script: output_script, + output_script, user_channel_id: user_id, }); Ok(()) @@ -2384,7 +2384,7 @@ impl }; let mut pending_events = self.pending_events.lock().unwrap(); pending_events.push(events::Event::FundingBroadcastSafe { - funding_txo: funding_txo, + funding_txo, user_channel_id: user_id, }); Ok(()) diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index c6bd92f5bc1..1c45da14a38 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -306,8 +306,8 @@ pub(super) fn build_failure_packet(shared_secret: &[u8], failure_type: u16, fail }; let mut packet = msgs::DecodedOnionErrorPacket { hmac: [0; 32], - failuremsg: failuremsg, - pad: pad, + failuremsg, + pad, }; let mut hmac = HmacEngine::::new(&um); diff --git a/lightning/src/ln/peer_channel_encryptor.rs b/lightning/src/ln/peer_channel_encryptor.rs index 8310edb64e2..d38d5ae6a16 100644 --- a/lightning/src/ln/peer_channel_encryptor.rs +++ b/lightning/src/ln/peer_channel_encryptor.rs @@ -95,14 +95,14 @@ impl PeerChannelEncryptor { PeerChannelEncryptor { their_node_id: Some(their_node_id), - secp_ctx: secp_ctx, + secp_ctx, noise_state: NoiseState::InProgress { state: NoiseStep::PreActOne, directional_state: DirectionalNoiseState::Outbound { ie: ephemeral_key, }, bidirectional_state: BidirectionalNoiseState { - h: h, + h, ck: NOISE_CK, }, } @@ -120,7 +120,7 @@ impl PeerChannelEncryptor { PeerChannelEncryptor { their_node_id: None, - secp_ctx: secp_ctx, + secp_ctx, noise_state: NoiseState::InProgress { state: NoiseStep::PreActOne, directional_state: DirectionalNoiseState::Inbound { @@ -129,7 +129,7 @@ impl PeerChannelEncryptor { temp_k2: None, }, bidirectional_state: BidirectionalNoiseState { - h: h, + h, ck: NOISE_CK, }, } @@ -321,10 +321,10 @@ impl PeerChannelEncryptor { let (sk, rk) = final_hkdf; self.noise_state = NoiseState::Finished { - sk: sk, + sk, sn: 0, sck: ck.clone(), - rk: rk, + rk, rn: 0, rck: ck, }; @@ -374,10 +374,10 @@ impl PeerChannelEncryptor { let (rk, sk) = final_hkdf; self.noise_state = NoiseState::Finished { - sk: sk, + sk, sn: 0, sck: ck.clone(), - rk: rk, + rk, rn: 0, rck: ck, }; diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index edb2084c716..6e997e53b15 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -323,7 +323,7 @@ impl PeerManager PeerManager Date: Tue, 6 Oct 2020 16:49:06 -0700 Subject: [PATCH 2/3] add clippy to travis integration --- .travis.yml | 4 ++++ CONTRIBUTING.md | 12 ++++++++++++ lightning/src/lib.rs | 1 - lightning/src/ln/channelmanager.rs | 1 + 4 files changed, 17 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 1e136798133..8c1f2fd0f0d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,6 +22,10 @@ script: - if [ "$BUILD_NET_TOKIO" == "1" ]; then RUSTFLAGS="-C link-dead-code" cargo build --verbose; fi - if [ "$BUILD_NET_TOKIO" != "1" ]; then RUSTFLAGS="-C link-dead-code" cargo build --verbose -p lightning; fi - rm -f target/debug/lightning-* # Make sure we drop old test binaries + # Run clippy on Rust 1.39.0 + - if [ "$(rustup show | grep default | grep 1.39.0)" != "" ]; then + rustup component add clippy && + cargo clippy -- -Aclippy::erasing_op -Aclippy::never_loop -Aclippy::if_same_then_else; fi # Test the appropriate workspace(s) - if [ "$BUILD_NET_TOKIO" == "1" ]; then RUSTFLAGS="-C link-dead-code" cargo test --verbose; fi - if [ "$BUILD_NET_TOKIO" != "1" ]; then RUSTFLAGS="-C link-dead-code" cargo test --verbose -p lightning; fi diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b270492bf44..241c6420856 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -77,6 +77,18 @@ Coding Conventions Use tabs. If you want to align lines, use spaces. Any desired alignment should display fine at any tab-length display setting. +Our CI enforces [clippy's](https://github.com/rust-lang/rust-clippy) default linting +[settings](https://rust-lang.github.io/rust-clippy/rust-1.39.0/index.html). +This includes all lint groups except for nursery, pedantic, and cargo in addition to allowing the following lints: +`erasing_op`, `never_loop`, `if_same_then_else`. + +If you use rustup, feel free to lint locally, otherwise you can just push to CI for automated linting. + +```bash +rustup component add clippy +cargo clippy +``` + Security -------- diff --git a/lightning/src/lib.rs b/lightning/src/lib.rs index 22483339d24..dbd89a2ccdf 100644 --- a/lightning/src/lib.rs +++ b/lightning/src/lib.rs @@ -36,4 +36,3 @@ pub mod util; pub mod chain; pub mod ln; pub mod routing; - diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index a1951d6bb62..379fe193009 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -16,6 +16,7 @@ //! It does not manage routing logic (see routing::router::get_route for that) nor does it manage constructing //! on-chain transactions (it only monitors the chain to watch for any force-closes that might //! imply it needs to fail HTLCs/payments/channels it manages). +//! use bitcoin::blockdata::block::BlockHeader; use bitcoin::blockdata::constants::genesis_block; From 262172cddba7f2af9cfd3f5035564ac04e8af3a5 Mon Sep 17 00:00:00 2001 From: Ryan Loomba Date: Thu, 8 Oct 2020 10:47:49 -0700 Subject: [PATCH 3/3] add linting to Github CI --- .github/workflows/build.yml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 6ea1e4e1e19..065a3241a8e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -184,3 +184,23 @@ jobs: [ "$(diff lightning-c-bindings/include/lightningpp.hpp.sorted lightning-c-bindings/include/lightningpp.hpp.new.sorted)" != "" ] && exit 3 git diff --exit-code fi + + linting: + runs-on: ubuntu-latest + env: + TOOLCHAIN: 1.39.0 + steps: + - name: Checkout source code + uses: actions/checkout@v2 + - name: Install Rust ${{ env.TOOLCHAIN }} toolchain + uses: actions-rs/toolchain@v1 + with: + toolchain: ${{ env.TOOLCHAIN }} + override: true + profile: minimal + - name: Install clippy + run: | + rustup component add clippy + - name: Run default clippy linting + run: | + cargo clippy -- -Aclippy::erasing_op -Aclippy::never_loop -Aclippy::if_same_then_else