Add Require Spend Subaddress

[sugargoat]

Motivation

We would like to provide a guard rail for the use of subaddress_to_spend_from, so that an account can enforce that transactions using the Txos within the account adhere to the scheme of spending from any Txo regardless of which subaddress it was sent to (the turnstile model), or requiring that a subaddress to spend from is always specified (so the Txos don’t get mixed together and create inconsistent subaddress balances)

In this PR

• Adds require_spend_subaddresses to the Account db as bool column
• Adds database migration
• Adds require_spend_subaddresses to the APIs for create and import account
• Adds logic to enforce building a transaction with an account with this enabled or not uses the correct params
• Renames subaddress_to_spend_from to spend_from_subaddress for brevity & clarity
• Adds tests for mode mismatches, and updates existing tests for spend_from_subaddress to correctly initialize the account with require_spend_subaddress
• Adds toggle to enable or disable this mode for an account
• Updates documentation for these API endpoints (Updates here)

Test Plan

• Update unittests for existing spend_from_subaddress tests
• Update e2e test and add new e2e tests for mismatches to enforce failures
• Add new unittests for require subaddress functionality in transaction service Going with minimum viable e2e testing

Future Work

• Add a --strict-params mode to prevent misspellings from accidentally omitting the parameter

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 50.49505% with 50 lines in your changes are missing coverage. Please review.

Project coverage is 55.61%. Comparing base (ab2af32) to head (c416c74).
Report is 193 commits behind head on main.

Files	Patch %	Lines
full-service/src/json_rpc/v2/api/wallet.rs	54.05%	10 Missing and 7 partials ⚠️
full-service/src/json_rpc/v2/api/request.rs	8.33%	10 Missing and 1 partial ⚠️
full-service/src/service/account.rs	52.63%	2 Missing and 7 partials ⚠️
full-service/src/service/transaction.rs	44.44%	0 Missing and 5 partials ⚠️
full-service/src/db/account.rs	66.66%	1 Missing and 1 partial ⚠️
...-service/src/json_rpc/v2/models/account_secrets.rs	33.33%	2 Missing ⚠️
full-service/src/db/schema.rs	0.00%	0 Missing and 1 partial ⚠️
full-service/src/json_rpc/v2/api/response.rs	0.00%	1 Missing ⚠️
full-service/src/json_rpc/v2/models/account.rs	50.00%	1 Missing ⚠️
full-service/src/service/gift_code.rs	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #989      +/-   ##
==========================================
- Coverage   60.12%   55.61%   -4.51%     
==========================================
  Files          88      124      +36     
  Lines       12356    16290    +3934     
  Branches     2010     2796     +786     
==========================================
+ Hits         7429     9060    +1631     
- Misses       3238     5145    +1907     
- Partials     1689     2085     +396     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[lnsiegel]

All the initial pass plumbing for subaddress_to_spend_from and subaddress_to_spend_from_mode looks good.

The description says that the PR "Adds logic to enforce building a transaction with an account with this enabled or not uses the correct params". But I'm not seeing the code that implements that logic. Maybe I'm missing it somehow. If not, should it be part of the PR?

[holtzman]

@sugargoat I like the renaming, and while we are at it, what about if we make it:

• spend_subaddress for the field that specifies which subaddress to spend from, and
• require_spend_subaddress for the mode that requires it?

This would be consistent and concise. If you are ok with this, I'd be happy to do the search-replace and push a commit.

[lnsiegel]

Renaming and init defaults look good to me!

[nick-mobilecoin]

Everything looks good, just need:

1. The build_unsigned_transaction @holtzman mentioned
2. Be sure we want to prevent one from spending from a subaddress if the account is not a require subaddress spend

[sugargoat]

@sugargoat I like the renaming, and while we are at it, what about if we make it:

• spend_subaddress for the field that specifies which subaddress to spend from, and
• require_spend_subaddress for the mode that requires it?

This would be consistent and concise. If you are ok with this, I'd be happy to do the search-replace and push a commit.

No worries - happy to get the renaming in order!

[holtzman]

Looks good. Thanks!