Merge #4809

4809: BUGFIX: Fix miswire of min max delegations amount  r=darthsiroftardis a=darthsiroftardis

CHANGELOG:

- Fixed an issue where the min and max delegations amounts were inverted
- Added a test and check to ensure that max delegation amount can never be strictly less than the min
- Removed the pruning of legacy contract hash and wash hash key during the migration of contract packages

Co-authored-by: Karan Dhareshwar <[email protected]>

binary_port/src/error_code.rs

@@ -274,6 +274,9 @@ pub enum ErrorCode {
     /// Gas price tolerance too low
     #[error("gas price tolerance too low")]
     GasPriceToleranceTooLow = 85,
+    /// Received V1 Transaction for spec exec.
+    #[error("received v1 transaction for speculative execution")]
+    ReceivedV1Transaction = 86,
 }
 
 impl TryFrom<u16> for ErrorCode {
@@ -367,6 +370,7 @@ impl TryFrom<u16> for ErrorCode {
             83 => Ok(ErrorCode::InvalidTransactionEntryPointCannotBeCall),
             84 => Ok(ErrorCode::InvalidTransactionInvalidTransactionKind),
             85 => Ok(ErrorCode::GasPriceToleranceTooLow),
+            86 => Ok(ErrorCode::ReceivedV1Transaction),
             _ => Err(UnknownErrorCode),
         }
     }

execution_engine_testing/test_support/src/wasm_test_builder.rs

@@ -1344,6 +1344,19 @@ where
         self
     }
 
+    /// Expect failure of the protocol upgrade.
+    pub fn expect_upgrade_failure(&mut self) -> &mut Self {
+        // Check first result, as only first result is interesting for a simple test
+        let result = self
+            .upgrade_results
+            .last()
+            .expect("Expected to be called after a system upgrade.");
+
+        assert!(result.is_err(), "Expected Failure got {:?}", result);
+
+        self
+    }
+
     /// Returns the "handle payment" contract, panics if it can't be found.
     pub fn get_handle_payment_contract(&self) -> EntityWithNamedKeys {
         let hash = self

execution_engine_testing/tests/src/test/system_contracts/upgrade.rs

@@ -707,3 +707,28 @@ fn should_correctly_migrate_and_prune_system_contract_records() {
             .expect("must have system entity");
     }
 }
+
+#[test]
+fn should_not_migrate_bids_with_invalid_min_max_delegation_amounts() {
+    let mut builder = LmdbWasmTestBuilder::default();
+
+    builder.run_genesis(LOCAL_GENESIS_REQUEST.clone());
+
+    let sem_ver = PROTOCOL_VERSION.value();
+    let new_protocol_version =
+        ProtocolVersion::from_parts(sem_ver.major, sem_ver.minor, sem_ver.patch + 1);
+
+    let mut upgrade_request = {
+        UpgradeRequestBuilder::new()
+            .with_current_protocol_version(PROTOCOL_VERSION)
+            .with_new_protocol_version(new_protocol_version)
+            .with_activation_point(DEFAULT_ACTIVATION_POINT)
+            .with_maximum_delegation_amount(250_000_000_000)
+            .with_minimum_delegation_amount(500_000_000_000)
+            .build()
+    };
+
+    builder
+        .upgrade(&mut upgrade_request)
+        .expect_upgrade_failure();
+}

node/Cargo.toml

@@ -51,7 +51,7 @@ num-rational = { version = "0.4.0", features = ["serde"] }
 num-traits = "0.2.10"
 num_cpus = "1"
 once_cell = "1"
-openssl = "0.10.55"
+openssl = "0.10.66"
 pin-project = "1.0.6"
 prometheus = "0.12.0"
 quanta = "0.7.2"

node/src/components/binary_port.rs

@@ -1019,6 +1019,9 @@ where
         SpeculativeExecutionResult::WasmV1(spec_exec_result) => {
             BinaryResponse::from_value(spec_exec_result, protocol_version)
         }
+        SpeculativeExecutionResult::ReceivedV1Transaction => {
+            BinaryResponse::new_error(ErrorCode::ReceivedV1Transaction, protocol_version)
+        }
     }
 }
 

node/src/components/contract_runtime/operations.rs

@@ -1016,19 +1016,49 @@ where
             return SpeculativeExecutionResult::invalid_gas_limit(transaction);
         }
     };
-    let wasm_v1_result = match WasmV1Request::new_session(
-        *state_root_hash,
-        block_time.into(),
-        gas_limit,
-        &transaction,
-    ) {
-        Ok(wasm_v1_request) => execution_engine_v1.execute(state_provider, wasm_v1_request),
-        Err(error) => WasmV1Result::invalid_executable_item(gas_limit, error),
-    };
-    SpeculativeExecutionResult::WasmV1(utils::spec_exec_from_wasm_v1_result(
-        wasm_v1_result,
-        block_header.block_hash(),
-    ))
+
+    if transaction.is_legacy_transaction() {
+        if transaction.is_native() {
+            let limit = Gas::from(chainspec.system_costs_config.mint_costs().transfer);
+            let protocol_version = chainspec.protocol_version();
+            let native_runtime_config = NativeRuntimeConfig::from_chainspec(chainspec);
+            let transaction_hash = transaction.hash();
+            let initiator_addr = transaction.initiator_addr();
+            let authorization_keys = transaction.authorization_keys();
+            let runtime_args = transaction.session_args().clone();
+
+            let result = state_provider.transfer(TransferRequest::with_runtime_args(
+                native_runtime_config.clone(),
+                *state_root_hash,
+                protocol_version,
+                transaction_hash,
+                initiator_addr,
+                authorization_keys,
+                runtime_args,
+            ));
+            SpeculativeExecutionResult::WasmV1(utils::spec_exec_from_transfer_result(
+                limit,
+                result,
+                block_header.block_hash(),
+            ))
+        } else {
+            let wasm_v1_result = match WasmV1Request::new_session(
+                *state_root_hash,
+                block_time.into(),
+                gas_limit,
+                &transaction,
+            ) {
+                Ok(wasm_v1_request) => execution_engine_v1.execute(state_provider, wasm_v1_request),
+                Err(error) => WasmV1Result::invalid_executable_item(gas_limit, error),
+            };
+            SpeculativeExecutionResult::WasmV1(utils::spec_exec_from_wasm_v1_result(
+                wasm_v1_result,
+                block_header.block_hash(),
+            ))
+        }
+    } else {
+        SpeculativeExecutionResult::ReceivedV1Transaction
+    }
 }
 
 #[allow(clippy::too_many_arguments)]

node/src/components/contract_runtime/types.rs

@@ -373,6 +373,7 @@ pub struct BlockAndExecutionArtifacts {
 pub enum SpeculativeExecutionResult {
     InvalidTransaction(InvalidTransaction),
     WasmV1(casper_binary_port::SpeculativeExecutionResult),
+    ReceivedV1Transaction,
 }
 
 impl SpeculativeExecutionResult {

node/src/components/contract_runtime/utils.rs

@@ -31,10 +31,13 @@ use casper_execution_engine::engine_state::{ExecutionEngineV1, WasmV1Result};
 use casper_storage::{
     data_access_layer::{
         DataAccessLayer, FlushRequest, FlushResult, ProtocolUpgradeRequest, ProtocolUpgradeResult,
+        TransferResult,
     },
     global_state::state::{lmdb::LmdbGlobalState, CommitProvider, StateProvider},
 };
-use casper_types::{BlockHash, Chainspec, Digest, EraId, GasLimited, Key, ProtocolUpgradeConfig};
+use casper_types::{
+    BlockHash, Chainspec, Digest, EraId, Gas, GasLimited, Key, ProtocolUpgradeConfig,
+};
 
 /// Maximum number of resource intensive tasks that can be run in parallel.
 ///
@@ -511,6 +514,25 @@ pub(super) fn calculate_prune_eras(
     Some(range.map(EraId::new).map(Key::EraInfo).collect())
 }
 
+pub(crate) fn spec_exec_from_transfer_result(
+    limit: Gas,
+    transfer_result: TransferResult,
+    block_hash: BlockHash,
+) -> SpeculativeExecutionResult {
+    let transfers = transfer_result.transfers().to_owned();
+    let consumed = limit;
+    let effects = transfer_result.effects().to_owned();
+    let messages = vec![];
+    let error_msg = transfer_result
+        .error()
+        .to_owned()
+        .map(|err| format!("{:?}", err));
+
+    SpeculativeExecutionResult::new(
+        block_hash, transfers, limit, consumed, effects, messages, error_msg,
+    )
+}
+
 pub(crate) fn spec_exec_from_wasm_v1_result(
     wasm_v1_result: WasmV1Result,
     block_hash: BlockHash,

storage/src/data_access_layer/mint.rs

@@ -156,4 +156,19 @@ impl TransferResult {
             TransferResult::Success { effects, .. } => effects.clone(),
         }
     }
+
+    pub fn transfers(&self) -> Vec<Transfer> {
+        match self {
+            TransferResult::RootNotFound | TransferResult::Failure(_) => vec![],
+            TransferResult::Success { transfers, .. } => transfers.clone(),
+        }
+    }
+
+    pub fn error(&self) -> Option<TransferError> {
+        if let Self::Failure(error) = self {
+            Some(error.clone())
+        } else {
+            None
+        }
+    }
 }

storage/src/system/protocol_upgrade.rs

@@ -187,8 +187,8 @@ where
         self.handle_legacy_accounts_migration()?;
         self.handle_legacy_contracts_migration()?;
         self.handle_bids_migration(
-            self.config.maximum_delegation_amount(),
             self.config.minimum_delegation_amount(),
+            self.config.maximum_delegation_amount(),
         )?;
         self.handle_era_info_migration()
     }
@@ -950,6 +950,9 @@ where
         chainspec_minimum: u64,
         chainspec_maximum: u64,
     ) -> Result<(), ProtocolUpgradeError> {
+        if chainspec_maximum < chainspec_minimum {
+            return Err(ProtocolUpgradeError::InvalidUpgradeConfig);
+        }
         debug!("handle bids migration");
         let mut tc = self.tracking_copy.borrow_mut();
         let existing_bid_keys = match tc.get_keys(&KeyTag::Bid) {

storage/src/tracking_copy/ext_entity.rs

@@ -661,12 +661,6 @@ where
             let entity_hash = AddressableEntityHash::new(contract_hash.value());
             let entity_key = Key::contract_entity_key(entity_hash);
             self.write(entity_key, StoredValue::AddressableEntity(updated_entity));
-
-            // Prune the legacy records.
-            // Prune the legacy contract.
-            self.prune(Key::Hash(contract_hash.value()));
-            // Prune the legacy Wasm record.
-            self.prune(Key::Hash(contract_wasm_hash.value()));
         }
 
         let access_key_value = CLValue::from_t(access_uref).map_err(Self::Error::CLValue)?;

types/Cargo.toml

@@ -43,7 +43,7 @@ serde = { version = "1", default-features = false, features = ["alloc", "derive"
 serde_bytes = { version = "0.11.5", default-features = false, features = ["alloc"] }
 serde_json = { version = "1.0.59", default-features = false, features = ["alloc"] }
 strum = { version = "0.24", features = ["derive"], optional = true }
-thiserror = {version = "1", optional = true }
+thiserror = { version = "1", optional = true }
 tracing = { version = "0.1.37", default-features = false }
 uint = { version = "0.9.0", default-features = false }
 untrusted = { version = "0.7.1", optional = true }
@@ -58,7 +58,7 @@ derp = "0.0.14"
 getrandom = "0.2.0"
 humantime = "2"
 once_cell = "1.5.2"
-openssl = "0.10.55"
+openssl = "0.10.66"
 pem = "0.8.1"
 proptest = "1.0.0"
 proptest-derive = "0.3.0"

types/src/transaction.rs

@@ -385,6 +385,14 @@ impl Transaction {
         }
     }
 
+    /// Is the transaction the legacy deploy variant.
+    pub fn is_legacy_transaction(&self) -> bool {
+        match self {
+            Transaction::Deploy(_) => true,
+            Transaction::V1(_) => false,
+        }
+    }
+
     // This method is not intended to be used by third party crates.
     #[doc(hidden)]
     #[cfg(feature = "json-schema")]