Add TxOut.LexicographicCompare method & test

Ordering txouts by their byte representation doesn't give a valid
lexicographic ordering due to (a) the value being represented in
little-endian (BIP69 requires that txouts with smaller values appear
first) and (b) F#'s comparision function on byte arrays always ordering
smaller arrays first (eg. [| 1 |] < [| 0; 0 |] < [| 1; 1 |]), rather
than ordering them in dictionary order (ie. [| 0; 0 |] < [| 1 |]
< [| 1; 1 |]).

This commit adds a TxOut.LexicographicCompare method which correctly
implements BIP69's txout lexicographic ordering spec. This is then used
for ordering outputs in the closing transaction.

Backported from upstream PR:
joemphilips#138

src/DotNetLightning.Core/Transactions/Transactions.fs

@@ -777,19 +777,19 @@ module Transactions =
             let outputs =
                 seq {
                     if toLocalAmount >= dustLimit then
-                        yield (toLocalAmount, localDestination)
+                        yield TxOut(toLocalAmount, localDestination)
                     if toRemoteAmount >= dustLimit then
-                        yield (toRemoteAmount, remoteDestination)
+                        yield TxOut(toRemoteAmount, remoteDestination)
                 }
-                |> Seq.sortBy (fun (money, dest) -> TxOut(money, dest).ToBytes())
+                |> Seq.sortWith TxOut.LexicographicCompare
             let psbt = 
                 let txb = (createTransactionBuilder network)
                            .AddCoins(commitTxInput)
                            .SendFees(closingFee)
                            .SetLockTime(!> 0u)
-                for (money, dest) in outputs do
-                    txb.Send(dest, money) |> ignore
-                let tx =  txb.BuildTransaction(false)
+                for txOut in outputs do
+                    txb.Send(txOut.ScriptPubKey, txOut.Value) |> ignore
+                let tx = txb.BuildTransaction(false)
                 tx.Version <- 2u
                 tx.Inputs.[0].Sequence <- !> UINT32_MAX
                 PSBT.FromTransaction(tx, network)

src/DotNetLightning.Core/Utils/NBitcoinExtensions.fs

@@ -22,6 +22,32 @@ module NBitcoinExtensions =
             res.[31] <- res.[31] ^^^ (uint8 (this.N >>> 0) &&& 0xffuy)
             res  |> uint256 |> ChannelId
 
+    type TxOut with
+        static member LexicographicCompare (txOut0: TxOut)
+                                           (txOut1: TxOut)
+                                               : int =
+            if txOut0.Value < txOut1.Value then
+                -1
+            elif txOut0.Value > txOut1.Value then
+                1
+            else
+                let script0 = txOut0.ScriptPubKey.ToBytes()
+                let script1 = txOut1.ScriptPubKey.ToBytes()
+                let rec compare (index: int) =
+                    if script0.Length = index && script1.Length = index then
+                        0
+                    elif script0.Length = index then
+                        -1
+                    elif script1.Length = index then
+                        1
+                    elif script0.[index] < script1.[index] then
+                        -1
+                    elif script0.[index] > script1.[index] then
+                        1
+                    else
+                        compare (index + 1)
+                compare 0
+
     type PSBT with
         member this.GetMatchingSig(pubkey: PubKey) =
             this.Inputs

tests/DotNetLightning.Core.Tests/DotNetLightning.Core.Tests.fsproj

@@ -23,6 +23,7 @@
     <Compile Include="GeneratorsTests.fs" />
     <Compile Include="GraphTests.fs" />
     <Compile Include="RevocationSetTests.fs" />
+    <Compile Include="TxOutLexicographicCompareTests.fs" />
     <Compile Include="RouteCalculationTests.fs" />
     <Compile Include="TransactionBolt3TestVectorTests.fs" />
     <Compile Include="ClaimReceivedHTLCTests.fs" />

tests/DotNetLightning.Core.Tests/TxOutLexicographicCompareTests.fs

@@ -0,0 +1,86 @@
+module TxOutLexicographicCompareTests
+
+open NBitcoin
+open Expecto
+open DotNetLightning.Utils
+
+[<Tests>]
+let tests =
+    let test (amount0: int)
+             (amount1: int)
+             (script0: array<byte>)
+             (script1: array<byte>)
+             (expected: int) =
+        let txOut0 = TxOut(Money amount0, Script script0)
+        let txOut1 = TxOut(Money amount1, Script script1)
+        let result = TxOut.LexicographicCompare txOut0 txOut1
+        Expect.equal
+            result
+            expected
+            (sprintf
+                "unexpected comparison result: \
+                TxOut.LexicographicCompare(TxOut(%i, %A), TxOut(%i, %A))"
+                amount0
+                script0
+                amount1
+                script1
+            )
+
+    testList "TxOutLexicographicCompare tests" [
+        testCase "smaller amount, shorter pubkey, lower bytes values" <| fun _ ->
+            test 1 2 [| 0uy |] [| 1uy; 1uy |] -1
+        testCase "smaller amount, shorter pubkey, equal bytes values" <| fun _ ->
+            test 1 2 [| 0uy |] [| 0uy; 0uy |] -1
+        testCase "smaller amount, shorter pubkey, greater bytes values" <| fun _ ->
+            test 1 2 [| 1uy |] [| 0uy; 0uy |] -1
+        testCase "smaller amount, equal lengths, lower bytes values" <| fun _ ->
+            test 1 2 [| 0uy |] [| 1uy |] -1
+        testCase "smaller amount, equal lengths, equal bytes values" <| fun _ ->
+            test 1 2 [| 0uy |] [| 0uy |] -1
+        testCase "smaller amount, equal lengths, greater bytes values" <| fun _ ->
+            test 1 2 [| 1uy |] [| 0uy |] -1
+        testCase "smaller amount, longer pubkey, lower bytes values" <| fun _ ->
+            test 1 2 [| 0uy; 0uy |] [| 1uy |] -1
+        testCase "smaller amount, longer pubkey, equal bytes values" <| fun _ ->
+            test 1 2 [| 0uy; 0uy |] [| 0uy |] -1
+        testCase "smaller amount, longer pubkey, greater bytes values" <| fun _ ->
+            test 1 2 [| 1uy; 1uy |] [| 0uy |] -1
+
+        testCase "same amount, shorter pubkey, lower bytes values" <| fun _ ->
+            test 1 1 [| 0uy |] [| 1uy; 1uy |] -1
+        testCase "same amount, shorter pubkey, equal bytes values" <| fun _ ->
+            test 1 1 [| 0uy |] [| 0uy; 0uy |] -1
+        testCase "same amount, shorter pubkey, greater bytes values" <| fun _ ->
+            test 1 1 [| 1uy |] [| 0uy; 0uy |] 1
+        testCase "same amount, equal lengths, lower bytes values" <| fun _ ->
+            test 1 1 [| 0uy |] [| 1uy |] -1
+        testCase "same amount, equal lengths, equal bytes values" <| fun _ ->
+            test 1 1 [| 0uy |] [| 0uy |] 0
+        testCase "same amount, equal lengths, greater bytes values" <| fun _ ->
+            test 1 1 [| 1uy |] [| 0uy |] 1
+        testCase "same amount, longer pubkey, lower bytes values" <| fun _ ->
+            test 1 1 [| 0uy; 0uy |] [| 1uy |] -1
+        testCase "same amount, longer pubkey, equal bytes values" <| fun _ ->
+            test 1 1 [| 0uy; 0uy |] [| 0uy |] 1
+        testCase "same amount, longer pubkey, greater bytes values" <| fun _ ->
+            test 1 1 [| 1uy; 1uy |] [| 0uy |] 1
+
+        testCase "greater amount, shorter pubkey, lower bytes values" <| fun _ ->
+            test 2 1 [| 0uy |] [| 1uy; 1uy |] 1
+        testCase "greater amount, shorter pubkey, equal bytes values" <| fun _ ->
+            test 2 1 [| 0uy |] [| 0uy; 0uy |] 1
+        testCase "greater amount, shorter pubkey, greater bytes values" <| fun _ ->
+            test 2 1 [| 1uy |] [| 0uy; 0uy |] 1
+        testCase "greater amount, equal lengths, lower bytes values" <| fun _ ->
+            test 2 1 [| 0uy |] [| 1uy |] 1
+        testCase "greater amount, equal lengths, equal bytes values" <| fun _ ->
+            test 2 1 [| 0uy |] [| 0uy |] 1
+        testCase "greater amount, equal lengths, greater bytes values" <| fun _ ->
+            test 2 1 [| 1uy |] [| 0uy |] 1
+        testCase "greater amount, longer pubkey, lower bytes values" <| fun _ ->
+            test 2 1 [| 0uy; 0uy |] [| 1uy |] 1
+        testCase "greater amount, longer pubkey, equal bytes values" <| fun _ ->
+            test 2 1 [| 0uy; 0uy |] [| 0uy |] 1
+        testCase "greater amount, longer pubkey, greater bytes values" <| fun _ ->
+            test 2 1 [| 1uy; 1uy |] [| 0uy |] 1
+    ]