Merge pull request joemphilips#92 from canndrew/init-message-legacy-compat

Init message legacy compatibility

Author: joemphilips

src/DotNetLightning.Core/Payment/PaymentRequest.fs

@@ -285,13 +285,11 @@ type TaggedField =
             let routeInfoBase32 = routeInfoBase256 |> Array.concat |>  Helpers.convert8BitsTo5
             this.WriteField(writer, routeInfoBase32)
         | FeaturesTaggedField f ->
-            if f.BitArray.Length = 0 then () else
-            let pad = if (f.BitArray.Length % 40 = 0) then 0 else 40 - (f.BitArray.Length % 40)
             let dBase32 =
-                let ba =
-                    [BitArray(pad, false); f.BitArray] |> BitArray.Concat |> fun baa -> baa.ToByteArray()
-                ba |> Helpers.convert8BitsTo5
-                |> Array.skipWhile((=)0uy)
+                let mutable byteArray = f.BitArray.ToByteArray()
+                while (byteArray.Length * 8) % 5 <> 0 do
+                    byteArray <- Array.concat [ [| 0uy |]; byteArray ]
+                byteArray |> Helpers.convert8BitsTo5 |> Array.skipWhile((=)0uy)
             this.WriteField(writer, dBase32)
 
 

src/DotNetLightning.Core/Serialize/Features.fs

@@ -29,6 +29,11 @@ type Feature = private {
     with
     member this.MandatoryBitPosition = this.Mandatory
     member this.OptionalBitPosition = this.Mandatory + 1
+    member this.BitPosition(support: FeaturesSupport) =
+        match support with
+        | Mandatory -> this.MandatoryBitPosition
+        | Optional -> this.OptionalBitPosition
+
     override this.ToString() = this.RfcName
 
     static member OptionDataLossProtect = {
@@ -89,7 +94,7 @@ module internal Feature =
         |> Map.add (Feature.BasicMultiPartPayment) ([Feature.PaymentSecret])
         |> Map.add (Feature.PaymentSecret) ([Feature.VariableLengthOnion])
 
-    let private isFeatureOn(features: BitArray) (bit: int) =
+    let isFeatureOn(features: BitArray) (bit: int) =
         (features.Length > bit) && features.Reverse().[bit]
 
     let hasFeature(features: BitArray) (f: Feature) (support: FeaturesSupport option) =
@@ -170,17 +175,14 @@ module internal Feature =
         |> Set
 
 
-/// Uses regular class instead of F# type for caching byte[] representation
 [<StructuredFormatDisplay("{PrettyPrint}")>]
 type FeatureBit private (bitArray) =
-    let mutable bytes = null
     member val BitArray: BitArray = bitArray with get, set
     member this.ByteArray
         with get() =
-            if isNull bytes then
-                bytes <- this.BitArray.ToByteArray()
-            bytes
-        and set(v: byte[]) = bytes <- v
+            this.BitArray.ToByteArray()
+        and set(bytes: byte[]) =
+            this.BitArray <- BitArray.FromBytes(bytes)
     static member TryCreate(ba: BitArray) =
         result {
             do! Feature.validateFeatureGraph(ba)
@@ -196,12 +198,8 @@ type FeatureBit private (bitArray) =
         let b: bool array = [||]
         b |> BitArray |> FeatureBit
     static member TryCreate(bytes: byte[]) =
-        result {
-            let! fb = FeatureBit.TryCreate(BitArray.FromBytes(bytes))
-            fb.ByteArray <- bytes
-            return fb
-        }
-        
+        FeatureBit.TryCreate(BitArray.FromBytes(bytes))
+
     static member TryCreate(v: int64) =
         BitArray.FromInt64(v) |> FeatureBit.TryCreate
 
@@ -230,6 +228,27 @@ type FeatureBit private (bitArray) =
     override this.ToString() =
         this.BitArray.PrintBits()
 
+    member this.SetFeature(feature: Feature) (support: FeaturesSupport) (on: bool): unit =
+        let index = feature.BitPosition support
+        let length = this.BitArray.Length
+        if length <= index then
+            this.BitArray.Length <- index + 1
+
+            //this.BitArray.RightShift(index - length + 1)
+
+            // NOTE: Calling RightShift gives me:
+            // "The field, constructor or member 'RightShift' is not defined."
+            // So I just re-implement it here
+            for i in (length - 1) .. -1 .. 0 do
+                this.BitArray.[i + index - length + 1] <- this.BitArray.[i]
+
+            // NOTE: this probably wouldn't be necessary if we were using
+            // RightShift, but the dotnet docs don't actualy specify that
+            // RightShift sets the leading bits to zero.
+            for i in 0 .. (index - length) do
+                this.BitArray.[i] <- false
+        this.BitArray.[this.BitArray.Length - index - 1] <- on
+
     member this.HasFeature(f, ?featureType) =
         Feature.hasFeature this.BitArray (f) (featureType)
 
@@ -249,12 +268,8 @@ type FeatureBit private (bitArray) =
 
     // --- equality and comparison members ----
     member this.Equals(o: FeatureBit) =
-        if this.BitArray.Length <> o.BitArray.Length then false else
-        let mutable result = true
-        for i in 0..this.BitArray.Length - 1 do
-            if this.BitArray.[i] <> o.BitArray.[i] then
-                result <- false
-        result
+        this.ByteArray = o.ByteArray
+
     interface IEquatable<FeatureBit> with
         member this.Equals(o: FeatureBit) = this.Equals(o)
     override this.Equals(other: obj) =

src/DotNetLightning.Core/Serialize/Msgs/Msgs.fs

@@ -9,6 +9,7 @@ open NBitcoin
 open DotNetLightning.Utils
 open DotNetLightning.Utils.Primitives
 open DotNetLightning.Utils.OnionError
+open DotNetLightning.Core.Utils.Extensions
 
 open DotNetLightning.Serialize
 
@@ -368,7 +369,8 @@ type Init =
         interface ISetupMsg
         interface ILightningSerializable<Init> with
             member this.Deserialize(ls: LightningReaderStream) =
-                // For backwards compatibility reason, we must consider legacy `global features` section. (see bolt 1)
+                // For backwards compatibility reason, we must consider legacy
+                // `global features` section. (see bolt 1)
                 let globalFeatures = ls.ReadWithLen()
                 let localFeatures = ls.ReadWithLen()
                 let oredFeatures =
@@ -389,11 +391,54 @@ type Init =
                     oredFeatures
                 this.Features <- oredFeatures |> FeatureBit.CreateUnsafe
                 this.TLVStream <- ls.ReadTLVStream() |> Array.map(InitTLV.FromGenericTLV)
+
             member this.Serialize(ls) =
-                // For backwards compatibility reason, we must consider legacy `global features` section. (see bolt 1)
-                let g: byte[] = [||]
-                ls.WriteWithLen(g)
-                ls.WriteWithLen(this.Features.ToByteArray())
+                // For legacy compatiblity we treat the VariableLengthOnion feature specially.
+                // Older versions of the lightning protocol spec had a
+                // distinction between global and local features, of which
+                // VariableLengthOnion was the only global feature. Although
+                // this distinction has since been removed, older clients still
+                // expect VariableLengthOnion to appear only in the
+                // global_features field of an init message and other features
+                // to appear only in the local_features field. This is still
+                // compatible with newer clients since those clients simply OR
+                // the two feature fields.
+                let localFeatures =
+                    let localFeatures = this.Features
+                    localFeatures.SetFeature
+                        Feature.VariableLengthOnion
+                        FeaturesSupport.Mandatory
+                        false
+                    localFeatures.SetFeature
+                        Feature.VariableLengthOnion
+                        FeaturesSupport.Optional
+                        false
+                    localFeatures.ToByteArray()
+                let globalFeatures =
+                    let globalFeatures = FeatureBit.Zero
+                    let mandatory =
+                        this.Features.HasFeature(
+                            Feature.VariableLengthOnion,
+                            FeaturesSupport.Mandatory
+                        )
+                    let optional =
+                        this.Features.HasFeature(
+                            Feature.VariableLengthOnion,
+                            FeaturesSupport.Optional
+                        )
+                    if mandatory then
+                        globalFeatures.SetFeature
+                            Feature.VariableLengthOnion
+                            FeaturesSupport.Mandatory
+                            true
+                    if optional then
+                        globalFeatures.SetFeature
+                            Feature.VariableLengthOnion
+                            FeaturesSupport.Optional
+                            true
+                    globalFeatures.ToByteArray()
+                ls.WriteWithLen(globalFeatures)
+                ls.WriteWithLen(localFeatures)
                 ls.WriteTLVStream(this.TLVStream |> Array.map(fun tlv -> tlv.ToGenericTLV()))
 
 [<CLIMutable>]

src/DotNetLightning.Core/Utils/Extensions.fs

@@ -101,11 +101,34 @@ type BitArrayExtensions() =
 type System.Collections.BitArray with
     member this.ToByteArray() =
         if this.Length = 0 then [||] else
-        let ret: byte[] = Array.zeroCreate (((this.Length - 1) / 8) + 1)
-        let boolArray: bool[] = this.Reverse()
-        let t = BitArray(boolArray)
-        t.CopyTo(ret, 0)
-        ret |> Array.rev
+
+        let leadingZeros =
+            match (Seq.tryFindIndex (fun b -> b) (Seq.cast this)) with
+            | Some i -> i
+            | None -> this.Length
+        let trueLength = this.Length - leadingZeros
+        let desiredLength = ((trueLength + 7) / 8) * 8
+        let difference = desiredLength - this.Length
+        let bitArray =
+            if difference < 0 then
+                // Drop zeroes from the front of the array until we have a multiple of 8 bits
+                let shortenedBitArray = BitArray(desiredLength)
+                for i in 0 .. (desiredLength - 1) do
+                    shortenedBitArray.[i] <- this.[i - difference]
+                shortenedBitArray
+            else if difference > 0 then
+                // Push zeroes to the front of the array until we have a multiple of 8 bits
+                let lengthenedBitArray = BitArray(desiredLength)
+                for i in 0 .. (this.Length - 1) do
+                    lengthenedBitArray.[i + difference] <- this.[i]
+                lengthenedBitArray
+            else
+                this
+
+        // Copy the bit array to a byte array, then flip the bytes.
+        let byteArray: byte[] = Array.zeroCreate(desiredLength / 8)
+        bitArray.CopyTo(byteArray, 0)
+        byteArray |> Array.map (fun b -> b.FlipBit())
 
     static member From5BitEncoding(b: byte[]) =
         let bitArray = System.Collections.BitArray(b.Length * 5)
@@ -125,11 +148,10 @@ type System.Collections.BitArray with
         BitArray.From5BitEncoding(b)
 
     member this.Reverse() =
-        let length = this.Length
-        let mutable result = Array.zeroCreate length
-        for i in 0 .. (length - 1) do
-            result.[i] <- this.[length - i - 1]
-        result
+        let boolArray: array<bool> = Array.ofSeq (Seq.cast this)
+        Array.Reverse boolArray
+        BitArray(boolArray)
+
     member this.PrintBits() =
         let sb = StringBuilder()
         for b in this do

tests/DotNetLightning.Core.Tests/PaymentTests.fs

@@ -172,17 +172,17 @@ let tests =
         testCase "Please send $30 for coffee beans to the same peer, which supports features 9, 15 and 99, using secret 0x1111111111111111111111111111111111111111111111111111111111111111" <| fun _ ->
             let data = "lnbc25m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5vdhkven9v5sxyetpdeessp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9q5sqqqqqqqqqqqqqqqpqsq67gye39hfg3zd8rgc80k32tvy9xk2xunwm5lzexnvpx6fd77en8qaq424dxgt56cag2dpt359k3ssyhetktkpqh24jqnjyw6uqd08sgptq44qu"
             let pr = PaymentRequest.Parse(data) |> Result.deref
-            Expect.equal pr.PrefixValue "lnbc" ""
-            Expect.equal pr.PaymentHash ("0001020304050607080900010203040506070809000102030405060708090102" |> uint256.Parse |> PaymentHash) ""
-            Expect.equal (pr.TimestampValue.ToUnixTimeSeconds()) 1496314658L ""
-            Expect.equal (pr.NodeIdValue) ("03e7156ae33b0a208d0744199163177e909e80176e55d97a2f221ede0f934dd9ad" |> hex.DecodeData |> PubKey |> NodeId) ""
-            Expect.equal (pr.Description) (Choice1Of2 "coffee beans") ""
-            Expect.isEmpty pr.FallbackAddresses ""
-            Expect.isSome (pr.Features) ""
-            Expect.isTrue(pr.Features.Value.HasFeature(Feature.PaymentSecret, Optional)) ""
-            Expect.isTrue(pr.Features.Value.HasFeature(Feature.VariableLengthOnion, Optional)) ""
-            Expect.equal (pr.ToString()) data ""
-            Expect.equal (pr.ToString(msgSigner)) data ""
+            Expect.equal pr.PrefixValue "lnbc" "pr.PrefixValue mismatch"
+            Expect.equal pr.PaymentHash ("0001020304050607080900010203040506070809000102030405060708090102" |> uint256.Parse |> PaymentHash) "pr.PaymentHash mismatch"
+            Expect.equal (pr.TimestampValue.ToUnixTimeSeconds()) 1496314658L "pr.TimestampValue mismatch"
+            Expect.equal (pr.NodeIdValue) ("03e7156ae33b0a208d0744199163177e909e80176e55d97a2f221ede0f934dd9ad" |> hex.DecodeData |> PubKey |> NodeId) "pr.NodeIdValue mismatch"
+            Expect.equal (pr.Description) (Choice1Of2 "coffee beans") "pr.Description mismatch"
+            Expect.isEmpty pr.FallbackAddresses "pr.FallbackAddress mismatch"
+            Expect.isSome (pr.Features) "pr.Features mismatch"
+            Expect.isTrue(pr.Features.Value.HasFeature(Feature.PaymentSecret, Optional)) "pr.Features.Value (PaymentSecret) mismatch"
+            Expect.isTrue(pr.Features.Value.HasFeature(Feature.VariableLengthOnion, Optional)) "pr.Features.Value (VariableLengthOnion) mismatch"
+            Expect.equal (pr.ToString()) data "pr.ToString() mismatch"
+            Expect.equal (pr.ToString(msgSigner)) data "pr.ToString(msgSigned) mismatch"
 
         testCase "Same, but adding invalid unknown feature 100" <| fun _ ->
             let data = "lnbc25m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5vdhkven9v5sxyetpdeessp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9q4psqqqqqqqqqqqqqqqpqsqq40wa3khl49yue3zsgm26jrepqr2eghqlx86rttutve3ugd05em86nsefzh4pfurpd9ek9w2vp95zxqnfe2u7ckudyahsa52q66tgzcp6t2dyk"

tests/DotNetLightning.Core.Tests/Serialization.fs

@@ -742,8 +742,19 @@ module SerializationTest =
                 ()
 
             testProperty "BitArray serialization" <| fun (ba : NonNull<byte[]>) ->
-                Expect.sequenceEqual (BitArray.FromBytes(ba.Get).ToByteArray()) (ba.Get) ""
-                
+                let backAndForth = BitArray.FromBytes(ba.Get).ToByteArray()
+                let finalArray = Array.zeroCreate ba.Get.Length
+                Array.Copy(backAndForth, 0, finalArray, ba.Get.Length - backAndForth.Length, backAndForth.Length)
+                Expect.equal ba.Get finalArray "BitArray.ToByteArray does not invert and trim BitArray.FromBytes"
+
+            testCase "FeatureBit to/from byte array preserves byte order, bit order and trims zero bytes" <| fun _ ->
+                let features = FeatureBit.Zero
+                features.ByteArray <- [| 0b00000000uy; 0b00100000uy; 0b10000010uy |]
+                Expect.equal
+                    features.ByteArray
+                    [| 0b00100000uy; 0b10000010uy |]
+                    "unexpected ByteArray value"
+
             testCase "features dependencies" <| fun _ ->
                 let testCases =
                     Map.empty