Outdated Ethereum event nonce fixes

.changelog/unreleased/bug-fixes/2035-outdated-eth-nonces.md

@@ -0,0 +1,2 @@
+- Fix Ethereum event validation/state updates when more than one validator is
+  running the chain ([\#2035](https://github.com/anoma/namada/pull/2035))

apps/src/lib/node/ledger/shell/finalize_block.rs

@@ -1760,7 +1760,7 @@ mod test_finalize_block {
                 transfer
             };
             let ethereum_event = EthereumEvent::TransfersToEthereum {
-                nonce: 0u64.into(),
+                nonce: 1u64.into(),
                 transfers: vec![transfer],
                 relayer: bertha,
             };

apps/src/lib/node/ledger/shell/mod.rs

@@ -256,18 +256,32 @@ impl EthereumReceiver {
         }
     }
 
-    /// Pull messages from the channel and add to queue
-    /// Since vote extensions require ordering of ethereum
-    /// events, we do that here. We also de-duplicate events
-    pub fn fill_queue(&mut self) {
+    /// Pull Ethereum events from the oracle and queue them to
+    /// be voted on.
+    ///
+    /// Since vote extensions require ordering of Ethereum
+    /// events, we do that here. We also de-duplicate events.
+    /// Events may be filtered out of the queue with a provided
+    /// predicate.
+    pub fn fill_queue<F>(&mut self, mut keep_event: F)
+    where
+        F: FnMut(&EthereumEvent) -> bool,
+    {
         let mut new_events = 0;
+        let mut filtered_events = 0;
         while let Ok(eth_event) = self.channel.try_recv() {
-            if self.queue.insert(eth_event) {
+            if keep_event(&eth_event) && self.queue.insert(eth_event) {
                 new_events += 1;
-            };
+            } else {
+                filtered_events += 1;
+            }
         }
-        if new_events > 0 {
-            tracing::info!(n = new_events, "received Ethereum events");
+        if new_events + filtered_events > 0 {
+            tracing::info!(
+                new_events,
+                filtered_events,
+                "received Ethereum events"
+            );
         }
     }
 
@@ -2187,6 +2201,75 @@ mod abciplus_mempool_tests {
         );
     }
 
+    /// Test that Ethereum events with outdated nonces are
+    /// not validated by `CheckTx`.
+    #[test]
+    fn test_outdated_nonce_mempool_validate() {
+        use namada::types::storage::InnerEthEventsQueue;
+
+        const LAST_HEIGHT: BlockHeight = BlockHeight(3);
+
+        let (mut shell, _recv, _, _) = test_utils::setup_at_height(LAST_HEIGHT);
+        shell
+            .wl_storage
+            .storage
+            .eth_events_queue
+            // sent transfers to namada nonce to 5
+            .transfers_to_namada = InnerEthEventsQueue::new_at(5.into());
+
+        let (protocol_key, _, _) = wallet::defaults::validator_keys();
+
+        // only bad events
+        {
+            let ethereum_event = EthereumEvent::TransfersToNamada {
+                nonce: 3u64.into(),
+                transfers: vec![],
+            };
+            let ext = {
+                let ext = ethereum_events::Vext {
+                    validator_addr: wallet::defaults::validator_address(),
+                    block_height: LAST_HEIGHT,
+                    ethereum_events: vec![ethereum_event],
+                }
+                .sign(&protocol_key);
+                assert!(ext.verify(&protocol_key.ref_to()).is_ok());
+                ext
+            };
+            let tx = EthereumTxData::EthEventsVext(ext)
+                .sign(&protocol_key, shell.chain_id.clone())
+                .to_bytes();
+            let rsp = shell.mempool_validate(&tx, Default::default());
+            assert!(rsp.code != 0, "Validation should have failed");
+        }
+
+        // at least one good event
+        {
+            let e1 = EthereumEvent::TransfersToNamada {
+                nonce: 3u64.into(),
+                transfers: vec![],
+            };
+            let e2 = EthereumEvent::TransfersToNamada {
+                nonce: 5u64.into(),
+                transfers: vec![],
+            };
+            let ext = {
+                let ext = ethereum_events::Vext {
+                    validator_addr: wallet::defaults::validator_address(),
+                    block_height: LAST_HEIGHT,
+                    ethereum_events: vec![e1, e2],
+                }
+                .sign(&protocol_key);
+                assert!(ext.verify(&protocol_key.ref_to()).is_ok());
+                ext
+            };
+            let tx = EthereumTxData::EthEventsVext(ext)
+                .sign(&protocol_key, shell.chain_id.clone())
+                .to_bytes();
+            let rsp = shell.mempool_validate(&tx, Default::default());
+            assert!(rsp.code == 0, "Validation should have passed");
+        }
+    }
+
     /// Test that we do not include protocol txs in the mempool,
     /// voting on ethereum events or signing bridge pool roots
     /// and nonces if the bridge is inactive.

apps/src/lib/node/ledger/shell/prepare_proposal.rs

@@ -514,10 +514,12 @@ mod test_prepare_proposal {
     #[cfg(feature = "abcipp")]
     use namada::types::key::common;
     use namada::types::key::RefTo;
-    use namada::types::storage::BlockHeight;
+    use namada::types::storage::{BlockHeight, InnerEthEventsQueue};
     use namada::types::token;
     use namada::types::token::Amount;
-    use namada::types::transaction::protocol::EthereumTxData;
+    use namada::types::transaction::protocol::{
+        ethereum_tx_data_variants, EthereumTxData,
+    };
     use namada::types::transaction::{Fee, TxType, WrapperTx};
     #[cfg(feature = "abcipp")]
     use namada::types::vote_extensions::bridge_pool_roots;
@@ -1652,4 +1654,115 @@ mod test_prepare_proposal {
         eprintln!("Proposal: {:?}", result.txs);
         assert!(result.txs.is_empty());
     }
+
+    /// Test that Ethereum events with outdated nonces are
+    /// not proposed during `PrepareProposal`.
+    #[test]
+    fn test_outdated_nonce_proposal() {
+        const LAST_HEIGHT: BlockHeight = BlockHeight(3);
+
+        let (mut shell, _recv, _, _) = test_utils::setup_at_height(LAST_HEIGHT);
+        shell
+            .wl_storage
+            .storage
+            .eth_events_queue
+            // sent transfers to namada nonce to 5
+            .transfers_to_namada = InnerEthEventsQueue::new_at(5.into());
+
+        let (protocol_key, _, _) = wallet::defaults::validator_keys();
+        let validator_addr = wallet::defaults::validator_address();
+
+        // test an extension containing solely events with
+        // bad nonces
+        {
+            let ethereum_event = EthereumEvent::TransfersToNamada {
+                // outdated nonce (3 < 5)
+                nonce: 3u64.into(),
+                transfers: vec![],
+            };
+            let ext = {
+                let ext = ethereum_events::Vext {
+                    validator_addr: validator_addr.clone(),
+                    block_height: LAST_HEIGHT,
+                    ethereum_events: vec![ethereum_event],
+                }
+                .sign(&protocol_key);
+                assert!(ext.verify(&protocol_key.ref_to()).is_ok());
+                ext
+            };
+            let tx = EthereumTxData::EthEventsVext(ext)
+                .sign(&protocol_key, shell.chain_id.clone())
+                .to_bytes();
+            let req = RequestPrepareProposal {
+                txs: vec![tx],
+                ..Default::default()
+            };
+            let proposed_txs =
+                shell.prepare_proposal(req).txs.into_iter().map(|tx_bytes| {
+                    Tx::try_from(tx_bytes.as_slice()).expect("Test failed")
+                });
+            // since no events with valid nonces are contained in the vote
+            // extension, we drop it from the proposal
+            for tx in proposed_txs {
+                if ethereum_tx_data_variants::EthEventsVext::try_from(&tx)
+                    .is_ok()
+                {
+                    panic!(
+                        "No ethereum events should have been found in the \
+                         proposal"
+                    );
+                }
+            }
+        }
+
+        // test an extension containing at least one event
+        // with a good nonce
+        {
+            let event1 = EthereumEvent::TransfersToNamada {
+                // outdated nonce (3 < 5)
+                nonce: 3u64.into(),
+                transfers: vec![],
+            };
+            let event2 = EthereumEvent::TransfersToNamada {
+                // outdated nonce (10 >= 5)
+                nonce: 10u64.into(),
+                transfers: vec![],
+            };
+            let ext = {
+                let ext = ethereum_events::Vext {
+                    validator_addr,
+                    block_height: LAST_HEIGHT,
+                    ethereum_events: vec![event1, event2.clone()],
+                }
+                .sign(&protocol_key);
+                assert!(ext.verify(&protocol_key.ref_to()).is_ok());
+                ext
+            };
+            let tx = EthereumTxData::EthEventsVext(ext)
+                .sign(&protocol_key, shell.chain_id.clone())
+                .to_bytes();
+            let req = RequestPrepareProposal {
+                txs: vec![tx],
+                ..Default::default()
+            };
+            let proposed_txs =
+                shell.prepare_proposal(req).txs.into_iter().map(|tx_bytes| {
+                    Tx::try_from(tx_bytes.as_slice()).expect("Test failed")
+                });
+            // find the event with the good nonce
+            let mut ext = 'ext: {
+                for tx in proposed_txs {
+                    if let Ok(ext) =
+                        ethereum_tx_data_variants::EthEventsVext::try_from(&tx)
+                    {
+                        break 'ext ext;
+                    }
+                }
+                panic!("No ethereum events found in proposal");
+            };
+            assert_eq!(ext.data.ethereum_events.len(), 2);
+            let found_event = ext.data.ethereum_events.remove(1);
+            assert_eq!(found_event, event2);
+        }
+    }
 }

apps/src/lib/node/ledger/shell/process_proposal.rs

@@ -2912,4 +2912,76 @@ mod test_process_proposal {
             );
         }
     }
+
+    /// Test that Ethereum events with outdated nonces are
+    /// not validated by `ProcessProposal`.
+    #[test]
+    fn test_outdated_nonce_process_proposal() {
+        use namada::types::storage::InnerEthEventsQueue;
+
+        const LAST_HEIGHT: BlockHeight = BlockHeight(3);
+
+        let (mut shell, _recv, _, _) = test_utils::setup_at_height(LAST_HEIGHT);
+        shell
+            .wl_storage
+            .storage
+            .eth_events_queue
+            // sent transfers to namada nonce to 5
+            .transfers_to_namada = InnerEthEventsQueue::new_at(5.into());
+
+        let (protocol_key, _, _) = wallet::defaults::validator_keys();
+
+        // only bad events
+        {
+            let ethereum_event = EthereumEvent::TransfersToNamada {
+                // outdated nonce (3 < 5)
+                nonce: 3u64.into(),
+                transfers: vec![],
+            };
+            let ext = {
+                let ext = ethereum_events::Vext {
+                    validator_addr: wallet::defaults::validator_address(),
+                    block_height: LAST_HEIGHT,
+                    ethereum_events: vec![ethereum_event],
+                }
+                .sign(&protocol_key);
+                assert!(ext.verify(&protocol_key.ref_to()).is_ok());
+                ext
+            };
+            let tx = EthereumTxData::EthEventsVext(ext)
+                .sign(&protocol_key, shell.chain_id.clone())
+                .to_bytes();
+            let req = ProcessProposal { txs: vec![tx] };
+            let rsp = shell.process_proposal(req);
+            assert!(rsp.is_err());
+        }
+
+        // at least one good event
+        {
+            let e1 = EthereumEvent::TransfersToNamada {
+                nonce: 3u64.into(),
+                transfers: vec![],
+            };
+            let e2 = EthereumEvent::TransfersToNamada {
+                nonce: 5u64.into(),
+                transfers: vec![],
+            };
+            let ext = {
+                let ext = ethereum_events::Vext {
+                    validator_addr: wallet::defaults::validator_address(),
+                    block_height: LAST_HEIGHT,
+                    ethereum_events: vec![e1, e2],
+                }
+                .sign(&protocol_key);
+                assert!(ext.verify(&protocol_key.ref_to()).is_ok());
+                ext
+            };
+            let tx = EthereumTxData::EthEventsVext(ext)
+                .sign(&protocol_key, shell.chain_id.clone())
+                .to_bytes();
+            let req = ProcessProposal { txs: vec![tx] };
+            let rsp = shell.process_proposal(req);
+            assert!(rsp.is_ok());
+        }
+    }
 }

apps/src/lib/node/ledger/shell/vote_extensions.rs

@@ -35,10 +35,8 @@ pub enum VoteExtensionError {
     ValsetUpdProofAvailable,
     #[error("The length of the transfers and their validity map differ")]
     TransfersLenMismatch,
-    #[error("The bridge pool nonce in the vote extension is invalid")]
-    InvalidBpNonce,
-    #[error("The transfer to Namada nonce in the vote extension is invalid")]
-    InvalidNamNonce,
+    #[error("The nonce in the Ethereum event is invalid")]
+    InvalidEthEventNonce,
     #[error("The vote extension was issued for an unexpected block height")]
     UnexpectedBlockHeight,
     #[error("The vote extension was issued for an unexpected epoch")]
@@ -428,8 +426,20 @@ where
                 }
             };
             match (&tx).try_into().ok()? {
-                EthereumTxData::EthEventsVext(_)
-                | EthereumTxData::BridgePoolVext(_) => Some(tx_bytes.clone()),
+                EthereumTxData::BridgePoolVext(_) => Some(tx_bytes.clone()),
+                EthereumTxData::EthEventsVext(ext) => {
+                    // NB: only propose events with at least
+                    // one valid nonce
+                    ext.data
+                        .ethereum_events
+                        .iter()
+                        .any(|event| {
+                            self.wl_storage
+                                .ethbridge_queries()
+                                .validate_eth_event_nonce(event)
+                        })
+                        .then(|| tx_bytes.clone())
+                }
                 EthereumTxData::ValSetUpdateVext(ext) => {
                     // only include non-stale validator set updates
                     // in block proposals. it might be sitting long