From: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Tue, 17 Apr 2018 02:03:35 +0000 (-0400) Subject: Merge pull request #20 from TheBlueMatt/master X-Git-Tag: v0.0.12~412 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=4fbe0c90b8d535e407bd8a3021a80ab0bf0dc06e;hp=69eb59b24d502f3df6f4dd3990aad99cc727385c;p=rust-lightning Merge pull request #20 from TheBlueMatt/master Rewrite parts of channel, cleanup a bunch of stuff --- diff --git a/Cargo.toml b/Cargo.toml index 8e1946b1e..f097d7fbc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,6 +8,7 @@ description = """ A Bitcoin Lightning implementation in Rust. Still super-early code-dump quality and is missing large chunks. See README in git repo for suggested projects if you want to contribute. Don't have to bother telling you not to use this for anything serious, because you'd have to finish building it to even try. """ +build = "build.rs" [features] # Supports tracking channels with a non-bitcoin chain hashes. Currently enables all kinds of fun DoS attacks. @@ -15,7 +16,14 @@ non_bitcoin_chain_hash_routing = [] fuzztarget = ["secp256k1/fuzztarget", "bitcoin/fuzztarget"] [dependencies] -bitcoin = { git = "https://github.com/rust-bitcoin/rust-bitcoin" } +bitcoin = "0.13" rust-crypto = "0.2" rand = "0.4" secp256k1 = "0.9" + +[build-dependencies] +gcc = "0.3" + +[dev-dependencies.bitcoin] +version = "0.13" +features = ["bitcoinconsensus"] diff --git a/build.rs b/build.rs new file mode 100644 index 000000000..7dd340397 --- /dev/null +++ b/build.rs @@ -0,0 +1,10 @@ +extern crate gcc; + +fn main() { + #[cfg(not(any(target_arch = "x86", target_arch = "x86_64", target_arch = "arm")))] + { + let mut cfg = gcc::Build::new(); + cfg.file("src/util/rust_crypto_nonstd_arch.c"); + cfg.compile("lib_rust_crypto_nonstd_arch.a"); + } +} diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 39f805d44..9b718badf 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -3,6 +3,10 @@ name = "lightning-fuzz" version = "0.0.1" authors = ["Automatically generated"] publish = false +# Because the function is unused it gets dropped before we link lightning, so +# we have to duplicate build.rs here. Note that this is only required for +# fuzztarget mode. +build = "../build.rs" [package.metadata] cargo-fuzz = true @@ -13,12 +17,15 @@ honggfuzz_fuzz = ["honggfuzz"] [dependencies] lightning = { path = "..", features = ["fuzztarget"] } -bitcoin = { git = "https://github.com/rust-bitcoin/rust-bitcoin", features = ["fuzztarget"] } +bitcoin = { version = "0.13", features = ["fuzztarget"] } secp256k1 = { version = "0.9", features = ["fuzztarget"] } rust-crypto = "0.2" honggfuzz = { version = "0.5", optional = true } afl = { version = "0.3", optional = true } +[build-dependencies] +gcc = "0.3" + # Prevent this from interfering with workspaces [workspace] members = ["."] diff --git a/fuzz/fuzz_targets/channel_target.rs b/fuzz/fuzz_targets/channel_target.rs index b64399610..20fd4b435 100644 --- a/fuzz/fuzz_targets/channel_target.rs +++ b/fuzz/fuzz_targets/channel_target.rs @@ -8,7 +8,7 @@ use bitcoin::util::hash::Sha256dHash; use bitcoin::network::serialize::{serialize, BitcoinHash}; use lightning::ln::channel::Channel; -use lightning::ln::channelmanager::PendingForwardHTLCInfo; +use lightning::ln::channelmanager::{HTLCFailReason, PendingForwardHTLCInfo}; use lightning::ln::msgs; use lightning::ln::msgs::MsgDecodable; use lightning::chain::chaininterface::{FeeEstimator, ConfirmationTarget}; @@ -241,11 +241,11 @@ pub fn do_test(data: &[u8]) { }, 4 => { let update_fail_htlc = decode_msg_with_len16!(msgs::UpdateFailHTLC, 32 + 8, 1); - return_err!(channel.update_fail_htlc(&update_fail_htlc)); + return_err!(channel.update_fail_htlc(&update_fail_htlc, HTLCFailReason::dummy())); }, 5 => { let update_fail_malformed_htlc = decode_msg!(msgs::UpdateFailMalformedHTLC, 32+8+32+2); - return_err!(channel.update_fail_malformed_htlc(&update_fail_malformed_htlc)); + return_err!(channel.update_fail_malformed_htlc(&update_fail_malformed_htlc, HTLCFailReason::dummy())); }, 6 => { let commitment_signed = decode_msg_with_len16!(msgs::CommitmentSigned, 32+64, 64); diff --git a/fuzz/src/util/rust_crypto_nonstd_arch.c b/fuzz/src/util/rust_crypto_nonstd_arch.c new file mode 120000 index 000000000..321d648a2 --- /dev/null +++ b/fuzz/src/util/rust_crypto_nonstd_arch.c @@ -0,0 +1 @@ +../../../src/util/rust_crypto_nonstd_arch.c \ No newline at end of file diff --git a/src/ln/chan_utils.rs b/src/ln/chan_utils.rs index dfe17c591..81fa29f55 100644 --- a/src/ln/chan_utils.rs +++ b/src/ln/chan_utils.rs @@ -157,7 +157,7 @@ pub struct HTLCOutputInCommitment { } #[inline] -pub fn get_htlc_redeemscript_with_explicit_keys(htlc: &HTLCOutputInCommitment, a_htlc_key: &PublicKey, b_htlc_key: &PublicKey, revocation_key: &PublicKey, offered: bool) -> Script { +pub fn get_htlc_redeemscript_with_explicit_keys(htlc: &HTLCOutputInCommitment, a_htlc_key: &PublicKey, b_htlc_key: &PublicKey, revocation_key: &PublicKey) -> Script { let payment_hash160 = { let mut ripemd = Ripemd160::new(); ripemd.input(&htlc.payment_hash); @@ -165,7 +165,7 @@ pub fn get_htlc_redeemscript_with_explicit_keys(htlc: &HTLCOutputInCommitment, a ripemd.result(&mut res); res }; - if offered { + if htlc.offered { Builder::new().push_opcode(opcodes::All::OP_DUP) .push_opcode(opcodes::All::OP_HASH160) .push_slice(&Hash160::from_data(&revocation_key.serialize())[..]) @@ -231,5 +231,5 @@ pub fn get_htlc_redeemscript_with_explicit_keys(htlc: &HTLCOutputInCommitment, a /// commitment secret. 'htlc' does *not* need to have its previous_output_index filled. #[inline] pub fn get_htlc_redeemscript(htlc: &HTLCOutputInCommitment, keys: &TxCreationKeys) -> Script { - get_htlc_redeemscript_with_explicit_keys(htlc, &keys.a_htlc_key, &keys.b_htlc_key, &keys.revocation_key, htlc.offered) + get_htlc_redeemscript_with_explicit_keys(htlc, &keys.a_htlc_key, &keys.b_htlc_key, &keys.revocation_key) } diff --git a/src/ln/channel.rs b/src/ln/channel.rs index b8a5bb66c..067b2bc53 100644 --- a/src/ln/channel.rs +++ b/src/ln/channel.rs @@ -17,7 +17,7 @@ use crypto::hkdf::{hkdf_extract,hkdf_expand}; use ln::msgs; use ln::msgs::{HandleError, MsgEncodable}; use ln::channelmonitor::ChannelMonitor; -use ln::channelmanager::PendingForwardHTLCInfo; +use ln::channelmanager::{PendingForwardHTLCInfo, HTLCFailReason}; use ln::chan_utils::{TxCreationKeys,HTLCOutputInCommitment}; use ln::chan_utils; use chain::chaininterface::{FeeEstimator,ConfirmationTarget}; @@ -25,7 +25,7 @@ use util::{transaction_utils,rng}; use util::sha2::Sha256; use std::default::Default; -use std::cmp; +use std::{cmp,mem}; use std::time::Instant; pub struct ChannelKeys { @@ -43,7 +43,7 @@ impl ChannelKeys { pub fn new_from_seed(seed: &[u8; 32]) -> Result { let mut prk = [0; 32]; hkdf_extract(Sha256::new(), b"rust-lightning key gen salt", seed, &mut prk); - let secp_ctx = Secp256k1::new(); + let secp_ctx = Secp256k1::without_caps(); let mut okm = [0; 32]; hkdf_expand(Sha256::new(), &prk, b"rust-lightning funding key info", &mut okm); @@ -84,19 +84,73 @@ impl ChannelKeys { #[derive(PartialEq)] enum HTLCState { + /// Added by remote, to be included in next local commitment tx. RemoteAnnounced, + /// Included in a received commitment_signed message (implying we've revoke_and_ack'ed it), but + /// the remote side hasn't yet revoked their previous state, which we need them to do before we + /// accept this HTLC. Implies AwaitingRemoteRevoke. + /// We also have not yet included this HTLC in a commitment_signed message, and are waiting on + /// a remote revoke_and_ack on a previous state before we can do so. + AwaitingRemoteRevokeToAnnounce, + /// Included in a received commitment_signed message (implying we've revoke_and_ack'ed it), but + /// the remote side hasn't yet revoked their previous state, which we need them to do before we + /// accept this HTLC. Implies AwaitingRemoteRevoke. + /// We have included this HTLC in our latest commitment_signed and are now just waiting on a + /// revoke_and_ack. + AwaitingAnnouncedRemoteRevoke, + /// Added by us and included in a commitment_signed (if we were AwaitingRemoteRevoke when we + /// created it we would have put it in the holding cell instead). When they next revoke_and_ack + /// we will promote to Committed (note that they may not accept it until the next time we + /// revoke, but we dont really care about that: + /// * they've revoked, so worst case we can announce an old state and get our (option on) + /// money back (though we wont), and, + /// * we'll send them a revoke when they send a commitment_signed, and since only they're + /// allowed to remove it, the "can only be removed once committed on both sides" requirement + /// doesn't matter to us and its up to them to enforce it, worst-case they jump ahead but + /// we'll never get out of sync). LocalAnnounced, Committed, + /// Remote removed this (outbound) HTLC. We're waiting on their commitment_signed to finalize + /// the change (though they'll need to revoke before we fail the payment). + RemoteRemoved, + /// Remote removed this and sent a commitment_signed (implying we've revoke_and_ack'ed it), but + /// the remote side hasn't yet revoked their previous state, which we need them to do before we + /// can do any backwards failing. Implies AwaitingRemoteRevoke. + /// We also have not yet removed this HTLC in a commitment_signed message, and are waiting on a + /// remote revoke_and_ack on a previous state before we can do so. + AwaitingRemoteRevokeToRemove, + /// Remote removed this and sent a commitment_signed (implying we've revoke_and_ack'ed it), but + /// the remote side hasn't yet revoked their previous state, which we need them to do before we + /// can do any backwards failing. Implies AwaitingRemoteRevoke. + /// We have removed this HTLC in our latest commitment_signed and are now just waiting on a + /// revoke_and_ack to drop completely. + AwaitingRemovedRemoteRevoke, + /// Removed by us and a new commitment_signed was sent (if we were AwaitingRemoteRevoke when we + /// created it we would have put it in the holding cell instead). When they next revoke_and_ack + /// we'll promote to LocalRemovedAwaitingCommitment if we fulfilled, otherwise we'll drop at + /// that point. + /// Note that we have to keep an eye on the HTLC until we've received a broadcastable + /// commitment transaction without it as otherwise we'll have to force-close the channel to + /// claim it before the timeout (obviously doesn't apply to revoked HTLCs that we can't claim + /// anyway). + LocalRemoved, + /// Removed by us, sent a new commitment_signed and got a revoke_and_ack. Just waiting on an + /// updated local commitment transaction. + LocalRemovedAwaitingCommitment, } -struct HTLCOutput { +struct HTLCOutput { //TODO: Refactor into Outbound/InboundHTLCOutput (will save memory and fewer panics) outbound: bool, // ie to an HTLC-Timeout transaction htlc_id: u64, amount_msat: u64, cltv_expiry: u32, payment_hash: [u8; 32], state: HTLCState, - // state == RemoteAnnounced implies pending_forward_state, otherwise it must be None + /// If we're in a Remote* removed state, set if they failed, otherwise None + fail_reason: Option, + /// If we're in LocalRemoved*, set to true if we fulfilled the HTLC, and can claim money + local_removed_fulfilled: bool, + /// state pre-committed Remote* implies pending_forward_state, otherwise it must be None pending_forward_state: Option, } @@ -113,13 +167,23 @@ impl HTLCOutput { } /// See AwaitingRemoteRevoke ChannelState for more info -struct HTLCOutputAwaitingACK { - // always outbound - amount_msat: u64, - cltv_expiry: u32, - payment_hash: [u8; 32], - onion_routing_packet: msgs::OnionPacket, - time_created: Instant, //TODO: Some kind of timeout thing-a-majig +enum HTLCUpdateAwaitingACK { + AddHTLC { + // always outbound + amount_msat: u64, + cltv_expiry: u32, + payment_hash: [u8; 32], + onion_routing_packet: msgs::OnionPacket, + time_created: Instant, //TODO: Some kind of timeout thing-a-majig + }, + ClaimHTLC { + payment_preimage: [u8; 32], + payment_hash: [u8; 32], // Only here for effecient duplicate detection + }, + FailHTLC { + payment_hash: [u8; 32], + err_packet: msgs::OnionErrorPacket, + }, } enum ChannelState { @@ -184,7 +248,7 @@ pub struct Channel { cur_remote_commitment_transaction_number: u64, value_to_self_msat: u64, // Excluding all pending_htlcs, excluding fees pending_htlcs: Vec, - holding_cell_htlcs: Vec, + holding_cell_htlc_updates: Vec, next_local_htlc_id: u64, next_remote_htlc_id: u64, channel_update_count: u32, @@ -221,6 +285,8 @@ pub struct Channel { their_delayed_payment_basepoint: PublicKey, their_htlc_basepoint: PublicKey, their_cur_commitment_point: PublicKey, + + their_prev_commitment_point: Option, their_node_id: PublicKey, their_shutdown_scriptpubkey: Option