HTLCs by aarani · Pull Request #27 · nblockchain/DotNetLightning.Kiss

aarani · 2021-06-11T10:45:51Z

Work in progress...

- LICENSE: change from MIT to AGPL (.Kiss fork) - Change package name suffix from .Core to .Kiss (skipping native build)

According to lightning spec, if the receiver agrees with the fee, it should reply with a closing_signed with the same fee_satoshis value so to transport this message we need to pass it inside MutualClosePerformed event, to be transported to the other peer. Source: https://github.com/lightningnetwork/lightning-rfc/blob/master/02-peer-protocol.md#requirements-6 Upstream PR: joemphilips#168

Hmac should be written during the packet serialization and not during the payload serialization

Original implementation from https://github.com/ElementsProject/lightning/blob/ade10e7fc4dacbb9d635b05152c7dc38c0896ce7/contrib/pyln-proto/pyln/proto/onion.py#L497

All-zero initial packets should only be used for generating test vectors because of possible privacy leak. "Create packet (reference test vector)" testcase updated to new testcases provided by the RFC repo to meet this change of spec. See more: lightning/bolts@8dd0b75 https://github.com/lightningnetwork/lightning-rfc/blob/master/04-onion-routing.md#packet-construction

See more here: https://github.com/lightningnetwork/lightning-rfc/blob/master/04-onion-routing.md#tlv_payload-format https://github.com/lightningnetwork/lightning-rfc/blob/master/01-messaging.md#fundamental-types

Secret field inside TLVPayload's PaymentData is PaymentSecret and not PaymentPreimage and It's used for mpp.

knocte · 2021-06-14T06:55:40Z

-                            channelPrivKeys.SignHtlcTx htlc.Value localPerCommitmentPoint
-                        )
-                    localHtlcSigsAndHTLCTxs |> List.map(fst), localHtlcSigsAndHTLCTxs |> List.map(snd) |> Seq.cast<IHTLCTx> |> List.ofSeq
+                let HTLCTxsAndSignatures =


local vars should be camelCase, not PascalCase; now, I understand this var name starts with "HTLC" which is an acronym, but I like to do what the .NET BCL do wrt naming sometimes: HTTP -> Http, therefore HTLC -> Htlc, so here it can be htlcTxsAndSignatures

knocte · 2021-06-14T07:04:50Z

                                   : TransactionSignature * PSBT =
            let htlcPrivKey = perCommitmentPoint.DeriveHtlcPrivKey this.HtlcBasepointSecret
            let htlcPubKey = htlcPrivKey.HtlcPubKey()
+            psbt.Settings.CustomBuilderExtensions <- ([new HTLCReceivedExtensions() :> BuilderExtension; new HTLCOfferedExtensions():> BuilderExtension] |> Seq.ofList)


nits:

missing a space before :>

please split line in many, it's too long

are HTLCReceivedExtensions and HTLCOfferedExtensions IDisposable? if not, then remove the new keyword

knocte · 2021-06-14T08:49:53Z

-            assert (stream.Length = padding1.Length)
-            xor(padding1, stream)
-            ) [||]
+    let rec internal generateFiller (keyType: string) (payloads: byte[] list) (sharedSecrets: Key list) =


use array<byte> and list<Key> pls

knocte · 2021-06-14T08:50:48Z

+        // set of filler bytes by using chacha20 with a key derived from the session
+        // key.
+        let DeterministicPacketFiller (sessionKey: Key) =
+            generateStream(generateKey("pad",sessionKey.ToBytes()), 1300)


1300: magic number? pls put in a constant

space after comma

knocte · 2021-06-14T08:51:14Z

+        // instances that required deterministic packet generation.
+        [<Obsolete("BlankPacketFiller is obsolete, see here: https://github.com/lightningnetwork/lightning-rfc/commit/8dd0b75809c9a7498bb9031a6674e5f58db509f4", false)>]
+        let BlankPacketFiller _=
+            Array.zeroCreate 1300


use the constant for 1300 here

knocte · 2021-06-14T08:51:32Z

    }
        with
-            static member Create (sessionKey: Key, pubKeys: PubKey list, payloads: byte[] list, ad: byte[]) =
+            static member Create (sessionKey: Key, pubKeys: PubKey list, payloads: byte[] list, ad: byte[], initialPacketFiller: Key -> byte[]) =


use array<byte> pls

knocte · 2021-06-14T08:51:50Z

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+<Project Sdk="Microsoft.NET.Sdk" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">


nit: no need to change this line

knocte · 2021-06-14T08:52:31Z

+            UInt64.FromTruncatedBytes(tlv.Value)
            |> LNMoney.MilliSatoshis
            |> AmountToForward
        | 4UL ->
-            NBitcoin.Utils.ToUInt32(tlv.Value, false)
+            UInt32.FromTruncatedBytes(tlv.Value)


no need for parens in the 2 lines of both hunks above

knocte · 2021-06-14T08:53:38Z

                    ScriptCoin(coin, redeem)
                let dest = Scripts.toLocalDelayed localRevocationPubKey toLocalDelay localDelayedPaymentPubKey
                // we have already done dust limit check above
+                txb.Extensions.Add (new HTLCReceivedExtensions())


is HTLCReceivedExtensions IDisposable? if not, then remove the new

knocte · 2021-06-14T08:53:51Z

            buf

+    member private x.GetLeadingZerosCount() =
+        if (x = 0UL) then


no need for parenthesis

knocte · 2021-06-14T08:54:07Z

+            3
+        else if (x &&& 0xffff000000000000UL = 0UL) then
+            2
+        else if (x &&& 0xff00000000000000UL = 0UL) then


please replace all else if occurrences with elif

(and no need for parenthesis in all lines above)

knocte · 2021-06-14T08:54:42Z

+    member x.GetTruncatedBytes() =
+        let numZeros =
+            x.GetLeadingZerosCount()
+        x.GetBytesBigEndian() |> Array.skip(numZeros)


no need for parenthesis at the end

knocte · 2021-06-14T08:55:15Z

+            3
+        else if (x &&& 0xffff0000u = 0u) then
+            2
+        else if (x &&& 0xff000000u = 0u) then


same here about parenthesis and elif

knocte · 2021-06-14T08:55:27Z

+    member x.GetTruncatedBytes() =
+        let numZeros =
+            x.GetLeadingZerosCount()
+        x.GetBytesBigEndian() |> Array.skip(numZeros)


no need for parenthesis here

knocte · 2021-06-14T08:56:29Z

+        | Ok data -> Some data
+        | Error _consumeAllError -> None
+
+type internal HTLCOfferedExtensions() =


rename to HtlcOfferedExtensions as per previous suggestion

knocte · 2021-06-14T10:52:46Z

                    |> expectTransactionError
                let! finalizedCommitTx = tmp
-                let sortedHTLCTXs = Helpers.sortBothHTLCs htlcTimeoutTxs htlcSuccessTxs
+                let sortedHTLCTXs = Helpers.sortBothHTLCs htlcTimeoutTxs htlcSuccessTxs 


nit: remove trailing space pls

knocte · 2021-06-14T10:53:44Z

+            |> List.ofSeq
+            |> Seq.ofList
+
+        Console.WriteLine(JsonConvert.SerializeObject(ops))


remove Console.WriteLine? if you leave it there, explain why, and use 2 parenthesis instead of 4

knocte · 2021-06-14T10:54:08Z

+            // should return null, indicating to NBitcoin that the sigScript
+            // could not be generated.
+            match pubKey with
+            | null -> null


let's use Option.ofObj instead of checking for null

knocte · 2021-06-14T10:54:54Z

+                let remoteHtlcSig = signer.Sign (parameters.RemoteHtlcPubKey.RawPubKey())
+                Script [
+                    Op.GetPushOp (remoteHtlcSig.ToBytes())
+                ]


there's duplication in these 2 blocks, let's have just 1 block

knocte · 2021-06-14T10:55:50Z

+            |> List.ofSeq
+            |> Seq.ofList
+
+        Console.WriteLine(JsonConvert.SerializeObject(ops))


remove CWL?

PSBT sign function needs transaction builder for any non-predefined script template

knocte and others added 6 commits May 31, 2021 13:52

DotNetLightning.Kiss fork

9d790f8

- LICENSE: change from MIT to AGPL (.Kiss fork) - Change package name suffix from .Core to .Kiss (skipping native build)

Mono-hop unidirectional payments

25c8ca9

Core: remove hmac from TLVPayload

d32c30c

Hmac should be written during the packet serialization and not during the payload serialization

Support variable payload length on fillerGeneration

b2f17bc

Original implementation from https://github.com/ElementsProject/lightning/blob/ade10e7fc4dacbb9d635b05152c7dc38c0896ce7/contrib/pyln-proto/pyln/proto/onion.py#L497

aarani force-pushed the wip/htlc branch from 42876d9 to 08b6971 Compare June 11, 2021 11:50

aarani added 3 commits June 12, 2021 00:19

Support for non fixed-size payloads when making packets

eccd3a1

TLVs: HopPayloadTLV uses truncated integers

56d538b

See more here: https://github.com/lightningnetwork/lightning-rfc/blob/master/04-onion-routing.md#tlv_payload-format https://github.com/lightningnetwork/lightning-rfc/blob/master/01-messaging.md#fundamental-types

Secret inside TLVPayload is PaymentSecret

04a9286

Secret field inside TLVPayload's PaymentData is PaymentSecret and not PaymentPreimage and It's used for mpp.

knocte reviewed Jun 14, 2021

View reviewed changes

nit fixes

d0b91b7

aarani force-pushed the wip/htlc branch from 287b4aa to d0b91b7 Compare June 16, 2021 07:48

Use sign function to sign HtlcTx

6e36d90

PSBT sign function needs transaction builder for any non-predefined script template

knocte force-pushed the master branch from 9f9f73e to 2883c78 Compare June 17, 2021 09:03

canndrew force-pushed the master branch 4 times, most recently from f817da9 to 0340b0b Compare June 18, 2021 07:38

aarani changed the base branch from master to upstream June 18, 2021 09:37

aarani changed the base branch from upstream to old/wip/monohop June 18, 2021 09:38

aarani changed the base branch from old/wip/monohop to wip/upstream June 18, 2021 09:38

aarani changed the base branch from wip/upstream to master June 18, 2021 09:38

knocte force-pushed the master branch 3 times, most recently from 0389f6a to 60d2401 Compare June 22, 2021 13:06

knocte force-pushed the master branch from 60d2401 to e2e13a5 Compare June 29, 2021 16:31

knocte force-pushed the master branch 4 times, most recently from 419cd8d to f0461b4 Compare November 17, 2021 06:50

knocte force-pushed the master branch 4 times, most recently from 37fe5af to 1f418fe Compare December 2, 2021 21:08

knocte force-pushed the master branch 5 times, most recently from bd34034 to b67e13f Compare August 23, 2022 04:36

knocte force-pushed the master branch from 5c85f50 to ba24f1e Compare August 29, 2022 07:03

		@@ -1,4 +1,4 @@
		<Project Sdk="Microsoft.NET.Sdk" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
		<Project Sdk="Microsoft.NET.Sdk" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

Uh oh!

Conversation

aarani commented Jun 11, 2021

Uh oh!

knocte Jun 14, 2021 • 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!

knocte Jun 14, 2021 • 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

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

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

knocte Jun 14, 2021 •

edited

Loading

knocte Jun 14, 2021 •

edited

Loading