Merge pull request #689 from anoma/james/ethbridge/shim-fix-backport

Backport shim fix to eth-bridge-integration
anoma · Oct 27, 2022 · 04cf854 · 04cf854
2 parents b977845 + 0fe5edf
commit 04cf854
Show file tree

Hide file tree

Showing 3 changed files with 92 additions and 69 deletions.
diff --git a/apps/src/lib/node/ledger/shell/process_proposal.rs b/apps/src/lib/node/ledger/shell/process_proposal.rs
@@ -15,7 +15,7 @@ use crate::node::ledger::shims::abcipp_shim_types::shim::response::ProcessPropos
 /// Contains stateful data about the number of vote extension
 /// digests found as protocol transactions in a proposed block.
 #[derive(Default)]
-pub(crate) struct DigestCounters {
+pub struct DigestCounters {
     /// The number of Ethereum events vote extensions found thus far.
     eth_ev_digest_num: usize,
     /// The number of validator set update vote extensions found thus far.
@@ -43,27 +43,14 @@ where
         &self,
         req: RequestProcessProposal,
     ) -> ProcessProposal {
-        let mut tx_queue_iter = self.storage.tx_queue.iter();
         tracing::info!(
             proposer = ?hex::encode(&req.proposer_address),
             height = req.height,
             hash = ?hex::encode(&req.hash),
             n_txs = req.txs.len(),
             "Received block proposal",
         );
-        // the number of vote extension digests included in the block proposal
-        let mut counters = DigestCounters::default();
-        let tx_results: Vec<_> = req
-            .txs
-            .iter()
-            .map(|tx_bytes| {
-                self.process_single_tx(
-                    tx_bytes,
-                    &mut tx_queue_iter,
-                    &mut counters,
-                )
-            })
-            .collect();
+        let (tx_results, counters) = self.check_proposal(&req.txs);
 
         // We should not have more than one `ethereum_events::VextDigest` in
         // a proposal from some round's leader.
@@ -128,6 +115,31 @@ where
         }
     }
 
+    /// Checks what the [`TxResult`]s would be for the transactions in a
+    /// proposed block, as well as counting the number of digest transactions
+    /// present. `ProcessProposal` should be able to make a decision on whether
+    /// a proposed block is acceptable or not based solely on what this function
+    /// returns.
+    pub fn check_proposal(
+        &self,
+        txs: &[Vec<u8>],
+    ) -> (Vec<TxResult>, DigestCounters) {
+        let mut tx_queue_iter = self.storage.tx_queue.iter();
+        // the number of vote extension digests included in the block proposal
+        let mut counters = DigestCounters::default();
+        let tx_results: Vec<_> = txs
+            .iter()
+            .map(|tx_bytes| {
+                self.check_proposal_tx(
+                    tx_bytes,
+                    &mut tx_queue_iter,
+                    &mut counters,
+                )
+            })
+            .collect();
+        (tx_results, counters)
+    }
+
     /// Validates a list of vote extensions, included in PrepareProposal.
     ///
     /// If a vote extension is [`Some`], then it was validated properly,
@@ -215,7 +227,7 @@ where
     /// INVARIANT: Any changes applied in this method must be reverted if the
     /// proposal is rejected (unless we can simply overwrite them in the
     /// next block).
-    pub(crate) fn process_single_tx<'a>(
+    pub(crate) fn check_proposal_tx<'a>(
         &self,
         tx_bytes: &[u8],
         tx_queue_iter: &mut impl Iterator<Item = &'a WrapperTx>,

diff --git a/apps/src/lib/node/ledger/shims/abcipp_shim.rs b/apps/src/lib/node/ledger/shims/abcipp_shim.rs
@@ -8,11 +8,17 @@ use futures::future::FutureExt;
 use namada::types::ethereum_events::EthereumEvent;
 #[cfg(not(feature = "abcipp"))]
 use namada::types::hash::Hash;
+#[cfg(not(feature = "abcipp"))]
+use namada::types::storage::BlockHash;
+#[cfg(not(feature = "abcipp"))]
+use namada::types::transaction::hash_tx;
 use tokio::sync::mpsc::{Receiver, UnboundedSender};
 use tower::Service;
 
 use super::super::Shell;
 use super::abcipp_shim_types::shim::request::{FinalizeBlock, ProcessedTx};
+#[cfg(not(feature = "abcipp"))]
+use super::abcipp_shim_types::shim::TxBytes;
 use super::abcipp_shim_types::shim::{Error, Request, Response};
 use crate::config;
 #[cfg(not(feature = "abcipp"))]
@@ -27,7 +33,8 @@ pub struct AbcippShim {
     service: Shell,
     #[cfg(not(feature = "abcipp"))]
     begin_block_request: Option<RequestBeginBlock>,
-    processed_txs: Vec<ProcessedTx>,
+    #[cfg(not(feature = "abcipp"))]
+    delivered_txs: Vec<TxBytes>,
     shell_recv: std::sync::mpsc::Receiver<(
         Req,
         tokio::sync::oneshot::Sender<Result<Resp, BoxError>>,
@@ -62,7 +69,8 @@ impl AbcippShim {
                 ),
                 #[cfg(not(feature = "abcipp"))]
                 begin_block_request: None,
-                processed_txs: vec![],
+                #[cfg(not(feature = "abcipp"))]
+                delivered_txs: vec![],
                 shell_recv,
             },
             AbciService { shell_send },
@@ -72,12 +80,8 @@ impl AbcippShim {
     #[cfg(not(feature = "abcipp"))]
     /// Get the hash of the txs in the block
     pub fn get_hash(&self) -> Hash {
-        use namada::types::transaction::hash_tx;
-        let bytes: Vec<u8> = self
-            .processed_txs
-            .iter()
-            .flat_map(|processed| processed.tx.clone())
-            .collect();
+        let bytes: Vec<u8> =
+            self.delivered_txs.iter().flat_map(Clone::clone).collect();
         hash_tx(bytes.as_slice())
     }
 
@@ -86,32 +90,28 @@ impl AbcippShim {
     pub fn run(mut self) {
         while let Ok((req, resp_sender)) = self.shell_recv.recv() {
             let resp = match req {
-                Req::ProcessProposal(proposal) => {
-                    let txs = proposal.txs.clone();
-                    self.service
-                        .call(Request::ProcessProposal(proposal))
-                        .map_err(Error::from)
-                        .and_then(|res| match res {
-                            Response::ProcessProposal(resp) => {
-                                let response =
-                                    Ok(Resp::ProcessProposal((&resp).into()));
-                                for (result, tx) in resp
-                                    .tx_results
-                                    .into_iter()
-                                    .zip(txs.into_iter())
-                                {
-                                    self.processed_txs
-                                        .push(ProcessedTx { tx, result });
-                                }
-                                response
-                            }
-                            _ => unreachable!(),
-                        })
-                }
+                Req::ProcessProposal(proposal) => self
+                    .service
+                    .call(Request::ProcessProposal(proposal))
+                    .map_err(Error::from)
+                    .and_then(|res| match res {
+                        Response::ProcessProposal(resp) => {
+                            Ok(Resp::ProcessProposal((&resp).into()))
+                        }
+                        _ => unreachable!(),
+                    }),
                 #[cfg(feature = "abcipp")]
                 Req::FinalizeBlock(block) => {
-                    let mut txs = vec![];
-                    std::mem::swap(&mut txs, &mut self.processed_txs);
+                    let unprocessed_txs = block.txs.clone();
+                    let (processing_results, _) =
+                        self.service.check_proposal(&block.txs);
+                    let mut txs = Vec::with_capacity(unprocessed_txs.len());
+                    for (result, tx) in processing_results
+                        .into_iter()
+                        .zip(unprocessed_txs.into_iter())
+                    {
+                        txs.push(ProcessedTx { tx, result });
+                    }
                     let mut finalize_req: FinalizeBlock = block.into();
                     finalize_req.txs = txs;
                     self.service
@@ -131,12 +131,23 @@ impl AbcippShim {
                     Ok(Resp::BeginBlock(Default::default()))
                 }
                 #[cfg(not(feature = "abcipp"))]
-                Req::DeliverTx(_) => Ok(Resp::DeliverTx(Default::default())),
+                Req::DeliverTx(tx) => {
+                    self.delivered_txs.push(tx.tx);
+                    Ok(Resp::DeliverTx(Default::default()))
+                }
                 #[cfg(not(feature = "abcipp"))]
                 Req::EndBlock(_) => {
-                    use namada::types::storage::BlockHash;
-                    let mut txs = vec![];
-                    std::mem::swap(&mut txs, &mut self.processed_txs);
+                    let (processing_results, _) =
+                        self.service.check_proposal(&self.delivered_txs);
+                    let mut txs = Vec::with_capacity(self.delivered_txs.len());
+                    let mut delivered = vec![];
+                    std::mem::swap(&mut self.delivered_txs, &mut delivered);
+                    for (result, tx) in processing_results
+                        .into_iter()
+                        .zip(delivered.into_iter())
+                    {
+                        txs.push(ProcessedTx { tx, result });
+                    }
                     let mut end_block_request: FinalizeBlock =
                         self.begin_block_request.take().unwrap().into();
                     let hash = self.get_hash();

diff --git a/wasm/checksums.json b/wasm/checksums.json
@@ -1,20 +1,20 @@
 {
-    "tx_bond.wasm": "tx_bond.bff6b2424d05de6dd951fd2d1660119768ca6896556e6f54d106b6aeec61f4ae.wasm",
+    "tx_bond.wasm": "tx_bond.5536bed92d94b1aaaa82fd3531b7d4fedb00ae1f39fede7ef1634e070e5f4455.wasm",
     "tx_bridge_pool.wasm": "tx_bridge_pool.e21563260c03cfdab1f195878f49bf93722027ad26fcd097cfebbc5c4d279082.wasm",
-    "tx_from_intent.wasm": "tx_from_intent.802709899f297a6e7574e61cee92a9b73d4f1d78db8b0fecd6f71aee10a62c1b.wasm",
-    "tx_ibc.wasm": "tx_ibc.3df9ad5e43985b6a0d73429534b5bbacc8f4c5b067b65cc6c9517ad76d575d73.wasm",
-    "tx_init_account.wasm": "tx_init_account.814d24ec3e149c141cb90293f32b7d9229cf2adcc21eb64763cebefa30651cb0.wasm",
-    "tx_init_nft.wasm": "tx_init_nft.ca7a4085bfa8935329e9c72814c9f468499f0b4bdc32722aef69b31701f554bb.wasm",
-    "tx_init_proposal.wasm": "tx_init_proposal.4201a913c38f90d9c7ed4a780f77eff359e0f3b8d20f0220237f72c06461e4a5.wasm",
-    "tx_init_validator.wasm": "tx_init_validator.2966562a461dd8bccceb23277f96f04c00eb7fccb6e122f31b070a2ae5170340.wasm",
-    "tx_mint_nft.wasm": "tx_mint_nft.8c344161a360b130a94ba502f4e4fe03bc9b30b5835db23b1920043eb743c641.wasm",
-    "tx_transfer.wasm": "tx_transfer.a5026af845ccaa564a7a45b83362f1ef527254f66228217811819dd2786c3172.wasm",
-    "tx_unbond.wasm": "tx_unbond.c3af1a051ec4f06a719c654af4aa483934e114af0dc3618a6d3f80f443f2aac3.wasm",
-    "tx_update_vp.wasm": "tx_update_vp.f01e33f9de91690ab4a8a70369bf0b7fd97ad72bdc601d0f5b7521893e2ca3f1.wasm",
-    "tx_vote_proposal.wasm": "tx_vote_proposal.36a43ac9488f36b3de7dc3cd1a7e7c6190e01258e1d46a6650c799d0a066b15a.wasm",
-    "tx_withdraw.wasm": "tx_withdraw.443820132a7af051215248d47a4e5636ed802ed35de9b634a9163321e26f0151.wasm",
-    "vp_nft.wasm": "vp_nft.cebe4febc075da02a6a9c37dbc8fc0a3495c644c37a4e0b21b55ea312f97c555.wasm",
-    "vp_testnet_faucet.wasm": "vp_testnet_faucet.c1018973f87dfe397cd7744a6371e8b82e3cfdfed1ac24c42601d17d37f87035.wasm",
-    "vp_token.wasm": "vp_token.27dda715efb921bf2fe232159782eeea00a81deb450bdf79e4e1d9b3e181bd47.wasm",
-    "vp_user.wasm": "vp_user.069f5eb052ee0507a1b2141cdd92c4f100ffe6a77f071c166bf3ed3bc7ac0d5d.wasm"
+    "tx_from_intent.wasm": "tx_from_intent.7243e31594d0ca5e656228bed5c84359a3d1dbfd24f1656eb9b52b0266ffaa47.wasm",
+    "tx_ibc.wasm": "tx_ibc.1c09d3f083a91f28708b7f09698c311e609b712da683dcbcd40e7267a1cf2adc.wasm",
+    "tx_init_account.wasm": "tx_init_account.95b3a1e4867160eb91f9a7812f53adf33bb44f04b9eb3f310854b7a1d7a26e77.wasm",
+    "tx_init_nft.wasm": "tx_init_nft.022446ca174c6ccb1e7818224ebc30515123e0bc148ec8dd59ce5b47f7447bd7.wasm",
+    "tx_init_proposal.wasm": "tx_init_proposal.8f93bb139e932ba24871efe22cb62177c1c7133f8957965c041accad3d04516e.wasm",
+    "tx_init_validator.wasm": "tx_init_validator.916a8c6a6008d5f35341e58ccafd8169b7596464f1f3253eea96c277f167fe1b.wasm",
+    "tx_mint_nft.wasm": "tx_mint_nft.9e12869d07ac36a9fdddf69710f2f6905c55bb4dc49c885e87d47b3d6c389ec2.wasm",
+    "tx_transfer.wasm": "tx_transfer.48c8bfdd119c0753a03c65764b0e099f6647dd158490fb5e04eab171c27b422e.wasm",
+    "tx_unbond.wasm": "tx_unbond.b2851a1eefd2e781411e495ef52ce6148893c02e06d0160298871fd231e299b4.wasm",
+    "tx_update_vp.wasm": "tx_update_vp.5b9786f039324205c464fec6c795d84ea3ec9b24d3c6c63e7462444811237855.wasm",
+    "tx_vote_proposal.wasm": "tx_vote_proposal.31c96b1b171d0020c0788170f9677606589324024a90e3d532da2b9a2cedf57e.wasm",
+    "tx_withdraw.wasm": "tx_withdraw.5e360d269c8d3ae574ae9cbf301e1d9eed8c3ad6c103deb5e1c46ddb35af5708.wasm",
+    "vp_nft.wasm": "vp_nft.29b7fb0b2c247bb5389166e82707abd256177f276bbaf40ccd201ecad8039bad.wasm",
+    "vp_testnet_faucet.wasm": "vp_testnet_faucet.e1b7805802df2871a93f584e4df97a4584cda9d58435b14c73791fe18d93e4f4.wasm",
+    "vp_token.wasm": "vp_token.8332429c84d70ab6614b20a41d208ac61126141d302319a99269f6fc9e08053a.wasm",
+    "vp_user.wasm": "vp_user.795a1dccb7b18f0abeb07161d07e31ce5d64221a52e85098627ab60115462ba4.wasm"
 }