Merge remote-tracking branch 'upstream/dev' into 4360_less_strict_res…

…tart_checks

execution_engine/src/core/engine_state/engine_config.rs

@@ -43,7 +43,7 @@ pub const DEFAULT_ALLOW_AUCTION_BIDS: bool = true;
 pub const DEFAULT_ALLOW_UNRESTRICTED_TRANSFERS: bool = true;
 /// Default gas cost refund ratio.
 pub const DEFAULT_REFUND_HANDLING: RefundHandling = RefundHandling::Refund {
-    refund_ratio: Ratio::new_raw(0, 1),
+    refund_ratio: Ratio::new_raw(99, 100),
 };
 /// Default fee handling.
 pub const DEFAULT_FEE_HANDLING: FeeHandling = FeeHandling::PayToProposer;

execution_engine/src/storage/trie_store/operations/mod.rs

@@ -593,8 +593,9 @@ where
                                 let trie_key = new_extension.trie_hash()?;
                                 new_elements.push((trie_key, new_extension))
                             }
-                            // The single sibling is an extension.  We output an extension to replace
-                            // the parent, prepending the sibling index to the sibling's affix.  In
+                            // The single sibling is an extension.  We output an extension to
+                            // replace the parent, prepending the
+                            // sibling index to the sibling's affix.  In
                             // the next loop iteration, we will handle the case where this extension
                             // might need to be combined with a grandparent extension.
                             Trie::Extension {

execution_engine/src/system/handle_payment/internal.rs

@@ -49,6 +49,10 @@ pub(crate) fn get_refund_purse<R: RuntimeProvider>(
 }
 
 /// Returns tuple where 1st element is the refund, and 2nd element is the fee.
+///
+/// # Note
+///
+/// Any dust amounts are added to the fee.
 fn calculate_refund_and_fee(
     gas_spent: U512,
     payment_purse_balance: U512,
@@ -299,3 +303,37 @@ mod tests {
         }
     }
 }
+
+#[cfg(test)]
+mod proptests {
+    use proptest::prelude::*;
+
+    use super::*;
+
+    const DENOM_MAX: u64 = 1000;
+    const BALANCE_MAX: u64 = 100_000_000;
+
+    prop_compose! {
+      fn proper_fraction(max: u64)
+                        (numerator in 0..=max)
+                        (numerator in Just(numerator), denom in numerator..=max) -> Ratio<u64> {
+        Ratio::new(numerator, denom)
+      }
+    }
+
+    prop_compose! {
+      fn balance_and_gas(max_balance: u64)(balance in 100..=max_balance)(balance in Just(balance), gas in 1..=balance) -> (U512, U512) {
+        (U512::from(balance), U512::from(gas))
+      }
+    }
+
+    proptest! {
+        #[test]
+        fn refund_and_fee_equals_balance(refund_ratio in proper_fraction(DENOM_MAX), (balance, gas) in balance_and_gas(BALANCE_MAX)) {
+            let refund = RefundHandling::Refund { refund_ratio };
+
+            let (refund, fee) = calculate_refund_and_fee(gas, balance, &refund).unwrap();
+            prop_assert_eq!(refund + fee, balance);
+        }
+    }
+}

execution_engine_testing/test_support/src/chainspec_config.rs

@@ -11,8 +11,10 @@ use serde::{Deserialize, Serialize};
 
 use casper_execution_engine::{
     core::engine_state::{
-        genesis::ExecConfigBuilder, run_genesis_request::RunGenesisRequest, ExecConfig,
-        GenesisAccount,
+        engine_config::{FeeHandling, RefundHandling},
+        genesis::ExecConfigBuilder,
+        run_genesis_request::RunGenesisRequest,
+        ExecConfig, GenesisAccount,
     },
     shared::{system_config::SystemConfig, wasm_config::WasmConfig},
 };
@@ -72,6 +74,10 @@ pub struct CoreConfig {
     pub(crate) strict_argument_checking: bool,
     /// The maximum amount of delegators per validator.
     pub(crate) max_delegators_per_validator: Option<u32>,
+    /// Refund handling.
+    pub(crate) refund_handling: RefundHandling,
+    /// Fee handling.
+    pub(crate) fee_handling: FeeHandling,
 }
 
 /// This struct can be parsed from a TOML-encoded chainspec file.  It means that as the
@@ -134,17 +140,37 @@ impl ChainspecConfig {
     ) -> Result<RunGenesisRequest, Error> {
         let chainspec_config = ChainspecConfig::from_path(filename)?;
 
+        // if you get a compilation error here, make sure to update the builder below accordingly
+        let ChainspecConfig {
+            core_config,
+            wasm_config,
+            system_costs_config,
+        } = chainspec_config;
+        let CoreConfig {
+            validator_slots,
+            auction_delay,
+            locked_funds_period,
+            vesting_schedule_period: _,
+            unbonding_delay,
+            round_seigniorage_rate,
+            max_associated_keys: _,
+            max_runtime_call_stack_height: _,
+            minimum_delegation_amount: _,
+            strict_argument_checking: _,
+            max_delegators_per_validator: _,
+            refund_handling: _,
+            fee_handling: _,
+        } = core_config;
+
         let exec_config = ExecConfigBuilder::new()
             .with_accounts(genesis_accounts)
-            .with_wasm_config(chainspec_config.wasm_config)
-            .with_system_config(chainspec_config.system_costs_config)
-            .with_validator_slots(chainspec_config.core_config.validator_slots)
-            .with_auction_delay(chainspec_config.core_config.auction_delay)
-            .with_locked_funds_period_millis(
-                chainspec_config.core_config.locked_funds_period.millis(),
-            )
-            .with_round_seigniorage_rate(chainspec_config.core_config.round_seigniorage_rate)
-            .with_unbonding_delay(chainspec_config.core_config.unbonding_delay)
+            .with_wasm_config(wasm_config)
+            .with_system_config(system_costs_config)
+            .with_validator_slots(validator_slots)
+            .with_auction_delay(auction_delay)
+            .with_locked_funds_period_millis(locked_funds_period.millis())
+            .with_round_seigniorage_rate(round_seigniorage_rate)
+            .with_unbonding_delay(unbonding_delay)
             .with_genesis_timestamp_millis(DEFAULT_GENESIS_TIMESTAMP_MILLIS)
             .build();
 

execution_engine_testing/test_support/src/wasm_test_builder.rs

@@ -74,7 +74,7 @@ use casper_types::{
 };
 
 use crate::{
-    chainspec_config::{ChainspecConfig, PRODUCTION_PATH},
+    chainspec_config::{ChainspecConfig, CoreConfig, PRODUCTION_PATH},
     utils, ExecuteRequestBuilder, StepRequestBuilder, DEFAULT_GAS_PRICE, DEFAULT_PROPOSER_ADDR,
     DEFAULT_PROTOCOL_VERSION, SYSTEM_ADDR,
 };
@@ -214,25 +214,40 @@ impl InMemoryWasmTestBuilder {
         let chainspec_config = ChainspecConfig::from_chainspec_path(chainspec_path)
             .expect("must build chainspec configuration");
 
+        // if you get a compilation error here, make sure to update the builder below accordingly
+        let ChainspecConfig {
+            core_config,
+            wasm_config,
+            system_costs_config,
+        } = chainspec_config;
+        let CoreConfig {
+            validator_slots: _,
+            auction_delay: _,
+            locked_funds_period: _,
+            vesting_schedule_period,
+            unbonding_delay: _,
+            round_seigniorage_rate: _,
+            max_associated_keys,
+            max_runtime_call_stack_height,
+            minimum_delegation_amount,
+            strict_argument_checking,
+            max_delegators_per_validator,
+            refund_handling,
+            fee_handling,
+        } = core_config;
+
         let engine_config = EngineConfigBuilder::new()
             .with_max_query_depth(DEFAULT_MAX_QUERY_DEPTH)
-            .with_max_associated_keys(chainspec_config.core_config.max_associated_keys)
-            .with_max_runtime_call_stack_height(
-                chainspec_config.core_config.max_runtime_call_stack_height,
-            )
-            .with_minimum_delegation_amount(chainspec_config.core_config.minimum_delegation_amount)
-            .with_strict_argument_checking(chainspec_config.core_config.strict_argument_checking)
-            .with_vesting_schedule_period_millis(
-                chainspec_config
-                    .core_config
-                    .vesting_schedule_period
-                    .millis(),
-            )
-            .with_max_delegators_per_validator(
-                chainspec_config.core_config.max_delegators_per_validator,
-            )
-            .with_wasm_config(chainspec_config.wasm_config)
-            .with_system_config(chainspec_config.system_costs_config)
+            .with_max_associated_keys(max_associated_keys)
+            .with_max_runtime_call_stack_height(max_runtime_call_stack_height)
+            .with_minimum_delegation_amount(minimum_delegation_amount)
+            .with_strict_argument_checking(strict_argument_checking)
+            .with_vesting_schedule_period_millis(vesting_schedule_period.millis())
+            .with_max_delegators_per_validator(max_delegators_per_validator)
+            .with_wasm_config(wasm_config)
+            .with_system_config(system_costs_config)
+            .with_refund_handling(refund_handling)
+            .with_fee_handling(fee_handling)
             .build();
 
         let global_state = InMemoryGlobalState::empty().expect("should create global state");
@@ -1570,8 +1585,6 @@ where
 
         // amount declared to be paid in payment code MINUS gas spent in last execution.
         let refundable_amount = Ratio::from(payment_amount) - Ratio::from(gas_amount.value());
-        (refundable_amount * refund_ratio)
-            .ceil() // assumes possible dust amounts are always transferred to the user
-            .to_integer()
+        (refundable_amount * refund_ratio).to_integer()
     }
 }

execution_engine_testing/tests/src/test/explorer/faucet.rs

@@ -338,6 +338,7 @@ fn should_fund_existing_account() {
         .expect_success()
         .commit();
 
+    let refund = builder.calculate_refund_amount(user_account_initial_balance);
     let user_purse_balance_after = builder.get_purse_balance(user_purse_uref);
     let one_distribution = Ratio::new(
         faucet_purse_fund_amount,
@@ -347,7 +348,7 @@ fn should_fund_existing_account() {
 
     assert_eq!(
         user_purse_balance_after,
-        user_purse_balance_before + one_distribution - user_account_initial_balance
+        user_purse_balance_before + one_distribution - user_account_initial_balance + refund
     );
 }
 
@@ -425,6 +426,7 @@ fn should_not_fund_once_exhausted() {
     let user_main_purse_balance_before =
         builder.get_purse_balance(builder.get_expected_account(user_account).main_purse());
 
+    let mut refund = U512::zero();
     for i in 0..num_funds {
         let faucet_call_by_user = helper
             .new_faucet_fund_request_builder()
@@ -435,6 +437,8 @@ fn should_not_fund_once_exhausted() {
             .build();
 
         builder.exec(faucet_call_by_user).expect_success().commit();
+
+        refund += builder.calculate_refund_amount(U512::from(payment_amount));
     }
 
     let user_main_purse_balance_after =
@@ -448,7 +452,7 @@ fn should_not_fund_once_exhausted() {
 
     assert_eq!(
         user_main_purse_balance_after - user_main_purse_balance_before,
-        one_distribution * num_funds - (payment_amount * num_funds),
+        one_distribution * num_funds - (payment_amount * num_funds) + refund,
         "users main purse balance must match expected amount after user faucet calls ({} != {}*{} [{}])", user_main_purse_balance_after, one_distribution, num_funds, one_distribution * num_funds,
     );
 
@@ -464,12 +468,14 @@ fn should_not_fund_once_exhausted() {
         .build();
 
     builder.exec(faucet_call_by_user).expect_success().commit();
+    let refund = builder.calculate_refund_amount(U512::from(payment_amount));
 
     let user_main_purse_balance_after =
         builder.get_purse_balance(builder.get_expected_account(user_account).main_purse());
+
     assert_eq!(
         user_main_purse_balance_before - user_main_purse_balance_after,
-        U512::from(payment_amount),
+        U512::from(payment_amount) - refund,
         "no funds are distributed after faucet"
     );
 
@@ -493,6 +499,7 @@ fn should_not_fund_once_exhausted() {
         .build();
 
     builder.exec(faucet_call_by_user).expect_success().commit();
+    let refund = builder.calculate_refund_amount(U512::from(payment_amount));
 
     let user_main_purse_balance_after =
         builder.get_purse_balance(builder.get_expected_account(user_account).main_purse());
@@ -517,7 +524,7 @@ fn should_not_fund_once_exhausted() {
     );
 
     assert_eq!(
-        user_main_purse_balance_after - user_main_purse_balance_before + payment_amount,
+        user_main_purse_balance_after - user_main_purse_balance_before + payment_amount - refund,
         // one_distribution * (num_funds + 1), // - user_fund_amount * 2
         // user_fund_amount,
         one_distribution,
@@ -835,6 +842,66 @@ fn should_allow_funding_by_an_authorized_account() {
     );
 }
 
+#[ignore]
+#[test]
+fn should_refund_proper_amount() {
+    let user_account = AccountHash::new([7u8; 32]);
+
+    let mut builder = InMemoryWasmTestBuilder::default();
+    builder.run_genesis(&PRODUCTION_RUN_GENESIS_REQUEST);
+
+    let payment_amount = U512::from(10_000_000_000u64);
+
+    let mut helper = FaucetDeployHelper::default();
+    builder
+        .exec(helper.fund_installer_request())
+        .expect_success()
+        .commit();
+
+    let user_account_initial_balance = U512::from(15_000_000_000u64);
+
+    let fund_user_request = FundAccountRequestBuilder::new()
+        .with_target_account(user_account)
+        .with_fund_amount(user_account_initial_balance)
+        .build();
+
+    builder.exec(fund_user_request).expect_success().commit();
+
+    builder
+        .exec(helper.faucet_install_request())
+        .expect_success()
+        .commit();
+
+    helper.query_and_set_faucet_contract_hash(&builder);
+
+    builder
+        .exec(helper.faucet_config_request())
+        .expect_success()
+        .commit();
+
+    let user_purse_uref = builder.get_expected_account(user_account).main_purse();
+    let user_purse_balance_before = builder.get_purse_balance(user_purse_uref);
+
+    builder
+        .exec(
+            helper
+                .new_faucet_fund_request_builder()
+                .with_user_account(user_account)
+                .with_payment_amount(payment_amount)
+                .build(),
+        )
+        .expect_success()
+        .commit();
+
+    let refund = builder.calculate_refund_amount(payment_amount);
+    let user_purse_balance_after = builder.get_purse_balance(user_purse_uref);
+
+    assert_eq!(
+        user_purse_balance_after,
+        user_purse_balance_before - payment_amount + refund
+    );
+}
+
 #[ignore]
 #[test]
 fn faucet_costs() {

node/src/components/consensus/cl_context.rs

@@ -13,6 +13,7 @@ use crate::{
 };
 
 #[derive(DataSize)]
+#[cfg_attr(test, derive(Clone))]
 pub struct Keypair {
     secret_key: Arc<SecretKey>,
     public_key: PublicKey,

node/src/components/consensus/highway_core/highway.rs

@@ -224,6 +224,16 @@ impl<C: Context> Highway<C> {
         self.active_validator = None;
     }
 
+    /// Gets the round exponent for the next message this instance will create.
+    #[cfg(test)]
+    #[allow(clippy::integer_arithmetic)]
+    pub(crate) fn get_round_exp(&self) -> Option<u8> {
+        self.active_validator.as_ref().map(|av| {
+            (av.next_round_length().millis() / self.state.params().min_round_length().millis())
+                .trailing_zeros() as u8
+        })
+    }
+
     /// Switches the active validator to a new round length.
     pub(crate) fn set_round_len(&mut self, new_round_len: TimeDiff) {
         if let Some(ref mut av) = self.active_validator {

node/src/components/consensus/protocols/highway.rs

@@ -367,7 +367,7 @@ impl<C: Context + 'static> HighwayProtocol<C> {
     fn calculate_round_length(&mut self, vv: &ValidVertex<C>, now: Timestamp) {
         let new_round_len = self
             .round_success_meter
-            .calculate_new_length(self.highway.state());
+            .calculate_new_length(self.highway.state(), now);
         // If the vertex contains a proposal, register it in the success meter.
         // It's important to do this _after_ the calculation above - otherwise we might try to
         // register the proposal before the meter is aware that a new round has started, and it