From d9fbad0b71c6c24a6451995daf09299824f88ac8 Mon Sep 17 00:00:00 2001 From: Brian Corbin Date: Wed, 4 May 2022 15:08:24 -0700 Subject: [PATCH] Release/v1.7.0 (#288) * Fix usage of SQLite database transactions. We were getting issues because of using deferred transactions for SQLite writes. Fixed the issue by using immediate transactions with exponential backoff. This strategy allows much higher throughput, with a benchmark creating 100 addresses in 485 milliseconds. Before the fix, we could get less than ten a second before database locked errors occurred. * Latest migration tester (#281) * feature/limit offset optional (#283) --- Cargo.lock | 2 +- cli/mobilecoin/cli.py | 4 +- cli/mobilecoin/client.py | 26 +- cli/test/client_tests.py | 4 +- cli/test/stress.py | 126 ++++ docs/SUMMARY.md | 3 - docs/accounts/address/README.md | 2 - .../address/get_addresses_for_account.md | 4 +- .../address/get_all_addresses_for_account.md | 71 --- docs/gift-codes/gift-code/build_gift_code.md | 2 +- docs/transactions/transaction-log/README.md | 1 - .../get_all_transaction_logs_for_account.md | 99 --- .../get_transaction_logs_for_account.md | 6 +- .../build_and_submit_transaction.md | 2 +- .../transaction/build_transaction.md | 2 +- docs/transactions/txo/README.md | 4 +- .../txo/get_all_txos_for_account.md | 158 ----- docs/transactions/txo/get_txos_for_account.md | 6 +- .../txo/get_txos_for_view_only_account.md | 6 +- full-service/Cargo.toml | 2 +- full-service/src/bin/main.rs | 24 +- full-service/src/db/account.rs | 82 +-- full-service/src/db/assigned_subaddress.rs | 60 +- full-service/src/db/gift_code.rs | 40 +- .../db/migration_testing/migration_testing.rs | 62 ++ full-service/src/db/migration_testing/mod.rs | 5 + .../src/db/migration_testing/seed_accounts.rs | 48 ++ .../db/migration_testing/seed_gift_codes.rs | 165 +++++ .../src/db/migration_testing/seed_txos.rs | 120 ++++ full-service/src/db/mod.rs | 5 +- full-service/src/db/transaction_log.rs | 128 ++-- full-service/src/db/txo.rs | 333 +++++----- full-service/src/db/view_only_account.rs | 63 +- .../src/db/view_only_transaction_log.rs | 20 +- full-service/src/db/view_only_txo.rs | 57 +- full-service/src/db/wallet_db.rs | 60 +- full-service/src/json_rpc/e2e.rs | 33 +- full-service/src/json_rpc/json_rpc_request.rs | 41 +- .../src/json_rpc/json_rpc_response.rs | 8 - full-service/src/json_rpc/wallet.rs | 117 +--- full-service/src/service/account.rs | 24 +- full-service/src/service/address.rs | 46 +- full-service/src/service/balance.rs | 256 ++++---- .../src/service/confirmation_number.rs | 20 +- full-service/src/service/gift_code.rs | 9 +- full-service/src/service/receipt.rs | 98 ++- full-service/src/service/sync.rs | 17 +- full-service/src/service/transaction.rs | 102 +-- .../src/service/transaction_builder.rs | 581 +++++++++--------- full-service/src/service/transaction_log.rs | 76 +-- full-service/src/service/txo.rs | 81 ++- full-service/src/service/view_only_account.rs | 41 +- .../src/service/view_only_transaction_log.rs | 17 +- full-service/src/service/view_only_txo.rs | 21 +- full-service/src/test_utils.rs | 12 +- 55 files changed, 1632 insertions(+), 1770 deletions(-) create mode 100644 cli/test/stress.py delete mode 100644 docs/accounts/address/get_all_addresses_for_account.md delete mode 100644 docs/transactions/transaction-log/get_all_transaction_logs_for_account.md delete mode 100644 docs/transactions/txo/get_all_txos_for_account.md create mode 100644 full-service/src/db/migration_testing/migration_testing.rs create mode 100644 full-service/src/db/migration_testing/mod.rs create mode 100644 full-service/src/db/migration_testing/seed_accounts.rs create mode 100644 full-service/src/db/migration_testing/seed_gift_codes.rs create mode 100644 full-service/src/db/migration_testing/seed_txos.rs diff --git a/Cargo.lock b/Cargo.lock index ebb6a80bb..dd914f9ab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2383,7 +2383,7 @@ dependencies = [ [[package]] name = "mc-full-service" -version = "1.6.0" +version = "1.7.0" dependencies = [ "anyhow", "base64 0.13.0", diff --git a/cli/mobilecoin/cli.py b/cli/mobilecoin/cli.py index 40a72a6ac..4fb6ef887 100644 --- a/cli/mobilecoin/cli.py +++ b/cli/mobilecoin/cli.py @@ -391,7 +391,7 @@ def history(self, account_id): account = self._load_account_prefix(account_id) account_id = account['account_id'] - transactions = self.client.get_all_transaction_logs_for_account(account_id) + transactions = self.client.get_transaction_logs_for_account(account_id, limit=1000) def block_key(t): submitted = t['submitted_block_index'] @@ -585,7 +585,7 @@ def address(self, action, **args): def address_list(self, account_id): account = self._load_account_prefix(account_id) - addresses = self.client.get_addresses_for_account(account['account_id']) + addresses = self.client.get_addresses_for_account(account['account_id'], limit=1000) print() print(_format_account_header(account)) diff --git a/cli/mobilecoin/client.py b/cli/mobilecoin/client.py index 25c4b5f86..2146c1d74 100755 --- a/cli/mobilecoin/client.py +++ b/cli/mobilecoin/client.py @@ -45,13 +45,17 @@ def _req(self, request_data): except ConnectionError: raise ConnectionError(f'Could not connect to wallet server at {self.url}.') + raw_response = None try: - response_data = json.load(r) + raw_response = r.read() + response_data = json.loads(raw_response) except ValueError: - raise ValueError('API returned invalid JSON:', r.text) + raise ValueError('API returned invalid JSON:', raw_response) if self.verbose: print(r.status, http.client.responses[r.status]) + print(repr(raw_response)) + print(len(raw_response), 'bytes') print(json.dumps(response_data, indent=2)) print() @@ -147,10 +151,14 @@ def export_account_secrets(self, account_id): }) return r['account_secrets'] - def get_all_txos_for_account(self, account_id): + def get_txos_for_account(self, account_id, offset=0, limit=100): r = self._req({ - "method": "get_all_txos_for_account", - "params": {"account_id": account_id} + "method": "get_txos_for_account", + "params": { + "account_id": account_id, + "offset": offset, + "limit": limit, + } }) return r['txo_map'] @@ -200,7 +208,7 @@ def assign_address_for_account(self, account_id, metadata=None): }) return r['address'] - def get_addresses_for_account(self, account_id, offset=0, limit=1000): + def get_addresses_for_account(self, account_id, offset=0, limit=100): r = self._req({ "method": "get_addresses_for_account", "params": { @@ -259,11 +267,13 @@ def submit_transaction(self, tx_proposal, account_id=None): }) return r['transaction_log'] - def get_all_transaction_logs_for_account(self, account_id): + def get_transaction_logs_for_account(self, account_id, offset=0, limit=100): r = self._req({ - "method": "get_all_transaction_logs_for_account", + "method": "get_transaction_logs_for_account", "params": { "account_id": account_id, + "offset": str(int(offset)), + "limit": str(int(limit)), }, }) return r['transaction_log_map'] diff --git a/cli/test/client_tests.py b/cli/test/client_tests.py index c2c4d8328..72052766f 100644 --- a/cli/test/client_tests.py +++ b/cli/test/client_tests.py @@ -113,7 +113,7 @@ def tests_with_wallet(c, source_wallet): # Check its balance and make sure it has txos. balance = c.poll_balance(source_account_id, seconds=60) assert pmob2mob(balance['unspent_pmob']) >= 1 - txos = c.get_all_txos_for_account(source_account_id) + txos = c.get_txos_for_account(source_account_id) assert len(txos) > 0 try: @@ -149,7 +149,7 @@ def test_transaction(c, source_account_id): assert pmob2mob(balance['unspent_pmob']) == Decimal('0.0') # Check transaction logs. - transaction_log_map = c.get_all_transaction_logs_for_account(dest_account_id) + transaction_log_map = c.get_transaction_logs_for_account(dest_account_id) amounts = [ pmob2mob(t['value_pmob']) for t in transaction_log_map.values() ] assert sorted( float(a) for a in amounts ) == [0.0996, 0.1], str(amounts) assert all( t['status'] == 'tx_status_succeeded' for t in transaction_log_map.values() ) diff --git a/cli/test/stress.py b/cli/test/stress.py new file mode 100644 index 000000000..c4e80b29b --- /dev/null +++ b/cli/test/stress.py @@ -0,0 +1,126 @@ +import aiohttp +import asyncio +from time import perf_counter + + +async def main(): + c = StressClient() + await test_account_create(c) + await test_subaddresses(c) + + +async def test_account_create(c, n=10): + accounts = await c.get_all_accounts() + num_accounts_before = len(accounts) + + account_ids = [] + async def create_one(i): + account = await c.create_account(str(i)) + account_ids.append(account['account_id']) + + with Timer() as timer: + await asyncio.gather(*[ + create_one(i) + for i in range(1, n+1) + ]) + + accounts = await c.get_all_accounts() + assert len(accounts) == num_accounts_before + n + + await asyncio.gather(*[ + c.remove_account(account_id) + for account_id in account_ids + ]) + + print('Created {} accounts in {:.3f}s'.format(n, timer.elapsed)) + + +async def test_subaddresses(c, n=10): + account = await c.create_account() + account_id = account['account_id'] + + addresses = await c.get_addresses_for_account(account_id) + assert len(addresses) == 2 + + with Timer() as timer: + await asyncio.gather(*[ + c.assign_address(account_id, str(i)) + for i in range(1, n+1) + ]) + + addresses = await c.get_addresses_for_account(account_id) + assert len(addresses) == 2 + n + + await c.remove_account(account_id) + + print('Created {} addresses in {:.3f}s'.format(n, timer.elapsed)) + + +class StressClient: + + async def _req(self, request_data): + default_params = { + "jsonrpc": "2.0", + "api_version": "2", + "id": 1, + } + request_data = {**request_data, **default_params} + async with aiohttp.ClientSession() as session: + async with session.post('http://localhost:9090/wallet', json=request_data) as response: + r = await response.json() + try: + return r['result'] + except KeyError: + print(r) + raise + + async def get_all_accounts(self): + r = await self._req({"method": "get_all_accounts"}) + return r['account_map'] + + async def create_account(self, name=''): + r = await self._req({ + "method": "create_account", + "params": {"name": name} + }) + return r['account'] + + async def remove_account(self, account_id): + return await self._req({ + "method": "remove_account", + "params": {"account_id": account_id} + }) + + async def assign_address(self, account_id, name=''): + return await self._req({ + "method": "assign_address_for_account", + "params": { + "account_id": account_id, + "metadata": name, + }, + }) + + async def get_addresses_for_account(self, account_id): + r = await self._req({ + "method": "get_addresses_for_account", + "params": { + "account_id": account_id, + "offset": "0", + "limit": "1000", + }, + }) + return r['address_map'] + + +class Timer: + def __enter__(self): + self._start_time = perf_counter() + return self + + def __exit__(self, *_): + end_time = perf_counter() + self.elapsed = end_time - self._start_time + + +if __name__ == '__main__': + asyncio.run(main()) diff --git a/docs/SUMMARY.md b/docs/SUMMARY.md index 1bbb44a61..176fcc93e 100644 --- a/docs/SUMMARY.md +++ b/docs/SUMMARY.md @@ -23,7 +23,6 @@ * [Export View Only Account Secrets](accounts/account-secrets/export_view_only_account_secrets.md) * [Address](accounts/address/README.md) * [Assign Address For Account](accounts/address/assign_address_for_account.md) - * [Get All Addresses For Account](accounts/address/get_all_addresses_for_account.md) * [Get Addresses For Account](accounts/address/get_addresses_for_account.md) * [Verify Address](accounts/address/verify_address.md) * [Balance](accounts/balance/README.md) @@ -42,7 +41,6 @@ * [Get TXO](transactions/txo/get_txo.md) * [Get MobileCoin Protocol TXO](transactions/txo/get_mc_protocol_txo.md) * [Get TXOs For Account](transactions/txo/get_txos_for_account.md) - * [Get All TXOs For Account](transactions/txo/get_all_txos_for_account.md) * [Get TXOs For Account](transactions/txo/get_txos_for_account.md) * [Get TXOs For View Only Account](transactions/txo/get_txos_for_view_only_account.md) * [Get All TXOs For Address](transactions/txo/get_txo_object.md) @@ -55,7 +53,6 @@ * [Transaction Log](transactions/transaction-log/README.md) * [Get Transaction Object](transactions/transaction-log/get_transaction_object.md) * [Get Transaction Log](transactions/transaction-log/get_transaction_log.md) - * [Get All Transaction Logs For Account](transactions/transaction-log/get_all_transaction_logs_for_account.md) * [Get Transaction Logs For Account](transactions/transaction-log/get_transaction_logs_for_account.md) * [Get All Transaction Logs For Block](transactions/transaction-log/get_all_transaction_logs_for_block.md) * [Get All Transaction Logs Ordered By Block](transactions/transaction-log/get_all_transaction_logs_ordered_by_block.md) diff --git a/docs/accounts/address/README.md b/docs/accounts/address/README.md index 74f657cd9..392faf762 100644 --- a/docs/accounts/address/README.md +++ b/docs/accounts/address/README.md @@ -22,8 +22,6 @@ Important: If you receive funds at a subaddress that has not yet been assigned, | `account_id` | string | A unique identifier for the assigned associated account. | | `metadata` | string | An arbitrary string attached to the object. | | `subaddress_index` | string \(uint64\) | The assigned subaddress index on the associated account. | -| `offset` | integer | The value to offset pagination requests. Requests will exclude all list items up to and including this object. | -| `limit` | integer | The limit of returned results. | ## Example diff --git a/docs/accounts/address/get_addresses_for_account.md b/docs/accounts/address/get_addresses_for_account.md index effa784ef..0048f9d01 100644 --- a/docs/accounts/address/get_addresses_for_account.md +++ b/docs/accounts/address/get_addresses_for_account.md @@ -9,8 +9,8 @@ description: Get assigned addresses for an account. | Required Param | Purpose | Requirements | | :--- | :--- | :--- | | `account_id` | The account on which to perform this action. | The account must exist in the wallet. | -| `offset` | integer | The value to offset pagination requests. Requests will exclude all list items up to and including this object. | -| `limit` | integer | The limit of returned results. This has a max value of 1000, and will return an error if exceeded. | +| `offset` | The pagination offset. Results start at the offset index. Optional, defaults to 0. | | +| `limit` | Limit for the number of results. Optional, defaults to 100 | | ## Example diff --git a/docs/accounts/address/get_all_addresses_for_account.md b/docs/accounts/address/get_all_addresses_for_account.md deleted file mode 100644 index b5f4cdf21..000000000 --- a/docs/accounts/address/get_all_addresses_for_account.md +++ /dev/null @@ -1,71 +0,0 @@ ---- -description: Get all assigned addresses for a given account. ---- - -# Get All Addresses For Account - -## Parameters - -| Required Param | Purpose | Requirements | -| :--- | :--- | :--- | -| `account_id` | The account on which to perform this action. | The account must exist in the wallet. | - -## Example - -{% tabs %} -{% tab title="Request Body" %} -```text -{ - "method": "get_all_addresses_for_account", - "params": { - "account_id": "a8c9c7acb96cf4ad9154eec9384c09f2c75a340b441924847fe5f60a41805bde" - }, - "jsonrpc": "2.0", - "id": 1 -} -``` -{% endtab %} - -{% tab title="Response" %} -```text -{ - "method": "get_all_addresses_for_account", - "result": { - "public_addresses": [ - "4bgkVAH1hs55dwLTGVpZER8ZayhqXbYqfuyisoRrmQPXoWcYQ3SQRTjsAytCiAgk21CRrVNysVw5qwzweURzDK9HL3rGXFmAAahb364kYe3", - "6prEWE8yEmHAznkZ3QUtHRmVf7q8DS6XpkjzecYCGMj7hVh8fivmCcujamLtugsvvmWE9P2WgTb2o7xGHw8FhiBr1hSrku1u9KKfRJFMenG", - "3P4GtGkp5UVBXUzBqirgj7QFetWn4PsFPsHBXbC6A8AXw1a9CMej969jneiN1qKcwdn6e1VtD64EruGVSFQ8wHk5xuBHndpV9WUGQ78vV7Z" - ], - "address_map": { - "4bgkVAH1hs55dwLTGVpZER8ZayhqXbYqfuyisoRrmQPXoWcYQ3SQRTjsAytCiAgk21CRrVNysVw5qwzweURzDK9HL3rGXFmAAahb364kYe3": { - "object": "address", - "public_address": "4bgkVAH1hs55dwLTGVpZER8ZayhqXbYqfuyisoRrmQPXoWcYQ3SQRTjsAytCiAgk21CRrVNysVw5qwzweURzDK9HL3rGXFmAAahb364kYe3", - "account_id": "3407fbbc250799f5ce9089658380c5fe152403643a525f581f359917d8d59d52", - "metadata": "Main", - "subaddress_index": "0" - }, - "6prEWE8yEmHAznkZ3QUtHRmVf7q8DS6XpkjzecYCGMj7hVh8fivmCcujamLtugsvvmWE9P2WgTb2o7xGHw8FhiBr1hSrku1u9KKfRJFMenG": { - "object": "address", - "public_address": "6prEWE8yEmHAznkZ3QUtHRmVf7q8DS6XpkjzecYCGMj7hVh8fivmCcujamLtugsvvmWE9P2WgTb2o7xGHw8FhiBr1hSrku1u9KKfRJFMenG", - "account_id": "3407fbbc250799f5ce9089658380c5fe152403643a525f581f359917d8d59d52", - "metadata": "Change", - "subaddress_index": "1" - }, - "3P4GtGkp5UVBXUzBqirgj7QFetWn4PsFPsHBXbC6A8AXw1a9CMej969jneiN1qKcwdn6e1VtD64EruGVSFQ8wHk5xuBHndpV9WUGQ78vV7Z": { - "object": "address", - "public_address": "3P4GtGkp5UVBXUzBqirgj7QFetWn4PsFPsHBXbC6A8AXw1a9CMej969jneiN1qKcwdn6e1VtD64EruGVSFQ8wHk5xuBHndpV9WUGQ78vV7Z", - "account_id": "3407fbbc250799f5ce9089658380c5fe152403643a525f581f359917d8d59d52", - "metadata": "", - "subaddress_index": "2" - } - } - }, - "error": null, - "jsonrpc": "2.0", - "id": 1, -} -verify_a -``` -{% endtab %} -{% endtabs %} - diff --git a/docs/gift-codes/gift-code/build_gift_code.md b/docs/gift-codes/gift-code/build_gift_code.md index 98b4fb2d0..b25cf2bff 100644 --- a/docs/gift-codes/gift-code/build_gift_code.md +++ b/docs/gift-codes/gift-code/build_gift_code.md @@ -13,7 +13,7 @@ description: Build a gift code in a tx_proposal that you can fund and submit to | Optional Param | Purpose | Requirements | | :--- | :--- | :--- | -| `input_txo_ids` | The specific TXOs to use as inputs to this transaction. | TXO IDs \(obtain from `get_all_txos_for_account`\) | +| `input_txo_ids` | The specific TXOs to use as inputs to this transaction. | TXO IDs \(obtain from `get_txos_for_account`\) | | `fee` | The fee amount to submit with this transaction. | If not provided, uses `MINIMUM_FEE` = .01 MOB. | | `tombstone_block` | The block after which this transaction expires. | If not provided, uses `cur_height` + 10. | | `max_spendable_value` | The maximum amount for an input TXO selected for this transaction. | | diff --git a/docs/transactions/transaction-log/README.md b/docs/transactions/transaction-log/README.md index cbdeb5df1..372c1ccdc 100644 --- a/docs/transactions/transaction-log/README.md +++ b/docs/transactions/transaction-log/README.md @@ -32,7 +32,6 @@ Due to the privacy properties of the MobileCoin ledger, transactions are ephemer | `comment` | string | An arbitrary string attached to the object. | | `failure_code` | integer | Code representing the cause of "failed" status. | | `failure_message` | string | Human parsable explanation of "failed" status. | -| `offset` | integer | The value to offset pagination requests for `transaction_log` list. Requests will exclude all list items up to and including this object. | ## Example diff --git a/docs/transactions/transaction-log/get_all_transaction_logs_for_account.md b/docs/transactions/transaction-log/get_all_transaction_logs_for_account.md deleted file mode 100644 index 910d797d9..000000000 --- a/docs/transactions/transaction-log/get_all_transaction_logs_for_account.md +++ /dev/null @@ -1,99 +0,0 @@ ---- -description: Get all transaction logs for a given account. ---- - -# Get All Transaction Logs For Account - -## Parameters - -| Required Param | Purpose | Requirements | -| :--- | :--- | :--- | -| `account_id` | The account on which to perform this action. | Account must exist in the wallet. | - -## Example - -{% tabs %} -{% tab title="Request Body" %} -```text -{ - "method": "get_all_transaction_logs_for_account", - "params": { - "account_id": "a4db032dcedc14e39608fe6f26deadf57e306e8c03823b52065724fb4d274c10" - }, - "jsonrpc": "2.0", - "id": 1 -} -``` -{% endtab %} - -{% tab title="Response" %} -```text -{ - "method": "get_all_transaction_logs_for_account", - "result": { - "transaction_log_ids": [ - "49da8168e26331fc9bc109d1e59f7ed572b453f232591de4196f9cefb381c3f4", - "ff1c85e7a488c2821110597ba75db30d913bb1595de549f83c6e8c56b06d70d1" - ], - "transaction_log_map": { - "49da8168e26331fc9bc109d1e59f7ed572b453f232591de4196f9cefb381c3f4": { - "object": "transaction_log", - "transaction_log_id": "49da8168e26331fc9bc109d1e59f7ed572b453f232591de4196f9cefb381c3f4", - "direction": "tx_direction_received", - "is_sent_recovered": null, - "account_id": "a4db032dcedc14e39608fe6f26deadf57e306e8c03823b52065724fb4d274c10", - "recipient_address_id": null, - "assigned_address_id": "7JvajhkAZYGmrpCY7ZpEiXRK5yW1ooTV7EWfDNu3Eyt572mH1wNb37BWiU6JqRUvgopPqSVZRexhXXpjF3wqLQR7HaJrcdbHmULujgFmzav", - "value_pmob": "8199980000000000", - "fee_pmob": null, - "submitted_block_index": null, - "finalized_block_index": "130689", - "status": "tx_status_succeeded", - "input_txo_ids": [], - "output_txo_ids": [ - "49da8168e26331fc9bc109d1e59f7ed572b453f232591de4196f9cefb381c3f4" - ], - "change_txo_ids": [], - "sent_time": null, - "comment": "", - "failure_code": null, - "failure_message": null - }, - "ff1c85e7a488c2821110597ba75db30d913bb1595de549f83c6e8c56b06d70d1": { - "object": "transaction_log", - "transaction_log_id": "ff1c85e7a488c2821110597ba75db30d913bb1595de549f83c6e8c56b06d70d1", - "direction": "tx_direction_sent", - "is_sent_recovered": null, - "account_id": "a4db032dcedc14e39608fe6f26deadf57e306e8c03823b52065724fb4d274c10", - "recipient_address_id": "7JvajhkAZYGmrpCY7ZpEiXRK5yW1ooTV7EWfDNu3Eyt572mH1wNb37BWiU6JqRUvgopPqSVZRexhXXpjF3wqLQR7HaJrcdbHmULujgFmzav", - "assigned_address_id": null, - "value_pmob": "8000000000008", - "fee_pmob": "10000000000", - "submitted_block_index": "152951", - "finalized_block_index": "152951", - "status": "tx_status_succeeded", - "input_txo_ids": [ - "135c3861be4034fccb8d0b329f86124cb6e2404cd4debf52a3c3a10cb4a7bdfb", - "c91b5f27e28460ef6c4f33229e70c4cfe6dc4bc1517a22122a86df9fb8e40815" - ], - "output_txo_ids": [ - "243494a0030bcbac40e87670b9288834047ef0727bcc6630a2fe2799439879ab" - ], - "change_txo_ids": [ - "58729797de0929eed37acb45225d3631235933b709c00015f46bfc002d5754fc" - ], - "sent_time": "2021-02-28 03:05:11 UTC", - "comment": "", - "failure_code": null, - "failure_message": null - } - } - }, - "error": null, - "jsonrpc": "2.0", - "id": 1, -} -``` -{% endtab %} -{% endtabs %} - diff --git a/docs/transactions/transaction-log/get_transaction_logs_for_account.md b/docs/transactions/transaction-log/get_transaction_logs_for_account.md index 05432f912..df9da680c 100644 --- a/docs/transactions/transaction-log/get_transaction_logs_for_account.md +++ b/docs/transactions/transaction-log/get_transaction_logs_for_account.md @@ -5,8 +5,8 @@ | Required Param | Purpose | Requirement | | :--- | :--- | :--- | | `transaction_log_id` | The transaction log ID to get. | Transaction log must exist in the wallet. | -| `offset` | integer | The value to offset pagination requests. Requests will exclude all list items up to and including this object. | -| `limit` | integer | The limit of returned results. This has a max value of 1000, and will return an error if exceeded. | +| `offset` | The pagination offset. Results start at the offset index. Optional, defaults to 0. | | +| `limit` | Limit for the number of results. Optional, defaults to 100 | | ## Example @@ -118,4 +118,4 @@ } ``` {% endtab %} -{% endtabs %} \ No newline at end of file +{% endtabs %} diff --git a/docs/transactions/transaction/build_and_submit_transaction.md b/docs/transactions/transaction/build_and_submit_transaction.md index ed2542161..1a6a6697d 100644 --- a/docs/transactions/transaction/build_and_submit_transaction.md +++ b/docs/transactions/transaction/build_and_submit_transaction.md @@ -17,7 +17,7 @@ description: >- | `recipient_public_address` | The recipient for this transaction | b58-encoded public address bytes | | `value_pmob` | The amount of MOB to send in this transaction | | | `addresses_and_values` | An array of public addresses and value tuples | addresses are b58-encoded public addresses, value is in pmob | -| `input_txo_ids` | Specific TXOs to use as inputs to this transaction | TXO IDs \(obtain from `get_all_txos_for_account`\) | +| `input_txo_ids` | Specific TXOs to use as inputs to this transaction | TXO IDs \(obtain from `get_txos_for_account`\) | | `fee` | The fee amount to submit with this transaction | If not provided, uses `MINIMUM_FEE` = .01 MOB | | `tombstone_block` | The block after which this transaction expires | If not provided, uses `cur_height` + 10 | | `max_spendable_value` | The maximum amount for an input TXO selected for this transaction | | diff --git a/docs/transactions/transaction/build_transaction.md b/docs/transactions/transaction/build_transaction.md index 002762a95..9f4e8e9d3 100644 --- a/docs/transactions/transaction/build_transaction.md +++ b/docs/transactions/transaction/build_transaction.md @@ -17,7 +17,7 @@ description: >- | `recipient_public_address` | The recipient for this transaction | b58-encoded public address bytes | | `value_pmob` | The amount of MOB to send in this transaction | | | `addresses_and_values` | An array of public addresses and value tuples | addresses are b58-encoded public addresses, value is in pmob | -| `input_txo_ids` | Specific TXOs to use as inputs to this transaction | TXO IDs (obtain from `get_all_txos_for_account`) | +| `input_txo_ids` | Specific TXOs to use as inputs to this transaction | TXO IDs (obtain from `get_txos_for_account`) | | `fee` | The fee amount to submit with this transaction | If not provided, uses `MINIMUM_FEE` = .01 MOB | | `tombstone_block` | The block after which this transaction expires | If not provided, uses `cur_height` + 10 | | `max_spendable_value` | The maximum amount for an input TXO selected for this transaction | | diff --git a/docs/transactions/txo/README.md b/docs/transactions/txo/README.md index 17d8880fe..f81417221 100644 --- a/docs/transactions/txo/README.md +++ b/docs/transactions/txo/README.md @@ -29,8 +29,6 @@ In order to construct a transaction, the wallet will select "Unspent Transaction | `assigned_address` | string \(uint64\) | The address corresponding to the subaddress index which was assigned as an intended sender for this TXO. | | `key_image` \(only on pending/spent\) | string \(hex\) | A fingerprint of the TXO derived from your private spend key materials, required to spend a TXO | | `confirmation` | string \(hex\) | A confirmation that the sender of the TXO can provide to validate that they participated in the construction of this TXO. | -| `offset` | integer | The value to offset pagination requests. Requests will exclude all list items up to and including this object. | -| `limit` | integer | The limit of returned results. | ## Example @@ -109,4 +107,4 @@ a minimal txo entity useful for view-only-accounts | `spent` | string | Whether or not this txo has been manually marked as spent. | | `txo_id_hex` | string | A synthetic ID created from properties of the TXO. This will be the same for a given TXO across systems. | -## Example \ No newline at end of file +## Example diff --git a/docs/transactions/txo/get_all_txos_for_account.md b/docs/transactions/txo/get_all_txos_for_account.md deleted file mode 100644 index b39eb248b..000000000 --- a/docs/transactions/txo/get_all_txos_for_account.md +++ /dev/null @@ -1,158 +0,0 @@ ---- -description: Get all TXOs for a given account. ---- - -# Get All TXOs For Account - -## Parameters - -| Parameter | Purpose | Requirements | -| :--- | :--- | :--- | -| `account_id` | The account on which to perform this action. | Account must exist in the wallet. | - -## Example - -{% tabs %} -{% tab title="Request Body" %} -```text -{ - "method": "get_all_txos_for_account", - "params": { - "account_id": "a8c9c7acb96cf4ad9154eec9384c09f2c75a340b441924847fe5f60a41805bde" - }, - "jsonrpc": "2.0", - "id": 1 -} -``` -{% endtab %} - -{% tab title="Response" %} -```text -{ - "method": "get_all_txos_for_account", - "result": { - "txo_ids": [ - "001cdcc1f0a22dc0ddcdaac6020cc03d919cbc3c36923f157b4a6bf0dc980167", - "00408833347550b046f0996afe92313745f76e307904686e93de5bab3590e9da", - "005b41a40be1401426f9a00965cc334e4703e4089adb8fa00616e7b25b92c6e5", - ... - ], - "txo_map": { - "001cdcc1f0a22dc0ddcdaac6020cc03d919cbc3c36923f157b4a6bf0dc980167": { - "account_status_map": { - "a4db032dcedc14e39608fe6f26deadf57e306e8c03823b52065724fb4d274c10": { - "txo_status": "spent", - "txo_type": "received" - } - }, - "assigned_subaddress": "7BeDc5jpZu72AuNavumc8qo8CRJijtQ7QJXyPo9dpnqULaPhe6GdaDNF7cjxkTrDfTcfMgWVgDzKzbvTTwp32KQ78qpx7bUnPYxAgy92caJ", - "e_fog_hint": "0a54bf0a5f37989b379b9db3e8937387c5033428b399d44ee524c02b53ce8b7fa7ffc7181a854255cefc68704f69eedd43a891d2ed65c9f6e4c0fc645c2bc156278395221100a4fc3a1d617d04f6eca8851e846a0100", - "is_spent_recovered": false, - "key_image": "0a20f041e3da520a6e3328d43a920b90bf87826a1602c9249cf6591dd32328a4544e", - "minted_account_id": null, - "object": "txo", - "confirmation": null, - "public_key": "0a201a592874a596aeb14cbeb1c7d3449cbd20dc8078ad7fff657e131d619145ef0a", - "received_account_id": "a4db032dcedc14e39608fe6f26deadf57e306e8c03823b52065724fb4d274c10", - "received_block_index": "128567", - "spent_block_index": "128569", - "subaddress_index": "0", - "target_key": "0a209e1067117870549a77a47de04bd810da052abfc23d60a0c433367bfc689b7428", - "txo_id": "001cdcc1f0a22dc0ddcdaac6020cc03d919cbc3c36923f157b4a6bf0dc980167", - "value_pmob": "990000000000" - }, - "84f30233774d728bb7844bed59d471fe55ee3680ab70ddc312840db0f978f3ba": { - "account_status_map": { - "36fdf8fbdaa35ad8e661209b8a7c7057f29bf16a1e399a34aa92c3873dfb853c": { - "txo_status": "unspent", - "txo_type": "received" - }, - "a4db032dcedc14e39608fe6f26deadf57e306e8c03823b52065724fb4d274c10": { - "txo_status": "secreted", - "txo_type": "minted" - } - }, - "assigned_subaddress": null, - "e_fog_hint": "0a5472b079a520696518cc7d7c3036e855cbbcf1a3e247db32ab2e62e835183077b862ef86ec4963a584650cc028eb645569f9de1392b88f8fd7fa07aa28c4e035fd5f4866f3db3d403a05d2adb5e4f2992c010b0100", - "is_spent_recovered": false, - "key_image": null, - "minted_account_id": "a4db032dcedc14e39608fe6f26deadf57e306e8c03823b52065724fb4d274c10", - "object": "txo", - "confirmation": "0a204488e153cce1e4bcdd4419eecb778f3d2d2b024b39aaa29532d2e47e238b2e31", - "public_key": "0a20e6736474f73e440686736bfd045d838c2b3bc056ffc647ad6b1c990f5a46b123", - "received_account_id": "36fdf8fbdaa35ad8e661209b8a7c7057f29bf16a1e399a34aa92c3873dfb853c", - "received_block_index": null, - "spent_block_index": null, - "subaddress_index": null, - "target_key": "0a20762d8a723aae2aa70cc11c62c91af715f957a7455b695641fe8c94210812cf1b", - "txo_id": "84f30233774d728bb7844bed59d471fe55ee3680ab70ddc312840db0f978f3ba", - "value_pmob": "200" - }, - "58c2c3780792ccf9c51014c7688a71f03732b633f8c5dfa49040fa7f51328280": { - "account_status_map": { - "a4db032dcedc14e39608fe6f26deadf57e306e8c03823b52065724fb4d274c10": { - "txo_status": "unspent", - "txo_type": "received" - } - }, - "assigned_subaddress": "7BeDc5jpZu72AuNavumc8qo8CRJijtQ7QJXyPo9dpnqULaPhe6GdaDNF7cjxkTrDfTcfMgWVgDzKzbvTTwp32KQ78qpx7bUnPYxAgy92caJ", - "e_fog_hint": "0a546f862ccf5e96a89b3ede770a70aa26ce8be704a7e5a73fff02d16ee1f694297b6c17d2e668d6181df047ae68730dfc7913b28aca66450ee1de0ca3b0bedb07664918899848f217bcbbe48be2ef40074ae5dd0100", - "is_spent_recovered": false, - "key_image": "0a20784ab38c4541ce23abbec6744431d6ae14101c49c6535b3e9bf3fd728db13848", - "minted_account_id": null, - "object": "txo", - "confirmation": null, - "public_key": "0a20d803a979c9ec0531f106363a885dde29101fcd70209f9ed686905512dfd14d5f", - "received_account_id": "a4db032dcedc14e39608fe6f26deadf57e306e8c03823b52065724fb4d274c10", - "received_block_index": "79", - "spent_block_index": null, - "subaddress_index": "0", - "target_key": "0a209abadbfcec6c81b3d184dc104e51cac4c4faa8bab4da21a3714901519810c20d", - "txo_id": "58c2c3780792ccf9c51014c7688a71f03732b633f8c5dfa49040fa7f51328280", - "value_pmob": "4000000000000" - }, - "b496f4f3ec3159bf48517aa7d9cda193ef8bfcac343f81eaed0e0a55849e4726": { - "account_status_map": { - "a4db032dcedc14e39608fe6f26deadf57e306e8c03823b52065724fb4d274c10": { - "txo_status": "secreted", - "txo_type": "minted" - } - }, - "assigned_subaddress": null, - "e_fog_hint": "0a54338fcf8609cf80dfe017bee2339b22b626af2957ef579ae8829f3d8e7fab6c20365b6a99727fcd5e3de7784fca7e1cbb77ec35e7f2c39ea47ef6121716119ba5a67f8a6026a6a6274e7262ea8ea8280782440100", - "is_spent_recovered": false, - "key_image": null, - "minted_account_id": "a4db032dcedc14e39608fe6f26deadf57e306e8c03823b52065724fb4d274c10", - "object": "txo", - "confirmation": null, - "public_key": "0a209432c589bb4e5101c26e935b70930dfe45c78417527fb994872ebd65fcb9c116", - "received_account_id": null, - "received_block_index": null, - "spent_block_index": null, - "subaddress_index": null, - "target_key": "0a208c75723e9b9a4af0c833bfe190c43900c3b41834cf37024f5fecfbe9919dff23", - "txo_id": "b496f4f3ec3159bf48517aa7d9cda193ef8bfcac343f81eaed0e0a55849e4726", - "value_pmob": "980000000000" - } - ] - } -} -``` -{% endtab %} -{% endtabs %} - -{% hint style="info" %} -Note, you may wish to filter TXOs using a tool like jq. For example, to get all unspent TXOs, you can use: - -```text -{ - "method": "get_all_txos_for_account", - "params": { - "account_id": "a4db032dcedc14e39608fe6f26deadf57e306e8c03823b52065724fb4d274c10" - }, - "jsonrpc": "2.0", - "id": 1, -} -``` -{% endhint %} - diff --git a/docs/transactions/txo/get_txos_for_account.md b/docs/transactions/txo/get_txos_for_account.md index b7b4d24c6..4ebe30ee0 100644 --- a/docs/transactions/txo/get_txos_for_account.md +++ b/docs/transactions/txo/get_txos_for_account.md @@ -9,8 +9,8 @@ description: Get TXOs for a given account with offset and limit parameters | Parameter | Purpose | Requirements | | :--- | :--- | :--- | | `account_id` | The account on which to perform this action. | Account must exist in the wallet. | -| `offset` | The value to offset pagination requests. Requests will exclude all list items up to and including this object. | | -| `limit` | The limit of returned results. | This has a max value of 1000, and will return an error if exceeded. | +| `offset` | The pagination offset. Results start at the offset index. Optional, defaults to 0. | | +| `limit` | Limit for the number of results. Optional, defaults to 100 | | ## Example @@ -144,4 +144,4 @@ description: Get TXOs for a given account with offset and limit parameters } ``` {% endtab %} -{% endtabs %} \ No newline at end of file +{% endtabs %} diff --git a/docs/transactions/txo/get_txos_for_view_only_account.md b/docs/transactions/txo/get_txos_for_view_only_account.md index 2bc50c0ca..7f1357ec0 100644 --- a/docs/transactions/txo/get_txos_for_view_only_account.md +++ b/docs/transactions/txo/get_txos_for_view_only_account.md @@ -9,8 +9,8 @@ description: Get view only TXOs for a given view only account with offset and li | Parameter | Purpose | Requirements | | :--- | :--- | :--- | | `account_id` | The account on which to perform this action. | Account must exist in the wallet. | -| `offset` | The value to offset pagination requests. Requests will exclude all list items up to and including this object. | | -| `limit` | The limit of returned results. | This has a max value of 1000, and will return an error if exceeded. | +| `offset` | The pagination offset. Results start at the offset index. Optional, defaults to 0. | | +| `limit` | Limit for the number of results. Optional, defaults to 100 | | ## Example @@ -71,4 +71,4 @@ description: Get view only TXOs for a given view only account with offset and li } ``` {% endtab %} -{% endtabs %} \ No newline at end of file +{% endtabs %} diff --git a/full-service/Cargo.toml b/full-service/Cargo.toml index 8730e9871..8b02a0ddc 100644 --- a/full-service/Cargo.toml +++ b/full-service/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mc-full-service" -version = "1.6.0" +version = "1.7.0" authors = ["MobileCoin"] edition = "2018" build = "build.rs" diff --git a/full-service/src/bin/main.rs b/full-service/src/bin/main.rs index d599708e0..264d77c73 100644 --- a/full-service/src/bin/main.rs +++ b/full-service/src/bin/main.rs @@ -3,8 +3,7 @@ //! MobileCoin wallet service #![feature(proc_macro_hygiene, decl_macro)] -use diesel::{connection::SimpleConnection, prelude::*, SqliteConnection}; -use diesel_migrations::embed_migrations; +use diesel::{prelude::*, SqliteConnection}; use dotenv::dotenv; use mc_attest_verifier::{MrSignerVerifier, Verifier, DEBUG_ENCLAVE}; use mc_common::logger::{create_app_logger, log, o, Logger}; @@ -31,8 +30,6 @@ use structopt::StructOpt; #[macro_use] extern crate diesel_migrations; -embed_migrations!("migrations/"); - // Exit codes. const EXIT_NO_DATABASE_CONNECTION: i32 = 2; const EXIT_WRONG_PASSWORD: i32 = 3; @@ -76,24 +73,7 @@ fn main() { eprintln!("Incorrect password for database {:?}.", config.wallet_db); exit(EXIT_WRONG_PASSWORD); }; - - // Our migrations sometimes violate foreign keys, so disable foreign key checks - // while we apply them. - // Unfortunately this has to happen outside the scope of a transaction. Quoting - // https://www.sqlite.org/pragma.html, - // "This pragma is a no-op within a transaction; foreign key constraint - // enforcement may only be enabled or disabled when there is no pending - // BEGIN or SAVEPOINT." - // Check foreign key constraints after the migration. If they fail, - // we will abort until the user resolves it. - conn.batch_execute("PRAGMA foreign_keys = OFF;") - .expect("failed disabling foreign keys"); - embedded_migrations::run_with_output(&conn, &mut std::io::stdout()) - .expect("failed running migrations"); - WalletDb::validate_foreign_keys(&conn); - conn.batch_execute("PRAGMA foreign_keys = ON;") - .expect("failed enabling foreign keys"); - + WalletDb::run_migrations(&conn); log::info!(logger, "Connected to database."); let wallet_db = WalletDb::new_from_url( diff --git a/full-service/src/db/account.rs b/full-service/src/db/account.rs index 8775e458b..1b435cc91 100644 --- a/full-service/src/db/account.rs +++ b/full-service/src/db/account.rs @@ -8,7 +8,7 @@ use crate::{ models::{Account, AssignedSubaddress, NewAccount, TransactionLog, Txo}, transaction_log::TransactionLogModel, txo::TxoModel, - WalletDbError, + Conn, WalletDbError, }, util::constants::{ DEFAULT_CHANGE_SUBADDRESS_INDEX, DEFAULT_FIRST_BLOCK_INDEX, DEFAULT_NEXT_SUBADDRESS_INDEX, @@ -18,11 +18,7 @@ use crate::{ }; use bip39::Mnemonic; -use diesel::{ - prelude::*, - r2d2::{ConnectionManager, PooledConnection}, - RunQueryDsl, -}; +use diesel::prelude::*; use mc_account_keys::{AccountKey, RootEntropy, RootIdentity}; use mc_account_keys_slip10::Slip10Key; use mc_crypto_digestible::{Digestible, MerlinTranscript}; @@ -60,7 +56,7 @@ pub trait AccountModel { fog_report_url: String, fog_report_id: String, fog_authority_spki: String, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(AccountID, String), WalletDbError>; /// Create an account. @@ -77,7 +73,7 @@ pub trait AccountModel { fog_report_url: String, fog_report_id: String, fog_authority_spki: String, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(AccountID, String), WalletDbError>; /// Create an account. @@ -94,7 +90,7 @@ pub trait AccountModel { next_subaddress_index: Option, name: &str, fog_enabled: bool, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(AccountID, String), WalletDbError>; /// Import account. @@ -108,7 +104,7 @@ pub trait AccountModel { fog_report_url: String, fog_report_id: String, fog_authority_spki: String, - conn: &PooledConnection>, + conn: &Conn, ) -> Result; /// Import account. @@ -122,53 +118,38 @@ pub trait AccountModel { fog_report_url: String, fog_report_id: String, fog_authority_spki: String, - conn: &PooledConnection>, + conn: &Conn, ) -> Result; /// List all accounts. /// /// Returns: /// * Vector of all Accounts in the DB - fn list_all( - conn: &PooledConnection>, - ) -> Result, WalletDbError>; + fn list_all(conn: &Conn) -> Result, WalletDbError>; /// Get a specific account. /// /// Returns: /// * Account - fn get( - account_id: &AccountID, - conn: &PooledConnection>, - ) -> Result; + fn get(account_id: &AccountID, conn: &Conn) -> Result; /// Get the accounts associated with the given Txo. - fn get_by_txo_id( - txo_id_hex: &str, - conn: &PooledConnection>, - ) -> Result, WalletDbError>; + fn get_by_txo_id(txo_id_hex: &str, conn: &Conn) -> Result, WalletDbError>; /// Update an account. /// The only updatable field is the name. Any other desired update requires /// adding a new account, and deleting the existing if desired. - fn update_name( - &self, - new_name: String, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError>; + fn update_name(&self, new_name: String, conn: &Conn) -> Result<(), WalletDbError>; /// Update the next block index this account will need to sync. fn update_next_block_index( &self, next_block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError>; /// Delete an account. - fn delete( - self, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError>; + fn delete(self, conn: &Conn) -> Result<(), WalletDbError>; } impl AccountModel for Account { @@ -181,7 +162,7 @@ impl AccountModel for Account { fog_report_url: String, fog_report_id: String, fog_authority_spki: String, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(AccountID, String), WalletDbError> { let fog_enabled = !fog_report_url.is_empty(); @@ -213,7 +194,7 @@ impl AccountModel for Account { fog_report_url: String, fog_report_id: String, fog_authority_spki: String, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(AccountID, String), WalletDbError> { let fog_enabled = !fog_report_url.is_empty(); @@ -247,7 +228,7 @@ impl AccountModel for Account { next_subaddress_index: Option, name: &str, fog_enabled: bool, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(AccountID, String), WalletDbError> { use crate::db::schema::accounts; @@ -323,7 +304,7 @@ impl AccountModel for Account { fog_report_url: String, fog_report_id: String, fog_authority_spki: String, - conn: &PooledConnection>, + conn: &Conn, ) -> Result { let (account_id, _public_address_b58) = Account::create_from_mnemonic( mnemonic, @@ -348,7 +329,7 @@ impl AccountModel for Account { fog_report_url: String, fog_report_id: String, fog_authority_spki: String, - conn: &PooledConnection>, + conn: &Conn, ) -> Result { let (account_id, _public_address_b58) = Account::create_from_root_entropy( root_entropy, @@ -364,9 +345,7 @@ impl AccountModel for Account { Account::get(&account_id, conn) } - fn list_all( - conn: &PooledConnection>, - ) -> Result, WalletDbError> { + fn list_all(conn: &Conn) -> Result, WalletDbError> { use crate::db::schema::accounts; Ok(accounts::table @@ -374,10 +353,7 @@ impl AccountModel for Account { .load::(conn)?) } - fn get( - account_id: &AccountID, - conn: &PooledConnection>, - ) -> Result { + fn get(account_id: &AccountID, conn: &Conn) -> Result { use crate::db::schema::accounts::dsl::{account_id_hex as dsl_account_id_hex, accounts}; match accounts @@ -393,10 +369,7 @@ impl AccountModel for Account { } } - fn get_by_txo_id( - txo_id_hex: &str, - conn: &PooledConnection>, - ) -> Result, WalletDbError> { + fn get_by_txo_id(txo_id_hex: &str, conn: &Conn) -> Result, WalletDbError> { let txo = Txo::get(txo_id_hex, conn)?; let mut accounts: Vec = Vec::::new(); @@ -414,11 +387,7 @@ impl AccountModel for Account { Ok(accounts) } - fn update_name( - &self, - new_name: String, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError> { + fn update_name(&self, new_name: String, conn: &Conn) -> Result<(), WalletDbError> { use crate::db::schema::accounts::dsl::{account_id_hex, accounts}; diesel::update(accounts.filter(account_id_hex.eq(&self.account_id_hex))) @@ -430,7 +399,7 @@ impl AccountModel for Account { fn update_next_block_index( &self, next_block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError> { use crate::db::schema::accounts::dsl::{account_id_hex, accounts}; diesel::update(accounts.filter(account_id_hex.eq(&self.account_id_hex))) @@ -439,10 +408,7 @@ impl AccountModel for Account { Ok(()) } - fn delete( - self, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError> { + fn delete(self, conn: &Conn) -> Result<(), WalletDbError> { use crate::db::schema::accounts::dsl::{account_id_hex, accounts}; // Delete transaction logs associated with this account diff --git a/full-service/src/db/assigned_subaddress.rs b/full-service/src/db/assigned_subaddress.rs index c3cebbcf7..312ec26b0 100644 --- a/full-service/src/db/assigned_subaddress.rs +++ b/full-service/src/db/assigned_subaddress.rs @@ -20,11 +20,8 @@ use mc_account_keys::AccountKey; use mc_crypto_keys::{CompressedRistrettoPublic, RistrettoPublic}; use mc_ledger_db::{Ledger, LedgerDB}; -use crate::db::WalletDbError; -use diesel::{ - prelude::*, - r2d2::{ConnectionManager, PooledConnection}, -}; +use crate::db::{Conn, WalletDbError}; +use diesel::prelude::*; pub trait AssignedSubaddressModel { /// Assign a subaddress to a contact. @@ -45,7 +42,7 @@ pub trait AssignedSubaddressModel { address_book_entry: Option, subaddress_index: u64, comment: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result; /// Create the next subaddress for a given account. @@ -56,21 +53,18 @@ pub trait AssignedSubaddressModel { account_id_hex: &str, comment: &str, ledger_db: &LedgerDB, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(String, i64), WalletDbError>; /// Get the AssignedSubaddress for a given assigned_subaddress_b58 - fn get( - public_address_b58: &str, - conn: &PooledConnection>, - ) -> Result; + fn get(public_address_b58: &str, conn: &Conn) -> Result; /// Get the Assigned Subaddress for a given index in an account, if it /// exists fn get_for_account_by_index( account_id_hex: &str, index: i64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result; /// Find an AssignedSubaddress by the subaddress spend public key @@ -79,22 +73,19 @@ pub trait AssignedSubaddressModel { /// * (subaddress_index, assigned_subaddress_b58) fn find_by_subaddress_spend_public_key( subaddress_spend_public_key: &RistrettoPublic, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(i64, String), WalletDbError>; /// List all AssignedSubaddresses for a given account. fn list_all( account_id_hex: &str, - offset: Option, - limit: Option, - conn: &PooledConnection>, + offset: Option, + limit: Option, + conn: &Conn, ) -> Result, WalletDbError>; /// Delete all AssignedSubaddresses for a given account. - fn delete_all( - account_id_hex: &str, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError>; + fn delete_all(account_id_hex: &str, conn: &Conn) -> Result<(), WalletDbError>; } impl AssignedSubaddressModel for AssignedSubaddress { @@ -103,7 +94,7 @@ impl AssignedSubaddressModel for AssignedSubaddress { address_book_entry: Option, subaddress_index: u64, comment: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result { use crate::db::schema::assigned_subaddresses; @@ -132,7 +123,7 @@ impl AssignedSubaddressModel for AssignedSubaddress { account_id_hex: &str, comment: &str, ledger_db: &LedgerDB, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(String, i64), WalletDbError> { use crate::db::schema::{ accounts::dsl::{account_id_hex as dsl_account_id_hex, accounts}, @@ -236,10 +227,7 @@ impl AssignedSubaddressModel for AssignedSubaddress { Ok((subaddress_b58, subaddress_index)) } - fn get( - public_address_b58: &str, - conn: &PooledConnection>, - ) -> Result { + fn get(public_address_b58: &str, conn: &Conn) -> Result { use crate::db::schema::assigned_subaddresses::dsl::{ assigned_subaddress_b58, assigned_subaddresses, }; @@ -265,7 +253,7 @@ impl AssignedSubaddressModel for AssignedSubaddress { fn get_for_account_by_index( account_id_hex: &str, index: i64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result { let account = Account::get(&AccountID(account_id_hex.to_string()), conn)?; @@ -278,7 +266,7 @@ impl AssignedSubaddressModel for AssignedSubaddress { fn find_by_subaddress_spend_public_key( subaddress_spend_public_key: &RistrettoPublic, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(i64, String), WalletDbError> { use crate::db::schema::assigned_subaddresses::{ account_id_hex, dsl::assigned_subaddresses, subaddress_index, subaddress_spend_key, @@ -306,9 +294,9 @@ impl AssignedSubaddressModel for AssignedSubaddress { fn list_all( account_id_hex: &str, - offset: Option, - limit: Option, - conn: &PooledConnection>, + offset: Option, + limit: Option, + conn: &Conn, ) -> Result, WalletDbError> { use crate::db::schema::assigned_subaddresses::{ account_id_hex as schema_account_id_hex, all_columns, dsl::assigned_subaddresses, @@ -319,7 +307,10 @@ impl AssignedSubaddressModel for AssignedSubaddress { .filter(schema_account_id_hex.eq(account_id_hex)); let addresses: Vec = if let (Some(o), Some(l)) = (offset, limit) { - addresses_query.offset(o).limit(l).load(conn)? + addresses_query + .offset(o as i64) + .limit(l as i64) + .load(conn)? } else { addresses_query.load(conn)? }; @@ -327,10 +318,7 @@ impl AssignedSubaddressModel for AssignedSubaddress { Ok(addresses) } - fn delete_all( - account_id_hex: &str, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError> { + fn delete_all(account_id_hex: &str, conn: &Conn) -> Result<(), WalletDbError> { use crate::db::schema::assigned_subaddresses::dsl::{ account_id_hex as schema_account_id_hex, assigned_subaddresses, }; diff --git a/full-service/src/db/gift_code.rs b/full-service/src/db/gift_code.rs index da8d799e1..09ed8a4e2 100644 --- a/full-service/src/db/gift_code.rs +++ b/full-service/src/db/gift_code.rs @@ -5,15 +5,11 @@ use crate::{ db::{ models::{GiftCode, NewGiftCode}, - WalletDbError, + Conn, WalletDbError, }, service::gift_code::EncodedGiftCode, }; -use diesel::{ - prelude::*, - r2d2::{ConnectionManager, PooledConnection}, - RunQueryDsl, -}; +use diesel::prelude::*; use displaydoc::Display; #[derive(Display, Debug)] @@ -41,32 +37,24 @@ pub trait GiftCodeModel { fn create( gift_code_b58: &EncodedGiftCode, value: i64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result; /// Get the details of a specific Gift Code. - fn get( - gift_code_b58: &EncodedGiftCode, - conn: &PooledConnection>, - ) -> Result; + fn get(gift_code_b58: &EncodedGiftCode, conn: &Conn) -> Result; /// Get all Gift Codes in this wallet. - fn list_all( - conn: &PooledConnection>, - ) -> Result, WalletDbError>; + fn list_all(conn: &Conn) -> Result, WalletDbError>; /// Delete a gift code. - fn delete( - self, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError>; + fn delete(self, conn: &Conn) -> Result<(), WalletDbError>; } impl GiftCodeModel for GiftCode { fn create( gift_code_b58: &EncodedGiftCode, value: i64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result { use crate::db::schema::gift_codes; @@ -83,10 +71,7 @@ impl GiftCodeModel for GiftCode { Ok(gift_code) } - fn get( - gift_code_b58: &EncodedGiftCode, - conn: &PooledConnection>, - ) -> Result { + fn get(gift_code_b58: &EncodedGiftCode, conn: &Conn) -> Result { use crate::db::schema::gift_codes::dsl::{gift_code_b58 as dsl_gift_code_b58, gift_codes}; match gift_codes @@ -102,9 +87,7 @@ impl GiftCodeModel for GiftCode { } } - fn list_all( - conn: &PooledConnection>, - ) -> Result, WalletDbError> { + fn list_all(conn: &Conn) -> Result, WalletDbError> { use crate::db::schema::gift_codes; Ok(gift_codes::table @@ -112,10 +95,7 @@ impl GiftCodeModel for GiftCode { .load::(conn)?) } - fn delete( - self, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError> { + fn delete(self, conn: &Conn) -> Result<(), WalletDbError> { use crate::db::schema::gift_codes::dsl::{gift_code_b58, gift_codes}; diesel::delete(gift_codes.filter(gift_code_b58.eq(&self.gift_code_b58))).execute(conn)?; diff --git a/full-service/src/db/migration_testing/migration_testing.rs b/full-service/src/db/migration_testing/migration_testing.rs new file mode 100644 index 000000000..a61d7641c --- /dev/null +++ b/full-service/src/db/migration_testing/migration_testing.rs @@ -0,0 +1,62 @@ +#[cfg(test)] +mod migration_testing { + use crate::{ + db::{ + account::AccountID, + migration_testing::{ + seed_accounts::{seed_accounts, test_accounts}, + seed_gift_codes::{seed_gift_codes, test_gift_codes}, + seed_txos::{seed_txos, test_txos}, + }, + }, + test_utils::{get_test_ledger, setup_wallet_service, WalletDbTestContext}, + }; + use diesel_migrations::{revert_latest_migration, run_pending_migrations}; + use mc_account_keys::PublicAddress; + use mc_common::logger::{test_with_logger, Logger}; + use rand::{rngs::StdRng, SeedableRng}; + + #[test_with_logger] + fn test_latest_migration(logger: Logger) { + // set up wallet and service. this will run all migrations + let mut rng: StdRng = SeedableRng::from_seed([20u8; 32]); + let known_recipients: Vec = Vec::new(); + let mut ledger_db = get_test_ledger(5, &known_recipients, 12, &mut rng); + let _db_test_context = WalletDbTestContext::default(); + let service = setup_wallet_service(ledger_db.clone(), logger.clone()); + let wallet_db = &service.wallet_db; + let conn = wallet_db.get_conn().unwrap(); + + // revert the last migration + revert_latest_migration(&conn).unwrap(); + + // seed the entities + let (txo_account, gift_code_account, gift_code_receiver_account) = seed_accounts(&service); + seed_txos(&conn, &mut ledger_db, &wallet_db, &logger, &txo_account); + let gift_codes = seed_gift_codes( + &conn, + &mut ledger_db, + &wallet_db, + &service, + &logger, + &gift_code_account, + &gift_code_receiver_account, + ); + + let txo_account_id = + AccountID::from(&mc_util_serial::decode(&txo_account.account_key).unwrap()); + + // validate expected state of entities in DB + test_accounts(&service); + test_txos(txo_account_id.clone(), &conn); + test_gift_codes(&gift_codes, &service); + + // run the last migration + run_pending_migrations(&conn).unwrap(); + + // validate expected state of entities in DB again, post-migration + test_accounts(&service); + test_txos(txo_account_id, &conn); + test_gift_codes(&gift_codes, &service); + } +} diff --git a/full-service/src/db/migration_testing/mod.rs b/full-service/src/db/migration_testing/mod.rs new file mode 100644 index 000000000..228ddb510 --- /dev/null +++ b/full-service/src/db/migration_testing/mod.rs @@ -0,0 +1,5 @@ +#[cfg(any(test))] +pub mod migration_testing; +pub mod seed_accounts; +pub mod seed_gift_codes; +pub mod seed_txos; diff --git a/full-service/src/db/migration_testing/seed_accounts.rs b/full-service/src/db/migration_testing/seed_accounts.rs new file mode 100644 index 000000000..f9c51a1e2 --- /dev/null +++ b/full-service/src/db/migration_testing/seed_accounts.rs @@ -0,0 +1,48 @@ +use crate::{ + db::models::Account, + service::{account::AccountService, WalletService}, +}; +use mc_connection_test_utils::MockBlockchainConnection; +use mc_fog_report_validation::MockFogPubkeyResolver; +use mc_ledger_db::LedgerDB; + +pub fn seed_accounts( + service: &WalletService, MockFogPubkeyResolver>, +) -> (Account, Account, Account) { + let txo_account = service + .create_account( + Some("txo_account".to_string()), + "".to_string(), + "".to_string(), + "".to_string(), + ) + .unwrap(); + + let gift_code_account = service + .create_account( + Some("gift_code_account".to_string()), + "".to_string(), + "".to_string(), + "".to_string(), + ) + .unwrap(); + + let gift_code_receiver_account = service + .create_account( + Some("gift_code_receiver_account".to_string()), + "".to_string(), + "".to_string(), + "".to_string(), + ) + .unwrap(); + + (txo_account, gift_code_account, gift_code_receiver_account) +} + +pub fn test_accounts( + service: &WalletService, MockFogPubkeyResolver>, +) { + let accounts = service.list_accounts().unwrap(); + + assert_eq!(accounts.len(), 3); +} diff --git a/full-service/src/db/migration_testing/seed_gift_codes.rs b/full-service/src/db/migration_testing/seed_gift_codes.rs new file mode 100644 index 000000000..d78c3a6f2 --- /dev/null +++ b/full-service/src/db/migration_testing/seed_gift_codes.rs @@ -0,0 +1,165 @@ +use crate::{ + db::{account::AccountID, models::Account, WalletDb}, + service::{ + gift_code::{EncodedGiftCode, GiftCodeService, GiftCodeStatus}, + WalletService, + }, + test_utils::{ + add_block_to_ledger_db, add_block_with_tx, add_block_with_tx_proposal, + manually_sync_account, MOB, + }, +}; +use diesel::{ + r2d2::{ConnectionManager, PooledConnection}, + SqliteConnection, +}; +use mc_account_keys::AccountKey; +use mc_common::logger::Logger; +use mc_connection_test_utils::MockBlockchainConnection; +use mc_crypto_rand::RngCore; +use mc_fog_report_validation::MockFogPubkeyResolver; +use mc_ledger_db::LedgerDB; +use mc_transaction_core::ring_signature::KeyImage; +use rand::{rngs::StdRng, SeedableRng}; + +pub struct SeedGiftCodesResult { + unsubmitted: EncodedGiftCode, + submitted: EncodedGiftCode, + claimed: EncodedGiftCode, +} +pub fn seed_gift_codes( + _conn: &PooledConnection>, + ledger_db: &mut LedgerDB, + wallet_db: &WalletDb, + service: &WalletService, MockFogPubkeyResolver>, + logger: &Logger, + account: &Account, + receiver_account: &Account, +) -> SeedGiftCodesResult { + let mut rng: StdRng = SeedableRng::from_seed([20u8; 32]); + + // Add a block with a transaction for the gifter account + let gifter_account_key: AccountKey = mc_util_serial::decode(&account.account_key).unwrap(); + let gifter_public_address = + &gifter_account_key.subaddress(account.main_subaddress_index as u64); + let gifter_account_id = AccountID(account.account_id_hex.to_string()); + + add_block_to_ledger_db( + ledger_db, + &vec![gifter_public_address.clone()], + 100 * MOB as u64, + &vec![KeyImage::from(rng.next_u64())], + &mut rng, + ); + manually_sync_account(ledger_db, wallet_db, &gifter_account_id, logger); + + // Create 3 gift codes + let (tx_proposal, gift_code_b58) = service + .build_gift_code( + &gifter_account_id, + 2 * MOB as u64, + Some("Gift code".to_string()), + None, + None, + None, + None, + ) + .unwrap(); + + // going to submit but not claim this code + let gift_code_1_submitted = service + .submit_gift_code( + &gifter_account_id, + &gift_code_b58.clone(), + &tx_proposal.clone(), + ) + .unwrap(); + + add_block_with_tx_proposal(ledger_db, tx_proposal); + manually_sync_account(&ledger_db, &service.wallet_db, &gifter_account_id, &logger); + + let (tx_proposal, gift_code_b58) = service + .build_gift_code( + &gifter_account_id, + 2 * MOB as u64, + Some("Gift code".to_string()), + None, + None, + None, + None, + ) + .unwrap(); + + // going to submit and claim this one + let gift_code_2_claimed = service + .submit_gift_code( + &gifter_account_id, + &gift_code_b58.clone(), + &tx_proposal.clone(), + ) + .unwrap(); + + add_block_with_tx_proposal(ledger_db, tx_proposal); + manually_sync_account(&ledger_db, &service.wallet_db, &gifter_account_id, &logger); + + // leave this code as pending + let (_tx_proposal, gift_code_b58_pending) = service + .build_gift_code( + &gifter_account_id, + 2 * MOB as u64, + Some("Gift code".to_string()), + None, + None, + None, + None, + ) + .unwrap(); + + // Claim the gift code to another account + manually_sync_account( + &ledger_db, + &service.wallet_db, + &AccountID(receiver_account.account_id_hex.clone()), + &logger, + ); + + let tx = service + .claim_gift_code( + &EncodedGiftCode(gift_code_2_claimed.gift_code_b58.clone()), + &AccountID(receiver_account.account_id_hex.clone()), + None, + ) + .unwrap(); + add_block_with_tx(ledger_db, tx); + manually_sync_account( + &ledger_db, + &service.wallet_db, + &AccountID(receiver_account.account_id_hex.clone()), + &logger, + ); + + SeedGiftCodesResult { + unsubmitted: gift_code_b58_pending, + submitted: EncodedGiftCode(gift_code_1_submitted.gift_code_b58), + claimed: EncodedGiftCode(gift_code_2_claimed.gift_code_b58), + } +} + +pub fn test_gift_codes( + gift_codes: &SeedGiftCodesResult, + service: &WalletService, MockFogPubkeyResolver>, +) { + let (status, _gift_code_value_opt, _memo) = service + .check_gift_code_status(&gift_codes.unsubmitted) + .unwrap(); + assert_eq!(status, GiftCodeStatus::GiftCodeSubmittedPending); + + let (status, _gift_code_value_opt, _memo) = service + .check_gift_code_status(&gift_codes.submitted) + .unwrap(); + assert_eq!(status, GiftCodeStatus::GiftCodeAvailable); + + let (status, _gift_code_value_opt, _memo) = + service.check_gift_code_status(&gift_codes.claimed).unwrap(); + assert_eq!(status, GiftCodeStatus::GiftCodeClaimed); +} diff --git a/full-service/src/db/migration_testing/seed_txos.rs b/full-service/src/db/migration_testing/seed_txos.rs new file mode 100644 index 000000000..465de08cc --- /dev/null +++ b/full-service/src/db/migration_testing/seed_txos.rs @@ -0,0 +1,120 @@ +use crate::{ + db::{ + account::AccountID, + models::{Account, TransactionLog, Txo}, + transaction_log::TransactionLogModel, + txo::TxoModel, + WalletDb, + }, + test_utils::{ + add_block_with_db_txos, add_block_with_tx_outs, create_test_minted_and_change_txos, + create_test_txo_for_recipient, manually_sync_account, MOB, + }, +}; +use diesel::{ + r2d2::{ConnectionManager, PooledConnection}, + SqliteConnection, +}; +use mc_common::logger::Logger; +use mc_crypto_rand::RngCore; +use mc_ledger_db::LedgerDB; +use mc_transaction_core::ring_signature::KeyImage; +use rand::{rngs::StdRng, SeedableRng}; + +// create 1 spent, 1 change (minted), and 1 orphaned txo +pub fn seed_txos( + _conn: &PooledConnection>, + ledger_db: &mut LedgerDB, + wallet_db: &WalletDb, + logger: &Logger, + account: &Account, +) { + let mut rng: StdRng = SeedableRng::from_seed([20u8; 32]); + // Create received txo for account + let account_key = mc_util_serial::decode(&account.account_key).unwrap(); + let (for_account_txo, for_account_key_image) = + create_test_txo_for_recipient(&account_key, 0, 1000 * MOB, &mut rng); + + // add this txo to the ledger + add_block_with_tx_outs( + ledger_db, + &[for_account_txo.clone()], + &[KeyImage::from(rng.next_u64())], + ); + + manually_sync_account( + &ledger_db, + &wallet_db, + &AccountID::from(&account_key), + &logger, + ); + + // "spend" the TXO by sending it to same account, but at a subaddress we + // have not yet assigned. At the DB layer, we accomplish this by + // constructing the output txos, then logging sent and received for this + // account. + let ((output_txo_id, _output_value), (change_txo_id, _change_value)) = + create_test_minted_and_change_txos( + account_key.clone(), + account_key.subaddress(4), + 33 * MOB, + wallet_db.clone(), + ledger_db.clone(), + logger.clone(), + ); + + add_block_with_db_txos( + ledger_db, + &wallet_db, + &[output_txo_id, change_txo_id], + &[KeyImage::from(for_account_key_image)], + ); + + manually_sync_account( + &ledger_db, + &wallet_db, + &AccountID::from(&account_key), + &logger, + ); +} + +pub fn test_txos( + account_id: AccountID, + conn: &PooledConnection>, +) { + // validate expected txo states + let txos = Txo::list_for_account(&account_id.to_string(), None, None, &conn).unwrap(); + assert_eq!(txos.len(), 3); + + // Check that we have 2 spendable (1 is orphaned) + let spendable: Vec<&Txo> = txos.iter().filter(|f| f.key_image.is_some()).collect(); + assert_eq!(spendable.len(), 2); + + // Check that we have one spent - went from [Received, Unspent] -> [Received, + // Spent] + let spent = Txo::list_spent(&account_id.to_string(), None, &conn).unwrap(); + assert_eq!(spent.len(), 1); + assert_eq!(spent[0].spent_block_index.clone().unwrap(), 13); + assert_eq!(spent[0].minted_account_id_hex, None); + + // Check that we have one orphaned - went from [Minted, Secreted] -> [Minted, + // Orphaned] + let orphaned = Txo::list_orphaned(&account_id.to_string(), &conn).unwrap(); + assert_eq!(orphaned.len(), 1); + assert!(orphaned[0].key_image.is_none()); + assert_eq!(orphaned[0].received_block_index.clone().unwrap(), 13); + assert!(orphaned[0].minted_account_id_hex.is_some()); + assert!(orphaned[0].received_account_id_hex.is_some()); + + // Check that we have one unspent (change) - went from [Minted, Secreted] -> + // [Minted, Unspent] + let unspent = Txo::list_unspent(&account_id.to_string(), None, &conn).unwrap(); + assert_eq!(unspent.len(), 1); + assert_eq!(unspent[0].received_block_index.clone().unwrap(), 13); + + // Check that a transaction log entry was created for each received TxOut (note: + // we are not creating submit logs in this test) + let transaction_logs = + TransactionLog::list_all(&account_id.to_string(), None, None, &conn).unwrap(); + assert_eq!(transaction_logs.len(), 3); +} diff --git a/full-service/src/db/mod.rs b/full-service/src/db/mod.rs index 3db28d5a7..d8b61419e 100644 --- a/full-service/src/db/mod.rs +++ b/full-service/src/db/mod.rs @@ -16,5 +16,8 @@ pub mod view_only_txo; mod wallet_db; mod wallet_db_error; -pub use wallet_db::WalletDb; +pub use wallet_db::{transaction, Conn, WalletDb}; pub use wallet_db_error::WalletDbError; + +#[cfg(any(test))] +pub mod migration_testing; diff --git a/full-service/src/db/transaction_log.rs b/full-service/src/db/transaction_log.rs index 3a8c6a2e2..eacdeeeb9 100644 --- a/full-service/src/db/transaction_log.rs +++ b/full-service/src/db/transaction_log.rs @@ -3,11 +3,7 @@ //! DB impl for the Transaction model. use chrono::Utc; -use diesel::{ - prelude::*, - r2d2::{ConnectionManager, PooledConnection}, - RunQueryDsl, -}; +use diesel::prelude::*; use mc_common::HashMap; use mc_crypto_digestible::{Digestible, MerlinTranscript}; use mc_mobilecoind::payments::TxProposal; @@ -23,7 +19,7 @@ use crate::db::{ TX_STATUS_SUCCEEDED, }, txo::{TxoID, TxoModel}, - WalletDbError, + Conn, WalletDbError, }; #[derive(Debug)] @@ -59,37 +55,26 @@ pub struct AssociatedTxos { pub trait TransactionLogModel { /// Get a transaction log from the TransactionId. - fn get( - transaction_id_hex: &str, - conn: &PooledConnection>, - ) -> Result; + fn get(transaction_id_hex: &str, conn: &Conn) -> Result; /// Get all transaction logs for the given block index. fn get_all_for_block_index( block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError>; /// Get all transaction logs ordered by finalized_block_index. - fn get_all_ordered_by_block_index( - conn: &PooledConnection>, - ) -> Result, WalletDbError>; + fn get_all_ordered_by_block_index(conn: &Conn) -> Result, WalletDbError>; /// Get the Txos associated with a given TransactionId, grouped according to /// their type. /// /// Returns: /// * AssoiatedTxos(inputs, outputs, change) - fn get_associated_txos( - &self, - conn: &PooledConnection>, - ) -> Result; + fn get_associated_txos(&self, conn: &Conn) -> Result; /// Select the TransactionLogs associated with a given TxoId. - fn select_for_txo( - txo_id_hex: &str, - conn: &PooledConnection>, - ) -> Result, WalletDbError>; + fn select_for_txo(txo_id_hex: &str, conn: &Conn) -> Result, WalletDbError>; /// List all TransactionLogs and their associated Txos for a given account. /// @@ -97,9 +82,9 @@ pub trait TransactionLogModel { /// * Vec(TransactionLog, AssociatedTxos(inputs, outputs, change)) fn list_all( account_id_hex: &str, - offset: Option, - limit: Option, - conn: &PooledConnection>, + offset: Option, + limit: Option, + conn: &Conn, ) -> Result, WalletDbError>; /// Log a received transaction. @@ -109,7 +94,7 @@ pub trait TransactionLogModel { txo_id_hex: &str, amount: u64, block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError>; /// Log a submitted transaction. @@ -127,32 +112,26 @@ pub trait TransactionLogModel { block_index: u64, comment: String, account_id_hex: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result; /// Remove all logs for an account - fn delete_all_for_account( - account_id_hex: &str, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError>; + fn delete_all_for_account(account_id_hex: &str, conn: &Conn) -> Result<(), WalletDbError>; fn update_tx_logs_associated_with_txo_to_succeeded( txo_id_hex: &str, finalized_block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError>; fn update_tx_logs_associated_with_txos_to_failed( txos: &[Txo], - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError>; } impl TransactionLogModel for TransactionLog { - fn get( - transaction_id_hex: &str, - conn: &PooledConnection>, - ) -> Result { + fn get(transaction_id_hex: &str, conn: &Conn) -> Result { use crate::db::schema::transaction_logs::dsl::{ transaction_id_hex as dsl_transaction_id_hex, transaction_logs, }; @@ -172,7 +151,7 @@ impl TransactionLogModel for TransactionLog { fn get_all_for_block_index( block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError> { use crate::db::schema::transaction_logs::{ all_columns, dsl::transaction_logs, finalized_block_index, @@ -186,9 +165,7 @@ impl TransactionLogModel for TransactionLog { Ok(matches) } - fn get_all_ordered_by_block_index( - conn: &PooledConnection>, - ) -> Result, WalletDbError> { + fn get_all_ordered_by_block_index(conn: &Conn) -> Result, WalletDbError> { use crate::db::schema::transaction_logs::{ all_columns, dsl::transaction_logs, finalized_block_index, }; @@ -201,10 +178,7 @@ impl TransactionLogModel for TransactionLog { Ok(matches) } - fn get_associated_txos( - &self, - conn: &PooledConnection>, - ) -> Result { + fn get_associated_txos(&self, conn: &Conn) -> Result { use crate::db::schema::{transaction_txo_types, txos}; // FIXME: WS-29 - use group_by rather than the processing below: @@ -239,10 +213,7 @@ impl TransactionLogModel for TransactionLog { }) } - fn select_for_txo( - txo_id_hex: &str, - conn: &PooledConnection>, - ) -> Result, WalletDbError> { + fn select_for_txo(txo_id_hex: &str, conn: &Conn) -> Result, WalletDbError> { use crate::db::schema::{transaction_logs, transaction_txo_types}; Ok(transaction_logs::table @@ -256,9 +227,9 @@ impl TransactionLogModel for TransactionLog { fn list_all( account_id_hex: &str, - offset: Option, - limit: Option, - conn: &PooledConnection>, + offset: Option, + limit: Option, + conn: &Conn, ) -> Result, WalletDbError> { use crate::db::schema::{transaction_logs, transaction_txo_types, txos}; @@ -281,7 +252,10 @@ impl TransactionLogModel for TransactionLog { let transactions: Vec<(TransactionLog, TransactionTxoType, Txo)> = if let (Some(o), Some(l)) = (offset, limit) { - transactions_query.offset(o).limit(l).load(conn)? + transactions_query + .offset(o as i64) + .limit(l as i64) + .load(conn)? } else { transactions_query.load(conn)? }; @@ -350,7 +324,7 @@ impl TransactionLogModel for TransactionLog { txo_id_hex: &str, amount: u64, block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError> { use crate::db::schema::transaction_txo_types; @@ -392,7 +366,7 @@ impl TransactionLogModel for TransactionLog { block_index: u64, comment: String, account_id_hex: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result { // Verify that the account exists. Account::get(&AccountID(account_id_hex.to_string()), conn)?; @@ -472,10 +446,7 @@ impl TransactionLogModel for TransactionLog { TransactionLog::get(&transaction_id.to_string(), conn) } - fn delete_all_for_account( - account_id_hex: &str, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError> { + fn delete_all_for_account(account_id_hex: &str, conn: &Conn) -> Result<(), WalletDbError> { use crate::db::schema::{ transaction_logs as cols, transaction_logs::dsl::transaction_logs, transaction_txo_types as types_cols, transaction_txo_types::dsl::transaction_txo_types, @@ -502,7 +473,7 @@ impl TransactionLogModel for TransactionLog { fn update_tx_logs_associated_with_txo_to_succeeded( txo_id_hex: &str, finalized_block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError> { use crate::db::schema::{transaction_logs, transaction_txo_types}; @@ -533,7 +504,7 @@ impl TransactionLogModel for TransactionLog { fn update_tx_logs_associated_with_txos_to_failed( txos: &[Txo], - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError> { use crate::db::schema::{transaction_logs, transaction_txo_types}; @@ -691,12 +662,13 @@ mod tests { ); // Build a transaction + let conn = wallet_db.get_conn().unwrap(); let (recipient, mut builder) = - builder_for_random_recipient(&account_key, &wallet_db, &ledger_db, &mut rng, &logger); + builder_for_random_recipient(&account_key, &ledger_db, &mut rng, &logger); builder.add_recipient(recipient.clone(), 50 * MOB).unwrap(); builder.set_tombstone(0).unwrap(); - builder.select_txos(None, false).unwrap(); - let tx_proposal = builder.build().unwrap(); + builder.select_txos(&conn, None, false).unwrap(); + let tx_proposal = builder.build(&conn).unwrap(); // Log submitted transaction from tx_proposal let tx_log = TransactionLog::log_submitted( @@ -704,7 +676,7 @@ mod tests { ledger_db.num_blocks().unwrap(), "".to_string(), &AccountID::from(&account_key).to_string(), - &wallet_db.get_conn().unwrap(), + &conn, ) .unwrap(); @@ -843,22 +815,23 @@ mod tests { ); // Build a transaction + let conn = wallet_db.get_conn().unwrap(); let (recipient, mut builder) = - builder_for_random_recipient(&account_key, &wallet_db, &ledger_db, &mut rng, &logger); + builder_for_random_recipient(&account_key, &ledger_db, &mut rng, &logger); // Add outlays all to the same recipient, so that we exceed u64::MAX in this tx let value = 100 * MOB - Mob::MINIMUM_FEE; builder.add_recipient(recipient.clone(), value).unwrap(); builder.set_tombstone(0).unwrap(); - builder.select_txos(None, false).unwrap(); - let tx_proposal = builder.build().unwrap(); + builder.select_txos(&conn, None, false).unwrap(); + let tx_proposal = builder.build(&conn).unwrap(); let tx_log = TransactionLog::log_submitted( tx_proposal.clone(), ledger_db.num_blocks().unwrap(), "".to_string(), &AccountID::from(&account_key).to_string(), - &wallet_db.get_conn().unwrap(), + &conn, ) .unwrap(); @@ -1022,14 +995,15 @@ mod tests { ); // Build a transaction for > i64::Max + let conn = wallet_db.get_conn().unwrap(); let (recipient, mut builder) = - builder_for_random_recipient(&account_key, &wallet_db, &ledger_db, &mut rng, &logger); + builder_for_random_recipient(&account_key, &ledger_db, &mut rng, &logger); builder .add_recipient(recipient.clone(), 10_000_000 * MOB) .unwrap(); builder.set_tombstone(0).unwrap(); - builder.select_txos(None, false).unwrap(); - let tx_proposal = builder.build().unwrap(); + builder.select_txos(&conn, None, false).unwrap(); + let tx_proposal = builder.build(&conn).unwrap(); assert_eq!(tx_proposal.outlays[0].value, 10_000_000_000_000_000_000); @@ -1039,7 +1013,7 @@ mod tests { ledger_db.num_blocks().unwrap(), "".to_string(), &AccountID::from(&account_key).to_string(), - &wallet_db.get_conn().unwrap(), + &conn, ) .unwrap(); @@ -1079,9 +1053,9 @@ mod tests { &logger, ); + let conn = wallet_db.get_conn().unwrap(); let mut builder = WalletTransactionBuilder::new( AccountID::from(&account_key).to_string(), - wallet_db.clone(), ledger_db.clone(), get_resolver_factory(&mut rng).unwrap(), logger.clone(), @@ -1091,8 +1065,8 @@ mod tests { .add_recipient(account_key.subaddress(0), 12 * MOB) .unwrap(); builder.set_tombstone(0).unwrap(); - builder.select_txos(None, false).unwrap(); - let tx_proposal = builder.build().unwrap(); + builder.select_txos(&conn, None, false).unwrap(); + let tx_proposal = builder.build(&conn).unwrap(); // Log submitted transaction from tx_proposal let tx_log = TransactionLog::log_submitted( @@ -1100,7 +1074,7 @@ mod tests { ledger_db.num_blocks().unwrap(), "".to_string(), &AccountID::from(&account_key).to_string(), - &wallet_db.get_conn().unwrap(), + &conn, ) .unwrap(); diff --git a/full-service/src/db/txo.rs b/full-service/src/db/txo.rs index 66fb877a0..f2f550687 100644 --- a/full-service/src/db/txo.rs +++ b/full-service/src/db/txo.rs @@ -2,11 +2,7 @@ //! DB impl for the Txo model. -use diesel::{ - prelude::*, - r2d2::{ConnectionManager, PooledConnection}, - RunQueryDsl, -}; +use diesel::prelude::*; use mc_account_keys::{AccountKey, PublicAddress}; use mc_common::HashMap; use mc_crypto_digestible::{Digestible, MerlinTranscript}; @@ -26,7 +22,7 @@ use crate::{ models::{ Account, AssignedSubaddress, NewTxo, Txo, TXO_USED_AS_CHANGE, TXO_USED_AS_OUTPUT, }, - WalletDbError, + Conn, WalletDbError, }, util::{b58::b58_encode_public_address, constants::DEFAULT_CHANGE_SUBADDRESS_INDEX}, }; @@ -82,7 +78,7 @@ pub trait TxoModel { value: u64, received_block_index: u64, account_id_hex: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result; /// Processes a TxProposal to create a new minted Txo and a change Txo. @@ -94,7 +90,7 @@ pub trait TxoModel { txo: &TxOut, tx_proposal: &TxProposal, outlay_index: usize, - conn: &PooledConnection>, + conn: &Conn, ) -> Result; /// Update an existing Txo to spendable by including its subaddress_index @@ -105,35 +101,35 @@ pub trait TxoModel { received_subaddress_index: Option, received_key_image: Option, block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError>; /// Update a Txo's received block count. fn update_received_block_index( &self, block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError>; /// Update a Txo's status to pending fn update_to_pending( &self, pending_tombstone_block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError>; /// Update a Txo's status to spent fn update_to_spent( txo_id_hex: &str, spent_block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError>; /// Update all Txo's that are pending with a pending_tombstone_block_index /// less than the target block index to unspent fn update_txos_exceeding_pending_tombstone_block_index_to_unspent( block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError>; /// Get all Txos associated with a given account. @@ -141,67 +137,55 @@ pub trait TxoModel { account_id_hex: &str, offset: Option, limit: Option, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError>; fn list_for_address( assigned_subaddress_b58: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError>; fn list_unspent( account_id_hex: &str, assigned_subaddress_b58: Option<&str>, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError>; /// Get a map from key images to unspent txos for this account. fn list_unspent_or_pending_key_images( account_id_hex: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError>; fn list_spent( account_id_hex: &str, assigned_subaddress_b58: Option<&str>, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError>; - fn list_secreted( - account_id_hex: &str, - conn: &PooledConnection>, - ) -> Result, WalletDbError>; + fn list_secreted(account_id_hex: &str, conn: &Conn) -> Result, WalletDbError>; - fn list_orphaned( - account_id_hex: &str, - conn: &PooledConnection>, - ) -> Result, WalletDbError>; + fn list_orphaned(account_id_hex: &str, conn: &Conn) -> Result, WalletDbError>; fn list_pending( account_id_hex: &str, assigned_subaddress_b58: Option<&str>, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError>; - fn list_minted( - account_id_hex: &str, - conn: &PooledConnection>, - ) -> Result, WalletDbError>; + fn list_minted(account_id_hex: &str, conn: &Conn) -> Result, WalletDbError>; fn list_pending_exceeding_block_index( account_id_hex: &str, block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError>; /// Get the details for a specific Txo. /// /// Returns: /// * Txo - fn get( - txo_id_hex: &str, - conn: &PooledConnection>, - ) -> Result; + fn get(txo_id_hex: &str, conn: &Conn) -> Result; /// Get several Txos by Txo public_keys /// @@ -209,7 +193,7 @@ pub trait TxoModel { /// * Vec fn select_by_public_key( public_keys: &[&CompressedRistrettoPublic], - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError>; /// Select several Txos by their TxoIds @@ -219,7 +203,7 @@ pub trait TxoModel { fn select_by_id( txo_ids: &[String], pending_tombstone_block_index: Option, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError>; /// Select a set of unspent Txos to reach a given value. @@ -231,7 +215,7 @@ pub trait TxoModel { target_value: u64, max_spendable_value: Option, pending_tombstone_block_index: Option, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError>; /// Validate a confirmation number for a Txo @@ -242,18 +226,13 @@ pub trait TxoModel { account_id: &AccountID, txo_id_hex: &str, confirmation: &TxOutConfirmationNumber, - conn: &PooledConnection>, + conn: &Conn, ) -> Result; - fn scrub_account( - account_id_hex: &str, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError>; + fn scrub_account(account_id_hex: &str, conn: &Conn) -> Result<(), WalletDbError>; /// Delete txos which are not referenced by any account or transaction. - fn delete_unreferenced( - conn: &PooledConnection>, - ) -> Result<(), WalletDbError>; + fn delete_unreferenced(conn: &Conn) -> Result<(), WalletDbError>; fn is_change(&self) -> bool; @@ -278,7 +257,7 @@ impl TxoModel for Txo { value: u64, received_block_index: u64, account_id_hex: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result { // Verify that the account exists. Account::get(&AccountID(account_id_hex.to_string()), conn)?; @@ -334,7 +313,7 @@ impl TxoModel for Txo { output: &TxOut, tx_proposal: &TxProposal, output_index: usize, - conn: &PooledConnection>, + conn: &Conn, ) -> Result { use crate::db::schema::txos; @@ -420,7 +399,7 @@ impl TxoModel for Txo { received_subaddress_index: Option, received_key_image: Option, block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError> { use crate::db::schema::txos; @@ -441,7 +420,7 @@ impl TxoModel for Txo { fn update_received_block_index( &self, block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError> { use crate::db::schema::txos::received_block_index; @@ -454,7 +433,7 @@ impl TxoModel for Txo { fn update_to_pending( &self, pending_tombstone_block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError> { use crate::db::schema::txos; @@ -467,7 +446,7 @@ impl TxoModel for Txo { fn update_to_spent( txo_id_hex: &str, spent_block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError> { use crate::db::schema::txos; @@ -482,7 +461,7 @@ impl TxoModel for Txo { fn update_txos_exceeding_pending_tombstone_block_index_to_unspent( block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError> { use crate::db::schema::txos; @@ -501,7 +480,7 @@ impl TxoModel for Txo { account_id_hex: &str, offset: Option, limit: Option, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError> { use crate::db::schema::txos; @@ -520,7 +499,7 @@ impl TxoModel for Txo { fn list_for_address( assigned_subaddress_b58: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError> { use crate::db::schema::txos; let subaddress = AssignedSubaddress::get(assigned_subaddress_b58, conn)?; @@ -534,7 +513,7 @@ impl TxoModel for Txo { fn list_unspent( account_id_hex: &str, assigned_subaddress_b58: Option<&str>, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError> { use crate::db::schema::txos; @@ -558,7 +537,7 @@ impl TxoModel for Txo { fn list_unspent_or_pending_key_images( account_id_hex: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError> { use crate::db::schema::txos; @@ -585,7 +564,7 @@ impl TxoModel for Txo { fn list_spent( account_id_hex: &str, assigned_subaddress_b58: Option<&str>, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError> { use crate::db::schema::txos; @@ -605,10 +584,7 @@ impl TxoModel for Txo { Ok(txos) } - fn list_secreted( - account_id_hex: &str, - conn: &PooledConnection>, - ) -> Result, WalletDbError> { + fn list_secreted(account_id_hex: &str, conn: &Conn) -> Result, WalletDbError> { use crate::db::schema::txos; // Secreted txos were minted by this account, but not received by this account, @@ -625,10 +601,7 @@ impl TxoModel for Txo { Ok(txos) } - fn list_orphaned( - account_id_hex: &str, - conn: &PooledConnection>, - ) -> Result, WalletDbError> { + fn list_orphaned(account_id_hex: &str, conn: &Conn) -> Result, WalletDbError> { use crate::db::schema::txos; let txos: Vec = txos::table @@ -642,7 +615,7 @@ impl TxoModel for Txo { fn list_pending( account_id_hex: &str, assigned_subaddress_b58: Option<&str>, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError> { use crate::db::schema::txos; @@ -667,7 +640,7 @@ impl TxoModel for Txo { fn list_pending_exceeding_block_index( account_id_hex: &str, block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError> { use crate::db::schema::txos; @@ -682,10 +655,7 @@ impl TxoModel for Txo { Ok(txos) } - fn list_minted( - account_id_hex: &str, - conn: &PooledConnection>, - ) -> Result, WalletDbError> { + fn list_minted(account_id_hex: &str, conn: &Conn) -> Result, WalletDbError> { use crate::db::schema::txos; let results = txos::table @@ -695,10 +665,7 @@ impl TxoModel for Txo { Ok(results) } - fn get( - txo_id_hex: &str, - conn: &PooledConnection>, - ) -> Result { + fn get(txo_id_hex: &str, conn: &Conn) -> Result { use crate::db::schema::txos; let txo = match txos::table @@ -719,7 +686,7 @@ impl TxoModel for Txo { fn select_by_public_key( public_keys: &[&CompressedRistrettoPublic], - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError> { use crate::db::schema::txos; @@ -736,23 +703,21 @@ impl TxoModel for Txo { fn select_by_id( txo_ids: &[String], pending_tombstone_block_index: Option, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError> { use crate::db::schema::txos; - conn.transaction(|| { - let txos: Vec = txos::table - .filter(txos::txo_id_hex.eq_any(txo_ids)) - .load(conn)?; + let txos: Vec = txos::table + .filter(txos::txo_id_hex.eq_any(txo_ids)) + .load(conn)?; - if let Some(pending_tombstone_block_index) = pending_tombstone_block_index { - for txo in &txos { - txo.update_to_pending(pending_tombstone_block_index, conn)?; - } + if let Some(pending_tombstone_block_index) = pending_tombstone_block_index { + for txo in &txos { + txo.update_to_pending(pending_tombstone_block_index, conn)?; } + } - Ok(txos) - }) + Ok(txos) } fn select_unspent_txos_for_value( @@ -760,115 +725,112 @@ impl TxoModel for Txo { target_value: u64, max_spendable_value: Option, pending_tombstone_block_index: Option, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError> { use crate::db::schema::txos; - conn.transaction(|| { - let spendable_txos: Vec = txos::table - .filter(txos::spent_block_index.is_null()) - .filter(txos::pending_tombstone_block_index.is_null()) - .filter(txos::subaddress_index.is_not_null()) - .filter(txos::key_image.is_not_null()) - .filter(txos::received_account_id_hex.eq(account_id_hex)) - .order_by(txos::value.desc()) - .load(conn)?; - - // The SQLite database cannot filter effectively on a u64 value, so filter for - // maximum value in memory. - let mut spendable_txos = if let Some(msv) = max_spendable_value { - spendable_txos - .into_iter() - .filter(|txo| (txo.value as u64) <= msv) - .collect() - } else { - spendable_txos - }; + let spendable_txos: Vec = txos::table + .filter(txos::spent_block_index.is_null()) + .filter(txos::pending_tombstone_block_index.is_null()) + .filter(txos::subaddress_index.is_not_null()) + .filter(txos::key_image.is_not_null()) + .filter(txos::received_account_id_hex.eq(account_id_hex)) + .order_by(txos::value.desc()) + .load(conn)?; - if spendable_txos.is_empty() { - return Err(WalletDbError::NoSpendableTxos); - } + // The SQLite database cannot filter effectively on a u64 value, so filter for + // maximum value in memory. + let mut spendable_txos = if let Some(msv) = max_spendable_value { + spendable_txos + .into_iter() + .filter(|txo| (txo.value as u64) <= msv) + .collect() + } else { + spendable_txos + }; - // The maximum spendable is limited by the maximal number of inputs we can use. - // Since the txos are sorted by decreasing value, this is the maximum - // value we can possibly spend in one transaction. - // Note, u128::Max = 340_282_366_920_938_463_463_374_607_431_768_211_455, which - // is far beyond the total number of pMOB in the MobileCoin system - // (250_000_000_000_000_000_000) - let max_spendable_in_wallet: u128 = spendable_txos + if spendable_txos.is_empty() { + return Err(WalletDbError::NoSpendableTxos); + } + + // The maximum spendable is limited by the maximal number of inputs we can use. + // Since the txos are sorted by decreasing value, this is the maximum + // value we can possibly spend in one transaction. + // Note, u128::Max = 340_282_366_920_938_463_463_374_607_431_768_211_455, which + // is far beyond the total number of pMOB in the MobileCoin system + // (250_000_000_000_000_000_000) + let max_spendable_in_wallet: u128 = spendable_txos + .iter() + .take(MAX_INPUTS as usize) + .map(|utxo| (utxo.value as u64) as u128) + .sum(); + // If we're trying to spend more than we have in the wallet, we may need to + // defrag + if target_value as u128 > max_spendable_in_wallet { + // See if we merged the UTXOs we would be able to spend this amount. + let total_unspent_value_in_wallet: u128 = spendable_txos .iter() - .take(MAX_INPUTS as usize) .map(|utxo| (utxo.value as u64) as u128) .sum(); - // If we're trying to spend more than we have in the wallet, we may need to - // defrag - if target_value as u128 > max_spendable_in_wallet { - // See if we merged the UTXOs we would be able to spend this amount. - let total_unspent_value_in_wallet: u128 = spendable_txos - .iter() - .map(|utxo| (utxo.value as u64) as u128) - .sum(); - if total_unspent_value_in_wallet >= target_value as u128 { - return Err(WalletDbError::InsufficientFundsFragmentedTxos); - } else { - return Err(WalletDbError::InsufficientFundsUnderMaxSpendable(format!( - "Max spendable value in wallet: {:?}, but target value: {:?}", - max_spendable_in_wallet, target_value - ))); - } + if total_unspent_value_in_wallet >= target_value as u128 { + return Err(WalletDbError::InsufficientFundsFragmentedTxos); + } else { + return Err(WalletDbError::InsufficientFundsUnderMaxSpendable(format!( + "Max spendable value in wallet: {:?}, but target value: {:?}", + max_spendable_in_wallet, target_value + ))); } + } - // Select the actual Txos to spend. We want to opportunistically fill up the - // input slots with dust, from any subaddress, so we take from the back - // of the Txo vec. This is a knapsack problem, and the selection could - // be improved. For now, we simply move the window of MAX_INPUTS up from - // the back of the sorted vector until we have a window with - // a large enough sum. - let mut selected_utxos: Vec = Vec::new(); - let mut total: u64 = 0; - loop { - if total >= target_value { - break; - } - - // Grab the next (smallest) utxo, in order to opportunistically sweep up dust - let next_utxo = spendable_txos.pop().ok_or_else(|| { - WalletDbError::InsufficientFunds(format!( - "Not enough Txos to sum to target value: {:?}", - target_value - )) - })?; - selected_utxos.push(next_utxo.clone()); - total += next_utxo.value as u64; - - // Cap at maximum allowed inputs. - if selected_utxos.len() > MAX_INPUTS as usize { - // Remove the lowest utxo. - let removed = selected_utxos.remove(0); - total -= removed.value as u64; - } + // Select the actual Txos to spend. We want to opportunistically fill up the + // input slots with dust, from any subaddress, so we take from the back + // of the Txo vec. This is a knapsack problem, and the selection could + // be improved. For now, we simply move the window of MAX_INPUTS up from + // the back of the sorted vector until we have a window with + // a large enough sum. + let mut selected_utxos: Vec = Vec::new(); + let mut total: u64 = 0; + loop { + if total >= target_value { + break; } - if selected_utxos.is_empty() || selected_utxos.len() > MAX_INPUTS as usize { - return Err(WalletDbError::InsufficientFunds( - "Logic error. Could not select Txos despite having sufficient funds" - .to_string(), - )); + // Grab the next (smallest) utxo, in order to opportunistically sweep up dust + let next_utxo = spendable_txos.pop().ok_or_else(|| { + WalletDbError::InsufficientFunds(format!( + "Not enough Txos to sum to target value: {:?}", + target_value + )) + })?; + selected_utxos.push(next_utxo.clone()); + total += next_utxo.value as u64; + + // Cap at maximum allowed inputs. + if selected_utxos.len() > MAX_INPUTS as usize { + // Remove the lowest utxo. + let removed = selected_utxos.remove(0); + total -= removed.value as u64; } - if let Some(pending_tombstone_block_index) = pending_tombstone_block_index { - for txo in &selected_utxos { - txo.update_to_pending(pending_tombstone_block_index, conn)?; - } + } + + if selected_utxos.is_empty() || selected_utxos.len() > MAX_INPUTS as usize { + return Err(WalletDbError::InsufficientFunds( + "Logic error. Could not select Txos despite having sufficient funds".to_string(), + )); + } + if let Some(pending_tombstone_block_index) = pending_tombstone_block_index { + for txo in &selected_utxos { + txo.update_to_pending(pending_tombstone_block_index, conn)?; } + } - Ok(selected_utxos) - }) + Ok(selected_utxos) } fn validate_confirmation( account_id: &AccountID, txo_id_hex: &str, confirmation: &TxOutConfirmationNumber, - conn: &PooledConnection>, + conn: &Conn, ) -> Result { let txo = Txo::get(txo_id_hex, conn)?; let public_key: RistrettoPublic = mc_util_serial::decode(&txo.public_key)?; @@ -877,10 +839,7 @@ impl TxoModel for Txo { Ok(confirmation.validate(&public_key, account_key.view_private_key())) } - fn scrub_account( - account_id_hex: &str, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError> { + fn scrub_account(account_id_hex: &str, conn: &Conn) -> Result<(), WalletDbError> { use crate::db::schema::txos; let txos_received_by_account = @@ -900,9 +859,7 @@ impl TxoModel for Txo { Ok(()) } - fn delete_unreferenced( - conn: &PooledConnection>, - ) -> Result<(), WalletDbError> { + fn delete_unreferenced(conn: &Conn) -> Result<(), WalletDbError> { use crate::db::schema::txos; let unreferenced_txos = txos::table @@ -1609,10 +1566,10 @@ mod tests { // Create TxProposal from the sender account, which contains the Confirmation // Number log::info!(logger, "Creating transaction builder"); + let conn = wallet_db.get_conn().unwrap(); let mut builder: WalletTransactionBuilder = WalletTransactionBuilder::new( AccountID::from(&sender_account_key).to_string(), - wallet_db.clone(), ledger_db.clone(), get_resolver_factory(&mut rng).unwrap(), logger.clone(), @@ -1620,9 +1577,9 @@ mod tests { builder .add_recipient(recipient_account_key.default_subaddress(), 50 * MOB) .unwrap(); - builder.select_txos(None, false).unwrap(); + builder.select_txos(&conn, None, false).unwrap(); builder.set_tombstone(0).unwrap(); - let proposal = builder.build().unwrap(); + let proposal = builder.build(&conn).unwrap(); // Sleep to make sure that the foreign keys exist std::thread::sleep(Duration::from_secs(3)); diff --git a/full-service/src/db/view_only_account.rs b/full-service/src/db/view_only_account.rs index 62acef0e1..ef3e17c9c 100644 --- a/full-service/src/db/view_only_account.rs +++ b/full-service/src/db/view_only_account.rs @@ -4,16 +4,14 @@ use crate::{ db::{ - models::{NewViewOnlyAccount, ViewOnlyAccount}, - schema, WalletDbError, + models::{NewViewOnlyAccount, ViewOnlyAccount, ViewOnlyTxo}, + schema, + view_only_txo::ViewOnlyTxoModel, + Conn, WalletDbError, }, util::encoding_helpers::{ristretto_to_vec, vec_to_hex}, }; -use diesel::{ - prelude::*, - r2d2::{ConnectionManager, PooledConnection}, - RunQueryDsl, -}; +use diesel::prelude::*; use mc_crypto_digestible::{Digestible, MerlinTranscript}; use mc_crypto_keys::{RistrettoPrivate, RistrettoPublic}; use std::{fmt, str}; @@ -45,45 +43,33 @@ pub trait ViewOnlyAccountModel { first_block_index: u64, import_block_index: u64, name: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result; /// Get a specific account. /// Returns: /// * ViewOnlyAccount - fn get( - account_id: &str, - conn: &PooledConnection>, - ) -> Result; + fn get(account_id: &str, conn: &Conn) -> Result; /// List all view-only-accounts. /// Returns: /// * Vector of all View Only Accounts in the DB - fn list_all( - conn: &PooledConnection>, - ) -> Result, WalletDbError>; + fn list_all(conn: &Conn) -> Result, WalletDbError>; /// Update an view-only-account name. /// The only updatable field is the name. Any other desired update requires /// adding a new account, and deleting the existing if desired. - fn update_name( - &self, - new_name: &str, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError>; + fn update_name(&self, new_name: &str, conn: &Conn) -> Result<(), WalletDbError>; /// Update the next block index this account will need to sync. fn update_next_block_index( &self, next_block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError>; /// Delete a view-only-account. - fn delete( - self, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError>; + fn delete(self, conn: &Conn) -> Result<(), WalletDbError>; } impl ViewOnlyAccountModel for ViewOnlyAccount { @@ -93,7 +79,7 @@ impl ViewOnlyAccountModel for ViewOnlyAccount { first_block_index: u64, import_block_index: u64, name: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result { use schema::view_only_accounts; @@ -117,10 +103,7 @@ impl ViewOnlyAccountModel for ViewOnlyAccount { ViewOnlyAccount::get(account_id_hex, conn) } - fn get( - account_id: &str, - conn: &PooledConnection>, - ) -> Result { + fn get(account_id: &str, conn: &Conn) -> Result { use schema::view_only_accounts::dsl::{ account_id_hex as dsl_account_id, view_only_accounts, }; @@ -138,9 +121,7 @@ impl ViewOnlyAccountModel for ViewOnlyAccount { } } - fn list_all( - conn: &PooledConnection>, - ) -> Result, WalletDbError> { + fn list_all(conn: &Conn) -> Result, WalletDbError> { use schema::view_only_accounts; Ok(view_only_accounts::table @@ -148,11 +129,7 @@ impl ViewOnlyAccountModel for ViewOnlyAccount { .load::(conn)?) } - fn update_name( - &self, - new_name: &str, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError> { + fn update_name(&self, new_name: &str, conn: &Conn) -> Result<(), WalletDbError> { use schema::view_only_accounts::dsl::{ account_id_hex as dsl_account_id, name as dsl_name, view_only_accounts, }; @@ -166,7 +143,7 @@ impl ViewOnlyAccountModel for ViewOnlyAccount { fn update_next_block_index( &self, next_block_index: u64, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(), WalletDbError> { use schema::view_only_accounts::dsl::{ account_id_hex as dsl_account_id, next_block_index as dsl_next_block, @@ -178,17 +155,15 @@ impl ViewOnlyAccountModel for ViewOnlyAccount { Ok(()) } - fn delete( - self, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError> { + fn delete(self, conn: &Conn) -> Result<(), WalletDbError> { use schema::view_only_accounts::dsl::{ account_id_hex as dsl_account_id, view_only_accounts, }; + // delete associated view-only-txos + ViewOnlyTxo::delete_all_for_account(&self.account_id_hex, conn)?; diesel::delete(view_only_accounts.filter(dsl_account_id.eq(&self.account_id_hex))) .execute(conn)?; - Ok(()) } } diff --git a/full-service/src/db/view_only_transaction_log.rs b/full-service/src/db/view_only_transaction_log.rs index 80734a8f7..587b54846 100644 --- a/full-service/src/db/view_only_transaction_log.rs +++ b/full-service/src/db/view_only_transaction_log.rs @@ -4,32 +4,28 @@ use crate::db::{ models::{NewViewOnlyTransactionLog, ViewOnlyTransactionLog}, - schema, WalletDbError, -}; -use diesel::{ - prelude::*, - r2d2::{ConnectionManager, PooledConnection}, - RunQueryDsl, + schema, Conn, WalletDbError, }; +use diesel::prelude::*; pub trait ViewOnlyTransactionLogModel { /// insert a new view only transaction log fn create( change_txo_id_hex: &str, input_txo_id_hex: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result; /// get a view only transaction log by change txo id fn get_by_change_txo_id( change_txo_id_hex: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result; /// get a all view only transaction logs for a change txo id fn find_all_by_change_txo_id( change_txo_id_hex: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError>; } @@ -37,7 +33,7 @@ impl ViewOnlyTransactionLogModel for ViewOnlyTransactionLog { fn create( change_txo_id_hex: &str, input_txo_id_hex: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result { use schema::view_only_transaction_logs; @@ -55,7 +51,7 @@ impl ViewOnlyTransactionLogModel for ViewOnlyTransactionLog { fn get_by_change_txo_id( txo_id_hex: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result { use schema::view_only_transaction_logs::dsl::{ change_txo_id_hex, view_only_transaction_logs, @@ -75,7 +71,7 @@ impl ViewOnlyTransactionLogModel for ViewOnlyTransactionLog { fn find_all_by_change_txo_id( txo_id_hex: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result, WalletDbError> { use schema::view_only_transaction_logs::dsl::{ change_txo_id_hex, view_only_transaction_logs, diff --git a/full-service/src/db/view_only_txo.rs b/full-service/src/db/view_only_txo.rs index ecd78e59f..13bd730b0 100644 --- a/full-service/src/db/view_only_txo.rs +++ b/full-service/src/db/view_only_txo.rs @@ -7,13 +7,9 @@ use crate::db::{ schema, txo::TxoID, view_only_account::ViewOnlyAccountModel, - WalletDbError, -}; -use diesel::{ - prelude::*, - r2d2::{ConnectionManager, PooledConnection}, - RunQueryDsl, + Conn, WalletDbError, }; +use diesel::prelude::*; use mc_transaction_core::tx::TxOut; pub trait ViewOnlyTxoModel { @@ -22,26 +18,20 @@ pub trait ViewOnlyTxoModel { tx_out: TxOut, value: u64, view_only_account_id_hex: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result; /// Get the details for a specific view only Txo. /// /// Returns: /// * ViewOnlyTxo - fn get( - txo_id_hex: &str, - conn: &PooledConnection>, - ) -> Result; + fn get(txo_id_hex: &str, conn: &Conn) -> Result; /// mark a group of view-only-txo as spent /// /// Returns: /// * () - fn set_spent( - txo_ids: Vec, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError>; + fn set_spent(txo_ids: Vec, conn: &Conn) -> Result<(), WalletDbError>; /// list view only txos for a view only account /// @@ -49,16 +39,13 @@ pub trait ViewOnlyTxoModel { /// * Vec fn list_for_account( account_id_hex: &str, - offset: Option, - limit: Option, - conn: &PooledConnection>, + offset: Option, + limit: Option, + conn: &Conn, ) -> Result, WalletDbError>; /// delete all view only txos for a view-only account - fn delete_all_for_account( - account_id_hex: &str, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError>; + fn delete_all_for_account(account_id_hex: &str, conn: &Conn) -> Result<(), WalletDbError>; } impl ViewOnlyTxoModel for ViewOnlyTxo { @@ -66,7 +53,7 @@ impl ViewOnlyTxoModel for ViewOnlyTxo { tx_out: TxOut, value: u64, view_only_account_id_hex: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result { use schema::view_only_txos; @@ -90,10 +77,7 @@ impl ViewOnlyTxoModel for ViewOnlyTxo { ViewOnlyTxo::get(&txo_id.to_string(), conn) } - fn get( - txo_id_hex: &str, - conn: &PooledConnection>, - ) -> Result { + fn get(txo_id_hex: &str, conn: &Conn) -> Result { use schema::view_only_txos; let txo = match view_only_txos::table @@ -114,9 +98,9 @@ impl ViewOnlyTxoModel for ViewOnlyTxo { fn list_for_account( account_id_hex: &str, - offset: Option, - limit: Option, - conn: &PooledConnection>, + offset: Option, + limit: Option, + conn: &Conn, ) -> Result, WalletDbError> { use schema::view_only_txos; @@ -124,7 +108,7 @@ impl ViewOnlyTxoModel for ViewOnlyTxo { .filter(view_only_txos::view_only_account_id_hex.eq(account_id_hex)); let txos: Vec = if let (Some(o), Some(l)) = (offset, limit) { - txos_query.offset(o).limit(l).load(conn)? + txos_query.offset(o as i64).limit(l as i64).load(conn)? } else { txos_query.load(conn)? }; @@ -132,10 +116,7 @@ impl ViewOnlyTxoModel for ViewOnlyTxo { Ok(txos) } - fn set_spent( - txo_ids: Vec, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError> { + fn set_spent(txo_ids: Vec, conn: &Conn) -> Result<(), WalletDbError> { use schema::view_only_txos::dsl::{ spent as dsl_spent, txo_id_hex as dsl_txo_id, view_only_txos, }; @@ -151,16 +132,12 @@ impl ViewOnlyTxoModel for ViewOnlyTxo { Ok(()) } - fn delete_all_for_account( - account_id_hex: &str, - conn: &PooledConnection>, - ) -> Result<(), WalletDbError> { + fn delete_all_for_account(account_id_hex: &str, conn: &Conn) -> Result<(), WalletDbError> { use schema::view_only_txos::dsl::{ view_only_account_id_hex as dsl_account_id, view_only_txos, }; diesel::delete(view_only_txos.filter(dsl_account_id.eq(account_id_hex))).execute(conn)?; - Ok(()) } } diff --git a/full-service/src/db/wallet_db.rs b/full-service/src/db/wallet_db.rs index bf38fd915..26ab7e3a6 100644 --- a/full-service/src/db/wallet_db.rs +++ b/full-service/src/db/wallet_db.rs @@ -3,10 +3,15 @@ use diesel::{ connection::SimpleConnection, prelude::*, r2d2::{ConnectionManager, Pool, PooledConnection}, - sql_types, + sql_types, SqliteConnection, }; +use diesel_migrations::embed_migrations; use mc_common::logger::{global_log, Logger}; -use std::{env, time::Duration}; +use std::{env, thread::sleep, time::Duration}; + +embed_migrations!("migrations/"); + +pub type Conn = PooledConnection>; #[derive(Debug)] pub struct ConnectionOptions { @@ -20,8 +25,9 @@ impl diesel::r2d2::CustomizeConnection { fn on_acquire(&self, conn: &mut SqliteConnection) -> Result<(), diesel::r2d2::Error> { (|| { - WalletDb::set_db_encryption_key_from_env(conn); - + if let Some(d) = self.busy_timeout { + conn.batch_execute(&format!("PRAGMA busy_timeout = {};", d.as_millis()))?; + } if self.enable_wal { conn.batch_execute(" PRAGMA journal_mode = WAL; -- better write-concurrency @@ -35,9 +41,7 @@ impl diesel::r2d2::CustomizeConnection } else { conn.batch_execute("PRAGMA foreign_keys = OFF;")?; } - if let Some(d) = self.busy_timeout { - conn.batch_execute(&format!("PRAGMA busy_timeout = {};", d.as_millis()))?; - } + WalletDb::set_db_encryption_key_from_env(conn); Ok(()) })() @@ -74,9 +78,7 @@ impl WalletDb { Ok(Self::new(pool, logger)) } - pub fn get_conn( - &self, - ) -> Result>, WalletDbError> { + pub fn get_conn(&self) -> Result { Ok(self.pool.get()?) } @@ -139,7 +141,45 @@ impl WalletDb { ); } } + + pub fn run_migrations(conn: &SqliteConnection) { + // Our migrations sometimes violate foreign keys, so disable foreign key checks + // while we apply them. + // This has to happen outside the scope of a transaction. Quoting + // https://www.sqlite.org/pragma.html, + // "This pragma is a no-op within a transaction; foreign key constraint + // enforcement may only be enabled or disabled when there is no pending + // BEGIN or SAVEPOINT." + // Check foreign key constraints after the migration. If they fail, + // we will abort until the user resolves it. + conn.batch_execute("PRAGMA foreign_keys = OFF;") + .expect("failed disabling foreign keys"); + embedded_migrations::run_with_output(conn, &mut std::io::stdout()) + .expect("failed running migrations"); + WalletDb::validate_foreign_keys(conn); + conn.batch_execute("PRAGMA foreign_keys = ON;") + .expect("failed enabling foreign keys"); + } +} + +/// Create an immediate SQLite transaction with retry. +/// Note: This function does not support nested transactions. +pub fn transaction(conn: &Conn, f: F) -> Result +where + F: Clone + FnOnce() -> Result, + E: From, +{ + for i in 0..NUM_RETRIES { + let r = conn.exclusive_transaction::(f.clone()); + if r.is_ok() || i == (NUM_RETRIES - 1) { + return r; + } + sleep(Duration::from_millis((BASE_DELAY_MS * 2_u32.pow(i)) as u64)); + } + panic!("Should never reach this point."); } +const BASE_DELAY_MS: u32 = 10; +const NUM_RETRIES: u32 = 5; /// Escape a string for consumption by SQLite. /// This function doubles all single quote characters within the string, then diff --git a/full-service/src/json_rpc/e2e.rs b/full-service/src/json_rpc/e2e.rs index 52f41638f..db58f2e41 100644 --- a/full-service/src/json_rpc/e2e.rs +++ b/full-service/src/json_rpc/e2e.rs @@ -1309,7 +1309,7 @@ mod e2e { let body = json!({ "jsonrpc": "2.0", "id": 1, - "method": "get_all_transaction_logs_for_account", + "method": "get_transaction_logs_for_account", "params": { "account_id": account_id, } @@ -1954,7 +1954,7 @@ mod e2e { let body = json!({ "jsonrpc": "2.0", "id": 1, - "method": "get_all_txos_for_account", + "method": "get_txos_for_account", "params": { "account_id": account_id, } @@ -1985,7 +1985,7 @@ mod e2e { let body = json!({ "jsonrpc": "2.0", "id": 1, - "method": "get_all_transaction_logs_for_account", + "method": "get_transaction_logs_for_account", "params": { "account_id": account_id, } @@ -2058,7 +2058,7 @@ mod e2e { let body = json!({ "jsonrpc": "2.0", "id": 1, - "method": "get_all_addresses_for_account", + "method": "get_addresses_for_account", "params": { "account_id": account_id, }, @@ -2600,7 +2600,7 @@ mod e2e { let body = json!({ "jsonrpc": "2.0", "id": 1, - "method": "get_all_txos_for_account", + "method": "get_txos_for_account", "params": { "account_id": account_id, } @@ -3165,7 +3165,7 @@ mod e2e { } #[test_with_logger] - fn test_get_all_txos(logger: Logger) { + fn test_get_txos(logger: Logger) { let mut rng: StdRng = SeedableRng::from_seed([20u8; 32]); let (client, mut ledger_db, db_ctx, _network_state) = setup(&mut rng, logger.clone()); @@ -3204,7 +3204,7 @@ mod e2e { let body = json!({ "jsonrpc": "2.0", "id": 1, - "method": "get_all_txos_for_account", + "method": "get_txos_for_account", "params": { "account_id": account_id, } @@ -3293,7 +3293,7 @@ mod e2e { let body = json!({ "jsonrpc": "2.0", "id": 1, - "method": "get_all_txos_for_account", + "method": "get_txos_for_account", "params": { "account_id": account_id, } @@ -4335,6 +4335,7 @@ mod e2e { fn test_e2e_hot_and_cold_view_only_flow(logger: Logger) { let mut rng: StdRng = SeedableRng::from_seed([20u8; 32]); let (client, mut ledger_db, db_ctx, _network_state) = setup(&mut rng, logger.clone()); + let wallet_db = db_ctx.get_db_instance(logger.clone()); // Add an account let body = json!({ @@ -4362,7 +4363,7 @@ mod e2e { ); manually_sync_account( &ledger_db, - &db_ctx.get_db_instance(logger.clone()), + &wallet_db, &AccountID(account_id.to_string()), &logger, ); @@ -4418,12 +4419,7 @@ mod e2e { let view_only_account_id = account_obj.get("account_id").unwrap().as_str().unwrap(); // sync view-only account and check balance - manually_sync_view_only_account( - &ledger_db, - &db_ctx.get_db_instance(logger.clone()), - &view_only_account_id, - &logger, - ); + manually_sync_view_only_account(&ledger_db, &wallet_db, &view_only_account_id, &logger); let body = json!({ "jsonrpc": "2.0", @@ -4479,12 +4475,7 @@ mod e2e { let payments_tx_proposal = mc_mobilecoind::payments::TxProposal::try_from(&json_tx_proposal).unwrap(); add_block_with_tx_proposal(&mut ledger_db, payments_tx_proposal); - manually_sync_view_only_account( - &ledger_db, - &db_ctx.get_db_instance(logger.clone()), - &view_only_account_id, - &logger, - ); + manually_sync_view_only_account(&ledger_db, &wallet_db, &view_only_account_id, &logger); assert_eq!( ledger_db.num_blocks().unwrap(), (BASE_TEST_BLOCK_HEIGHT + 2) as u64 diff --git a/full-service/src/json_rpc/json_rpc_request.rs b/full-service/src/json_rpc/json_rpc_request.rs index 256bd4eb1..2117f6d55 100644 --- a/full-service/src/json_rpc/json_rpc_request.rs +++ b/full-service/src/json_rpc/json_rpc_request.rs @@ -45,7 +45,12 @@ impl TryFrom<&JsonRPCRequest> for JsonCommandRequest { type Error = String; fn try_from(src: &JsonRPCRequest) -> Result { - let src_json: serde_json::Value = serde_json::json!(src); + let mut src_json: serde_json::Value = serde_json::json!(src); + + // Resolve deprecated method names to an alias. + let method = src_json.get_mut("method").ok_or("Missing method")?; + *method = method_alias(method.as_str().ok_or("Method is not a string")?).into(); + serde_json::from_value(src_json).map_err(|e| format!("Could not get value {:?}", e)) } } @@ -148,24 +153,15 @@ pub enum JsonCommandRequest { }, get_addresses_for_account { account_id: String, - offset: String, - limit: String, + offset: Option, + limit: Option, }, get_all_accounts, - get_all_addresses_for_account { - account_id: String, - }, get_all_gift_codes, - get_all_transaction_logs_for_account { - account_id: String, - }, get_all_transaction_logs_for_block { block_index: String, }, get_all_transaction_logs_ordered_by_block, - get_all_txos_for_account { - account_id: String, - }, get_all_txos_for_address { address: String, }, @@ -200,21 +196,21 @@ pub enum JsonCommandRequest { }, get_transaction_logs_for_account { account_id: String, - offset: String, - limit: String, + offset: Option, + limit: Option, }, get_txo { txo_id: String, }, get_txos_for_account { account_id: String, - offset: String, - limit: String, + offset: Option, + limit: Option, }, get_txos_for_view_only_account { account_id: String, - offset: String, - limit: String, + offset: Option, + limit: Option, }, get_view_only_account { account_id: String, @@ -284,3 +280,12 @@ pub enum JsonCommandRequest { }, version, } + +fn method_alias(m: &str) -> &str { + match m { + "get_all_addresses_for_account" => "get_addresses_for_account", + "get_all_transaction_logs_for_account" => "get_transaction_logs_for_account", + "get_all_txos_for_account" => "get_txos_for_account", + _ => m, + } +} diff --git a/full-service/src/json_rpc/json_rpc_response.rs b/full-service/src/json_rpc/json_rpc_response.rs index d726cf942..49aad17c8 100644 --- a/full-service/src/json_rpc/json_rpc_response.rs +++ b/full-service/src/json_rpc/json_rpc_response.rs @@ -201,10 +201,6 @@ pub enum JsonCommandResponse { get_all_gift_codes { gift_codes: Vec, }, - get_all_transaction_logs_for_account { - transaction_log_ids: Vec, - transaction_log_map: Map, - }, get_all_transaction_logs_for_block { transaction_log_ids: Vec, transaction_log_map: Map, @@ -212,10 +208,6 @@ pub enum JsonCommandResponse { get_all_transaction_logs_ordered_by_block { transaction_log_map: Map, }, - get_all_txos_for_account { - txo_ids: Vec, - txo_map: Map, - }, get_all_txos_for_address { txo_ids: Vec, txo_map: Map, diff --git a/full-service/src/json_rpc/wallet.rs b/full-service/src/json_rpc/wallet.rs index 1df4a615c..e29032461 100644 --- a/full-service/src/json_rpc/wallet.rs +++ b/full-service/src/json_rpc/wallet.rs @@ -491,13 +491,7 @@ where offset, limit, } => { - let o = offset.parse::().map_err(format_error)?; - let l = limit.parse::().map_err(format_error)?; - - if l > 1000 { - return Err(format_error("limit must not exceed 1000")); - } - + let (o, l) = page_helper(offset, limit)?; let addresses = service .get_addresses_for_account(&AccountID(account_id), Some(o), Some(l)) .map_err(format_error)?; @@ -542,31 +536,6 @@ where account_map, } } - JsonCommandRequest::get_all_addresses_for_account { account_id } => { - let addresses = service - .get_addresses_for_account(&AccountID(account_id), None, None) - .map_err(format_error)?; - let address_map: Map = Map::from_iter( - addresses - .iter() - .map(|a| { - ( - a.assigned_subaddress_b58.clone(), - serde_json::to_value(&(Address::from(a))) - .expect("Could not get json value"), - ) - }) - .collect::>(), - ); - - JsonCommandResponse::get_addresses_for_account { - public_addresses: addresses - .iter() - .map(|a| a.assigned_subaddress_b58.clone()) - .collect(), - address_map, - } - } JsonCommandRequest::get_all_gift_codes {} => JsonCommandResponse::get_all_gift_codes { gift_codes: service .list_gift_codes() @@ -575,30 +544,6 @@ where .map(GiftCode::from) .collect(), }, - JsonCommandRequest::get_all_transaction_logs_for_account { account_id } => { - let transaction_logs_and_txos = service - .list_transaction_logs(&AccountID(account_id), None, None) - .map_err(format_error)?; - let transaction_log_map: Map = Map::from_iter( - transaction_logs_and_txos - .iter() - .map(|(t, a)| { - ( - t.transaction_id_hex.clone(), - serde_json::json!(json_rpc::transaction_log::TransactionLog::new(t, a)), - ) - }) - .collect::>(), - ); - - JsonCommandResponse::get_all_transaction_logs_for_account { - transaction_log_ids: transaction_logs_and_txos - .iter() - .map(|(t, _a)| t.transaction_id_hex.to_string()) - .collect(), - transaction_log_map, - } - } JsonCommandRequest::get_all_transaction_logs_for_block { block_index } => { let transaction_logs_and_txos = service .get_all_transaction_logs_for_block( @@ -645,26 +590,6 @@ where transaction_log_map, } } - JsonCommandRequest::get_all_txos_for_account { account_id } => { - let txos = service - .list_txos(&AccountID(account_id), None, None) - .map_err(format_error)?; - let txo_map: Map = Map::from_iter( - txos.iter() - .map(|t| { - ( - t.txo_id_hex.clone(), - serde_json::to_value(Txo::from(t)).expect("Could not get json value"), - ) - }) - .collect::>(), - ); - - JsonCommandResponse::get_all_txos_for_account { - txo_ids: txos.iter().map(|t| t.txo_id_hex.clone()).collect(), - txo_map, - } - } JsonCommandRequest::get_all_txos_for_address { address } => { let txos = service .get_all_txos_for_address(&address) @@ -796,13 +721,7 @@ where offset, limit, } => { - let o = offset.parse::().map_err(format_error)?; - let l = limit.parse::().map_err(format_error)?; - - if l > 1000 { - return Err(format_error("limit must not exceed 1000")); - } - + let (o, l) = page_helper(offset, limit)?; let transaction_logs_and_txos = service .list_transaction_logs(&AccountID(account_id), Some(o), Some(l)) .map_err(format_error)?; @@ -837,15 +756,7 @@ where offset, limit, } => { - let o = offset - .parse::() - .map_err(format_invalid_request_error)?; - let l = limit.parse::().map_err(format_invalid_request_error)?; - - if l > 1000 { - return Err(format_error("limit must not exceed 1000")); - } - + let (o, l) = page_helper(offset, limit)?; let txos = service .list_txos(&AccountID(account_id), Some(o), Some(l)) .map_err(format_error)?; @@ -870,15 +781,7 @@ where offset, limit, } => { - let o = offset - .parse::() - .map_err(format_invalid_request_error)?; - let l = limit.parse::().map_err(format_invalid_request_error)?; - - if l > 1000 { - return Err(format_error("limit must not exceed 1000")); - } - + let (o, l) = page_helper(offset, limit)?; let txos = service .list_view_only_txos(&account_id, Some(o), Some(l)) .map_err(format_error)?; @@ -1135,6 +1038,18 @@ fn health() -> Result<(), ()> { Ok(()) } +fn page_helper(offset: Option, limit: Option) -> Result<(u64, u64), JsonRPCError> { + let offset = match offset { + Some(o) => o.parse::().map_err(format_error)?, + None => 0, // Default offset is zero, at the start of the records. + }; + let limit = match limit { + Some(l) => l.parse::().map_err(format_error)?, + None => 100, // Default page size is one hundred records. + }; + Ok((offset, limit)) +} + /// Returns an instance of a Rocket server. pub fn consensus_backed_rocket( rocket_config: rocket::Config, diff --git a/full-service/src/service/account.rs b/full-service/src/service/account.rs index 98828694a..ade3a960c 100644 --- a/full-service/src/service/account.rs +++ b/full-service/src/service/account.rs @@ -6,7 +6,7 @@ use crate::{ db::{ account::{AccountID, AccountModel}, models::Account, - WalletDbError, + transaction, WalletDbError, }, service::{ ledger::{LedgerService, LedgerServiceError}, @@ -16,7 +16,6 @@ use crate::{ }; use base64; use bip39::{Language, Mnemonic, MnemonicType}; -use diesel::Connection; use displaydoc::Display; use mc_account_keys::RootEntropy; use mc_account_keys_slip10; @@ -196,7 +195,7 @@ where let import_block_index = local_block_height; // -1 +1 let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { + transaction(&conn, || { let (account_id, _public_address_b58) = Account::create_from_mnemonic( &mnemonic, Some(first_block_index), @@ -208,7 +207,6 @@ where fog_authority_spki, &conn, )?; - let account = Account::get(&account_id, &conn)?; Ok(account) }) @@ -253,7 +251,7 @@ where let import_block = self.ledger_db.num_blocks()? - 1; let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { + transaction(&conn, || { Ok(Account::import( &mnemonic, name, @@ -293,7 +291,7 @@ where let import_block = self.ledger_db.num_blocks()? - 1; let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { + transaction(&conn, || { Ok(Account::import_legacy( &RootEntropy::from(&entropy_bytes), name, @@ -310,12 +308,12 @@ where fn list_accounts(&self) -> Result, AccountServiceError> { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| Ok(Account::list_all(&conn)?)) + Ok(Account::list_all(&conn)?) } fn get_account(&self, account_id: &AccountID) -> Result { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| Ok(Account::get(account_id, &conn)?)) + Ok(Account::get(account_id, &conn)?) } fn update_account_name( @@ -324,20 +322,16 @@ where name: String, ) -> Result { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { - Account::get(account_id, &conn)?.update_name(name, &conn)?; - Ok(Account::get(account_id, &conn)?) - }) + Account::get(account_id, &conn)?.update_name(name, &conn)?; + Ok(Account::get(account_id, &conn)?) } fn remove_account(&self, account_id: &AccountID) -> Result { log::info!(self.logger, "Deleting account {}", account_id,); - let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { + transaction(&conn, || { let account = Account::get(account_id, &conn)?; account.delete(&conn)?; - Ok(true) }) } diff --git a/full-service/src/service/address.rs b/full-service/src/service/address.rs index 28ae46338..632f4cdeb 100644 --- a/full-service/src/service/address.rs +++ b/full-service/src/service/address.rs @@ -5,7 +5,7 @@ use crate::{ db::{ account::AccountID, assigned_subaddress::AssignedSubaddressModel, - models::AssignedSubaddress, WalletDbError, + models::AssignedSubaddress, transaction, WalletDbError, }, service::WalletService, util::b58::b58_decode_public_address, @@ -14,7 +14,6 @@ use mc_common::logger::log; use mc_connection::{BlockchainConnection, UserTxConnection}; use mc_fog_report_validation::FogPubkeyResolver; -use diesel::Connection; use displaydoc::Display; /// Errors for the Address Service. @@ -55,8 +54,8 @@ pub trait AddressService { fn get_addresses_for_account( &self, account_id: &AccountID, - offset: Option, - limit: Option, + offset: Option, + limit: Option, ) -> Result, AddressServiceError>; fn get_address_for_account( @@ -79,35 +78,32 @@ where account_id: &AccountID, metadata: Option<&str>, ) -> Result { - let conn = &self.wallet_db.get_conn()?; - conn.transaction(|| { + let conn = self.wallet_db.get_conn()?; + transaction(&conn, || { let (public_address_b58, _subaddress_index) = AssignedSubaddress::create_next_for_account( &account_id.to_string(), metadata.unwrap_or(""), &self.ledger_db, - conn, + &conn, )?; - - Ok(AssignedSubaddress::get(&public_address_b58, conn)?) + Ok(AssignedSubaddress::get(&public_address_b58, &conn)?) }) } fn get_addresses_for_account( &self, account_id: &AccountID, - offset: Option, - limit: Option, + offset: Option, + limit: Option, ) -> Result, AddressServiceError> { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { - Ok(AssignedSubaddress::list_all( - &account_id.to_string(), - offset, - limit, - &conn, - )?) - }) + Ok(AssignedSubaddress::list_all( + &account_id.to_string(), + offset, + limit, + &conn, + )?) } fn get_address_for_account( @@ -116,13 +112,11 @@ where index: i64, ) -> Result { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { - Ok(AssignedSubaddress::get_for_account_by_index( - &account_id.to_string(), - index, - &conn, - )?) - }) + Ok(AssignedSubaddress::get_for_account_by_index( + &account_id.to_string(), + index, + &conn, + )?) } fn verify_address(&self, public_address: &str) -> Result { diff --git a/full-service/src/service/balance.rs b/full-service/src/service/balance.rs index 2f0b22f23..959468a5c 100644 --- a/full-service/src/service/balance.rs +++ b/full-service/src/service/balance.rs @@ -10,19 +10,13 @@ use crate::{ txo::TxoModel, view_only_account::ViewOnlyAccountModel, view_only_txo::ViewOnlyTxoModel, - WalletDbError, + Conn, WalletDbError, }, service::{ ledger::{LedgerService, LedgerServiceError}, WalletService, }, }; - -use diesel::{ - prelude::*, - r2d2::{ConnectionManager, PooledConnection}, - Connection, -}; use displaydoc::Display; use mc_common::HashMap; use mc_connection::{BlockchainConnection, UserTxConnection}; @@ -161,24 +155,22 @@ where let account_id_hex = &account_id.to_string(); let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { - let (unspent, pending, spent, secreted, orphaned) = - Self::get_balance_inner(account_id_hex, &conn)?; - - let network_block_height = self.get_network_block_height()?; - let local_block_height = self.ledger_db.num_blocks()?; - let account = Account::get(account_id, &conn)?; - - Ok(Balance { - unspent, - pending, - spent, - secreted, - orphaned, - network_block_height, - local_block_height, - synced_blocks: account.next_block_index as u64, - }) + let (unspent, pending, spent, secreted, orphaned) = + Self::get_balance_inner(account_id_hex, &conn)?; + + let network_block_height = self.get_network_block_height()?; + let local_block_height = self.ledger_db.num_blocks()?; + let account = Account::get(account_id, &conn)?; + + Ok(Balance { + unspent, + pending, + spent, + secreted, + orphaned, + network_block_height, + local_block_height, + synced_blocks: account.next_block_index as u64, }) } @@ -187,25 +179,23 @@ where account_id: &str, ) -> Result { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { - let txos = ViewOnlyTxo::list_for_account(account_id, None, None, &conn)?; - let total_value = txos.iter().map(|t| (t.value as u64) as u128).sum::(); - let spent = txos - .iter() - .filter(|t| t.spent) - .map(|t| (t.value as u64) as u128) - .sum::(); - - let network_block_height = self.get_network_block_height()?; - let local_block_height = self.ledger_db.num_blocks()?; - let account = ViewOnlyAccount::get(account_id, &conn)?; - - Ok(ViewOnlyBalance { - balance: total_value - spent, - network_block_height, - local_block_height, - synced_blocks: account.next_block_index as u64, - }) + let txos = ViewOnlyTxo::list_for_account(account_id, None, None, &conn)?; + let total_value = txos.iter().map(|t| (t.value as u64) as u128).sum::(); + let spent = txos + .iter() + .filter(|t| t.spent) + .map(|t| (t.value as u64) as u128) + .sum::(); + + let network_block_height = self.get_network_block_height()?; + let local_block_height = self.ledger_db.num_blocks()?; + let account = ViewOnlyAccount::get(account_id, &conn)?; + + Ok(ViewOnlyBalance { + balance: total_value - spent, + network_block_height, + local_block_height, + synced_blocks: account.next_block_index as u64, }) } @@ -214,44 +204,40 @@ where let local_block_height = self.ledger_db.num_blocks()?; let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { - let assigned_address = AssignedSubaddress::get(address, &conn)?; - - // Orphaned txos have no subaddress assigned, so none of these txos can - // be orphaned. - let orphaned: u128 = 0; - - let unspent = - Txo::list_unspent(&assigned_address.account_id_hex, Some(address), &conn)? - .iter() - .map(|txo| (txo.value as u64) as u128) - .sum::(); - let pending = - Txo::list_pending(&assigned_address.account_id_hex, Some(address), &conn)? - .iter() - .map(|txo| (txo.value as u64) as u128) - .sum::(); - let spent = Txo::list_spent(&assigned_address.account_id_hex, Some(address), &conn)? - .iter() - .map(|txo| (txo.value as u64) as u128) - .sum::(); - let secreted = Txo::list_secreted(&assigned_address.account_id_hex, &conn)? - .iter() - .map(|txo| (txo.value as u64) as u128) - .sum::(); - - let account = Account::get(&AccountID(assigned_address.account_id_hex), &conn)?; - - Ok(Balance { - unspent, - pending, - spent, - secreted, - orphaned, - network_block_height, - local_block_height, - synced_blocks: account.next_block_index as u64, - }) + let assigned_address = AssignedSubaddress::get(address, &conn)?; + + // Orphaned txos have no subaddress assigned, so none of these txos can + // be orphaned. + let orphaned: u128 = 0; + + let unspent = Txo::list_unspent(&assigned_address.account_id_hex, Some(address), &conn)? + .iter() + .map(|txo| (txo.value as u64) as u128) + .sum::(); + let pending = Txo::list_pending(&assigned_address.account_id_hex, Some(address), &conn)? + .iter() + .map(|txo| (txo.value as u64) as u128) + .sum::(); + let spent = Txo::list_spent(&assigned_address.account_id_hex, Some(address), &conn)? + .iter() + .map(|txo| (txo.value as u64) as u128) + .sum::(); + let secreted = Txo::list_secreted(&assigned_address.account_id_hex, &conn)? + .iter() + .map(|txo| (txo.value as u64) as u128) + .sum::(); + + let account = Account::get(&AccountID(assigned_address.account_id_hex), &conn)?; + + Ok(Balance { + unspent, + pending, + spent, + secreted, + orphaned, + network_block_height, + local_block_height, + synced_blocks: account.next_block_index as u64, }) } @@ -268,59 +254,57 @@ where let network_block_height = self.get_network_block_height()?; let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { - let accounts = Account::list_all(&conn)?; - let mut account_map = HashMap::default(); - let view_only_accounts = ViewOnlyAccount::list_all(&conn)?; - let mut view_only_account_map = HashMap::default(); - - let mut unspent: u128 = 0; - let mut pending: u128 = 0; - let mut spent: u128 = 0; - let mut secreted: u128 = 0; - let mut orphaned: u128 = 0; - - let mut min_synced_block_index = network_block_height - 1; - let mut account_ids = Vec::new(); - for account in accounts { - let account_id = AccountID(account.account_id_hex.clone()); - let balance = Self::get_balance_inner(&account_id.to_string(), &conn)?; - account_map.insert(account_id.clone(), account.clone()); - unspent += balance.0; - pending += balance.1; - spent += balance.2; - secreted += balance.3; - orphaned += balance.4; - - // account.next_block_index is an index in range [0..ledger_db.num_blocks()] - min_synced_block_index = std::cmp::min( - min_synced_block_index, - (account.next_block_index as u64).saturating_sub(1), - ); - account_ids.push(account_id); - } - - let mut view_only_account_ids = Vec::new(); - for account in view_only_accounts { - let account_id = account.account_id_hex.clone(); - view_only_account_map.insert(account_id.clone(), account.clone()); - view_only_account_ids.push(account_id); - } - - Ok(WalletStatus { - unspent, - pending, - spent, - secreted, - orphaned, - network_block_height, - local_block_height: self.ledger_db.num_blocks()?, - min_synced_block_index: min_synced_block_index as u64, - account_ids, - account_map, - view_only_account_ids, - view_only_account_map, - }) + let accounts = Account::list_all(&conn)?; + let mut account_map = HashMap::default(); + let view_only_accounts = ViewOnlyAccount::list_all(&conn)?; + let mut view_only_account_map = HashMap::default(); + + let mut unspent: u128 = 0; + let mut pending: u128 = 0; + let mut spent: u128 = 0; + let mut secreted: u128 = 0; + let mut orphaned: u128 = 0; + + let mut min_synced_block_index = network_block_height - 1; + let mut account_ids = Vec::new(); + for account in accounts { + let account_id = AccountID(account.account_id_hex.clone()); + let balance = Self::get_balance_inner(&account_id.to_string(), &conn)?; + account_map.insert(account_id.clone(), account.clone()); + unspent += balance.0; + pending += balance.1; + spent += balance.2; + secreted += balance.3; + orphaned += balance.4; + + // account.next_block_index is an index in range [0..ledger_db.num_blocks()] + min_synced_block_index = std::cmp::min( + min_synced_block_index, + (account.next_block_index as u64).saturating_sub(1), + ); + account_ids.push(account_id); + } + + let mut view_only_account_ids = Vec::new(); + for account in view_only_accounts { + let account_id = account.account_id_hex.clone(); + view_only_account_map.insert(account_id.clone(), account.clone()); + view_only_account_ids.push(account_id); + } + + Ok(WalletStatus { + unspent, + pending, + spent, + secreted, + orphaned, + network_block_height, + local_block_height: self.ledger_db.num_blocks()?, + min_synced_block_index: min_synced_block_index as u64, + account_ids, + account_map, + view_only_account_ids, + view_only_account_map, }) } } @@ -332,7 +316,7 @@ where { fn get_balance_inner( account_id_hex: &str, - conn: &PooledConnection>, + conn: &Conn, ) -> Result<(u128, u128, u128, u128, u128), BalanceServiceError> { // Note: We need to cast to u64 first, because i64 could have wrapped, then to // u128 diff --git a/full-service/src/service/confirmation_number.rs b/full-service/src/service/confirmation_number.rs index 0054408bc..b8d5452e9 100644 --- a/full-service/src/service/confirmation_number.rs +++ b/full-service/src/service/confirmation_number.rs @@ -22,8 +22,6 @@ use mc_fog_report_validation::FogPubkeyResolver; use mc_ledger_db::Ledger; use mc_transaction_core::tx::TxOutConfirmationNumber; -use diesel::Connection; - /// Errors for the Txo Service. #[derive(Display, Debug)] #[allow(clippy::large_enum_variant)] @@ -159,15 +157,13 @@ where confirmation_hex: &str, ) -> Result { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { - let confirmation: TxOutConfirmationNumber = - mc_util_serial::decode(&hex::decode(confirmation_hex)?)?; - Ok(Txo::validate_confirmation( - &AccountID(account_id.to_string()), - &txo_id.to_string(), - &confirmation, - &conn, - )?) - }) + let confirmation: TxOutConfirmationNumber = + mc_util_serial::decode(&hex::decode(confirmation_hex)?)?; + Ok(Txo::validate_confirmation( + &AccountID(account_id.to_string()), + &txo_id.to_string(), + &confirmation, + &conn, + )?) } } diff --git a/full-service/src/service/gift_code.rs b/full-service/src/service/gift_code.rs index 416ca0d3d..352f07101 100644 --- a/full-service/src/service/gift_code.rs +++ b/full-service/src/service/gift_code.rs @@ -12,7 +12,7 @@ use crate::{ account::{AccountID, AccountModel}, gift_code::GiftCodeModel, models::{Account, GiftCode}, - WalletDbError, + transaction, WalletDbError, }, service::{ account::AccountServiceError, @@ -27,7 +27,6 @@ use crate::{ }, }; use bip39::{Language, Mnemonic, MnemonicType}; -use diesel::Connection; use displaydoc::Display; use mc_account_keys::{AccountKey, DEFAULT_SUBADDRESS_INDEX}; use mc_account_keys_slip10::Slip10KeyGenerator; @@ -399,7 +398,7 @@ where b58_encode_public_address(&gift_code_account_key.default_subaddress())?; let conn = self.wallet_db.get_conn()?; - let from_account = conn.transaction(|| Account::get(from_account_id, &conn))?; + let from_account = Account::get(from_account_id, &conn)?; let tx_proposal = self.build_transaction( &from_account.account_id_hex, @@ -447,7 +446,7 @@ where // Save the gift code to the database before attempting to send it out. let conn = self.wallet_db.get_conn()?; - let gift_code = conn.transaction(|| GiftCode::create(gift_code_b58, value, &conn))?; + let gift_code = transaction(&conn, || GiftCode::create(gift_code_b58, value, &conn))?; self.submit_transaction( tx_proposal.clone(), @@ -695,7 +694,7 @@ where gift_code_b58: &EncodedGiftCode, ) -> Result { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| GiftCode::get(gift_code_b58, &conn)?.delete(&conn))?; + transaction(&conn, || GiftCode::get(gift_code_b58, &conn)?.delete(&conn))?; Ok(true) } } diff --git a/full-service/src/service/receipt.rs b/full-service/src/service/receipt.rs index 458f032ee..208bfa6bc 100644 --- a/full-service/src/service/receipt.rs +++ b/full-service/src/service/receipt.rs @@ -18,7 +18,6 @@ use crate::{ }, WalletService, }; -use diesel::Connection; use displaydoc::Display; use mc_account_keys::AccountKey; use mc_connection::{BlockchainConnection, UserTxConnection}; @@ -187,58 +186,53 @@ where receiver_receipt: &ReceiverReceipt, ) -> Result<(ReceiptTransactionStatus, Option), ReceiptServiceError> { let conn = &self.wallet_db.get_conn()?; - conn.transaction(|| { - let assigned_address = AssignedSubaddress::get(address, conn)?; - let account_id = AccountID(assigned_address.account_id_hex); - let account = Account::get(&account_id, conn)?; - // Get the transaction from the database, with status. - let txos = Txo::select_by_public_key(&[&receiver_receipt.public_key], conn)?; - - // Return if the Txo from the receipt is not in this wallet yet. - if txos.is_empty() { - return Ok((ReceiptTransactionStatus::TransactionPending, None)); + let assigned_address = AssignedSubaddress::get(address, conn)?; + let account_id = AccountID(assigned_address.account_id_hex); + let account = Account::get(&account_id, conn)?; + // Get the transaction from the database, with status. + let txos = Txo::select_by_public_key(&[&receiver_receipt.public_key], conn)?; + + // Return if the Txo from the receipt is not in this wallet yet. + if txos.is_empty() { + return Ok((ReceiptTransactionStatus::TransactionPending, None)); + } + let txo = txos[0].clone(); + + // Return if the Txo from the receipt has a pending tombstone block index + if txo.pending_tombstone_block_index.is_some() { + return Ok((ReceiptTransactionStatus::TransactionPending, Some(txo))); + } + + // Decrypt the amount to get the expected value + let account_key: AccountKey = mc_util_serial::decode(&account.account_key)?; + let public_key: RistrettoPublic = RistrettoPublic::try_from(&receiver_receipt.public_key)?; + let shared_secret = get_tx_out_shared_secret(account_key.view_private_key(), &public_key); + let expected_value = match receiver_receipt.amount.get_value(&shared_secret) { + Ok((v, _blinding)) => v, + Err(AmountError::InconsistentCommitment) => { + return Ok((ReceiptTransactionStatus::FailedAmountDecryption, Some(txo))) } - let txo = txos[0].clone(); - - // Return if the Txo from the receipt has a pending tombstone block index - if txo.pending_tombstone_block_index.is_some() { - return Ok((ReceiptTransactionStatus::TransactionPending, Some(txo))); - } - - // Decrypt the amount to get the expected value - let account_key: AccountKey = mc_util_serial::decode(&account.account_key)?; - let public_key: RistrettoPublic = - RistrettoPublic::try_from(&receiver_receipt.public_key)?; - let shared_secret = - get_tx_out_shared_secret(account_key.view_private_key(), &public_key); - let expected_value = match receiver_receipt.amount.get_value(&shared_secret) { - Ok((v, _blinding)) => v, - Err(AmountError::InconsistentCommitment) => { - return Ok((ReceiptTransactionStatus::FailedAmountDecryption, Some(txo))) - } - }; - // Check that the value of the received Txo matches the expected value. - if (txo.value as u64) != expected_value { - return Ok(( - ReceiptTransactionStatus::AmountMismatch(format!( - "Expected: {}, Got: {}", - expected_value, txo.value - )), - Some(txo), - )); - } - - // Validate the confirmation number. - let confirmation_hex = - hex::encode(mc_util_serial::encode(&receiver_receipt.confirmation)); - let confirmation: TxOutConfirmationNumber = - mc_util_serial::decode(&hex::decode(confirmation_hex)?)?; - if !Txo::validate_confirmation(&account_id, &txo.txo_id_hex, &confirmation, conn)? { - return Ok((ReceiptTransactionStatus::InvalidConfirmation, Some(txo))); - } - - Ok((ReceiptTransactionStatus::TransactionSuccess, Some(txo))) - }) + }; + // Check that the value of the received Txo matches the expected value. + if (txo.value as u64) != expected_value { + return Ok(( + ReceiptTransactionStatus::AmountMismatch(format!( + "Expected: {}, Got: {}", + expected_value, txo.value + )), + Some(txo), + )); + } + + // Validate the confirmation number. + let confirmation_hex = hex::encode(mc_util_serial::encode(&receiver_receipt.confirmation)); + let confirmation: TxOutConfirmationNumber = + mc_util_serial::decode(&hex::decode(confirmation_hex)?)?; + if !Txo::validate_confirmation(&account_id, &txo.txo_id_hex, &confirmation, conn)? { + return Ok((ReceiptTransactionStatus::InvalidConfirmation, Some(txo))); + } + + Ok((ReceiptTransactionStatus::TransactionSuccess, Some(txo))) } fn create_receiver_receipts( diff --git a/full-service/src/service/sync.rs b/full-service/src/service/sync.rs index d925ba998..5a4238c33 100644 --- a/full-service/src/service/sync.rs +++ b/full-service/src/service/sync.rs @@ -10,12 +10,13 @@ use crate::{ Account, AssignedSubaddress, TransactionLog, Txo, ViewOnlyAccount, ViewOnlyTransactionLog, ViewOnlyTxo, }, + transaction, transaction_log::TransactionLogModel, txo::TxoModel, view_only_account::ViewOnlyAccountModel, view_only_transaction_log::ViewOnlyTransactionLogModel, view_only_txo::ViewOnlyTxoModel, - WalletDb, + Conn, WalletDb, }, error::SyncError, util::b58::b58_encode_public_address, @@ -36,10 +37,6 @@ use mc_transaction_core::{ }; use rayon::prelude::*; -use diesel::{ - prelude::*, - r2d2::{ConnectionManager, PooledConnection}, -}; use std::{ convert::TryFrom, sync::{ @@ -50,7 +47,7 @@ use std::{ time::Instant, }; -const BLOCKS_CHUNK_SIZE: u64 = 10_000; +const BLOCKS_CHUNK_SIZE: u64 = 1_000; /// Sync thread - holds objects needed to cleanly terminate the sync thread. pub struct SyncThread { @@ -179,11 +176,11 @@ pub fn sync_view_only_account( fn sync_view_only_account_next_chunk( ledger_db: &LedgerDB, - conn: &PooledConnection>, + conn: &Conn, logger: &Logger, account_id_hex: &str, ) -> Result { - conn.transaction::(|| { + transaction(conn, || { // Get the account data. If it is no longer available, the account has been // removed and we can simply return. let view_only_account = ViewOnlyAccount::get(account_id_hex, conn)?; @@ -290,11 +287,11 @@ pub fn sync_account( fn sync_account_next_chunk( ledger_db: &LedgerDB, - conn: &PooledConnection>, + conn: &Conn, logger: &Logger, account_id_hex: &str, ) -> Result { - conn.transaction::(|| { + transaction(conn, || { // Get the account data. If it is no longer available, the account has been // removed and we can simply return. let account = Account::get(&AccountID(account_id_hex.to_string()), conn)?; diff --git a/full-service/src/service/transaction.rs b/full-service/src/service/transaction.rs index 64f248754..c971d263a 100644 --- a/full-service/src/service/transaction.rs +++ b/full-service/src/service/transaction.rs @@ -5,6 +5,7 @@ use crate::{ db::{ models::TransactionLog, + transaction, transaction_log::{AssociatedTxos, TransactionLogModel}, WalletDbError, }, @@ -25,8 +26,6 @@ use crate::service::address::{AddressService, AddressServiceError}; use displaydoc::Display; use std::{convert::TryFrom, iter::empty, sync::atomic::Ordering}; -use diesel::Connection; - /// Errors for the Transaction Service. #[derive(Display, Debug)] #[allow(clippy::large_enum_variant)] @@ -186,61 +185,62 @@ where max_spendable_value: Option, log_tx_proposal: Option, ) -> Result { - let mut builder = WalletTransactionBuilder::new( - account_id_hex.to_string(), - self.wallet_db.clone(), - self.ledger_db.clone(), - self.fog_resolver_factory.clone(), - self.logger.clone(), - ); + let conn = self.wallet_db.get_conn()?; + transaction(&conn, || { + let mut builder = WalletTransactionBuilder::new( + account_id_hex.to_string(), + self.ledger_db.clone(), + self.fog_resolver_factory.clone(), + self.logger.clone(), + ); + + for (recipient_public_address, value) in addresses_and_values { + if !self.verify_address(recipient_public_address)? { + return Err(TransactionServiceError::InvalidPublicAddress( + recipient_public_address.to_string(), + )); + }; + let recipient = b58_decode_public_address(recipient_public_address)?; + builder.add_recipient(recipient, value.parse::()?)?; + } - for (recipient_public_address, value) in addresses_and_values { - if !self.verify_address(recipient_public_address)? { - return Err(TransactionServiceError::InvalidPublicAddress( - recipient_public_address.to_string(), - )); - }; - let recipient = b58_decode_public_address(recipient_public_address)?; - builder.add_recipient(recipient, value.parse::()?)?; - } + if let Some(tombstone) = tombstone_block { + builder.set_tombstone(tombstone.parse::()?)?; + } else { + builder.set_tombstone(0)?; + } - if let Some(tombstone) = tombstone_block { - builder.set_tombstone(tombstone.parse::()?)?; - } else { - builder.set_tombstone(0)?; - } + builder.set_fee(match fee { + Some(f) => f.parse()?, + None => self.get_network_fee(), + })?; - if let Some(inputs) = input_txo_ids { - builder.set_txos(inputs, log_tx_proposal.unwrap_or_default())?; - } else { - let max_spendable = if let Some(msv) = max_spendable_value { - Some(msv.parse::()?) + if let Some(inputs) = input_txo_ids { + builder.set_txos(&conn, inputs, log_tx_proposal.unwrap_or_default())?; } else { - None - }; - builder.select_txos(max_spendable, log_tx_proposal.unwrap_or_default())?; - } - - builder.set_fee(match fee { - Some(f) => f.parse()?, - None => self.get_network_fee(), - })?; + let max_spendable = if let Some(msv) = max_spendable_value { + Some(msv.parse::()?) + } else { + None + }; + builder.select_txos(&conn, max_spendable, log_tx_proposal.unwrap_or_default())?; + } - let tx_proposal = builder.build()?; + let tx_proposal = builder.build(&conn)?; - if log_tx_proposal.unwrap_or_default() { - let conn = self.wallet_db.get_conn()?; - let block_index = self.ledger_db.num_blocks()? - 1; - let _transaction_log = TransactionLog::log_submitted( - tx_proposal.clone(), - block_index, - "".to_string(), - account_id_hex, - &conn, - )?; - } + if log_tx_proposal.unwrap_or_default() { + let block_index = self.ledger_db.num_blocks()? - 1; + let _transaction_log = TransactionLog::log_submitted( + tx_proposal.clone(), + block_index, + "".to_string(), + account_id_hex, + &conn, + )?; + } - Ok(tx_proposal) + Ok(tx_proposal) + }) } fn submit_transaction( @@ -280,7 +280,7 @@ where // Log the transaction. let result = if let Some(a) = account_id_hex { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { + transaction(&conn, || { let transaction_log = TransactionLog::log_submitted( tx_proposal, block_index, diff --git a/full-service/src/service/transaction_builder.rs b/full-service/src/service/transaction_builder.rs index dd0fa1c61..ab424b41f 100644 --- a/full-service/src/service/transaction_builder.rs +++ b/full-service/src/service/transaction_builder.rs @@ -13,7 +13,7 @@ use crate::{ account::{AccountID, AccountModel}, models::{Account, Txo}, txo::TxoModel, - WalletDb, + Conn, }, error::WalletTransactionBuilderError, }; @@ -40,7 +40,6 @@ use mc_transaction_core::{ use mc_transaction_std::{InputCredentials, NoMemoBuilder, TransactionBuilder}; use mc_util_uri::FogUri; -use diesel::prelude::*; use rand::Rng; use std::{convert::TryFrom, str::FromStr, sync::Arc}; @@ -54,9 +53,6 @@ pub struct WalletTransactionBuilder { /// Account ID (hex-encoded) from which to construct a transaction. account_id_hex: String, - /// The wallet DB. - wallet_db: WalletDb, - /// The ledger DB. ledger_db: LedgerDB, @@ -85,14 +81,12 @@ pub struct WalletTransactionBuilder { impl WalletTransactionBuilder { pub fn new( account_id_hex: String, - wallet_db: WalletDb, ledger_db: LedgerDB, fog_resolver_factory: Arc Result + Send + Sync + 'static>, logger: Logger, ) -> Self { WalletTransactionBuilder { account_id_hex, - wallet_db, ledger_db, inputs: vec![], outlays: vec![], @@ -107,6 +101,7 @@ impl WalletTransactionBuilder { /// txos are included. pub fn set_txos( &mut self, + conn: &Conn, input_txo_ids: &[String], update_to_pending: bool, ) -> Result<(), WalletTransactionBuilderError> { @@ -116,11 +111,7 @@ impl WalletTransactionBuilder { None }; - let txos = Txo::select_by_id( - &input_txo_ids.to_vec(), - pending_tombstone_block_index, - &self.wallet_db.get_conn()?, - )?; + let txos = Txo::select_by_id(&input_txo_ids.to_vec(), pending_tombstone_block_index, conn)?; let unspent: Vec = txos .iter() @@ -142,6 +133,7 @@ impl WalletTransactionBuilder { /// Selects Txos from the account. pub fn select_txos( &mut self, + conn: &Conn, max_spendable_value: Option, update_to_pending: bool, ) -> Result<(), WalletTransactionBuilderError> { @@ -171,7 +163,7 @@ impl WalletTransactionBuilder { total_value, max_spendable_value, pending_tombstone_block_index, - &self.wallet_db.get_conn()?, + conn, )?; Ok(()) @@ -214,7 +206,7 @@ impl WalletTransactionBuilder { } /// Consumes self - pub fn build(&self) -> Result { + pub fn build(&self, conn: &Conn) -> Result { if self.inputs.is_empty() { return Err(WalletTransactionBuilderError::NoInputs); } @@ -223,265 +215,261 @@ impl WalletTransactionBuilder { return Err(WalletTransactionBuilderError::TombstoneNotSet); } - let conn = self.wallet_db.get_conn()?; - - conn.transaction::(|| { - let account: Account = - Account::get(&AccountID(self.account_id_hex.to_string()), &conn)?; - let from_account_key: AccountKey = mc_util_serial::decode(&account.account_key)?; - - // Collect all required FogUris from public addresses, then pass to resolver - // factory - let fog_resolver = { - let change_address = - from_account_key.subaddress(account.change_subaddress_index as u64); - let fog_uris = core::slice::from_ref(&change_address) - .iter() - .chain(self.outlays.iter().map(|(receiver, _amount)| receiver)) - .filter_map(|x| extract_fog_uri(x).transpose()) - .collect::, _>>()?; - (self.fog_resolver_factory)(&fog_uris) - .map_err(WalletTransactionBuilderError::FogPubkeyResolver)? - }; - - // Create transaction builder. - // TODO: After servers that support memos are deployed, use RTHMemoBuilder here - let memo_builder = NoMemoBuilder::default(); - let mut transaction_builder = TransactionBuilder::new(fog_resolver, memo_builder); - transaction_builder.set_fee(self.fee.unwrap_or(Mob::MINIMUM_FEE))?; + let account: Account = Account::get(&AccountID(self.account_id_hex.to_string()), conn)?; + let from_account_key: AccountKey = mc_util_serial::decode(&account.account_key)?; - // Get membership proofs for our inputs - let indexes = self - .inputs + // Collect all required FogUris from public addresses, then pass to resolver + // factory + let fog_resolver = { + let change_address = + from_account_key.subaddress(account.change_subaddress_index as u64); + let fog_uris = core::slice::from_ref(&change_address) .iter() - .map(|utxo| { - let txo: TxOut = mc_util_serial::decode(&utxo.txo)?; - self.ledger_db.get_tx_out_index_by_hash(&txo.hash()) - }) - .collect::, mc_ledger_db::Error>>()?; - let proofs = self.ledger_db.get_tx_out_proof_of_memberships(&indexes)?; + .chain(self.outlays.iter().map(|(receiver, _amount)| receiver)) + .filter_map(|x| extract_fog_uri(x).transpose()) + .collect::, _>>()?; + (self.fog_resolver_factory)(&fog_uris) + .map_err(WalletTransactionBuilderError::FogPubkeyResolver)? + }; - let inputs_and_proofs: Vec<(Txo, TxOutMembershipProof)> = self - .inputs - .clone() - .into_iter() - .zip(proofs.into_iter()) - .collect(); + // Create transaction builder. + // TODO: After servers that support memos are deployed, use RTHMemoBuilder here + let memo_builder = NoMemoBuilder::default(); + let mut transaction_builder = TransactionBuilder::new(fog_resolver, memo_builder); + transaction_builder.set_fee(self.fee.unwrap_or(Mob::MINIMUM_FEE))?; - let excluded_tx_out_indices: Vec = inputs_and_proofs - .iter() - .map(|(utxo, _membership_proof)| { - let txo: TxOut = mc_util_serial::decode(&utxo.txo)?; - self.ledger_db - .get_tx_out_index_by_hash(&txo.hash()) - .map_err(WalletTransactionBuilderError::LedgerDB) - }) - .collect::, WalletTransactionBuilderError>>()?; + // Get membership proofs for our inputs + let indexes = self + .inputs + .iter() + .map(|utxo| { + let txo: TxOut = mc_util_serial::decode(&utxo.txo)?; + self.ledger_db.get_tx_out_index_by_hash(&txo.hash()) + }) + .collect::, mc_ledger_db::Error>>()?; + let proofs = self.ledger_db.get_tx_out_proof_of_memberships(&indexes)?; + + let inputs_and_proofs: Vec<(Txo, TxOutMembershipProof)> = self + .inputs + .clone() + .into_iter() + .zip(proofs.into_iter()) + .collect(); - let rings = self.get_rings(inputs_and_proofs.len(), &excluded_tx_out_indices)?; + let excluded_tx_out_indices: Vec = inputs_and_proofs + .iter() + .map(|(utxo, _membership_proof)| { + let txo: TxOut = mc_util_serial::decode(&utxo.txo)?; + self.ledger_db + .get_tx_out_index_by_hash(&txo.hash()) + .map_err(WalletTransactionBuilderError::LedgerDB) + }) + .collect::, WalletTransactionBuilderError>>()?; - if rings.len() != inputs_and_proofs.len() { - return Err(WalletTransactionBuilderError::RingSizeMismatch); - } + let rings = self.get_rings(inputs_and_proofs.len(), &excluded_tx_out_indices)?; + + if rings.len() != inputs_and_proofs.len() { + return Err(WalletTransactionBuilderError::RingSizeMismatch); + } + + if self.outlays.is_empty() { + return Err(WalletTransactionBuilderError::NoRecipient); + } + + // Unzip each vec of tuples into a tuple of vecs. + let mut rings_and_proofs: Vec<(Vec, Vec)> = rings + .into_iter() + .map(|tuples| tuples.into_iter().unzip()) + .collect(); - if self.outlays.is_empty() { - return Err(WalletTransactionBuilderError::NoRecipient); + // Add inputs to the tx. + for (utxo, proof) in inputs_and_proofs.iter() { + let db_tx_out: TxOut = mc_util_serial::decode(&utxo.txo)?; + let (mut ring, mut membership_proofs) = rings_and_proofs + .pop() + .ok_or(WalletTransactionBuilderError::RingsAndProofsEmpty)?; + if ring.len() != membership_proofs.len() { + return Err(WalletTransactionBuilderError::RingSizeMismatch); } - // Unzip each vec of tuples into a tuple of vecs. - let mut rings_and_proofs: Vec<(Vec, Vec)> = rings - .into_iter() - .map(|tuples| tuples.into_iter().unzip()) - .collect(); - - // Add inputs to the tx. - for (utxo, proof) in inputs_and_proofs.iter() { - let db_tx_out: TxOut = mc_util_serial::decode(&utxo.txo)?; - let (mut ring, mut membership_proofs) = rings_and_proofs - .pop() - .ok_or(WalletTransactionBuilderError::RingsAndProofsEmpty)?; - if ring.len() != membership_proofs.len() { - return Err(WalletTransactionBuilderError::RingSizeMismatch); + // Add the input to the ring. + let position_opt = ring.iter().position(|txo| *txo == db_tx_out); + let real_key_index = match position_opt { + Some(position) => { + // The input is already present in the ring. + // This could happen if ring elements are sampled randomly from the + // ledger. + position } - - // Add the input to the ring. - let position_opt = ring.iter().position(|txo| *txo == db_tx_out); - let real_key_index = match position_opt { - Some(position) => { - // The input is already present in the ring. - // This could happen if ring elements are sampled randomly from the - // ledger. - position + None => { + // The input is not already in the ring. + if ring.is_empty() { + // Append the input and its proof of membership. + ring.push(db_tx_out.clone()); + membership_proofs.push(proof.clone()); + } else { + // Replace the first element of the ring. + ring[0] = db_tx_out.clone(); + membership_proofs[0] = proof.clone(); } - None => { - // The input is not already in the ring. - if ring.is_empty() { - // Append the input and its proof of membership. - ring.push(db_tx_out.clone()); - membership_proofs.push(proof.clone()); - } else { - // Replace the first element of the ring. - ring[0] = db_tx_out.clone(); - membership_proofs[0] = proof.clone(); - } - // The real input is always the first element. This is safe because - // TransactionBuilder sorts each ring. - 0 - } - }; - - if ring.len() != membership_proofs.len() { - return Err(WalletTransactionBuilderError::RingSizeMismatch); + // The real input is always the first element. This is safe because + // TransactionBuilder sorts each ring. + 0 } + }; - let public_key = RistrettoPublic::try_from(&db_tx_out.public_key).unwrap(); - - let subaddress_index = if let Some(s) = utxo.subaddress_index { - s - } else { - return Err(WalletTransactionBuilderError::NullSubaddress( - utxo.txo_id_hex.to_string(), - )); - }; - - let onetime_private_key = recover_onetime_private_key( - &public_key, - from_account_key.view_private_key(), - &from_account_key.subaddress_spend_private(subaddress_index as u64), - ); - - let key_image = KeyImage::from(&onetime_private_key); - log::debug!( - self.logger, - "Adding input: ring {:?}, utxo index {:?}, key image {:?}, pubkey {:?}", - ring, - real_key_index, - key_image, - public_key - ); - - transaction_builder.add_input(InputCredentials::new( - ring, - membership_proofs, - real_key_index, - onetime_private_key, - *from_account_key.view_private_key(), - )?); + if ring.len() != membership_proofs.len() { + return Err(WalletTransactionBuilderError::RingSizeMismatch); } - // Add outputs to our destinations. - // Note that we make an assumption currently when logging submitted Txos that - // they were built with only one recipient, and one change txo. - let mut total_value = 0; - let mut tx_out_to_outlay_index: HashMap = HashMap::default(); - let mut outlay_confirmation_numbers = Vec::default(); - let mut rng = rand::thread_rng(); - for (i, (recipient, out_value)) in self.outlays.iter().enumerate() { - let (tx_out, confirmation_number) = - transaction_builder.add_output(*out_value as u64, recipient, &mut rng)?; - - tx_out_to_outlay_index.insert(tx_out, i); - outlay_confirmation_numbers.push(confirmation_number); - - total_value += *out_value; - } + let public_key = RistrettoPublic::try_from(&db_tx_out.public_key).unwrap(); - // Figure out if we have change. - let input_value = inputs_and_proofs - .iter() - .fold(0, |acc, (utxo, _proof)| acc + utxo.value); - if (total_value + transaction_builder.get_fee()) > input_value as u64 { - return Err(WalletTransactionBuilderError::InsufficientInputFunds( - format!( - "Total value required to send transaction {:?}, but only {:?} in inputs", - total_value + transaction_builder.get_fee(), - input_value - ), + let subaddress_index = if let Some(s) = utxo.subaddress_index { + s + } else { + return Err(WalletTransactionBuilderError::NullSubaddress( + utxo.txo_id_hex.to_string(), )); - } + }; - let change = input_value as u64 - total_value - transaction_builder.get_fee(); + let onetime_private_key = recover_onetime_private_key( + &public_key, + from_account_key.view_private_key(), + &from_account_key.subaddress_spend_private(subaddress_index as u64), + ); + + let key_image = KeyImage::from(&onetime_private_key); + log::debug!( + self.logger, + "Adding input: ring {:?}, utxo index {:?}, key image {:?}, pubkey {:?}", + ring, + real_key_index, + key_image, + public_key + ); + + transaction_builder.add_input(InputCredentials::new( + ring, + membership_proofs, + real_key_index, + onetime_private_key, + *from_account_key.view_private_key(), + )?); + } - // If we do, add an output for that as well. - if change > 0 { - let change_public_address = - from_account_key.subaddress(account.change_subaddress_index as u64); - // FIXME: verify that fog resolver knows to send change with hint encrypted to - // the main public address - transaction_builder.add_output(change, &change_public_address, &mut rng)?; - // FIXME: CBB - map error to indicate error with change - } + // Add outputs to our destinations. + // Note that we make an assumption currently when logging submitted Txos that + // they were built with only one recipient, and one change txo. + let mut total_value = 0; + let mut tx_out_to_outlay_index: HashMap = HashMap::default(); + let mut outlay_confirmation_numbers = Vec::default(); + let mut rng = rand::thread_rng(); + for (i, (recipient, out_value)) in self.outlays.iter().enumerate() { + let (tx_out, confirmation_number) = + transaction_builder.add_output(*out_value as u64, recipient, &mut rng)?; - // Set tombstone block. - transaction_builder.set_tombstone_block(self.tombstone); + tx_out_to_outlay_index.insert(tx_out, i); + outlay_confirmation_numbers.push(confirmation_number); - // Build tx. - let tx = transaction_builder.build(&mut rng)?; + total_value += *out_value; + } - // Map each TxOut in the constructed transaction to its respective outlay. - let outlay_index_to_tx_out_index: HashMap = tx - .prefix - .outputs - .iter() - .enumerate() - .filter_map(|(tx_out_index, tx_out)| { - tx_out_to_outlay_index - .get(tx_out) - .map(|outlay_index| (*outlay_index, tx_out_index)) - }) - .collect(); - - // Sanity check: All of our outlays should have a unique index in the map. - assert_eq!(outlay_index_to_tx_out_index.len(), self.outlays.len()); - let mut found_tx_out_indices: HashSet<&usize> = HashSet::default(); - for i in 0..self.outlays.len() { - let tx_out_index = outlay_index_to_tx_out_index - .get(&i) - .expect("index not in map"); - if !found_tx_out_indices.insert(tx_out_index) { - panic!("duplicate index {} found in map", tx_out_index); - } + // Figure out if we have change. + let input_value = inputs_and_proofs + .iter() + .fold(0, |acc, (utxo, _proof)| acc + utxo.value); + if (total_value + transaction_builder.get_fee()) > input_value as u64 { + return Err(WalletTransactionBuilderError::InsufficientInputFunds( + format!( + "Total value required to send transaction {:?}, but only {:?} in inputs", + total_value + transaction_builder.get_fee(), + input_value + ), + )); + } + + let change = input_value as u64 - total_value - transaction_builder.get_fee(); + + // If we do, add an output for that as well. + if change > 0 { + let change_public_address = + from_account_key.subaddress(account.change_subaddress_index as u64); + // FIXME: verify that fog resolver knows to send change with hint encrypted to + // the main public address + transaction_builder.add_output(change, &change_public_address, &mut rng)?; + // FIXME: CBB - map error to indicate error with change + } + + // Set tombstone block. + transaction_builder.set_tombstone_block(self.tombstone); + + // Build tx. + let tx = transaction_builder.build(&mut rng)?; + + // Map each TxOut in the constructed transaction to its respective outlay. + let outlay_index_to_tx_out_index: HashMap = tx + .prefix + .outputs + .iter() + .enumerate() + .filter_map(|(tx_out_index, tx_out)| { + tx_out_to_outlay_index + .get(tx_out) + .map(|outlay_index| (*outlay_index, tx_out_index)) + }) + .collect(); + + // Sanity check: All of our outlays should have a unique index in the map. + assert_eq!(outlay_index_to_tx_out_index.len(), self.outlays.len()); + let mut found_tx_out_indices: HashSet<&usize> = HashSet::default(); + for i in 0..self.outlays.len() { + let tx_out_index = outlay_index_to_tx_out_index + .get(&i) + .expect("index not in map"); + if !found_tx_out_indices.insert(tx_out_index) { + panic!("duplicate index {} found in map", tx_out_index); } + } - // Make the UnspentTxOut for each Txo - // FIXME: WS-27 - I would prefer to provide just the txo_id_hex per txout, but - // this at least preserves some interoperability between - // mobilecoind and wallet-service. However, this is - // pretty clunky and I would rather not expose a storage - // type from mobilecoind just to get around having to write a bunch of - // tedious json conversions. - // Return the TxProposal - let selected_utxos = inputs_and_proofs - .iter() - .map(|(utxo, _membership_proof)| { - let decoded_tx_out = mc_util_serial::decode(&utxo.txo).unwrap(); - let decoded_key_image = - mc_util_serial::decode(&utxo.key_image.clone().unwrap()).unwrap(); - - UnspentTxOut { - tx_out: decoded_tx_out, - subaddress_index: utxo.subaddress_index.unwrap() as u64, // verified not null earlier - key_image: decoded_key_image, - value: utxo.value as u64, - attempted_spend_height: 0, // NOTE: these are null because not tracked here - attempted_spend_tombstone: 0, - } - }) - .collect(); - Ok(TxProposal { - utxos: selected_utxos, - outlays: self - .outlays - .iter() - .map(|(recipient, value)| Outlay { - receiver: recipient.clone(), - value: *value, - }) - .collect::>(), - tx, - outlay_index_to_tx_out_index, - outlay_confirmation_numbers, + // Make the UnspentTxOut for each Txo + // FIXME: WS-27 - I would prefer to provide just the txo_id_hex per txout, but + // this at least preserves some interoperability between + // mobilecoind and wallet-service. However, this is + // pretty clunky and I would rather not expose a storage + // type from mobilecoind just to get around having to write a bunch of + // tedious json conversions. + // Return the TxProposal + let selected_utxos = inputs_and_proofs + .iter() + .map(|(utxo, _membership_proof)| { + let decoded_tx_out = mc_util_serial::decode(&utxo.txo).unwrap(); + let decoded_key_image = + mc_util_serial::decode(&utxo.key_image.clone().unwrap()).unwrap(); + + UnspentTxOut { + tx_out: decoded_tx_out, + subaddress_index: utxo.subaddress_index.unwrap() as u64, /* verified not null + * earlier */ + key_image: decoded_key_image, + value: utxo.value as u64, + attempted_spend_height: 0, // NOTE: these are null because not tracked here + attempted_spend_tombstone: 0, + } }) + .collect(); + Ok(TxProposal { + utxos: selected_utxos, + outlays: self + .outlays + .iter() + .map(|(recipient, value)| Outlay { + receiver: recipient.clone(), + value: *value, + }) + .collect::>(), + tx, + outlay_index_to_tx_out_index, + outlay_confirmation_numbers, }) } @@ -590,8 +578,9 @@ mod tests { ); // Construct a transaction + let conn = wallet_db.get_conn().unwrap(); let (recipient, mut builder) = - builder_for_random_recipient(&account_key, &wallet_db, &ledger_db, &mut rng, &logger); + builder_for_random_recipient(&account_key, &ledger_db, &mut rng, &logger); // Send value specifically for your smallest Txo size. Should take 2 inputs // and also make change. @@ -599,10 +588,10 @@ mod tests { builder.add_recipient(recipient.clone(), value).unwrap(); // Select the txos for the recipient - builder.select_txos(None, false).unwrap(); + builder.select_txos(&conn, None, false).unwrap(); builder.set_tombstone(0).unwrap(); - let proposal = builder.build().unwrap(); + let proposal = builder.build(&conn).unwrap(); assert_eq!(proposal.outlays.len(), 1); assert_eq!(proposal.outlays[0].receiver, recipient); assert_eq!(proposal.outlays[0].value, value); @@ -644,14 +633,15 @@ mod tests { assert_eq!(balance, 21_000_000 * MOB as u128); // Now try to send a transaction with a value > u64::MAX + let conn = wallet_db.get_conn().unwrap(); let (recipient, mut builder) = - builder_for_random_recipient(&account_key, &wallet_db, &ledger_db, &mut rng, &logger); + builder_for_random_recipient(&account_key, &ledger_db, &mut rng, &logger); let value = u64::MAX; builder.add_recipient(recipient.clone(), value).unwrap(); // Select the txos for the recipient - should error because > u64::MAX - match builder.select_txos(None, false) { + match builder.select_txos(&conn, None, false) { Ok(_) => panic!("Should not be allowed to construct outbound values > u64::MAX"), Err(WalletTransactionBuilderError::OutboundValueTooLarge) => {} Err(e) => panic!("Unexpected error {:?}", e), @@ -688,8 +678,9 @@ mod tests { ) .unwrap(); + let conn = wallet_db.get_conn().unwrap(); let (recipient, mut builder) = - builder_for_random_recipient(&account_key, &wallet_db, &ledger_db, &mut rng, &logger); + builder_for_random_recipient(&account_key, &ledger_db, &mut rng, &logger); // Setting value to exactly the input will fail because you need funds for fee builder @@ -697,10 +688,10 @@ mod tests { .unwrap(); builder - .set_txos(&vec![txos[0].txo_id_hex.clone()], false) + .set_txos(&conn, &vec![txos[0].txo_id_hex.clone()], false) .unwrap(); builder.set_tombstone(0).unwrap(); - match builder.build() { + match builder.build(&conn) { Ok(_) => { panic!("Should not be able to construct Tx with > inputs value as output value") } @@ -710,7 +701,7 @@ mod tests { // Now build, setting to multiple TXOs let (recipient, mut builder) = - builder_for_random_recipient(&account_key, &wallet_db, &ledger_db, &mut rng, &logger); + builder_for_random_recipient(&account_key, &ledger_db, &mut rng, &logger); // Set value to just slightly more than what fits in the one TXO builder @@ -719,12 +710,13 @@ mod tests { builder .set_txos( + &conn, &vec![txos[0].txo_id_hex.clone(), txos[1].txo_id_hex.clone()], false, ) .unwrap(); builder.set_tombstone(0).unwrap(); - let proposal = builder.build().unwrap(); + let proposal = builder.build(&conn).unwrap(); assert_eq!(proposal.outlays.len(), 1); assert_eq!(proposal.outlays[0].receiver, recipient); assert_eq!(proposal.outlays[0].value, txos[0].value as u64 + 10); @@ -754,14 +746,15 @@ mod tests { &logger, ); + let conn = wallet_db.get_conn().unwrap(); let (recipient, mut builder) = - builder_for_random_recipient(&account_key, &wallet_db, &ledger_db, &mut rng, &logger); + builder_for_random_recipient(&account_key, &ledger_db, &mut rng, &logger); // Setting value to exactly the input will fail because you need funds for fee builder.add_recipient(recipient.clone(), 80 * MOB).unwrap(); // Test that selecting Txos with max_spendable < all our txo values fails - match builder.select_txos(Some(10), false) { + match builder.select_txos(&conn, Some(10), false) { Ok(_) => panic!("Should not be able to construct tx when max_spendable < all txos"), Err(WalletTransactionBuilderError::WalletDb(WalletDbError::NoSpendableTxos)) => {} Err(e) => panic!("Unexpected error {:?}", e), @@ -769,7 +762,7 @@ mod tests { // We should be able to try again, with max_spendable at 70, but will not hit // our outlay target (80 * MOB) - match builder.select_txos(Some(70 * MOB), false) { + match builder.select_txos(&conn, Some(70 * MOB), false) { Ok(_) => panic!("Should not be able to construct tx when max_spendable < all txos"), Err(WalletTransactionBuilderError::WalletDb( WalletDbError::InsufficientFundsUnderMaxSpendable(_), @@ -779,9 +772,9 @@ mod tests { // Now, we should succeed if we set max_spendable = 80 * MOB, because we will // pick up both 70 and 80 - builder.select_txos(Some(80 * MOB), false).unwrap(); + builder.select_txos(&conn, Some(80 * MOB), false).unwrap(); builder.set_tombstone(0).unwrap(); - let proposal = builder.build().unwrap(); + let proposal = builder.build(&conn).unwrap(); assert_eq!(proposal.outlays.len(), 1); assert_eq!(proposal.outlays[0].receiver, recipient); assert_eq!(proposal.outlays[0].value, 80 * MOB); @@ -799,6 +792,7 @@ mod tests { let wallet_db = db_test_context.get_db_instance(logger.clone()); let known_recipients: Vec = Vec::new(); let mut ledger_db = get_test_ledger(5, &known_recipients, 12, &mut rng); + let conn = wallet_db.get_conn().unwrap(); // Start sync thread let _sync_thread = SyncThread::start(ledger_db.clone(), wallet_db.clone(), logger.clone()); @@ -812,48 +806,48 @@ mod tests { ); let (recipient, mut builder) = - builder_for_random_recipient(&account_key, &wallet_db, &ledger_db, &mut rng, &logger); + builder_for_random_recipient(&account_key, &ledger_db, &mut rng, &logger); builder.add_recipient(recipient.clone(), 10 * MOB).unwrap(); - builder.select_txos(None, false).unwrap(); + builder.select_txos(&conn, None, false).unwrap(); // Sanity check that our ledger is the height we think it is assert_eq!(ledger_db.num_blocks().unwrap(), 13); // We must set tombstone block before building - match builder.build() { + match builder.build(&conn) { Ok(_) => panic!("Expected TombstoneNotSet error"), Err(WalletTransactionBuilderError::TombstoneNotSet) => {} Err(e) => panic!("Unexpected error {:?}", e), } let (recipient, mut builder) = - builder_for_random_recipient(&account_key, &wallet_db, &ledger_db, &mut rng, &logger); + builder_for_random_recipient(&account_key, &ledger_db, &mut rng, &logger); builder.add_recipient(recipient.clone(), 10 * MOB).unwrap(); - builder.select_txos(None, false).unwrap(); + builder.select_txos(&conn, None, false).unwrap(); // Set to default builder.set_tombstone(0).unwrap(); // Not setting the tombstone results in tombstone = 0. This is an acceptable // value, - let proposal = builder.build().unwrap(); + let proposal = builder.build(&conn).unwrap(); assert_eq!(proposal.tx.prefix.tombstone_block, 23); // Build a transaction and explicitly set tombstone let (recipient, mut builder) = - builder_for_random_recipient(&account_key, &wallet_db, &ledger_db, &mut rng, &logger); + builder_for_random_recipient(&account_key, &ledger_db, &mut rng, &logger); builder.add_recipient(recipient.clone(), 10 * MOB).unwrap(); - builder.select_txos(None, false).unwrap(); + builder.select_txos(&conn, None, false).unwrap(); // Set to default builder.set_tombstone(20).unwrap(); // Not setting the tombstone results in tombstone = 0. This is an acceptable // value, - let proposal = builder.build().unwrap(); + let proposal = builder.build(&conn).unwrap(); assert_eq!(proposal.tx.prefix.tombstone_block, 20); } @@ -878,23 +872,24 @@ mod tests { &logger, ); + let conn = wallet_db.get_conn().unwrap(); let (recipient, mut builder) = - builder_for_random_recipient(&account_key, &wallet_db, &ledger_db, &mut rng, &logger); + builder_for_random_recipient(&account_key, &ledger_db, &mut rng, &logger); builder.add_recipient(recipient.clone(), 10 * MOB).unwrap(); - builder.select_txos(None, false).unwrap(); + builder.select_txos(&conn, None, false).unwrap(); builder.set_tombstone(0).unwrap(); // Verify that not setting fee results in default fee - let proposal = builder.build().unwrap(); + let proposal = builder.build(&conn).unwrap(); assert_eq!(proposal.tx.prefix.fee, Mob::MINIMUM_FEE); // You cannot set fee to 0 let (recipient, mut builder) = - builder_for_random_recipient(&account_key, &wallet_db, &ledger_db, &mut rng, &logger); + builder_for_random_recipient(&account_key, &ledger_db, &mut rng, &logger); builder.add_recipient(recipient.clone(), 10 * MOB).unwrap(); - builder.select_txos(None, false).unwrap(); + builder.select_txos(&conn, None, false).unwrap(); builder.set_tombstone(0).unwrap(); match builder.set_fee(0) { Ok(_) => panic!("Should not be able to set fee to 0"), @@ -903,15 +898,15 @@ mod tests { } // Verify that not setting fee results in default fee - let proposal = builder.build().unwrap(); + let proposal = builder.build(&conn).unwrap(); assert_eq!(proposal.tx.prefix.fee, Mob::MINIMUM_FEE); // Setting fee less than minimum fee should fail let (recipient, mut builder) = - builder_for_random_recipient(&account_key, &wallet_db, &ledger_db, &mut rng, &logger); + builder_for_random_recipient(&account_key, &ledger_db, &mut rng, &logger); builder.add_recipient(recipient.clone(), 10 * MOB).unwrap(); - builder.select_txos(None, false).unwrap(); + builder.select_txos(&conn, None, false).unwrap(); builder.set_tombstone(0).unwrap(); match builder.set_fee(0) { Ok(_) => panic!("Should not be able to set fee to 0"), @@ -921,13 +916,13 @@ mod tests { // Setting fee greater than MINIMUM_FEE works let (recipient, mut builder) = - builder_for_random_recipient(&account_key, &wallet_db, &ledger_db, &mut rng, &logger); + builder_for_random_recipient(&account_key, &ledger_db, &mut rng, &logger); builder.add_recipient(recipient.clone(), 10 * MOB).unwrap(); - builder.select_txos(None, false).unwrap(); + builder.select_txos(&conn, None, false).unwrap(); builder.set_tombstone(0).unwrap(); builder.set_fee(Mob::MINIMUM_FEE * 10).unwrap(); - let proposal = builder.build().unwrap(); + let proposal = builder.build(&conn).unwrap(); assert_eq!(proposal.tx.prefix.fee, Mob::MINIMUM_FEE * 10); } @@ -952,17 +947,18 @@ mod tests { &logger, ); + let conn = wallet_db.get_conn().unwrap(); let (recipient, mut builder) = - builder_for_random_recipient(&account_key, &wallet_db, &ledger_db, &mut rng, &logger); + builder_for_random_recipient(&account_key, &ledger_db, &mut rng, &logger); // Set value to consume the whole TXO and not produce change let value = 70 * MOB - Mob::MINIMUM_FEE; builder.add_recipient(recipient.clone(), value).unwrap(); - builder.select_txos(None, false).unwrap(); + builder.select_txos(&conn, None, false).unwrap(); builder.set_tombstone(0).unwrap(); // Verify that not setting fee results in default fee - let proposal = builder.build().unwrap(); + let proposal = builder.build(&conn).unwrap(); assert_eq!(proposal.tx.prefix.fee, Mob::MINIMUM_FEE); assert_eq!(proposal.outlays.len(), 1); assert_eq!(proposal.outlays[0].receiver, recipient); @@ -994,19 +990,20 @@ mod tests { &logger, ); + let conn = wallet_db.get_conn().unwrap(); let (recipient, mut builder) = - builder_for_random_recipient(&account_key, &wallet_db, &ledger_db, &mut rng, &logger); + builder_for_random_recipient(&account_key, &ledger_db, &mut rng, &logger); builder.add_recipient(recipient.clone(), 10 * MOB).unwrap(); builder.add_recipient(recipient.clone(), 20 * MOB).unwrap(); builder.add_recipient(recipient.clone(), 30 * MOB).unwrap(); builder.add_recipient(recipient.clone(), 40 * MOB).unwrap(); - builder.select_txos(None, false).unwrap(); + builder.select_txos(&conn, None, false).unwrap(); builder.set_tombstone(0).unwrap(); // Verify that not setting fee results in default fee - let proposal = builder.build().unwrap(); + let proposal = builder.build(&conn).unwrap(); assert_eq!(proposal.tx.prefix.fee, Mob::MINIMUM_FEE); assert_eq!(proposal.outlays.len(), 4); assert_eq!(proposal.outlays[0].receiver, recipient); @@ -1048,7 +1045,7 @@ mod tests { ); let (recipient, mut builder) = - builder_for_random_recipient(&account_key, &wallet_db, &ledger_db, &mut rng, &logger); + builder_for_random_recipient(&account_key, &ledger_db, &mut rng, &logger); builder .add_recipient(recipient.clone(), 7_000_000 * MOB) @@ -1060,7 +1057,7 @@ mod tests { .add_recipient(recipient.clone(), 7_000_000 * MOB) .unwrap(); - match builder.select_txos(None, false) { + match builder.select_txos(&wallet_db.get_conn().unwrap(), None, false) { Ok(_) => panic!("Should not be able to select txos with > u64::MAX output value"), Err(WalletTransactionBuilderError::OutboundValueTooLarge) => {} Err(e) => panic!("Unexpected error {:?}", e), @@ -1089,7 +1086,7 @@ mod tests { ); let (recipient, mut builder) = - builder_for_random_recipient(&account_key, &wallet_db, &ledger_db, &mut rng, &logger); + builder_for_random_recipient(&account_key, &ledger_db, &mut rng, &logger); builder.add_recipient(recipient.clone(), 10 * MOB).unwrap(); diff --git a/full-service/src/service/transaction_log.rs b/full-service/src/service/transaction_log.rs index 59856489a..fffe9fc29 100644 --- a/full-service/src/service/transaction_log.rs +++ b/full-service/src/service/transaction_log.rs @@ -7,17 +7,15 @@ use crate::{ account::AccountID, models::TransactionLog, transaction_log::{AssociatedTxos, TransactionLogModel}, + WalletDbError, }, error::WalletServiceError, WalletService, }; +use displaydoc::Display; use mc_connection::{BlockchainConnection, UserTxConnection}; use mc_fog_report_validation::FogPubkeyResolver; -use crate::db::WalletDbError; -use diesel::connection::Connection; -use displaydoc::Display; - /// Errors for the Transaction Log Service. #[derive(Display, Debug)] #[allow(clippy::large_enum_variant)] @@ -48,8 +46,8 @@ pub trait TransactionLogService { fn list_transaction_logs( &self, account_id: &AccountID, - offset: Option, - limit: Option, + offset: Option, + limit: Option, ) -> Result, WalletServiceError>; /// Get a specific transaction log. @@ -78,18 +76,16 @@ where fn list_transaction_logs( &self, account_id: &AccountID, - offset: Option, - limit: Option, + offset: Option, + limit: Option, ) -> Result, WalletServiceError> { let conn = &self.wallet_db.get_conn()?; - conn.transaction(|| { - Ok(TransactionLog::list_all( - &account_id.to_string(), - offset, - limit, - conn, - )?) - }) + Ok(TransactionLog::list_all( + &account_id.to_string(), + offset, + limit, + conn, + )?) } fn get_transaction_log( @@ -97,12 +93,10 @@ where transaction_id_hex: &str, ) -> Result<(TransactionLog, AssociatedTxos), TransactionLogServiceError> { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { - let transaction_log = TransactionLog::get(transaction_id_hex, &conn)?; - let associated = transaction_log.get_associated_txos(&conn)?; + let transaction_log = TransactionLog::get(transaction_id_hex, &conn)?; + let associated = transaction_log.get_associated_txos(&conn)?; - Ok((transaction_log, associated)) - }) + Ok((transaction_log, associated)) } fn get_all_transaction_logs_for_block( @@ -110,33 +104,29 @@ where block_index: u64, ) -> Result, WalletServiceError> { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { - let transaction_logs = TransactionLog::get_all_for_block_index(block_index, &conn)?; - let mut res: Vec<(TransactionLog, AssociatedTxos)> = Vec::new(); - for transaction_log in transaction_logs { - res.push(( - transaction_log.clone(), - transaction_log.get_associated_txos(&conn)?, - )); - } - Ok(res) - }) + let transaction_logs = TransactionLog::get_all_for_block_index(block_index, &conn)?; + let mut res: Vec<(TransactionLog, AssociatedTxos)> = Vec::new(); + for transaction_log in transaction_logs { + res.push(( + transaction_log.clone(), + transaction_log.get_associated_txos(&conn)?, + )); + } + Ok(res) } fn get_all_transaction_logs_ordered_by_block( &self, ) -> Result, WalletServiceError> { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { - let transaction_logs = TransactionLog::get_all_ordered_by_block_index(&conn)?; - let mut res: Vec<(TransactionLog, AssociatedTxos)> = Vec::new(); - for transaction_log in transaction_logs { - res.push(( - transaction_log.clone(), - transaction_log.get_associated_txos(&conn)?, - )); - } - Ok(res) - }) + let transaction_logs = TransactionLog::get_all_ordered_by_block_index(&conn)?; + let mut res: Vec<(TransactionLog, AssociatedTxos)> = Vec::new(); + for transaction_log in transaction_logs { + res.push(( + transaction_log.clone(), + transaction_log.get_associated_txos(&conn)?, + )); + } + Ok(res) } } diff --git a/full-service/src/service/txo.rs b/full-service/src/service/txo.rs index d797a3d6e..5b713a054 100644 --- a/full-service/src/service/txo.rs +++ b/full-service/src/service/txo.rs @@ -13,7 +13,6 @@ use crate::{ service::transaction::{TransactionService, TransactionServiceError}, WalletService, }; -use diesel::prelude::*; use displaydoc::Display; use mc_connection::{BlockchainConnection, UserTxConnection}; use mc_fog_report_validation::FogPubkeyResolver; @@ -103,24 +102,22 @@ where offset: Option, ) -> Result, TxoServiceError> { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { - Ok(Txo::list_for_account( - &account_id.to_string(), - limit, - offset, - &conn, - )?) - }) + Ok(Txo::list_for_account( + &account_id.to_string(), + limit, + offset, + &conn, + )?) } fn list_spent_txos(&self, account_id: &AccountID) -> Result, TxoServiceError> { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| Ok(Txo::list_spent(&account_id.to_string(), None, &conn)?)) + Ok(Txo::list_spent(&account_id.to_string(), None, &conn)?) } fn get_txo(&self, txo_id: &TxoID) -> Result { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| Ok(Txo::get(&txo_id.to_string(), &conn)?)) + Ok(Txo::get(&txo_id.to_string(), &conn)?) } fn split_txo( @@ -134,43 +131,41 @@ where use crate::service::txo::TxoServiceError::TxoNotSpendableByAnyAccount; let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { - let txo_details = Txo::get(&txo_id.to_string(), &conn)?; - - let account_id_hex = txo_details - .received_account_id_hex - .ok_or(TxoNotSpendableByAnyAccount(txo_details.txo_id_hex))?; - - let address_to_split_into: AssignedSubaddress = - AssignedSubaddress::get_for_account_by_index( - &account_id_hex, - subaddress_index.unwrap_or(0), - &conn, - )?; - - let mut addresses_and_values = Vec::new(); - for output_value in output_values.iter() { - addresses_and_values.push(( - address_to_split_into.assigned_subaddress_b58.clone(), - output_value.to_string(), - )) - } - - Ok(self.build_transaction( + let txo_details = Txo::get(&txo_id.to_string(), &conn)?; + + let account_id_hex = txo_details + .received_account_id_hex + .ok_or(TxoNotSpendableByAnyAccount(txo_details.txo_id_hex))?; + + let address_to_split_into: AssignedSubaddress = + AssignedSubaddress::get_for_account_by_index( &account_id_hex, - &addresses_and_values, - Some(&[txo_id.to_string()].to_vec()), - fee, - tombstone_block, - None, - None, - )?) - }) + subaddress_index.unwrap_or(0), + &conn, + )?; + + let mut addresses_and_values = Vec::new(); + for output_value in output_values.iter() { + addresses_and_values.push(( + address_to_split_into.assigned_subaddress_b58.clone(), + output_value.to_string(), + )) + } + + Ok(self.build_transaction( + &account_id_hex, + &addresses_and_values, + Some(&[txo_id.to_string()].to_vec()), + fee, + tombstone_block, + None, + None, + )?) } fn get_all_txos_for_address(&self, address: &str) -> Result, TxoServiceError> { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| Ok(Txo::list_for_address(address, &conn)?)) + Ok(Txo::list_for_address(address, &conn)?) } } diff --git a/full-service/src/service/view_only_account.rs b/full-service/src/service/view_only_account.rs index ccd8e47d2..62adfe04f 100644 --- a/full-service/src/service/view_only_account.rs +++ b/full-service/src/service/view_only_account.rs @@ -4,14 +4,13 @@ use crate::{ db::{ - models::{ViewOnlyAccount, ViewOnlyTxo}, + models::ViewOnlyAccount, + transaction, view_only_account::{ViewOnlyAccountID, ViewOnlyAccountModel}, - view_only_txo::ViewOnlyTxoModel, }, service::{account::AccountServiceError, WalletService}, util::constants::DEFAULT_FIRST_BLOCK_INDEX, }; -use diesel::Connection; use mc_common::logger::log; use mc_connection::{BlockchainConnection, UserTxConnection}; use mc_crypto_keys::RistrettoPrivate; @@ -77,16 +76,14 @@ where let first_block_index = first_block_index.unwrap_or(DEFAULT_FIRST_BLOCK_INDEX); let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { - Ok(ViewOnlyAccount::create( - &account_id_hex, - &view_private_key, - first_block_index, - import_block_index, - name, - &conn, - )?) - }) + Ok(ViewOnlyAccount::create( + &account_id_hex, + &view_private_key, + first_block_index, + import_block_index, + name, + &conn, + )?) } fn get_view_only_account( @@ -96,12 +93,12 @@ where log::info!(self.logger, "fetching view-only-account {:?}", account_id); let conn = self.wallet_db.get_conn()?; - conn.transaction(|| Ok(ViewOnlyAccount::get(account_id, &conn)?)) + Ok(ViewOnlyAccount::get(account_id, &conn)?) } fn list_view_only_accounts(&self) -> Result, AccountServiceError> { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| Ok(ViewOnlyAccount::list_all(&conn)?)) + Ok(ViewOnlyAccount::list_all(&conn)?) } fn update_view_only_account_name( @@ -110,23 +107,17 @@ where name: &str, ) -> Result { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { - ViewOnlyAccount::get(account_id, &conn)?.update_name(name, &conn)?; - Ok(ViewOnlyAccount::get(account_id, &conn)?) - }) + ViewOnlyAccount::get(account_id, &conn)?.update_name(name, &conn)?; + Ok(ViewOnlyAccount::get(account_id, &conn)?) } fn remove_view_only_account(&self, account_id: &str) -> Result { log::info!(self.logger, "Deleting view only account {}", account_id,); let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { - // delete associated view-only-txos - ViewOnlyTxo::delete_all_for_account(account_id, &conn)?; - - let account = ViewOnlyAccount::get(account_id, &conn)?; + let account = ViewOnlyAccount::get(account_id, &conn)?; + transaction(&conn, || { account.delete(&conn)?; - Ok(true) }) } diff --git a/full-service/src/service/view_only_transaction_log.rs b/full-service/src/service/view_only_transaction_log.rs index 6404692f0..f5d3ec4d4 100644 --- a/full-service/src/service/view_only_transaction_log.rs +++ b/full-service/src/service/view_only_transaction_log.rs @@ -4,12 +4,11 @@ use crate::{ db::{ - models::ViewOnlyTransactionLog, txo::TxoID, + models::ViewOnlyTransactionLog, transaction, txo::TxoID, view_only_transaction_log::ViewOnlyTransactionLogModel, WalletDbError, }, WalletService, }; -use diesel::prelude::*; use mc_connection::{BlockchainConnection, UserTxConnection}; use mc_fog_report_validation::FogPubkeyResolver; use mc_mobilecoind::payments::TxProposal; @@ -39,8 +38,6 @@ where &self, transaction_proposal: TxProposal, ) -> Result, WalletDbError> { - let conn = self.wallet_db.get_conn()?; - let mut input_txo_ids: Vec = vec![]; // get all of the inputs for the transaction @@ -54,7 +51,8 @@ where let change_txo_id = TxoID::from(change_txo).to_string(); // create a view only log for each input txo - conn.transaction::, WalletDbError, _>(|| { + let conn = self.wallet_db.get_conn()?; + transaction(&conn, || { let mut logs = vec![]; for txo_id in input_txo_ids { @@ -76,8 +74,7 @@ where txo_id: &str, ) -> Result, WalletDbError> { let conn = self.wallet_db.get_conn()?; - - conn.transaction(|| ViewOnlyTransactionLog::find_all_by_change_txo_id(txo_id, &conn)) + ViewOnlyTransactionLog::find_all_by_change_txo_id(txo_id, &conn) } } @@ -172,10 +169,10 @@ mod tests { // Create TxProposal from the sender account, which contains the Confirmation // Number log::info!(logger, "Creating transaction builder"); + let conn = wallet_db.get_conn().unwrap(); let mut builder: WalletTransactionBuilder = WalletTransactionBuilder::new( AccountID::from(&sender_account_key).to_string(), - wallet_db.clone(), ledger_db.clone(), get_resolver_factory(&mut rng).unwrap(), logger.clone(), @@ -183,9 +180,9 @@ mod tests { builder .add_recipient(recipient_account_key.default_subaddress(), 40 * MOB) .unwrap(); - builder.select_txos(None, false).unwrap(); + builder.select_txos(&conn, None, false).unwrap(); builder.set_tombstone(0).unwrap(); - let proposal = builder.build().unwrap(); + let proposal = builder.build(&conn).unwrap(); // find change txo from proposal let change_txo = get_change_txout_from_proposal(&proposal).unwrap(); diff --git a/full-service/src/service/view_only_txo.rs b/full-service/src/service/view_only_txo.rs index 03b478188..84d2ac39b 100644 --- a/full-service/src/service/view_only_txo.rs +++ b/full-service/src/service/view_only_txo.rs @@ -3,11 +3,10 @@ //! Service for managing view-only Txos. use crate::{ - db::{models::ViewOnlyTxo, view_only_txo::ViewOnlyTxoModel}, + db::{models::ViewOnlyTxo, transaction, view_only_txo::ViewOnlyTxoModel}, service::txo::TxoServiceError, WalletService, }; -use diesel::prelude::*; use mc_connection::{BlockchainConnection, UserTxConnection}; use mc_fog_report_validation::FogPubkeyResolver; @@ -18,8 +17,8 @@ pub trait ViewOnlyTxoService { fn list_view_only_txos( &self, account_id: &str, - limit: Option, - offset: Option, + limit: Option, + offset: Option, ) -> Result, TxoServiceError>; /// set a group of txos as spent. @@ -34,20 +33,18 @@ where fn list_view_only_txos( &self, account_id: &str, - limit: Option, - offset: Option, + limit: Option, + offset: Option, ) -> Result, TxoServiceError> { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { - Ok(ViewOnlyTxo::list_for_account( - account_id, limit, offset, &conn, - )?) - }) + Ok(ViewOnlyTxo::list_for_account( + account_id, limit, offset, &conn, + )?) } fn set_view_only_txos_spent(&self, txo_ids: Vec) -> Result { let conn = self.wallet_db.get_conn()?; - conn.transaction(|| { + transaction(&conn, || { ViewOnlyTxo::set_spent(txo_ids, &conn)?; Ok(true) }) diff --git a/full-service/src/test_utils.rs b/full-service/src/test_utils.rs index 0b793638d..a0991e317 100644 --- a/full-service/src/test_utils.rs +++ b/full-service/src/test_utils.rs @@ -510,16 +510,16 @@ pub fn create_test_minted_and_change_txos( // Use the builder to create valid TxOuts for this account let mut builder = WalletTransactionBuilder::::new( AccountID::from(&src_account_key).to_string(), - wallet_db.clone(), ledger_db, get_resolver_factory(&mut rng).unwrap(), logger, ); + let conn = wallet_db.get_conn().unwrap(); builder.add_recipient(recipient, value).unwrap(); - builder.select_txos(None, false).unwrap(); + builder.select_txos(&conn, None, false).unwrap(); builder.set_tombstone(0).unwrap(); - let tx_proposal = builder.build().unwrap(); + let tx_proposal = builder.build(&conn).unwrap(); // There should be 2 outputs, one to dest and one change assert_eq!(tx_proposal.tx.prefix.outputs.len(), 2); @@ -533,7 +533,7 @@ pub fn create_test_minted_and_change_txos( &tx_out, &tx_proposal, outlay_txo_index, - &wallet_db.get_conn().unwrap(), + &conn, ) .unwrap(); assert!(processed_output.recipient.is_some()); @@ -547,7 +547,7 @@ pub fn create_test_minted_and_change_txos( &change_tx_out, &tx_proposal, change_txo_index, - &wallet_db.get_conn().unwrap(), + &conn, ) .unwrap(); assert_eq!(processed_change.recipient, None,); @@ -621,7 +621,6 @@ pub fn random_account_with_seed_values( pub fn builder_for_random_recipient( account_key: &AccountKey, - wallet_db: &WalletDb, ledger_db: &LedgerDB, mut rng: &mut StdRng, logger: &Logger, @@ -632,7 +631,6 @@ pub fn builder_for_random_recipient( // Construct a transaction let builder: WalletTransactionBuilder = WalletTransactionBuilder::new( AccountID::from(account_key).to_string(), - wallet_db.clone(), ledger_db.clone(), get_resolver_factory(&mut rng).unwrap(), logger.clone(),