Merge pull request #739 from rloomba/rloomba/add-clippy-to-travis

Add clippy to Travis integration

.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

.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

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
 --------
 

lightning/src/chain/channelmonitor.rs

@@ -981,7 +981,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 
 			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<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 			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);

lightning/src/lib.rs

@@ -36,4 +36,3 @@ pub mod util;
 pub mod chain;
 pub mod ln;
 pub mod routing;
-

lightning/src/ln/channel.rs

@@ -487,14 +487,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 		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<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 		} 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<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 			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<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
 		Ok((msgs::FundingSigned {
 			channel_id: self.channel_id,
-			signature: signature
+			signature
 		}, channel_monitor))
 	}
 
@@ -2135,8 +2135,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
 		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<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 				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<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
 		Some(msgs::UpdateFee {
 			channel_id: self.channel_id,
-			feerate_per_kw: feerate_per_kw,
+			feerate_per_kw,
 		})
 	}
 
@@ -2623,7 +2623,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 			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<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 			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<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 			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<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 							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<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 		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<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 			temporary_channel_id,
 			funding_txid: funding_txo.txid,
 			funding_output_index: funding_txo.index,
-			signature: signature
+			signature
 		})
 	}
 
@@ -3624,7 +3624,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
 		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 },

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;
@@ -1126,7 +1127,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 				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 +1222,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 
 		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 +1448,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 		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 +1746,14 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 										}
 									} 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 +2305,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 		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 +2385,7 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
 		};
 		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(())

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::<Sha256>::new(&um);

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,
 		};

lightning/src/ln/peer_handler.rs

@@ -323,7 +323,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref> PeerManager<D
 			pending_outbound_buffer_first_msg_offset: 0,
 			awaiting_write_event: false,
 
-			pending_read_buffer: pending_read_buffer,
+			pending_read_buffer,
 			pending_read_buffer_pos: 0,
 			pending_read_is_header: false,
 
@@ -360,7 +360,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref> PeerManager<D
 			pending_outbound_buffer_first_msg_offset: 0,
 			awaiting_write_event: false,
 
-			pending_read_buffer: pending_read_buffer,
+			pending_read_buffer,
 			pending_read_buffer_pos: 0,
 			pending_read_is_header: false,
 

lightning/src/util/chacha20poly1305rfc.rs

@@ -51,8 +51,8 @@ mod real_chachapoly {
 			ChaCha20Poly1305RFC::pad_mac_16(&mut mac, aad.len());
 
 			ChaCha20Poly1305RFC {
-				cipher: cipher,
-				mac: mac,
+				cipher,
+				mac,
 				finished: false,
 				data_len: 0,
 				aad_len: aad.len() as u64,