Check P2WPKH script against expected before gen'ing an output event

This fixes a bug in 3518f1f85d where
we may generate an output event for a P2WPKH output which is not
ours if the transaction has a sequence/lock_time combination which
false-positives our remote tx detection.

Also note that the TODO is removed as this should already be
covered without issue if the client properly replays the chain on
restart.
This commit is contained in:
Matt Corallo 2018-11-20 15:09:47 -05:00
parent 90b0ed937e
commit 3af20fd507

View file

@ -889,16 +889,18 @@ impl ChannelMonitor {
if commitment_number >= self.get_min_seen_secret() { if commitment_number >= self.get_min_seen_secret() {
let secret = self.get_secret(commitment_number).unwrap(); let secret = self.get_secret(commitment_number).unwrap();
let per_commitment_key = ignore_error!(SecretKey::from_slice(&self.secp_ctx, &secret)); let per_commitment_key = ignore_error!(SecretKey::from_slice(&self.secp_ctx, &secret));
let (revocation_pubkey, b_htlc_key) = match self.key_storage { let (revocation_pubkey, b_htlc_key, local_payment_key) = match self.key_storage {
KeyStorage::PrivMode { ref revocation_base_key, ref htlc_base_key, .. } => { KeyStorage::PrivMode { ref revocation_base_key, ref htlc_base_key, ref payment_base_key, .. } => {
let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key); let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key);
(ignore_error!(chan_utils::derive_public_revocation_key(&self.secp_ctx, &per_commitment_point, &PublicKey::from_secret_key(&self.secp_ctx, &revocation_base_key))), (ignore_error!(chan_utils::derive_public_revocation_key(&self.secp_ctx, &per_commitment_point, &PublicKey::from_secret_key(&self.secp_ctx, &revocation_base_key))),
ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &per_commitment_point, &PublicKey::from_secret_key(&self.secp_ctx, &htlc_base_key)))) ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &per_commitment_point, &PublicKey::from_secret_key(&self.secp_ctx, &htlc_base_key))),
Some(ignore_error!(chan_utils::derive_private_key(&self.secp_ctx, &per_commitment_point, &payment_base_key))))
}, },
KeyStorage::SigsMode { ref revocation_base_key, ref htlc_base_key, .. } => { KeyStorage::SigsMode { ref revocation_base_key, ref htlc_base_key, .. } => {
let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key); let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key);
(ignore_error!(chan_utils::derive_public_revocation_key(&self.secp_ctx, &per_commitment_point, &revocation_base_key)), (ignore_error!(chan_utils::derive_public_revocation_key(&self.secp_ctx, &per_commitment_point, &revocation_base_key)),
ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &per_commitment_point, &htlc_base_key))) ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &per_commitment_point, &htlc_base_key)),
None)
}, },
}; };
let delayed_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &self.their_delayed_payment_base_key.unwrap())); let delayed_key = ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, &PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key), &self.their_delayed_payment_base_key.unwrap()));
@ -910,6 +912,13 @@ impl ChannelMonitor {
let revokeable_redeemscript = chan_utils::get_revokeable_redeemscript(&revocation_pubkey, self.our_to_self_delay, &delayed_key); let revokeable_redeemscript = chan_utils::get_revokeable_redeemscript(&revocation_pubkey, self.our_to_self_delay, &delayed_key);
let revokeable_p2wsh = revokeable_redeemscript.to_v0_p2wsh(); let revokeable_p2wsh = revokeable_redeemscript.to_v0_p2wsh();
let local_payment_p2wpkh = if let Some(payment_key) = local_payment_key {
// Note that the Network here is ignored as we immediately drop the address for the
// script_pubkey version.
let payment_hash160 = Hash160::from_data(&PublicKey::from_secret_key(&self.secp_ctx, &payment_key).serialize());
Some(Builder::new().push_opcode(opcodes::All::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script())
} else { None };
let mut total_value = 0; let mut total_value = 0;
let mut values = Vec::new(); let mut values = Vec::new();
let mut inputs = Vec::new(); let mut inputs = Vec::new();
@ -929,23 +938,12 @@ impl ChannelMonitor {
htlc_idxs.push(None); htlc_idxs.push(None);
values.push(outp.value); values.push(outp.value);
total_value += outp.value; total_value += outp.value;
} else if outp.script_pubkey.is_v0_p2wpkh() { } else if Some(&outp.script_pubkey) == local_payment_p2wpkh.as_ref() {
match self.key_storage { spendable_outputs.push(SpendableOutputDescriptor::DynamicOutputP2WPKH {
KeyStorage::PrivMode { ref payment_base_key, .. } => { outpoint: BitcoinOutPoint { txid: commitment_txid, vout: idx as u32 },
let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &per_commitment_key); key: local_payment_key.unwrap(),
if let Ok(local_key) = chan_utils::derive_private_key(&self.secp_ctx, &per_commitment_point, &payment_base_key) { output: outp.clone(),
spendable_outputs.push(SpendableOutputDescriptor::DynamicOutputP2WPKH { });
outpoint: BitcoinOutPoint { txid: commitment_txid, vout: idx as u32 },
key: local_key,
output: outp.clone(),
});
}
}
KeyStorage::SigsMode { .. } => {
//TODO: we need to ensure an offline client will generate the event when it
// cames back online after only the watchtower saw the transaction
}
}
} }
} }
@ -1083,7 +1081,6 @@ impl ChannelMonitor {
Some(their_htlc_base_key) => ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, revocation_point, &their_htlc_base_key)), Some(their_htlc_base_key) => ignore_error!(chan_utils::derive_public_key(&self.secp_ctx, revocation_point, &their_htlc_base_key)),
}; };
for (idx, outp) in tx.output.iter().enumerate() { for (idx, outp) in tx.output.iter().enumerate() {
if outp.script_pubkey.is_v0_p2wpkh() { if outp.script_pubkey.is_v0_p2wpkh() {
match self.key_storage { match self.key_storage {
@ -1095,11 +1092,8 @@ impl ChannelMonitor {
output: outp.clone(), output: outp.clone(),
}); });
} }
} },
KeyStorage::SigsMode { .. } => { KeyStorage::SigsMode { .. } => {}
//TODO: we need to ensure an offline client will generate the event when it
// cames back online after only the watchtower saw the transaction
}
} }
break; // Only to_remote ouput is claimable break; // Only to_remote ouput is claimable
} }