Split parsing and transaction management for local transactions between Chanmon/Onchain #559

ariard · 2020-03-24T00:29:41Z

ChannelMonitor is more and more scoped to be reduce to parsing only, which means any outpoint part of a channel on which we should take actions is going to be spend by OnchainTxHandler. Doing so on a high-level, let OnchainTxHandler manage dynamic fees and broadcasting frequency without intertwining with detection.

To achieve this goal, we need to cache more transactions elements in OnchainTxHandler, some of them are static (csv_delay), other per channel update (htlc pubkeys, feerate). Detection should hand only hand over a identifier to OnchainTxHandler from which a fully-fledged transaction should be generated.

Access to Local and HTLC transactions is still maintained as it's needed in for channel force-closure or manual transaction broadcasting (fallen-behind case). Post-anchor output, dynamic bumping of pre-signed transactions means we should also remove ChannelMonitor::broadcast_latest_local_commitment_txn and block asynchronously register outpoint to claim with OnchainTxHandler.

We take benefits of this refactoring to also move local/HTLC transaction signature behind external signer interface.

Replace #540

TheBlueMatt

I think we maybe got out of sync somewhat on goals - I should have been clearer on my thinking wrt LocalCommitmentTransaction. At a high level I still like this direction, I just think it needs more work on encapsulation of sub-structs within OnchainTxHandler.

lightning/src/ln/chan_utils.rs

lightning/src/ln/onchaintx.rs

ariard · 2020-03-25T23:56:36Z

Updated 5aeb582 with comments addressed.

TheBlueMatt

Just did an initial pass. I think there's still a bit to go on structure.

lightning/src/ln/chan_utils.rs

TheBlueMatt · 2020-03-26T01:29:10Z

lightning/src/ln/chan_utils.rs

@@ -530,6 +530,7 @@ impl LocalCommitmentTransaction {
 		} }
 	}

+	/// Generate a new LocalCommitmentTransaction based on a raw transaction, both parties funding pubkeys and remote signatures


This should likely not be public.

It's used in Channel?

Right, sorry, I meant pub(crate), but shouldnt be pub.

lightning/src/ln/chan_utils.rs

TheBlueMatt · 2020-03-26T01:33:52Z

lightning/src/ln/chan_utils.rs

 	pub fn without_valid_witness(&self) -> &Transaction { &self.tx }
+	/// Expose raw transaction with asserting witness is complete


If this is gonna be pub, probably just return an Option<> instead.

lightning/src/ln/chan_utils.rs

TheBlueMatt · 2020-03-26T02:01:40Z

lightning/src/ln/onchaintx.rs

@@ -760,4 +760,12 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
 		self.prev_local_commitment = self.local_commitment.take();
 		self.local_commitment = Some(tx);
 	}
+
+	pub(super) fn get_fully_signed_local_tx(&mut self, channel_value_satoshis: u64) -> Option<Transaction> {


I still don't think this is where it needs to be - this needs to "lock" the channelmonitor, after this is called we should be refusing to update local_commitment (and docs should be updated in conjunction).

Yes, as current ChannelMonitor code is structured, you may need to give back multiples time a signed local commitment transaction, but of course we should enforce signature happens once. That's what the code at this commit is doing, it's enforced in has_local_sig in add_local_sig`. Or do you further suggestion?

lightning/src/ln/onchaintx.rs

ariard · 2020-03-28T03:19:47Z

At 2dbb9a7, implement monitor-failure after LocalCommitmentTransaction signature.

lightning/src/ln/chan_utils.rs

lightning/src/util/enforcing_trait_impls.rs

TheBlueMatt · 2020-04-01T00:06:47Z

lightning/src/ln/onchaintx.rs

@@ -709,4 +778,47 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
 			self.pending_claim_requests.remove(&req);
 		}
 	}
+
+	pub(super) fn provide_latest_local_tx(&mut self, tx: LocalCommitmentTransaction) -> Result<(), ()> {


Using a result here is the lazy option - this should be infallible and we should panic on programming errors which are fundamentally unsafe. At a high level, I think the right approach is to

gate ChannelMonitor requesting such transactions on the channel having been closed,

have ChannelManager close the channel on would_broadcast_at_height(height + 2),

then have ChannelMonitor close it at the regular height (allowing itself to "close" the channel).

That way we shouldn't have any races where ChannelMonitor tries to get the latest local transaction but the channel isn't closed.

Then,

ChannelMonitor::get_latest_local_commitment_transaction should return a Result (since its the public API),

If a channel is already force-closed, we should allow it to return fine,

If a channel is still open - we should return an Err and the client should force-close the channel in the manager (or require a ChannelMonitor::channel_was_force_closed() call).

lightning/src/ln/functional_tests.rs

TheBlueMatt · 2020-04-01T00:24:12Z

lightning/src/ln/onchaintx.rs

+	#[cfg(test)]
+	pub(super) fn get_fully_signed_copy_local_tx(&mut self, channel_value_satoshis: u64) -> Option<Transaction> {
+		if let Some(ref mut local_commitment) = self.local_commitment {
+			let mut local_commitment = local_commitment.clone();


The thing that should be enforcing that we only ever sign one version is the enforcing_trait_impls stuff, not here. We need a #[cfg(test)] different version of sign_local_commitment.

After further analysis, this test-only function doesn't make sense at all, sign_local_commitment only enforce uniqueness of signature for a commitment transaction version (has_local_sig). For testing purposes what we want is a) signature of multiple different commitment transactions b) avoid Monitor panic/error after local-commitment-state-lockdown. So modified provide_latest_local_tx to not return err for test-only.

Right, we don't currently enforce anything, we will do it at the ChannelKeys layer. Thus, we need a test-only version of sign_local_comitment.

Updated version in enforcing_trait_impls, with a TODO in keysinterface

lightning/src/chain/keysinterface.rs

codecov · 2020-04-06T22:59:59Z

Codecov Report

❗ No coverage uploaded for pull request base (master@7ec16e5). Click here to learn what that means.
The diff coverage is n/a.

TheBlueMatt

This is starting to look great. A few minor comments and I really think this needs a test of the new logic around channelmonitors failing to update after a force-close or heigh-based broadcast.

lightning/src/ln/chan_utils.rs

TheBlueMatt · 2020-04-07T02:15:54Z

lightning/src/ln/channelmonitor.rs

@@ -1253,7 +1253,9 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 			feerate_per_kw,
 			htlc_outputs,
 		});
-		self.onchain_tx_handler.provide_latest_local_tx(commitment_tx);
+		if let Err(_) = self.onchain_tx_handler.provide_latest_local_tx(commitment_tx) {
+			return Err(MonitorUpdateError("Local commitment signed has already been signed, no further update of LOCAL commitment transaction is allowed"));


Shouldnt this occur before we update the local state in the ChannelMonitor itself?

I moved it, but added a big note than you may use in the loosely future a watchtower with a different implementation monitoring backend, which doesn't reject update in this case and may broadcast the latest local signed commitment received as update (assuming it has key access). And you want this backend to be able to claim outputs. That's really an edge-case so just a comment for now.

TheBlueMatt · 2020-04-07T02:43:52Z

lightning/src/ln/onchaintx.rs

+		// commitment transaction view to avoid delivery of revocation secret to counterparty
+		// for the aformentionned signed transaction.
+		if let Some(ref local_commitment) = self.local_commitment {
+			#[cfg(not(test))] //Test framework may need to generate revoked commitment tx, don't generate a monitor error


Oof, I strongly disagree with this - this is an error condition that we should be writing tests to trigger, not turning off in testing. I really think the previous design of an exposed test-only version is much better.

devrandom · 2020-04-08T19:36:51Z

lightning/src/chain/keysinterface.rs

+	/// TODO: Document the things someone using this interface should enforce before signing.
+	/// TODO: Add more input vars to enable better checking (preferably removing commitment_tx and
+	/// making the callee generate it via some util function we expose)!
+	fn sign_local_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, local_commitment_tx: &mut LocalCommitmentTransaction, funding_redeemscript: &Script, channel_value_satoshis: u64, secp_ctx: &Secp256k1<T>);


I think a pattern of returning a Result<Signature, Error> (rather than mutating the tx) is a cleaner API:

is a pure function, so easy to see that it doesn't have any unexpected side-effects

allows return of an error result. for example, see here where an error may happen and we don't want to fail silently. In the future, there may be other failures we want to communicate, related to policy checks.

Agreed. Sadly, this PR has gotten large, and kinda leaves a few of the API bits around signing logic to future PRs. See #567 for the tracking issue.

Thanks for your input, I agree we have room to improve the ChannelKeys API. I would rather do it in follow-up PRs, at first this one was scoped to move generation of local transactions in one place (OnchainTxHandler). Extending ChannelKeys to sign local was just an easy way to avoid breaking signature during the move.

I referenced your comments in #567 to be sure to not forget them.

TheBlueMatt · 2020-04-08T20:23:43Z

lightning/src/ln/chan_utils.rs

+	//TODO: modify Channel methods to integrate HTLC material at LocalCommitmentTransaction generation to drop Option here
+	local_keys: Option<TxCreationKeys>,
+	feerate_per_kw: Option<u64>,
+	per_htlc: HashMap<u32, (HTLCOutputInCommitment, Option<Signature>, Option<Transaction>)>


Oof. Can we skip the HashMap here and just use a Vec<Option<>> - there should only ever be at max two (three with anchor) Nones in the list.

TheBlueMatt · 2020-04-08T20:57:27Z

lightning/src/chain/keysinterface.rs

+	/// TODO: Document the things someone using this interface should enforce before signing.
+	/// TODO: Add more input vars to enable better checking (preferably removing commitment_tx and
+	/// making the callee generate it via some util function we expose)!
+	fn sign_local_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, local_commitment_tx: &mut LocalCommitmentTransaction, funding_redeemscript: &Script, channel_value_satoshis: u64, secp_ctx: &Secp256k1<T>);


Agreed. Sadly, this PR has gotten large, and kinda leaves a few of the API bits around signing logic to future PRs. See #567 for the tracking issue.

TheBlueMatt

Got most of the way through and found a number of things that seem to violate clearly-stated precondions :/.

lightning/src/chain/keysinterface.rs

TheBlueMatt · 2020-04-10T21:30:55Z

lightning/src/ln/chan_utils.rs

+		if self.local_keys.is_none() || self.feerate_per_kw.is_none() { return; }
+		let local_keys = self.local_keys.as_ref().unwrap();
+		let txid = self.txid();
+		for this_htlc in self.per_htlc.iter_mut() {


Oof. I didn't mean to scan the whole vec each time we want to sign something, you can keep them in htlc_index order. Again, this will get fixed as a side-effect of a better API so fine to leave it, but that hurts.

That's not a hot path given that's only when we go onchain, so good enough to wait for an API clean up :)

lightning/src/ln/chan_utils.rs

lightning/src/ln/channel.rs

TheBlueMatt · 2020-04-10T22:13:45Z

lightning/src/ln/channelmonitor.rs

 		if self.their_to_self_delay.is_none() {
 			return Err(MonitorUpdateError("Got a local commitment tx info update before we'd set basic information about the channel"));
 		}
+		let txid = commitment_tx.txid();
+		self.current_local_commitment_number = 0xffff_ffff_ffff - ((((commitment_tx.without_valid_witness().input[0].sequence as u64 & 0xffffff) << 3*8) | (commitment_tx.without_valid_witness().lock_time as u64 & 0xffffff)) ^ self.commitment_transaction_number_obscure_factor);


Wait, huh. I thought we were doing all the updates after the tx_handler.provide_latest_local_ call. Isn't this a severe bug - if the remote ChannelMonitor is updating we may end up with the wrong current_local_commitment_number set, breaking claiming?

It's not nasty as this, because it would mean we already have a lockdown ChannelMonitor and a local commitment transaction broadcast, as far as I can tell it would just mean an already-closed channel wouldn't be call for force-closure a second time by ChannelManager deserialization. But yes that's clearly a bug so fixed it

TheBlueMatt · 2020-04-10T22:33:16Z

lightning/src/ln/channelmonitor.rs

+				for htlc in local_tx.htlc_outputs.iter() {
+					if let Some(htlc_index) = htlc.0.transaction_output_index {
+						let preimage = if let Some(preimage) = self.payment_preimages.get(&htlc.0.payment_hash) { Some(*preimage) } else { None };
+						if let Some(htlc_tx) = self.onchain_tx_handler.get_fully_signed_htlc_tx(txid, htlc_index, preimage) {


This seems to violate the docs for sign_htlc_transacion, which says "If the transaction is an
/// HTLC-Success transaction (ie htlc.offered is false), preimage must be set!"?

Yes this is currently enforced in OnchainTxHandler::get_fully_signed_htlc_tx. IIRC I doubt a bit between checking in ChannelMonitor or OnchainTxHandler, I think that's the same and only temporary waiting for a further API refactor.

Ah, I missed that you added a check in add_htlc_sig that didn't exist in sign_htlc_transaction without updating the comment. Its kinda awkward that the LocalCommitmentTransaction layer checks that the caller did something sane, but, for now, OK.

TheBlueMatt · 2020-04-14T01:35:30Z

Needs rebase.

ariard · 2020-04-14T02:50:37Z

Rebased, but wasn't any conflict

TheBlueMatt · 2020-04-14T19:38:56Z

lightning/src/ln/chan_utils.rs

+		let local_keys = Readable::read(reader)?;
+		let feerate_per_kw = Readable::read(reader)?;
+		let htlcs_count: u64 = Readable::read(reader)?;
+		let mut per_htlc = Vec::with_capacity(cmp::min(htlcs_count as usize, MAX_ALLOC_SIZE / std::mem::size_of::<(HTLCOutputInCommitment, Option<Signature>, Option<Transaction>)>()));


This doesnt compile on 1.22 - you need ::std::mem

Should be solved at f07505e

Extend external signer interface to sign local commitment transactions on its behalf without seckey passing. This move will allow us to remove key access from ChannelMonitor hot memory in further work. Local commitment transaction should stay half-signed by remote until we need to broadcast for a channel force-close or a HTLC to timeout onchain. Add an unsafe test-only version of sign_local_commitment to fulfill our test_framework needs.

TheBlueMatt

Just a few trivial things left, Everything else is good to go, I think.

TheBlueMatt · 2020-04-17T01:45:44Z

lightning/src/ln/functional_tests.rs

@@ -2346,48 +2346,39 @@ fn claim_htlc_outputs_single_tx() {
 		}

 		let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
-		assert_eq!(node_txn.len(), 21);
+		assert_eq!(node_txn.len(), 9);


Huh? As of the tip of this PR The list of txns appears to be duplicative, and neither adds up to 9.

lightning/src/ln/functional_tests.rs

TheBlueMatt · 2020-04-17T01:52:03Z

lightning/src/ln/functional_tests.rs

+	let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+	assert_eq!(updates.update_fulfill_htlcs.len(), 1);
+	nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]);
+	if let Some(ref mut channel) = nodes[0].node.channel_state.lock().unwrap().by_id.get_mut(&chan_1.2) {


Oof, lets not cheat - this is a supported operation, use the public API.

Yeah but I think I need to implement another ManyChannelMonitor for simulating one monitor being out-of-sync and the other fine. I can add a TODO, when we add test stuff for watchtowers ?

You should be able to just add a utility there to track the latest updates and get the update from there. I think it should be a rather trivial change.

TheBlueMatt · 2020-04-17T03:15:37Z

lightning/src/ln/functional_tests.rs

+	watchtower.simple_monitor.block_connected(&header, 200, &vec![], &vec![]);
+
+	// Try to update ChannelMonitor
+	assert!(nodes[1].node.claim_funds(preimage, 9_000_000));


Needs rebase, just add a &None for the secret.

It was good, I just forget to commit on last push...

TheBlueMatt · 2020-04-17T03:21:12Z

lightning/src/ln/onchaintx.rs

+				pending_claim_updates.push((*first_claim_txid, new_timer, new_feerate));
+			}
+		}
+		for updates in pending_claim_updates {


This seems to not be needed. At least if I revert most of this hunk at the end it still passes borrowck etc.

Hmmmm? I hit a borrow checker with original hunk, you can't have both pending_claim_requests as mutable and generate_claim_tx

Hmm, this diff compiles for me?

diff --git a/lightning/src/ln/onchaintx.rs b/lightning/src/ln/onchaintx.rs index 2f08fe291..3b8238ddc 100644 --- a/lightning/src/ln/onchaintx.rs +++ b/lightning/src/ln/onchaintx.rs @@ -713,18 +713,14 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> { // Build, bump and rebroadcast tx accordingly log_trace!(self, "Bumping {} candidates", bump_candidates.len()); - let mut pending_claim_updates = Vec::with_capacity(bump_candidates.len()); for (first_claim_txid, claim_material) in bump_candidates.iter() { if let Some((new_timer, new_feerate, bump_tx)) = self.generate_claim_tx(height, &claim_material, &*fee_estimator) { log_trace!(self, "Broadcast onchain {}", log_tx!(bump_tx)); broadcaster.broadcast_transaction(&bump_tx); - pending_claim_updates.push((*first_claim_txid, new_timer, new_feerate)); - } - } - for updates in pending_claim_updates { - if let Some(claim_material) = self.pending_claim_requests.get_mut(&updates.0) { - claim_material.height_timer = updates.1; - claim_material.feerate_previous = updates.2; + if let Some(claim_material) = self.pending_claim_requests.get_mut(first_claim_txid) { + claim_material.height_timer = new_timer; + claim_material.feerate_previous = new_feerate; + } } } }

Previously, we would regenerate this class of txn twice due to block-rescan triggered by new watching outputs registered. This commmit doesn't change behavior, it only tweaks TestBroadcaster to ensure we modify cleanly tests anticipating next commit refactor.

As transaction generation and signature is headed to be moved inside OnchainTxHandler, cache any usefule witness element.

As transaction generation and signature is headed to be moved inside OnchainTxHandler, cache local_commitment_tx signed by remote. If access to local commitment transaction is needed, we extend Onchain TxHandler API to do so.

To prevent any unsafe state discrepancy between offchain and onchain, once local commitment transaction has been signed due to an event (either block height for HTLC-timeout or channel force-closure), don't allow any further update of local commitment transaction view to avoid delivery of revocation secret to counterparty for the aformentionned signed transaction.

Local Commitment Transaction can't be bumped without anchor outputs so their generation is one-time for now. We move them in OnchainTxHandler for simplifying ChannelMonitor and to prepare storage of keys material behind one external signer interface. Some tests break due to change in transaction broadcast order but number of transactions broadcast should stay the same.

By caching current local commitment number instead of deciphering it from local commitment tx, we may remove local commitment tx from ChannelMonitor in next commit.

Implementing dynamic fee bumping implied to cache transaction material including its witness, to generate a bumped version if needed. ChannelMonitor is slowly rescoped to its parsing function with ongoing patchset and data duplicata are removed. If signed local commitment tx access is needed, it's done through OnchainTxHandler extended API For test framework purpose, we use the test-only method ChannelMonitor::unsafe_get_latest_local_commitment_txn to intentionally generate unsafe local commitment to exerce revocation logic.

Caching of input material for HTLC transaction was introducted prevently but since then API (InputMaterial) has changed between ChannelMonitor and OnchainTxHandler

Extend external signer interface to sign HTLC transactions on its behalf without seckey passing. This move will allow us to remove key access access from ChannelMonitor hot memory in further work. HTLC transactions should stay half-signed by remote until we need to broadcast them for timing-out/claiming HTLCs onchain.

Splitting further parsing from transaction generation, we cache transaction elements needed for local HTLC transaction inside OnchainTxHandler. Duplicated data will be removed from ChannelMonitor in future commits.

csv_local is csv_delay encumbering local revokable_redeemscript for to_local an htlc output on local commitment/HTLC transactions.

In case of channel force-closure, access to local commitment transactions and its dependent HTLCs is needed. Instead of using broadcast_by_local_state which registers outpoint to claim and outputs to watch which are going to be discarded in this case, we simply ask OnchainTxHandler to build and sign HTLC transactions through new API.

HTLC Transaction can't be bumped without sighash changes so their gneeration is one-time for nwo. We move them in OnchainTxHandler for simplifying ChannelMonitor and to prepare storage of keys material behind one external signer interface. Some tests break due to change in transaction broadcaster order. Number of transactions may vary because of temporary anti-duplicata tweak can't dissociate between 2- broadcast from different origins (ChannelMonitor, ChannelManager) and 2-broadcast from same component.

Local commitment transaction broadcast can be triggered by a) a Channel force-close or b) reaching some block height implying a onchain HTLC-timeout. If one of this condition is fulfilled, commitment is signed and from then any state update would be rejected. ChannelMonitor init at Channel creation need to be refactored before to make get_fully_signed_local_tx infaillible to avoid choking in the test framework.

Channel shouldn't send a ChannelForceClosed update followed by a LatestLocalCommitmentTxInfo as it would be a programming error leading to risk of money loss. Force-closing the channel will broadcast the local commitment transaction, if the revocation secret for this one is released after its broadcast, it would allow remote party to claim outputs on this transaction using the revocation path.

This test tries the new lockdown logic in case of a signed-and-broadcast local commitment transaction while a concurrent ChannelMonitorUpdate for a next _local_ commitment is submitted from offchain. Update is rejected as expected with a ChannelMonitorUpdateErr.

TheBlueMatt reviewed Mar 24, 2020

View reviewed changes

lightning/src/ln/chan_utils.rs Outdated Show resolved Hide resolved

lightning/src/ln/onchaintx.rs Outdated Show resolved Hide resolved

ariard force-pushed the 2020-03-move-local-commitment branch 2 times, most recently from ba4e8ee to 5aeb582 Compare March 25, 2020 23:56

TheBlueMatt reviewed Mar 26, 2020

View reviewed changes

ariard force-pushed the 2020-03-move-local-commitment branch 2 times, most recently from 4ebfc00 to 2dbb9a7 Compare March 28, 2020 03:18

TheBlueMatt reviewed Apr 1, 2020

View reviewed changes

ariard force-pushed the 2020-03-move-local-commitment branch from 2dbb9a7 to 9f4c44a Compare April 5, 2020 03:18

ariard force-pushed the 2020-03-move-local-commitment branch from 3cfee0d to fd364d8 Compare April 6, 2020 23:07

TheBlueMatt reviewed Apr 7, 2020

View reviewed changes

ariard force-pushed the 2020-03-move-local-commitment branch from fd364d8 to eff2232 Compare April 7, 2020 23:17

devrandom reviewed Apr 8, 2020

View reviewed changes

TheBlueMatt reviewed Apr 8, 2020

View reviewed changes

ariard force-pushed the 2020-03-move-local-commitment branch from 12de2f1 to b07747a Compare April 9, 2020 23:43

TheBlueMatt reviewed Apr 10, 2020

View reviewed changes

ariard force-pushed the 2020-03-move-local-commitment branch from b07747a to 7f4b668 Compare April 14, 2020 00:08

ariard force-pushed the 2020-03-move-local-commitment branch from 7f4b668 to 922b325 Compare April 14, 2020 02:49

TheBlueMatt added this to the 0.0.11 milestone Apr 14, 2020

TheBlueMatt reviewed Apr 14, 2020

View reviewed changes

ariard force-pushed the 2020-03-move-local-commitment branch 2 times, most recently from f07505e to a51477b Compare April 15, 2020 23:14

ariard force-pushed the 2020-03-move-local-commitment branch from a51477b to 68c5522 Compare April 16, 2020 02:29

TheBlueMatt reviewed Apr 17, 2020

View reviewed changes

Antoine Riard added 17 commits April 17, 2020 17:43

Cache funding_redeemscript in OnchainTxHandler

04a17b2

As transaction generation and signature is headed to be moved inside OnchainTxHandler, cache any usefule witness element.

Cache local_commitment_tx in OnchainTxHandler

73e0a01

As transaction generation and signature is headed to be moved inside OnchainTxHandler, cache local_commitment_tx signed by remote. If access to local commitment transaction is needed, we extend Onchain TxHandler API to do so.

Cache current local commitment number in ChannelMonitor.

493ffb8

By caching current local commitment number instead of deciphering it from local commitment tx, we may remove local commitment tx from ChannelMonitor in next commit.

Remove per_input_material introduce in commit 0011713

7e395e0

Caching of input material for HTLC transaction was introducted prevently but since then API (InputMaterial) has changed between ChannelMonitor and OnchainTxHandler

Cache HTLC transaction material inside OnchainTxHandler

010fb30

Splitting further parsing from transaction generation, we cache transaction elements needed for local HTLC transaction inside OnchainTxHandler. Duplicated data will be removed from ChannelMonitor in future commits.

Cache per-HTLC data in OnchainTxHandler::HTLCTxCache

080afeb

Splitting further parsing from transaction generation, we cache transaction elements needed for local HTLC transaction inside OnchainTxHandler. Duplicated data will be removed from ChannelMonitor in future commits.

Cache csv_local inside OnchainTxHandler

6b8a516

csv_local is csv_delay encumbering local revokable_redeemscript for to_local an htlc output on local commitment/HTLC transactions.

Remove temporary anti-duplicata logic

9faf6ca

ariard force-pushed the 2020-03-move-local-commitment branch from 68c5522 to 95830ed Compare April 17, 2020 22:43

TheBlueMatt merged commit 02c1925 into lightningdevkit:master Apr 18, 2020

		pub fn without_valid_witness(&self) -> &Transaction { &self.tx }
		/// Expose raw transaction with asserting witness is complete

Split parsing and transaction management for local transactions between Chanmon/Onchain #559

Split parsing and transaction management for local transactions between Chanmon/Onchain #559

Uh oh!

Conversation

ariard commented Mar 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ariard commented Mar 25, 2020

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ariard commented Mar 28, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Apr 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ariard Apr 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

ariard commented Mar 24, 2020 •

edited

Loading

codecov bot commented Apr 6, 2020 •

edited

Loading

ariard Apr 7, 2020 •

edited

Loading