Merge #4371

4371: Fix issue in `Key` and increase test coverage r=Fraser999 a=Fraser999 This fixes an issue in `Key::from_formatted_str` where `Key::Package` was not being handled. There are also several cleanups and further tests added. Closes #4363. Co-authored-by: Fraser Hutchison <fraser@casperlabs.io>
casper-network · Oct 23, 2023 · 9addc50 · 9addc50
2 parents a1cdc10 + f6b1fd4
commit 9addc50
Show file tree

Hide file tree

Showing 20 changed files with 340 additions and 302 deletions.
diff --git a/execution_engine/src/engine_state/execution_kind.rs b/execution_engine/src/engine_state/execution_kind.rs
@@ -88,7 +88,7 @@ impl ExecutionKind {
                 })?;
 
                 let entity_hash = match entity_key {
-                    Key::Hash(hash) | Key::AddressableEntity((_, hash)) => {
+                    Key::Hash(hash) | Key::AddressableEntity(_, hash) => {
                         AddressableEntityHash::new(hash)
                     }
                     _ => return Err(Error::InvalidKeyVariant),

diff --git a/execution_engine/src/engine_state/genesis.rs b/execution_engine/src/engine_state/genesis.rs
@@ -1173,13 +1173,13 @@ where
             package
         };
 
-        let byte_code_key = Key::ByteCode((ByteCodeKind::Empty, byte_code_hash.value()));
+        let byte_code_key = Key::ByteCode(ByteCodeKind::Empty, byte_code_hash.value());
 
         self.tracking_copy
             .borrow_mut()
             .write(byte_code_key, StoredValue::ByteCode(byte_code));
 
-        let entity_key = Key::AddressableEntity((package_kind.tag(), entity_hash.value()));
+        let entity_key = Key::AddressableEntity(package_kind.tag(), entity_hash.value());
 
         self.tracking_copy
             .borrow_mut()

diff --git a/execution_engine/src/engine_state/upgrade.rs b/execution_engine/src/engine_state/upgrade.rs
@@ -223,10 +223,10 @@ where
         if let Some(StoredValue::AddressableEntity(system_entity)) = self
             .tracking_copy
             .borrow_mut()
-            .read(&Key::AddressableEntity((
+            .read(&Key::AddressableEntity(
                 PackageKindTag::System,
                 contract_hash.value(),
-            )))
+            ))
             .map_err(|_| {
                 ProtocolUpgradeError::UnableToRetrieveSystemContract(
                     system_contract_type.to_string(),
@@ -315,7 +315,7 @@ where
             contract_package
         };
 
-        let byte_code_key = Key::ByteCode((ByteCodeKind::Empty, byte_code_hash.value()));
+        let byte_code_key = Key::ByteCode(ByteCodeKind::Empty, byte_code_hash.value());
         self.tracking_copy
             .borrow_mut()
             .write(byte_code_key, StoredValue::ByteCode(byte_code));

diff --git a/execution_engine/src/runtime/mod.rs b/execution_engine/src/runtime/mod.rs
@@ -1375,10 +1375,10 @@ where
 
             let byte_code_key = match package.get_package_kind() {
                 PackageKind::System(_) | PackageKind::Account(_) => {
-                    Key::ByteCode((ByteCodeKind::Empty, byte_code_addr))
+                    Key::ByteCode(ByteCodeKind::Empty, byte_code_addr)
                 }
                 PackageKind::SmartContract => {
-                    Key::ByteCode((ByteCodeKind::V1CasperWasm, byte_code_addr))
+                    Key::ByteCode(ByteCodeKind::V1CasperWasm, byte_code_addr)
                 }
             };
 
@@ -1711,14 +1711,14 @@ where
             entity.update_session_entity(ByteCodeHash::new(byte_code_hash), entry_points);
 
         self.context.metered_write_gs_unsafe(
-            Key::ByteCode((ByteCodeKind::V1CasperWasm, byte_code_hash)),
+            Key::ByteCode(ByteCodeKind::V1CasperWasm, byte_code_hash),
             byte_code,
         )?;
 
         let package_kind = package.get_package_kind();
 
         self.context.metered_write_gs_unsafe(
-            Key::AddressableEntity((package_kind.tag(), entity_hash.value())),
+            Key::AddressableEntity(package_kind.tag(), entity_hash.value()),
             updated_session_entity,
         )?;
 
@@ -1773,11 +1773,11 @@ where
         };
 
         self.context.metered_write_gs_unsafe(
-            Key::ByteCode((ByteCodeKind::V1CasperWasm, byte_code_hash)),
+            Key::ByteCode(ByteCodeKind::V1CasperWasm, byte_code_hash),
             byte_code,
         )?;
 
-        let entity_key = Key::AddressableEntity((PackageKindTag::SmartContract, entity_hash));
+        let entity_key = Key::AddressableEntity(PackageKindTag::SmartContract, entity_hash);
 
         let entity = AddressableEntity::new(
             package_hash,
@@ -3303,8 +3303,7 @@ where
                 self.context
                     .metered_write_gs_unsafe(byte_code_key, StoredValue::ByteCode(byte_code))?;
 
-                let entity_key =
-                    Key::AddressableEntity((package_kind.tag(), contract_hash.value()));
+                let entity_key = Key::AddressableEntity(package_kind.tag(), contract_hash.value());
 
                 self.context
                     .metered_write_gs_unsafe(entity_key, updated_entity.clone())?;

diff --git a/execution_engine/src/runtime_context/mod.rs b/execution_engine/src/runtime_context/mod.rs
@@ -711,15 +711,15 @@ where
             | Key::ChecksumRegistry
             | Key::BidAddr(_)
             | Key::Package(_)
-            | Key::AddressableEntity(_)
-            | Key::ByteCode(_) => true,
+            | Key::AddressableEntity(..)
+            | Key::ByteCode(..) => true,
         }
     }
 
     /// Tests whether addition to `key` is valid.
     pub fn is_addable(&self, key: &Key) -> bool {
         match key {
-            Key::AddressableEntity((_, entity_addr)) => {
+            Key::AddressableEntity(_, entity_addr) => {
                 match self.get_entity_key().into_entity_hash() {
                     Some(entity_hash) => entity_hash == AddressableEntityHash::new(*entity_addr),
                     None => false,
@@ -742,7 +742,7 @@ where
             | Key::ChecksumRegistry
             | Key::BidAddr(_)
             | Key::Package(_)
-            | Key::ByteCode(_) => false,
+            | Key::ByteCode(..) => false,
         }
     }
 
@@ -766,8 +766,8 @@ where
             | Key::ChecksumRegistry
             | Key::BidAddr(_)
             | Key::Package(_)
-            | Key::AddressableEntity(_)
-            | Key::ByteCode(_) => false,
+            | Key::AddressableEntity(..)
+            | Key::ByteCode(..) => false,
         }
     }
 

diff --git a/execution_engine/src/runtime_context/tests.rs b/execution_engine/src/runtime_context/tests.rs
@@ -116,7 +116,7 @@ fn random_account_key<G: RngCore>(entropy_source: &mut G) -> Key {
 fn random_contract_key<G: RngCore>(entropy_source: &mut G) -> Key {
     let mut key_hash = [0u8; 32];
     entropy_source.fill_bytes(&mut key_hash);
-    Key::AddressableEntity((PackageKindTag::SmartContract, key_hash))
+    Key::AddressableEntity(PackageKindTag::SmartContract, key_hash)
 }
 
 // Create URef Key.

diff --git a/execution_engine/src/tracking_copy/tests.rs b/execution_engine/src/tracking_copy/tests.rs
@@ -451,7 +451,7 @@ proptest! {
             AssociatedKeys::default(),
             ActionThresholds::default(),
         ));
-        let contract_key = Key::AddressableEntity((PackageKindTag::SmartContract,hash));
+        let contract_key = Key::AddressableEntity(PackageKindTag::SmartContract,hash);
 
         // create account which knows about contract
         let mut account_named_keys = NamedKeys::new();

diff --git a/execution_engine_testing/tests/src/test/contract_api/dictionary.rs b/execution_engine_testing/tests/src/test/contract_api/dictionary.rs
@@ -104,7 +104,7 @@ fn query_dictionary_item(
                 return Err("Provided base key is not an account".to_string());
             }
         }
-        Key::AddressableEntity(_) => {
+        Key::AddressableEntity(..) => {
             if let Some(name) = dictionary_name {
                 let stored_value = builder.query(None, key, &[])?;
 

diff --git a/execution_engine_testing/tests/src/test/explorer/faucet.rs b/execution_engine_testing/tests/src/test/explorer/faucet.rs
@@ -863,9 +863,8 @@ fn should_allow_funding_by_an_authorized_account() {
 #[test]
 fn faucet_costs() {
     // This test will fail if execution costs vary.  The expected costs should not be updated
-    // without understanding why the cost has changed.  If the costs do change, it should be
-    // reflected in the "Costs by Entry Point" section of the faucet crate's README.md.
-    const EXPECTED_FAUCET_INSTALL_COST: u64 = 84_201_467_790;
+    // without understanding why the cost has changed.
+    const EXPECTED_FAUCET_INSTALL_COST: u64 = 84_201_467_720;
     const EXPECTED_FAUCET_SET_VARIABLES_COST: u64 = 650_487_100;
     const EXPECTED_FAUCET_CALL_BY_INSTALLER_COST: u64 = 3_247_573_380;
     const EXPECTED_FAUCET_CALL_BY_USER_COST: u64 = 3_368_370_660;

diff --git a/execution_engine_testing/tests/src/test/regression/regression_20211110.rs b/execution_engine_testing/tests/src/test/regression/regression_20211110.rs
@@ -64,7 +64,7 @@ fn regression_20211110() {
         .get(CONTRACT_HASH_NAME)
         .unwrap()
     {
-        Key::AddressableEntity((_, addr)) => AddressableEntityHash::new(*addr),
+        Key::AddressableEntity(_, addr) => AddressableEntityHash::new(*addr),
         _ => panic!("Couldn't find regression contract."),
     };
 

diff --git a/execution_engine_testing/tests/src/test/regression/transforms_must_be_ordered.rs b/execution_engine_testing/tests/src/test/regression/transforms_must_be_ordered.rs
@@ -41,7 +41,7 @@ fn contract_transforms_should_be_ordered_in_the_effects() {
         .get("ordered-transforms-contract-hash")
         .unwrap()
     {
-        Key::AddressableEntity((_package_kind, addr)) => AddressableEntityHash::new(*addr),
+        Key::AddressableEntity(_package_kind, addr) => AddressableEntityHash::new(*addr),
         _ => panic!("Couldn't find ordered-transforms contract."),
     };
 

diff --git a/node/src/components/transaction_acceptor/tests.rs b/node/src/components/transaction_acceptor/tests.rs
@@ -753,7 +753,7 @@ impl reactor::Reactor for Reactor {
                     } else if let Key::Account(account_hash) = key {
                         let account = create_account(account_hash, self.test_scenario);
                         Some(AddressableEntity::from(account))
-                    } else if let Key::AddressableEntity(_) = key {
+                    } else if let Key::AddressableEntity(..) = key {
                         match self.test_scenario {
                             TestScenario::FromPeerCustomPaymentContract(
                                 ContractScenario::MissingContractAtHash,

diff --git a/resources/test/rpc_schema.json b/resources/test/rpc_schema.json
@@ -4714,14 +4714,14 @@
             ]
           },
           "ByteCode": {
-            "description": "A container for contract's WASM bytes.",
+            "description": "A container for contract's Wasm bytes.",
             "type": "object",
             "required": [
-              "byte_code_kind",
-              "bytes"
+              "bytes",
+              "kind"
             ],
             "properties": {
-              "byte_code_kind": {
+              "kind": {
                 "$ref": "#/components/schemas/ByteCodeKind"
               },
               "bytes": {

diff --git a/smart_contracts/contracts/explorer/faucet/README.md b/smart_contracts/contracts/explorer/faucet/README.md
@@ -30,12 +30,3 @@ After an interval passes after then last user was funded, the available amount w
 `distributions_per_interval`, `available_amount`, `time_interval` and `max_distributions_per_interval`
 must be set and must be a number greater than `0` for the contract to run properly.
 If you try to invoke the contract before these variables are set, then you'll get an error.
-
-### Costs by Entry Point
-
-| feature                  | cost             |
-|--------------------------|------------------|
-| faucet install           | `84_007_229_270` |
-| faucet set variables     | `648_705_070`    |
-| faucet call by installer | `3_245_605_770`  |
-| faucet call by user      | `3_364_807_470`  |
diff --git a/smart_contracts/contracts/tutorial/increment-counter/src/main.rs b/smart_contracts/contracts/tutorial/increment-counter/src/main.rs
@@ -19,7 +19,7 @@ pub extern "C" fn call() {
     // Read the Counter smart contract's ContractHash.
     let contract_hash = {
         let counter_uref = runtime::get_key(COUNTER_KEY).unwrap_or_revert_with(ApiError::GetKey);
-        if let Key::AddressableEntity((_, hash)) = counter_uref {
+        if let Key::AddressableEntity(_, hash) = counter_uref {
             AddressableEntityHash::new(hash)
         } else {
             runtime::revert(ApiError::User(66));

diff --git a/types/src/addressable_entity.rs b/types/src/addressable_entity.rs
@@ -329,7 +329,7 @@ impl TryFrom<Key> for AddressableEntityHash {
     type Error = ApiError;
 
     fn try_from(value: Key) -> Result<Self, Self::Error> {
-        if let Key::AddressableEntity((_, entity_addr)) = value {
+        if let Key::AddressableEntity(_, entity_addr) = value {
             Ok(AddressableEntityHash::new(entity_addr))
         } else {
             Err(ApiError::Formatting)