Repository: stellar/rs-soroban-env
Pull request #626 (2023-01-06 20:29:55)

Add auth_required support for non-account balances
This is based off of an earlier [prototype](https://github.com/stellar/rs-soroban-env/pull/578). The goal of this change is to enforce auth_required on non-account balances. The admin will need to authorize users before they can hold a balance.


Patch:
diff --git a/soroban-env-host/src/native_contract/token/balance.rs b/soroban-env-host/src/native_contract/token/balance.rs
index 3447a339..f7eba22e 100644
--- a/soroban-env-host/src/native_contract/token/balance.rs
+++ b/soroban-env-host/src/native_contract/token/balance.rs
@@ -11,6 +11,17 @@ use soroban_env_common::xdr::{
 use soroban_env_common::{CheckedEnv, TryIntoVal};
 
 use super::error::ContractError;
+use super::storage_types::BalanceValue;
+
+/// This module handles all balance and authorization related logic for both
+/// Accounts and non-Accounts. For Accounts, a trustline is expected (unless this
+/// contract is for the native asset) and trustline semantics will be followed,
+/// while non-Accounts will use ContractData.
+///
+/// Even though non-account balances don't use trustlines, some issuer/trustline
+/// semantics have been implemented for these balances. If the asset issuer has
+/// the AUTH_REQUIRED flag set, then the non-account identifier must first be authorized
+/// by the issuer/admin before it's allowed to hold a balance.
 
 // Metering: *mostly* covered by components. Not sure about `try_into_val`.
 pub fn read_balance(e: &Host, id: Identifier) -> Result<i128, HostError> {
@@ -18,8 +29,9 @@ pub fn read_balance(e: &Host, id: Identifier) -> Result<i128, HostError> {
         Identifier::Account(acc_id) => Ok(get_classic_balance(e, acc_id)?.0.into()),
         Identifier::Contract(_) | Identifier::Ed25519(_) => {
             let key = DataKey::Balance(id);
-            if let Ok(balance) = e.get_contract_data(key.try_into_val(e)?) {
-                Ok(balance.try_into_val(e)?)
+            if let Ok(raw_balance) = e.get_contract_data(key.try_into_val(e)?) {
+                let balance: BalanceValue = raw_balance.try_into_val(e)?;
+                Ok(balance.amount)
             } else {
                 Ok(0)
             }
@@ -35,10 +47,17 @@ pub fn get_spendable_balance(e: &Host, id: Identifier) -> Result<i128, HostError
     }
 }
 
-// Metering: *mostly* covered by components. Not sure about `try_into_val`.
-fn write_balance(e: &Host, id: Identifier, amount: i128) -> Result<(), HostError> {
+fn write_balance_and_auth(
+    e: &Host,
+    id: Identifier,
+    amount: i128,
+    authorized: bool,
+) -> Result<(), HostError> {
     let key = DataKey::Balance(id);
-    e.put_contract_data(key.try_into_val(e)?, amount.try_into_val(e)?)?;
+    e.put_contract_data(
+        key.try_into_val(e)?,
+        BalanceValue { amount, authorized }.try_into_val(e)?,
+    )?;
     Ok(())
 }
 
@@ -63,10 +82,13 @@ pub fn receive_balance(e: &Host, id: Identifier, amount: i128) -> Result<(), Hos
         }
         Identifier::Contract(_) | Identifier::Ed25519(_) => {
             let balance = read_balance(e, id.clone())?;
+
             let new_balance = balance
                 .checked_add(amount)
                 .ok_or_else(|| e.err_status(ContractError::OverflowError))?;
-            write_balance(e, id, new_balance)
+
+            // balance passed the authorization check at the top of this function, so write true.
+            write_balance_and_auth(e, id, new_balance, true)
         }
     }
 }
@@ -88,21 +110,35 @@ pub fn spend_balance_no_authorization_check(
             transfer_classic_balance(e, acc_id, -(i64_amount as i64))
         }
         Identifier::Contract(_) | Identifier::Ed25519(_) => {
-            let balance = read_balance(e, id.clone())?;
-            if balance < amount {
-                Err(err!(
+            // If a balance exists, calculate new amount and write the existing authorized state as is because
+            // this can be used to clawback when deauthorized.
+            let key = DataKey::Balance(id.clone());
+            if let Ok(raw_balance) = e.get_contract_data(key.try_into_val(e)?) {
+                let balance: BalanceValue = raw_balance.try_into_val(e)?;
+                if balance.amount < amount {
+                    return Err(err!(
+                        e,
+                        ContractError::BalanceError,
+                        "balance is not sufficient to spend: {} < {}",
+                        balance,
+                        amount
+                    ));
+                } else {
+                    let new_balance = balance
+                        .amount
+                        .checked_sub(amount)
+                        .ok_or_else(|| e.err_status(ContractError::OverflowError))?;
+                    write_balance_and_auth(e, id, new_balance, balance.authorized)?
+                }
+            } else if amount > 0 {
+                return Err(err!(
                     e,
                     ContractError::BalanceError,
-                    "balance is not sufficient to spend: {} < {}",
-                    balance,
+                    "balance is not sufficient to spend: 0 < {}",
                     amount
-                ))
-            } else {
-                let new_balance = balance
-                    .checked_sub(amount)
-                    .ok_or_else(|| e.err_status(ContractError::OverflowError))?;
-                write_balance(e, id, new_balance)
+                ));
             }
+            Ok(())
         }
     }
 }
@@ -124,11 +160,12 @@ pub fn is_authorized(e: &Host, id: Identifier) -> Result<bool, HostError> {
     match id {
         Identifier::Account(acc_id) => is_account_authorized(e, acc_id),
         Identifier::Contract(_) | Identifier::Ed25519(_) => {
-            let key = DataKey::State(id);
-            if let Ok(state) = e.get_contract_data(key.try_into_val(e)?) {
-                Ok(state.try_into()?)
+            let key = DataKey::Balance(id);
+            if let Ok(raw_balance) = e.get_contract_data(key.try_into_val(e)?) {
+                let balance: BalanceValue = raw_balance.try_into_val(e)?;
+                Ok(balance.authorized)
             } else {
-                Ok(true)
+                Ok(!is_asset_auth_required(e)?)
             }
         }
     }
@@ -139,9 +176,15 @@ pub fn write_authorization(e: &Host, id: Identifier, authorize: bool) -> Result<
     match id {
         Identifier::Account(acc_id) => set_authorization(e, acc_id, authorize),
         Identifier::Contract(_) | Identifier::Ed25519(_) => {
-            let key = DataKey::State(id);
-            e.put_contract_data(key.try_into_val(e)?, authorize.into())?;
-            Ok(())
+            let key = DataKey::Balance(id.clone());
+            if let Ok(raw_balance) = e.get_contract_data(key.try_into_val(e)?) {
+                let balance: BalanceValue = raw_balance.try_into_val(e)?;
+                write_balance_and_auth(e, id, balance.amount, authorize)
+            } else {
+                // Balance does not exist, so write a 0 amount along with the authorization flag.
+                // No need to check auth_required because this function can only be called by the admin.
+                write_balance_and_auth(e, id, 0, authorize)
+            }
         }
     }
 }
@@ -560,3 +603,16 @@ fn set_trustline_authorization(
         storage.put(&lk, &le, e.as_budget())
     })
 }
+
+fn is_issuer_auth_required(e: &Host, issuer_id: AccountId) -> Result<bool, HostError> {
+    let issuer_acc = e.load_account(issuer_id.clone())?;
+    Ok(issuer_acc.flags & (AccountFlags::RequiredFlag as u32) != 0)
+}
+
+fn is_asset_auth_required(e: &Host) -> Result<bool, HostError> {
+    match read_metadata(e)? {
+        Metadata::Native => Ok(false),
+        Metadata::AlphaNum4(asset) => is_issuer_auth_required(e, asset.issuer),
+        Metadata::AlphaNum12(asset) => is_issuer_auth_required(e, asset.issuer),
+    }
+}
diff --git a/soroban-env-host/src/native_contract/token/storage_types.rs b/soroban-env-host/src/native_contract/token/storage_types.rs
index b402a3b9..a49626ca 100644
--- a/soroban-env-host/src/native_contract/token/storage_types.rs
+++ b/soroban-env-host/src/native_contract/token/storage_types.rs
@@ -9,12 +9,17 @@ pub struct AllowanceDataKey {
     pub spender: Identifier,
 }
 
+#[contracttype]
+pub struct BalanceValue {
+    pub amount: i128,
+    pub authorized: bool,
+}
+
 #[contracttype]
 pub enum DataKey {
     Allowance(AllowanceDataKey),
     Balance(Identifier),
     Nonce(Identifier),
-    State(Identifier),
     Admin,
     Metadata,
 }


Test patch:
diff --git a/soroban-env-host/src/test/token.rs b/soroban-env-host/src/test/token.rs
index d10d2019..e3ca9e8e 100644
--- a/soroban-env-host/src/test/token.rs
+++ b/soroban-env-host/src/test/token.rs
@@ -2728,3 +2728,56 @@ fn test_classic_transfers_not_possible_for_unauthorized_asset() {
         100_000_000
     );
 }
+
+#[test]
+fn test_non_account_auth_required() {
+    let test = TokenTest::setup();
+
+    // the admin is the issuer_key
+    let admin_acc = signer_to_account_id(&test.host, &test.issuer_key);
+    let user = TestSigner::Ed25519(&test.user_key);
+
+    let admin_id = Identifier::Account(admin_acc.clone());
+    let user_id = user.get_identifier(&test.host);
+
+    let acc_invoker = TestSigner::AccountInvoker;
+    let token = test.default_token_with_admin_id(admin_id.clone());
+
+    test.create_classic_account(
+        &admin_acc,
+        vec![(&test.admin_key, 100)],
+        10_000_000,
+        1,
+        [1, 0, 0, 0],
+        None,
+        None,
+        AccountFlags::RequiredFlag as u32,
+    );
+
+    // user_id is deauthorized by default because the issuer has AUTH_REQUIRED set,
+    assert!(!token.authorized(user_id.clone()).unwrap());
+
+    // xfer to user_id will fail because the issuer has the AUTH_REQUIRED flag set
+    assert_eq!(
+        to_contract_err(
+            test.run_from_account(admin_acc.clone(), || {
+                token.xfer(&acc_invoker, 0, user_id.clone(), 1)
+            })
+            .err()
+            .unwrap()
+        ),
+        ContractError::BalanceDeauthorizedError
+    );
+
+    // authorize user_id
+    test.run_from_account(admin_acc.clone(), || {
+        token.set_auth(&acc_invoker, 0, user_id.clone(), true)
+    })
+    .unwrap();
+
+    // user_id can now hold a balance
+    test.run_from_account(admin_acc.clone(), || {
+        token.xfer(&acc_invoker, 0, user_id.clone(), 1)
+    })
+    .unwrap();
+}