-
Notifications
You must be signed in to change notification settings - Fork 133
Prepare tap channels for v1 proofs #1779
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
GeorgeTsagk
wants to merge
6
commits into
feature-bits
Choose a base branch
from
channels-v1
base: feature-bits
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
86a01fb
tapchannel: parameterize stxo for commitments
GeorgeTsagk f4cbbab
tapchannel: include stxo proofs on channel funding
GeorgeTsagk ffaeb0e
tapchannelmsg: include stxo flag in commitment blob
GeorgeTsagk 15feacf
tapchannel: add stxo support for aux leaves
GeorgeTsagk 5cfb32b
tapchannel: use stxo on aux chan closer
GeorgeTsagk 623d31d
tapchannel: support stxo on aux sweeper
GeorgeTsagk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First thought at seeing this change: do we actually need to propagate this all the way here just to do something like fetch the set of aux leaves, or to apply an HTLC view?
Even before a channel is created, we know if we're going to use the STXO feature and also noop feature (for new channels). Therefore, when preparing the funding and commitment blobs, we can just store this information right there and there. So then we don't need to query for feature bits each time we want to make a new commitment or validate one from the remote peer.
The other dependnacy that pops up here is: these methods needs to always work, even if we're not currently connected to a peer. IIUC, this new aux negotiator interface can only query live peer state (what did they send us in
init
, etc).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where exactly are you referring to? If you're referring to the funding/commit blobs then we can't really rely on those values for future decisions. Given that (at least for a short time window) downgrading from v1 will be allowed, then we need to know what the latest state of our peer is. Last commitment could have been using v1, but then upon reconnection our peer could stop supporting it.
I think for
ApplyHTLCView
we know that the channel must be active for the method to be called, which means we have communicated/agreed upon feature bits with corresponding peer. You're right for some of these cases that we could skip using the AuxChanNegotiator and rely on the commit blob, will change those hooks accordingly.Generally, if the method affects the creation of the next commitment we'll want to consult the negotiator. If it just cares about some existing commitment, we could look up the flag in the blob.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not? We generate that blob ourselves. If say someone changes that information to say that there's only 5 beefbux in a channel but we had 10, then we'd act on that information. You're correct that atm it isn't directly cryptographically authenticated, but we assume that the information we wrote is correct.
We can protect against downgrading if we store the current negotiated feature bit information in the funding blob. If we come online, and the peer tells us we're not using the new channel type, then we can fail to proceed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, you're correct about that. We only call that when accepting or creating a new commitment.