Merge #4690

4690: Remove balance by block request r=jacek-casper a=jacek-casper The request was initially added because we were not able to determine the block timestamp necessary for holds lookup without knowing the associated block. Since then, that has changed and we now can always determine the timestamp, which makes the block request redundant - its behavior can be reproduced with the `BalanceByStateRoot` request in all cases, because the state root identifier type already allows passing through block hash and height. In this PR I remove `BalanceByBlock` and rename `BalanceByStateRoot` to just `Balance`. Co-authored-by: Jacek Malec <[email protected]>
casper-network · May 3, 2024 · 7317868 · 7317868
2 parents 8a56be6 + 6d30a8e
commit 7317868
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 93 deletions.
diff --git a/binary_port/src/state_request.rs b/binary_port/src/state_request.rs
@@ -5,7 +5,7 @@ use rand::Rng;
 
 use casper_types::{
     bytesrepr::{self, FromBytes, ToBytes, U8_SERIALIZED_LENGTH},
-    BlockIdentifier, Digest, GlobalStateIdentifier, Key, KeyTag,
+    Digest, GlobalStateIdentifier, Key, KeyTag,
 };
 
 use crate::PurseIdentifier;
@@ -16,8 +16,7 @@ const ITEM_TAG: u8 = 0;
 const ALL_ITEMS_TAG: u8 = 1;
 const TRIE_TAG: u8 = 2;
 const DICTIONARY_ITEM_TAG: u8 = 3;
-const BALANCE_BY_BLOCK_TAG: u8 = 4;
-const BALANCE_BY_STATE_ROOT_TAG: u8 = 5;
+const BALANCE_TAG: u8 = 4;
 
 /// A request to get data from the global state.
 #[derive(Clone, Debug, PartialEq)]
@@ -50,15 +49,8 @@ pub enum GlobalStateRequest {
         /// Dictionary item identifier.
         identifier: DictionaryItemIdentifier,
     },
-    /// Get balance by block and purse.
-    BalanceByBlock {
-        /// Block identifier, `None` means "latest block".
-        block_identifier: Option<BlockIdentifier>,
-        /// Purse identifier.
-        purse_identifier: PurseIdentifier,
-    },
     /// Get balance by state root and purse.
-    BalanceByStateRoot {
+    Balance {
         /// Global state identifier, `None` means "latest block state".
         state_identifier: Option<GlobalStateIdentifier>,
         /// Purse identifier.
@@ -97,11 +89,7 @@ impl GlobalStateRequest {
                     .then(|| GlobalStateIdentifier::random(rng)),
                 identifier: DictionaryItemIdentifier::random(rng),
             },
-            BALANCE_BY_BLOCK_TAG => GlobalStateRequest::BalanceByBlock {
-                block_identifier: rng.gen::<bool>().then(|| BlockIdentifier::random(rng)),
-                purse_identifier: PurseIdentifier::random(rng),
-            },
-            BALANCE_BY_STATE_ROOT_TAG => GlobalStateRequest::BalanceByStateRoot {
+            BALANCE_TAG => GlobalStateRequest::Balance {
                 state_identifier: rng
                     .gen::<bool>()
                     .then(|| GlobalStateIdentifier::random(rng)),
@@ -151,19 +139,11 @@ impl ToBytes for GlobalStateRequest {
                 state_identifier.write_bytes(writer)?;
                 identifier.write_bytes(writer)
             }
-            GlobalStateRequest::BalanceByBlock {
-                block_identifier,
-                purse_identifier,
-            } => {
-                BALANCE_BY_BLOCK_TAG.write_bytes(writer)?;
-                block_identifier.write_bytes(writer)?;
-                purse_identifier.write_bytes(writer)
-            }
-            GlobalStateRequest::BalanceByStateRoot {
+            GlobalStateRequest::Balance {
                 state_identifier,
                 purse_identifier,
             } => {
-                BALANCE_BY_STATE_ROOT_TAG.write_bytes(writer)?;
+                BALANCE_TAG.write_bytes(writer)?;
                 state_identifier.write_bytes(writer)?;
                 purse_identifier.write_bytes(writer)
             }
@@ -191,11 +171,7 @@ impl ToBytes for GlobalStateRequest {
                     state_identifier,
                     identifier,
                 } => state_identifier.serialized_length() + identifier.serialized_length(),
-                GlobalStateRequest::BalanceByBlock {
-                    block_identifier,
-                    purse_identifier,
-                } => block_identifier.serialized_length() + purse_identifier.serialized_length(),
-                GlobalStateRequest::BalanceByStateRoot {
+                GlobalStateRequest::Balance {
                     state_identifier,
                     purse_identifier,
                 } => state_identifier.serialized_length() + purse_identifier.serialized_length(),
@@ -246,22 +222,11 @@ impl FromBytes for GlobalStateRequest {
                     remainder,
                 ))
             }
-            BALANCE_BY_BLOCK_TAG => {
-                let (block_identifier, remainder) = FromBytes::from_bytes(remainder)?;
-                let (purse_identifier, remainder) = FromBytes::from_bytes(remainder)?;
-                Ok((
-                    GlobalStateRequest::BalanceByBlock {
-                        block_identifier,
-                        purse_identifier,
-                    },
-                    remainder,
-                ))
-            }
-            BALANCE_BY_STATE_ROOT_TAG => {
+            BALANCE_TAG => {
                 let (state_identifier, remainder) = FromBytes::from_bytes(remainder)?;
                 let (purse_identifier, remainder) = FromBytes::from_bytes(remainder)?;
                 Ok((
-                    GlobalStateRequest::BalanceByStateRoot {
+                    GlobalStateRequest::Balance {
                         state_identifier,
                         purse_identifier,
                     },

diff --git a/node/src/components/binary_port.rs b/node/src/components/binary_port.rs
@@ -398,22 +398,7 @@ where
                 Err(err) => BinaryResponse::new_error(err, protocol_version),
             }
         }
-        GlobalStateRequest::BalanceByBlock {
-            block_identifier,
-            purse_identifier,
-        } => {
-            let Some(header) = resolve_block_header(effect_builder, block_identifier).await else {
-                return BinaryResponse::new_empty(protocol_version);
-            };
-            get_balance(
-                effect_builder,
-                *header.state_root_hash(),
-                purse_identifier,
-                protocol_version,
-            )
-            .await
-        }
-        GlobalStateRequest::BalanceByStateRoot {
+        GlobalStateRequest::Balance {
             state_identifier,
             purse_identifier,
         } => {

diff --git a/node/src/components/binary_port/event.rs b/node/src/components/binary_port/event.rs
@@ -51,10 +51,7 @@ impl Display for Event {
                         GlobalStateRequest::DictionaryItem { .. } => {
                             write!(f, "get dictionary item")
                         }
-                        GlobalStateRequest::BalanceByBlock { .. } => {
-                            write!(f, "get balance by block")
-                        }
-                        GlobalStateRequest::BalanceByStateRoot { .. } => {
+                        GlobalStateRequest::Balance { .. } => {
                             write!(f, "get balance by state root",)
                         }
                     },

diff --git a/node/src/reactor/main_reactor/tests/binary_port.rs b/node/src/reactor/main_reactor/tests/binary_port.rs
@@ -338,8 +338,7 @@ async fn binary_port_component() {
         try_spec_exec_invalid(&mut rng),
         try_accept_transaction_invalid(&mut rng),
         try_accept_transaction(&secret_signing_key),
-        get_balance_by_block(*highest_block.hash()),
-        get_balance_by_state_root(state_root_hash, test_account_hash),
+        get_balance(state_root_hash, test_account_hash),
     ];
 
     for TestCase {
@@ -834,34 +833,13 @@ fn get_dictionary_item_by_named_key(
     }
 }
 
-fn get_balance_by_block(block_hash: BlockHash) -> TestCase {
+fn get_balance(state_root_hash: Digest, account_hash: AccountHash) -> TestCase {
     TestCase {
-        name: "get_balance_by_block",
-        request: BinaryRequest::Get(GetRequest::State(Box::new(
-            GlobalStateRequest::BalanceByBlock {
-                block_identifier: Some(BlockIdentifier::Hash(block_hash)),
-                purse_identifier: PurseIdentifier::Payment,
-            },
-        ))),
-        asserter: Box::new(|response| {
-            assert_response::<BalanceResponse, _>(
-                response,
-                Some(PayloadType::BalanceResponse),
-                |res| res.available_balance == U512::zero(),
-            )
-        }),
-    }
-}
-
-fn get_balance_by_state_root(state_root_hash: Digest, account_hash: AccountHash) -> TestCase {
-    TestCase {
-        name: "get_balance_by_state_root",
-        request: BinaryRequest::Get(GetRequest::State(Box::new(
-            GlobalStateRequest::BalanceByStateRoot {
-                state_identifier: Some(GlobalStateIdentifier::StateRootHash(state_root_hash)),
-                purse_identifier: PurseIdentifier::Account(account_hash),
-            },
-        ))),
+        name: "get_balance",
+        request: BinaryRequest::Get(GetRequest::State(Box::new(GlobalStateRequest::Balance {
+            state_identifier: Some(GlobalStateIdentifier::StateRootHash(state_root_hash)),
+            purse_identifier: PurseIdentifier::Account(account_hash),
+        }))),
         asserter: Box::new(|response| {
             assert_response::<BalanceResponse, _>(
                 response,