From 9c1d41c387533e0d392039b386b3ad5f361c0870 Mon Sep 17 00:00:00 2001
From: KhalilBellakrid <khalil.bellakrid@ledger.fr>
Date: Mon, 14 Oct 2019 23:13:21 +0200
Subject: [PATCH] Bitcoin: support choosing change address path

---
 .../keychains/CommonBitcoinLikeKeychains.cpp  |  3 +-
 .../BitcoinLikeUtxoPicker.cpp                 | 17 ++++++++--
 .../bitcoin_P2PKH_transaction_tests.cpp       | 32 +++++++++++++++++--
 3 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/core/src/wallet/bitcoin/keychains/CommonBitcoinLikeKeychains.cpp b/core/src/wallet/bitcoin/keychains/CommonBitcoinLikeKeychains.cpp
index 7779796aca..4d871252eb 100644
--- a/core/src/wallet/bitcoin/keychains/CommonBitcoinLikeKeychains.cpp
+++ b/core/src/wallet/bitcoin/keychains/CommonBitcoinLikeKeychains.cpp
@@ -176,7 +176,8 @@ namespace ledger {
                                                             uint32_t to) {
             auto maxObservableIndex = (purpose == KeyPurpose::CHANGE ? _state.maxConsecutiveChangeIndex + _state.nonConsecutiveChangeIndexes.size() : _state.maxConsecutiveReceiveIndex + _state.nonConsecutiveReceiveIndexes.size()) + _observableRange;
             auto length = std::min<size_t >(to - from, maxObservableIndex - from);
-            std::vector<BitcoinLikeKeychain::Address> result(length +1);
+            std::vector<BitcoinLikeKeychain::Address> result;
+            result.reserve(length + 1);
             for (auto i = 0; i <= length; i++) {
                 if (purpose == KeyPurpose::RECEIVE) {
                     result.push_back(derive(KeyPurpose::RECEIVE, from + i));
diff --git a/core/src/wallet/bitcoin/transaction_builders/BitcoinLikeUtxoPicker.cpp b/core/src/wallet/bitcoin/transaction_builders/BitcoinLikeUtxoPicker.cpp
index 9cdac54f15..12e6fe7d8c 100644
--- a/core/src/wallet/bitcoin/transaction_builders/BitcoinLikeUtxoPicker.cpp
+++ b/core/src/wallet/bitcoin/transaction_builders/BitcoinLikeUtxoPicker.cpp
@@ -119,8 +119,21 @@ namespace ledger {
 
             if (buddy->changeAmount > BigInt(_currency.bitcoinLikeNetworkParameters.value().DustAmount)) {
                 // TODO implement multi change
-                // TODO implement use specific change address
-                auto changeAddress = buddy->keychain->getFreshAddress(BitcoinLikeKeychain::CHANGE)->toString();
+                // If multiple change paths provided we take the last one
+                auto changeAddressIndex = Try<uint32_t>::from([&](){
+                    if (buddy->request.changePaths.empty()) {
+                        throw make_exception(api::ErrorCode::INVALID_ARGUMENT, "No change path provided by user.");
+                    }
+                    auto it = buddy->request.changePaths.back();
+                    return std::stoul(it.substr(it.find_last_of('/') + 1), nullptr, 0);
+                });
+                auto changeAddress = changeAddressIndex.isFailure() ?
+                                     buddy->keychain->getFreshAddress(BitcoinLikeKeychain::CHANGE)->toString():
+                                     buddy->keychain->getAllObservableAddresses(
+                                             BitcoinLikeKeychain::CHANGE,
+                                             changeAddressIndex.getValue(),
+                                             changeAddressIndex.getValue()
+                                     )[0]->toString();
 
                 auto amount = buddy->changeAmount;
                 auto script = BitcoinLikeScript::fromAddress(changeAddress, _currency);
diff --git a/core/test/integration/transactions/bitcoin_P2PKH_transaction_tests.cpp b/core/test/integration/transactions/bitcoin_P2PKH_transaction_tests.cpp
index 0d570ad168..fc17466152 100644
--- a/core/test/integration/transactions/bitcoin_P2PKH_transaction_tests.cpp
+++ b/core/test/integration/transactions/bitcoin_P2PKH_transaction_tests.cpp
@@ -126,8 +126,36 @@ TEST_F(BitcoinMakeP2PKHTransaction, Toto) {
     builder->sendToAddress(api::Amount::fromLong(currency, 1000), "ms8C1x7qHa3WJM986NKyx267i2LFGaHRZn");
     builder->pickInputs(api::BitcoinLikePickingStrategy::DEEP_OUTPUTS_FIRST, 0xFFFFFFFF);
     builder->setFeesPerByte(api::Amount::fromLong(currency, 10));
-    auto f = builder->build();
-    auto tx = ::wait(f);
+
+    // Change as fresh address
+    auto freshChangeAddress = bla->getKeychain()->getFreshAddress(BitcoinLikeKeychain::CHANGE);
+    auto changePath = freshChangeAddress->getDerivationPath().value_or("");
+    EXPECT_GT(changePath.size(), 0);
+    auto tx = ::wait(builder->build());
+    auto addressFoundInOutputs = [=] (const std::shared_ptr<api::BitcoinLikeTransaction> &inputTx,
+                                      const std::string &inputAddress) {
+        return std::find_if(inputTx->getOutputs().begin(),
+                            inputTx->getOutputs().end(),
+                            [&] (const std::shared_ptr<api::BitcoinLikeOutput> &out) {
+                                if (!out) {
+                                    return false;
+                                }
+                                return out->getAddress().value_or("") == inputAddress;
+                            }
+        ) != inputTx->getOutputs().end();
+    };
+    EXPECT_TRUE(addressFoundInOutputs(tx, freshChangeAddress->toString()));
+
+    // Change as chosen address
+    // Compare change address to the one set
+    auto changeAddress = bla->getKeychain()->getAllObservableAddresses(BitcoinLikeKeychain::CHANGE, 5, 5);
+    EXPECT_GT(changeAddress.size(), 0);
+    changePath = changeAddress[0]->getDerivationPath().value_or("");
+    EXPECT_GT(changePath.size(), 0);
+    builder->addChangePath(changePath);
+    tx = ::wait(builder->build());
+    EXPECT_TRUE(addressFoundInOutputs(tx, changeAddress[0]->toString()));
+
     std::cout << hex::toString(tx->serialize()) << std::endl;
     std::cout << tx->getOutputs()[0]->getAddress().value_or("NOP") << std::endl;
     auto parsedTx = BitcoinLikeTransactionBuilder::parseRawUnsignedTransaction(wallet->getCurrency(), tx->serialize(), 0);