Skip to content
This repository has been archived by the owner on Oct 31, 2024. It is now read-only.

Commit

Permalink
fix: reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
Marko Atanasievski committed Apr 3, 2023
1 parent aa032ba commit dea7066
Show file tree
Hide file tree
Showing 11 changed files with 58 additions and 95 deletions.
6 changes: 1 addition & 5 deletions crates/topos-api/proto/topos/tce/v1/api.proto
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,10 @@ message GetLastPendingCertificatesRequest {
repeated topos.shared.v1.SubnetId subnet_ids = 1;
}

message OptionalCertificate {
optional topos.uci.v1.Certificate value = 1;
}

message GetLastPendingCertificatesResponse {
// Bytes and array types (SubnetId) could not be key in the map type according to specifications,
// so we use SubnetId hex encoded string with 0x prefix as key
map<string, OptionalCertificate> last_pending_certificate = 1;
map<string, topos.uci.v1.OptionalCertificate> last_pending_certificate = 1;
}

message WatchCertificatesRequest {
Expand Down
5 changes: 5 additions & 0 deletions crates/topos-api/proto/topos/uci/v1/certification.proto
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,8 @@ message Certificate {
topos.shared.v1.StarkProof proof = 8;
topos.shared.v1.Frost signature = 9;
}


message OptionalCertificate {
optional Certificate value = 1;
}
Binary file modified crates/topos-api/src/generated/topos.bin
Binary file not shown.
8 changes: 1 addition & 7 deletions crates/topos-api/src/generated/topos.tce.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,19 +108,13 @@ pub struct GetLastPendingCertificatesRequest {
}
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct OptionalCertificate {
#[prost(message, optional, tag = "1")]
pub value: ::core::option::Option<super::super::uci::v1::Certificate>,
}
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct GetLastPendingCertificatesResponse {
/// Bytes and array types (SubnetId) could not be key in the map type according to specifications,
/// so we use SubnetId hex encoded string with 0x prefix as key
#[prost(map = "string, message", tag = "1")]
pub last_pending_certificate: ::std::collections::HashMap<
::prost::alloc::string::String,
OptionalCertificate,
super::super::uci::v1::OptionalCertificate,
>,
}
#[allow(clippy::derive_partial_eq_without_eq)]
Expand Down
6 changes: 6 additions & 0 deletions crates/topos-api/src/generated/topos.uci.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,9 @@ pub struct Certificate {
#[prost(message, optional, tag = "9")]
pub signature: ::core::option::Option<super::super::shared::v1::Frost>,
}
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct OptionalCertificate {
#[prost(message, optional, tag = "1")]
pub value: ::core::option::Option<Certificate>,
}
6 changes: 3 additions & 3 deletions crates/topos-api/tests/tce_layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ use topos_api::tce::v1::api_service_server::{ApiService, ApiServiceServer};
use topos_api::tce::v1::watch_certificates_request::{Command, OpenStream};
use topos_api::tce::v1::{
GetLastPendingCertificatesRequest, GetLastPendingCertificatesResponse, GetSourceHeadRequest,
GetSourceHeadResponse, OptionalCertificate, SourceStreamPosition, SubmitCertificateRequest,
GetSourceHeadResponse, SourceStreamPosition, SubmitCertificateRequest,
SubmitCertificateResponse, WatchCertificatesRequest, WatchCertificatesResponse,
};
use topos_api::uci::v1::Certificate;
use topos_api::uci::v1::{Certificate, OptionalCertificate};
use uuid::Uuid;

use topos_test_sdk::constants::*;
Expand Down Expand Up @@ -75,7 +75,7 @@ async fn create_tce_layer() {
for subnet_id in subnet_ids {
map.insert(
subnet_id.to_string(),
topos_api::tce::v1::OptionalCertificate {
OptionalCertificate {
value: Some(Certificate {
source_subnet_id: subnet_id.into(),
id: Some(return_certificate_id.clone()),
Expand Down
2 changes: 1 addition & 1 deletion crates/topos-sequencer-tce-proxy/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,4 @@ byteorder = "1.4.3"
dockertest = "0.3.1"
serial_test = "0.9"
topos-tce-storage = { path = "../topos-tce-storage" }
topos-test-sdk = { path = "../topos-test-sdk/", features=["tce"] }
topos-test-sdk = { path = "../topos-test-sdk/", features=["tce"] }
10 changes: 5 additions & 5 deletions crates/topos-sequencer-tce-proxy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub enum Error {
},
#[error("Certificate source head empty for subnet id {subnet_id}")]
SourceHeadEmpty { subnet_id: SubnetId },
#[error("Unable to get last pending certificates for subnet ids {subnet_id}: {details}")]
#[error("Unable to get last pending certificates for subnet id {subnet_id}: {details}")]
UnableToGetLastPendingCertificates {
subnet_id: SubnetId,
details: String,
Expand Down Expand Up @@ -548,7 +548,7 @@ impl TceProxyWorker {

// Get pending certificates from the TCE node. Source head certificate
// is latest pending certificate for this subnet
let mut source_head_certificate: Option<Certificate> = match tce_client
let mut source_last_pending_certificate: Option<Certificate> = match tce_client
.get_last_pending_certificates(vec![tce_client.subnet_id])
.await
{
Expand All @@ -561,11 +561,11 @@ impl TceProxyWorker {
}
};

if source_head_certificate.is_none() {
if source_last_pending_certificate.is_none() {
// There are no pending certificates on the TCE
// Retrieve source head from TCE node (latest delivered certificate), so that
// we know from where to start creating certificates
source_head_certificate = match tce_client.get_source_head().await {
source_last_pending_certificate = match tce_client.get_source_head().await {
Ok(certificate) => Some(certificate),
Err(Error::SourceHeadEmpty { subnet_id: _ }) => {
//This is also OK, TCE node does not have any data about certificates
Expand Down Expand Up @@ -644,7 +644,7 @@ impl TceProxyWorker {
events: evt_rcv,
config,
},
source_head_certificate,
source_last_pending_certificate,
))
}

Expand Down
90 changes: 24 additions & 66 deletions crates/topos-sequencer-tce-proxy/tests/tce_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ use topos_core::api::tce::v1::{
watch_certificates_request, watch_certificates_response,
watch_certificates_response::CertificatePushed, GetLastPendingCertificatesRequest,
GetLastPendingCertificatesResponse, GetSourceHeadRequest, GetSourceHeadResponse,
OptionalCertificate, SourceStreamPosition, SubmitCertificateRequest,
SourceStreamPosition, SubmitCertificateRequest,
};
use topos_core::api::uci::v1::Certificate;
use topos_core::api::uci::v1::{Certificate, OptionalCertificate};
use topos_core::uci::{self, SUBNET_ID_LENGTH};
use tracing::{debug, error, info};

Expand Down Expand Up @@ -240,10 +240,7 @@ async fn test_tce_get_last_pending_certificates(
let mut context = start_node.await;

let source_subnet_id: SubnetId = SOURCE_SUBNET_ID_1.into();
let target_subnet_id: SubnetId = TARGET_SUBNET_ID_1.into();
let prev_certificate_id: CertificateId = PREV_CERTIFICATE_ID.into();
let certificate_id_1: CertificateId = CERTIFICATE_ID_1.into();
let certificate_id_2: CertificateId = CERTIFICATE_ID_2.into();
let certificates = create_certificate_chain(SOURCE_SUBNET_ID_1, &[TARGET_SUBNET_ID_1], 10);

// Test get last pending certificates for empty TCE history
// Reply should be empty
Expand All @@ -268,63 +265,24 @@ async fn test_tce_get_last_pending_certificates(
};
assert_eq!(response, expected_response);

let test_certificate = Certificate {
source_subnet_id: Some(source_subnet_id.clone()),
id: Some(certificate_id_1.clone()),
prev_id: Some(prev_certificate_id),
target_subnets: vec![target_subnet_id.clone()],
state_root: [0u8; 32].to_vec(),
tx_root_hash: [0u8; 32].to_vec(),
verifier: 0,
proof: Some(StarkProof { value: Vec::new() }),
signature: Some(Default::default()),
};

match context
.api_grpc_client
.submit_certificate(SubmitCertificateRequest {
certificate: Some(test_certificate.clone()),
})
.await
.map(|r| r.into_inner())
{
Ok(response) => {
debug!("Successfully submitted the Certificate {:?}", response);
}
Err(e) => {
error!("Unable to submit the certificate: {e:?}");
return Err(Box::from(e));
}
};

let test_certificate_2 = Certificate {
source_subnet_id: Some(source_subnet_id.clone()),
id: Some(certificate_id_2),
prev_id: Some(certificate_id_1),
target_subnets: vec![target_subnet_id],
state_root: [0u8; 32].to_vec(),
tx_root_hash: [0u8; 32].to_vec(),
verifier: 0,
proof: Some(StarkProof { value: Vec::new() }),
signature: Some(Default::default()),
};

match context
.api_grpc_client
.submit_certificate(SubmitCertificateRequest {
certificate: Some(test_certificate_2.clone()),
})
.await
.map(|r| r.into_inner())
{
Ok(response) => {
debug!("Successfully submitted the Certificate 2 {:?}", response);
}
Err(e) => {
error!("Unable to submit the certificate 2 : {e:?}");
return Err(Box::from(e));
}
};
for cert in &certificates {
match context
.api_grpc_client
.submit_certificate(SubmitCertificateRequest {
certificate: Some(cert.clone().into()),
})
.await
.map(|r| r.into_inner())
{
Ok(response) => {
debug!("Successfully submitted the Certificate {:?}", response);
}
Err(e) => {
error!("Unable to submit the certificate: {e:?}");
return Err(Box::from(e));
}
};
}

// Test get last pending certificate
let response = context
Expand All @@ -336,17 +294,17 @@ async fn test_tce_get_last_pending_certificates(
.map(|r| r.into_inner())
.expect("valid response");

let last_pending_certificates = vec![(
let expected_last_pending_certificates = vec![(
"0x".to_string() + &hex::encode(source_subnet_id.value.as_slice()),
OptionalCertificate {
value: Some(test_certificate_2),
value: Some(certificates.iter().last().unwrap().clone().into()),
},
)]
.into_iter()
.collect::<HashMap<String, OptionalCertificate>>();

let expected_response = GetLastPendingCertificatesResponse {
last_pending_certificate: last_pending_certificates,
last_pending_certificate: expected_last_pending_certificates,
};
assert_eq!(response, expected_response);

Expand Down
5 changes: 3 additions & 2 deletions crates/topos-tce-api/src/grpc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ use tonic::{Request, Response, Status, Streaming};
use topos_core::api::tce::v1::{
api_service_server::ApiService, GetLastPendingCertificatesRequest,
GetLastPendingCertificatesResponse, GetSourceHeadRequest, GetSourceHeadResponse,
OptionalCertificate, SubmitCertificateRequest, SubmitCertificateResponse,
WatchCertificatesRequest, WatchCertificatesResponse,
SubmitCertificateRequest, SubmitCertificateResponse, WatchCertificatesRequest,
WatchCertificatesResponse,
};
use topos_core::api::uci::v1::OptionalCertificate;
use topos_core::uci::SubnetId;
use tracing::{error, field, info, instrument, Instrument, Span};
use tracing_opentelemetry::OpenTelemetrySpanExt;
Expand Down
15 changes: 9 additions & 6 deletions crates/topos-tce-storage/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,20 +393,23 @@ async fn get_pending_certificates(storage: RocksDBStorage) {

let certificates_for_source_subnet_1 =
create_certificate_chain(SOURCE_SUBNET_ID_1, &[TARGET_SUBNET_ID_2], 15);
let certificates_for_source_subnet_2 =
create_certificate_chain(SOURCE_SUBNET_ID_2, &[TARGET_SUBNET_ID_2], 15);

// Persist the first 10 Cert of each Subnets
for cert in &certificates_for_source_subnet_1[0..10] {
storage.persist(cert, None).await.unwrap();
}
for cert in &certificates_for_source_subnet_2[0..10] {
storage.persist(cert, None).await.unwrap();
}

// Add the last 5 cert of each Subnet as pending certificate
for cert in &certificates_for_source_subnet_1[10..] {
storage.add_pending_certificate(cert).await.unwrap();
expected_pending_certificates.push((expected_pending_certificates_count, cert.clone()));
expected_pending_certificates_count += 1;
}

let certificates_for_source_subnet_2 =
create_certificate_chain(SOURCE_SUBNET_ID_2, &[TARGET_SUBNET_ID_2], 15);
for cert in &certificates_for_source_subnet_2[0..10] {
storage.persist(cert, None).await.unwrap();
}
for cert in &certificates_for_source_subnet_2[10..] {
storage.add_pending_certificate(cert).await.unwrap();
expected_pending_certificates.push((expected_pending_certificates_count, cert.clone()));
Expand Down

0 comments on commit dea7066

Please sign in to comment.