From f2ae807891c7278617d78bc4ed603f379880656b Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 13 Mar 2022 23:24:46 +0000 Subject: [PATCH] Implement opportunistic shielding in `SpendableInputs::LimitToAmount` We now use the total funds in each available pool to figure out whether a particular desired selection order can provide the requested amount, and can alter the selection order each time we add another pool. --- src/wallet/gtest/test_note_selection.cpp | 160 +++++++++++++++++------ src/wallet/wallet.cpp | 85 +++++++++--- 2 files changed, 182 insertions(+), 63 deletions(-) diff --git a/src/wallet/gtest/test_note_selection.cpp b/src/wallet/gtest/test_note_selection.cpp index ccfb63599..30932b7ed 100644 --- a/src/wallet/gtest/test_note_selection.cpp +++ b/src/wallet/gtest/test_note_selection.cpp @@ -64,10 +64,21 @@ class SpendableInputsTest : std::set, // Recipient pools std::set, - // Expected pool selection order - std::vector>> { + // List of expected pool selection orders + std::vector>>> { }; +TEST_P(SpendableInputsTest, OrderListIsSequentiallyIncreasing) +{ + // The list of selection orders encodes the "failover" as we exceed the + // funds available in the selected pools. For simplicity, we require these + // to be sequentially increasing in length. + auto order = std::get<2>(GetParam()); + for (int i = 0; i < order.size(); i++) { + EXPECT_EQ(order[i].size(), i + 1); + } +} + TEST_P(SpendableInputsTest, SelectsSproutBeforeFirst) { auto available = std::get<0>(GetParam()); @@ -89,14 +100,14 @@ TEST_P(SpendableInputsTest, SelectsSproutBeforeFirst) } } - // Limit to 5 zatoshis. + // Limit to 5 zatoshis (which can be satisfied by any pool). EXPECT_TRUE(inputs.LimitToAmount(5, 1, recipientPools)); EXPECT_EQ(inputs.Total(), 5); // We only have Sprout notes. EXPECT_EQ(inputs.orchardNoteMetadata.size(), 0); EXPECT_EQ(inputs.sproutNoteEntries.size(), 5); - for (auto pool : order) { + for (auto pool : order[0]) { switch (pool) { case OutputPool::Sapling: EXPECT_EQ(inputs.saplingNoteEntries.size(), 0); break; case OutputPool::Transparent: EXPECT_EQ(inputs.utxos.size(), 0); break; @@ -125,16 +136,16 @@ TEST_P(SpendableInputsTest, SelectsSproutThenFirst) } } - // Limit to 14 zatoshis. + // Limit to 14 zatoshis (which requires two pools). EXPECT_TRUE(inputs.LimitToAmount(14, 1, std::get<1>(GetParam()))); EXPECT_EQ(inputs.Total(), 14); - // We have all Sprout notes and some from the first pool. + // We have all Sprout notes and some from the first pool in the first order. EXPECT_EQ(inputs.orchardNoteMetadata.size(), 0); EXPECT_EQ(inputs.sproutNoteEntries.size(), 10); - for (int i = 0; i < order.size(); i++) { + for (int i = 0; i < order[0].size(); i++) { auto expected = i == 0 ? 4 : 0; - switch (order[i]) { + switch (order[0][i]) { case OutputPool::Sapling: EXPECT_EQ(inputs.saplingNoteEntries.size(), expected); break; case OutputPool::Transparent: EXPECT_EQ(inputs.utxos.size(), expected); break; } @@ -162,16 +173,16 @@ TEST_P(SpendableInputsTest, SelectsFirstBeforeSecond) } } - // Limit to 8 zatoshis. + // Limit to 8 zatoshis (which can be satisfied by any pool). EXPECT_TRUE(inputs.LimitToAmount(8, 1, std::get<1>(GetParam()))); EXPECT_EQ(inputs.Total(), 8); - // We only have the first pool selected. + // We use the first order and only have the first pool selected. EXPECT_EQ(inputs.orchardNoteMetadata.size(), 0); EXPECT_EQ(inputs.sproutNoteEntries.size(), 0); - for (int i = 0; i < order.size(); i++) { + for (int i = 0; i < order[0].size(); i++) { auto expected = i == 0 ? 8 : 0; - switch (order[i]) { + switch (order[0][i]) { case OutputPool::Sapling: EXPECT_EQ(inputs.saplingNoteEntries.size(), expected); break; case OutputPool::Transparent: EXPECT_EQ(inputs.utxos.size(), expected); break; } @@ -199,25 +210,35 @@ TEST_P(SpendableInputsTest, SelectsFirstThenSecond) } } - // Limit to 13 zatoshis. + // Limit to 13 zatoshis (which requires two pools). // If we only have one pool available, we won't have sufficient funds. auto sufficientFunds = inputs.LimitToAmount(13, 1, std::get<1>(GetParam())); if (available.size() == 1) { EXPECT_FALSE(sufficientFunds); EXPECT_EQ(inputs.Total(), 10); + + // We have selected all of the available pool. + EXPECT_EQ(inputs.orchardNoteMetadata.size(), 0); + EXPECT_EQ(inputs.sproutNoteEntries.size(), 0); + for (int i = 0; i < order[0].size(); i++) { + switch (order[0][i]) { + case OutputPool::Sapling: EXPECT_EQ(inputs.saplingNoteEntries.size(), 10); break; + case OutputPool::Transparent: EXPECT_EQ(inputs.utxos.size(), 10); break; + } + } } else { EXPECT_TRUE(sufficientFunds); EXPECT_EQ(inputs.Total(), 13); - } - // We have all of the first pool and (if present) some of the second. - EXPECT_EQ(inputs.orchardNoteMetadata.size(), 0); - EXPECT_EQ(inputs.sproutNoteEntries.size(), 0); - for (int i = 0; i < order.size(); i++) { - auto expected = i == 0 ? 10 : i == 1 ? 3 : 0; - switch (order[i]) { - case OutputPool::Sapling: EXPECT_EQ(inputs.saplingNoteEntries.size(), expected); break; - case OutputPool::Transparent: EXPECT_EQ(inputs.utxos.size(), expected); break; + // We have all of the first pool and some of the second. + EXPECT_EQ(inputs.orchardNoteMetadata.size(), 0); + EXPECT_EQ(inputs.sproutNoteEntries.size(), 0); + for (int i = 0; i < order[1].size(); i++) { + auto expected = i == 0 ? 10 : i == 1 ? 3 : 0; + switch (order[1][i]) { + case OutputPool::Sapling: EXPECT_EQ(inputs.saplingNoteEntries.size(), expected); break; + case OutputPool::Transparent: EXPECT_EQ(inputs.utxos.size(), expected); break; + } } } } @@ -249,23 +270,78 @@ TEST_P(SpendableInputsTest, SelectsSproutAndFirstThenSecond) if (available.size() == 1) { EXPECT_FALSE(sufficientFunds); EXPECT_EQ(inputs.Total(), 20); + + // We have selected all of the available pool. + EXPECT_EQ(inputs.orchardNoteMetadata.size(), 0); + EXPECT_EQ(inputs.sproutNoteEntries.size(), 10); + for (int i = 0; i < order[0].size(); i++) { + switch (order[0][i]) { + case OutputPool::Sapling: EXPECT_EQ(inputs.saplingNoteEntries.size(), 10); break; + case OutputPool::Transparent: EXPECT_EQ(inputs.utxos.size(), 10); break; + } + } } else { EXPECT_TRUE(sufficientFunds); EXPECT_EQ(inputs.Total(), 24); - } - // We have all of Sprout and the first pool, and (if present) some of the second. - EXPECT_EQ(inputs.orchardNoteMetadata.size(), 0); - EXPECT_EQ(inputs.sproutNoteEntries.size(), 10); - for (int i = 0; i < order.size(); i++) { - auto expected = i == 0 ? 10 : i == 1 ? 4 : 0; - switch (order[i]) { - case OutputPool::Sapling: EXPECT_EQ(inputs.saplingNoteEntries.size(), expected); break; - case OutputPool::Transparent: EXPECT_EQ(inputs.utxos.size(), expected); break; + // We have all of Sprout and the first pool, and some of the second. + EXPECT_EQ(inputs.orchardNoteMetadata.size(), 0); + EXPECT_EQ(inputs.sproutNoteEntries.size(), 10); + for (int i = 0; i < order[1].size(); i++) { + auto expected = i == 0 ? 10 : i == 1 ? 4 : 0; + switch (order[1][i]) { + case OutputPool::Sapling: EXPECT_EQ(inputs.saplingNoteEntries.size(), expected); break; + case OutputPool::Transparent: EXPECT_EQ(inputs.utxos.size(), expected); break; + } } } } +TEST_P(SpendableInputsTest, OpportunisticShielding) +{ + auto available = std::get<0>(GetParam()); + auto recipientPools = std::get<1>(GetParam()); + auto order = std::get<2>(GetParam()); + auto wtx = FakeWalletTx(); + + // If we don't have multiple pools of which one is transparent, this test + // doesn't apply. + if (!(available.size() > 1 && available.count(OutputPool::Transparent))) { + return; + } + + // Create a set of inputs from the available pools. + auto inputs = FakeSpendableInputs(available, false, &wtx); + EXPECT_EQ(inputs.Total(), 10 * available.size()); + + // Remove notes from the shielded pools, so we have more transparent funds. + EXPECT_EQ(inputs.orchardNoteMetadata.size(), 0); + EXPECT_EQ(inputs.sproutNoteEntries.size(), 0); + for (auto pool : available) { + switch (pool) { + case OutputPool::Sapling: + while (inputs.saplingNoteEntries.size() > 3) { + inputs.saplingNoteEntries.pop_back(); + } + EXPECT_EQ(inputs.saplingNoteEntries.size(), 3); + break; + case OutputPool::Transparent: EXPECT_EQ(inputs.utxos.size(), 10); break; + } + } + + // Limit to 7 zatoshis. We can't satisfy this with two shielded pools, so we + // will trigger the opportunistic shielding logic, which causes us to select + // all transparent notes. Because transparent is sufficient to reach the + // target amount, we don't select any shielded notes. + EXPECT_TRUE(inputs.LimitToAmount(7, 1, std::get<1>(GetParam()))); + EXPECT_EQ(inputs.Total(), 10); + + EXPECT_EQ(inputs.orchardNoteMetadata.size(), 0); + EXPECT_EQ(inputs.saplingNoteEntries.size(), 0); + EXPECT_EQ(inputs.sproutNoteEntries.size(), 0); + EXPECT_EQ(inputs.utxos.size(), 10); +} + const std::set SET_T({OutputPool::Transparent}); const std::set SET_S({OutputPool::Sapling}); const std::set SET_TS({OutputPool::Transparent, OutputPool::Sapling}); @@ -279,16 +355,16 @@ INSTANTIATE_TEST_CASE_P( ExhaustiveCases, SpendableInputsTest, ::testing::Values( - // Available | Recipients | Order // Rationale - // ----------|------------|----------//---------- - std::make_tuple(SET_T, SET_T, VEC_T), // N/A - std::make_tuple(SET_T, SET_S, VEC_T), // N/A - std::make_tuple(SET_T, SET_TS, VEC_T), // N/A - std::make_tuple(SET_S, SET_T, VEC_S), // N/A - std::make_tuple(SET_S, SET_S, VEC_S), // N/A - std::make_tuple(SET_S, SET_TS, VEC_S), // N/A - std::make_tuple(SET_TS, SET_T, VEC_ST), // Hide sender if possible. - std::make_tuple(SET_TS, SET_S, VEC_TS), // Opportunistic shielding. - std::make_tuple(SET_TS, SET_TS, VEC_TS) // Opportunistic shielding. + // Available | Recipients | Order // Rationale + // ----------|---------------------|-------------------//---------- + std::make_tuple(SET_T, SET_T, std::vector({VEC_T})), // N/A + std::make_tuple(SET_T, SET_S, std::vector({VEC_T})), // N/A + std::make_tuple(SET_T, SET_TS, std::vector({VEC_T})), // N/A + std::make_tuple(SET_S, SET_T, std::vector({VEC_S})), // N/A + std::make_tuple(SET_S, SET_S, std::vector({VEC_S})), // N/A + std::make_tuple(SET_S, SET_TS, std::vector({VEC_S})), // N/A + std::make_tuple(SET_TS, SET_T, std::vector({VEC_S, VEC_TS})), // Hide sender, opportunistic shielding + std::make_tuple(SET_TS, SET_S, std::vector({VEC_S, VEC_TS})), // Fully shielded, opportunistic shielding + std::make_tuple(SET_TS, SET_TS, std::vector({VEC_S, VEC_TS})) // Hide sender, opportunistic shielding ) ); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index cf100d00e..ceb644f5a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -35,6 +35,7 @@ #include #include +#include #include #include @@ -7080,6 +7081,10 @@ bool SpendableInputs::LimitToAmount( // we do not yet have sufficient funds return totalSelected == amountRequired || totalSelected - amountRequired > dustThreshold; }; + auto wouldSuffice = [&](CAmount extra) { + auto totalWithExtra = totalSelected + extra; + return totalWithExtra == amountRequired || totalWithExtra - amountRequired > dustThreshold; + }; // Select Sprout notes for spending first - if possible, we want users to // spend any notes that they still have in the Sprout pool. @@ -7095,8 +7100,21 @@ bool SpendableInputs::LimitToAmount( sproutNoteEntries.erase(sproutIt, sproutNoteEntries.end()); // Check what input pools we have available. - bool haveTransparent = !utxos.empty(); - bool haveSapling = !saplingNoteEntries.empty(); + CAmount availableTransparent = std::accumulate( + utxos.begin(), utxos.end(), CAmount(0), [](CAmount acc, const COutput& utxo) { + return acc + utxo.Value(); + }); + CAmount availableSapling = std::accumulate( + saplingNoteEntries.begin(), + saplingNoteEntries.end(), + CAmount(0), + [](CAmount acc, const SaplingNoteEntry& entry) { + return acc + entry.note.value(); + }); + assert(availableTransparent >= 0); + assert(availableSapling >= 0); + bool haveTransparent = availableTransparent > 0; + bool haveSapling = availableSapling > 0; std::set available; if (haveTransparent) { available.insert(OutputPool::Transparent); @@ -7107,31 +7125,49 @@ bool SpendableInputs::LimitToAmount( // Now determine the order in which to select the remaining notes and coins. // We do this in a way that minimizes information leakage while moving funds - // into the shielded pool where possible. + // into the shielded pool where possible. The rules below follow several + // general principles: + // + // - If we have sufficient funds in a single shielded pool, we prefer to + // select funds from that pool (especially if the pool matches a recipient + // pool). + // - If we don't have sufficient funds in a single shielded pool, we prefer + // to select funds from older shielded pools first, to generally migrate + // funds towards newer shielded pools. We do not perform opportunistic + // migration however (at this time). + // - If we have transparent recipients, we prefer to select funds across all + // shielded pools before the transparent pool. The address and amount for + // these recipients is necessarily revealed, but we can hide the sender. + // - If we don't have suffient funds in shielded pools and are required to + // select transparent coins, we always select all transparent coins first. + // Given that the transaction will necessarily reveal sender information, + // we use it to opportunistically shield transparent coins. // // In the following table: + // - "Available" denotes the pools in which we have selectable notes. + // - "Recipients" lists the pools in which we are required to create outputs. + // - "Order" is a comma-separated list of pool selection orders. The order + // used is the first order in the list that can select sufficient funds. // - T: transparent pool // - S: Sapling pool // - // Available | Recipients | Order | Rationale - // ----------|------------|-------|---------- - // T | T | T | N/A - // T | S | T | N/A - // T | TS | T | N/A - // S | T | S | N/A - // S | S | S | N/A - // S | TS | S | N/A - // TS | T | ST | Hide sender if possible. - // TS | S | TS | Opportunistic shielding. - // TS | TS | TS | Opportunistic shielding. + // Available | Recipients | Order | Rationale + // ----------|------------|--------|---------- + // T | ** | T | N/A + // S | ** | S | N/A + // TS | T | S, TS | Hide sender, opportunistic shielding + // TS | S | S, TS | Fully shielded, opportunistic shielding + // TS | TS | S, TS | Hide sender, opportunistic shielding std::vector selectionOrder; + bool opportunisticShielding = false; if (available.size() <= 1) { // We have at most one input pool, so we don't need selection logic. selectionOrder.assign(available.begin(), available.end()); - } else if (recipientPools.size() == 1 && recipientPools.count(OutputPool::Transparent)) { - // Hide sender if possible. + } else if (wouldSuffice(availableSapling)) { + // Either fully-shielded, or we hide the sender. selectionOrder = { OutputPool::Sapling, + // Pools below here are erased. OutputPool::Transparent, }; } else { @@ -7140,6 +7176,7 @@ bool SpendableInputs::LimitToAmount( OutputPool::Transparent, OutputPool::Sapling, }; + opportunisticShielding = true; } // Ensure we provided a total selection order (so that all unselected notes @@ -7155,12 +7192,18 @@ bool SpendableInputs::LimitToAmount( [](COutput i, COutput j) -> bool { return i.Value() > j.Value(); }); - auto utxoIt = utxos.begin(); - while (utxoIt != utxos.end() && !haveSufficientFunds()) { - totalSelected += utxoIt->Value(); - ++utxoIt; + if (opportunisticShielding) { + // Select all transparent coins. + totalSelected += availableTransparent; + } else { + // Only select as many as we need. + auto utxoIt = utxos.begin(); + while (utxoIt != utxos.end() && !haveSufficientFunds()) { + totalSelected += utxoIt->Value(); + ++utxoIt; + } + utxos.erase(utxoIt, utxos.end()); } - utxos.erase(utxoIt, utxos.end()); break; }