-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Conversation
2e7a8c8
to
9edf85d
Compare
Exposed + (puny) test |
src/wallet/wallet2.cpp
Outdated
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; |
There was a problem hiding this comment.
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);
src/wallet/wallet2.cpp
Outdated
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
src/wallet/wallet2.cpp
Outdated
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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
I think this introduces a slight bias. See crypto: replace rand<T>()%N idiom with unbiased rand_idx(N) #5392
-
AIUI
output_index
already represents the absolute index of the chosen rct output. What's the point of then determining the corresponding block and then uniformly choosing another rct output in that block?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
9edf85d
to
1e0ff1a
Compare
5bb47a1
to
4f0646d
Compare
4f0646d
to
588e670
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed
Based on python code by sarang:
https://github.com/SarangNoether/skunkworks/blob/outputs/outputs/simulate.py