diff --git a/src/Stratis.FederatedPeg.Features.FederationGateway/Interfaces/ICrossChainTransfer.cs b/src/Stratis.FederatedPeg.Features.FederationGateway/Interfaces/ICrossChainTransfer.cs index 723ebfa7b43..e09a0a6a55e 100644 --- a/src/Stratis.FederatedPeg.Features.FederationGateway/Interfaces/ICrossChainTransfer.cs +++ b/src/Stratis.FederatedPeg.Features.FederationGateway/Interfaces/ICrossChainTransfer.cs @@ -7,6 +7,7 @@ namespace Stratis.FederatedPeg.Features.FederationGateway.Interfaces /// public enum CrossChainTransferStatus { + Suspended = 'U', Partial = 'P', FullySigned = 'F', SeenInBlock = 'S', diff --git a/src/Stratis.FederatedPeg.Features.FederationGateway/Interfaces/ICrossChainTransferStore.cs b/src/Stratis.FederatedPeg.Features.FederationGateway/Interfaces/ICrossChainTransferStore.cs index 14b1ad8d427..1f0a8a1091e 100644 --- a/src/Stratis.FederatedPeg.Features.FederationGateway/Interfaces/ICrossChainTransferStore.cs +++ b/src/Stratis.FederatedPeg.Features.FederationGateway/Interfaces/ICrossChainTransferStore.cs @@ -25,7 +25,7 @@ public interface ICrossChainTransferStore : IDisposable /// Records the mature deposits from on the counter-chain. /// The value of is incremented at the end of this call. /// - /// The deposits. + /// The deposits in order of occurrence on the source chain. /// /// The transfers are set to of /// or depending on whether enough funds are available in the federation wallet. diff --git a/src/Stratis.FederatedPeg.Features.FederationGateway/Interfaces/IFederationWalletManager.cs b/src/Stratis.FederatedPeg.Features.FederationGateway/Interfaces/IFederationWalletManager.cs index ad04e886518..262d9ef8fcf 100644 --- a/src/Stratis.FederatedPeg.Features.FederationGateway/Interfaces/IFederationWalletManager.cs +++ b/src/Stratis.FederatedPeg.Features.FederationGateway/Interfaces/IFederationWalletManager.cs @@ -61,6 +61,13 @@ public interface IFederationWalletManager /// A value indicating whether this transaction affects the wallet. bool ProcessTransaction(Transaction transaction, int? blockHeight = null, Block block = null, bool isPropagated = true); + /// + /// Removes a transaction not yet broadcasted or included in a block. + /// + /// The transaction to remove. + /// A value indicating whether this transaction affects the wallet. + bool RemoveTransaction(Transaction transaction); + /// /// Saves the wallet into the file system. /// diff --git a/src/Stratis.FederatedPeg.Features.FederationGateway/TargetChain/CrossChainTransfer.cs b/src/Stratis.FederatedPeg.Features.FederationGateway/TargetChain/CrossChainTransfer.cs index 18dce503fc5..f63816687b9 100644 --- a/src/Stratis.FederatedPeg.Features.FederationGateway/TargetChain/CrossChainTransfer.cs +++ b/src/Stratis.FederatedPeg.Features.FederationGateway/TargetChain/CrossChainTransfer.cs @@ -88,6 +88,9 @@ public void ReadWrite(BitcoinStream stream) stream.ReadWrite(ref this.depositAmount); stream.ReadWrite(ref this.partialTransaction); + if (!stream.Serializing && this.partialTransaction.Inputs.Count == 0 && this.partialTransaction.Outputs.Count == 0) + this.partialTransaction = null; + if (this.status == CrossChainTransferStatus.SeenInBlock) { stream.ReadWrite(ref this.blockHash); @@ -98,7 +101,13 @@ public void ReadWrite(BitcoinStream stream) /// public bool IsValid() { - if (this.depositTransactionId == null || this.PartialTransaction == null || this.depositTargetAddress == null || this.depositAmount == 0) + if (this.depositTransactionId == null || this.depositTargetAddress == null || this.depositAmount == 0) + return false; + + if (this.status == CrossChainTransferStatus.Suspended) + return true; + + if (this.PartialTransaction == null) return false; if (this.status == CrossChainTransferStatus.SeenInBlock && this.blockHash == null) diff --git a/src/Stratis.FederatedPeg.Features.FederationGateway/TargetChain/CrossChainTransferStore.cs b/src/Stratis.FederatedPeg.Features.FederationGateway/TargetChain/CrossChainTransferStore.cs index 876a9fb82cc..245e12f067d 100644 --- a/src/Stratis.FederatedPeg.Features.FederationGateway/TargetChain/CrossChainTransferStore.cs +++ b/src/Stratis.FederatedPeg.Features.FederationGateway/TargetChain/CrossChainTransferStore.cs @@ -189,12 +189,15 @@ private ICrossChainTransfer[] ValidateCrossChainTransfers(ICrossChainTransfer[] var tracker = new StatusChangeTracker(); foreach (CrossChainTransfer partialTransfer in crossChainTransfers) { - // Verify that the transaction input UTXO's have been reserved by the wallet. - if (partialTransfer.Status == CrossChainTransferStatus.Partial || partialTransfer.Status == CrossChainTransferStatus.FullySigned) + if (partialTransfer != null) { - if (!ValidateTransaction(partialTransfer.PartialTransaction, wallet, partialTransfer.Status == CrossChainTransferStatus.FullySigned)) + // Verify that the transaction input UTXO's have been reserved by the wallet. + if (partialTransfer.Status == CrossChainTransferStatus.Partial || partialTransfer.Status == CrossChainTransferStatus.FullySigned) { - tracker.SetTransferStatus(partialTransfer, CrossChainTransferStatus.Rejected); + if (!ValidateTransaction(partialTransfer.PartialTransaction, wallet, partialTransfer.Status == CrossChainTransferStatus.FullySigned)) + { + tracker.SetTransferStatus(partialTransfer, CrossChainTransferStatus.Rejected); + } } } } @@ -217,6 +220,17 @@ private ICrossChainTransfer[] ValidateCrossChainTransfers(ICrossChainTransfer[] this.UpdateLookups(tracker); + // Remove any remnants of the transaction from the wallet. + foreach (KeyValuePair kv in tracker) + { + if (kv.Value == CrossChainTransferStatus.Rejected) + { + this.federationWalletManager.RemoveTransaction(kv.Key.PartialTransaction); + } + } + + this.federationWalletManager.SaveWallet(); + return crossChainTransfers; } catch (Exception err) @@ -290,65 +304,122 @@ public Task RecordLatestMatureDepositsAsync(IDeposit[] deposits) return Task.Run(() => { + this.logger.LogTrace("()"); + + this.Synchronize(); + FederationWallet wallet = this.federationWalletManager.GetWallet(); - this.logger.LogTrace("()"); + ICrossChainTransfer[] transfers = this.ValidateCrossChainTransfers(this.Get(deposits.Select(d => d.Id).ToArray())); - using (DBreeze.Transactions.Transaction dbreezeTransaction = this.DBreeze.GetTransaction()) - { - dbreezeTransaction.SynchronizeTables(transferTableName, commonTableName); + var tracker = new StatusChangeTracker(); + bool walletUpdated = false; - // Check if the deposits already exist which could happen if it was found on the chain. - CrossChainTransfer[] transfers = this.Get(dbreezeTransaction, deposits.Select(d => d.Id).ToArray()); - int currentDepositHeight = this.NextMatureDepositHeight; + // Deposits are assumed to be in order of occurrence on the source chain. + // If we fail to build a transacion the transfer and subsequent transfers + // in the orderd list will be set to suspended. + bool haveSuspendedTransfers = false; - try + for (int i = 0; i < deposits.Length; i++) + { + // Check if the deposits already exist which could happen if it was found on the chain. + if (transfers[i] == null || transfers[i].Status == CrossChainTransferStatus.Suspended) { - var tracker = new StatusChangeTracker(); + IDeposit deposit = deposits[i]; - for (int i = 0; i < deposits.Length; i++) - { - IDeposit deposit = deposits[i]; - Script scriptPubKey = BitcoinAddress.Create(deposit.TargetAddress, this.network).ScriptPubKey; + Transaction transaction = null; + CrossChainTransferStatus status = CrossChainTransferStatus.Suspended; + Script scriptPubKey = BitcoinAddress.Create(deposit.TargetAddress, this.network).ScriptPubKey; + if (!haveSuspendedTransfers) + { var recipient = new Recipient { Amount = deposit.Amount, ScriptPubKey = scriptPubKey }; - if (transfers[i] == null) + transaction = BuildDeterministicTransaction(deposit.Id, recipient); + + if (transaction != null) { - Transaction transaction = BuildDeterministicTransaction(deposit.Id, recipient); + // Reserve the UTXOs before building the next transaction. + walletUpdated |= this.federationWalletManager.ProcessTransaction(transaction, isPropagated: false); - if (transaction == null) - throw new Exception("Failed to build transaction"); + status = CrossChainTransferStatus.Partial; + } + else + { + haveSuspendedTransfers = true; + } + } - transfers[i] = new CrossChainTransfer((transaction != null) ? CrossChainTransferStatus.Partial : CrossChainTransferStatus.Rejected, - deposit.Id, scriptPubKey, deposit.Amount, transaction, 0, -1 /* Unknown */); + if (transfers[i] == null) + { + transfers[i] = new CrossChainTransfer(status, deposit.Id, scriptPubKey, deposit.Amount, transaction, 0, -1 /* Unknown */); - tracker.SetTransferStatus(transfers[i]); + tracker.SetTransferStatus(transfers[i]); + } + else if (transaction != null) + { + transfers[i].SetPartialTransaction(transaction); + tracker.SetTransferStatus(transfers[i], CrossChainTransferStatus.Partial); + } + else + { + // If we can't fix suspended transfers then exit this loop now. + break; + } + } + } - this.PutTransfer(dbreezeTransaction, transfers[i]); + if (tracker.Count != 0) + { + using (DBreeze.Transactions.Transaction dbreezeTransaction = this.DBreeze.GetTransaction()) + { + dbreezeTransaction.SynchronizeTables(transferTableName, commonTableName); - // Reserve the UTXOs before building the next transaction. - this.federationWalletManager.ProcessTransaction(transaction, isPropagated: false); + int currentDepositHeight = this.NextMatureDepositHeight; + + try + { + // Update new or modified transfers. + foreach (KeyValuePair kv in tracker) + { + this.PutTransfer(dbreezeTransaction, kv.Key); } - } - // Commit additions - this.SaveNextMatureHeight(dbreezeTransaction, this.NextMatureDepositHeight + 1); + // Ensure we get called for a retry by NOT advancing the chain A tip if the block + // contained any suspended transfers. + if (!haveSuspendedTransfers) + { + this.SaveNextMatureHeight(dbreezeTransaction, this.NextMatureDepositHeight + 1); + } - dbreezeTransaction.Commit(); + dbreezeTransaction.Commit(); - // Do this last to maintain DB integrity. We are assuming that this won't throw. - this.UpdateLookups(tracker); - } - catch (Exception err) - { - // Restore expected store state in case the calling code retries / continues using the store. - this.NextMatureDepositHeight = currentDepositHeight; - this.RollbackAndThrowTransactionError(dbreezeTransaction, err, "DEPOSIT_ERROR"); + this.UpdateLookups(tracker); + } + catch (Exception err) + { + // Undo reserved UTXO's. + if (walletUpdated) + { + foreach (KeyValuePair kv in tracker) + { + if (kv.Value == CrossChainTransferStatus.Partial) + { + this.federationWalletManager.RemoveTransaction(kv.Key.PartialTransaction); + } + } + + this.federationWalletManager.SaveWallet(); + } + + // Restore expected store state in case the calling code retries / continues using the store. + this.NextMatureDepositHeight = currentDepositHeight; + this.RollbackAndThrowTransactionError(dbreezeTransaction, err, "DEPOSIT_ERROR"); + } } } @@ -889,7 +960,7 @@ public void UpdateLookups(StatusChangeTracker tracker) { this.TransferStatusUpdated(kv.Key, kv.Value); - if (kv.Key.BlockHash != 0) + if (kv.Key.BlockHash != null) { if (!this.depositIdsByBlockHash[kv.Key.BlockHash].Contains(kv.Key.DepositTransactionId)) this.depositIdsByBlockHash[kv.Key.BlockHash].Add(kv.Key.DepositTransactionId); diff --git a/src/Stratis.FederatedPeg.Features.FederationGateway/Wallet/FederationWalletManager.cs b/src/Stratis.FederatedPeg.Features.FederationGateway/Wallet/FederationWalletManager.cs index 6c93694f87f..dadfe5ac8b5 100644 --- a/src/Stratis.FederatedPeg.Features.FederationGateway/Wallet/FederationWalletManager.cs +++ b/src/Stratis.FederatedPeg.Features.FederationGateway/Wallet/FederationWalletManager.cs @@ -453,6 +453,55 @@ public bool ProcessTransaction(Transaction transaction, int? blockHeight = null, return foundSendingTrx || foundReceivingTrx; } + /// + public bool RemoveTransaction(Transaction transaction) + { + Guard.NotNull(transaction, nameof(transaction)); + uint256 hash = transaction.GetHash(); + this.logger.LogTrace("({0}:'{1}')", nameof(transaction), hash); + + bool updatedWallet = false; + + // Check the inputs - include those that have a reference to a transaction containing one of our scripts and the same index. + foreach (TxIn input in transaction.Inputs) + { + if (!this.outpointLookup.TryGetValue(input.PrevOut, out TransactionData tTx)) + { + continue; + } + + // Get the transaction being spent and unspend it. + TransactionData spentTransaction = this.Wallet.MultiSigAddress.Transactions.SingleOrDefault(t => (t.Id == tTx.Id) && (t.Index == tTx.Index)); + if (spentTransaction != null) + { + spentTransaction.SpendingDetails = null; + spentTransaction.MerkleProof = null; + updatedWallet = true; + } + } + + foreach (TxOut utxo in transaction.Outputs) + { + // Check if the outputs contain one of our addresses. + if (this.Wallet.MultiSigAddress.ScriptPubKey == utxo.ScriptPubKey) + { + int index = transaction.Outputs.IndexOf(utxo); + + // Remove any UTXO's that were provided by this transaction from wallet. + TransactionData foundTransaction = this.Wallet.MultiSigAddress.Transactions.FirstOrDefault(t => (t.Id == hash) && (t.Index == index)); + if (foundTransaction != null) + { + this.RemoveInputKeysLookupLock(foundTransaction); + this.Wallet.MultiSigAddress.Transactions.Remove(foundTransaction); + updatedWallet = true; + } + } + } + + this.logger.LogTrace("(-)"); + + return updatedWallet; + } /// /// Adds a transaction that credits the wallet with new coins. @@ -664,6 +713,19 @@ private void AddInputKeysLookupLock(TransactionData transactionData) } } + /// + /// Remove from the list of unspent outputs kept in memory for faster lookups. + /// + private void RemoveInputKeysLookupLock(TransactionData transactionData) + { + Guard.NotNull(transactionData, nameof(transactionData)); + + lock (this.lockObject) + { + this.outpointLookup.Remove(new OutPoint(transactionData.Id, transactionData.Index)); + } + } + public void TransactionFoundInternal(Script script) { this.logger.LogTrace("()"); diff --git a/src/Stratis.FederatedPeg.Tests/CrossChainTransferStoreTests.cs b/src/Stratis.FederatedPeg.Tests/CrossChainTransferStoreTests.cs index 4ace98c7f4f..65a478144aa 100644 --- a/src/Stratis.FederatedPeg.Tests/CrossChainTransferStoreTests.cs +++ b/src/Stratis.FederatedPeg.Tests/CrossChainTransferStoreTests.cs @@ -42,7 +42,7 @@ public class CrossChainTransferStoreTests private IWalletFeePolicy walletFeePolicy; private IAsyncLoopFactory asyncLoopFactory; private Dictionary blockDict; - private Transaction[] fundingTransactions; + private List fundingTransactions; private FederationWallet wallet; private ExtKey[] federationKeys; private ExtKey extendedKey; @@ -96,6 +96,8 @@ public CrossChainTransferStoreTests() SetExtendedKey(0); + this.fundingTransactions = new List(); + this.blockDict = new Dictionary(); this.blockDict[this.network.GenesisHash] = this.network.GetGenesis(); @@ -126,22 +128,27 @@ private void SetExtendedKey(int keyNum) this.withdrawalExtractor = new WithdrawalExtractor(this.loggerFactory, this.federationGatewaySettings, this.opReturnDataReader, this.network); } - private void AddFunding() + private Transaction AddFundingTransaction(Money[] amounts) { - Transaction tran1 = this.network.CreateTransaction(); - Transaction tran2 = this.network.CreateTransaction(); + Transaction transaction = this.network.CreateTransaction(); - tran1.Outputs.Add(new TxOut(Money.COIN * 90, this.wallet.MultiSigAddress.ScriptPubKey)); - tran1.Outputs.Add(new TxOut(Money.COIN * 80, this.wallet.MultiSigAddress.ScriptPubKey)); - tran2.Outputs.Add(new TxOut(Money.COIN * 70, this.wallet.MultiSigAddress.ScriptPubKey)); + foreach (Money amount in amounts) + { + transaction.Outputs.Add(new TxOut(amount, this.wallet.MultiSigAddress.ScriptPubKey)); + } - tran1.AddInput(new TxIn(new OutPoint(0, 0), new Script(OpcodeType.OP_1))); - tran2.AddInput(new TxIn(new OutPoint(0, 0), new Script(OpcodeType.OP_1))); + transaction.AddInput(new TxIn(new OutPoint(0, 0), new Script(OpcodeType.OP_1))); - this.fundingTransactions = new[] { tran1, tran2 }; + this.AppendBlock(transaction); + this.fundingTransactions.Add(transaction); - this.AppendBlock(tran1); - this.AppendBlock(tran2); + return transaction; + } + + private void AddFunding() + { + AddFundingTransaction(new Money[] { Money.COIN * 90, Money.COIN * 80 }); + AddFundingTransaction(new Money[] { Money.COIN * 70 }); } /// @@ -339,6 +346,111 @@ public void StoringDepositsWhenWalletBalanceSufficientSucceedsWithDeterministicT } } + + /// + /// Recording deposits when the wallet UTXOs are sufficient succeeds with deterministic transactions. + /// + [Fact] + public void StoringDepositsWhenWalletBalanceInSufficientSucceedsWithSuspendStatus() + { + var dataFolder = new DataFolder(CreateTestDir(this)); + + this.CreateWalletManagerAndTransactionHandler(dataFolder); + this.AddFunding(); + this.AppendBlocks(5); + + MultiSigAddress multiSigAddress = this.wallet.MultiSigAddress; + + using (var crossChainTransferStore = new CrossChainTransferStore(this.network, dataFolder, this.chain, this.federationGatewaySettings, this.dateTimeProvider, + this.loggerFactory, this.withdrawalExtractor, this.fullNode, this.blockRepository, this.federationWalletManager, this.federationWalletTransactionHandler)) + { + crossChainTransferStore.Initialize(); + crossChainTransferStore.Start(); + + Assert.Equal(this.chain.Tip.HashBlock, crossChainTransferStore.TipHashAndHeight.Hash); + Assert.Equal(this.chain.Tip.Height, crossChainTransferStore.TipHashAndHeight.Height); + + BitcoinAddress address1 = (new Key()).PubKey.Hash.GetAddress(this.network); + BitcoinAddress address2 = (new Key()).PubKey.Hash.GetAddress(this.network); + + Deposit deposit1 = new Deposit(0, new Money(160m, MoneyUnit.BTC), address1.ToString(), crossChainTransferStore.NextMatureDepositHeight, 1); + Deposit deposit2 = new Deposit(1, new Money(100m, MoneyUnit.BTC), address2.ToString(), crossChainTransferStore.NextMatureDepositHeight, 1); + + crossChainTransferStore.RecordLatestMatureDepositsAsync(new[] { deposit1, deposit2 }).GetAwaiter().GetResult(); + + ICrossChainTransfer[] transfers = crossChainTransferStore.GetAsync(new uint256[] { 0, 1 }).GetAwaiter().GetResult().ToArray(); + + Transaction[] transactions = transfers.Select(t => t.PartialTransaction).ToArray(); + + Assert.Equal(2, transactions.Length); + + // Transactions[0] inputs. + Assert.Equal(2, transactions[0].Inputs.Count); + Assert.Equal(this.fundingTransactions[0].GetHash(), transactions[0].Inputs[0].PrevOut.Hash); + Assert.Equal((uint)0, transactions[0].Inputs[0].PrevOut.N); + Assert.Equal(this.fundingTransactions[0].GetHash(), transactions[0].Inputs[1].PrevOut.Hash); + Assert.Equal((uint)1, transactions[0].Inputs[1].PrevOut.N); + + // Transaction[0] outputs. + Assert.Equal(3, transactions[0].Outputs.Count); + + // Transaction[0] output value - change. + Assert.Equal(new Money(9.99m, MoneyUnit.BTC), transactions[0].Outputs[0].Value); + Assert.Equal(multiSigAddress.ScriptPubKey, transactions[0].Outputs[0].ScriptPubKey); + + // Transaction[0] output value - recipient 1. + Assert.Equal(new Money(160m, MoneyUnit.BTC), transactions[0].Outputs[1].Value); + Assert.Equal(address1.ScriptPubKey, transactions[0].Outputs[1].ScriptPubKey); + + // Transaction[0] output value - op_return. + Assert.Equal(new Money(0m, MoneyUnit.BTC), transactions[0].Outputs[2].Value); + Assert.Equal(deposit1.Id.ToString(), new OpReturnDataReader(this.loggerFactory, this.network).TryGetTransactionId(transactions[0])); + + Assert.Null(transactions[1]); + + Assert.Equal(2, transfers.Length); + Assert.Equal(CrossChainTransferStatus.Partial, transfers[0].Status); + Assert.Equal(deposit1.Amount, new Money(transfers[0].DepositAmount)); + Assert.Equal(address1.ScriptPubKey, transfers[0].DepositTargetAddress); + Assert.Equal(CrossChainTransferStatus.Suspended, transfers[1].Status); + + // Add more funds and resubmit the deposits. + AddFundingTransaction(new Money[] { Money.COIN * 1000 }); + crossChainTransferStore.RecordLatestMatureDepositsAsync(new[] { deposit1, deposit2 }).GetAwaiter().GetResult(); + transfers = crossChainTransferStore.GetAsync(new uint256[] { 0, 1 }).GetAwaiter().GetResult().ToArray(); + transactions = transfers.Select(t => t.PartialTransaction).ToArray(); + + // Transactions[1] inputs. + Assert.Equal(2, transactions[1].Inputs.Count); + Assert.Equal(this.fundingTransactions[1].GetHash(), transactions[1].Inputs[0].PrevOut.Hash); + Assert.Equal((uint)0, transactions[1].Inputs[0].PrevOut.N); + + // Transaction[1] outputs. + Assert.Equal(3, transactions[1].Outputs.Count); + + // Transaction[1] output value - change. + Assert.Equal(new Money(969.99m, MoneyUnit.BTC), transactions[1].Outputs[0].Value); + Assert.Equal(multiSigAddress.ScriptPubKey, transactions[1].Outputs[0].ScriptPubKey); + + // Transaction[1] output value - recipient 2. + Assert.Equal(new Money(100m, MoneyUnit.BTC), transactions[1].Outputs[1].Value); + Assert.Equal(address2.ScriptPubKey, transactions[1].Outputs[1].ScriptPubKey); + + // Transaction[1] output value - op_return. + Assert.Equal(new Money(0m, MoneyUnit.BTC), transactions[1].Outputs[2].Value); + Assert.Equal(deposit2.Id.ToString(), new OpReturnDataReader(this.loggerFactory, this.network).TryGetTransactionId(transactions[1])); + + Assert.Equal(2, transfers.Length); + Assert.Equal(CrossChainTransferStatus.Partial, transfers[1].Status); + Assert.Equal(deposit2.Amount, new Money(transfers[1].DepositAmount)); + Assert.Equal(address2.ScriptPubKey, transfers[1].DepositTargetAddress); + + (Money confirmed, Money unconfirmed) spendable = this.wallet.GetSpendableAmount(); + + Assert.Equal(new Money(979.98m, MoneyUnit.BTC), spendable.unconfirmed); + } + } + /// /// Tests whether the store merges signatures as expected. ///