Throw error on duplicate withdrawal detected in federation wallet

src/Stratis.Features.FederatedPeg/Interfaces/IFederationWalletManager.cs

@@ -97,7 +97,8 @@ public interface IFederationWalletManager : ILockProtected
         /// <summary>
         /// Saves the wallet into the file system.
         /// </summary>
-        void SaveWallet();
+        /// <param name="force">If <c>false</c> (default) the wallet is not guaranteed to be saved if saved recently.</param>
+        void SaveWallet(bool force = false);
 
         /// <summary>
         /// Gets some general information about a wallet.

src/Stratis.Features.FederatedPeg/Wallet/FederationWalletManager.cs

@@ -61,11 +61,18 @@ public class FederationWalletManager : LockProtected, IFederationWalletManager
         /// <summary>Timer for saving wallet files to the file system.</summary>
         private const int WalletSavetimeIntervalInMinutes = 5;
 
+        /// <summary>Minimum time to wait between saving wallet if not forced.</summary>
+        private const int WalletSavetimeMinIntervalInMinutes = 1;
+
         /// <summary>Keep at least this many transactions in the wallet despite the
         /// max reorg age limit for spent transactions. This is so that it never
         /// looks like the wallet has become empty to the user.</summary>
         private const int MinimumRetainedTransactions = 100;
 
+
+        /// <summary>The last time the wallet was saved.</summary>
+        private DateTime lastWalletSave;
+
         /// <summary>The async loop we need to wait upon before we can shut down this manager.</summary>
         private IAsyncLoop asyncLoop;
 
@@ -170,6 +177,7 @@ public FederationWalletManager(
             this.isFederationActive = false;
             this.blockStore = blockStore;
             this.signals = signals;
+            this.lastWalletSave = DateTime.Now;
 
             nodeStats.RegisterStats(this.AddComponentStats, StatsType.Component, this.GetType().Name);
             nodeStats.RegisterStats(this.AddInlineStats, StatsType.Inline, this.GetType().Name, 800);
@@ -272,6 +280,7 @@ public void Start()
                 if (this.fileStorage.Exists(WalletFileName))
                 {
                     this.Wallet = this.fileStorage.LoadByFileName(WalletFileName);
+                    Guard.Assert(this.Wallet.MultiSigAddress.Address == this.federatedPegSettings.MultiSigAddress.ToString());
                     this.RemoveUnconfirmedTransactionData();
                 }
                 else
@@ -289,7 +298,7 @@ public void Start()
                 // save the wallets file every 5 minutes to help against crashes.
                 this.asyncLoop = this.asyncProvider.CreateAndRunAsyncLoop("wallet persist job", token =>
                 {
-                    this.SaveWallet();
+                    this.SaveWallet(true);
                     this.logger.LogInformation("Wallets saved to file at {0}.", this.dateTimeProvider.GetUtcNow());
 
                     return Task.CompletedTask;
@@ -306,7 +315,7 @@ public void Stop()
             lock (this.lockObject)
             {
                 this.asyncLoop?.Dispose();
-                this.SaveWallet();
+                this.SaveWallet(true);
             }
         }
 
@@ -482,9 +491,25 @@ public bool ProcessTransaction(Transaction transaction, int? blockHeight = null,
                 IWithdrawal withdrawal = this.withdrawalExtractor.ExtractWithdrawalFromTransaction(transaction, blockHash, blockHeight ?? 0);
                 if (withdrawal != null)
                 {
-                    // Exit if already present and included in a block.
+                    // Check the wallet for any existing transactions related to this deposit id.
                     List<(Transaction transaction, IWithdrawal withdrawal)> walletData = this.FindWithdrawalTransactions(withdrawal.DepositId);
-                    if ((walletData.Count == 1) && (walletData[0].withdrawal.BlockNumber != 0))
+
+                    // Already redeemed in a confirmed block?
+                    if (walletData.Count != 0 && blockHeight != null)
+                    {
+                        (_, IWithdrawal existingWithdrawal) = walletData.LastOrDefault(d => d.withdrawal.BlockNumber != 0 && d.withdrawal.Id != withdrawal.Id);
+
+                        if (existingWithdrawal != null)
+                        {
+                            string error = string.Format("Deposit '{0}' last redeemed in transaction '{1}' (height {2}) and then redeemed again in '{3}' (height {4})!.", withdrawal.DepositId, existingWithdrawal.Id, existingWithdrawal.BlockNumber, withdrawal.Id, withdrawal.BlockNumber);
+
+                            // Just display an error on the console for now.
+                            this.logger.LogError(error);
+
+                            // Fall through and keep the latest withdrawal.
+                        }
+                    }
+                    else if ((walletData.Count == 1) && (walletData[0].withdrawal.BlockNumber != 0))
                     {
                         this.logger.LogDebug("Deposit '{0}' already included in block.", withdrawal.DepositId);
                         return false;
@@ -871,16 +896,18 @@ private SpendingDetails BuildSpendingDetails(Transaction transaction,
         }
 
         /// <inheritdoc />
-        public void SaveWallet()
+        public void SaveWallet(bool force = false)
         {
             lock (this.lockObject)
             {
+                // If this is not a forced save then check that we're not saving the wallet too often.
+                if (!force && ((DateTime.Now - this.lastWalletSave) < TimeSpan.FromMinutes(WalletSavetimeMinIntervalInMinutes)))
+                    return;
+
                 if (this.Wallet != null)
                 {
-                    lock (this.lockObject)
-                    {
-                        this.fileStorage.SaveToFile(this.Wallet, WalletFileName);
-                    }
+                    this.fileStorage.SaveToFile(this.Wallet, WalletFileName);
+                    this.lastWalletSave = DateTime.Now;
                 }
             }
         }
@@ -928,7 +955,8 @@ public bool RemoveWithdrawalTransactions(uint256 depositId)
 
                 foreach ((Transaction transaction, IWithdrawal withdrawal) in this.FindWithdrawalTransactions(depositId))
                 {
-                    walletUpdated |= this.RemoveTransaction(transaction);
+                    if (withdrawal.BlockNumber == 0)
+                        walletUpdated |= this.RemoveTransaction(transaction);
                 }
 
                 return walletUpdated;