Change BlockHeightOffset from 16bit to 32bit

It's true that, as mentioned in the code, OP_CSV only allows 16bit
offsets. However it is possible to have blocks that are more than
UINT16_MAX blocks apart, and lightning's accept_channel message contains
a 32bit block height offset for the minimum_depth field. Because of
this, the AcceptChannel type was mis-using the 32bit BlockHeight type to
represent a relative block height, even though BlockHeight is intended
to represent absolute block height.

For the sake of using types consistently I've changed BlockHeightOffset
to be 32bits and changed the type of AcceptChannel.MinimumDepth to be a
BlockHeightOffset.

Author: canndrew

src/DotNetLightning.Core/Channel/Channel.fs

@@ -253,7 +253,7 @@ module Channel =
                                       MaxHTLCValueInFlightMsat = localParams.MaxHTLCValueInFlightMSat
                                       ChannelReserveSatoshis = localParams.ChannelReserveSatoshis
                                       HTLCMinimumMSat = localParams.HTLCMinimumMSat
-                                      MinimumDepth = cs.Config.ChannelHandshakeConfig.MinimumDepth.Value |> uint32 |> BlockHeight
+                                      MinimumDepth = cs.Config.ChannelHandshakeConfig.MinimumDepth
                                       ToSelfDelay = localParams.ToSelfDelay
                                       MaxAcceptedHTLCs = localParams.MaxAcceptedHTLCs
                                       FundingPubKey = channelKeys.FundingKey.PubKey

src/DotNetLightning.Core/Channel/ChannelCommands.fs

@@ -127,7 +127,7 @@ type RemoteParams = {
                 DelayedPaymentBasePoint = msg.DelayedPaymentBasepoint
                 HTLCBasePoint = msg.HTLCBasepoint
                 Features = remoteInit.Features
-                MinimumDepth = BlockHeightOffset <| uint16 msg.MinimumDepth.Value
+                MinimumDepth = msg.MinimumDepth
             }
 
         static member FromOpenChannel (nodeId) (remoteInit: Init) (msg: OpenChannel) (channelHandshakeConfig: ChannelHandshakeConfig) =

src/DotNetLightning.Core/Channel/ChannelConstants.fs

@@ -27,11 +27,11 @@ module ChannelConstants =
 
     /// The amount of time we require our counterparty wait to claim their money (i.e. time between when
     /// we, or our watchtower, mush check for them having a broadcast a theft transaction).
-    let BREAKDOWN_TIMEOUT = BlockHeightOffset(6us * 24us * 7us) // one week
-    let MAX_LOCAL_BREAKDOWN_TIMEOUT = BlockHeightOffset(6us * 24us * 14us) // two weeks
+    let BREAKDOWN_TIMEOUT = BlockHeightOffset(6u * 24u * 7u) // one week
+    let MAX_LOCAL_BREAKDOWN_TIMEOUT = BlockHeightOffset(6u * 24u * 14u) // two weeks
 
     /// Specified in BOLT 11
-    let MIN_CLTV_EXPIRY = 9us |> BlockHeightOffset
+    let MIN_CLTV_EXPIRY = 9u |> BlockHeightOffset
 
     let MAX_CLTV_EXPIRY = BREAKDOWN_TIMEOUT
 
@@ -61,4 +61,4 @@ module ChannelConstants =
     let HTLC_SUCCESS_TX_WEIGHT = 703UL
 
     [<Literal>]
-    let HTLC_TIMEOUT_TX_WEIGHT = 663UL
+    let HTLC_TIMEOUT_TX_WEIGHT = 663UL

src/DotNetLightning.Core/Channel/ChannelError.fs

@@ -465,7 +465,7 @@ module internal AcceptChannelMsgValidation =
         let check4 = check msg.MaxAcceptedHTLCs (<) config.MinMaxAcceptedHTLCs "max accepted htlcs (%A) is less than the user specified limit (%A)"
         let check5 = check msg.DustLimitSatoshis (<) config.MinDustLimitSatoshis "dust limit satoshis (%A) is less then the user specified limit (%A)"
         let check6 = check msg.DustLimitSatoshis (>) config.MaxDustLimitSatoshis "dust limit satoshis (%A) is greater then the user specified limit (%A)"
-        let check7 = check (msg.MinimumDepth.Value) (>) (config.MaxMinimumDepth.Value |> uint32) "We consider the minimum depth (%A) to be unreasonably large. Our max minimum depth is (%A)"
+        let check7 = check msg.MinimumDepth (>) config.MaxMinimumDepth "We consider the minimum depth (%A) to be unreasonably large. Our max minimum depth is (%A)"
 
         (check1 |> Validation.ofResult) *^> check2 *^> check3 *^> check4 *^> check5 *^> check6 *^> check7
 

src/DotNetLightning.Core/Payment/PaymentRequest.fs

@@ -280,7 +280,7 @@ type TaggedField =
                 Array.blit (hopInfo.ShortChannelId.ToBytes()) 0 hopInfoBase256 33 8
                 Array.blit ((hopInfo.FeeBase.MilliSatoshi |> uint32).GetBytesBigEndian()) 0 hopInfoBase256 41 4
                 Array.blit ((hopInfo.FeeProportionalMillionths |> uint32).GetBytesBigEndian()) 0 hopInfoBase256 45 4
-                Array.blit (hopInfo.CLTVExpiryDelta.Value.GetBytesBigEndian()) 0 hopInfoBase256 49 2
+                Array.blit ((hopInfo.CLTVExpiryDelta.Value |> uint16).GetBytesBigEndian()) 0 hopInfoBase256 49 2
                 routeInfoBase256.Add(hopInfoBase256)
             let routeInfoBase32 = routeInfoBase256 |> Array.concat |>  Helpers.convert8BitsTo5
             this.WriteField(writer, routeInfoBase32)
@@ -451,7 +451,7 @@ type private Bolt11Data = {
                                         let schId = r.ReadULongBE(64) |> ShortChannelId.FromUInt64
                                         let feeBase = r.ReadULongBE(32) |> LNMoney.MilliSatoshis
                                         let feeProportional = r.ReadULongBE(32) |> uint32
-                                        let cltvExpiryDelta =  r.ReadULongBE(16) |> uint16 |> BlockHeightOffset
+                                        let cltvExpiryDelta =  r.ReadULongBE(16) |> uint32 |> BlockHeightOffset
                                         let hopInfo = { NodeId = nodeId
                                                         ShortChannelId = schId
                                                         FeeBase = feeBase

src/DotNetLightning.Core/Routing/Graph.fs

@@ -268,7 +268,7 @@ module Graph =
                 ) ""
 
     module internal RoutingHeuristics =
-        let BLOCK_TIME_TWO_MONTHS = 8640us |> BlockHeightOffset
+        let BLOCK_TIME_TWO_MONTHS = 8640u |> BlockHeightOffset
         let CAPACITY_CHANNEL_LOW = LNMoney.Satoshis(1000L)
         let CAPACITY_CHANNEL_HIGH = DotNetLightning.Channel.ChannelConstants.MAX_FUNDING_SATOSHIS.Satoshi |> LNMoney.Satoshis
 

src/DotNetLightning.Core/Routing/Router.fs

@@ -56,7 +56,7 @@ module Routing =
     /// Defined in BOLT7
     let ROUTE_MAX_LENGTH = 20
     /// Max allowed cltv for a route
-    let DEFAULT_ROUTE_MAX_CLTV = BlockHeightOffset(1008us)
+    let DEFAULT_ROUTE_MAX_CLTV = BlockHeightOffset(1008u)
     /// The default number of routes we'll search for when findRoute is called with randomize = true
     [<Literal>]
     let private DEFAULT_ROUTES_COUNT = 3

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

@@ -453,7 +453,7 @@ with
             this.ChannelReserveSatoshis <- Money.Satoshis(ls.ReadInt64(false))
             this.HTLCMinimumMsat <- LNMoney.MilliSatoshis(ls.ReadInt64(false))
             this.FeeRatePerKw <- FeeRatePerKw(ls.ReadUInt32(false))
-            this.ToSelfDelay <- BlockHeightOffset(ls.ReadUInt16(false))
+            this.ToSelfDelay <- BlockHeightOffset(ls.ReadUInt16(false) |> uint32)
             this.MaxAcceptedHTLCs <- ls.ReadUInt16(false)
             this.FundingPubKey <- ls.ReadPubKey()
             this.RevocationBasepoint <- ls.ReadPubKey()
@@ -475,7 +475,7 @@ with
             ls.Write(this.ChannelReserveSatoshis.Satoshi, false)
             ls.Write(this.HTLCMinimumMsat.MilliSatoshi, false)
             ls.Write(this.FeeRatePerKw.Value, false)
-            ls.Write(this.ToSelfDelay.Value, false)
+            ls.Write(this.ToSelfDelay.Value |> uint16, false)
             ls.Write(this.MaxAcceptedHTLCs, false)
             ls.Write(this.FundingPubKey.ToBytes())
             ls.Write(this.RevocationBasepoint.ToBytes())
@@ -493,7 +493,7 @@ type AcceptChannel = {
     mutable MaxHTLCValueInFlightMsat: LNMoney
     mutable ChannelReserveSatoshis: Money
     mutable HTLCMinimumMSat: LNMoney
-    mutable MinimumDepth: BlockHeight
+    mutable MinimumDepth: BlockHeightOffset
     mutable ToSelfDelay: BlockHeightOffset
     mutable MaxAcceptedHTLCs: uint16
     mutable FundingPubKey: PubKey
@@ -513,8 +513,8 @@ with
             this.MaxHTLCValueInFlightMsat <- ls.ReadUInt64(false) |> LNMoney.MilliSatoshis
             this.ChannelReserveSatoshis <- ls.ReadUInt64(false) |> Money.Satoshis
             this.HTLCMinimumMSat <- ls.ReadUInt64(false) |> LNMoney.MilliSatoshis
-            this.MinimumDepth <- ls.ReadUInt32(false) |> BlockHeight
-            this.ToSelfDelay <- ls.ReadUInt16(false) |> BlockHeightOffset
+            this.MinimumDepth <- ls.ReadUInt32(false) |> BlockHeightOffset
+            this.ToSelfDelay <- ls.ReadUInt16(false) |> uint32 |> BlockHeightOffset
             this.MaxAcceptedHTLCs <- ls.ReadUInt16(false)
             this.FundingPubKey <- ls.ReadPubKey()
             this.RevocationBasepoint <- ls.ReadPubKey()
@@ -532,7 +532,7 @@ with
             ls.Write(this.ChannelReserveSatoshis.Satoshi, false)
             ls.Write(this.HTLCMinimumMSat.MilliSatoshi, false)
             ls.Write(this.MinimumDepth.Value, false)
-            ls.Write(this.ToSelfDelay.Value, false)
+            ls.Write(this.ToSelfDelay.Value |> uint16, false)
             ls.Write(this.MaxAcceptedHTLCs, false)
             ls.Write(this.FundingPubKey.ToBytes())
             ls.Write(this.RevocationBasepoint.ToBytes())
@@ -1094,7 +1094,7 @@ type UnsignedChannelUpdate = {
             this.Timestamp <- ls.ReadUInt32(false)
             this.MessageFlags <- ls.ReadByte()
             this.ChannelFlags <- ls.ReadByte()
-            this.CLTVExpiryDelta <- ls.ReadUInt16(false) |> BlockHeightOffset
+            this.CLTVExpiryDelta <- ls.ReadUInt16(false) |> uint32 |> BlockHeightOffset
             this.HTLCMinimumMSat <- ls.ReadUInt64(false) |> LNMoney.MilliSatoshis
             this.FeeBaseMSat <- ls.ReadUInt32(false) |> uint64 |> LNMoney.MilliSatoshis
             this.FeeProportionalMillionths <- ls.ReadUInt32(false)
@@ -1109,7 +1109,7 @@ type UnsignedChannelUpdate = {
             ls.Write(this.Timestamp, false)
             ls.Write(this.MessageFlags)
             ls.Write(this.ChannelFlags)
-            ls.Write(this.CLTVExpiryDelta.Value, false)
+            ls.Write(this.CLTVExpiryDelta.Value |> uint16, false)
             ls.Write(this.HTLCMinimumMSat.MilliSatoshi, false)
             ls.Write(uint32 this.FeeBaseMSat.MilliSatoshi, false)
             ls.Write(uint32 this.FeeProportionalMillionths, false)

src/DotNetLightning.Core/Utils/Config.fs

@@ -16,7 +16,7 @@ type ChannelHandshakeConfig = {
 
     static member Zero =
         {
-            MinimumDepth = BlockHeightOffset 6us
+            MinimumDepth = BlockHeightOffset 6u
         }
 
 /// Optional Channel limits which are applied during channel creation.
@@ -70,7 +70,7 @@ type ChannelHandshakeLimits = {
             MinMaxAcceptedHTLCs = 0us
             MinDustLimitSatoshis = Money.Satoshis(546m)
             MaxDustLimitSatoshis = Money.Coins(21_000_000m)
-            MaxMinimumDepth = 144us |> BlockHeightOffset
+            MaxMinimumDepth = 144u |> BlockHeightOffset
             ForceChannelAnnouncementPreference = true
             MaxClosingNegotiationIterations = 20
         }

src/DotNetLightning.Core/Utils/Primitives.fs

@@ -29,35 +29,31 @@ module Primitives =
                 output.[1] <- byte d
             output
 
-   /// Absolute block height
+    /// Absolute block height
     [<Struct>]
     type BlockHeight = | BlockHeight of uint32 with
         static member Zero = 0u |> BlockHeight
         static member One = 1u |> BlockHeight
         member x.Value = let (BlockHeight v) = x in v
-        member x.AsOffset() =
-            x.Value |> Checked.uint16 |> BlockHeightOffset
         static member (+) (a: BlockHeight, b: BlockHeightOffset) =
-                a.Value + (uint32 b.Value ) |> BlockHeight
+                a.Value + b.Value |> BlockHeight
 
         static member (-) (a: BlockHeight, b: BlockHeightOffset) =
-            a.Value - (uint32 b.Value) |> BlockHeight
+            a.Value - b.Value |> BlockHeight
 
         static member (-) (a: BlockHeight, b: BlockHeight) =
-            a.Value - (b.Value) |> uint16 |> BlockHeightOffset
+            a.Value - b.Value |> BlockHeightOffset
 
     /// **Description**
     ///
-    /// Relative block height used for `OP_CSV` locks,
-    /// Since OP_CSV allow only block number of 0 ~ 65535, it is safe
-    /// to restrict into the range smaller than BlockHeight
-    and  [<Struct>] BlockHeightOffset = | BlockHeightOffset of uint16 with
+    /// Relative block height used for `OP_CSV` locks.
+    and  [<Struct>] BlockHeightOffset = | BlockHeightOffset of uint32 with
         member x.Value = let (BlockHeightOffset v) = x in v
-        static member op_Implicit (v: uint16) =
+        static member op_Implicit (v: uint32) =
             BlockHeightOffset v
-        static member One = BlockHeightOffset(1us)
-        static member Zero = BlockHeightOffset(0us)
-        static member MaxValue = UInt16.MaxValue |> BlockHeightOffset
+        static member One = BlockHeightOffset(1u)
+        static member Zero = BlockHeightOffset(0u)
+        static member MaxValue = UInt32.MaxValue |> BlockHeightOffset
         static member (+) (a: BlockHeightOffset, b: BlockHeightOffset) =
             a.Value + b.Value |> BlockHeightOffset
         static member (-) (a: BlockHeightOffset, b: BlockHeightOffset) =

src/DotNetLightning.Infrastructure/ActorManagers/ChannelManager.fs

@@ -119,7 +119,7 @@ type ChannelManager(log: ILogger<ChannelManager>,
                             found <- true
                             let txIndex = txInBlock |> fst |> TxIndexInBlock
                             let (nodeId, _) = kv.Value
-                            let depth = 1us |> BlockHeightOffset
+                            let depth = 1u |> BlockHeightOffset
                             let newValue = (nodeId, Some(txIndex, height))
                             this.FundingTxs.TryUpdate(kv.Key, newValue, kv.Value) |> ignore
                             for kv in this.Actors do

src/DotNetLightning.Infrastructure/ChainConfig.fs

@@ -77,13 +77,13 @@ type ChainConfig() =
     /// if `htlc_minimum_msat` sent by other nodes `open_channel` is larger than this.
     /// re will refuse to open channel.
     member val MaxHTLCMinimumMSat: LNMoney = LNMoney.MilliSatoshis(100L) with get, set
-    member val ToRemoteDelayBlocks: BlockHeightOffset = BlockHeightOffset(720us) with get, set
-    member val MaxToLocalDelayBlocks: BlockHeightOffset = BlockHeightOffset(2016us) with get, set
+    member val ToRemoteDelayBlocks: BlockHeightOffset = BlockHeightOffset(720u) with get, set
+    member val MaxToLocalDelayBlocks: BlockHeightOffset = BlockHeightOffset(2016u) with get, set
 
     /// The number of confirmation when we consider the funding tx is locked.
-    member val MinimumDepth = BlockHeightOffset(3us) with get, set
+    member val MinimumDepth = BlockHeightOffset(3u) with get, set
     /// We do not accept channel if other node wants to wait more than this (in block number) before funding tx is locked.
-    member val MaxMinimumDepth  = BlockHeightOffset(144us) with get, set
+    member val MaxMinimumDepth  = BlockHeightOffset(144u) with get, set
 
     // member val SmartFeeNBlocks: BlockHeight
     member val FeeBaseMSat: LNMoney = LNMoney.MilliSatoshis(1000L) with get, set

tests/DotNetLightning.Core.Tests/Generators/Msgs.fs

@@ -89,7 +89,7 @@ let openChannelGen =
         <*> (moneyGen)
         <*> (lnMoneyGen)
         <*> (FeeRatePerKw <!> Arb.generate<uint32>)
-        <*> (BlockHeightOffset <!> Arb.generate<uint16>)
+        <*> (BlockHeightOffset <!> Arb.generate<uint32>)
         <*> Arb.generate<uint16>
         <*> pubKeyGen
         <*> pubKeyGen
@@ -126,8 +126,8 @@ let acceptChannelGen =
         <*> lnMoneyGen
         <*> moneyGen
         <*> lnMoneyGen
-        <*> (Arb.generate<uint32> |> Gen.map(BlockHeight))
-        <*> (BlockHeightOffset <!> Arb.generate<uint16>)
+        <*> (BlockHeightOffset <!> Arb.generate<uint32>)
+        <*> (BlockHeightOffset <!> Arb.generate<uint32>)
         <*> Arb.generate<uint16>
         <*> pubKeyGen
         <*> pubKeyGen
@@ -420,7 +420,7 @@ let private unsignedChannelUpdateGen = gen {
     let! ts = Arb.generate<uint32>
     let! messageFlags = Arb.generate<uint8>
     let! channelFlags = Arb.generate<uint8>
-    let! cltvE = BlockHeightOffset <!> Arb.generate<uint16>
+    let! cltvE = BlockHeightOffset <!> Arb.generate<uint32>
     let! htlcMin = lnMoneyGen
     let! feeBase = lnMoneyGen
     let! feePM = Arb.generate<uint32>

tests/DotNetLightning.Core.Tests/GraphTests.fs

@@ -20,7 +20,7 @@ module Constants =
     let DEFAULT_ROUTE_PARAMS = { RouteParams.Randomize = false
                                  MaxFeeBase = LNMoney.MilliSatoshis(21000L)
                                  MaxFeePCT = 0.03
-                                 RouteMaxCLTV = 2016us |> BlockHeightOffset
+                                 RouteMaxCLTV = 2016u |> BlockHeightOffset
                                  RouteMaxLength = 6
                                  Ratios = None }
     let privKey1 = Key(hex.DecodeData("0101010101010101010101010101010101010101010101010101010101010101"))
@@ -52,7 +52,7 @@ let makeUpdateCore(shortChannelId: ShortChannelId,
                     cltvDelta: BlockHeightOffset option
                     ): (ChannelDesc * UnsignedChannelUpdate) =
     let minHtlc = Option.defaultValue Constants.DEFAULT_AMOUNT_MSAT minHtlc
-    let cltvDelta = Option.defaultValue (BlockHeightOffset(0us)) cltvDelta
+    let cltvDelta = Option.defaultValue (BlockHeightOffset(0u)) cltvDelta
     let desc = { ChannelDesc.ShortChannelId = shortChannelId
                  A = nodeid1
                  B = nodeid2 }

tests/DotNetLightning.Core.Tests/KeyRepositoryTests.fs

@@ -91,7 +91,7 @@ let tests =
                                                               fundingAmount
 
             let localDustLimit = Money.Satoshis(546L)
-            let toLocalDelay = 200us |> BlockHeightOffset
+            let toLocalDelay = 200u |> BlockHeightOffset
             let specBase = { CommitmentSpec.HTLCs = htlcMap; FeeRatePerKw = 15000u |> FeeRatePerKw;
                              ToLocal = LNMoney.MilliSatoshis(6988000000L); ToRemote =  3000000000L |> LNMoney.MilliSatoshis}
             let commitTx =
@@ -115,7 +115,7 @@ let tests =
             Expect.isTrue (commitTx3.CanExtractTransaction()) (sprintf "failed to finalize commitTx %A" commitTx3)
 
             let remoteDustLimit = Money.Satoshis(1000L)
-            let remoteDelay = 160us |> BlockHeightOffset
+            let remoteDelay = 160u |> BlockHeightOffset
             let remoteCommitTx =
                 Transactions.makeCommitTx fundingSCoin
                                           0UL

tests/DotNetLightning.Core.Tests/PaymentTests.fs

@@ -119,12 +119,12 @@ let tests =
                     ShortChannelId = ShortChannelId.FromUInt64(72623859790382856UL)
                     FeeBase = 1L |> LNMoney.MilliSatoshis
                     FeeProportionalMillionths = 20u
-                    CLTVExpiryDelta = 3us |> BlockHeightOffset }
+                    CLTVExpiryDelta = 3u |> BlockHeightOffset }
                   { ExtraHop.NodeId = "039e03a901b85534ff1e92c43c74431f7ce72046060fcf7a95c37e148f78c77255" |> hex.DecodeData |> PubKey |> NodeId
                     ShortChannelId =  217304205466536202UL |> ShortChannelId.FromUInt64
                     FeeBase = 2L |> LNMoney.MilliSatoshis
                     FeeProportionalMillionths = 30u
-                    CLTVExpiryDelta = 4us |> BlockHeightOffset }
+                    CLTVExpiryDelta = 4u |> BlockHeightOffset }
                 ]
             Expect.equal pr.RoutingInfo ([routingInfo]) ""
             Expect.equal pr.TagsValue.Fields.Length 4 ""