Unify Peer and Client flows in the transaction Acceptor

node/src/components/transaction_acceptor.rs

@@ -30,7 +30,6 @@ use crate::{
         requests::{ContractRuntimeRequest, StorageRequest},
         EffectBuilder, EffectExt, Effects, Responder,
     },
-    fatal,
     types::MetaTransaction,
     utils::Source,
     NodeRng,
@@ -211,22 +210,18 @@ impl TransactionAcceptor {
             }
         };
 
-        if event_metadata.source.is_client() {
-            let account_hash = match event_metadata.transaction.initiator_addr() {
-                InitiatorAddr::PublicKey(public_key) => public_key.to_account_hash(),
-                InitiatorAddr::AccountHash(account_hash) => account_hash,
-            };
-            let entity_addr = EntityAddr::Account(account_hash.value());
-            effect_builder
-                .get_addressable_entity(*block_header.state_root_hash(), entity_addr)
-                .event(move |result| Event::GetAddressableEntityResult {
-                    event_metadata,
-                    maybe_entity: result.into_option(),
-                    block_header,
-                })
-        } else {
-            self.verify_payment(effect_builder, event_metadata, block_header)
-        }
+        let account_hash = match event_metadata.transaction.initiator_addr() {
+            InitiatorAddr::PublicKey(public_key) => public_key.to_account_hash(),
+            InitiatorAddr::AccountHash(account_hash) => account_hash,
+        };
+        let entity_addr = EntityAddr::Account(account_hash.value());
+        effect_builder
+            .get_addressable_entity(*block_header.state_root_hash(), entity_addr)
+            .event(move |result| Event::GetAddressableEntityResult {
+                event_metadata,
+                maybe_entity: result.into_option(),
+                block_header,
+            })
     }
 
     fn handle_get_entity_result<REv: ReactorEventT>(
@@ -280,15 +275,6 @@ impl TransactionAcceptor {
         block_header: Box<BlockHeader>,
         maybe_balance: Option<U512>,
     ) -> Effects<Event> {
-        if !event_metadata.source.is_client() {
-            // This would only happen due to programmer error and should crash the node. Balance
-            // checks for transactions received from a peer will cause the network to stall.
-            return fatal!(
-                effect_builder,
-                "Balance checks for transactions received from peers should never occur."
-            )
-            .ignore();
-        }
         match maybe_balance {
             None => {
                 let initiator_addr = event_metadata.transaction.initiator_addr();

node/src/components/transaction_acceptor/tests.rs

@@ -250,6 +250,7 @@ enum TestScenario {
         ContractVersionExistance,
     ),
     VmCasperV2ByPackageHash,
+    FromPeerInsufficientBalance(TxnType),
 }
 
 impl TestScenario {
@@ -268,7 +269,8 @@ impl TestScenario {
             | TestScenario::FromPeerCustomPaymentContractPackage(_)
             | TestScenario::FromPeerSessionContract(..)
             | TestScenario::FromPeerSessionContractPackage(..)
-            | TestScenario::InvalidFieldsFromPeer => Source::Peer(NodeId::random(rng)),
+            | TestScenario::InvalidFieldsFromPeer
+            | TestScenario::FromPeerInsufficientBalance(_) => Source::Peer(NodeId::random(rng)),
             TestScenario::FromClientInvalidTransaction(_)
             | TestScenario::FromClientInvalidTransactionZeroPayment(_)
             | TestScenario::FromClientSlightlyFutureDatedTransaction(_)
@@ -391,6 +393,7 @@ impl TestScenario {
             | TestScenario::FromPeerAccountWithInsufficientWeight(txn_type)
             | TestScenario::FromClientMissingAccount(txn_type)
             | TestScenario::FromClientInsufficientBalance(txn_type)
+            | TestScenario::FromPeerInsufficientBalance(txn_type)
             | TestScenario::FromClientValidTransaction(txn_type)
             | TestScenario::FromClientRepeatedValidTransaction(txn_type)
             | TestScenario::FromClientAccountWithInvalidAssociatedKeys(txn_type)
@@ -868,16 +871,15 @@ impl TestScenario {
             TestScenario::FromPeerRepeatedValidTransaction(_)
                     | TestScenario::FromPeerExpired(_)
                     | TestScenario::FromPeerValidTransaction(_)
-                    | TestScenario::FromPeerMissingAccount(_) // account check skipped if from peer
-                    | TestScenario::FromPeerAccountWithInsufficientWeight(_) // account check skipped if from peer
-                    | TestScenario::FromPeerAccountWithInvalidAssociatedKeys(_) // account check skipped if from peer
+                    | TestScenario::BalanceCheckForDeploySentByPeer
                     | TestScenario::FromClientRepeatedValidTransaction(_)
                     | TestScenario::FromClientValidTransaction(_)
                     | TestScenario::FromClientSlightlyFutureDatedTransaction(_)
                     | TestScenario::FromClientSignedByAdmin(..) => true,
             TestScenario::FromPeerInvalidTransaction(_)
                     | TestScenario::FromPeerInvalidTransactionZeroPayment(_)
                     | TestScenario::FromClientInsufficientBalance(_)
+                    | TestScenario::FromPeerInsufficientBalance(_)
                     | TestScenario::FromClientMissingAccount(_)
                     | TestScenario::FromClientInvalidTransaction(_)
                     | TestScenario::FromClientInvalidTransactionZeroPayment(_)
@@ -893,7 +895,10 @@ impl TestScenario {
                     | TestScenario::DeployWithoutTransferAmount
                     | TestScenario::DeployWithoutTransferTarget
                     | TestScenario::DeployWithPaymentOne
-                    | TestScenario::BalanceCheckForDeploySentByPeer
+
+            | TestScenario::FromPeerMissingAccount(_) // account check skipped if from peer
+            | TestScenario::FromPeerAccountWithInsufficientWeight(_) // account check skipped if from peer
+            | TestScenario::FromPeerAccountWithInvalidAssociatedKeys(_) // account check skipped if from peer
                     | TestScenario::FromClientExpired(_) => false,
             TestScenario::FromPeerCustomPaymentContract(contract_scenario)
                     | TestScenario::FromPeerSessionContract(_, contract_scenario)
@@ -1175,6 +1180,7 @@ impl reactor::Reactor for Reactor {
                     let motes = if matches!(
                         self.test_scenario,
                         TestScenario::FromClientInsufficientBalance(_)
+                            | TestScenario::FromPeerInsufficientBalance(_)
                     ) {
                         baseline_amount - 1
                     } else {
@@ -1627,7 +1633,10 @@ async fn run_transaction_acceptor_without_timeout(
             // announcement with the appropriate source.
             TestScenario::FromPeerInvalidTransaction(_)
             | TestScenario::FromPeerInvalidTransactionZeroPayment(_)
-            | TestScenario::BalanceCheckForDeploySentByPeer
+            | TestScenario::FromPeerMissingAccount(_)
+            | TestScenario::FromPeerAccountWithInvalidAssociatedKeys(_)
+            | TestScenario::FromPeerAccountWithInsufficientWeight(_)
+            | TestScenario::FromPeerInsufficientBalance(_)
             | TestScenario::InvalidFieldsFromPeer => {
                 matches!(
                     event,
@@ -1642,9 +1651,7 @@ async fn run_transaction_acceptor_without_timeout(
             // Check that a new and valid, transaction sent by a peer raises an
             // `AcceptedNewTransaction` announcement with the appropriate source.
             TestScenario::FromPeerValidTransaction(_)
-            | TestScenario::FromPeerMissingAccount(_)
-            | TestScenario::FromPeerAccountWithInvalidAssociatedKeys(_)
-            | TestScenario::FromPeerAccountWithInsufficientWeight(_)
+            | TestScenario::BalanceCheckForDeploySentByPeer
             | TestScenario::FromPeerExpired(_) => {
                 matches!(
                     event,
@@ -1859,13 +1866,25 @@ async fn should_reject_zero_payment_transaction_v1_from_peer() {
 async fn should_accept_valid_deploy_from_peer_for_missing_account() {
     let result =
         run_transaction_acceptor(TestScenario::FromPeerMissingAccount(TxnType::Deploy)).await;
-    assert!(result.is_ok())
+    assert!(matches!(
+        result,
+        Err(super::Error::Parameters {
+            failure: ParameterFailure::NoSuchAddressableEntity { .. },
+            ..
+        })
+    ))
 }
 
 #[tokio::test]
 async fn should_accept_valid_transaction_v1_from_peer_for_missing_account() {
     let result = run_transaction_acceptor(TestScenario::FromPeerMissingAccount(TxnType::V1)).await;
-    assert!(result.is_ok())
+    assert!(matches!(
+        result,
+        Err(super::Error::Parameters {
+            failure: ParameterFailure::NoSuchAddressableEntity { .. },
+            ..
+        })
+    ))
 }
 
 #[tokio::test]
@@ -1874,7 +1893,13 @@ async fn should_accept_valid_deploy_from_peer_for_account_with_invalid_associate
         TxnType::Deploy,
     ))
     .await;
-    assert!(result.is_ok())
+    assert!(matches!(
+        result,
+        Err(super::Error::Parameters {
+            failure: ParameterFailure::InvalidAssociatedKeys,
+            ..
+        })
+    ))
 }
 
 #[tokio::test]
@@ -1883,7 +1908,13 @@ async fn should_accept_valid_transaction_v1_from_peer_for_account_with_invalid_a
         TxnType::V1,
     ))
     .await;
-    assert!(result.is_ok())
+    assert!(matches!(
+        result,
+        Err(super::Error::Parameters {
+            failure: ParameterFailure::InvalidAssociatedKeys,
+            ..
+        })
+    ))
 }
 
 #[tokio::test]
@@ -1892,7 +1923,13 @@ async fn should_accept_valid_deploy_from_peer_for_account_with_insufficient_weig
         TxnType::Deploy,
     ))
     .await;
-    assert!(result.is_ok())
+    assert!(matches!(
+        result,
+        Err(super::Error::Parameters {
+            failure: ParameterFailure::InsufficientSignatureWeight,
+            ..
+        })
+    ))
 }
 
 #[tokio::test]
@@ -1901,7 +1938,13 @@ async fn should_accept_valid_transaction_v1_from_peer_for_account_with_insuffici
         TxnType::V1,
     ))
     .await;
-    assert!(result.is_ok())
+    assert!(matches!(
+        result,
+        Err(super::Error::Parameters {
+            failure: ParameterFailure::InsufficientSignatureWeight,
+            ..
+        })
+    ))
 }
 
 #[tokio::test]
@@ -2111,6 +2154,32 @@ async fn should_reject_valid_transaction_v1_from_client_for_insufficient_balance
     ))
 }
 
+#[tokio::test]
+async fn should_reject_valid_deploy_from_peer_for_insufficient_balance() {
+    let result =
+        run_transaction_acceptor(TestScenario::FromPeerInsufficientBalance(TxnType::Deploy)).await;
+    assert!(matches!(
+        result,
+        Err(super::Error::Parameters {
+            failure: ParameterFailure::InsufficientBalance { .. },
+            ..
+        })
+    ))
+}
+
+#[tokio::test]
+async fn should_reject_valid_transaction_v1_from_peer_for_insufficient_balance() {
+    let result =
+        run_transaction_acceptor(TestScenario::FromPeerInsufficientBalance(TxnType::V1)).await;
+    assert!(matches!(
+        result,
+        Err(super::Error::Parameters {
+            failure: ParameterFailure::InsufficientBalance { .. },
+            ..
+        })
+    ))
+}
+
 #[tokio::test]
 async fn should_reject_valid_deploy_from_client_for_unknown_balance() {
     let result = run_transaction_acceptor(TestScenario::AccountWithUnknownBalance).await;
@@ -2799,8 +2868,7 @@ async fn should_accept_expired_transaction_v1_from_peer() {
 }
 
 #[tokio::test]
-#[should_panic]
-async fn should_panic_when_balance_checking_for_deploy_sent_by_peer() {
+async fn should_not_panic_when_balance_checking_for_deploy_sent_by_peer() {
     let test_scenario = TestScenario::BalanceCheckForDeploySentByPeer;
     let result = run_transaction_acceptor(test_scenario).await;
     assert!(result.is_ok())