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

[r2r] HD wallet tx history #1285

Merged
merged 14 commits into from
Jun 13, 2022
Merged

[r2r] HD wallet tx history #1285

merged 14 commits into from
Jun 13, 2022

Conversation

sergeyboyko0791
Copy link

@sergeyboyko0791 sergeyboyko0791 commented May 17, 2022

#964
#740

  • Add BeBigUint to mm2_db::indexed_db;
  • Add IndexedDbTxHistoryStorage;
  • Extend mm2_db::indexed_db::DbTable with the methods that take multiple index keys as arguments;
  • Add Transaction History V2 WalletId (that may include HD wallet rmd160);
  • Add SqlQuery that acts like SqlBuilder but also manages SQL parameters;
  • Add <WalletId>_tx_from_address, <WalletId>_tx_to_address SQL tables

Please also note that I've left TODO comments with questions and suggestions how to improve the performance at the next iterations.

* Add `coins::tx_history_storage` module
* Move `load_history`, `save_history`, `clear` from `TxHistoryDb` implementation to static functions
* Cover `BeBigUint` with unit tests
* Add `indexed_db::DbTable::count`, `indexed_db::DbTable::count_all`
* Add methods to `indexed_db::DbTable` that take multiple index keys as arguments
* Remove `DbTransaction::wait_for_complete` as it doesn't work properly
* Add tx history `WalletId`
* Add `SqlQuery` that acts like `SqlBuilder` but manages parameters
* Add `<WalletId>_tx_from_address`, `<WalletId>_tx_to_address` SQL tables
@sergeyboyko0791 sergeyboyko0791 changed the title [wip] HD wallet tx history [r2r] HD wallet tx history May 31, 2022
@sergeyboyko0791 sergeyboyko0791 self-assigned this May 31, 2022
Copy link
Member

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

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

Great work 🔥 Here are my notes and suggestions

mm2src/coins/tx_history_storage/mod.rs Show resolved Hide resolved
mm2src/db_common/src/sqlite.rs Outdated Show resolved Hide resolved
mm2src/mm2_db/src/indexed_db/db_driver.rs Outdated Show resolved Hide resolved
mm2src/mm2_db/src/indexed_db/drivers/cursor/cursor.rs Outdated Show resolved Hide resolved
mm2src/mm2_db/src/indexed_db/drivers/cursor/cursor.rs Outdated Show resolved Hide resolved
mm2src/mm2_db/src/indexed_db/drivers/object_store.rs Outdated Show resolved Hide resolved
mm2src/mm2_db/src/indexed_db/drivers/object_store.rs Outdated Show resolved Hide resolved
mm2src/mm2_db/src/indexed_db/indexed_cursor.rs Outdated Show resolved Hide resolved
mm2src/mm2_db/src/indexed_db/indexed_db.rs Show resolved Hide resolved
Copy link
Collaborator

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

Great Work! only one inquiry.

mm2src/coins/my_tx_history_v2.rs Show resolved Hide resolved
* Add `try_serialize_index_value` macro
* Avoid declaring public methods, traits, types
Copy link
Member

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

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

Thanks for great work! I have mostly cosmetic changes requests and a couple of questions.

mm2src/coins/tx_history_storage/tx_history_v2_tests.rs Outdated Show resolved Hide resolved
mm2src/coins/tx_history_storage/tx_history_v2_tests.rs Outdated Show resolved Hide resolved
mm2src/coins/tx_history_storage/tx_history_v2_tests.rs Outdated Show resolved Hide resolved
mm2src/coins/tx_history_storage/tx_history_v2_tests.rs Outdated Show resolved Hide resolved
mm2src/coins/tx_history_storage/tx_history_v2_tests.rs Outdated Show resolved Hide resolved
mm2src/coins/tx_history_storage/mod.rs Show resolved Hide resolved
mm2src/db_common/src/sql_query.rs Outdated Show resolved Hide resolved
mm2src/db_common/src/sql_query.rs Outdated Show resolved Hide resolved
* Move big JSON payloads to separate files
* Add the `indexed_db::test_transaction_abort_on_error` test
* Allow the `SqlQuery` to validate SQL identifiers and values
@artemii235
Copy link
Member

@sergeyboyko0791 Please solve git conflicts, so I can proceed with the next review iteration :)

artemii235
artemii235 previously approved these changes Jun 10, 2022
Copy link
Member

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

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

🔥

@artemii235
Copy link
Member

@ozkanonur @shamardy Could you please take one more look too?

shamardy
shamardy previously approved these changes Jun 10, 2022
Copy link
Collaborator

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

Only one non-blocker comment :)

Copy link
Member

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

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

Only 2 notes :)

mm2src/coins/my_tx_history_v2.rs Outdated Show resolved Hide resolved
mm2src/db_common/src/sqlite.rs Outdated Show resolved Hide resolved
* Avoid adding `mm2src/coins/for_tests/RICK_<ID>.json` files
Copy link
Member

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

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

great work 🔥

@artemii235 artemii235 merged commit dec68f5 into dev Jun 13, 2022
@artemii235 artemii235 deleted the hd-wallet-tx-history branch June 13, 2022 08:51
laruh pushed a commit that referenced this pull request Jun 16, 2022
#964
#740

* Restructure TX history storage modules
* Add `coins::tx_history_storage` module
* Move `load_history`, `save_history`, `clear` from `TxHistoryDb` implementation to static functions

* Add `BeBigUint` into `common::indexed_db`
* Cover `BeBigUint` with unit tests

* Add `IndexedDbTxHistoryStorage`
* Add `indexed_db::DbTable::count`, `indexed_db::DbTable::count_all`
* Add methods to `indexed_db::DbTable` that take multiple index keys as arguments
* Remove `DbTransaction::wait_for_complete` as it doesn't work properly

* Integrate HD addresses into TxHistory V2 Storage scheme
* Add tx history `WalletId`
* Add `SqlQuery` that acts like `SqlBuilder` but manages parameters
* Add `<WalletId>_tx_from_address`, `<WalletId>_tx_to_address` SQL tables

* Minor changes
* Add `try_serialize_index_value` macro
* Avoid declaring public methods, traits, types

* Fix PR issues
* Move big JSON payloads to separate files
* Add the `indexed_db::test_transaction_abort_on_error` test
* Allow the `SqlQuery` to validate SQL identifiers and values

* Combine `tx_from_address` and `tx_to_address` tables into single `tx_address`

* Fix clippy warning

* Fix PR issues
* Avoid adding `mm2src/coins/for_tests/RICK_<ID>.json` files

* Minor changes
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.

4 participants