Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wallet2: use a gamma distribution to pick fake outs #3528

Merged
merged 1 commit into from
Aug 15, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 114 additions & 29 deletions src/wallet/wallet2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2502,7 +2502,7 @@ bool wallet2::refresh(bool trusted_daemon, uint64_t & blocks_fetched, bool& rece
return ok;
}
//----------------------------------------------------------------------------------------------------
bool wallet2::get_output_distribution(uint64_t &start_height, std::vector<uint64_t> &distribution)
bool wallet2::get_rct_distribution(uint64_t &start_height, std::vector<uint64_t> &distribution)
{
uint32_t rpc_version;
boost::optional<std::string> result = m_node_rpc_proxy.get_rpc_version(rpc_version);
Expand Down Expand Up @@ -6129,22 +6129,42 @@ void wallet2::get_outs(std::vector<std::vector<tools::wallet2::get_outs_entry>>
bool is_shortly_after_segregation_fork = height >= segregation_fork_height && height < segregation_fork_height + SEGREGATION_FORK_VICINITY;
bool is_after_segregation_fork = height >= segregation_fork_height;

// if we have at least one rct out, get the distribution, or fall back to the previous system
uint64_t rct_start_height;
std::vector<uint64_t> rct_offsets;
bool has_rct = false;
for (size_t idx: selected_transfers)
if (m_transfers[idx].is_rct())
{ has_rct = true; break; }
const bool has_rct_distribution = has_rct && get_rct_distribution(rct_start_height, rct_offsets);
if (has_rct_distribution)
{
// check we're clear enough of rct start, to avoid corner cases below
THROW_WALLET_EXCEPTION_IF(rct_offsets.size() <= CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE,
error::get_output_distribution, "Not enough rct outputs");
}

// get histogram for the amounts we need
cryptonote::COMMAND_RPC_GET_OUTPUT_HISTOGRAM::request req_t = AUTO_VAL_INIT(req_t);
cryptonote::COMMAND_RPC_GET_OUTPUT_HISTOGRAM::response resp_t = AUTO_VAL_INIT(resp_t);
m_daemon_rpc_mutex.lock();
// request histogram for all outputs, except 0 if we have the rct distribution
for(size_t idx: selected_transfers)
req_t.amounts.push_back(m_transfers[idx].is_rct() ? 0 : m_transfers[idx].amount());
std::sort(req_t.amounts.begin(), req_t.amounts.end());
auto end = std::unique(req_t.amounts.begin(), req_t.amounts.end());
req_t.amounts.resize(std::distance(req_t.amounts.begin(), end));
req_t.unlocked = true;
req_t.recent_cutoff = time(NULL) - RECENT_OUTPUT_ZONE;
bool r = net_utils::invoke_http_json_rpc("/json_rpc", "get_output_histogram", req_t, resp_t, m_http_client, rpc_timeout);
m_daemon_rpc_mutex.unlock();
THROW_WALLET_EXCEPTION_IF(!r, error::no_connection_to_daemon, "transfer_selected");
THROW_WALLET_EXCEPTION_IF(resp_t.status == CORE_RPC_STATUS_BUSY, error::daemon_busy, "get_output_histogram");
THROW_WALLET_EXCEPTION_IF(resp_t.status != CORE_RPC_STATUS_OK, error::get_histogram_error, resp_t.status);
if (!m_transfers[idx].is_rct() || !has_rct_distribution)
req_t.amounts.push_back(m_transfers[idx].is_rct() ? 0 : m_transfers[idx].amount());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ternary operation should be pointless now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because amount() will return 0 for rct ? Yes, that seems right, but there are other places where the amount is known (when you're sending) and I feel a bit uneasy not having this explicit case here if it inspires other place. But I think technically you're right.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure that m_amount (which is the same as amount()) for rct outputs is the decoded amount, not zero.

td.m_amount = tx_scan_info[o].amount;

if (!req_t.amounts.empty())
{
std::sort(req_t.amounts.begin(), req_t.amounts.end());
auto end = std::unique(req_t.amounts.begin(), req_t.amounts.end());
req_t.amounts.resize(std::distance(req_t.amounts.begin(), end));
req_t.unlocked = true;
req_t.recent_cutoff = time(NULL) - RECENT_OUTPUT_ZONE;
m_daemon_rpc_mutex.lock();
bool r = net_utils::invoke_http_json_rpc("/json_rpc", "get_output_histogram", req_t, resp_t, m_http_client, rpc_timeout);
m_daemon_rpc_mutex.unlock();
THROW_WALLET_EXCEPTION_IF(!r, error::no_connection_to_daemon, "transfer_selected");
THROW_WALLET_EXCEPTION_IF(resp_t.status == CORE_RPC_STATUS_BUSY, error::daemon_busy, "get_output_histogram");
THROW_WALLET_EXCEPTION_IF(resp_t.status != CORE_RPC_STATUS_OK, error::get_histogram_error, resp_t.status);
}

// if we want to segregate fake outs pre or post fork, get distribution
std::unordered_map<uint64_t, std::pair<uint64_t, uint64_t>> segregation_limit;
Expand Down Expand Up @@ -6200,6 +6220,36 @@ void wallet2::get_outs(std::vector<std::vector<tools::wallet2::get_outs_entry>>
COMMAND_RPC_GET_OUTPUTS_BIN::request req = AUTO_VAL_INIT(req);
COMMAND_RPC_GET_OUTPUTS_BIN::response daemon_resp = AUTO_VAL_INIT(daemon_resp);

struct gamma_engine
{
typedef uint64_t result_type;
static constexpr result_type min() { return 0; }
static constexpr result_type max() { return std::numeric_limits<result_type>::max(); }
result_type operator()() { return crypto::rand<result_type>(); }
} engine;
static const double shape = 19.28/*16.94*/;
//static const double shape = m_testnet ? 17.02 : 17.28;
static const double scale = 1/1.61;
std::gamma_distribution<double> gamma(shape, scale);
auto pick_gamma = [&]()
{
double x = gamma(engine);
x = exp(x);
uint64_t block_offset = x / DIFFICULTY_TARGET_V2; // this assumes constant target over the whole rct range
if (block_offset >= rct_offsets.size() - 1)
return std::numeric_limits<uint64_t>::max(); // bad pick
block_offset = rct_offsets.size() - 2 - block_offset;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this select outputs that are still locked? The rct_offsets looks to be the up to the latest block, and I think the num_outs call is filtering away locked outputs below. I think this is correct behavior but I wanted to start some discussion based on the recommendations from the paper.

Specifically my question is whether the gamma distribution should be over just the unlocked outputs, or over all outputs. It looks like the paper just tracked the spend time of 0 and 1 mixin outputs, and noted that the time distribution was different than Bitcoin. Which is to be expected, since Bitcoin outputs do not have a lock period. So again, I think the authors intended the parameters to be used over all outputs, but I was curious whether anyone else had information on this topic.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can pick locked outputs indeed, and they're rejected later. This is necessary also because coinbase outs are locked for 60 blocks, so all locked outs aren't necessarily at the end. In any case, I think using one or the other would yield a very similar distribution anyway (just a feeling, no evidence).

THROW_WALLET_EXCEPTION_IF(block_offset >= rct_offsets.size() - 1, error::wallet_internal_error, "Bad offset calculation");
THROW_WALLET_EXCEPTION_IF(rct_offsets[block_offset + 1] < rct_offsets[block_offset],
error::get_output_distribution, "Decreasing offsets in rct distribution: " +
std::to_string(block_offset) + ": " + std::to_string(rct_offsets[block_offset]) + ", " +
std::to_string(block_offset + 1) + ": " + std::to_string(rct_offsets[block_offset + 1]));
uint64_t n_rct = rct_offsets[block_offset + 1] - rct_offsets[block_offset];
if (n_rct == 0)
return rct_offsets[block_offset] ? rct_offsets[block_offset] - 1 : 0;
return rct_offsets[block_offset] + crypto::rand<uint64_t>() % n_rct;
};

size_t num_selected_transfers = 0;
for(size_t idx: selected_transfers)
{
Expand All @@ -6210,6 +6260,7 @@ void wallet2::get_outs(std::vector<std::vector<tools::wallet2::get_outs_entry>>
// request more for rct in base recent (locked) coinbases are picked, since they're locked for longer
size_t requested_outputs_count = base_requested_outputs_count + (td.is_rct() ? CRYPTONOTE_MINED_MONEY_UNLOCK_WINDOW - CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE : 0);
size_t start = req.outputs.size();
bool use_histogram = amount != 0 || !has_rct_distribution;

const bool output_is_pre_fork = td.m_block_height < segregation_fork_height;
uint64_t num_outs = 0, num_recent_outs = 0;
Expand Down Expand Up @@ -6265,26 +6316,41 @@ void wallet2::get_outs(std::vector<std::vector<tools::wallet2::get_outs_entry>>
num_post_fork_outs = num_outs - segregation_limit[amount].first;
}

LOG_PRINT_L1("" << num_outs << " unlocked outputs of size " << print_money(amount));
THROW_WALLET_EXCEPTION_IF(num_outs == 0, error::wallet_internal_error,
"histogram reports no unlocked outputs for " + boost::lexical_cast<std::string>(amount) + ", not even ours");
THROW_WALLET_EXCEPTION_IF(num_recent_outs > num_outs, error::wallet_internal_error,
"histogram reports more recent outs than outs for " + boost::lexical_cast<std::string>(amount));
if (use_histogram)
{
LOG_PRINT_L1("" << num_outs << " unlocked outputs of size " << print_money(amount));
THROW_WALLET_EXCEPTION_IF(num_outs == 0, error::wallet_internal_error,
"histogram reports no unlocked outputs for " + boost::lexical_cast<std::string>(amount) + ", not even ours");
THROW_WALLET_EXCEPTION_IF(num_recent_outs > num_outs, error::wallet_internal_error,
"histogram reports more recent outs than outs for " + boost::lexical_cast<std::string>(amount));
}
else
{
// the base offset of the first rct output in the first unlocked block (or the one to be if there's none)
num_outs = rct_offsets[rct_offsets.size() - CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE];
LOG_PRINT_L1("" << num_outs << " unlocked rct outputs");
THROW_WALLET_EXCEPTION_IF(num_outs == 0, error::wallet_internal_error,
"histogram reports no unlocked rct outputs, not even ours");
}

// how many fake outs to draw on a pre-fork triangular distribution
// how many fake outs to draw on a pre-fork distribution
size_t pre_fork_outputs_count = requested_outputs_count * pre_fork_num_out_ratio;
size_t post_fork_outputs_count = requested_outputs_count * post_fork_num_out_ratio;
// how many fake outs to draw otherwise
size_t normal_output_count = requested_outputs_count - pre_fork_outputs_count - post_fork_outputs_count;

// X% of those outs are to be taken from recent outputs
size_t recent_outputs_count = normal_output_count * RECENT_OUTPUT_RATIO;
if (recent_outputs_count == 0)
recent_outputs_count = 1; // ensure we have at least one, if possible
if (recent_outputs_count > num_recent_outs)
recent_outputs_count = num_recent_outs;
if (td.m_global_output_index >= num_outs - num_recent_outs && recent_outputs_count > 0)
--recent_outputs_count; // if the real out is recent, pick one less recent fake out
size_t recent_outputs_count = 0;
if (use_histogram)
{
// X% of those outs are to be taken from recent outputs
recent_outputs_count = normal_output_count * RECENT_OUTPUT_RATIO;
if (recent_outputs_count == 0)
recent_outputs_count = 1; // ensure we have at least one, if possible
if (recent_outputs_count > num_recent_outs)
recent_outputs_count = num_recent_outs;
if (td.m_global_output_index >= num_outs - num_recent_outs && recent_outputs_count > 0)
--recent_outputs_count; // if the real out is recent, pick one less recent fake out
}
LOG_PRINT_L1("Fake output makeup: " << requested_outputs_count << " requested: " << recent_outputs_count << " recent, " <<
pre_fork_outputs_count << " pre-fork, " << post_fork_outputs_count << " post-fork, " <<
(requested_outputs_count - recent_outputs_count - pre_fork_outputs_count - post_fork_outputs_count) << " full-chain");
Expand Down Expand Up @@ -6364,7 +6430,26 @@ void wallet2::get_outs(std::vector<std::vector<tools::wallet2::get_outs_entry>>

uint64_t i;
const char *type = "";
if (num_found - 1 < recent_outputs_count) // -1 to account for the real one we seeded with
if (amount == 0 && has_rct_distribution)
{
// gamma distribution
if (num_found -1 < recent_outputs_count + pre_fork_outputs_count)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case recent_outputs_count is known to be 0, right? Why bother adding?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it matches the ordering so it's a lot clearer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

{
do i = pick_gamma(); while (i >= segregation_limit[amount].first);
type = "pre-fork gamma";
}
else if (num_found -1 < recent_outputs_count + pre_fork_outputs_count + post_fork_outputs_count)
{
do i = pick_gamma(); while (i < segregation_limit[amount].first || i >= num_outs);
type = "post-fork gamma";
}
else
{
do i = pick_gamma(); while (i >= num_outs);
type = "gamma";
}
}
else if (num_found - 1 < recent_outputs_count) // -1 to account for the real one we seeded with
{
// triangular distribution over [a,b) with a=0, mode c=b=up_index_limit
uint64_t r = crypto::rand<uint64_t>() % ((uint64_t)1 << 53);
Expand Down Expand Up @@ -6429,7 +6514,7 @@ void wallet2::get_outs(std::vector<std::vector<tools::wallet2::get_outs_entry>>

// get the keys for those
m_daemon_rpc_mutex.lock();
r = epee::net_utils::invoke_http_bin("/get_outs.bin", req, daemon_resp, m_http_client, rpc_timeout);
bool r = epee::net_utils::invoke_http_bin("/get_outs.bin", req, daemon_resp, m_http_client, rpc_timeout);
m_daemon_rpc_mutex.unlock();
THROW_WALLET_EXCEPTION_IF(!r, error::no_connection_to_daemon, "get_outs.bin");
THROW_WALLET_EXCEPTION_IF(daemon_resp.status == CORE_RPC_STATUS_BUSY, error::daemon_busy, "get_outs.bin");
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/wallet2.h
Original file line number Diff line number Diff line change
Expand Up @@ -1209,7 +1209,7 @@ namespace tools
void cache_ringdb_key();
void clear_ringdb_key();

bool get_output_distribution(uint64_t &start_height, std::vector<uint64_t> &distribution);
bool get_rct_distribution(uint64_t &start_height, std::vector<uint64_t> &distribution);

uint64_t get_segregation_fork_height() const;

Expand Down