Return an error if estimated deposit sweep fee is too high (#3666)

Here we change the default behavior of the wallet maintainer (and
`maintainer-cli` tool) when the estimated deposit sweep fee exceeds the
maximum value allowed by the `Bridge`. So far, the estimated fee was
adjusted to the maximum allowed value. Now, an error is returned. That's
because this is an abnormal situation and the maintainer should rather
wait until the fee goes back to normal value than block the wallet in
the `WalletCoordinator` with something that has no chance of being
included in a block.

cmd/maintainercli.go

@@ -567,7 +567,7 @@ var estimateDepositsSweepFeeCommandDescription = "Estimates the satoshi " +
 	"assume only P2WSH deposits are part of the transaction so the " +
 	"estimation may be underpriced if the actual transaction contains " +
 	"legacy P2SH deposits. If the estimated fee exceeds the maximum fee " +
-	"allowed by the Bridge contract, the maximum fee is returned as result"
+	"allowed by the Bridge contract, an error is returned as result"
 
 var submitDepositSweepProofCommand = cobra.Command{
 	Use:              "submit-deposit-sweep-proof",

pkg/maintainer/wallet/deposit_sweep.go

@@ -340,7 +340,7 @@ func ProposeDepositsSweep(
 			perDepositMaxFee,
 		)
 		if err != nil {
-			return fmt.Errorf("cannot estimate sweep transaction fee: [%w]", err)
+			return fmt.Errorf("cannot estimate sweep transaction fee: [%v]", err)
 		}
 
 		fee = estimatedFee
@@ -414,7 +414,7 @@ func ProposeDepositsSweep(
 //   - 1 P2WPKH output
 //
 // If any of the estimated fees exceed the maximum fee allowed by the Bridge
-// contract, the maximum fee is returned as result.
+// contract, an error is returned as result.
 func EstimateDepositsSweepFee(
 	chain Chain,
 	btcChain bitcoin.Chain,
@@ -502,8 +502,11 @@ func estimateDepositsSweepFee(
 
 	// Compute the maximum possible total fee for the entire sweep transaction.
 	totalMaxFee := uint64(depositsCount) * perDepositMaxFee
-	// Make sure the proposed total fee does not exceed the maximum possible total fee.
-	totalFee = int64(math.Min(float64(totalFee), float64(totalMaxFee)))
+
+	if uint64(totalFee) > totalMaxFee {
+		return 0, 0, fmt.Errorf("estimated fee exceeds the maximum fee")
+	}
+
 	// Compute the actual sat/vbyte fee for informational purposes.
 	satPerVByteFee := math.Round(float64(totalFee) / float64(transactionSize))
 

pkg/maintainer/wallet/deposit_sweep_test.go

@@ -1,6 +1,7 @@
 package wallet_test
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/go-test/deep"
@@ -136,13 +137,15 @@ func TestProposeDepositsSweep(t *testing.T) {
 				btcChain.SetTransactionConfirmations(deposit.FundingTxHash, tbtc.DepositSweepRequiredFundingTxConfirmations)
 			}
 
-			err := tbtcChain.SetDepositSweepProposalValidationResult(
-				scenario.ExpectedDepositSweepProposal,
-				nil,
-				true,
-			)
-			if err != nil {
-				t.Fatal(err)
+			if scenario.ExpectedDepositSweepProposal != nil {
+				err := tbtcChain.SetDepositSweepProposalValidationResult(
+					scenario.ExpectedDepositSweepProposal,
+					nil,
+					true,
+				)
+				if err != nil {
+					t.Fatal(err)
+				}
 			}
 
 			btcChain.SetEstimateSatPerVByteFee(1, scenario.EstimateSatPerVByteFee)
@@ -156,13 +159,25 @@ func TestProposeDepositsSweep(t *testing.T) {
 				scenario.DepositsReferences(),
 				false,
 			)
-			if err != nil {
-				t.Fatal(err)
+
+			if !reflect.DeepEqual(scenario.ExpectedErr, err) {
+				t.Errorf(
+					"unexpected error\n"+
+						"expected: [%+v]\n"+
+						"actual:   [%+v]",
+					scenario.ExpectedErr,
+					err,
+				)
+			}
+
+			var expectedDepositSweepProposals []*tbtc.DepositSweepProposal
+			if p := scenario.ExpectedDepositSweepProposal; p != nil {
+				expectedDepositSweepProposals = append(expectedDepositSweepProposals, p)
 			}
 
 			if diff := deep.Equal(
 				tbtcChain.DepositSweepProposals(),
-				[]*tbtc.DepositSweepProposal{scenario.ExpectedDepositSweepProposal},
+				expectedDepositSweepProposals,
 			); diff != nil {
 				t.Errorf("invalid deposits: %v", diff)
 			}

pkg/maintainer/wallet/internal/test/marshaling.go

@@ -130,6 +130,10 @@ type depositSweepProposal struct {
 }
 
 func (dsp *depositSweepProposal) convert() (*tbtc.DepositSweepProposal, error) {
+	if dsp == nil {
+		return nil, nil
+	}
+
 	result := &tbtc.DepositSweepProposal{}
 
 	if len(dsp.WalletPublicKeyHash) > 0 {
@@ -179,7 +183,8 @@ func (psts *ProposeSweepTestScenario) UnmarshalJSON(data []byte) error {
 		}
 		SweepTxFee                   int64
 		EstimateSatPerVByteFee       int64
-		ExpectedDepositSweepProposal depositSweepProposal
+		ExpectedDepositSweepProposal *depositSweepProposal
+		ExpectedErr                  string
 	}
 
 	var unmarshaled proposeSweepTestScenario
@@ -237,6 +242,11 @@ func (psts *ProposeSweepTestScenario) UnmarshalJSON(data []byte) error {
 		)
 	}
 
+	// Unmarshal expected error
+	if len(unmarshaled.ExpectedErr) > 0 {
+		psts.ExpectedErr = fmt.Errorf(unmarshaled.ExpectedErr)
+	}
+
 	return nil
 }
 

pkg/maintainer/wallet/internal/test/testdata/propose_sweep_scenario_2.json

@@ -21,23 +21,5 @@
   ],
   "SweepTxFee": 0,
   "EstimateSatPerVByteFee": 26,
-  "ExpectedDepositSweepProposal": {
-    "WalletPublicKeyHash": "0x03b74d6893ad46dfdd01b9e0e3b3385f4fce2d1e",
-    "DepositsKeys": [
-      {
-        "FundingTxHash": "d91868ca43db4deb96047d727a5e782f282864fde2d9364f8c562c8998ba64bf",
-        "FundingOutputIndex": 1
-      },
-      {
-        "FundingTxHash": "a3d1781b59d5e8680772a8bb7f897c4ff0459d3465d7fa678f80a4f0ec900574",
-        "FundingOutputIndex": 0
-      },
-      {
-        "FundingTxHash": "b822b302dab7c1fcc3292782635be133538a0f803468a2d847023c24f867f479",
-        "FundingOutputIndex": 3
-      }
-    ],
-    "SweepTxFee": 903,
-    "DepositsRevealBlocks": [11, 31, 32]
-  }
+  "ExpectedErr": "cannot estimate sweep transaction fee: [estimated fee exceeds the maximum fee]"
 }

pkg/maintainer/wallet/internal/test/wallettest.go

@@ -113,6 +113,7 @@ type ProposeSweepTestScenario struct {
 	SweepTxFee                   int64
 	EstimateSatPerVByteFee       int64
 	ExpectedDepositSweepProposal *tbtc.DepositSweepProposal
+	ExpectedErr                  error
 }
 
 func (psts *ProposeSweepTestScenario) DepositsReferences() []*walletmtr.DepositReference {