refactor(proto)!: remove proto timestamp wrapper types (#5833)

Description --- We were making use of 3 proto wrapper types. These types deserialize into option types and in some cases are not what we want. This PR will update 2 of these types, while saving the third for an additional PR. Motivation and Context --- Use standard rust types, instead of library specific types. Closes: #5797 How Has This Been Tested? --- CI What process can a PR reviewer use to test or verify this change? --- Pay close attention to changes around timestamps. Could get tricky. Notably, we were using `Some(0)` in some tests for timestamps. This was a valid timestamp for us, because it wasn't `None`. Now we're treating `0` as an invalid timestamp. So any test with `None` became `0` but tests with `Some(0)` are presumably tests that _should_ have had valid timestamps but this was the easiest minimal value that was accepted.   Breaking Changes --- - [ ] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [x] Other - Please specify Messages between new and old nodes are likely to break.
tari-project · Oct 23, 2023 · 43b994e · 43b994e
1 parent 6e52f44
commit 43b994e
Show file tree

Hide file tree

Showing 26 changed files with 204 additions and 214 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/applications/minotari_app_grpc/proto/network.proto b/applications/minotari_app_grpc/proto/network.proto
@@ -39,12 +39,12 @@ message Peer {
     /// Peer's addresses
     repeated Address addresses = 3;
     /// Last connection attempt to peer
-    google.protobuf.Timestamp last_connection = 4;
+    uint64 last_connection = 4;
     /// Flags for the peer.
     uint32 flags = 5;
-    google.protobuf.Timestamp banned_until= 6;
+    uint64 banned_until= 6;
     string banned_reason= 7;
-    google.protobuf.Timestamp offline_at = 8;
+    uint64 offline_at = 8;
     /// Features supported by the peer
     uint32 features = 9;
     /// used as information for more efficient protocol negotiation.

diff --git a/applications/minotari_app_grpc/proto/wallet.proto b/applications/minotari_app_grpc/proto/wallet.proto
@@ -188,7 +188,7 @@ message TransactionInfo {
     uint64 fee = 7;
     bool is_cancelled = 8;
     bytes excess_sig = 9;
-    google.protobuf.Timestamp timestamp = 10;
+    uint64 timestamp = 10;
     string message = 11;
 }
 

diff --git a/applications/minotari_app_grpc/src/conversions/peer.rs b/applications/minotari_app_grpc/src/conversions/peer.rs
@@ -23,7 +23,7 @@
 use tari_comms::{connectivity::ConnectivityStatus, net_address::MultiaddrWithStats, peer_manager::Peer};
 use tari_utilities::ByteArray;
 
-use crate::{conversions::naive_datetime_to_timestamp, tari_rpc as grpc};
+use crate::tari_rpc as grpc;
 
 #[allow(clippy::cast_possible_truncation)]
 #[allow(clippy::cast_sign_loss)]
@@ -32,14 +32,18 @@ impl From<Peer> for grpc::Peer {
         let public_key = peer.public_key.to_vec();
         let node_id = peer.node_id.to_vec();
         let mut addresses = Vec::with_capacity(peer.addresses.len());
-        let last_connection = peer.addresses.last_seen().map(naive_datetime_to_timestamp);
+        let last_connection = peer
+            .addresses
+            .last_seen()
+            .map(|f| f.timestamp() as u64)
+            .unwrap_or_default();
         for address in peer.addresses.addresses() {
             addresses.push(address.clone().into())
         }
         let flags = u32::from(peer.flags.bits());
-        let banned_until = peer.banned_until.map(naive_datetime_to_timestamp);
+        let banned_until = peer.banned_until.map(|f| f.timestamp() as u64).unwrap_or_default();
         let banned_reason = peer.banned_reason.to_string();
-        let offline_at = peer.offline_at().map(naive_datetime_to_timestamp);
+        let offline_at = peer.offline_at().map(|f| f.timestamp() as u64).unwrap_or_default();
         let features = peer.features.bits();
 
         let supported_protocols = peer.supported_protocols.into_iter().map(|p| p.to_vec()).collect();

diff --git a/applications/minotari_console_wallet/src/automation/commands.rs b/applications/minotari_console_wallet/src/automation/commands.rs
@@ -1117,7 +1117,7 @@ async fn get_tip_height(wallet: &WalletSqlite) -> Option<u64> {
             .await
             .ok()
             .and_then(|t| t.metadata)
-            .and_then(|m| m.height_of_longest_chain),
+            .map(|m| m.height_of_longest_chain),
         None => None,
     }
 }
diff --git a/applications/minotari_console_wallet/src/grpc/wallet_grpc_server.rs b/applications/minotari_console_wallet/src/grpc/wallet_grpc_server.rs
@@ -28,59 +28,56 @@ use futures::{
     SinkExt,
 };
 use log::*;
-use minotari_app_grpc::{
-    conversions::naive_datetime_to_timestamp,
-    tari_rpc::{
-        self,
-        payment_recipient::PaymentType,
-        wallet_server,
-        CheckConnectivityResponse,
-        ClaimHtlcRefundRequest,
-        ClaimHtlcRefundResponse,
-        ClaimShaAtomicSwapRequest,
-        ClaimShaAtomicSwapResponse,
-        CoinSplitRequest,
-        CoinSplitResponse,
-        CommitmentSignature,
-        CreateBurnTransactionRequest,
-        CreateBurnTransactionResponse,
-        CreateTemplateRegistrationRequest,
-        CreateTemplateRegistrationResponse,
-        GetAddressResponse,
-        GetBalanceRequest,
-        GetBalanceResponse,
-        GetCoinbaseRequest,
-        GetCoinbaseResponse,
-        GetCompletedTransactionsRequest,
-        GetCompletedTransactionsResponse,
-        GetConnectivityRequest,
-        GetIdentityRequest,
-        GetIdentityResponse,
-        GetTransactionInfoRequest,
-        GetTransactionInfoResponse,
-        GetUnspentAmountsResponse,
-        GetVersionRequest,
-        GetVersionResponse,
-        ImportUtxosRequest,
-        ImportUtxosResponse,
-        RegisterValidatorNodeRequest,
-        RegisterValidatorNodeResponse,
-        RevalidateRequest,
-        RevalidateResponse,
-        SendShaAtomicSwapRequest,
-        SendShaAtomicSwapResponse,
-        SetBaseNodeRequest,
-        SetBaseNodeResponse,
-        TransactionDirection,
-        TransactionEvent,
-        TransactionEventRequest,
-        TransactionEventResponse,
-        TransactionInfo,
-        TransactionStatus,
-        TransferRequest,
-        TransferResponse,
-        TransferResult,
-    },
+use minotari_app_grpc::tari_rpc::{
+    self,
+    payment_recipient::PaymentType,
+    wallet_server,
+    CheckConnectivityResponse,
+    ClaimHtlcRefundRequest,
+    ClaimHtlcRefundResponse,
+    ClaimShaAtomicSwapRequest,
+    ClaimShaAtomicSwapResponse,
+    CoinSplitRequest,
+    CoinSplitResponse,
+    CommitmentSignature,
+    CreateBurnTransactionRequest,
+    CreateBurnTransactionResponse,
+    CreateTemplateRegistrationRequest,
+    CreateTemplateRegistrationResponse,
+    GetAddressResponse,
+    GetBalanceRequest,
+    GetBalanceResponse,
+    GetCoinbaseRequest,
+    GetCoinbaseResponse,
+    GetCompletedTransactionsRequest,
+    GetCompletedTransactionsResponse,
+    GetConnectivityRequest,
+    GetIdentityRequest,
+    GetIdentityResponse,
+    GetTransactionInfoRequest,
+    GetTransactionInfoResponse,
+    GetUnspentAmountsResponse,
+    GetVersionRequest,
+    GetVersionResponse,
+    ImportUtxosRequest,
+    ImportUtxosResponse,
+    RegisterValidatorNodeRequest,
+    RegisterValidatorNodeResponse,
+    RevalidateRequest,
+    RevalidateResponse,
+    SendShaAtomicSwapRequest,
+    SendShaAtomicSwapResponse,
+    SetBaseNodeRequest,
+    SetBaseNodeResponse,
+    TransactionDirection,
+    TransactionEvent,
+    TransactionEventRequest,
+    TransactionEventResponse,
+    TransactionInfo,
+    TransactionStatus,
+    TransferRequest,
+    TransferResponse,
+    TransferResult,
 };
 use minotari_wallet::{
     connectivity_service::{OnlineStatus, WalletConnectivityInterface},
@@ -777,7 +774,7 @@ impl wallet_server::Wallet for WalletGrpcServer {
                         is_cancelled: txn.cancelled.is_some(),
                         direction: TransactionDirection::from(txn.direction) as i32,
                         fee: txn.fee.into(),
-                        timestamp: Some(naive_datetime_to_timestamp(txn.timestamp)),
+                        timestamp: txn.timestamp.timestamp() as u64,
                         excess_sig: txn
                             .transaction
                             .first_kernel_excess_sig()
@@ -1113,7 +1110,7 @@ fn convert_wallet_transaction_into_transaction_info(
             direction: TransactionDirection::Inbound as i32,
             fee: 0,
             excess_sig: Default::default(),
-            timestamp: Some(naive_datetime_to_timestamp(tx.timestamp)),
+            timestamp: tx.timestamp.timestamp() as u64,
             message: tx.message,
         },
         PendingOutbound(tx) => TransactionInfo {
@@ -1126,7 +1123,7 @@ fn convert_wallet_transaction_into_transaction_info(
             direction: TransactionDirection::Outbound as i32,
             fee: tx.fee.into(),
             excess_sig: Default::default(),
-            timestamp: Some(naive_datetime_to_timestamp(tx.timestamp)),
+            timestamp: tx.timestamp.timestamp() as u64,
             message: tx.message,
         },
         Completed(tx) => TransactionInfo {
@@ -1138,7 +1135,7 @@ fn convert_wallet_transaction_into_transaction_info(
             is_cancelled: tx.cancelled.is_some(),
             direction: TransactionDirection::from(tx.direction) as i32,
             fee: tx.fee.into(),
-            timestamp: Some(naive_datetime_to_timestamp(tx.timestamp)),
+            timestamp: tx.timestamp.timestamp() as u64,
             excess_sig: tx
                 .transaction
                 .first_kernel_excess_sig()

diff --git a/base_layer/core/src/base_node/chain_metadata_service/service.rs b/base_layer/core/src/base_node/chain_metadata_service/service.rs
@@ -234,6 +234,7 @@ mod test {
     };
     use tari_service_framework::reply_channel;
     use tari_test_utils::unpack_enum;
+    use tari_utilities::epoch_time::EpochTime;
     use tokio::{sync::broadcast, task};
 
     use super::*;
@@ -254,14 +255,14 @@ mod test {
     fn create_sample_proto_chain_metadata() -> proto::ChainMetadata {
         let diff: u128 = 1;
         proto::ChainMetadata {
-            height_of_longest_chain: Some(1),
+            height_of_longest_chain: 1,
             best_block: vec![
                 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27,
                 28, 29, 30, 31,
             ],
             pruned_height: 0,
             accumulated_difficulty: diff.to_be_bytes().to_vec(),
-            timestamp: Some(0),
+            timestamp: EpochTime::now().as_u64(),
         }
     }
 
@@ -290,7 +291,7 @@ mod test {
         let (mut service, liveness_mock_state, mut base_node_receiver, _) = setup();
 
         let mut proto_chain_metadata = create_sample_proto_chain_metadata();
-        proto_chain_metadata.height_of_longest_chain = Some(123);
+        proto_chain_metadata.height_of_longest_chain = 123;
         let chain_metadata = proto_chain_metadata.clone().try_into().unwrap();
 
         task::spawn(async move {
@@ -309,7 +310,7 @@ mod test {
         unpack_enum!(LivenessRequest::SetMetadataEntry(metadata_key, data) = last_call);
         assert_eq!(metadata_key, MetadataKey::ChainMetadata);
         let chain_metadata = proto::ChainMetadata::decode(data.as_slice()).unwrap();
-        assert_eq!(chain_metadata.height_of_longest_chain, Some(123));
+        assert_eq!(chain_metadata.height_of_longest_chain, 123);
     }
     #[tokio::test]
     async fn handle_liveness_event_ok() {
@@ -332,7 +333,7 @@ mod test {
         assert_eq!(*metadata.node_id(), node_id);
         assert_eq!(
             metadata.claimed_chain_metadata().height_of_longest_chain(),
-            proto_chain_metadata.height_of_longest_chain.unwrap()
+            proto_chain_metadata.height_of_longest_chain
         );
     }
 

diff --git a/base_layer/core/src/base_node/proto/chain_metadata.proto b/base_layer/core/src/base_node/proto/chain_metadata.proto
@@ -9,7 +9,7 @@ package tari.base_node;
 
 message ChainMetadata {
     // The current chain height, or the block number of the longest valid chain, or `None` if there is no chain
-    google.protobuf.UInt64Value height_of_longest_chain = 1;
+    uint64 height_of_longest_chain = 1;
     // The block hash of the current tip of the longest valid chain, or `None` for an empty chain
     bytes best_block = 2;
     // The current geometric mean of the pow of the chain tip, or `None` if there is no chain
@@ -20,5 +20,5 @@ message ChainMetadata {
     // Archival nodes wil always have an `pruned_height` of zero.
     uint64 pruned_height = 6;
     // Timestamp of the last block in the chain, or `None` if there is no chain
-    google.protobuf.UInt64Value timestamp = 7;
+    uint64 timestamp = 7;
 }
diff --git a/base_layer/core/src/base_node/proto/chain_metadata.rs b/base_layer/core/src/base_node/proto/chain_metadata.rs
@@ -45,9 +45,8 @@ impl TryFrom<proto::ChainMetadata> for ChainMetadata {
         let mut acc_diff = [0; ACC_DIFFICULTY_ARRAY_LEN];
         acc_diff.copy_from_slice(&metadata.accumulated_difficulty[0..ACC_DIFFICULTY_ARRAY_LEN]);
         let accumulated_difficulty = u128::from_be_bytes(acc_diff);
-        let height_of_longest_chain = metadata
-            .height_of_longest_chain
-            .ok_or_else(|| "Height of longest chain is missing".to_string())?;
+        let height_of_longest_chain = metadata.height_of_longest_chain;
+
         let pruning_horizon = if metadata.pruned_height == 0 {
             metadata.pruned_height
         } else {
@@ -67,7 +66,7 @@ impl TryFrom<proto::ChainMetadata> for ChainMetadata {
             pruning_horizon,
             metadata.pruned_height,
             accumulated_difficulty,
-            metadata.timestamp.unwrap_or_default(),
+            metadata.timestamp,
         ))
     }
 }
@@ -76,17 +75,17 @@ impl From<ChainMetadata> for proto::ChainMetadata {
     fn from(metadata: ChainMetadata) -> Self {
         let accumulated_difficulty = metadata.accumulated_difficulty().to_be_bytes().to_vec();
         Self {
-            height_of_longest_chain: Some(metadata.height_of_longest_chain()),
+            height_of_longest_chain: metadata.height_of_longest_chain(),
             best_block: metadata.best_block().to_vec(),
             pruned_height: metadata.pruned_height(),
             accumulated_difficulty,
-            timestamp: Some(metadata.timestamp()),
+            timestamp: metadata.timestamp(),
         }
     }
 }
 
 impl proto::ChainMetadata {
     pub fn height_of_longest_chain(&self) -> u64 {
-        self.height_of_longest_chain.unwrap_or(0)
+        self.height_of_longest_chain
     }
 }
diff --git a/base_layer/core/src/base_node/proto/wallet_rpc.proto b/base_layer/core/src/base_node/proto/wallet_rpc.proto
@@ -39,7 +39,7 @@ message TxQueryResponse {
   uint64 confirmations = 3;
   bool is_synced = 4;
   uint64 height_of_longest_chain = 5;
-  google.protobuf.UInt64Value mined_timestamp = 6;
+  uint64 mined_timestamp = 6;
 }
 
 message TxQueryBatchResponse {
@@ -48,15 +48,15 @@ message TxQueryBatchResponse {
   bytes block_hash = 3;
   uint64 confirmations = 4;
   uint64 block_height = 5;
-  google.protobuf.UInt64Value mined_timestamp = 6;
+  uint64 mined_timestamp = 6;
 }
 
 message TxQueryBatchResponses {
   repeated TxQueryBatchResponse responses = 1;
   bool is_synced = 2;
   bytes tip_hash = 3;
   uint64 height_of_longest_chain = 4;
-  google.protobuf.UInt64Value tip_mined_timestamp = 5;
+  uint64 tip_mined_timestamp = 5;
 }
 
 message FetchMatchingUtxos {