Merge branch 'master' into Jiajie/de-register-proposer

Author: jiajieey

.github/workflows/on-pull-request.yml

@@ -54,7 +54,7 @@ jobs:
 
   rollup-prover:
     name: Rollup prover integration Test
-    timeout-minutes: 420
+    timeout-minutes: 480
     needs: [build, forge, resync] # This test is long so start it after we're sure the others have passed
     runs-on: self-hosted
     env:

doc/nf_4.md

@@ -682,3 +682,14 @@ Withdraws the stake of a de-registered proposer, actually, the amount withdrawn
 ## Production deployment
 
 TBD once environment is confirmed
+
+## Appendix
+
+### Note on `VerifierKey.sol`
+
+There is a VerificationKey.sol contract in blockchain_assets/contracts i.e. the normal place for a Solidity contract. This contract's job is to put the verification key into memory, which it does via assembly code, directly placing the values into memory locations. This is to support the RollupVerifier contract, which also uses assembly code that expects to find a key in particular locations. VerificationKey.sol is only relevant for the full Rollup prover. Neither the Mock prover nor the unit tests reference it.
+VerificationKey.sol is never actually used directly. When the Deployer runs, it overwrites the contract file with a new one, which is similar in format, but contains the Rollup verification keys that have been computed during key generation. Note that this overwriting only happens when we are using a full rollup prover. This explains why VerificationKey.sol is not updated in Git, even though the keys change: the updated file is never seen by Git because we never push from a machine that is running a full rollup prover.
+The overwriting is an unusual approach. Normally one would have a static VerificationKey.sol which contains a function that lets us update the key on deployment. However, the current approach is taken because we want to control exactly how the key data is stored in memory so that we can benefit from the speed increase we get from assembly. Achieving this is much easier if all the key data is hard-coded. We therefore generate the code in Rust so that we can have hard-coded keys (in Solidity) that can be still changed (in Rust) at deployment if they need to be updated.
+The unit tests compute their own keys and use a test contract (test_contracts/TestVerifier.sol) which is kept separate from the 'production' contract. This seems sensible because we don't want the unit test to change the main contract as it's more a test of the key generation.
+Is there a better way? Probably not. It's the use of Assembly, which creates the requirement to store a large amount of key data in specific locations, that makes this a sensible approach in this case.
+We don't actually need VerificationKey.sol so it could be removed from the repository, in principle. However RollupVerifier.sol expects it to be there. Having a work around for that seems more trouble than keeping a 'placeholder' VerificationKey.sol.

docker-compose.yml

@@ -367,6 +367,10 @@ services:
     build:
       dockerfile: nightfall_deployer/Dockerfile
       context: .
+      ulimits:
+        nofile:
+          soft: 65536
+          hard: 65536
     platform: linux/amd64 # Required for building on M1 Macs
     # command: ["tail", "-f","/dev/null"] # For debugging if required
     networks:

nightfall_client/src/driven/db/mongo.rs

@@ -164,7 +164,8 @@ impl From<DBMembershipProof> for MembershipProof<Fr254> {
 pub struct CommitmentEntry {
     pub preimage: Preimage,
     pub status: CommitmentStatus,
-    #[serde(serialize_with = "ark_se_hex", deserialize_with = "ark_de_hex")]
+    
+    #[serde(rename = "_id",serialize_with = "ark_se_hex", deserialize_with = "ark_de_hex")]
     pub key: Fr254,
     #[serde(
         serialize_with = "ark_se_hex",
@@ -203,7 +204,8 @@ impl Commitment for CommitmentEntry {
     }
 }
 impl CommitmentEntryDB for CommitmentEntry {
-    fn new(preimage: Preimage, key: Fr254, nullifier: Fr254, status: CommitmentStatus) -> Self {
+    fn new(preimage: Preimage, nullifier: Fr254, status: CommitmentStatus) -> Self {
+        let key = preimage.hash().expect("failed to hash preimage");
         Self {
             preimage,
             status,
@@ -328,7 +330,7 @@ impl CommitmentDB<Fr254, CommitmentEntry> for Client {
     }
 
     async fn get_commitment(&self, k: &Fr254) -> Option<CommitmentEntry> {
-        let filter = doc! { "key": hex::encode(k.into_bigint().to_bytes_le()) };
+        let filter = doc! { "_id": hex::encode(k.into_bigint().to_bytes_le()) };
         self.database(DB)
             .collection::<CommitmentEntry>("commitments")
             .find_one(filter)
@@ -365,7 +367,7 @@ impl CommitmentDB<Fr254, CommitmentEntry> for Client {
             .into_iter()
             .map(|c| hex::encode(c.into_bigint().to_bytes_le()))
             .collect::<Vec<_>>();
-        let filter = doc! { "key": { "$in": commitment_str }};
+        let filter = doc! { "_id": { "$in": commitment_str }};
         let update = doc! {"$set": { "status": "PendingSpend" }};
         self.database(DB)
             .collection::<CommitmentEntry>("commitments")
@@ -380,7 +382,7 @@ impl CommitmentDB<Fr254, CommitmentEntry> for Client {
             .into_iter()
             .map(|c| hex::encode(c.into_bigint().to_bytes_le()))
             .collect::<Vec<_>>();
-        let filter = doc! { "key": { "$in": commitment_str }};
+        let filter = doc! { "_id": { "$in": commitment_str }};
         let update = doc! {"$set": { "status": "PendingCreation" }};
         self.database(DB)
             .collection::<CommitmentEntry>("commitments")
@@ -419,7 +421,7 @@ impl CommitmentDB<Fr254, CommitmentEntry> for Client {
             .collect::<Vec<_>>();
         let l1_hash = l1_hash.map(|h| h.encode_hex());
         let l2_blocknumber = l2_blocknumber.map(|b| b.encode_hex());
-        let filter = doc! { "key": { "$in": commitment_str }};
+        let filter = doc! { "_id": { "$in": commitment_str }};
         let update = doc! {"$set": { "status": "Unspent", "layer_1_transaction_hash": l1_hash, "layer_2_block_number": l2_blocknumber }};
         self.database(DB)
             .collection::<CommitmentEntry>("commitments")
@@ -431,7 +433,7 @@ impl CommitmentDB<Fr254, CommitmentEntry> for Client {
 
     // we compute a nullifier for each spend commitment that we process.
     async fn add_nullifier(&self, key: &Fr254, nullifier: Fr254) -> Option<()> {
-        let filter = doc! { "key": hex::encode(key.into_bigint().to_bytes_le())};
+        let filter = doc! { "_id": hex::encode(key.into_bigint().to_bytes_le())};
         let update =
             doc! {"$set": { "nullifier": hex::encode(nullifier.into_bigint().to_bytes_le()) }};
 
@@ -443,7 +445,10 @@ impl CommitmentDB<Fr254, CommitmentEntry> for Client {
         Some(())
     }
 
-    async fn store_commitment(&self, commitment: CommitmentEntry) -> Option<()> {
+    async fn store_commitment(
+        &self, 
+        commitment: CommitmentEntry
+    ) -> Option<()> {
         let result = self
             .database(DB)
             .collection::<CommitmentEntry>("commitments")
@@ -464,7 +469,7 @@ impl CommitmentDB<Fr254, CommitmentEntry> for Client {
         }
     }
 
-    /// function to store multiple commitments in the database, optionally ignoring duplicate key errors
+    /// function to store multiple commitments in the database, optionally ignoring duplicate _id errors
     async fn store_commitments(
         &self,
         commitments: &[CommitmentEntry],
@@ -480,13 +485,13 @@ impl CommitmentDB<Fr254, CommitmentEntry> for Client {
             .await;
         match res {
             Ok(_) => Some(()),
-            // unpack the Mongo error and check if it's a duplicate key error. If so, behave according to dup_key_check
+            // unpack the Mongo error and check if it's a duplicate _id error. If so, behave according to dup_key_check
             Err(e) => {
                 match e.kind.as_ref() {
                     ErrorKind::Write(WriteError(write_error)) => {
                         if write_error.code == 11000 && !dup_key_check {
-                            println!("Duplicate key error: {:?}", write_error);
-                            // duplicate key error but we don't care
+                            println!("Duplicate _id error: {:?}", write_error);
+                            // duplicate _id error but we don't care
                             Some(())
                         } else {
                             None

nightfall_client/src/driven/event_handlers/nightfall_event.rs

@@ -323,20 +323,12 @@ async fn process_propose_block_event<N: NightfallContract>(
             debug!("Commitment {} is not owned by us", commitment_hash);
         } else {
             info!("Received commitment owned by us, with hash {}", test_hash);
-            // We can transfer to ourselves, so we need to check if we already have the commitment in our database
-            let found_commitment = db.get_commitment(&commitment_hash).await;
-            if found_commitment.is_some() {
-                debug!("Commitment {} already in database", commitment_hash);
-                // If we already have the commitment, we don't need to do anything
-                continue;
-            }
             // store our newly received commitment in our commitment db
             let nullifier = test_preimage
                 .nullifier_hash(&nullifier_key)
                 .map_err(|_| EventHandlerError::HashError)?;
             let commitment_entry = CommitmentEntry::new(
                 test_preimage,
-                commitment_hash,
                 nullifier,
                 CommitmentStatus::Unspent,
             );

nightfall_client/src/drivers/rest/client_nf_3.rs

@@ -398,7 +398,6 @@ pub async fn handle_deposit<N: NightfallContract>(
     let commitment_hash = preimage_value.hash().expect("Could not hash commitment");
     let commitment_entry = CommitmentEntry::new(
         preimage_value,
-        commitment_hash,
         nullifier,
         CommitmentStatus::PendingCreation,
     );
@@ -432,7 +431,6 @@ pub async fn handle_deposit<N: NightfallContract>(
 
         let commitment_entry = CommitmentEntry::new(
             preimage_fee,
-            commitment_hash,
             nullifier,
             CommitmentStatus::PendingCreation,
         );

nightfall_client/src/drivers/rest/client_operation.rs

@@ -84,7 +84,6 @@ where
                     .expect("Nullifiers must be hashable");
                 let commitment_entry = CommitmentEntry::new(
                     commitment.get_preimage(),
-                    commitment_hash,
                     nullifier,
                     CommitmentStatus::PendingCreation,
                 );

nightfall_client/src/ports/db.rs

@@ -41,7 +41,7 @@ where
 }
 
 pub trait CommitmentEntryDB: Commitment {
-    fn new(preimage: Preimage, key: Fr254, nullifier: Fr254, status: CommitmentStatus) -> Self;
+    fn new(preimage: Preimage, nullifier: Fr254, status: CommitmentStatus) -> Self;
     fn get_status(&self) -> CommitmentStatus;
 }