Merge c189bfd260 into merged_master (Bitcoin PR #17824)

This commit is contained in:
Andrew Poelstra 2020-11-26 01:08:22 +00:00
commit 7acaa0481b
3 changed files with 111 additions and 29 deletions

View file

@ -2525,6 +2525,13 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm
++it;
}
unsigned int limit_ancestor_count = 0;
unsigned int limit_descendant_count = 0;
chain().getPackageLimits(limit_ancestor_count, limit_descendant_count);
size_t max_ancestors = (size_t)std::max<int64_t>(1, limit_ancestor_count);
size_t max_descendants = (size_t)std::max<int64_t>(1, limit_descendant_count);
bool fRejectLongChains = gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS);
// ELEMENTS: filter coins for assets we are interested in; always keep policyAsset for fees
for (std::vector<COutput>::iterator it = vCoins.begin(); it != vCoins.end() && coin_control.HasSelected();) {
CAsset asset = it->GetInputCoin().asset;
@ -2543,14 +2550,7 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm
// explicitly shuffling the outputs before processing
Shuffle(vCoins.begin(), vCoins.end(), FastRandomContext());
}
std::vector<OutputGroup> groups = GroupOutputs(vCoins, !coin_control.m_avoid_partial_spends);
unsigned int limit_ancestor_count;
unsigned int limit_descendant_count;
chain().getPackageLimits(limit_ancestor_count, limit_descendant_count);
size_t max_ancestors = (size_t)std::max<int64_t>(1, limit_ancestor_count);
size_t max_descendants = (size_t)std::max<int64_t>(1, limit_descendant_count);
bool fRejectLongChains = gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS);
std::vector<OutputGroup> groups = GroupOutputs(vCoins, !coin_control.m_avoid_partial_spends, max_ancestors);
// We will have to do coin selection on the difference between the target and the provided values.
// If value_to_select <= 0 for all asset types, we are done; but unlike in Bitcoin, this may be
@ -5121,33 +5121,50 @@ bool CWalletTx::IsImmatureCoinBase() const
return GetBlocksToMaturity() > 0;
}
std::vector<OutputGroup> CWallet::GroupOutputs(const std::vector<COutput>& outputs, bool single_coin) const {
std::vector<OutputGroup> CWallet::GroupOutputs(const std::vector<COutput>& outputs, bool single_coin, const size_t max_ancestors) const {
std::vector<OutputGroup> groups;
std::map<std::pair<CAsset, CTxDestination>, OutputGroup> gmap;
std::pair<CAsset, CTxDestination> dst;
std::set<std::pair<CAsset, CTxDestination>> full_groups;
for (const auto& output : outputs) {
if (output.fSpendable) {
std::pair<CAsset, CTxDestination> dst;
CInputCoin input_coin = output.GetInputCoin();
dst.first = input_coin.asset;
size_t ancestors, descendants;
chain().getTransactionAncestry(output.tx->GetHash(), ancestors, descendants);
if (!single_coin && ExtractDestination(output.tx->tx->vout[output.i].scriptPubKey, dst.second)) {
// Limit output groups to no more than 10 entries, to protect
// against inadvertently creating a too-large transaction
// when using -avoidpartialspends
if (gmap[dst].m_outputs.size() >= OUTPUT_GROUP_MAX_ENTRIES) {
groups.push_back(gmap[dst]);
gmap.erase(dst);
auto it = gmap.find(dst);
if (it != gmap.end()) {
// Limit output groups to no more than OUTPUT_GROUP_MAX_ENTRIES
// number of entries, to protect against inadvertently creating
// a too-large transaction when using -avoidpartialspends to
// prevent breaking consensus or surprising users with a very
// high amount of fees.
if (it->second.m_outputs.size() >= OUTPUT_GROUP_MAX_ENTRIES) {
groups.push_back(it->second);
it->second = OutputGroup{};
full_groups.insert(dst);
}
it->second.Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants);
} else {
gmap[dst].Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants);
}
gmap[dst].Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants);
} else {
groups.emplace_back(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants);
}
}
}
if (!single_coin) {
for (const auto& it : gmap) groups.push_back(it.second);
for (auto& it : gmap) {
auto& group = it.second;
if (full_groups.count(it.first) > 0) {
// Make this unattractive as we want coin selection to avoid it if possible
group.m_ancestors = max_ancestors - 1;
}
groups.push_back(group);
}
}
return groups;
}

View file

@ -943,7 +943,7 @@ public:
bool IsSpentKey(const uint256& hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
void SetSpentKeyState(WalletBatch& batch, const uint256& hash, unsigned int n, bool used, std::set<CTxDestination>& tx_destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
std::vector<OutputGroup> GroupOutputs(const std::vector<COutput>& outputs, bool single_coin) const;
std::vector<OutputGroup> GroupOutputs(const std::vector<COutput>& outputs, bool single_coin, const size_t max_ancestors) const;
bool IsLockedCoin(uint256 hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
void LockCoin(const COutPoint& output) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);

View file

@ -85,15 +85,19 @@ class AvoidReuseTest(BitcoinTestFramework):
self.sync_all()
self.test_change_remains_change(self.nodes[1])
reset_balance(self.nodes[1], self.nodes[0].getnewaddress())
self.test_fund_send_fund_senddirty()
self.test_sending_from_reused_address_without_avoid_reuse()
reset_balance(self.nodes[1], self.nodes[0].getnewaddress())
self.test_fund_send_fund_send("legacy")
self.test_sending_from_reused_address_fails("legacy")
reset_balance(self.nodes[1], self.nodes[0].getnewaddress())
self.test_fund_send_fund_send("p2sh-segwit")
self.test_sending_from_reused_address_fails("p2sh-segwit")
reset_balance(self.nodes[1], self.nodes[0].getnewaddress())
self.test_fund_send_fund_send("bech32")
self.test_sending_from_reused_address_fails("bech32")
reset_balance(self.nodes[1], self.nodes[0].getnewaddress())
self.test_getbalances_used()
reset_balance(self.nodes[1], self.nodes[0].getnewaddress())
self.test_full_destination_group_is_preferred()
reset_balance(self.nodes[1], self.nodes[0].getnewaddress())
self.test_all_destination_groups_are_used()
def test_persistence(self):
'''Test that wallet files persist the avoid_reuse flag.'''
@ -162,13 +166,13 @@ class AvoidReuseTest(BitcoinTestFramework):
for logical_tx in node.listtransactions():
assert logical_tx.get('address') != changeaddr
def test_fund_send_fund_senddirty(self):
def test_sending_from_reused_address_without_avoid_reuse(self):
'''
Test the same as test_fund_send_fund_send, except send the 10 BTC with
Test the same as test_sending_from_reused_address_fails, except send the 10 BTC with
the avoid_reuse flag set to false. This means the 10 BTC send should succeed,
where it fails in test_fund_send_fund_send.
where it fails in test_sending_from_reused_address_fails.
'''
self.log.info("Test fund send fund send dirty")
self.log.info("Test sending from reused address with avoid_reuse=false")
fundaddr = self.nodes[1].getnewaddress()
retaddr = self.nodes[0].getnewaddress()
@ -213,7 +217,7 @@ class AvoidReuseTest(BitcoinTestFramework):
assert_approx(self.nodes[1].getbalance()["bitcoin"], 5, 0.001)
assert_approx(self.nodes[1].getbalance(avoid_reuse=False)["bitcoin"], 5, 0.001)
def test_fund_send_fund_send(self, second_addr_type):
def test_sending_from_reused_address_fails(self, second_addr_type):
'''
Test the simple case where [1] generates a new address A, then
[0] sends 10 BTC to A.
@ -222,7 +226,7 @@ class AvoidReuseTest(BitcoinTestFramework):
[1] tries to spend 10 BTC (fails; dirty).
[1] tries to spend 4 BTC (succeeds; change address sufficient)
'''
self.log.info("Test fund send fund send")
self.log.info("Test sending from reused {} address fails".format(second_addr_type))
fundaddr = self.nodes[1].getnewaddress(label="", address_type="legacy")
retaddr = self.nodes[0].getnewaddress()
@ -313,5 +317,66 @@ class AvoidReuseTest(BitcoinTestFramework):
assert_unspent(self.nodes[1], total_count=2, total_sum=6, reused_count=1, reused_sum=1)
assert_balances(self.nodes[1], mine={"used": 1, "trusted": 5})
def test_full_destination_group_is_preferred(self):
'''
Test the case where [1] only has 11 outputs of 1 BTC in the same reused
address and tries to send a small payment of 0.5 BTC. The wallet
should use 10 outputs from the reused address as inputs and not a
single 1 BTC input, in order to join several outputs from the reused
address.
'''
self.log.info("Test that full destination groups are preferred in coin selection")
# Node under test should be empty
assert_equal(self.nodes[1].getbalance(avoid_reuse=False)['bitcoin'], 0)
new_addr = self.nodes[1].getnewaddress()
ret_addr = self.nodes[0].getnewaddress()
# Send 11 outputs of 1 BTC to the same, reused address in the wallet
for _ in range(11):
self.nodes[0].sendtoaddress(new_addr, 1)
self.nodes[0].generate(1)
self.sync_all()
# Sending a transaction that is smaller than each one of the
# available outputs
txid = self.nodes[1].sendtoaddress(address=ret_addr, amount=0.5)
inputs = self.nodes[1].getrawtransaction(txid, 1)["vin"]
# The transaction should use 10 inputs exactly
assert_equal(len(inputs), 10)
def test_all_destination_groups_are_used(self):
'''
Test the case where [1] only has 22 outputs of 1 BTC in the same reused
address and tries to send a payment of 20.5 BTC. The wallet
should use all 22 outputs from the reused address as inputs.
'''
self.log.info("Test that all destination groups are used")
# Node under test should be empty
assert_equal(self.nodes[1].getbalance(avoid_reuse=False)['bitcoin'], 0)
new_addr = self.nodes[1].getnewaddress()
ret_addr = self.nodes[0].getnewaddress()
# Send 22 outputs of 1 BTC to the same, reused address in the wallet
for _ in range(22):
self.nodes[0].sendtoaddress(new_addr, 1)
self.nodes[0].generate(1)
self.sync_all()
# Sending a transaction that needs to use the full groups
# of 10 inputs but also the incomplete group of 2 inputs.
txid = self.nodes[1].sendtoaddress(address=ret_addr, amount=20.5)
inputs = self.nodes[1].getrawtransaction(txid, 1)["vin"]
# The transaction should use 22 inputs exactly
assert_equal(len(inputs), 22)
if __name__ == '__main__':
AvoidReuseTest().main()