Fix wallet to use segwit in the correct derivation path (#415)

* Preparing to remove the Bech32Address field

* Remove the IsSegwit flag and fixing all errors

* fix build errors and enable legacy wallets to work side by side

* Working coed plus UI

* fix api error message

* Make the wallet version an integer and backwards compatible

* Fix build errors on tests

* Fix incorrect script pubkey when creating an address

* Fix some more wallet tests

* All tests passing

* use select instead of drop down

* clean some comments

* remove the IsSegwit flag from the rpc client

* delete forgotten segwit flag

* fix build

src/Features/Blockcore.Features.ColdStaking/Api/Controllers/ColdStakingController.cs

@@ -148,7 +148,7 @@ public IActionResult GetColdStakingAddress([FromQuery]GetColdStakingAddressReque
 
                 var model = new GetColdStakingAddressResponse
                 {
-                    Address = request.Segwit ? address?.Bech32Address : address?.Address
+                    Address = address?.Address
                 };
 
                 if (model.Address == null)
@@ -213,7 +213,7 @@ private IActionResult SetupColdStakingInternal(SetupColdStakingRequest request,
 
                 Transaction transaction = this.ColdStakingManager.GetColdStakingSetupTransaction(
                     this.walletTransactionHandler, request.ColdWalletAddress, request.HotWalletAddress,
-                    request.WalletName, request.WalletAccount, request.WalletPassword, amount, feeAmount, request.SegwitChangeAddress, request.PayToScript, createHotAccount);
+                    request.WalletName, request.WalletAccount, request.WalletPassword, amount, feeAmount, request.PayToScript, createHotAccount);
 
                 var model = new SetupColdStakingResponse
                 {

src/Features/Blockcore.Features.ColdStaking/Api/Models/ColdStakingModels.cs

@@ -108,11 +108,6 @@ public class GetColdStakingAddressRequest
         [JsonProperty(PropertyName = "isColdWalletAddress")]
         public bool IsColdWalletAddress { get; set; }
 
-        /// <summary>
-        /// Whether to return the P2WPKH (segwit bech32) addresses, or a regular P2PKH address
-        /// </summary>
-        public bool Segwit { get; set; }
-
         /// <summary>Creates a string containing the properties of this object.</summary>
         /// <returns>A string containing the properties of the object.</returns>
         public override string ToString()
@@ -241,11 +236,6 @@ public class SetupColdStakingRequest
         [JsonProperty(PropertyName = "fees")]
         public string Fees { get; set; }
 
-        /// <summary>
-        /// Whether to send the change to a P2WPKH (segwit bech32) addresses, or a regular P2PKH address
-        /// </summary>
-        public bool SegwitChangeAddress { get; set; }
-
         /// <summary>
         /// Use script outputs (P2SH and P2WSH) for cold staking
         /// </summary>

src/Features/Blockcore.Features.ColdStaking/ColdStakingManager.cs

@@ -240,7 +240,10 @@ public HdAccount GetOrCreateColdStakingAccount(string walletName, bool isColdWal
                 accountName = HotWalletAccountName;
             }
 
-            account = wallet.AddNewAccount(walletPassword, this.dateTimeProvider.GetTimeOffset(), accountIndex, accountName);
+            HdAccount defaultAccount =  wallet.GetAccount(0);
+            int purposeField = defaultAccount.Purpose;
+
+            account = wallet.AddNewAccount(walletPassword, this.dateTimeProvider.GetTimeOffset(), purposeField, accountIndex, accountName);
 
             // Maintain at least one unused address at all times. This will ensure that wallet recovery will also work.
             IEnumerable<HdAddress> newAddresses = account.CreateAddresses(wallet.Network, 1, false);
@@ -258,9 +261,9 @@ public HdAccount GetOrCreateColdStakingAccount(string walletName, bool isColdWal
         /// track addresses under the <see cref="ColdWalletAccountIndex"/> HD account.
         /// </summary>
         /// <inheritdoc />
-        public override Wallet.Types.Wallet RecoverWallet(string password, string name, string mnemonic, DateTime creationTime, string passphrase, int? coinType = null, bool? isColdStakingWallet = false)
+        public override Wallet.Types.Wallet RecoverWallet(string password, string name, string mnemonic, DateTime creationTime, string passphrase, int? purpose = null, int? coinType = null, bool? isColdStakingWallet = false)
         {
-            Wallet.Types.Wallet wallet = base.RecoverWallet(password, name, mnemonic, creationTime, passphrase, coinType);
+            Wallet.Types.Wallet wallet = base.RecoverWallet(password, name, mnemonic, creationTime, passphrase, purpose, coinType);
 
             if (isColdStakingWallet.HasValue && isColdStakingWallet == true)
             {
@@ -324,15 +327,14 @@ internal HdAddress GetFirstUnusedColdStakingAddress(string walletName, bool isCo
         /// <param name="walletPassword">The wallet password.</param>
         /// <param name="amount">The amount to cold stake.</param>
         /// <param name="feeAmount">The fee to pay for the cold staking setup transaction.</param>
-        /// <param name="useSegwitChangeAddress">Use a segwit style change address.</param>
         /// <param name="payToScript">Indicate script staking (P2SH or P2WSH outputs).</param>
         /// <returns>The <see cref="Transaction"/> for setting up cold staking.</returns>
         /// <exception cref="WalletException">Thrown if any of the rules listed in the remarks section of this method are broken.</exception>
         /// 
 
         internal Transaction GetColdStakingSetupTransaction(IWalletTransactionHandler walletTransactionHandler,
             string coldWalletAddress, string hotWalletAddress, string walletName, string walletAccount,
-            string walletPassword, Money amount, Money feeAmount, bool useSegwitChangeAddress = false, bool payToScript = false, bool createHotAccount = true)
+            string walletPassword, Money amount, Money feeAmount, bool payToScript = false, bool createHotAccount = true)
         {
             Guard.NotNull(walletTransactionHandler, nameof(walletTransactionHandler));
             Guard.NotEmpty(coldWalletAddress, nameof(coldWalletAddress));
@@ -357,8 +359,8 @@ internal Transaction GetColdStakingSetupTransaction(IWalletTransactionHandler wa
                 hotAccount = this.GetColdStakingAccount(this.GetWalletByName(walletName), false);
             }
 
-            HdAddress coldAddress = coldAccount?.ExternalAddresses.FirstOrDefault(s => s.Address == coldWalletAddress || s.Bech32Address == coldWalletAddress);
-            HdAddress hotAddress = hotAccount?.ExternalAddresses.FirstOrDefault(s => s.Address == hotWalletAddress || s.Bech32Address == hotWalletAddress);
+            HdAddress coldAddress = coldAccount?.ExternalAddresses.FirstOrDefault(s => s.Address == coldWalletAddress);
+            HdAddress hotAddress = hotAccount?.ExternalAddresses.FirstOrDefault(s => s.Address == hotWalletAddress);
 
             bool thisIsColdWallet = coldAddress != null;
             bool thisIsHotWallet = hotAddress != null;
@@ -383,7 +385,7 @@ internal Transaction GetColdStakingSetupTransaction(IWalletTransactionHandler wa
             KeyId coldPubKeyHash = null;
 
             // Check if this is a segwit address
-            if (coldAddress?.Bech32Address == coldWalletAddress || hotAddress?.Bech32Address == hotWalletAddress)
+            if ((coldAddress != null && coldAddress.IsBip84()) || (hotAddress != null && hotAddress.IsBip84()))
             {
                 hotPubKeyHash = new BitcoinWitPubKeyAddress(hotWalletAddress, wallet.Network).Hash.AsKeyId();
                 coldPubKeyHash = new BitcoinWitPubKeyAddress(coldWalletAddress, wallet.Network).Hash.AsKeyId();
@@ -427,7 +429,6 @@ internal Transaction GetColdStakingSetupTransaction(IWalletTransactionHandler wa
                 TransactionFee = feeAmount,
                 MinConfirmations = 0,
                 Shuffle = false,
-                UseSegwitChangeAddress = useSegwitChangeAddress,
                 WalletPassword = walletPassword,
                 Recipients = new List<Recipient>() { new Recipient { Amount = amount, ScriptPubKey = destination } }
             };
@@ -500,14 +501,14 @@ internal Transaction GetColdStakingWithdrawalTransaction(IWalletTransactionHandl
             }
 
             // Prevent reusing cold stake addresses as regular withdrawal addresses.
-            if (coldAccount.ExternalAddresses.Concat(coldAccount.InternalAddresses).Any(s => s.Address == receivingAddress || s.Bech32Address == receivingAddress))
+            if (coldAccount.ExternalAddresses.Concat(coldAccount.InternalAddresses).Any(s => s.Address == receivingAddress ))
             {
                 this.logger.LogTrace("(-)[COLDSTAKE_INVALID_COLD_WALLET_ADDRESS_USAGE]");
                 throw new WalletException("You can't send the money to a cold staking cold wallet account.");
             }
 
             HdAccount hotAccount = this.GetColdStakingAccount(wallet, false);
-            if (hotAccount != null && hotAccount.ExternalAddresses.Concat(hotAccount.InternalAddresses).Any(s => s.Address == receivingAddress || s.Bech32Address == receivingAddress))
+            if (hotAccount != null && hotAccount.ExternalAddresses.Concat(hotAccount.InternalAddresses).Any(s => s.Address == receivingAddress))
             {
                 this.logger.LogTrace("(-)[COLDSTAKE_INVALID_HOT_WALLET_ADDRESS_USAGE]");
                 throw new WalletException("You can't send the money to a cold staking hot wallet account.");
@@ -531,7 +532,7 @@ internal Transaction GetColdStakingWithdrawalTransaction(IWalletTransactionHandl
             {
                 AccountReference = accountReference,
                 // Specify a dummy change address to prevent a change (internal) address from being created.
-                // Will be changed after the transacton is built and before it is signed.
+                // Will be changed after the transaction is built and before it is signed.
                 ChangeAddress = coldAccount.ExternalAddresses.First(),
                 TransactionFee = feeAmount,
                 MinConfirmations = 0,

src/Features/Blockcore.Features.ColdStaking/UI/Pages/ColdStakeDelegate.razor

@@ -159,8 +159,6 @@
 
     private decimal Fee { get; set; }
 
-    public bool SegwitChangeAddress { get; set; }
-
     public bool PayToScript { get; set; }
 
     protected override Task OnInitializedAsync()
@@ -202,8 +200,6 @@
                 this.Alert = "Segwit is not activated, can't use segwit address.";
                 return;
             }
-
-            this.SegwitChangeAddress = true;
         }
         else
         {
@@ -226,13 +222,7 @@
             return;
         }
 
-        if (this.SegwitChangeAddress != address.IsBip84())
-        {
-            this.Alert = "Trying to send to a segwit address from a non segwit wallet.";
-            return;
-        }
-
-        this.ColdWalletAddress = this.SegwitChangeAddress ? address?.Bech32Address : address?.Address;
+        this.ColdWalletAddress = address?.Address;
         if (this.ColdWalletAddress == null)
         {
             this.Alert = "The cold staking account does not exist.";
@@ -252,7 +242,6 @@
                 this.Password,
                 new Money(this.Amount, MoneyUnit.BTC),
                 new Money(this.Fee, MoneyUnit.BTC),
-                this.SegwitChangeAddress,
                 this.PayToScript);
 
             await this.BroadcasterManager.BroadcastTransactionAsync(transaction);

src/Features/Blockcore.Features.ColdStaking/UI/Pages/ColdStakeSetup.razor

@@ -179,8 +179,6 @@
 
     private decimal Fee { get; set; }
 
-    public bool SegwitChangeAddress { get; set; }
-
     public bool PayToScript { get; set; }
 
     protected override Task OnInitializedAsync()
@@ -191,10 +189,7 @@
 
         var address = this.ColdStakingManager.GetFirstUnusedColdStakingAddress(this.walletname, true);
 
-        // this.SegwitChangeAddress = this.NodeDeployments.GetFlags().ScriptFlags.HasFlag(ScriptVerify.Witness);
-        this.SegwitChangeAddress = address.IsBip84();
-
-        this.ColdWalletAddress = this.SegwitChangeAddress ? address?.Bech32Address : address?.Address;
+        this.ColdWalletAddress = address?.Address;
 
         if (this.ColdWalletAddress == null)
         {
@@ -209,7 +204,7 @@
     {
         var address = this.ColdStakingManager.GetFirstUnusedColdStakingAddress(hotWalletName, false);
 
-        this.HotWalletAddress = this.SegwitChangeAddress ? address?.Bech32Address : address?.Address;
+        this.HotWalletAddress = address?.Address;
 
         if (this.HotWalletAddress == null)
         {
@@ -261,7 +256,6 @@
                 this.Password,
                 new Money(this.Amount, MoneyUnit.BTC),
                 new Money(this.Fee, MoneyUnit.BTC),
-                this.SegwitChangeAddress,
                 this.PayToScript);
 
             await this.BroadcasterManager.BroadcastTransactionAsync(transaction);

src/Features/Blockcore.Features.ColdStaking/UI/Pages/ColdStakeWithdraw.razor

@@ -180,9 +180,7 @@
     {
         var result = this.WalletManager.GetUnusedAddress(new Wallet.Types.WalletAccountReference(this.walletname, "account 0"));
 
-        var segwit = this.NodeDeployments.GetFlags().ScriptFlags.HasFlag(ScriptVerify.Witness);
-
-        this.Address = segwit ? result.Bech32Address : result.Address;
+        this.Address = result.Address;
     }
     public void MaxAmount()
     {

src/Features/Blockcore.Features.Miner/Api/Controllers/StakingController.cs

@@ -206,7 +206,7 @@ public IActionResult StakingExpiry([FromBody] StakingExpiryRequest request)
                 {
                     foreach (HdAddress address in account.GetCombinedAddresses())
                     {
-                        if ((address.Address == request.Address) || address.Bech32Address == request.Address)
+                        if ((address.Address == request.Address))
                         {
                             address.StakingExpiry = request.StakingExpiry;
                         }
@@ -272,7 +272,7 @@ public IActionResult GetStakingNotExpired([FromBody] StakingNotExpiredRequest re
                         {
                             addressItem = new GetStakingAddressesModelItem
                             {
-                                Addresses = request.Segwit ? address.Bech32Address : address.Address,
+                                Addresses = address.Address,
                                 Expiry = address.StakingExpiry,
                                 Expired = address.StakingExpiry < DateTime.UtcNow,
                             };
@@ -288,7 +288,7 @@ public IActionResult GetStakingNotExpired([FromBody] StakingNotExpiredRequest re
                                 {
                                     addressItem = new GetStakingAddressesModelItem
                                     {
-                                        Addresses = request.Segwit ? address.Bech32Address : address.Address,
+                                        Addresses = address.Address,
                                         Expiry = address.StakingExpiry,
                                         Expired = address.StakingExpiry < DateTime.UtcNow,
                                     };

src/Features/Blockcore.Features.Miner/Api/Models/RequestModels.cs

@@ -92,7 +92,5 @@ public class StakingNotExpiredRequest : RequestModel
         /// </summary>
         [Required(ErrorMessage = "Name of wallet.")]
         public string WalletName { get; set; }
-
-        public bool Segwit { get; set; }
     }
 }

src/Features/Blockcore.Features.RPC/RPCClient.cs

@@ -1403,11 +1403,11 @@ public async Task<decimal> EstimatePriorityAsync(int nblock)
         /// <param name="commentTx">A locally-stored (not broadcast) comment assigned to this transaction. Default is no comment</param>
         /// <param name="commentDest">A locally-stored (not broadcast) comment assigned to this transaction. Meant to be used for describing who the payment was sent to. Default is no comment</param>
         /// <returns>The TXID of the sent transaction</returns>
-        public uint256 SendToAddress(BitcoinAddress address, Money amount, string commentTx = null, string commentDest = null, decimal? fee = null, bool isSegwit = false)
+        public uint256 SendToAddress(BitcoinAddress address, Money amount, string commentTx = null, string commentDest = null, decimal? fee = null)
         {
             uint256 txid = null;
 
-            txid = SendToAddressAsync(address, amount, commentTx, commentDest, fee, isSegwit).GetAwaiter().GetResult();
+            txid = SendToAddressAsync(address, amount, commentTx, commentDest, fee).GetAwaiter().GetResult();
             return txid;
         }
 
@@ -1419,7 +1419,7 @@ public uint256 SendToAddress(BitcoinAddress address, Money amount, string commen
         /// <param name="commentTx">A locally-stored (not broadcast) comment assigned to this transaction. Default is no comment</param>
         /// <param name="commentDest">A locally-stored (not broadcast) comment assigned to this transaction. Meant to be used for describing who the payment was sent to. Default is no comment</param>
         /// <returns>The TXID of the sent transaction</returns>
-        public async Task<uint256> SendToAddressAsync(BitcoinAddress address, Money amount, string commentTx = null, string commentDest = null, decimal? fee = null, bool isSegwit = false)
+        public async Task<uint256> SendToAddressAsync(BitcoinAddress address, Money amount, string commentTx = null, string commentDest = null, decimal? fee = null)
         {
             var parameters = new List<object>();
             parameters.Add(address.ToString());
@@ -1434,9 +1434,6 @@ public async Task<uint256> SendToAddressAsync(BitcoinAddress address, Money amou
             if (fee != null)
                 parameters.Add(fee.ToString());
 
-            if (isSegwit)
-                parameters.Add(isSegwit.ToString());
-
             RPCResponse resp = await SendCommandAsync(RPCOperations.sendtoaddress, parameters.ToArray()).ConfigureAwait(false);
             return uint256.Parse(resp.Result.ToString());
         }