Upstream merge 4 #106
Upstream merge 4 #106
Conversation
as per "An Empirical Analysis of Linkability in the Monero Blockchain", by Miller et al.
…#4020 And percent if usefull (% < 99)
It was accepting any character for the dot (yeah, massive big I know)
avoids people thinking it's somehow a generic AE system
Fixes failing test during Arch package build (due to attempt to write to ~/.bitmonero/...). Prefix temp dir path with "monero-" because we are not putting it on the system, so good to identify ourselves in case the dir gets left over due to crash, etc. (Note: prefix changed from monero- to aeon-)
…e-names /monero#4166
That takes a lot of time for even not so large wallets
…lets /monero#4188
It was actually incorrect, as it would not return commitment
Blocks have a very wide range, whereas actual size is the relevant quantity to consider when syncing
…onero#4207 Non fluffy block nodes should now be very rare
Many people are using this as a "let's see what this does" command when something doesn't work as they thought it should, and thus destroying info that they might still need.
Note: /monero#3999 was cherry picked previously in /aeon#30 which got merged before 3999 got merged upstream. 3999 was then later force pushed with some changes, introducing some inconsistency which is resolved here.
|
OS: OSX 10.13.6 high sierra |
|
OS: Ubuntu 18.04 (VM) Nice to see this prompt now (even though I was using m.2 with a VM): |
|
OS: Tested wallet, daemon and rpc on both live and testnet - no issues. |
|
OS: Windows 10 x64 (Msys2-x64) Builds ok, no issues. Daemon and wallet CLI function properly with basic functions. Daemon syncs ok on mainnet. Docker build armv8OS: Ubuntu 18.10.1 (VM) |
|
The gamma distribution implementation is likely to be problematic on AEON due to the overweighting of transactions (especially coinbases) from small blocks. Although after a quick look at the commit it wasn't clear to me whether or not it would even be used for non-rct. Monero came up with some sort of partial mitigation (I don't see that commit here) which involved picking from a small range of blocks rather than one but even that won't be very effective on AEON with its currently-smaller tx volume. |
|
I'm pretty sure the gamma selection doesn't affect pre-RingCT Aeon as seen in the patch: - if (num_found - 1 < recent_outputs_count) // -1 to account for the real one we seeded with
+ if (amount == 0 && has_rct_distribution)
+ ...
+ else if (num_found - 1 < recent_outputs_count) // -1 to account for the real one we seeded withI've also confirmed that it works fine on stagenet where almost all blocks are empty. |
|
@stoffu We probably do want some sort of gamma selection as an AEON enhancement if we're not planning on switching to RingCT imminently. The old selection method was a quick patch on the original even worse one, and, further, gets worse and worse over time due to chain age. However that can be addressed separately from the upstream merge. |
|
I thought the community agreed they wanted ringCT and bulletproofs to create fungibity for Aeon? Am I correct that RingCT would require a hard fork to activate? |
|
RingCT would require a hard fork so is out of scope with an upstream merge PR. I only mentioned it here because of the gamma commit being included. |
|
Is there a reason this is being delayed? |
Possibly due to #109 since we are not going RingCT this fork. |
From IRC, smooth, "camthegeek: not stuck, we have a solution to that, it will go in after the upstream merge" |
|
Approved |
I had a private chat with @camthegeek and made some arguments which can be summarized as follows:
Merging upstream patches is in itself not a too difficult task, but it's also quite easy to make mistakes since there are many instances of merge conflicts. Also, some patches require adapting the changes to Aeon (e.g. replacing string literals).
While I do feel happy to see people's willingness to contribute to the effort of merging upstream patches, I'd necessarily feel uneasy about blindly trusting the work offered by them, especially when they seem to have relatively limited experience with the Monero codebase and development in general. Therefore, I'd have to redo the same merging task myself again and compare the result with the offered one in order to be sure that the patches were merged properly. This seems wasteful since the same work is repeated by two people.
I know this sounds arrogant, but I believe I'm the most familiar with the codebase and thus the most suitable for the task of merging upstream patches among the Aeon community members. For the time being, I'll be able to keep working on merging upstream patches on a regular basis; i.e. I'll make sure to submit the next upstream merge PR as soon as possible after the previous one gets merged. If the situation around me changes and I become unable to keep working on Aeon in the future, I'll announce as such.
I don't mean to discourage people from working on the development. One important way of contributing (in addition to testing) is to review the upstream merge PRs and find out any mistakes I make. Such reviewing will also be very helpful for familiarizing yourself more with the codebase. More people becoming familiar with the codebase is crucially important since the development shouldn't be dependent on a small number of people who can suddenly disappear for various reasons (car accident, illness, etc).
That said, this PR replicates the work done by @camthegeek in #100:
This pull request merges upstream patches after monero-project#4129 which is the last one included in #90. This PR includes upstream PRs from monero-project#3528 merged on August 15 up to monero-project#3999 merged on August 24:
Some noteworthy remarks:
The gamma output selection introduced in wallet2: use a gamma distribution to pick fake outs monero-project/monero#3528 is only effective for RingCT outputs, so it has no effect for Aeon currently. FYI, this PR introduced a flaw in Monero that got later fixed in wallet2: extend fake out picks away from the gamma pick monero-project/monero#4697.
An optional flag
--kdf-roundswas added to the wallet CLI/RPC in wallet: allow adjusting number of rounds for the key derivation function monero-project/monero#4103 which is meant to be a protection against brute forcing passwords.wallet-rpc: add get_address_index command monero-project/monero#4170 adds
get_address_indexcommand to the wallet RPC.wallet-rpc: filter getbalance response by address index monero-project/monero#4171 allows you to filter the wallet RPC
getbalanceresponse byaddress_indices.simplewallet: add set_tx_key for importing tx keys from 3rd party wallets monero-project/monero#4188 adds
set_tx_keycommand to the wallet CLI that allows you to import tx keys generated by some other 3rd party wallet software.wallet2: store keys encrypted monero-project/monero#4131 improves the security of the wallet CLI by keeping the wallet secret keys encrypted when the wallet file is loaded on memory and decrypting them only when needed for signing.
rpc-wallet: refresh command added monero-project/monero#4242 adds
refreshcommand to the wallet RPC.add and use constant time 32 byte equality function monero-project/monero#3999 was already cherry picked previously in Upstream merge 1 #30 which got merged before add and use constant time 32 byte equality function monero-project/monero#3999 got merged in Monero. add and use constant time 32 byte equality function monero-project/monero#3999 was then later replaced by force push, introducing some inconsistency which is resolved here by cherry picking again.