Merge #5047

5047: GH-5039: Tweak costs and chainspec settings r=EdHastingsCasperAssociation a=mpapierski

Closes #5039 

This PR adds a gas charge for calling a non-existing system contract entry point. Additionally, some charges for incorrectly charged system contract entrypoints are raised significantly (10k motes -> 2.5 CSPR or more depending on complexity).

This PR changes `fee_handling` and `refund_handling` to be consistent with 1.5.x settings (refund 99% and pay to proposer)


Co-authored-by: Michał Papierski <[email protected]>
Co-authored-by: edhastings <[email protected]>

execution_engine/src/runtime/mod.rs

@@ -859,8 +859,11 @@ where
                     mint_runtime.mint_into_existing_purse(existing_purse, amount);
                 CLValue::from_t(result).map_err(Self::reverter)
             })(),
-
-            _ => CLValue::from_t(()).map_err(Self::reverter),
+            _ => {
+                // Code should never reach this point as existence of the entrypoint is validated
+                // before reaching this pointt.
+                Ok(CLValue::unit())
+            }
         };
 
         // Charge just for the amount that particular entry point cost - using gas cost from the
@@ -954,7 +957,11 @@ where
                 let maybe_purse = runtime.get_refund_purse().map_err(Self::reverter)?;
                 CLValue::from_t(maybe_purse).map_err(Self::reverter)
             })(),
-            _ => CLValue::from_t(()).map_err(Self::reverter),
+            _ => {
+                // Code should never reach here as existence of the entrypoint is validated before
+                // reaching this point.
+                Ok(CLValue::unit())
+            }
         };
 
         self.gas(
@@ -1290,8 +1297,11 @@ where
 
                 CLValue::from_t(()).map_err(Self::reverter)
             })(),
-
-            _ => CLValue::from_t(()).map_err(Self::reverter),
+            _ => {
+                // Code should never reach here as existence of the entrypoint is validated before
+                // reaching this point.
+                Ok(CLValue::unit())
+            }
         };
 
         // Charge for the gas spent during execution in an isolated runtime.
@@ -1693,11 +1703,24 @@ where
             return Err(ExecError::InvalidContext);
         }
 
-        let entry_point = footprint
-            .entry_points()
-            .get(entry_point_name)
-            .cloned()
-            .ok_or_else(|| ExecError::NoSuchMethod(entry_point_name.to_owned()))?;
+        let entry_point = match footprint.entry_points().get(entry_point_name) {
+            Some(entry_point) => entry_point,
+            None => {
+                match footprint.entity_kind() {
+                    EntityKind::System(_) => {
+                        self.charge_system_contract_call(
+                            self.context()
+                                .engine_config()
+                                .system_config()
+                                .no_such_entrypoint(),
+                        )?;
+                    }
+                    EntityKind::Account(_) => {}
+                    EntityKind::SmartContract(_) => {}
+                }
+                return Err(ExecError::NoSuchMethod(entry_point_name.to_owned()));
+            }
+        };
 
         let entry_point_type = entry_point.entry_point_type();
 
@@ -1757,7 +1780,7 @@ where
         // if session the caller's context
         // else the called contract's context
         let context_entity_key =
-            self.get_context_key_for_contract_call(entity_addr, &entry_point)?;
+            self.get_context_key_for_contract_call(entity_addr, entry_point)?;
 
         let context_entity_hash = context_entity_key
             .into_entity_hash_addr()

execution_engine_testing/tests/src/test/regression/ee_1163.rs

@@ -1,11 +1,11 @@
 use casper_engine_test_support::{
     LmdbWasmTestBuilder, TransferRequestBuilder, DEFAULT_ACCOUNT_ADDR, LOCAL_GENESIS_REQUEST,
 };
-use casper_execution_engine::engine_state::{Error, WASMLESS_TRANSFER_FIXED_GAS_PRICE};
+use casper_execution_engine::engine_state::Error;
 use casper_storage::{data_access_layer::TransferRequest, system::transfer::TransferError};
 use casper_types::{
-    account::AccountHash, system::handle_payment, FeeHandling, Gas, MintCosts, Motes, RuntimeArgs,
-    SystemConfig, U512,
+    account::AccountHash, system::handle_payment, Gas, MintCosts, Motes, RuntimeArgs, SystemConfig,
+    U512,
 };
 
 const ACCOUNT_1_ADDR: AccountHash = AccountHash::new([1u8; 32]);
@@ -21,38 +21,15 @@ fn should_enforce_limit_for_user_error(
     request: TransferRequest,
 ) -> Error {
     let transfer_cost = Gas::from(SystemConfig::default().mint_costs().transfer);
-    let transfer_cost_motes =
-        Motes::from_gas(transfer_cost, WASMLESS_TRANSFER_FIXED_GAS_PRICE).expect("gas overflow");
-
-    let default_account = builder
-        .get_entity_by_account_hash(*DEFAULT_ACCOUNT_ADDR)
-        .expect("should have default account");
-    let main_purse = default_account.main_purse();
-    let purse_balance_before = builder.get_purse_balance(main_purse);
-    let proposer_purse_balance_before = builder.get_proposer_purse_balance();
 
     builder.transfer_and_commit(request);
 
-    let purse_balance_after = builder.get_purse_balance(main_purse);
-    let proposer_purse_balance_after = builder.get_proposer_purse_balance();
-
     let response = builder
         .get_exec_result_owned(0)
         .expect("should have result");
 
     assert_eq!(response.limit(), transfer_cost);
-    let fee_handling = builder.chainspec().core_config.fee_handling;
-    if fee_handling == FeeHandling::PayToProposer {
-        assert_eq!(
-            purse_balance_before - transfer_cost_motes.value(),
-            purse_balance_after
-        );
-        assert_eq!(
-            proposer_purse_balance_before + transfer_cost_motes.value(),
-            proposer_purse_balance_after
-        );
-    }
-    // Verify handle payment postconditions
+    assert_eq!(response.consumed(), transfer_cost);
 
     let handle_payment = builder.get_handle_payment_contract();
     let payment_purse = handle_payment

execution_engine_testing/tests/src/test/system_costs.rs

@@ -625,10 +625,6 @@ fn should_charge_for_erroneous_system_contract_calls() {
     let mint_hash = builder.get_mint_contract_hash();
     let handle_payment_hash = builder.get_handle_payment_contract_hash();
 
-    let account = builder
-        .get_entity_by_account_hash(*DEFAULT_ACCOUNT_ADDR)
-        .expect("should have account");
-
     let system_config = builder.chainspec().system_costs_config;
 
     // Entrypoints that could fail early due to missing arguments
@@ -641,37 +637,37 @@ fn should_charge_for_erroneous_system_contract_calls() {
         (
             auction_hash,
             auction::METHOD_WITHDRAW_BID,
-            system_config.auction_costs().withdraw_bid.into(),
+            system_config.auction_costs().withdraw_bid,
         ),
         (
             auction_hash,
             auction::METHOD_DELEGATE,
-            system_config.auction_costs().delegate.into(),
+            system_config.auction_costs().delegate,
         ),
         (
             auction_hash,
             auction::METHOD_UNDELEGATE,
-            system_config.auction_costs().undelegate.into(),
+            system_config.auction_costs().undelegate,
         ),
         (
             auction_hash,
             auction::METHOD_REDELEGATE,
-            system_config.auction_costs().redelegate.into(),
+            system_config.auction_costs().redelegate,
         ),
         (
             auction_hash,
             auction::METHOD_RUN_AUCTION,
-            system_config.auction_costs().run_auction.into(),
+            system_config.auction_costs().run_auction,
         ),
         (
             auction_hash,
             auction::METHOD_SLASH,
-            system_config.auction_costs().slash.into(),
+            system_config.auction_costs().slash,
         ),
         (
             auction_hash,
             auction::METHOD_DISTRIBUTE,
-            system_config.auction_costs().distribute.into(),
+            system_config.auction_costs().distribute,
         ),
         (
             mint_hash,
@@ -703,6 +699,21 @@ fn should_charge_for_erroneous_system_contract_calls() {
         //     handle_payment::METHOD_FINALIZE_PAYMENT,
         //     system_config.handle_payment_costs().finalize_payment,
         // ),
+        (
+            auction_hash,
+            "this_entrypoint_does_not_exists_1",
+            system_config.no_such_entrypoint(),
+        ),
+        (
+            mint_hash,
+            "this_entrypoint_does_not_exists_2",
+            system_config.no_such_entrypoint(),
+        ),
+        (
+            handle_payment_hash,
+            "this_entrypoint_does_not_exists_3",
+            system_config.no_such_entrypoint(),
+        ),
     ];
 
     for (contract_hash, entrypoint, expected_cost) in entrypoint_calls {
@@ -714,31 +725,22 @@ fn should_charge_for_erroneous_system_contract_calls() {
         )
         .build();
 
-        let balance_before = builder.get_purse_balance(account.main_purse());
-
-        let proposer_reward_starting_balance = builder.get_proposer_purse_balance();
-
         builder.exec(exec_request).commit();
 
         let _error = builder
             .get_last_exec_result()
             .expect("should have results")
             .error()
+            .cloned()
             .unwrap_or_else(|| panic!("should have error while executing {}", entrypoint));
 
-        let transaction_fee =
-            builder.get_proposer_purse_balance() - proposer_reward_starting_balance;
-
-        let balance_after = builder.get_purse_balance(account.main_purse());
+        // assert!(matches!(
+        //     error,
+        //     Error::Exec(ExecError::NoSuchMethod(ref no_such_method)) if no_such_method ==
+        // entrypoint), "{:?}",  error);
 
         let call_cost = U512::from(expected_cost);
-        assert_eq!(
-            balance_after,
-            balance_before - transaction_fee,
-            "Calling a failed entrypoint {} does not incur expected cost of {}",
-            entrypoint,
-            expected_cost,
-        );
+
         assert_eq!(
             builder.last_exec_gas_consumed().value(),
             call_cost,
@@ -856,6 +858,7 @@ fn should_verify_wasm_add_bid_wasm_cost_is_not_recursive() {
     let new_handle_payment_costs = HandlePaymentCosts::default();
 
     let system_costs_config = SystemConfig::new(
+        1,
         new_auction_costs,
         new_mint_costs,
         new_handle_payment_costs,

node/Cargo.toml

@@ -83,7 +83,7 @@ tokio-serde = { version = "0.8.0", features = ["bincode"] }
 tokio-stream = { version = "0.1.4", features = ["sync"] }
 tokio-util = { version = "0.6.4", features = ["codec"] }
 mio = "0.8.11"
-toml = { version = "0.7.8", features = ["preserve_order"] }
+toml = { version = "0.8.19", features = ["preserve_order"] }
 tower = { version = "0.4.6", features = ["limit"] }
 tracing = "0.1.18"
 tracing-futures = "0.2.5"

node/src/reactor/main_reactor/tests.rs

@@ -3283,8 +3283,14 @@ async fn run_gas_price_scenario(gas_price_scenario: GasPriceScenario) {
         .highest_complete_block()
         .maybe_current_gas_price()
         .expect("must have gas price");
-    let cost =
-        fixture.chainspec.system_costs_config.mint_costs().transfer * (current_gas_price as u32);
+
+    let cost = match fixture.chainspec.core_config.pricing_handling {
+        PricingHandling::Classic => 0,
+        PricingHandling::Fixed => {
+            fixture.chainspec.system_costs_config.mint_costs().transfer * (current_gas_price as u32)
+        }
+    };
+
     assert_eq!(holds_after, holds_before + U512::from(cost));
 
     // Run the network at zero load and ensure the value falls back to the floor.