valid contract address should be known in constructor and be a privat…

…e input to kernel even for contract deployment (#179)

cpp/src/aztec3/circuits/apps/test_apps/basic_contract_deployment/basic_contract_deployment.cpp

@@ -50,7 +50,7 @@ OptionalPrivateCircuitPublicInputs<NT> constructor(FunctionExecutionContext& exe
 
     exec_ctx.finalise();
 
-    info("public inputs: ", public_inputs);
+    // info("public inputs: ", public_inputs);
 
     return public_inputs.to_native_type<C>();
 }

cpp/src/aztec3/circuits/apps/test_apps/escrow/deposit.cpp

@@ -62,7 +62,7 @@ OptionalPrivateCircuitPublicInputs<NT> deposit(FunctionExecutionContext& exec_ct
 
     exec_ctx.finalise();
 
-    info("public inputs: ", public_inputs);
+    // info("public inputs: ", public_inputs);
 
     return public_inputs.to_native_type<C>();
     // TODO: also return note preimages and nullifier preimages.

cpp/src/aztec3/circuits/apps/test_apps/escrow/transfer.cpp

@@ -105,7 +105,7 @@ OptionalPrivateCircuitPublicInputs<NT> transfer(FunctionExecutionContext& exec_c
 
     exec_ctx.finalise();
 
-    info("public inputs: ", public_inputs);
+    // info("public inputs: ", public_inputs);
 
     return public_inputs.to_native_type<C>();
 };

cpp/src/aztec3/circuits/apps/test_apps/escrow/withdraw.cpp

@@ -97,7 +97,7 @@ OptionalPrivateCircuitPublicInputs<NT> withdraw(FunctionExecutionContext& exec_c
     /// TODO: merkle membership check
     // public_inputs.historic_private_data_tree_root
 
-    info("public inputs: ", public_inputs);
+    // info("public inputs: ", public_inputs);
 
     return public_inputs.to_native_type<C>();
 };

cpp/src/aztec3/circuits/apps/test_apps/private_to_private_function_call/function_2_1.cpp

@@ -68,7 +68,7 @@ void function_2_1(FunctionExecutionContext& exec_ctx, std::array<NT::fr, ARGS_LE
 
     exec_ctx.finalise();
 
-    info("public inputs: ", public_inputs);
+    // info("public inputs: ", public_inputs);
 
     // TODO: also return note preimages and nullifier preimages.
 };

cpp/src/aztec3/circuits/kernel/private/.test.cpp

@@ -7,6 +7,8 @@
 #include "init.hpp"
 #include "c_bind.h"
 
+#include <aztec3/circuits/hash.hpp>
+
 #include <aztec3/circuits/apps/test_apps/escrow/deposit.hpp>
 #include <aztec3/circuits/apps/test_apps/basic_contract_deployment/basic_contract_deployment.hpp>
 
@@ -43,6 +45,7 @@
 
 namespace {
 
+using aztec3::circuits::compute_constructor_hash;
 using aztec3::circuits::abis::CallContext;
 using aztec3::circuits::abis::CallStackItem;
 using aztec3::circuits::abis::CallType;
@@ -469,8 +472,8 @@ TEST(private_kernel_tests, test_basic_contract_deployment)
     // Some private circuit proof (`constructor`, in this case)
     //***************************************************************************
 
-    // Set this to 0 and then fill it in with correct contract address
-    const NT::address new_contract_address = 0;
+    // contract address for newly deployed contract will be derived
+    // below after constructor VK is computed
     // const NT::fr new_contract_leaf_index = 1;
     const NT::fr new_portal_contract_address = 23456;
     const NT::fr contract_address_salt = 34567;
@@ -488,7 +491,7 @@ TEST(private_kernel_tests, test_basic_contract_deployment)
 
     CallContext<NT> call_context{
         .msg_sender = msg_sender,
-        .storage_contract_address = new_contract_address,
+        .storage_contract_address = 0, // will be replaced with contract address once it is calculated
         .portal_contract_address = 0,
         .is_delegate_call = false,
         .is_static_call = false,
@@ -498,6 +501,10 @@ TEST(private_kernel_tests, test_basic_contract_deployment)
     NT::fr arg0 = 5;
     NT::fr arg1 = 1;
     NT::fr arg2 = 999;
+    std::array<NT::fr, ARGS_LENGTH> args = { 0 };
+    args[0] = arg0;
+    args[1] = arg1;
+    args[2] = arg2;
 
     Composer dummy_constructor_composer = Composer("../barretenberg/cpp/srs_db/ignition");
     {
@@ -507,15 +514,15 @@ TEST(private_kernel_tests, test_basic_contract_deployment)
         // contract_deployment_data will need to contain the constructor_vk_hash... but the constructor's vk can only be
         // computed after the composer has composed the circuit!
         ContractDeploymentData<NT> dummy_contract_deployment_data{
-            .constructor_vk_hash = 0, // dummy
-            .function_tree_root = 0,  // TODO actually get this?
-            .contract_address_salt = contract_address_salt,
-            .portal_contract_address = new_portal_contract_address,
+            .constructor_vk_hash = 0, // zeros are okay for dummy call
+            .function_tree_root = 0,
+            .contract_address_salt = 0,
+            .portal_contract_address = 0,
         };
 
         DB dummy_db;
         NativeOracle dummy_constructor_oracle = NativeOracle(dummy_db,
-                                                             new_contract_address,
+                                                             0, // contract address not known during VK computation
                                                              function_data,
                                                              call_context,
                                                              dummy_contract_deployment_data,
@@ -539,12 +546,21 @@ TEST(private_kernel_tests, test_basic_contract_deployment)
     DB db;
 
     ContractDeploymentData<NT> contract_deployment_data{
-        .constructor_vk_hash = constructor_vk_hash, // TODO actually get this?
-        .function_tree_root = 0,                    // TODO actually get this?
+        .constructor_vk_hash = constructor_vk_hash,
+        .function_tree_root = 0, // TODO actually get this?
         .contract_address_salt = contract_address_salt,
         .portal_contract_address = new_portal_contract_address,
     };
 
+    // Get constructor hash for use when deriving contract address
+    auto constructor_hash = compute_constructor_hash<NT>(function_data, args, constructor_vk_hash);
+
+    // Derive contract address so that it can be used inside the constructor itself
+    const NT::address new_contract_address = compute_contract_address<NT>(
+        msg_sender, contract_address_salt, contract_deployment_data.function_tree_root, constructor_hash);
+    // update the contract address in the call context now that it is known
+    call_context.storage_contract_address = new_contract_address;
+
     NativeOracle constructor_oracle = NativeOracle(
         db, new_contract_address, function_data, call_context, contract_deployment_data, msg_sender_private_key);
     OracleWrapper constructor_oracle_wrapper = OracleWrapper(constructor_composer, constructor_oracle);
@@ -702,8 +718,8 @@ TEST(private_kernel_tests, test_native_basic_contract_deployment)
     // Some private circuit proof (`constructor`, in this case)
     //***************************************************************************
 
-    // Set this to 0 and then fill it in with correct contract address
-    const NT::address new_contract_address = 0;
+    // contract address for newly deployed contract will be derived
+    // below after constructor VK is computed
     // const NT::fr new_contract_leaf_index = 1;
     const NT::fr new_portal_contract_address = 23456;
     const NT::fr contract_address_salt = 34567;
@@ -727,7 +743,7 @@ TEST(private_kernel_tests, test_native_basic_contract_deployment)
 
     CallContext<NT> call_context{
         .msg_sender = msg_sender,
-        .storage_contract_address = new_contract_address,
+        .storage_contract_address = 0, // will be replaced with contract address once it is calculated
         .portal_contract_address = 0,
         .is_delegate_call = false,
         .is_static_call = false,
@@ -737,6 +753,10 @@ TEST(private_kernel_tests, test_native_basic_contract_deployment)
     NT::fr arg0 = 5;
     NT::fr arg1 = 1;
     NT::fr arg2 = 999;
+    std::array<NT::fr, ARGS_LENGTH> args = { 0 };
+    args[0] = arg0;
+    args[1] = arg1;
+    args[2] = arg2;
 
     Composer dummy_constructor_composer = Composer("../barretenberg/cpp/srs_db/ignition");
     {
@@ -746,15 +766,15 @@ TEST(private_kernel_tests, test_native_basic_contract_deployment)
         // contract_deployment_data will need to contain the constructor_vk_hash... but the constructor's vk can only be
         // computed after the composer has composed the circuit!
         ContractDeploymentData<NT> dummy_contract_deployment_data{
-            .constructor_vk_hash = 0, // dummy
-            .function_tree_root = 0,  // TODO actually get this?
-            .contract_address_salt = contract_address_salt,
-            .portal_contract_address = new_portal_contract_address,
+            .constructor_vk_hash = 0, // zeros are okay for dummy call
+            .function_tree_root = 0,
+            .contract_address_salt = 0,
+            .portal_contract_address = 0,
         };
 
         DB dummy_db;
         NativeOracle dummy_constructor_oracle = NativeOracle(dummy_db,
-                                                             new_contract_address,
+                                                             0, // contract address not known during VK computation
                                                              function_data,
                                                              call_context,
                                                              dummy_contract_deployment_data,
@@ -778,12 +798,22 @@ TEST(private_kernel_tests, test_native_basic_contract_deployment)
     DB db;
 
     ContractDeploymentData<NT> contract_deployment_data{
-        .constructor_vk_hash = constructor_vk_hash, // TODO actually get this?
-        .function_tree_root = 0,                    // TODO actually get this?
+        .constructor_vk_hash = constructor_vk_hash,
+        .function_tree_root = 0, // TODO actually get this?
         .contract_address_salt = contract_address_salt,
         .portal_contract_address = new_portal_contract_address,
     };
 
+    // Get constructor hash for use when deriving contract address
+    auto constructor_hash = compute_constructor_hash<NT>(function_data, args, constructor_vk_hash);
+
+    // Derive contract address so that it can be used inside the constructor itself
+    const NT::address new_contract_address = compute_contract_address<NT>(
+        msg_sender, contract_address_salt, contract_deployment_data.function_tree_root, constructor_hash);
+
+    // update the contract address in the call context now that it is known
+    call_context.storage_contract_address = new_contract_address;
+
     NativeOracle constructor_oracle = NativeOracle(
         db, new_contract_address, function_data, call_context, contract_deployment_data, msg_sender_private_key);
     OracleWrapper constructor_oracle_wrapper = OracleWrapper(constructor_composer, constructor_oracle);

cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit.cpp

@@ -83,16 +83,10 @@ void update_end_values(PrivateInputs<NT> const& private_inputs, PublicInputs<NT>
     const auto& contract_deployment_data =
         private_inputs.signed_tx_request.tx_request.tx_context.contract_deployment_data;
 
-    { // contracts
+    { // contract deployment
         // input storage contract address must be 0 if its a constructor call and non-zero otherwise
         auto is_contract_deployment = public_inputs.constants.tx_context.is_contract_deployment_tx;
 
-        if (is_contract_deployment) {
-            ASSERT(storage_contract_address == 0);
-        } else {
-            ASSERT(storage_contract_address != 0);
-        }
-
         auto private_call_vk_hash = stdlib::recursion::verification_key<CT::bn254>::compress_native(
             private_inputs.private_call.vk, GeneratorIndex::VK);
 
@@ -109,6 +103,14 @@ void update_end_values(PrivateInputs<NT> const& private_inputs, PublicInputs<NT>
                                                              contract_deployment_data.function_tree_root,
                                                              constructor_hash);
 
+        if (is_contract_deployment) {
+            // must imply == derived address
+            ASSERT(storage_contract_address == contract_address);
+        } else {
+            // non-contract deployments must specify contract address being interacted with
+            ASSERT(storage_contract_address != 0);
+        }
+
         // compute contract address nullifier
         auto blake_input = contract_address.to_field().to_buffer();
         auto contract_address_nullifier = NT::fr::serialize_from_buffer(NT::blake3s(blake_input).data());

cpp/src/aztec3/circuits/kernel/private/private_kernel_circuit.cpp

@@ -111,14 +111,9 @@ void update_end_values(PrivateInputs<CT> const& private_inputs, PublicInputs<CT>
     const auto& contract_deployment_data =
         private_inputs.signed_tx_request.tx_request.tx_context.contract_deployment_data;
 
-    { // contracts
+    { // contract deployment
         // input storage contract address must be 0 if its a constructor call and non-zero otherwise
         auto is_contract_deployment = public_inputs.constants.tx_context.is_contract_deployment_tx;
-        is_contract_deployment.must_imply(storage_contract_address == CT::fr(0),
-                                          "storage_contract_address is not zero for contract deployment");
-        (!is_contract_deployment)
-            .must_imply(storage_contract_address != CT::fr(0),
-                        "storage_contract_address is zero for a private function");
 
         auto private_call_vk_hash = private_inputs.private_call.vk->compress(GeneratorIndex::VK);
         auto constructor_hash = compute_constructor_hash<CT>(private_inputs.signed_tx_request.tx_request.function_data,
@@ -128,12 +123,22 @@ void update_end_values(PrivateInputs<CT> const& private_inputs, PublicInputs<CT>
         is_contract_deployment.must_imply(contract_deployment_data.constructor_vk_hash == private_call_vk_hash,
                                           "constructor_vk_hash does not match private call vk hash");
 
-        // compute the contract address
+        // compute the contract address (only valid if this is a contract deployment)
         auto contract_address = compute_contract_address<CT>(deployer_address,
                                                              contract_deployment_data.contract_address_salt,
                                                              contract_deployment_data.function_tree_root,
                                                              constructor_hash);
 
+        // must imply == derived address
+        is_contract_deployment.must_imply(
+            storage_contract_address == contract_address,
+            "storage_contract_address must match derived address for contract deployment");
+
+        // non-contract deployments must specify contract address being interacted with
+        (!is_contract_deployment)
+            .must_imply(storage_contract_address != CT::fr(0),
+                        "storage_contract_address must be nonzero for a private function");
+
         // compute contract address nullifier
         auto blake_input = CT::byte_array(contract_address.to_field());
         auto contract_address_nullifier = CT::fr(CT::blake3s(blake_input));