Merge pull request #3632 from anoma/grarco/check-masp-action

Check masp action

.changelog/unreleased/improvements/3632-check-masp-action.md

@@ -0,0 +1,2 @@
+- Masp vp and protocol now ensure that a transaction can push at most one MASP
+  action. ([\#3632](https://github.com/anoma/namada/pull/3632))

crates/governance/src/vp/mod.rs

@@ -1366,7 +1366,7 @@ mod test {
             .write_log_mut()
             .write(&balance_key, amount.serialize_to_vec())
             .expect("write failed");
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
     }
 
     #[cfg(test)]
@@ -1612,7 +1612,7 @@ mod test {
             Ok(_)
         );
 
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().unwrap();
 
         let governance_balance_key = balance_key(&nam(), &ADDRESS);
@@ -2330,7 +2330,7 @@ mod test {
             Ok(_)
         );
 
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().unwrap();
 
         let height = state.in_mem().get_block_height().0 + (7 * 2);
@@ -2459,7 +2459,7 @@ mod test {
             Ok(_)
         );
 
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().unwrap();
 
         let height = state.in_mem().get_block_height().0 + (7 * 2);
@@ -2588,7 +2588,7 @@ mod test {
             Ok(_)
         );
 
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().unwrap();
 
         let height = state.in_mem().get_block_height().0 + (7 * 2);
@@ -2717,7 +2717,7 @@ mod test {
             Ok(_)
         );
 
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().unwrap();
 
         let height = state.in_mem().get_block_height().0 + (9 * 2);
@@ -2863,7 +2863,7 @@ mod test {
             Ok(_)
         );
 
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().unwrap();
 
         let height = state.in_mem().get_block_height().0 + (10 * 2);
@@ -3009,7 +3009,7 @@ mod test {
             Ok(_)
         );
 
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().unwrap();
 
         let height = state.in_mem().get_block_height().0 + (10 * 2);

crates/ibc/src/vp/mod.rs

@@ -724,7 +724,7 @@ mod tests {
             .write_log_mut()
             .write(&client_update_height_key, host_height.encode_vec())
             .expect("write failed");
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
     }
 
     fn get_connection_id() -> ConnectionId {
@@ -1148,7 +1148,7 @@ mod tests {
         let mut keys_changed = BTreeSet::new();
         let mut state = init_storage();
         insert_init_client(&mut state);
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().expect("commit failed");
 
         // for next block
@@ -1272,7 +1272,7 @@ mod tests {
         let mut keys_changed = BTreeSet::new();
         let mut state = init_storage();
         insert_init_client(&mut state);
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().expect("commit failed");
         // for next block
         state
@@ -1475,7 +1475,7 @@ mod tests {
         let mut keys_changed = BTreeSet::new();
         let mut state = init_storage();
         insert_init_client(&mut state);
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().expect("commit failed");
         // for next block
         state
@@ -1604,7 +1604,7 @@ mod tests {
             .write_log_mut()
             .write(&conn_key, bytes)
             .expect("write failed");
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().expect("commit failed");
         // for next block
         state
@@ -1713,7 +1713,7 @@ mod tests {
             .write_log_mut()
             .write(&conn_key, bytes)
             .expect("write failed");
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().expect("commit failed");
         // for next block
         state
@@ -1809,7 +1809,7 @@ mod tests {
             .write_log_mut()
             .write(&conn_key, bytes)
             .expect("write failed");
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().expect("commit failed");
         // for next block
         state
@@ -1931,7 +1931,7 @@ mod tests {
             .write_log_mut()
             .write(&conn_key, bytes)
             .expect("write failed");
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().expect("commit failed");
         // for next block
         state
@@ -2061,7 +2061,7 @@ mod tests {
             .write_log_mut()
             .write(&channel_key, bytes)
             .expect("write failed");
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().expect("commit failed");
         // for next block
         state
@@ -2168,7 +2168,7 @@ mod tests {
             .write_log_mut()
             .write(&channel_key, bytes)
             .expect("write failed");
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().expect("commit failed");
         // for next block
         state
@@ -2281,7 +2281,7 @@ mod tests {
             .write_log_mut()
             .write(&balance_key, amount.serialize_to_vec())
             .expect("write failed");
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().expect("commit failed");
         // for next block
         state
@@ -2426,7 +2426,7 @@ mod tests {
             .write_log_mut()
             .write(&channel_key, bytes)
             .expect("write failed");
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().expect("commit failed");
         // for next block
         state
@@ -2671,7 +2671,7 @@ mod tests {
             .write_log_mut()
             .write(&commitment_key, bytes)
             .expect("write failed");
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().expect("commit failed");
         // for next block
         state
@@ -2834,7 +2834,7 @@ mod tests {
             .write_log_mut()
             .write(&commitment_key, bytes)
             .expect("write failed");
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().expect("commit failed");
         // for next block
         state
@@ -2991,7 +2991,7 @@ mod tests {
             .write_log_mut()
             .write(&commitment_key, bytes)
             .expect("write failed");
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().expect("commit failed");
         // for next block
         state
@@ -3133,7 +3133,7 @@ mod tests {
             .write(&metadata_key, metadata.serialize_to_vec())
             .expect("write failed");
 
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().expect("commit failed");
         // for next block
         state
@@ -3285,7 +3285,7 @@ mod tests {
             .write_log_mut()
             .write(&channel_key, bytes)
             .expect("write failed");
-        state.write_log_mut().commit_batch();
+        state.write_log_mut().commit_batch_and_current_tx();
         state.commit_block().expect("commit failed");
         // for next block
         state

crates/node/src/protocol.rs

@@ -405,7 +405,13 @@ where
                     let actions =
                         state.read_actions().map_err(Error::StateError)?;
                     if let Some(masp_section_ref) =
-                        action::get_masp_section_ref(&actions)
+                        action::get_masp_section_ref(&actions).map_err(
+                            |msg| {
+                                Error::StateError(state::Error::Temporary {
+                                    error: msg.to_string(),
+                                })
+                            },
+                        )?
                     {
                         extended_tx_result
                             .masp_tx_refs
@@ -488,7 +494,10 @@ where
     // Commit tx to the block write log even in case of subsequent errors (if
     // the fee payment failed instead, than the previous two functions must
     // have propagated an error)
-    shell_params.state.write_log_mut().commit_batch();
+    shell_params
+        .state
+        .write_log_mut()
+        .commit_batch_and_current_tx();
 
     let (batch_results, masp_section_refs) = payment_result.map_or_else(
         || (TxResult::default(), None),
@@ -753,9 +762,14 @@ where
                 if is_masp_transfer(&result.changed_keys)
                     && result.is_accepted()
                 {
-                    if let Some(masp_tx_id) =
-                        action::get_masp_section_ref(&actions)
-                    {
+                    if let Some(masp_tx_id) = action::get_masp_section_ref(
+                        &actions,
+                    )
+                    .map_err(|msg| {
+                        Error::StateError(state::Error::Temporary {
+                            error: msg.to_string(),
+                        })
+                    })? {
                         Some(MaspTxResult {
                             tx_result: result,
                             masp_section_ref: Either::Left(masp_tx_id),

crates/node/src/shell/finalize_block.rs

@@ -349,7 +349,7 @@ where
         match extended_dispatch_result {
             Ok(extended_tx_result) => match tx_data.tx.header.tx_type {
                 TxType::Wrapper(_) => {
-                    self.state.write_log_mut().commit_batch();
+                    self.state.write_log_mut().commit_batch_and_current_tx();
 
                     // Return withouth emitting any events
                     return Some(WrapperCache {
@@ -392,7 +392,7 @@ where
                     .extend(Code(ResultCode::InvalidTx));
                 // Drop the batch write log which could contain invalid data.
                 // Important data that could be valid (e.g. a valid fee payment)
-                // must have already been moved to the bloc kwrite log by now
+                // must have already been moved to the block write log by now
                 self.state.write_log_mut().drop_batch();
             }
             Err(dispatch_error) => {
@@ -476,7 +476,7 @@ where
             self.state.write_log_mut().drop_batch();
             tx_logs.tx_event.extend(Code(ResultCode::WasmRuntimeError));
         } else {
-            self.state.write_log_mut().commit_batch();
+            self.state.write_log_mut().commit_batch_and_current_tx();
             self.state
                 .in_mem_mut()
                 .block
@@ -550,8 +550,12 @@ where
                 .results
                 .accept(tx_data.tx_index);
             temp_log.commit(tx_logs, response);
-            // Commit the successful inner transactions before the error
-            self.state.write_log_mut().commit_batch();
+            // Commit the successful inner transactions before the error. Drop
+            // the current tx write log which might be still populated with data
+            // to be discarded (this is the case when we propagate an error
+            // from the function that runs the actual batch)
+            self.state.write_log_mut().drop_tx();
+            self.state.write_log_mut().commit_batch_only();
         }
 
         if commit_batch_hash {

crates/node/src/shell/prepare_proposal.rs

@@ -142,7 +142,7 @@ where
                 );
                 match result {
                     Ok(gas) => {
-                        temp_state.write_log_mut().commit_batch();
+                        temp_state.write_log_mut().commit_batch_and_current_tx();
                         Some((tx_bytes.to_owned(), gas))
                     },
                     Err(()) => {

crates/node/src/shell/process_proposal.rs

@@ -153,7 +153,7 @@ where
                 );
                 let error_code = ResultCode::from_u32(result.code).unwrap();
                 if let ResultCode::Ok = error_code {
-                    temp_state.write_log_mut().commit_batch();
+                    temp_state.write_log_mut().commit_batch_and_current_tx();
                 } else {
                     tracing::info!(
                         "Process proposal rejected an invalid tx. Error code: \

crates/shielded_token/src/vp.rs

@@ -416,14 +416,14 @@ where
             tx
         } else {
             // Get the Transaction object from the actions
-            let masp_section_ref = namada_tx::action::get_masp_section_ref(
-                &actions,
-            )
-            .ok_or_else(|| {
-                native_vp::Error::new_const(
-                    "Missing MASP section reference in action",
-                )
-            })?;
+            let masp_section_ref =
+                namada_tx::action::get_masp_section_ref(&actions)
+                    .map_err(native_vp::Error::new_const)?
+                    .ok_or_else(|| {
+                        native_vp::Error::new_const(
+                            "Missing MASP section reference in action",
+                        )
+                    })?;
             batched_tx
                 .tx
                 .get_masp_section(&masp_section_ref)

crates/state/src/wl_state.rs

@@ -694,7 +694,7 @@ where
     /// Commit the current transaction's write log and the entire batch to the
     /// block. Starts a new transaction and batch write log.
     pub fn commit_tx_batch(&mut self) {
-        self.write_log.commit_batch()
+        self.write_log.commit_batch_and_current_tx()
     }
 
     /// Drop the current transaction's write log when it's declined by any of