From: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Sat, 10 Oct 2020 22:21:11 +0000 (-0700) Subject: Merge pull request #738 from TheBlueMatt/2020-10-opt-test X-Git-Tag: v0.0.12~12 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=df778b605a28580905cb5ca63b3ec8bbe99afc2f;hp=e808d50b9d04f9cfde5e9fd9989964be31755c5d;p=rust-lightning Merge pull request #738 from TheBlueMatt/2020-10-opt-test Fix passing -O1 to build from `cargo test` --- diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c8cdf666..065a3241 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -109,6 +109,8 @@ jobs: run: | git remote add upstream https://github.com/rust-bitcoin/rust-lightning git fetch upstream + export GIT_COMMITTER_EMAIL="rl-ci@example.com" + export GIT_COMMITTER_NAME="RL CI" git rebase upstream/main - name: For each commit, run cargo check (including in fuzz) run: ci/check-each-commit.sh upstream/main @@ -182,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 diff --git a/.travis.yml b/.travis.yml index 1e136798..8c1f2fd0 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 b270492b..241c6420 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/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 0a9cf699..31e4565e 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/lib.rs b/lightning/src/lib.rs index 22483339..dbd89a2c 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/channel.rs b/lightning/src/ln/channel.rs index 934f155d..8782bb3a 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 6b2f50e9..379fe193 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; @@ -1126,7 +1127,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 +1222,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 +1448,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 +1746,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 +2305,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 +2385,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 c6bd92f5..1c45da14 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 8310edb6..d38d5ae6 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 edb2084c..6e997e53 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -323,7 +323,7 @@ impl PeerManager PeerManager Result<(), fmt::Error> { - write!(f, "Network map\n[Channels]\n")?; + writeln!(f, "Network map\n[Channels]")?; for (key, val) in self.channels.iter() { - write!(f, " {}: {}\n", key, val)?; + writeln!(f, " {}: {}", key, val)?; } - write!(f, "[Nodes]\n")?; + writeln!(f, "[Nodes]")?; for (key, val) in self.nodes.iter() { - write!(f, " {}: {}\n", log_pubkey!(key), val)?; + writeln!(f, " {}: {}", log_pubkey!(key), val)?; } Ok(()) } diff --git a/lightning/src/util/chacha20poly1305rfc.rs b/lightning/src/util/chacha20poly1305rfc.rs index 41a218bd..3908116c 100644 --- a/lightning/src/util/chacha20poly1305rfc.rs +++ b/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, diff --git a/lightning/src/util/macro_logger.rs b/lightning/src/util/macro_logger.rs index c2c5122d..e667c4d7 100644 --- a/lightning/src/util/macro_logger.rs +++ b/lightning/src/util/macro_logger.rs @@ -80,9 +80,9 @@ pub(crate) struct DebugRoute<'a>(pub &'a Route); impl<'a> std::fmt::Display for DebugRoute<'a> { fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { for (idx, p) in self.0.paths.iter().enumerate() { - write!(f, "path {}:\n", idx)?; + writeln!(f, "path {}:", idx)?; for h in p.iter() { - write!(f, " node_id: {}, short_channel_id: {}, fee_msat: {}, cltv_expiry_delta: {}\n", log_pubkey!(h.pubkey), h.short_channel_id, h.fee_msat, h.cltv_expiry_delta)?; + writeln!(f, " node_id: {}, short_channel_id: {}, fee_msat: {}, cltv_expiry_delta: {}", log_pubkey!(h.pubkey), h.short_channel_id, h.fee_msat, h.cltv_expiry_delta)?; } } Ok(())