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(),