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: "output lineup" fake out selection #5389

Merged
merged 2 commits into from Apr 18, 2019

Conversation

moneromooo-monero
Copy link
Collaborator

@moneromooo-monero
Copy link
Collaborator Author

Exposed + (puny) test

gamma = std::gamma_distribution<double>(shape, scale);
THROW_WALLET_EXCEPTION_IF(rct_offsets.size() <= CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE, error::wallet_internal_error, "Bad offset calculation");
const size_t blocks_in_a_year = 86400 * 365 / DIFFICULTY_TARGET_V2;
const size_t blocks_to_consider = rct_offsets.size() < blocks_in_a_year;
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't you mean this?

-   const size_t blocks_to_consider = rct_offsets.size() < blocks_in_a_year;
+   const size_t blocks_to_consider = std::min(rct_offsets.size(), blocks_in_a_year);

end = rct_offsets.data() + rct_offsets.size() - CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE;
num_rct_outputs = *(end - 1);
THROW_WALLET_EXCEPTION_IF(num_rct_outputs == 0, error::wallet_internal_error, "No rct outputs");
average_output_time = DIFFICULTY_TARGET_V2 * rct_offsets.size() / num_rct_outputs; // this assumes constant target over the whole rct range
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't you mean this?

-   average_output_time = DIFFICULTY_TARGET_V2 * rct_offsets.size() / num_rct_outputs; // this assumes constant target over the whole rct range
+   average_output_time = DIFFICULTY_TARGET_V2 * blocks_to_consider / outputs_to_consider; // this assumes constant target over the whole rct range

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks. Kinda forgot to finish it :D

if (n_rct == 0)
return std::numeric_limits<uint64_t>::max(); // bad pick
MDEBUG("Picking 1/" << n_rct << " in block " << index);
return first_rct + crypto::rand<uint64_t>() % n_rct;
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was wondering the same. I think it's because the Miller et al paper has this construction. You may want to ask sarang. The one thing I can think of is that, for very recent blocks with a large number of outputs, those outputs' ordering is arbitrary (based on tx fee), and keeping the probability from the lineup's first step can bias the selection. I wouldn't think it's much though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I asked sarang, who confirmed this is the reason.

Copy link
Contributor

@fluffypony fluffypony left a comment

Choose a reason for hiding this comment

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

Reviewed

@fluffypony fluffypony merged commit 588e670 into monero-project:master Apr 18, 2019
fluffypony added a commit that referenced this pull request Apr 18, 2019
588e670 simplewallet: fix output representation offset (moneromooo-monero)
35e0a96 wallet2: "output lineup" fake out selection (moneromooo-monero)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants