Merge remote-tracking branch 'origin/main' into v1-contract-state

.github/workflows/build-test.yaml

@@ -174,9 +174,9 @@ jobs:
     - name: Run clippy (without extra features)
       run: |
         cargo clippy --manifest-path concordium-node/Cargo.toml --all -- -Dclippy::all
-    - name: Run clippy (with features 'instrumentation', 'collector', 'network_dump', 'database_emitter')
+    - name: Run clippy (with features 'instrumentation', 'collector', 'network_dump')
       run: |
-        cargo clippy --manifest-path concordium-node/Cargo.toml --features=instrumentation,collector,network_dump,database_emitter --all -- -Dclippy::all
+        cargo clippy --manifest-path concordium-node/Cargo.toml --features=instrumentation,collector,network_dump --all -- -Dclippy::all
     - name: Run clippy on collector backend
       run: |
         cargo clippy --manifest-path collector-backend/Cargo.toml -- -Dclippy::all

CHANGELOG.md

@@ -22,7 +22,10 @@
   This affects all queries whose input was a block or transaction hash.
   These queries now return `InvalidArgument` error, as opposed to `Unknown`
   which they returned previously.
-
+- Fix issue #244: Collector to keep querying. Remove the parameter for maximum allowed
+  times a gRPC call can fail and keeps `node-collector` querying forever.
+- `GetAccountInfo` endpoint supports querying the account via the account index.
+
 ## concordium-node 3.0.1
 
 - Fix a starvation bug in some cases of parallel node queries.

concordium-consensus/src/Concordium/External.hs

@@ -29,7 +29,6 @@ import qualified Data.FixedByteString as FBS
 import Concordium.Afgjort.Finalize.Types (FinalizationInstance (FinalizationInstance))
 import Concordium.Birk.Bake
 import Concordium.Constants.Time (defaultEarlyBlockThreshold, defaultMaxBakingDelay)
-import Concordium.Crypto.ByteStringHelpers
 import Concordium.GlobalState
 import Concordium.GlobalState.Persistent.LMDB (addDatabaseVersion)
 import Concordium.GlobalState.Persistent.TreeState (InitException (..))
@@ -891,15 +890,6 @@ decodeBlockHash blockcstr = readMaybe <$> peekCString blockcstr
 decodeAccountAddress :: CString -> IO (Either String AccountAddress)
 decodeAccountAddress acctstr = addressFromBytes <$> BS.packCString acctstr
 
--- |Decode a null-terminated string as either an account address (base-58) or a
--- credential registration ID (base-16).
-decodeAccountAddressOrCredId :: CString -> IO (Maybe (Either CredentialRegistrationID AccountAddress))
-decodeAccountAddressOrCredId str = do
-    bs <- BS.packCString str
-    return $ case addressFromBytes bs of
-        Left _ -> Left <$> bsDeserializeBase16 bs
-        Right acc -> Just $ Right acc
-
 -- |Decode an instance address from a null-terminated JSON-encoded string.
 decodeInstanceAddress :: CString -> IO (Maybe ContractAddress)
 decodeInstanceAddress inststr = AE.decodeStrict <$> BS.packCString inststr
@@ -1071,8 +1061,9 @@ getModuleList cptr blockcstr = do
 getAccountInfo :: StablePtr ConsensusRunner -> CString -> CString -> IO CString
 getAccountInfo cptr blockcstr acctcstr = do
     mblock <- decodeBlockHash blockcstr
-    maccount <- decodeAccountAddressOrCredId acctcstr
-    case (mblock, maccount) of
+    acctbs <- BS.packCString acctcstr
+    let account = decodeAccountIdentifier acctbs
+    case (mblock, account) of
         (Just bh, Just acct) -> jsonQuery cptr (Q.getAccountInfo bh acct)
         _ -> jsonCString AE.Null
 

concordium-consensus/src/Concordium/GlobalState/Basic/BlockState.hs

@@ -424,6 +424,10 @@ instance (IsProtocolVersion pv, Monad m) => BS.BlockStateQuery (PureBlockStateMo
            Nothing -> return Nothing
            Just ai -> return $ (ai, ) <$> bs ^? blockAccounts . Accounts.indexedAccount ai
 
+    {-# INLINE getAccountByIndex #-}
+    getAccountByIndex bs ai =
+      return $ (ai, ) <$> bs ^? blockAccounts . Accounts.indexedAccount ai
+
     {-# INLINE getBakerAccount #-}
     getBakerAccount bs (BakerId ai) =
       return $ bs ^? blockAccounts . Accounts.indexedAccount ai

concordium-consensus/src/Concordium/GlobalState/BlockState.hs

@@ -291,6 +291,9 @@ class (ContractStateOperations m, AccountOperations m) => BlockStateQuery m wher
     -- |Query an account by the id of the credential that belonged to it.
     getAccountByCredId :: BlockState m -> CredentialRegistrationID -> m (Maybe (AccountIndex, Account m))
 
+    -- |Query an account by the account index that belonged to it.
+    getAccountByIndex :: BlockState m -> AccountIndex -> m (Maybe (AccountIndex, Account m))
+
     -- |Get the contract state from the contract table of the state instance.
     getContractInstance :: BlockState m -> ContractAddress -> m (Maybe (InstanceInfo m))
 
@@ -760,6 +763,7 @@ instance (Monad (t m), MonadTrans t, BlockStateQuery m) => BlockStateQuery (MGST
   getAccount s = lift . getAccount s
   accountExists s = lift . accountExists s
   getAccountByCredId s = lift . getAccountByCredId s
+  getAccountByIndex s = lift . getAccountByIndex s
   getBakerAccount s = lift . getBakerAccount s
   getContractInstance s = lift . getContractInstance s
   getModuleList = lift . getModuleList
@@ -790,6 +794,7 @@ instance (Monad (t m), MonadTrans t, BlockStateQuery m) => BlockStateQuery (MGST
   {-# INLINE getAccount #-}
   {-# INLINE accountExists #-}
   {-# INLINE getAccountByCredId #-}
+  {-# INLINE getAccountByIndex #-}
   {-# INLINE getBakerAccount #-}
   {-# INLINE getContractInstance #-}
   {-# INLINE getModuleList #-}

concordium-consensus/src/Concordium/GlobalState/Paired.hs

@@ -222,6 +222,17 @@ instance (Monad m, C.HasGlobalStateContext (PairGSContext lc rc) r, BlockStateQu
             return Nothing
           (Nothing, _) -> error $ "Cannot get account with credid " ++ show cid ++ " in left implementation"
           (_, Nothing) -> error $ "Cannot get account with credid " ++ show cid ++ " in right implementation"
+    getAccountByIndex (ls, rs) idx = do
+        a1 <- coerceBSML (getAccountByIndex ls idx)
+        a2 <- coerceBSMR (getAccountByIndex rs idx)
+        case (a1, a2) of
+          (Just (ai1, a1'), Just (ai2, a2')) ->
+            assert ((getHash a1' :: H.Hash) == getHash a2' && ai1 == ai2) $
+              return $ Just (ai1, (a1', a2'))
+          (Nothing, Nothing) ->
+            return Nothing
+          (Nothing, _) -> error $ "Cannot get account by index " ++ show idx ++ " in left implementation"
+          (_, Nothing) -> error $ "Cannot get account by index " ++ show idx ++ " in right implementation"
     getBakerAccount (ls, rs) bid = do
         a1 <- coerceBSML (getBakerAccount ls bid)
         a2 <- coerceBSMR (getBakerAccount rs bid)

concordium-consensus/src/Concordium/GlobalState/Persistent/BlockState.hs

@@ -899,6 +899,10 @@ doGetAccountByCredId pbs cid = do
         bsp <- loadPBS pbs
         Accounts.getAccountByCredId cid (bspAccounts bsp)
 
+doGetAccountByIndex :: (IsProtocolVersion pv, MonadBlobStore m) => PersistentBlockState pv -> AccountIndex -> m (Maybe (AccountIndex, PersistentAccount pv))
+doGetAccountByIndex pbs idx = do
+        bsp <- loadPBS pbs
+        fmap (idx, ) <$> Accounts.indexedAccount idx (bspAccounts bsp)
 
 doGetAccountIndex :: (IsProtocolVersion pv, MonadBlobStore m) => PersistentBlockState pv -> AccountAddress -> m (Maybe AccountIndex)
 doGetAccountIndex pbs addr = do
@@ -1314,6 +1318,7 @@ instance (IsProtocolVersion pv, PersistentState r m) => BlockStateQuery (Persist
     getAccount = doGetAccount . hpbsPointers
     accountExists = doGetAccountExists . hpbsPointers
     getAccountByCredId = doGetAccountByCredId . hpbsPointers
+    getAccountByIndex = doGetAccountByIndex . hpbsPointers
     getContractInstance = doGetInstance . hpbsPointers
     getModuleList = doGetModuleList . hpbsPointers
     getAccountList = doAccountList . hpbsPointers

concordium-consensus/src/Concordium/Queries.hs

@@ -460,19 +460,22 @@ getModuleList :: BlockHash -> MVR gsconf finconf (Maybe [ModuleRef])
 getModuleList = liftSkovQueryBlock $ BS.getModuleList <=< blockState
 
 -- |Get the details of an account in the block state.
--- The account can be given either via an address, or via a credential registration id.
+-- The account can be given via an address, an account index or a credential registration id.
 -- In the latter case we lookup the account the credential is associated with, even if it was
 -- removed from the account.
 getAccountInfo ::
     BlockHash ->
-    Either CredentialRegistrationID AccountAddress ->
+    AccountIdentifier ->
     MVR gsconf finconf (Maybe AccountInfo)
 getAccountInfo blockHash acct =
     join
         <$> liftSkovQueryBlock
             ( \bp -> do
                 bs <- blockState bp
-                macc <- either (BS.getAccountByCredId bs) (BS.getAccount bs) acct
+                macc <- case acct of 
+                                AccAddress addr -> BS.getAccount bs addr
+                                AccIndex idx -> BS.getAccountByIndex bs idx
+                                CredRegID crid -> BS.getAccountByCredId bs crid
                 forM macc $ \(aiAccountIndex, acc) -> do
                     aiAccountNonce <- BS.getAccountNonce acc
                     aiAccountAmount <- BS.getAccountAmount acc

concordium-node/Cargo.toml

@@ -19,8 +19,6 @@ network_dump = []
 static = [ ]
 profiling = [ "static" ]
 collector = [ "reqwest/default-tls", "serde/derive", "rmp-serde", "collector-backend" ]
-database_emitter = []
-genesis_tester = [ "tempfile" ]
 
 [profile.release]
 codegen-units = 1
@@ -129,25 +127,10 @@ name = "node-collector"
 path = "src/bin/collector.rs"
 required-features = [ "collector" ]
 
-[[bin]]
-name = "network_stress_test"
-path = "src/bin/network_stress_test.rs"
-required-features = [ "test_utils" ]
-
 [[bin]]
 name = "bootstrap_checker"
 path = "src/bin/bootstrap_checker.rs"
 
-[[bin]]
-name = "database_emitter"
-path = "src/bin/database_emitter.rs"
-required-features = [ "database_emitter" ]
-
-[[bin]]
-name = "genesis_tester"
-path = "src/bin/genesis_tester.rs"
-required-features = [ "genesis_tester" ]
-
 [[bench]]
 name = "p2p_lib_benchmark"
 required-features = [ "test_utils" ]

concordium-node/README.md

@@ -20,8 +20,6 @@
 * static - build against static haskell libraries (Linux only)
 * profiling - build against haskell libraries with profiling support enabled (Linux only)
 * collector - enables the build of the node-collector and backend
-* database_emitter - enables building the database emitter binary to inject a database exported to a set of nodes
-* genesis_tester - a tool used by a CI to validate the genesis data
 * dedup_benchmarks - enable support in the benchmarks for deduplication queues
 
 ## Building the node

concordium-node/src/bin/collector.rs

@@ -3,16 +3,7 @@ use collector_backend::{IsInBakingCommittee, NodeInfo};
 use concordium_node::utils::setup_macos_logger;
 use concordium_node::{common::grpc_api, req_with_auth, utils::setup_logger};
 use serde_json::Value;
-use std::{
-    borrow::ToOwned,
-    default::Default,
-    fmt,
-    process::exit,
-    str::FromStr,
-    sync::atomic::{AtomicUsize, Ordering as AtomicOrdering},
-    thread,
-    time::Duration,
-};
+use std::{borrow::ToOwned, fmt, process::exit, str::FromStr, time::Duration};
 use structopt::StructOpt;
 use tonic::{metadata::MetadataValue, transport::channel::Channel, Request};
 #[macro_use]
@@ -50,59 +41,59 @@ struct ConfigCli {
         env = "CONCORDIUM_NODE_COLLECTOR_GRPC_AUTHENTICATION_TOKEN",
         hide_env_values = true
     )]
-    pub grpc_auth_token: String,
+    pub grpc_auth_token:        String,
     #[structopt(
         long = "grpc-host",
         help = "gRPC host to collect from",
         default_value = "http://127.0.0.1:10000",
         env = "CONCORDIUM_NODE_COLLECTOR_GRPC_HOST",
         use_delimiter = true // default delimiter is a comma
     )]
-    pub grpc_hosts: Vec<String>,
+    pub grpc_hosts:             Vec<String>,
     #[structopt(
         long = "node-name",
         help = "Node name",
         env = "CONCORDIUM_NODE_COLLECTOR_NODE_NAME",
         use_delimiter = true // default delimiter is a comma
     )]
-    pub node_names: Vec<NodeName>,
+    pub node_names:             Vec<NodeName>,
     #[structopt(
         long = "collector-url",
         help = "Alias submitted of the node collected from",
         default_value = "http://localhost:3000/post/nodes",
         env = "CONCORDIUM_NODE_COLLECTOR_URL"
     )]
-    pub collector_url: String,
+    pub collector_url:          String,
     #[structopt(
         long = "print-config",
         help = "Print out config struct",
         env = "CONCORDIUM_NODE_COLLECTOR_PRINT_CONFIG"
     )]
-    pub print_config: bool,
+    pub print_config:           bool,
     #[structopt(
         long = "debug",
         short = "d",
         help = "Debug mode",
         env = "CONCORDIUM_NODE_COLLECTOR_DEBUG"
     )]
-    pub debug: bool,
+    pub debug:                  bool,
     #[structopt(long = "trace", help = "Trace mode", env = "CONCORDIUM_NODE_COLLECTOR_TRACE")]
-    pub trace: bool,
+    pub trace:                  bool,
     #[structopt(long = "info", help = "Info mode", env = "CONCORDIUM_NODE_COLLECTOR_INFO")]
-    pub info: bool,
+    pub info:                   bool,
     #[structopt(
         long = "no-log-timestamp",
         help = "Do not output timestamp in log output",
         env = "CONCORDIUM_NODE_COLLECTOR_NO_LOG_TIMESTAMP"
     )]
-    pub no_log_timestamp: bool,
+    pub no_log_timestamp:       bool,
     #[structopt(
         long = "collect-interval",
         help = "Interval in miliseconds to sleep between runs of the collector",
         default_value = "5000",
         env = "CONCORDIUM_NODE_COLLECTOR_COLLECT_INTERVAL"
     )]
-    pub collector_interval: u64,
+    pub collector_interval:     u64,
     #[structopt(
         long = "artificial-start-delay",
         help = "Time (in ms) to delay when the first gRPC request is sent to the node",
@@ -111,12 +102,12 @@ struct ConfigCli {
     )]
     pub artificial_start_delay: u64,
     #[structopt(
-        long = "max-grpc-failures-allowed",
-        help = "Maximum allowed times a gRPC call can fail before terminating the program",
-        default_value = "50",
-        env = "CONCORDIUM_NODE_COLLECTOR_MAX_GRPC_FAILURES_ALLOWED"
+        long = "grpc-timeout",
+        help = "Time (in seconds) for gRPC request timeouts",
+        default_value = "30",
+        env = "CONCORDIUM_NODE_COLLECTOR_GRPC_TIMEOUT"
     )]
-    pub max_grpc_failures_allowed: u64,
+    pub grpc_timeout:           u64,
     #[cfg(target_os = "macos")]
     #[structopt(
         long = "use-mac-log",
@@ -127,7 +118,7 @@ struct ConfigCli {
         env = "CONCORDIUM_NODE_COLLECTOR_USE_MAC_LOG",
         conflicts_with = "log-config"
     )]
-    pub use_mac_log: Option<String>,
+    pub use_mac_log:            Option<String>,
 }
 
 #[tokio::main]
@@ -159,18 +150,15 @@ async fn main() {
 
     if conf.artificial_start_delay > 0 {
         info!("Delaying first collection from the node for {} ms", conf.artificial_start_delay);
-        thread::sleep(Duration::from_millis(conf.artificial_start_delay));
+        tokio::time::sleep(Duration::from_millis(conf.artificial_start_delay)).await;
     }
 
-    let atomic_counter: AtomicUsize = Default::default();
+    let mut interval = tokio::time::interval(Duration::from_millis(conf.collector_interval));
     #[allow(unreachable_code)]
     loop {
-        let grpc_failure_count = atomic_counter.load(AtomicOrdering::Relaxed);
-        trace!("Failure count is {}/{}", grpc_failure_count, conf.max_grpc_failures_allowed);
         for (node_name, grpc_host) in conf.node_names.iter().zip(conf.grpc_hosts.iter()) {
             trace!("Processing node {}/{}", node_name, grpc_host);
-            match collect_data(node_name.clone(), grpc_host.to_owned(), &conf.grpc_auth_token).await
-            {
+            match collect_data(node_name.clone(), grpc_host.to_owned(), &conf).await {
                 Ok(node_info) => {
                     trace!("Node data collected successfully from {}/{}", node_name, grpc_host);
                     match rmp_serde::encode::to_vec(&node_info) {
@@ -188,36 +176,35 @@ async fn main() {
                     }
                 }
                 Err(e) => {
-                    let _ = atomic_counter.fetch_add(1, AtomicOrdering::SeqCst);
                     error!(
                         "gRPC failed with \"{}\" for {}, sleeping for {} ms",
                         e, &grpc_host, conf.collector_interval
                     );
                 }
             }
-
-            if grpc_failure_count + 1 >= conf.max_grpc_failures_allowed as usize {
-                error!("Too many gRPC failures, exiting!");
-                exit(1);
-            }
         }
         trace!("Sleeping for {} ms", conf.collector_interval);
-        thread::sleep(Duration::from_millis(conf.collector_interval));
+        interval.tick().await;
     }
 }
 
 #[allow(clippy::cognitive_complexity)]
 async fn collect_data<'a>(
     node_name: NodeName,
     grpc_host: String,
-    grpc_auth_token: &str,
+    conf: &ConfigCli,
 ) -> anyhow::Result<NodeInfo> {
+    let grpc_auth_token = &conf.grpc_auth_token;
+    let grpc_timeout = conf.grpc_timeout;
     info!(
         "Collecting node information via gRPC from {}/{}/{}",
         node_name, grpc_host, grpc_auth_token
     );
-
-    let channel = Channel::from_shared(grpc_host).unwrap().connect().await?;
+    let channel = Channel::from_shared(grpc_host)
+        .unwrap()
+        .timeout(Duration::from_secs(grpc_timeout))
+        .connect()
+        .await?;
     let mut client = grpc_api::p2p_client::P2pClient::new(channel);
 
     let empty_req = || req_with_auth!(grpc_api::Empty {}, grpc_auth_token);