feat(aztec-nr)!: Remove nr Point struct in favour of EmbeddedCurvePoint

barretenberg/cpp/src/barretenberg/vm2/common/standard_affine_point.hpp

@@ -92,9 +92,8 @@ template <typename AffinePoint> class StandardAffinePoint {
     }
 
   private:
-    // TODO(MW): Clarify - docs here no longer true
     // The affine point for operations, this will always match the raw coordinates unless the point is infinity.
-    // In that case, the point will be set to barretenberg's infinity representation - which is not (0,0).
+    // In that case, the point will be set to AffinePoint's infinity representation - which may not be (0,0).
     AffinePoint point;
     // These are the raw x and y coordinates, that are set when constructing the point. When an operation results
     // in infinity, these will be set to (0,0) to match noir's expected representation.

barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/ecc.cpp

@@ -138,10 +138,10 @@ void Ecc::add(MemoryInterface& memory,
                                  .result = result,
                                  .dst_address = dst_address });
     } catch (const InternalEccException& e) {
-        // Note this point is not on the curve, but corresponds
-        // to default values the circuit will assign.
-        // TODO(MW): This is now a point on the curve technically (inf) - check this doesnt cause issues now res.is_inf
-        // is true:
+        // Note this point is technically infinity, but we are treating it as 'empty' to corresponds
+        // to default values the circuit will assign. Since we have caught an InternalEccException,
+        // we have an error which the circuit should recognise and assign sel_should_exec == 0, so res will not be
+        // treated as inf.
         EmbeddedCurvePoint res = EmbeddedCurvePoint(0, 0);
         add_memory_events.emit({ .execution_clk = execution_clk,
                                  .space_id = space_id,

noir-projects/aztec-nr/aztec/src/keys/ecdh_shared_secret.nr

@@ -2,7 +2,7 @@ use crate::protocol::{
     address::aztec_address::AztecAddress,
     constants::{DOM_SEP__APP_SILOED_ECDH_SHARED_SECRET, DOM_SEP__ECDH_FIELD_MASK, DOM_SEP__ECDH_SUBKEY},
     hash::poseidon2_hash_with_separator,
-    point::Point,
+    point::EmbeddedCurvePoint,
     scalar::Scalar,
     traits::{FromField, ToField},
 };
@@ -18,16 +18,14 @@ use std::{embedded_curve_ops::multi_scalar_mul, ops::Neg};
 ///       Shared secret S = esk * Pk = sk * Epk
 ///
 /// See also: https://en.wikipedia.org/wiki/Elliptic-curve_Diffie%E2%80%93Hellman
-pub fn derive_ecdh_shared_secret(secret: Scalar, public_key: Point) -> Point {
-    // TODO(F-553): Drop the `.to_embedded()` / `.into()` round-trip once the custom `Point` wrapper is removed and we
-    // use `EmbeddedCurvePoint` directly.
-    multi_scalar_mul([public_key.to_embedded()], [secret]).into()
+pub fn derive_ecdh_shared_secret(secret: Scalar, public_key: EmbeddedCurvePoint) -> EmbeddedCurvePoint {
+    multi_scalar_mul([public_key], [secret])
 }
 
 /// Computes an app-siloed shared secret from a raw ECDH shared secret point and a contract address.
 ///
 /// `s_app = h(DOM_SEP__APP_SILOED_ECDH_SHARED_SECRET, S.x, S.y, contract_address)`
-pub fn compute_app_siloed_shared_secret(shared_secret: Point, contract_address: AztecAddress) -> Field {
+pub fn compute_app_siloed_shared_secret(shared_secret: EmbeddedCurvePoint, contract_address: AztecAddress) -> Field {
     poseidon2_hash_with_separator(
         [shared_secret.x, shared_secret.y, contract_address.to_field()],
         DOM_SEP__APP_SILOED_ECDH_SHARED_SECRET,
@@ -54,21 +52,19 @@ unconstrained fn test_consistency_with_typescript() {
         lo: 0x00000000000000000000000000000000649e7ca01d9de27b21624098b897babd,
         hi: 0x0000000000000000000000000000000023b3127c127b1f29a7adff5cccf8fb06,
     };
-    let point = Point {
+    let point = EmbeddedCurvePoint {
         x: 0x2688431c705a5ff3e6c6f2573c9e3ba1c1026d2251d0dbbf2d810aa53fd1d186,
         y: 0x1e96887b117afca01c00468264f4f80b5bb16d94c1808a448595f115556e5c8e,
-        is_infinite: false,
     };
 
     let shared_secret = derive_ecdh_shared_secret(secret, point);
 
     // This is just pasted from a test run. The original typescript code from which this could be generated seems to
     // have been deleted by someone, and soon the typescript code for encryption and decryption won't be needed, so
     // this will have to do.
-    let hard_coded_shared_secret = Point {
+    let hard_coded_shared_secret = EmbeddedCurvePoint {
         x: 0x15d55a5b3b2caa6a6207f313f05c5113deba5da9927d6421bcaa164822b911bc,
         y: 0x0974c3d0825031ae933243d653ebb1a0b08b90ee7f228f94c5c74739ea3c871e,
-        is_infinite: false,
     };
     assert_eq(shared_secret, hard_coded_shared_secret);
 }
@@ -78,8 +74,8 @@ unconstrained fn test_shared_secret_computation_in_both_directions() {
     let secret_a = Scalar { lo: 0x1234, hi: 0x2345 };
     let secret_b = Scalar { lo: 0x3456, hi: 0x4567 };
 
-    let pk_a: Point = std::embedded_curve_ops::fixed_base_scalar_mul(secret_a).into();
-    let pk_b: Point = std::embedded_curve_ops::fixed_base_scalar_mul(secret_b).into();
+    let pk_a: EmbeddedCurvePoint = std::embedded_curve_ops::fixed_base_scalar_mul(secret_a);
+    let pk_b: EmbeddedCurvePoint = std::embedded_curve_ops::fixed_base_scalar_mul(secret_b);
 
     let shared_secret = derive_ecdh_shared_secret(secret_a, pk_b);
     let shared_secret_alt = derive_ecdh_shared_secret(secret_b, pk_a);
@@ -92,8 +88,8 @@ unconstrained fn test_shared_secret_computation_from_address_in_both_directions(
     let secret_a = Scalar { lo: 0x1234, hi: 0x2345 };
     let secret_b = Scalar { lo: 0x3456, hi: 0x4567 };
 
-    let mut pk_a: Point = std::embedded_curve_ops::fixed_base_scalar_mul(secret_a).into();
-    let mut pk_b: Point = std::embedded_curve_ops::fixed_base_scalar_mul(secret_b).into();
+    let mut pk_a: EmbeddedCurvePoint = std::embedded_curve_ops::fixed_base_scalar_mul(secret_a);
+    let mut pk_b: EmbeddedCurvePoint = std::embedded_curve_ops::fixed_base_scalar_mul(secret_b);
 
     let address_b = AztecAddress::from_field(pk_b.x);
 
@@ -120,7 +116,7 @@ unconstrained fn test_shared_secret_computation_from_address_in_both_directions(
 #[test]
 unconstrained fn test_app_siloed_shared_secret_differs_per_contract() {
     let secret_a = Scalar { lo: 0x1234, hi: 0x2345 };
-    let pk_b: Point = std::embedded_curve_ops::fixed_base_scalar_mul(Scalar { lo: 0x3456, hi: 0x4567 }).into();
+    let pk_b: EmbeddedCurvePoint = std::embedded_curve_ops::fixed_base_scalar_mul(Scalar { lo: 0x3456, hi: 0x4567 });
 
     let shared_secret = derive_ecdh_shared_secret(secret_a, pk_b);
 

noir-projects/aztec-nr/aztec/src/keys/ephemeral.nr

@@ -1,11 +1,11 @@
 use std::embedded_curve_ops::{EmbeddedCurveScalar, fixed_base_scalar_mul};
 
-use crate::protocol::{point::Point, scalar::Scalar};
+use crate::protocol::{point::EmbeddedCurvePoint, scalar::Scalar};
 
 use crate::{oracle::random::random, utils::point::get_sign_of_point};
 
 /// Generates a random ephemeral key pair.
-pub fn generate_ephemeral_key_pair() -> (Scalar, Point) {
+pub fn generate_ephemeral_key_pair() -> (Scalar, EmbeddedCurvePoint) {
     // @todo Need to draw randomness from the full domain of Fq not only Fr
 
     // Safety: we use the randomness to preserve the privacy of both the sender and recipient via encryption, so a
@@ -17,9 +17,7 @@ pub fn generate_ephemeral_key_pair() -> (Scalar, Point) {
     // TODO(#12757): compute the key pair without constraining eph_sk twice (once in from_field, once in the black box
     // called by fixed_base_scalar_mul).
     let eph_sk = EmbeddedCurveScalar::from_field(randomness);
-    // TODO(F-553): Drop the `.into()` once the custom `Point` wrapper is removed and we use `EmbeddedCurvePoint`
-    // directly. Applies to the other `fixed_base_scalar_mul(...).into()` call sites in this file as well.
-    let eph_pk: Point = fixed_base_scalar_mul(eph_sk).into();
+    let eph_pk: EmbeddedCurvePoint = fixed_base_scalar_mul(eph_sk);
 
     (eph_sk, eph_pk)
 }
@@ -31,13 +29,13 @@ pub fn generate_ephemeral_key_pair() -> (Scalar, Point) {
 ///
 /// This is useful as it means it is possible to just broadcast the x-coordinate as a single `Field` and then
 /// reconstruct the original public key using [`crate::utils::point::point_from_x_coord_and_sign`] with `sign: true`.
-pub fn generate_positive_ephemeral_key_pair() -> (Scalar, Point) {
+pub fn generate_positive_ephemeral_key_pair() -> (Scalar, EmbeddedCurvePoint) {
     // Safety: we use the randomness to preserve the privacy of both the sender and recipient via encryption, so a
     // malicious sender could use non-random values to reveal the plaintext. But they already know it themselves
     // anyway, and so the recipient already trusts them to not disclose this information. We can therefore assume that
     // the sender will cooperate in the random value generation.
     let eph_sk = unsafe { generate_secret_key_for_positive_public_key() };
-    let eph_pk: Point = fixed_base_scalar_mul(eph_sk).into();
+    let eph_pk: EmbeddedCurvePoint = fixed_base_scalar_mul(eph_sk);
 
     assert(get_sign_of_point(eph_pk), "Got an ephemeral public key with a negative y coordinate");
 
@@ -53,7 +51,7 @@ unconstrained fn generate_secret_key_for_positive_public_key() -> EmbeddedCurveS
 
         // @todo Need to draw randomness from the full domain of Fq not only Fr
         sk = EmbeddedCurveScalar::from_field(random());
-        let pk: Point = fixed_base_scalar_mul(sk).into();
+        let pk: EmbeddedCurvePoint = fixed_base_scalar_mul(sk);
         if get_sign_of_point(pk) {
             break;
         }

noir-projects/aztec-nr/aztec/src/messages/encryption/poseidon2.nr

@@ -1,7 +1,7 @@
 use std::hash::poseidon2_permutation;
 use std::option::Option;
 
-use crate::protocol::point::Point;
+use crate::protocol::point::EmbeddedCurvePoint;
 
 global TWO_POW_128: Field = 0x100000000000000000000000000000000;
 
@@ -27,7 +27,7 @@ global TWO_POW_128: Field = 0x100000000000000000000000000000000;
 /// @param encryption_nonce is only needed if your use case needs to protect against replay attacks.
 pub fn poseidon2_encrypt<let L: u32>(
     msg: [Field; L],
-    shared_secret: Point,
+    shared_secret: EmbeddedCurvePoint,
     encryption_nonce: Field,
 ) -> [Field; ((L + 2) / 3) * 3 + 1] {
     // TODO: assert(encryption_nonce < 2^128), assert(L < 2^120);
@@ -78,7 +78,7 @@ pub fn poseidon2_encrypt<let L: u32>(
 
 pub fn poseidon2_decrypt<let L: u32>(
     ciphertext: [Field; ((L + 3 - 1) / 3) * 3 + 1],
-    shared_secret: Point,
+    shared_secret: EmbeddedCurvePoint,
     encryption_nonce: Field,
 ) -> Option<[Field; L]> {
     let mut s = [0, shared_secret.x, shared_secret.y, encryption_nonce + (L as Field) * TWO_POW_128];
@@ -139,7 +139,6 @@ pub fn poseidon2_decrypt<let L: u32>(
 }
 
 mod test {
-    use crate::protocol::point::Point;
     use super::{poseidon2_decrypt, poseidon2_encrypt, TWO_POW_128};
     use std::embedded_curve_ops::{EmbeddedCurveScalar, fixed_base_scalar_mul, multi_scalar_mul};
 
@@ -152,7 +151,7 @@ mod test {
 
         let eph_sk = 0x5678;
         let eph_pk = fixed_base_scalar_mul(EmbeddedCurveScalar::from_field(eph_sk));
-        let shared_secret: Point = multi_scalar_mul([bob_pk], [EmbeddedCurveScalar::from_field(eph_sk)]).into();
+        let shared_secret = multi_scalar_mul([bob_pk], [EmbeddedCurveScalar::from_field(eph_sk)]);
 
         let encryption_nonce = 3; // TODO. Can even be a timestamp. Why is this even needed?
 
@@ -162,7 +161,7 @@ mod test {
 
         // Bob sees: [Epk, ciphertext, encryption_nonce]:
 
-        let shared_secret: Point = multi_scalar_mul([eph_pk], [EmbeddedCurveScalar::from_field(bob_sk)]).into();
+        let shared_secret = multi_scalar_mul([eph_pk], [EmbeddedCurveScalar::from_field(bob_sk)]);
 
         let result = poseidon2_decrypt(ciphertext, shared_secret, encryption_nonce);
 
@@ -193,7 +192,7 @@ mod test {
 
         let eph_sk = 0x5678;
         let eph_pk = fixed_base_scalar_mul(EmbeddedCurveScalar::from_field(eph_sk));
-        let shared_secret: Point = multi_scalar_mul([bob_pk], [EmbeddedCurveScalar::from_field(eph_sk)]).into();
+        let shared_secret = multi_scalar_mul([bob_pk], [EmbeddedCurveScalar::from_field(eph_sk)]);
 
         let msg = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
 
@@ -205,7 +204,7 @@ mod test {
 
         // Bob sees: [Epk, ciphertext, encryption_nonce]:
 
-        let mut shared_secret: Point = multi_scalar_mul([eph_pk], [EmbeddedCurveScalar::from_field(bob_sk)]).into();
+        let mut shared_secret = multi_scalar_mul([eph_pk], [EmbeddedCurveScalar::from_field(bob_sk)]);
         // Let's intentionally corrupt the shared secret, so that decryption should fail
         shared_secret.x += 1;
 
@@ -222,7 +221,7 @@ mod test {
         let bob_pk = fixed_base_scalar_mul(EmbeddedCurveScalar::from_field(bob_sk));
 
         let eph_sk = 0x5678;
-        let shared_secret: Point = multi_scalar_mul([bob_pk], [EmbeddedCurveScalar::from_field(eph_sk)]).into();
+        let shared_secret = multi_scalar_mul([bob_pk], [EmbeddedCurveScalar::from_field(eph_sk)]);
 
         let encryption_nonce = 3; // TODO. Can even be a timestamp. Why is this even needed?
 

noir-projects/aztec-nr/aztec/src/oracle/keys.nr

@@ -1,4 +1,8 @@
-use crate::protocol::{address::{AztecAddress, PartialAddress}, point::Point, public_keys::{IvpkM, PublicKeys}};
+use crate::protocol::{
+    address::{AztecAddress, PartialAddress},
+    point::EmbeddedCurvePoint,
+    public_keys::{IvpkM, PublicKeys},
+};
 
 // TODO(F-498): review naming consistency
 pub unconstrained fn get_public_keys_and_partial_address(address: AztecAddress) -> (PublicKeys, PartialAddress) {
@@ -20,7 +24,7 @@ pub unconstrained fn try_get_public_keys_and_partial_address(
     get_public_keys_and_partial_address_oracle(address).map(|result: [Field; 6]| {
         let keys = PublicKeys {
             npk_m_hash: result[0],
-            ivpk_m: IvpkM { inner: Point { x: result[1], y: result[2], is_infinite: false } },
+            ivpk_m: IvpkM { inner: EmbeddedCurvePoint { x: result[1], y: result[2] } },
             ovpk_m_hash: result[3],
             tpk_m_hash: result[4],
         };

noir-projects/aztec-nr/aztec/src/oracle/shared_secret.nr

@@ -1,5 +1,5 @@
 use crate::ephemeral::EphemeralArray;
-use crate::protocol::{address::AztecAddress, hash::sha256_to_field, point::Point};
+use crate::protocol::{address::AztecAddress, hash::sha256_to_field, point::EmbeddedCurvePoint};
 
 global GET_SHARED_SECRETS_REQUEST_SLOT: Field = sha256_to_field("AZTEC_NR::GET_SHARED_SECRETS_REQUEST_SLOT".as_bytes());
 
@@ -11,7 +11,11 @@ unconstrained fn get_shared_secrets_oracle(
 ) -> Field {}
 
 /// Convenience wrapper around [`get_shared_secrets`] for a single ephemeral public key.
-pub unconstrained fn get_shared_secret(address: AztecAddress, eph_pk: Point, contract_address: AztecAddress) -> Field {
+pub unconstrained fn get_shared_secret(
+    address: AztecAddress,
+    eph_pk: EmbeddedCurvePoint,
+    contract_address: AztecAddress,
+) -> Field {
     get_shared_secrets::<1>(address, BoundedVec::from_array([eph_pk]), contract_address).get(0)
 }
 
@@ -36,10 +40,10 @@ pub unconstrained fn get_shared_secret(address: AztecAddress, eph_pk: Point, con
 /// [`derive_shared_secret_subkey`](crate::keys::ecdh_shared_secret::derive_shared_secret_subkey).
 pub unconstrained fn get_shared_secrets<let N: u32>(
     address: AztecAddress,
-    eph_pks: BoundedVec<Point, N>,
+    eph_pks: BoundedVec<EmbeddedCurvePoint, N>,
     contract_address: AztecAddress,
 ) -> BoundedVec<Field, N> {
-    let request_array: EphemeralArray<Point> = EphemeralArray::at(GET_SHARED_SECRETS_REQUEST_SLOT).clear();
+    let request_array: EphemeralArray<EmbeddedCurvePoint> = EphemeralArray::at(GET_SHARED_SECRETS_REQUEST_SLOT).clear();
     eph_pks.for_each(|pk| request_array.push(pk));
 
     let response_slot = get_shared_secrets_oracle(address, request_array.slot, contract_address);

noir-projects/aztec-nr/aztec/src/publish_contract_instance.nr

@@ -19,35 +19,34 @@ pub fn publish_contract_instance_for_public_execution(context: &mut PrivateConte
     }
 
     // Adapted from
-    // noir-contracts/contracts/protocol/contract_instance_registry_contract/src/interface/ContractInstanceRegistry.nr
+    // noir-contracts/contracts/protocol/contract_instance_registry_contract/src/interface/ContractInstanceRegistry.ts
     // That file
     // was autogenerated running the following command from noir-projects/noir-contracts:
-    // ../../yarn-project/node_modules/.bin/aztec-cli codegen
-    // target/contract_instance_registry_contract-ContractInstanceRegistry.json --nr -o
+    // ../../yarn-project/node_modules/.bin/aztec codegen
+    // target/contract_instance_registry_contract-ContractInstanceRegistry.json -o
     // ./contracts/contract_instance_registry_contract/src/interface
     //
-    // PublicKeys serializes to 6 fields (npk_m_hash, ivpk_m as a 3-field point with
-    // is_infinite, ovpk_m_hash, tpk_m_hash). Total args: 4 (salt, class_id, init_hash,
-    // immutables_hash) + 6 (public_keys) + 1 (universal_deploy) = 11.
-    let mut serialized_args = [0; 11];
+    // PublicKeys serializes to 5 fields (npk_m_hash, ivpk_m as a 2-field point, ovpk_m_hash, tpk_m_hash). Total args: 4
+    // (salt, class_id, init_hash, immutables_hash) + 5 (public_keys) + 1 (universal_deploy) = 10.
+    let mut serialized_args = [0; 10];
     serialized_args[0] = instance.salt;
     serialized_args[1] = instance.contract_class_id.to_field();
     serialized_args[2] = instance.initialization_hash;
     serialized_args[3] = instance.immutables_hash;
 
     let serialized_public_keys = instance.public_keys.serialize();
 
-    for i in 0..6 {
+    for i in 0..5 {
         serialized_args[i + 4] = serialized_public_keys[i];
     }
 
-    serialized_args[10] = universal_deploy as Field;
+    serialized_args[9] = universal_deploy as Field;
 
     let _call_result = context.call_private_function(
         CONTRACT_INSTANCE_REGISTRY_CONTRACT_ADDRESS,
         comptime {
             FunctionSelector::from_signature(
-                "publish_for_public_execution(Field,(Field),Field,Field,(Field,((Field,Field,bool)),Field,Field),bool)",
+                "publish_for_public_execution(Field,(Field),Field,Field,(Field,((Field,Field)),Field,Field),bool)",
             )
         },
         serialized_args,