Core: remove the commit fee clamp, add max fee

src/DotNetLightning.Core/Channel/Channel.fs

@@ -1113,9 +1113,17 @@ and Channel = {
             | (None, None) ->
                 Error ReceivedClosingSignedBeforeSendingOrReceivingShutdown
         let remoteChannelKeys = this.SavedChannelState.StaticChannelConfig.RemoteChannelPubKeys
-        let lastCommitFeeSatoshi =
-            this.SavedChannelState.StaticChannelConfig.FundingScriptCoin.TxOut.Value - (this.SavedChannelState.LocalCommit.PublishableTxs.CommitTx.Value.TotalOut)
-        do! checkRemoteProposedHigherFeeThanBaseFee lastCommitFeeSatoshi msg.FeeSatoshis
+
+        let! idealFee =
+            this.FirstClosingFee
+                localShutdownScriptPubKey
+                remoteShutdownScriptPubKey
+                    |> expectTransactionError
+
+        let maxFee =
+            this.SavedChannelState.StaticChannelConfig.LocalParams.MutualCloseMaxFeeMultiplier
+            * idealFee
+
         do!
             checkRemoteProposedFeeWithinNegotiatedRange
                 (List.tryHead this.NegotiatingState.LocalClosingFeesProposed)
@@ -1146,18 +1154,20 @@ and Channel = {
         if (areWeInDeal || hasTooManyNegotiationDone) then
             return this, MutualClose (finalizedTx, None)
         else
-            let lastLocalClosingFee = this.NegotiatingState.LocalClosingFeesProposed |> List.tryHead
-            let! localF = 
-                match lastLocalClosingFee with
-                | Some v -> Ok v
-                | None ->
-                    this.FirstClosingFee
-                        localShutdownScriptPubKey
-                        remoteShutdownScriptPubKey
-                    |> expectTransactionError
+            let maybeLastLocalClosingFee = this.NegotiatingState.LocalClosingFeesProposed |> List.tryHead
+            let localF = 
+                match maybeLastLocalClosingFee with
+                | Some lastLocalClosingFee -> lastLocalClosingFee
+                | None -> idealFee
+
             let nextClosingFee =
                 Channel.NextClosingFee (localF, msg.FeeSatoshis)
-            if (Some nextClosingFee = lastLocalClosingFee) then
+
+            if this.SavedChannelState.StaticChannelConfig.IsFunder && nextClosingFee > maxFee then
+                 return!
+                     Error <| ProposalExceedsMaxFee(nextClosingFee, maxFee)
+
+            if (Some nextClosingFee = maybeLastLocalClosingFee) then
                 return this, MutualClose (finalizedTx, None)
             else if (nextClosingFee = msg.FeeSatoshis) then
                 // we have reached on agreement!

src/DotNetLightning.Core/Channel/ChannelError.fs

@@ -53,8 +53,8 @@ type ChannelError =
     | FundingTxNotGiven of msg: string
     | OnceConfirmedFundingTxHasBecomeUnconfirmed of height: BlockHeight * depth: BlockHeightOffset32
     | CannotCloseChannel of msg: string
-    | RemoteProposedHigherFeeThanBaseFee of baseFee: Money * proposedFee: Money
     | RemoteProposedFeeOutOfNegotiatedRange of ourPreviousFee: Money * theirPreviousFee: Money * theirNextFee: Money
+    | ProposalExceedsMaxFee of proposalFee: Money * maxFee: Money
     | NoUpdatesToSign
     | CannotSignCommitmentBeforeRevocation
     | InsufficientConfirmations of requiredDepth: BlockHeightOffset32 * currentDepth: BlockHeightOffset32
@@ -95,8 +95,8 @@ type ChannelError =
         | CannotSignCommitmentBeforeRevocation -> Ignore
         | InsufficientConfirmations(_, _) -> Ignore
         | InvalidOperationAddHTLC _ -> Ignore
-        | RemoteProposedHigherFeeThanBaseFee(_, _) -> Close
         | RemoteProposedFeeOutOfNegotiatedRange(_, _, _) -> Close
+        | ProposalExceedsMaxFee(_, _) -> Ignore
 
     member this.Message =
         match this with
@@ -125,15 +125,17 @@ type ChannelError =
             sprintf "once confirmed funding tx has become less confirmed than threshold %A! This is probably caused by reorg. current depth is: %A " height depth
         | ReceivedShutdownWhenRemoteHasUnsignedOutgoingHTLCs msg ->
             sprintf "They sent shutdown msg (%A) while they have pending unsigned HTLCs, this is protocol violation" msg
-        | RemoteProposedHigherFeeThanBaseFee(baseFee, proposedFee) ->
-            "remote proposed a closing fee higher than commitment fee of the final commitment transaction. "
-            + sprintf "commitment fee=%A; fee remote proposed=%A;" baseFee proposedFee
         | RemoteProposedFeeOutOfNegotiatedRange(ourPreviousFee, theirPreviousFee, theirNextFee) ->
             "remote proposed a closing fee which was not strictly between the previous fee that \
             we proposed and the previous fee that they proposed. "
             + sprintf
                 "our previous fee = %A; their previous fee = %A; their next fee = %A"
                 ourPreviousFee theirPreviousFee theirNextFee
+        | ProposalExceedsMaxFee(proposalFee, maxFee) ->
+            sprintf
+                "latest fee proposal (%i) exceeds max fee (%i)"
+                proposalFee.Satoshi
+                maxFee.Satoshi
         | CryptoError cryptoError ->
             sprintf "Crypto error: %s" cryptoError.Message
         | TransactionRelatedErrors transactionErrors ->
@@ -352,12 +354,6 @@ module internal ChannelError =
     let receivedShutdownWhenRemoteHasUnsignedOutgoingHTLCs msg =
         msg |> ReceivedShutdownWhenRemoteHasUnsignedOutgoingHTLCs |> Error
 
-    let checkRemoteProposedHigherFeeThanBaseFee baseFee proposedFee =
-        if (baseFee < proposedFee) then
-            RemoteProposedHigherFeeThanBaseFee(baseFee, proposedFee) |> Error
-        else
-            Ok()
-
     let checkRemoteProposedFeeWithinNegotiatedRange (ourPreviousFeeOpt: Option<Money>)
                                                     (theirPreviousFeeOpt: Option<Money>)
                                                     (theirNextFee: Money) =

src/DotNetLightning.Core/Channel/ChannelOperations.fs

@@ -67,6 +67,11 @@ type LocalParams = {
     ToSelfDelay: BlockHeightOffset16
     MaxAcceptedHTLCs: uint16
     Features: FeatureBits
+    // MutualCloseMaxFeeMultiplier is a multiplier we'll apply to the ideal fee
+    // of the funder, to decide when the negotiated fee is too high. By
+    // default, we want to bail out if we attempt to negotiate a fee that's
+    // 3x higher than our ideal fee.
+    MutualCloseMaxFeeMultiplier: int
 }
 
 type RemoteParams = {

src/DotNetLightning.Core/Utils/LNMoney.fs

@@ -48,23 +48,30 @@ type LNMoney = | LNMoney of int64 with
     static member Satoshis(satoshis: decimal) =
         LNMoney.FromUnit(satoshis * (decimal LNMoneyUnit.Satoshi), LNMoneyUnit.MilliSatoshi)
 
+    static member MilliSatoshis(sats: int64) =
+        LNMoney sats
+
+    static member MilliSatoshis(sats: uint64) =
+        LNMoney(Checked.int64 sats)
+
+    static member MilliSatoshis(sats: int) =
+        LNMoney(Checked.int64 sats)
+
+    static member MilliSatoshis(sats: uint32) =
+        LNMoney(Checked.int64 sats)
+
     static member Satoshis(sats: int64) =
         LNMoney.MilliSatoshis(Checked.op_Multiply 1000L sats)
 
-    static member inline Satoshis(sats) =
-        LNMoney.Satoshis(int64 sats)
-
     static member Satoshis(sats: uint64) =
         LNMoney.MilliSatoshis(Checked.op_Multiply 1000UL sats)
 
-    static member MilliSatoshis(sats: int64) =
-        LNMoney(sats)
+    static member Satoshis(sats: int) =
+        LNMoney.Satoshis(Checked.int64 sats)
 
-    static member inline MilliSatoshis(sats) =
-        LNMoney(int64 sats)
+    static member Satoshis(sats: uint32) =
+        LNMoney.Satoshis(Checked.int64 sats)
 
-    static member MilliSatoshis(sats: uint64) =
-        LNMoney(Checked.int64 sats)
 
     static member Zero = LNMoney(0L)
     static member One = LNMoney(1L)

tests/DotNetLightning.Core.Tests/TransactionTests.fs

@@ -63,6 +63,7 @@ let testList = testList "transaction tests" [
             ToSelfDelay = 144us |> BlockHeightOffset16
             MaxAcceptedHTLCs = 1000us
             Features = FeatureBits.Zero
+            MutualCloseMaxFeeMultiplier = 3
         }
 
         let remoteLocalParam : LocalParams = {
@@ -73,6 +74,7 @@ let testList = testList "transaction tests" [
             ToSelfDelay = 144us |> BlockHeightOffset16
             MaxAcceptedHTLCs = 1000us
             Features = FeatureBits.Zero
+            MutualCloseMaxFeeMultiplier = 3
         }
 
         let remoteParam : RemoteParams = {

tests/EventAggregator.Tests/EventAggregator.Tests.fsproj

@@ -12,10 +12,10 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="Expecto" Version="8.*" />
-    <PackageReference Include="FSharp.Core" Version="4.*" />
-    <PackageReference Include="YoloDev.Expecto.TestSdk" Version="0.*" />
-    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.*" />
+    <PackageReference Include="Expecto" Version="8.13.2" />
+    <PackageReference Include="FSharp.Core" Version="4.7.2" />
+    <PackageReference Include="YoloDev.Expecto.TestSdk" Version="0.12.20" />
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.11.0" />
   </ItemGroup>
 
   <ItemGroup>