fix(Revert): "refactor: purge unconstrained functions where possible" (…

…#5911)

Reverts #5819. The use of constrained
functions made the writing of tests etc nicer, but the testing time
absolutely exploded as an effect 💀

Co-authored-by: ludamad <[email protected]>

noir-projects/noir-contracts/Nargo.toml

@@ -40,5 +40,6 @@ members = [
     "contracts/token_blacklist_contract",
     "contracts/token_bridge_contract",
     "contracts/uniswap_contract",
+    "contracts/reader_contract",
     "contracts/multi_call_entrypoint_contract",
 ]

noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr

@@ -41,6 +41,18 @@ contract AvmTest {
     /************************************************************************
      * Storage
      ************************************************************************/
+    unconstrained fn view_storage_single() -> pub Field {
+        storage.single.read()
+    }
+
+    unconstrained fn view_storage_list() -> pub [Field; 2] {
+        storage.list.read().serialize()
+    }
+
+    unconstrained fn view_storage_map(address: AztecAddress) -> pub u32 {
+        storage.map.at(address).read()
+    }
+
     #[aztec(public-vm)]
     fn set_storage_single(a: Field) {
         storage.single.write(a);

noir-projects/noir-contracts/contracts/gas_token_contract/src/main.nr

@@ -55,8 +55,8 @@ contract GasToken {
         rebate.to_field()
     }
 
-    #[aztec(public)]
-    fn balance_of_public(owner: AztecAddress) -> pub Field {
+    // utility function for testing
+    unconstrained fn balance_of_public(owner: AztecAddress) -> pub Field {
         storage.balances.at(owner).read().to_field()
     }
 }

noir-projects/noir-contracts/contracts/lending_contract/src/asset.nr

@@ -13,16 +13,14 @@ struct Asset {
     oracle: AztecAddress,
 }
 
-global SERIALIZED_LEN: Field = 6;
+global SERIALIZED_LEN: Field = 4;
 
 impl Serialize<SERIALIZED_LEN> for Asset {
     fn serialize(Asset: Asset) -> [Field; SERIALIZED_LEN] {
         [
-            Asset.interest_accumulator.lo as Field,
-            Asset.interest_accumulator.hi as Field,
+            Asset.interest_accumulator.to_integer(),
             Asset.last_updated_ts as Field,
-            Asset.loan_to_value.lo as Field,
-            Asset.loan_to_value.hi as Field,
+            Asset.loan_to_value.to_integer(),
             Asset.oracle.to_field()
         ]
     }
@@ -32,10 +30,10 @@ impl Deserialize<SERIALIZED_LEN> for Asset {
     // Right now we are wasting so many writes. If changing last_updated_ts
     // we will end up rewriting all of them, wasting writes.
     fn deserialize(fields: [Field; SERIALIZED_LEN]) -> Asset {
-        let interest_accumulator = U128 {lo: fields[0], hi: fields[1]};
-        let last_updated_ts = fields[2] as u64;
-        let loan_to_value = U128 {lo: fields[3], hi: fields[4]};
-        let oracle = AztecAddress::from_field(fields[5]);
+        let interest_accumulator = U128::from_integer(fields[0]);
+        let last_updated_ts = fields[1] as u64;
+        let loan_to_value = U128::from_integer(fields[2]);
+        let oracle = AztecAddress::from_field(fields[3]);
 
         Asset {
             interest_accumulator,

noir-projects/noir-contracts/contracts/lending_contract/src/main.nr

@@ -1,5 +1,4 @@
 mod asset;
-mod position;
 mod interest_math;
 mod helpers;
 
@@ -15,7 +14,6 @@ contract Lending {
     use dep::aztec::context::{PublicContext, Context, gas::GasOpts};
 
     use crate::asset::Asset;
-    use crate::position::Position;
     use crate::interest_math::compute_multiplier;
     use crate::helpers::{covered_by_collateral, DebtReturn, debt_updates, debt_value, compute_identifier};
     use dep::token::Token;
@@ -31,6 +29,12 @@ contract Lending {
         static_debt: Map<AztecAddress, PublicMutable<Field>>, // abusing keys very heavily
     }
 
+    struct Position {
+        collateral: Field,
+        static_debt: Field,
+        debt: Field,
+    }
+
     // Constructs the contract.
     #[aztec(private)]
     #[aztec(initializer)]
@@ -261,13 +265,11 @@ contract Lending {
         storage.static_debt.at(owner).write(debt_returns.static_debt.to_integer());
     }
 
-    #[aztec(public)]
-    fn get_asset(asset_id: Field) -> pub Asset {
+    unconstrained fn get_asset(asset_id: Field) -> pub Asset {
         storage.assets.at(asset_id).read()
     }
 
-    #[aztec(public)]
-    fn get_position(owner: AztecAddress) -> pub Position {
+    unconstrained fn get_position(owner: AztecAddress) -> pub Position {
         let collateral = storage.collateral.at(owner).read();
         let static_debt = storage.static_debt.at(owner).read();
         let asset: Asset = storage.assets.at(0).read();
@@ -278,8 +280,7 @@ contract Lending {
         Position { collateral, static_debt, debt }
     }
 
-    #[aztec(public)]
-    fn get_assets() -> pub [AztecAddress; 2] {
+    unconstrained fn get_assets() -> pub [AztecAddress; 2] {
         [storage.collateral_asset.read(), storage.stable_coin.read()]
     }
 }

noir-projects/noir-contracts/contracts/price_feed_contract/src/asset.nr

@@ -8,7 +8,7 @@ global ASSET_SERIALIZED_LEN: Field = 1;
 
 impl Serialize<ASSET_SERIALIZED_LEN> for Asset {
     fn serialize(asset: Asset) -> [Field; ASSET_SERIALIZED_LEN] {
-        [asset.price.to_field()]
+        [asset.price.to_integer()]
     }
 }
 

noir-projects/noir-contracts/contracts/price_feed_contract/src/main.nr

@@ -21,4 +21,8 @@ contract PriceFeed {
     fn get_price(asset_id: Field) -> Asset {
         storage.assets.at(asset_id).read()
     }
+
+    unconstrained fn fetch_price(asset_id: Field) -> pub Asset {
+        storage.assets.at(asset_id).read()
+    }
 }

noir-projects/noir-contracts/contracts/reader_contract/Nargo.toml

@@ -0,0 +1,9 @@
+[package]
+name = "reader_contract"
+authors = [""]
+compiler_version = ">=0.25.0"
+type = "contract"
+
+[dependencies]
+aztec = { path = "../../../aztec-nr/aztec" }
+compressed_string = { path = "../../../aztec-nr/compressed-string" }

noir-projects/noir-contracts/contracts/reader_contract/src/main.nr

@@ -0,0 +1,69 @@
+contract Reader {
+    use dep::aztec::prelude::{AztecAddress, FunctionSelector, Deserialize};
+
+    use dep::compressed_string::FieldCompressedString;
+
+    #[aztec(private)]
+    fn constructor() {}
+
+    #[aztec(public)]
+    fn check_name_public(who: AztecAddress, what: str<31>) {
+        let selector = FunctionSelector::from_signature("public_get_name()");
+        let name: FieldCompressedString = context.call_public_function_no_args(who, selector).deserialize_into();
+        let _what = FieldCompressedString::from_string(what);
+        assert(name.is_eq(_what));
+    }
+
+    #[aztec(private)]
+    fn check_name_private(who: AztecAddress, what: str<31>) {
+        let selector = FunctionSelector::from_signature("private_get_name()");
+        let name: FieldCompressedString = context.call_private_function_no_args(who, selector).unpack_into();
+        let _what = FieldCompressedString::from_string(what);
+        assert(name.is_eq(_what));
+    }
+
+    unconstrained fn get_name(who: AztecAddress) -> pub str<6> {
+        // We cannot yet call an unconstrained function from another
+        "Reader"
+    }
+
+    #[aztec(public)]
+    fn check_symbol_public(who: AztecAddress, what: str<31>) {
+        let selector = FunctionSelector::from_signature("public_get_symbol()");
+        let symbol: FieldCompressedString = context.call_public_function_no_args(who, selector).deserialize_into();
+        let _what = FieldCompressedString::from_string(what);
+        assert(symbol.is_eq(_what));
+    }
+
+    #[aztec(private)]
+    fn check_symbol_private(who: AztecAddress, what: str<31>) {
+        let selector = FunctionSelector::from_signature("private_get_symbol()");
+        let symbol: FieldCompressedString = context.call_private_function_no_args(who, selector).unpack_into();
+        let _what = FieldCompressedString::from_string(what);
+        assert(symbol.is_eq(_what));
+    }
+
+    unconstrained fn get_symbol(who: AztecAddress) -> pub str<3> {
+        // We cannot yet call an unconstrained function from another
+        "RDR"
+    }
+
+    #[aztec(public)]
+    fn check_decimals_public(who: AztecAddress, what: u8) {
+        let selector = FunctionSelector::from_signature("public_get_decimals()");
+        let ret: u8 = context.call_public_function_no_args(who, selector).deserialize_into();
+        assert(ret == what);
+    }
+
+    #[aztec(private)]
+    fn check_decimals_private(who: AztecAddress, what: u8) {
+        let selector = FunctionSelector::from_signature("private_get_decimals()");
+        let result: u8 = context.call_private_function_no_args(who, selector).unpack_into();
+        assert(result == what);
+    }
+
+    unconstrained fn get_decimals(who: AztecAddress) -> pub u8 {
+        // We cannot yet call an unconstrained function from another
+        18
+    }
+}

noir-projects/noir-contracts/contracts/stateful_test_contract/src/main.nr

@@ -94,8 +94,7 @@ contract StatefulTest {
         balance_utils::get_balance(owner_balance)
     }
 
-    #[aztec(public)]
-    fn get_public_value(owner: AztecAddress) -> pub Field {
+    unconstrained fn get_public_value(owner: AztecAddress) -> pub Field {
         storage.public_values.at(owner).read()
     }
 }

noir-projects/noir-contracts/contracts/token_blacklist_contract/src/main.nr

@@ -38,16 +38,6 @@ contract TokenBlacklist {
         slow_update: SharedImmutable<AztecAddress>,
     }
 
-    #[aztec(public)]
-    fn total_supply() -> pub Field {
-        storage.total_supply.read().to_field()
-    }
-
-    #[aztec(public)]
-    fn balance_of_public(owner: AztecAddress) -> pub Field {
-        storage.public_balances.at(owner).read().to_field()
-    }
-
     // docs:start:constructor
     #[aztec(public)]
     #[aztec(initializer)]
@@ -237,7 +227,8 @@ contract TokenBlacklist {
 
         storage.balances.sub(from, U128::from_integer(amount));
 
-        TokenBlacklist::at(context.this_address())._increase_public_balance(to, amount).enqueue(&mut context);
+        let selector = FunctionSelector::from_signature("_increase_public_balance((Field),Field)");
+        context.call_public_function(context.this_address(), selector, [to.to_field(), amount]);
     }
 
     // docs:start:transfer_private
@@ -275,7 +266,8 @@ contract TokenBlacklist {
 
         storage.balances.sub(from, U128::from_integer(amount));
 
-        TokenBlacklist::at(context.this_address())._reduce_total_supply(amount).enqueue(&mut context);
+        let selector = FunctionSelector::from_signature("_reduce_total_supply(Field)");
+        context.call_public_function(context.this_address(), selector, [amount]);
     }
 
     /// Internal ///
@@ -297,7 +289,15 @@ contract TokenBlacklist {
 
     /// Unconstrained ///
 
+    unconstrained fn total_supply() -> pub Field {
+        storage.total_supply.read().to_field()
+    }
+
     unconstrained fn balance_of_private(owner: AztecAddress) -> pub Field {
         storage.balances.balance_of(owner).to_field()
     }
+
+    unconstrained fn balance_of_public(owner: AztecAddress) -> pub Field {
+        storage.public_balances.at(owner).read().to_field()
+    }
 }

noir-projects/noir-contracts/contracts/token_bridge_contract/src/main.nr

@@ -26,7 +26,8 @@ contract TokenBridge {
     #[aztec(private)]
     #[aztec(initializer)]
     fn constructor(token: AztecAddress) {
-        TokenBridge::at(context.this_address())._initialize(token).enqueue(&mut context);
+        let selector = FunctionSelector::from_signature("_initialize((Field))");
+        context.call_public_function(context.this_address(), selector, [token.to_field()]);
     }
     // docs:end:token_bridge_storage_and_constructor
 
@@ -98,7 +99,11 @@ contract TokenBridge {
         // `mint_private` on token is public. So we call an internal public function
         // which then calls the public method on the token contract.
         // Since the secret_hash is passed, no secret is leaked.
-        TokenBridge::at(context.this_address())._call_mint_on_token(amount, secret_hash_for_redeeming_minted_notes).enqueue(&mut context);
+        context.call_public_function(
+            context.this_address(),
+            FunctionSelector::from_signature("_call_mint_on_token(Field,Field)"),
+            [amount, secret_hash_for_redeeming_minted_notes]
+        );
     }
     // docs:end:claim_private
 
@@ -119,19 +124,31 @@ contract TokenBridge {
 
         // docs:start:call_assert_token_is_same
         // Assert that user provided token address is same as seen in storage.
-        TokenBridge::at(context.this_address())._assert_token_is_same(token).enqueue(&mut context);
+        context.call_public_function(
+            context.this_address(),
+            FunctionSelector::from_signature("_assert_token_is_same((Field))"),
+            [token.to_field()]
+        );
         // docs:end:call_assert_token_is_same
 
         // Burn tokens
         Token::at(token).burn(context.msg_sender(), amount, nonce).call(&mut context);
     }
     /// docs:end:exit_to_l1_private
 
-    // docs:start:read_token
+    // View function that is callable by other contracts.
+    // Unconstrained can't be called by others since it isn't safe.
     #[aztec(public)]
     fn get_token() -> AztecAddress {
         storage.token.read()
     }
+
+    // /// Unconstrained ///
+
+    // docs:start:read_token
+    unconstrained fn token() -> pub AztecAddress {
+        storage.token.read()
+    }
     // docs:end:read_token
 
     #[aztec(public)]