HTLCs #27

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

src/DotNetLightning.Core/Channel/CommitmentsModule.fs

-                            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

src/DotNetLightning.Core/Crypto/KeyExtensions.fs

@@ -285,6 +286,7 @@ module KeyExtensions =
                                   : 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

src/DotNetLightning.Core/Crypto/Sphinx.fs

-            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

src/DotNetLightning.Core/Crypto/Sphinx.fs

+        // 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

src/DotNetLightning.Core/Crypto/Sphinx.fs

+        // 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

src/DotNetLightning.Core/Crypto/Sphinx.fs

    type PacketAndSecrets = {
        Packet: OnionPacket
        /// Shared secrets (one per node in the route). Known (and needed) only if you're creating the
        /// packet. Empty if you're just forwarding the packet to the next node
        SharedSecrets: (Key * PubKey) list
    }
        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

src/DotNetLightning.Core/DotNetLightning.Core.fsproj

@@ -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

src/DotNetLightning.Core/Serialization/TLVs.fs

+            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

src/DotNetLightning.Core/Transactions/Transactions.fs

@@ -566,6 +567,7 @@ module Transactions =
                    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