Add util to track justice transactions for watchtowers #2552
Conversation
Now that we've introduced the `JusticeTxTracker` util, we can reuse it inside of our test utils.
| destination_script: Script, | ||
| } | ||
|
|
||
| impl_writeable_tlv_based!(JusticeTxTracker, { |
There was a problem hiding this comment.
So I know I suggested just implementing writeable here, but it really does make this whole thing kinda a pain to use. I wonder if we shouldn't store the latest commitment tx info in ChannelMonitor so that getting the revoked transaction given a ChannelMonitorUpdate isn't just one function call and requires no storage outside of the ChannelMonitor.
There was a problem hiding this comment.
Huh yea, that does seem a lot easier. Will probably get around to trying this out sometime this weekend/early next week
There was a problem hiding this comment.
Yea, no rush, just figured I'd mention.
| } | ||
| } | ||
|
|
||
| impl Writeable for VecDeque<(Event, Option<EventCompletionAction>)> { |
There was a problem hiding this comment.
IMO its a bit weird to move stuff that's only used in one file into ser.rs, I'd kinda rather keep it with the struct definition.
There was a problem hiding this comment.
Don't return transactions with outputs below the dust limit?
Note, you have the transaction-relay dust limit (GetDustThreshold) and the channel-level dust limit negotiated at opening (dust_limit_satoshis). While the former has been documented in the spec https://github.com/lightning/bolts/blob/master/03-transactions.md#dust-limits to pick it up as the latter. There is no quantitative indication on the max size accepted of the dust_limit_satoshis.
I think you’re asking on the former (GetDustThreshold) and if this is the case, I think yes we can disregard justice transaction for outputs below the dust limit. I think any future bump of the dust limit will be discussed far ahead in the ecosystem and the default value is already documented in that sense “Changing the dust limit changes which transactions are standard and should be done with care and ideally rarely”.
edited: If your question is about the justice transaction output being under the dust there is the option to aggregate justice transactions - assuming you don’t delegate the broadcast to a non-trusted third-party.
|
|
||
| pub(crate) struct UnsignedJusticeData { | ||
| justice_tx: Transaction, | ||
| value: u64, |
There was a problem hiding this comment.
This can be called amount_sat to dissociate well from amount_msat, used e.g in HTLCOutputInCommitment.
| } | ||
| } | ||
|
|
||
| /// A simple utility object that can be used to track the state required to build and sign a |
There was a problem hiding this comment.
I think this is unclear if this utility object aims to be hosted by a LDK user itself.
Note if you’re outsourcing your justice transaction broadcast to an external watchtower, if you have a range of RBF feerates (as build_to_local_justice_tx()) let you do so there is no guarantee the outsourced watchtower will not broadcast the highest-feerate transaction, which might be overpaying current mempool feerates. I don’t know if more documentation of this watchtower utility aims to be added on this PR or in the future.
|
Sorry about the delay here, been busy with school etc.! I plan to pick this back up after the semester is over ~Dec 14 or so (along with my other orphaned PR #2457), if anyone wants this sooner, feel free to pick it up. |
|
Hey @alecchendev I was going to pick this PR up if that's alright. |
|
@sangbida Go for it! Thank you! |
|
Hey, I think I'm a bit out of my depth for this one, so I'm going to pause work on this for now and find something a little smaller to pick up. If anyone else would like to pick this up in the meantime, feel free to do so :) |
|
Hey there, any news here regarding this PR? We desperately need the Watchtowers for our project and would be huge for lightning and improve its acceptance |
Follow-up on #2337 adding a utility to help track and build signed justice transactions.
To do