Merge pull request #228 from NicolasDorier/validator

Move string validation logic out of WalletManager
stratisproject · Jul 13, 2017 · 2827dd5 · 2827dd5
2 parents 21c0d30 + 4f21c58
commit 2827dd5
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 22 deletions.
diff --git a/Stratis.Bitcoin.IntegrationTests/WalletTests.cs b/Stratis.Bitcoin.IntegrationTests/WalletTests.cs
@@ -51,7 +51,7 @@ public void WalletCanReceiveAndSendCorrectly()
 
                 // send coins to the receiver
                 var sendto = stratisReceiver.FullNode.WalletManager.GetUnusedAddress(new WalletAccountReference("mywallet", "account 0"));
-                var trx = stratisSender.FullNode.WalletManager.BuildTransaction(new WalletAccountReference("mywallet", "account 0"), "123456", sendto.Address, Money.COIN * 100, FeeType.Medium, 101);
+                var trx = stratisSender.FullNode.WalletManager.BuildTransaction(new WalletAccountReference("mywallet", "account 0"), "123456", sendto.ScriptPubKey, Money.COIN * 100, FeeType.Medium, 101);
 
                 // broadcast to the other node
                 stratisSender.FullNode.WalletManager.SendTransaction(trx.hex);
@@ -158,7 +158,7 @@ public void WalletCanReorg()
                 // ====================
                 // send coins to the receiver
                 var sendto = stratisReceiver.FullNode.WalletManager.GetUnusedAddress(new WalletAccountReference("mywallet", "account 0"));
-                var transaction1 = stratisSender.FullNode.WalletManager.BuildTransaction(new WalletAccountReference("mywallet", "account 0"), "123456", sendto.Address, Money.COIN * 100, FeeType.Medium, 101);
+                var transaction1 = stratisSender.FullNode.WalletManager.BuildTransaction(new WalletAccountReference("mywallet", "account 0"), "123456", sendto.ScriptPubKey, Money.COIN * 100, FeeType.Medium, 101);
 
                 // broadcast to the other node
                 stratisSender.FullNode.WalletManager.SendTransaction(transaction1.hex);
@@ -194,7 +194,7 @@ public void WalletCanReorg()
 
                 // send more coins to the wallet
                 sendto = stratisReceiver.FullNode.WalletManager.GetUnusedAddress(new WalletAccountReference("mywallet", "account 0"));
-                var transaction2 = stratisSender.FullNode.WalletManager.BuildTransaction(new WalletAccountReference("mywallet", "account 0"), "123456", sendto.Address, Money.COIN * 10, FeeType.Medium, 101);
+                var transaction2 = stratisSender.FullNode.WalletManager.BuildTransaction(new WalletAccountReference("mywallet", "account 0"), "123456", sendto.ScriptPubKey, Money.COIN * 10, FeeType.Medium, 101);
                 stratisSender.FullNode.WalletManager.SendTransaction(transaction2.hex);
                 // wait for the trx to arrive
                 TestHelper.WaitLoop(() => stratisReceiver.CreateRPCClient().GetRawMempool().Length > 0);

diff --git a/Stratis.Bitcoin.Tests/Wallet/WalletControllerTest.cs b/Stratis.Bitcoin.Tests/Wallet/WalletControllerTest.cs
@@ -1181,7 +1181,7 @@ public void BuildTransactionWithValidRequestAllowingUnconfirmedReturnsWalletBuil
         {
             var mockWalletWrapper = new Mock<IWalletManager>();
             var key = new Key();
-            mockWalletWrapper.Setup(m => m.BuildTransaction(new WalletAccountReference("myWallet", "Account 1"), "test", key.PubKey.GetAddress(Network.Main).ToString(), new Money(150000), FeeType.High, 0))
+            mockWalletWrapper.Setup(m => m.BuildTransaction(new WalletAccountReference("myWallet", "Account 1"), "test", key.PubKey.GetAddress(Network.Main).ScriptPubKey, new Money(150000), FeeType.High, 0))
                 .Returns(("hexString", new uint256(15), new Money(150)));
 
 
@@ -1211,7 +1211,7 @@ public void BuildTransactionWithValidRequestNotAllowingUnconfirmedReturnsWalletB
         {
             var mockWalletWrapper = new Mock<IWalletManager>();
             var key = new Key();
-            mockWalletWrapper.Setup(m => m.BuildTransaction(new WalletAccountReference("myWallet", "Account 1"), "test", key.PubKey.GetAddress(Network.Main).ToString(), new Money(150000), FeeType.High, 1))
+            mockWalletWrapper.Setup(m => m.BuildTransaction(new WalletAccountReference("myWallet", "Account 1"), "test", key.PubKey.GetAddress(Network.Main).ScriptPubKey, new Money(150000), FeeType.High, 1))
                 .Returns(("hexString", new uint256(15), new Money(150)));
 
             var controller = new WalletController(mockWalletWrapper.Object, new Mock<IWalletSyncManager>().Object, It.IsAny<ConnectionManager>(), Network.Main, new Mock<ConcurrentChain>().Object, It.IsAny<DataFolder>());
@@ -1262,7 +1262,7 @@ public void BuildTransactionWithExceptionReturnsBadRequest()
         {
             var mockWalletWrapper = new Mock<IWalletManager>();
             var key = new Key();
-            mockWalletWrapper.Setup(m => m.BuildTransaction(new WalletAccountReference("myWallet", "Account 1"), "test", key.PubKey.GetAddress(Network.Main).ToString(), new Money(150000), FeeType.High, 1))
+            mockWalletWrapper.Setup(m => m.BuildTransaction(new WalletAccountReference("myWallet", "Account 1"), "test", key.PubKey.GetAddress(Network.Main).ScriptPubKey, new Money(150000), FeeType.High, 1))
                 .Throws(new InvalidOperationException("Issue building transaction."));
 
             var controller = new WalletController(mockWalletWrapper.Object, new Mock<IWalletSyncManager>().Object, It.IsAny<ConnectionManager>(), Network.Main, new Mock<ConcurrentChain>().Object, It.IsAny<DataFolder>());

diff --git a/Stratis.Bitcoin/Wallet/Controllers/WalletController.cs b/Stratis.Bitcoin/Wallet/Controllers/WalletController.cs
@@ -418,10 +418,11 @@ public IActionResult BuildTransaction([FromBody] BuildTransactionRequest request
                 var errors = this.ModelState.Values.SelectMany(e => e.Errors.Select(m => m.ErrorMessage));
                 return ErrorHelpers.BuildErrorResponse(HttpStatusCode.BadRequest, "Formatting error", string.Join(Environment.NewLine, errors));
             }
-
+            var destination = BitcoinAddress.Create(request.DestinationAddress, this.network).ScriptPubKey;
+
             try
             {
-                var transactionResult = this.walletManager.BuildTransaction(new WalletAccountReference(request.WalletName, request.AccountName), request.Password, request.DestinationAddress, request.Amount, FeeParser.Parse(request.FeeType), request.AllowUnconfirmed ? 0 : 1);
+                var transactionResult = this.walletManager.BuildTransaction(new WalletAccountReference(request.WalletName, request.AccountName), request.Password, destination, request.Amount, FeeParser.Parse(request.FeeType), request.AllowUnconfirmed ? 0 : 1);
                 var model = new WalletBuildTransactionModel
                 {
                     Hex = transactionResult.hex,

diff --git a/Stratis.Bitcoin/Wallet/IWalletManager.cs b/Stratis.Bitcoin/Wallet/IWalletManager.cs
@@ -135,12 +135,12 @@ public interface IWalletManager : IDisposable
         /// </summary>
         /// <param name="accountReference">The name of the wallet and account</param>
         /// <param name="password">The password used to decrypt the private key.</param>
-        /// <param name="destinationAddress">The destination address to send the funds to.</param>
+        /// <param name="destinationScript">The destination to send the funds to.</param>
         /// <param name="amount">The amount of funds to be sent.</param>
         /// <param name="feeType">The type of fee to be included.</param>
         /// <param name="minConfirmations">The minimum number of confirmations we require for unspent outputs to be included.</param>
         /// <returns></returns>
-        (string hex, uint256 transactionId, Money fee) BuildTransaction(WalletAccountReference accountReference, string password, string destinationAddress, Money amount, FeeType feeType, int minConfirmations);
+        (string hex, uint256 transactionId, Money fee) BuildTransaction(WalletAccountReference accountReference, string password, Script destinationScript, Money amount, FeeType feeType, int minConfirmations);
 
         /// <summary>
         /// Remove all the thransactions in the wallet that are above this block height

diff --git a/Stratis.Bitcoin/Wallet/Models/RequestModels.cs b/Stratis.Bitcoin/Wallet/Models/RequestModels.cs
@@ -4,6 +4,8 @@
 using System.ComponentModel.DataAnnotations;
 using Newtonsoft.Json;
 using Newtonsoft.Json.Converters;
+using NBitcoin;
+using Stratis.Bitcoin.Wallet.Validations;
 
 namespace Stratis.Bitcoin.Wallet.Models
 {
@@ -99,6 +101,7 @@ public class BuildTransactionRequest : RequestModel
         public string Password { get; set; }
 
         [Required(ErrorMessage = "A destination address is required.")]
+        [IsBitcoinAddress()]
         public string DestinationAddress { get; set; }
 
         [Required(ErrorMessage = "An amount is required.")]

diff --git a/Stratis.Bitcoin/Wallet/Validations/IsBitcoinAddressAttribute.cs b/Stratis.Bitcoin/Wallet/Validations/IsBitcoinAddressAttribute.cs
@@ -0,0 +1,22 @@
+using NBitcoin;
+using System;
+using System.Collections.Generic;
+using System.ComponentModel.DataAnnotations;
+using System.Text;
+
+namespace Stratis.Bitcoin.Wallet.Validations
+{
+    public class IsBitcoinAddressAttribute : ValidationAttribute
+    {
+        protected override ValidationResult IsValid(object value, ValidationContext validationContext)
+        {
+            var network = (Network)validationContext.GetService(typeof(Network));
+            try
+            {
+                BitcoinAddress.Create(value as string, network);
+                return ValidationResult.Success;
+            }
+            catch { return new ValidationResult("Invalid address"); }
+        }
+    }
+}
diff --git a/Stratis.Bitcoin/Wallet/WalletManager.cs b/Stratis.Bitcoin/Wallet/WalletManager.cs
@@ -461,7 +461,7 @@ public ISecret GetKeyForAddress(string password, HdAddress address)
         }
 
         /// <inheritdoc />
-        public (string hex, uint256 transactionId, Money fee) BuildTransaction(WalletAccountReference accountReference, string password, string destinationAddress, Money amount, FeeType feeType, int minConfirmations)
+        public (string hex, uint256 transactionId, Money fee) BuildTransaction(WalletAccountReference accountReference, string password, Script destinationScript, Money amount, FeeType feeType, int minConfirmations)
         {
             if (amount == Money.Zero)
             {
@@ -472,17 +472,6 @@ public ISecret GetKeyForAddress(string password, HdAddress address)
             Wallet wallet = this.GetWalletByName(accountReference.WalletName);
             HdAccount account = this.GetAccounts(wallet).GetAccountByName(accountReference.AccountName);
 
-            // get script destination address
-            Script destinationScript = null;
-            try
-            {
-                destinationScript = PayToPubkeyHashTemplate.Instance.GenerateScriptPubKey(new BitcoinPubKeyAddress(destinationAddress, wallet.Network));
-            }
-            catch
-            {
-                throw new WalletException("Invalid address.");
-            }
-
             // get a list of transactions outputs that have not been spent
             var spendableTransactions = account.GetSpendableTransactions().ToList();