Merge #4969

4969: Fixed bug which caused calling entry points for smart contracts deplo… r=zajko a=zajko

…yed as "Transaction::Deploy" (or created in v1.x) to always fail. The fix involves changing the way we fetch entry point data in DataAccessLayer. If entry point is not found under Key::EntryPoint we fallback to see if there is a contract entry for the given contract hash. If a Contract entry exists, we query it's `entry_points` field.

Co-authored-by: Jakub Zajkowski <jakub@casperlabs.io>

node/src/components/contract_runtime.rs

@@ -32,7 +32,7 @@ use casper_execution_engine::engine_state::{EngineConfigBuilder, ExecutionEngine
 use casper_storage::{
     data_access_layer::{
         AddressableEntityRequest, AddressableEntityResult, BlockStore, DataAccessLayer,
-        EntryPointsRequest, ExecutionResultsChecksumRequest, FlushRequest, FlushResult,
+        EntryPointExistsRequest, ExecutionResultsChecksumRequest, FlushRequest, FlushResult,
         GenesisRequest, GenesisResult, TrieRequest,
     },
     global_state::{
@@ -452,18 +452,23 @@ impl ContractRuntime {
                 }
                 .ignore()
             }
-            ContractRuntimeRequest::GetEntryPoint {
+            ContractRuntimeRequest::GetEntryPointExists {
                 state_root_hash,
-                key,
+                contract_hash,
+                entry_point_name,
                 responder,
             } => {
                 trace!(?state_root_hash, "get entry point");
                 let metrics = Arc::clone(&self.metrics);
                 let data_access_layer = Arc::clone(&self.data_access_layer);
                 async move {
                     let start = Instant::now();
-                    let request = EntryPointsRequest::new(state_root_hash, key);
-                    let result = data_access_layer.entry_point(request);
+                    let request = EntryPointExistsRequest::new(
+                        state_root_hash,
+                        entry_point_name,
+                        contract_hash,
+                    );
+                    let result = data_access_layer.entry_point_exists(request);
                     metrics.entry_points.observe(start.elapsed().as_secs_f64());
                     trace!(?result, "get addressable entity");
                     responder.respond(result).await

node/src/components/contract_runtime/operations.rs

@@ -17,7 +17,7 @@ use casper_storage::{
         mint::BalanceIdentifierTransferArgs,
         AuctionMethod, BalanceHoldKind, BalanceHoldRequest, BalanceIdentifier, BalanceRequest,
         BiddingRequest, BlockGlobalRequest, BlockGlobalResult, BlockRewardsRequest,
-        BlockRewardsResult, DataAccessLayer, EntryPointsRequest, EntryPointsResult,
+        BlockRewardsResult, DataAccessLayer, EntryPointRequest, EntryPointResult,
         EraValidatorsRequest, EraValidatorsResult, EvictItem, FeeRequest, FeeResult, FlushRequest,
         HandleFeeMode, HandleFeeRequest, HandleRefundMode, HandleRefundRequest,
         InsufficientBalanceHandling, ProofHandling, PruneRequest, PruneResult, StepRequest,
@@ -28,16 +28,14 @@ use casper_storage::{
         StateProvider, StateReader,
     },
     system::runtime_native::Config as NativeRuntimeConfig,
-    tracking_copy::TrackingCopyError,
 };
 use casper_types::{
     bytesrepr::{self, ToBytes, U32_SERIALIZED_LENGTH},
     execution::{Effects, ExecutionResult, TransformKindV2, TransformV2},
     system::handle_payment::ARG_AMOUNT,
     BlockHash, BlockHeader, BlockTime, BlockV2, CLValue, Chainspec, ChecksumRegistry, Digest,
-    EntityAddr, EntryPointAddr, EraEndV2, EraId, FeeHandling, Gas, InvalidTransaction,
-    InvalidTransactionV1, Key, ProtocolVersion, PublicKey, RefundHandling, Transaction,
-    AUCTION_LANE_ID, MINT_LANE_ID, U512,
+    EntityAddr, EraEndV2, EraId, FeeHandling, Gas, InvalidTransaction, InvalidTransactionV1, Key,
+    ProtocolVersion, PublicKey, RefundHandling, Transaction, AUCTION_LANE_ID, MINT_LANE_ID, U512,
 };
 
 use super::{
@@ -1251,19 +1249,13 @@ fn invoked_contract_will_pay(
         Some((hash_addr, entry_point_name)) => (hash_addr, entry_point_name),
     };
     let entity_addr = EntityAddr::new_smart_contract(hash_addr);
-    let entry_point_addr =
-        match EntryPointAddr::new_v1_entry_point_addr(entity_addr, &entry_point_name) {
-            Ok(addr) => addr,
-            Err(bre) => return Err(StateResultError::Failure(TrackingCopyError::BytesRepr(bre))),
-        };
-    let entry_point_key = Key::entry_point(entry_point_addr);
-    let entry_point_request = EntryPointsRequest::new(state_root_hash, entry_point_key);
+    let entry_point_request = EntryPointRequest::new(state_root_hash, entry_point_name, hash_addr);
     let entry_point_response = state_provider.entry_point(entry_point_request);
     match entry_point_response {
-        EntryPointsResult::RootNotFound => Err(StateResultError::RootNotFound),
-        EntryPointsResult::ValueNotFound(msg) => Err(StateResultError::ValueNotFound(msg)),
-        EntryPointsResult::Failure(tce) => Err(StateResultError::Failure(tce)),
-        EntryPointsResult::Success { entry_point } => {
+        EntryPointResult::RootNotFound => Err(StateResultError::RootNotFound),
+        EntryPointResult::ValueNotFound(msg) => Err(StateResultError::ValueNotFound(msg)),
+        EntryPointResult::Failure(tce) => Err(StateResultError::Failure(tce)),
+        EntryPointResult::Success { entry_point } => {
             if entry_point.will_pay_direct_invocation() {
                 Ok(Some(entity_addr))
             } else {

node/src/components/contract_runtime/tests.rs

@@ -397,24 +397,31 @@ async fn should_not_set_shared_pre_state_to_lower_block_height() {
 }
 
 #[cfg(test)]
-mod trie_chunking_tests {
+mod test_mod {
     use std::sync::Arc;
 
     use prometheus::Registry;
+    use rand::Rng;
     use tempfile::tempdir;
 
-    use casper_storage::global_state::{
-        state::{CommitProvider, StateProvider},
-        trie::Trie,
+    use casper_storage::{
+        data_access_layer::{EntryPointExistsRequest, EntryPointExistsResult},
+        global_state::{
+            state::{CommitProvider, StateProvider},
+            trie::Trie,
+        },
     };
     use casper_types::{
         account::AccountHash,
         bytesrepr,
+        contracts::{ContractPackageHash, EntryPoint, EntryPoints},
         execution::{TransformKindV2, TransformV2},
         global_state::Pointer,
         testing::TestRng,
-        ActivationPoint, CLValue, Chainspec, ChunkWithProof, CoreConfig, Digest, EraId, Key,
-        ProtocolConfig, StoredValue, TimeDiff, DEFAULT_FEE_HANDLING, DEFAULT_GAS_HOLD_INTERVAL,
+        ActivationPoint, CLType, CLValue, Chainspec, ChunkWithProof, Contract, ContractWasmHash,
+        CoreConfig, Digest, EntityAddr, EntryPointAccess, EntryPointAddr, EntryPointPayment,
+        EntryPointType, EntryPointValue, EraId, HashAddr, Key, NamedKeys, ProtocolConfig,
+        ProtocolVersion, StoredValue, TimeDiff, DEFAULT_FEE_HANDLING, DEFAULT_GAS_HOLD_INTERVAL,
         DEFAULT_REFUND_HANDLING,
     };
 
@@ -428,14 +435,68 @@ mod trie_chunking_tests {
     #[derive(Debug, Clone)]
     struct TestPair(Key, StoredValue);
 
+    fn create_pre_condor_contract(
+        rng: &mut TestRng,
+        contract_hash: Key,
+        entry_point_name: &str,
+        protocol_version: ProtocolVersion,
+    ) -> Vec<TestPair> {
+        let mut entry_points = EntryPoints::new();
+        let entry_point = EntryPoint::new(
+            entry_point_name,
+            vec![],
+            CLType::Unit,
+            EntryPointAccess::Public,
+            EntryPointType::Caller,
+        );
+        entry_points.add_entry_point(entry_point);
+
+        let contract_package_hash = ContractPackageHash::new(rng.gen());
+        let contract_wasm_hash = ContractWasmHash::new(rng.gen());
+        let named_keys = NamedKeys::new();
+        let contract = Contract::new(
+            contract_package_hash,
+            contract_wasm_hash,
+            named_keys,
+            entry_points,
+            protocol_version,
+        );
+        vec![TestPair(contract_hash, StoredValue::Contract(contract))]
+    }
+
+    fn create_entry_point(entity_addr: EntityAddr, entry_point_name: &str) -> Vec<TestPair> {
+        let mut entry_points = EntryPoints::new();
+        let entry_point = EntryPoint::new(
+            entry_point_name,
+            vec![],
+            CLType::Unit,
+            EntryPointAccess::Public,
+            EntryPointType::Caller,
+        );
+        entry_points.add_entry_point(entry_point);
+        let key = Key::EntryPoint(
+            EntryPointAddr::new_v1_entry_point_addr(entity_addr, entry_point_name).unwrap(),
+        );
+        let entry_point = casper_types::EntryPoint::new(
+            entry_point_name,
+            vec![],
+            CLType::Unit,
+            EntryPointAccess::Public,
+            EntryPointType::Caller,
+            EntryPointPayment::Caller,
+        );
+        let entry_point_value = EntryPointValue::V1CasperVm(entry_point);
+        vec![TestPair(key, StoredValue::EntryPoint(entry_point_value))]
+    }
+
     // Creates the test pairs that contain data of size
     // greater than the chunk limit.
-    fn create_test_pairs_with_large_data() -> [TestPair; 2] {
+    fn create_test_pairs_with_large_data() -> Vec<TestPair> {
         let val = CLValue::from_t(
             String::from_utf8(vec![b'a'; ChunkWithProof::CHUNK_SIZE_BYTES * 2]).unwrap(),
         )
         .unwrap();
-        [
+        vec![
             TestPair(
                 Key::Account(AccountHash::new([1_u8; 32])),
                 StoredValue::CLValue(val.clone()),
@@ -472,7 +533,7 @@ mod trie_chunking_tests {
 
     // Creates a test ContractRuntime and feeds the underlying GlobalState with `test_pair`.
     // Returns [`ContractRuntime`] instance and the new Merkle root after applying the `test_pair`.
-    fn create_test_state(rng: &mut TestRng, test_pair: [TestPair; 2]) -> (ContractRuntime, Digest) {
+    fn create_test_state(rng: &mut TestRng, test_pair: Vec<TestPair>) -> (ContractRuntime, Digest) {
         let temp_dir = tempdir().unwrap();
         let chainspec = Chainspec {
             protocol_config: ProtocolConfig {
@@ -531,6 +592,57 @@ mod trie_chunking_tests {
         }
     }
 
+    #[test]
+    fn fetching_enty_points_falls_back_to_contract() {
+        let rng = &mut TestRng::new();
+        let hash_addr: HashAddr = rng.gen();
+        let contract_hash = Key::Hash(hash_addr);
+        let entry_point_name = "ep1";
+        let initial_state = create_pre_condor_contract(
+            rng,
+            contract_hash,
+            entry_point_name,
+            ProtocolVersion::V2_0_0,
+        );
+        let (contract_runtime, state_hash) = create_test_state(rng, initial_state);
+        let request =
+            EntryPointExistsRequest::new(state_hash, entry_point_name.to_string(), hash_addr);
+        let res = contract_runtime
+            .data_access_layer()
+            .entry_point_exists(request);
+        assert!(matches!(res, EntryPointExistsResult::Success));
+    }
+
+    #[test]
+    fn fetching_enty_points_fetches_entry_point_from_v2() {
+        let rng = &mut TestRng::new();
+        let hash_addr: HashAddr = rng.gen();
+        let entity_addr = EntityAddr::new_smart_contract(hash_addr);
+        let entry_point_name = "ep1";
+        let initial_state = create_entry_point(entity_addr, entry_point_name);
+        let (contract_runtime, state_hash) = create_test_state(rng, initial_state);
+        let request =
+            EntryPointExistsRequest::new(state_hash, entry_point_name.to_string(), hash_addr);
+        let res = contract_runtime
+            .data_access_layer()
+            .entry_point_exists(request);
+        assert!(matches!(res, EntryPointExistsResult::Success));
+    }
+
+    #[test]
+    fn fetching_enty_points_fetches_fail_when_asking_for_non_existing() {
+        let rng = &mut TestRng::new();
+        let hash_addr: HashAddr = rng.gen();
+        let entity_addr = EntityAddr::new_smart_contract(hash_addr);
+        let initial_state = create_entry_point(entity_addr, "ep1");
+        let (contract_runtime, state_hash) = create_test_state(rng, initial_state);
+        let request = EntryPointExistsRequest::new(state_hash, "ep2".to_string(), hash_addr);
+        let res = contract_runtime
+            .data_access_layer()
+            .entry_point_exists(request);
+        assert!(matches!(res, EntryPointExistsResult::ValueNotFound { .. }));
+    }
+
     #[test]
     fn returns_trie_or_chunk() {
         let rng = &mut TestRng::new();

node/src/components/transaction_acceptor.rs

@@ -16,7 +16,7 @@ use casper_storage::data_access_layer::{balance::BalanceHandling, BalanceRequest
 use casper_types::{
     account::AccountHash, addressable_entity::AddressableEntity, system::auction::ARG_AMOUNT,
     AddressableEntityHash, AddressableEntityIdentifier, BlockHeader, Chainspec, EntityAddr,
-    EntityKind, EntityVersion, EntityVersionKey, EntryPoint, EntryPointAddr, ExecutableDeployItem,
+    EntityKind, EntityVersion, EntityVersionKey, ExecutableDeployItem,
     ExecutableDeployItemIdentifier, InitiatorAddr, Key, Package, PackageAddr, PackageHash,
     PackageIdentifier, Timestamp, Transaction, TransactionEntryPoint, TransactionInvocationTarget,
     TransactionRuntime, TransactionTarget, DEFAULT_ENTRY_POINT_NAME, U512,
@@ -621,33 +621,20 @@ impl TransactionAcceptor {
         };
 
         match maybe_entry_point_name {
-            Some(entry_point_name) => {
-                match EntryPointAddr::new_v1_entry_point_addr(
-                    EntityAddr::SmartContract(contract_hash.value()),
-                    &entry_point_name,
-                ) {
-                    Ok(entry_point_addr) => effect_builder
-                        .get_entry_point_value(
-                            *block_header.state_root_hash(),
-                            Key::EntryPoint(entry_point_addr),
-                        )
-                        .event(move |entry_point_result| Event::GetEntryPointResult {
-                            event_metadata,
-                            block_header,
-                            is_payment,
-                            entry_point_name,
-                            addressable_entity,
-                            maybe_entry_point: entry_point_result.into_v1_entry_point(),
-                        }),
-                    Err(_) => {
-                        let error = Error::parameter_failure(
-                            &block_header,
-                            ParameterFailure::NoSuchEntryPoint { entry_point_name },
-                        );
-                        self.reject_transaction(effect_builder, *event_metadata, error)
-                    }
-                }
-            }
+            Some(entry_point_name) => effect_builder
+                .does_entry_point_exist(
+                    *block_header.state_root_hash(),
+                    contract_hash.value(),
+                    entry_point_name.clone(),
+                )
+                .event(move |entry_point_result| Event::GetEntryPointResult {
+                    event_metadata,
+                    block_header,
+                    is_payment,
+                    entry_point_name,
+                    addressable_entity,
+                    entry_point_exists: entry_point_result.is_some(),
+                }),
 
             None => {
                 if is_payment {
@@ -667,31 +654,19 @@ impl TransactionAcceptor {
         is_payment: bool,
         entry_point_name: String,
         addressable_entity: AddressableEntity,
-        maybe_entry_point: Option<EntryPoint>,
+        entry_point_exist: bool,
     ) -> Effects<Event> {
         match addressable_entity.kind() {
             EntityKind::SmartContract(TransactionRuntime::VmCasperV1)
             | EntityKind::Account(_)
             | EntityKind::System(_) => {
-                let entry_point = match maybe_entry_point {
-                    Some(entry_point) => entry_point,
-                    None => {
-                        let error = Error::parameter_failure(
-                            &block_header,
-                            ParameterFailure::NoSuchEntryPoint { entry_point_name },
-                        );
-                        return self.reject_transaction(effect_builder, *event_metadata, error);
-                    }
-                };
-
-                if entry_point_name != *entry_point.name() {
+                if !entry_point_exist {
                     let error = Error::parameter_failure(
                         &block_header,
                         ParameterFailure::NoSuchEntryPoint { entry_point_name },
                     );
                     return self.reject_transaction(effect_builder, *event_metadata, error);
-                };
-
+                }
                 if is_payment {
                     return self.verify_body(effect_builder, event_metadata, block_header);
                 }
@@ -1027,15 +1002,15 @@ impl<REv: ReactorEventT> Component<REv> for TransactionAcceptor {
                 is_payment,
                 entry_point_name,
                 addressable_entity,
-                maybe_entry_point,
+                entry_point_exists,
             } => self.handle_get_entry_point_result(
                 effect_builder,
                 event_metadata,
                 block_header,
                 is_payment,
                 entry_point_name,
                 addressable_entity,
-                maybe_entry_point,
+                entry_point_exists,
             ),
             Event::PutToStorageResult {
                 event_metadata,

node/src/components/transaction_acceptor/event.rs

@@ -3,8 +3,8 @@ use std::fmt::{self, Display, Formatter};
 use serde::Serialize;
 
 use casper_types::{
-    AddressableEntity, AddressableEntityHash, BlockHeader, EntityVersion, EntryPoint, Package,
-    PackageHash, Timestamp, Transaction, U512,
+    AddressableEntity, AddressableEntityHash, BlockHeader, EntityVersion, Package, PackageHash,
+    Timestamp, Transaction, U512,
 };
 
 use super::{Error, Source};
@@ -102,7 +102,7 @@ pub(crate) enum Event {
         is_payment: bool,
         entry_point_name: String,
         addressable_entity: AddressableEntity,
-        maybe_entry_point: Option<EntryPoint>,
+        entry_point_exists: bool,
     },
 }