-
Notifications
You must be signed in to change notification settings - Fork 4
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
WIP - track more information about commit history #22
base: geewalletLightningMilestone3
Are you sure you want to change the base?
WIP - track more information about commit history #22
Conversation
- LICENSE: change from MIT to AGPL (.Kiss fork) - Change package name suffix from .Core to .Kiss (skipping native build)
It seems that these attributes can cause this compilation error in F#4.0: error FS0927: The kind of the type specified by its attributes does not match the kind implied by its definition Newer versions of F# (like 4.5 or even newer, like the one being used by .NETCore to build the binary that is later published in nuget) allow compiling this code with no issues, but if you reference the generated assembly later from an old F# compiler, it could generate exceptions at runtime, e.g.: System.BadFormatImageException (or other types) whose inner exception could be the following: System.TypeLoadException : Could not load type of field 'GWallet.Backend.UtxoCoin.Lightning.SerializedChannel:MinSafeDepth@' (6) due to: Expected reference type but got type kind 17 FSharp.Core's Result type is also affected by this so in this commit we create a replacement for it that is only used in the BouncyCastle build (which we now rename as 'Portability' build). Forward-ported from a4a59d0 Co-authored-by: Andres G. Aragoneses <knocte@gmail.com> Co-authored-by: Andrew Cann <shum@canndrew.org>
This NBitcoin issue: MetacoSA/NBitcoin#931 somehow still hasn't been fixed properly (our geewallet CI still encounters it, surprisingly). So let's reapply the workaround[1] that we had removed[2]. [1] joemphilips@d813a97 [2] joemphilips@256893c
ResultUtils: new OptionCE, a computation expression for creating options, similar to the result computation expression which DNL already has. Core.Utils: new SeqConsumerCE, a computation expression which makes it easy to write code which consumes a sequence, one element at a time.
In order to support this, this commit adds two elements: 1) CommitmentToLocalExtension This is an NBitcoin BuilderExtension that tells NBitcoin.TransactionBuilder how to recognise and sign to_local txouts from lightning commitment transactions. This is needed for force-closing to spend to_local outputs. 2) tryGetFundsFromLocalCommitmentTx When force-closing a channel this function is used to recover funds from our commitment transaction. It generates a transaction which can be broadcast once the timelock on the to_local output has expired.
This function recovers funds from the to_remote output of a remote commitment tx which has been broadcast to the blockchain.
d6d19f7
to
95e95b6
Compare
I see you force-pushed this PR, is it to address the above? if yes, state so here pls |
MaybeBestUnpublishedTx = None } | ||
return [ AcceptedShutdownWhenNoPendingHTLCs(closingSignedMsg |> Some, nextState) ] | ||
else | ||
let nextState = { NegotiatingData.ChannelId = cm.ChannelId | ||
Commitments = cm | ||
LocalShutdown = localShutdown | ||
RemoteShutdown = msg | ||
ClosingTxProposed = [ [] ] | ||
ClosingTxProposed = [ ] |
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.
let's rather use List.Empty, as per geewallet's CONTRIBUTING
Signed = cm.LocalChanges.Proposed | ||
} | ||
RemoteChanges = { | ||
cm.RemoteChanges with |
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.
many identation warnings above ^
} | ||
RemoteChanges = { | ||
cm.RemoteChanges with | ||
ACKed = [] |
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.
same here about List.Empty
LocalSignaturesOfRemoteCommitments = | ||
List.append | ||
cm.LocalSignaturesOfRemoteCommitments | ||
[ !> signature.Signature ] |
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.
nit: I think foo::List.Empty
is more readable [foo]
LocalChanges = ourChanges1 | ||
RemoteChanges = theirChanges1 | ||
OriginChannels = originChannels1 | ||
} |
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.
you're not really changing the lines above, so it's not worth it to change the style
de70b20
to
85d6eb8
Compare
3ff7695
to
1d1de3f
Compare
This is a WIP PR to track more information about commit history in DNL so that we can recover funds from remote commitment txs.