From 2d4873cf84f33b899dc39887503f182fcb3cedfd Mon Sep 17 00:00:00 2001 From: David Banks <47112877+dbanks12@users.noreply.github.com> Date: Wed, 5 Apr 2023 17:21:25 -0400 Subject: [PATCH] valid contract address should be known in constructor and be a private input to kernel even for contract deployment (#179) --- .../basic_contract_deployment.cpp | 2 +- .../apps/test_apps/escrow/deposit.cpp | 2 +- .../apps/test_apps/escrow/transfer.cpp | 2 +- .../apps/test_apps/escrow/withdraw.cpp | 2 +- .../function_2_1.cpp | 2 +- .../aztec3/circuits/kernel/private/.test.cpp | 70 +++++++++++++------ .../private/native_private_kernel_circuit.cpp | 16 +++-- .../kernel/private/private_kernel_circuit.cpp | 19 +++-- 8 files changed, 76 insertions(+), 39 deletions(-) diff --git a/cpp/src/aztec3/circuits/apps/test_apps/basic_contract_deployment/basic_contract_deployment.cpp b/cpp/src/aztec3/circuits/apps/test_apps/basic_contract_deployment/basic_contract_deployment.cpp index 64773498..76844774 100644 --- a/cpp/src/aztec3/circuits/apps/test_apps/basic_contract_deployment/basic_contract_deployment.cpp +++ b/cpp/src/aztec3/circuits/apps/test_apps/basic_contract_deployment/basic_contract_deployment.cpp @@ -50,7 +50,7 @@ OptionalPrivateCircuitPublicInputs constructor(FunctionExecutionContext& exe exec_ctx.finalise(); - info("public inputs: ", public_inputs); + // info("public inputs: ", public_inputs); return public_inputs.to_native_type(); } diff --git a/cpp/src/aztec3/circuits/apps/test_apps/escrow/deposit.cpp b/cpp/src/aztec3/circuits/apps/test_apps/escrow/deposit.cpp index 3506aeec..c2ed7322 100644 --- a/cpp/src/aztec3/circuits/apps/test_apps/escrow/deposit.cpp +++ b/cpp/src/aztec3/circuits/apps/test_apps/escrow/deposit.cpp @@ -62,7 +62,7 @@ OptionalPrivateCircuitPublicInputs deposit(FunctionExecutionContext& exec_ct exec_ctx.finalise(); - info("public inputs: ", public_inputs); + // info("public inputs: ", public_inputs); return public_inputs.to_native_type(); // TODO: also return note preimages and nullifier preimages. diff --git a/cpp/src/aztec3/circuits/apps/test_apps/escrow/transfer.cpp b/cpp/src/aztec3/circuits/apps/test_apps/escrow/transfer.cpp index d81b6596..7d99a7bc 100644 --- a/cpp/src/aztec3/circuits/apps/test_apps/escrow/transfer.cpp +++ b/cpp/src/aztec3/circuits/apps/test_apps/escrow/transfer.cpp @@ -105,7 +105,7 @@ OptionalPrivateCircuitPublicInputs transfer(FunctionExecutionContext& exec_c exec_ctx.finalise(); - info("public inputs: ", public_inputs); + // info("public inputs: ", public_inputs); return public_inputs.to_native_type(); }; diff --git a/cpp/src/aztec3/circuits/apps/test_apps/escrow/withdraw.cpp b/cpp/src/aztec3/circuits/apps/test_apps/escrow/withdraw.cpp index d6d9b401..320ff01d 100644 --- a/cpp/src/aztec3/circuits/apps/test_apps/escrow/withdraw.cpp +++ b/cpp/src/aztec3/circuits/apps/test_apps/escrow/withdraw.cpp @@ -97,7 +97,7 @@ OptionalPrivateCircuitPublicInputs 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(); }; diff --git a/cpp/src/aztec3/circuits/apps/test_apps/private_to_private_function_call/function_2_1.cpp b/cpp/src/aztec3/circuits/apps/test_apps/private_to_private_function_call/function_2_1.cpp index 3d4c88ed..a0069955 100644 --- a/cpp/src/aztec3/circuits/apps/test_apps/private_to_private_function_call/function_2_1.cpp +++ b/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 + #include #include @@ -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 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 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 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 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(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( + 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 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 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 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 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(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( + 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); diff --git a/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit.cpp b/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit.cpp index 5ea41dd4..553ac312 100644 --- a/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit.cpp +++ b/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit.cpp @@ -83,16 +83,10 @@ void update_end_values(PrivateInputs const& private_inputs, PublicInputs 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::compress_native( private_inputs.private_call.vk, GeneratorIndex::VK); @@ -109,6 +103,14 @@ void update_end_values(PrivateInputs const& private_inputs, PublicInputs 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()); diff --git a/cpp/src/aztec3/circuits/kernel/private/private_kernel_circuit.cpp b/cpp/src/aztec3/circuits/kernel/private/private_kernel_circuit.cpp index 3d851736..e17c4dbb 100644 --- a/cpp/src/aztec3/circuits/kernel/private/private_kernel_circuit.cpp +++ b/cpp/src/aztec3/circuits/kernel/private/private_kernel_circuit.cpp @@ -111,14 +111,9 @@ void update_end_values(PrivateInputs const& private_inputs, PublicInputs 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(private_inputs.signed_tx_request.tx_request.function_data, @@ -128,12 +123,22 @@ void update_end_values(PrivateInputs const& private_inputs, PublicInputs 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(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));