Core: remove the commit fee clamp, add max fee

This commit removes the check that compares mutual
close fee with commit tx fee, this check is outdated
and has been changed by this spec PR [1].

To prevent unreasonably high fee, this commit introduces
MutualCloseMaxFeeMultiplier setting.

[1] lightning/bolts#847

Author: aarani

src/DotNetLightning.Core/Channel/Channel.fs

@@ -1475,10 +1475,15 @@ and Channel =
                 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
@@ -1550,6 +1555,12 @@ and Channel =
                             nextClosingFee
                         |> expectTransactionError
 
+                    if this.SavedChannelState.StaticChannelConfig.IsFunder
+                       && nextClosingFee > maxFee then
+                        return!
+                            Error
+                            <| ProposalExceedsMaxFee(nextClosingFee, maxFee)
+
                     let nextState =
                         { this.NegotiatingState with
                             LocalClosingFeesProposed =

src/DotNetLightning.Core/Channel/ChannelError.fs

@@ -62,11 +62,11 @@ type ChannelError =
         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
@@ -108,8 +108,8 @@ type ChannelError =
         | CannotSignCommitmentBeforeRevocation -> Ignore
         | InsufficientConfirmations(_, _) -> Ignore
         | InvalidOperationAddHTLC _ -> Ignore
-        | RemoteProposedHigherFeeThanBaseFee(_, _) -> Close
         | RemoteProposedFeeOutOfNegotiatedRange(_, _, _) -> Close
+        | ProposalExceedsMaxFee(_, _) -> Ignore
 
     member this.Message =
         match this with
@@ -161,12 +161,6 @@ type ChannelError =
             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
@@ -178,6 +172,11 @@ type ChannelError =
                 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 ->
@@ -409,12 +408,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>)

src/DotNetLightning.Core/Channel/ChannelOperations.fs

@@ -80,6 +80,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 =

tests/DotNetLightning.Core.Tests/ChannelTests.fs

@@ -103,6 +103,7 @@ let tests =
                         ToSelfDelay = 144us |> BlockHeightOffset16
                         MaxAcceptedHTLCs = 1000us
                         Features = FeatureBits.Zero
+                        MutualCloseMaxFeeMultiplier = 3
                     }
 
                 let remoteParams: RemoteParams =

tests/DotNetLightning.Core.Tests/TransactionTests.fs

@@ -101,6 +101,7 @@ let testList =
                         ToSelfDelay = 144us |> BlockHeightOffset16
                         MaxAcceptedHTLCs = 1000us
                         Features = FeatureBits.Zero
+                        MutualCloseMaxFeeMultiplier = 3
                     }
 
                 let remoteLocalParam: LocalParams =
@@ -112,6 +113,7 @@ let testList =
                         ToSelfDelay = 144us |> BlockHeightOffset16
                         MaxAcceptedHTLCs = 1000us
                         Features = FeatureBits.Zero
+                        MutualCloseMaxFeeMultiplier = 3
                     }
 
                 let remoteParam: RemoteParams =
@@ -532,6 +534,7 @@ let testList =
                         ToSelfDelay = 144us |> BlockHeightOffset16
                         MaxAcceptedHTLCs = 1000us
                         Features = FeatureBits.Zero
+                        MutualCloseMaxFeeMultiplier = 3
                     }
 
                 let remoteLocalParam: LocalParams =
@@ -543,6 +546,7 @@ let testList =
                         ToSelfDelay = 144us |> BlockHeightOffset16
                         MaxAcceptedHTLCs = 1000us
                         Features = FeatureBits.Zero
+                        MutualCloseMaxFeeMultiplier = 3
                     }
 
                 let remoteParams: RemoteParams =
@@ -887,6 +891,7 @@ let testList =
                         ToSelfDelay = 144us |> BlockHeightOffset16
                         MaxAcceptedHTLCs = 1000us
                         Features = FeatureBits.Zero
+                        MutualCloseMaxFeeMultiplier = 3
                     }
 
                 let remoteLocalParam: LocalParams =
@@ -898,6 +903,7 @@ let testList =
                         ToSelfDelay = 144us |> BlockHeightOffset16
                         MaxAcceptedHTLCs = 1000us
                         Features = FeatureBits.Zero
+                        MutualCloseMaxFeeMultiplier = 3
                     }
 
                 let remoteParam: RemoteParams =