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

Faster wallet refreshes #3716

Merged
merged 17 commits into from Jun 27, 2018
Merged

Conversation

moneromooo-monero
Copy link
Collaborator

This has random stuff in it, because I need random stuff that's not merged, so all this has to be plonked in this tree for me to hack on it. I'll rebase once those things are merged.

Warning:
If you run this, your blockchain DB will upgrade to v2, which means you won't be able to run any daemon that does not have this patch (I'll get it merged once 0.12.1.0 is out of the way).

Speeds up between 2x and 5x. Speed tests welcome.

If your wallet is not a mining wallet (ie, you never got any coinbase output on it), setting refresh-type=no-coinbase now gives a pretty good speed boost.

vtnerd's wallet asm patch will give more speedup, as the wallet is using 90% of its time in the fe API.

@glv2
Copy link
Contributor

glv2 commented May 21, 2018

Here are some test results of a wallet scanning the whole blockchain:

computer

  • OS: GNU/Linux x86-64
  • CPU: Intel Core i7-3630QM @ 2.40GHz
  • Storage: Samsung SSD 850 PRO

master branch (4b728d7)

  • blocks: 1577488
  • time: 31.3 minutes

master branch + PR3716

  • blocks: 1577521
  • time: 8.6 minutes

master branch + PR3716 + refresh-type=no-coinbase

  • blocks: 1577526
  • time: 7.2 minutes

@moneromooo-monero moneromooo-monero changed the title [DO NOT MERGE] Faster wallet refreshes, plus random stuff Faster wallet refreshes Jun 10, 2018
@moneromooo-monero
Copy link
Collaborator Author

Now ready for review.

@stoffu
Copy link
Contributor

stoffu commented Jun 11, 2018

Computer:

  • OS: Windows 7 Professional, 64bit
  • CPU: Intel Core i7-5690X @ 3.00GHz
  • Storage: Samsung SSD 850 PRO

Master (25e7a7d):

  • optimize-coinbase -> 26m 46s (1592555 blocks)
  • no-coinbase -> 24m 24s (1592573 blocks)

PR-3716 (c4fda6d):

  • optimize-coinbase -> 5m 31s (1592557 blocks)
  • no-coinbase -> 4m 52s (1592578 blocks)

I can do two more tests on OSX machines with SSD and HDD.

@stoffu
Copy link
Contributor

stoffu commented Jun 11, 2018

Note for testers: if you are to measure the time of scanning the entire blockchain from genesis, not only the restore height has to be set to 0, but also the wallet creation time must not be set to current. That is, if you create a new wallet and set refresh-from-block-height 0 and then rescan_bc, the wallet will NOT do the full scan of the blockchain (instead it skips old blocks based on timestamps). The correct way is to restore the wallet from seed and setting the restore height to 0.

@stoffu
Copy link
Contributor

stoffu commented Jun 11, 2018

Computer:

  • OS: Mac OSX 10.12.6
  • CPU: 2.9 GHz Intel Core i7
  • Storage: APPLE SSD SM2048L

Master:

  • optimize-coinbase -> 23m 12s (1592682 blocks)
  • no-coinbase -> 21m 31s (1592694 blocks)

PR-3716:

  • optimize-coinbase -> 11m 9s (1592697 blocks)
  • no-coinbase -> 9m 17s (1592702 blocks)

@stoffu
Copy link
Contributor

stoffu commented Jun 13, 2018

Computer:

  • OS: macOS Sierra 10.12.6
  • CPU: 2.3 GHz Intel Core i7
  • Storage: APPLE HDD HTS541010A9E662

Master:

  • optimize-coinbase -> 1h 58m 24s (1592635 blocks)
  • no-coinbase -> 1h 56m 42s (1593176 blocks)

PR-3716:

  • optimize-coinbase -> 2h 0m 18s (1593251 blocks)
  • no-coinbase -> 1h 57m 50s (1593310 blocks)

@moneromooo-monero
Copy link
Collaborator Author

moneromooo-monero commented Jun 13, 2018

That one seems odd since HDD means I/O is a bottleneck, and the new version reads much less from the database, since it stores prunable and unprunable data separately... Nevermind, the new db format patch is already merged in master.

@SamsungGalaxyPlayer
Copy link
Collaborator

SamsungGalaxyPlayer commented Jun 14, 2018

Computer: Purism Librem 13
OS: PureOS GNU/Linux 8
Processor: Intel Core i7-6500U CPU @ 2.50 GHz
OS Hard Drive: 120 GB SATA SSD (unknown type)
Attached Hard Drive: 1TB Samsung 970 EVO

Master:

  • optimize-coinbase
    • 970 EVO -> 0h 28m 01s; 1598776 blocks (951.1 blocks per second)
  • no-coinbase
    • 970 EVO -> 0h 24m 20s; 1598750 blocks (1095.0 blocks per second)

PR-3716:

  • optimize-coinbase
    • 970 EVO -> 0h 20m 43s (limited by CPU); 1596508 blocks (1284.4 blocks per second)
  • no-coinbase
    • 970 EVO -> 0h 17m 25s (limited by CPU); 1597181 blocks (1528.4 blocks per second)

@@ -593,7 +597,7 @@ namespace cryptonote
return true;
}
sorted_txs.push_back(std::move(txs.front()));
txs.pop_front();
txs.erase(txs.begin());
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this an unneeded burden which can be avoided by having a counter and doing sorted_txs.push_back(std::move(txs[counter]));?

{
error = false;

try
{
drop_from_short_history(short_chain_history, 3);

// prepend the last 3 blocks, should be enough to guard against a block or two's reorg
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment seems still relevant. Why remove?

hw::reset_mode rst(hwdev);

hwdev.set_mode(hw::device::TRANSACTION_PARSE);
if (!hwdev.generate_key_derivation(tx_pub_key, keys.m_view_secret_key, derivation))
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the line 1133(before) tx_pub_key = pub_key_field.pub_key; was deleted, where does tx_pub_key get its value?

hwdev.set_mode(hw::device::TRANSACTION_PARSE);
const cryptonote::account_keys &keys = m_account.get_keys();

auto gender = [&](wallet2::is_out_data &iod) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Had to wonder for a while what this name means :D

@@ -1282,6 +1295,8 @@ namespace tools
std::string m_ring_database;
bool m_ring_history_saved;
std::unique_ptr<ringdb> m_ringdb;
boost::optional<crypto::chacha_key> m_ringdb_key;
unsigned int m_ringdb_key_refs;
Copy link
Contributor

Choose a reason for hiding this comment

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

Unused?

@@ -5791,6 +5810,7 @@ bool wallet2::get_ring(const crypto::chacha_key &key, const crypto::key_image &k

bool wallet2::get_rings(const crypto::hash &txid, std::vector<std::pair<crypto::key_image, std::vector<uint64_t>>> &outs)
{
key_ref kref(*this);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed?

@@ -2332,6 +2333,8 @@ bool wallet2::delete_address_book_row(std::size_t row_id) {
//----------------------------------------------------------------------------------------------------
void wallet2::refresh(uint64_t start_height, uint64_t & blocks_fetched, bool& received_money)
{
key_ref kref(*this);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, but it speeds up saving rings from incoming txes if there are several.

Speeds up syncing with a lot of outgoing outputs as key generation
runs Cryptonight.
@luigi1111 luigi1111 merged commit 0e4c7d0 into monero-project:master Jun 27, 2018
@stoffu stoffu mentioned this pull request Nov 6, 2018
stoffu added a commit to stoffu/monero that referenced this pull request Nov 22, 2018
Motivated by https://monero.stackexchange.com/questions/10483

Some exchanges appear to have customized the wallet software
in an inappropriate way, making the tx extra field partially
unreadable. PR monero-project#3716 changed the wallet behavior disallowing
such partially valid tx extra.

An example tx reported by the user is
e87c675a85f34ecac58a8846613d25062f1813e1023c552b705afad32b972c38
where the normal tx pubkey appears again with the aditional
tx pubkeys tag `04` which is inappropriate.
Agreene pushed a commit to Agreene/monero that referenced this pull request Jan 28, 2019
Motivated by https://monero.stackexchange.com/questions/10483

Some exchanges appear to have customized the wallet software
in an inappropriate way, making the tx extra field partially
unreadable. PR monero-project#3716 changed the wallet behavior disallowing
such partially valid tx extra.

An example tx reported by the user is
e87c675a85f34ecac58a8846613d25062f1813e1023c552b705afad32b972c38
where the normal tx pubkey appears again with the aditional
tx pubkeys tag `04` which is inappropriate.
HarrisonHesslink pushed a commit to EquilibriaCC/Equilibria that referenced this pull request Apr 29, 2019
Motivated by https://monero.stackexchange.com/questions/10483

Some exchanges appear to have customized the wallet software
in an inappropriate way, making the tx extra field partially
unreadable. PR monero-project#3716 changed the wallet behavior disallowing
such partially valid tx extra.

An example tx reported by the user is
e87c675a85f34ecac58a8846613d25062f1813e1023c552b705afad32b972c38
where the normal tx pubkey appears again with the aditional
tx pubkeys tag `04` which is inappropriate.
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

5 participants