-
Notifications
You must be signed in to change notification settings - Fork 12
perf(contract): contract should not store full attestation submission #1663
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
d0579af
fa86044
695ee21
6358418
0e46166
c694b84
8321e49
6ad6e5a
39489d5
986c431
42a4524
aec3a44
c3ce22a
c20900d
8781ce0
b86a281
a4d14f1
177e0c1
a6c7a01
fba0bc7
602918a
80d1162
d80d86f
59a028c
3eb4d5c
ca1cd2b
82036b5
a918aff
673963b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -29,6 +29,54 @@ pub enum Attestation { | |||||
| Mock(MockAttestation), | ||||||
| } | ||||||
|
|
||||||
| #[derive( | ||||||
| Clone, | ||||||
| Debug, | ||||||
| Eq, | ||||||
| PartialEq, | ||||||
| Ord, | ||||||
| PartialOrd, | ||||||
| Hash, | ||||||
| Serialize, | ||||||
| Deserialize, | ||||||
| BorshSerialize, | ||||||
| BorshDeserialize, | ||||||
| )] | ||||||
| #[cfg_attr( | ||||||
| all(feature = "abi", not(target_arch = "wasm32")), | ||||||
| derive(schemars::JsonSchema) | ||||||
| )] | ||||||
| pub enum VerifiedAttestation { | ||||||
| Dtack(VerifiedDstackAttestation), | ||||||
| Mock(MockAttestation), | ||||||
| } | ||||||
|
|
||||||
| #[derive( | ||||||
| Clone, | ||||||
| Debug, | ||||||
| Eq, | ||||||
| PartialEq, | ||||||
| Ord, | ||||||
| PartialOrd, | ||||||
| Hash, | ||||||
| Serialize, | ||||||
| Deserialize, | ||||||
| BorshSerialize, | ||||||
| BorshDeserialize, | ||||||
| )] | ||||||
| #[cfg_attr( | ||||||
| all(feature = "abi", not(target_arch = "wasm32")), | ||||||
| derive(schemars::JsonSchema) | ||||||
| )] | ||||||
| pub struct VerifiedDstackAttestation { | ||||||
| /// The digest of the MPC image running. | ||||||
| pub mpc_image_hash: [u8; 32], | ||||||
| /// The digest of the MPC image running. | ||||||
| pub launcher_compose_hash: [u8; 32], | ||||||
| /// Unix timestamp for when the attestation was created. | ||||||
| pub creation_time_stamp_seonds: u64, | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
you will need to change in 5 places |
||||||
| } | ||||||
|
|
||||||
| #[derive( | ||||||
| Clone, | ||||||
| Eq, | ||||||
|
|
@@ -78,8 +126,8 @@ pub enum MockAttestation { | |||||
| WithConstraints { | ||||||
| mpc_docker_image_hash: Option<Sha256Digest>, | ||||||
| launcher_docker_compose_hash: Option<Sha256Digest>, | ||||||
| /// Unix time stamp for when this attestation expires. | ||||||
| expiry_time_stamp_seconds: Option<u64>, | ||||||
| /// Unix time stamp for when this attestation was created. | ||||||
| creation_time_stamp_seconds: Option<u64>, | ||||||
| }, | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,10 @@ pub struct InitConfig { | |
| pub cleanup_orphaned_node_migrations_tera_gas: Option<u64>, | ||
| /// Prepaid gas for a `remove_non_participant_update_votes` call. | ||
| pub remove_non_participant_update_votes_tera_gas: Option<u64>, | ||
| /// Contract defined time to live for node attestations. Participants | ||
| /// must refresh their attestations before `attestation_max_validity_duration_seconds` | ||
| /// to avoid their attestation being invalidated. | ||
| pub attestation_max_validity_duration_seconds: Option<u64>, | ||
| } | ||
|
|
||
| /// Configuration parameters of the contract. | ||
|
|
@@ -87,6 +91,10 @@ pub struct Config { | |
| pub cleanup_orphaned_node_migrations_tera_gas: u64, | ||
| /// Prepaid gas for a `remove_non_participant_update_votes` call. | ||
| pub remove_non_participant_update_votes_tera_gas: u64, | ||
| /// Contract defined time to live for node attestations. Participants | ||
| /// must refresh their attestations before `attestation_max_validity_duration_seconds` | ||
| /// to avoid their attestation being invalidated. | ||
| pub attestation_max_validity_duration_seconds: u64, | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
|
|
@@ -108,6 +116,7 @@ mod tests { | |
| clean_tee_status_tera_gas: Some(10), | ||
| cleanup_orphaned_node_migrations_tera_gas: Some(3), | ||
| remove_non_participant_update_votes_tera_gas: Some(5), | ||
| attestation_max_validity_duration_seconds: Some(1912312), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this equals to 22 days, why did you choose this number?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test is just testing a serialization and deserialization works as expected. The values are not used for anything here. |
||
| }; | ||
| let json = serde_json::to_string(&original_config).unwrap(); | ||
| let serialized_and_deserialized_config: InitConfig = serde_json::from_str(&json).unwrap(); | ||
|
|
@@ -155,6 +164,7 @@ mod tests { | |
| clean_tee_status_tera_gas: None, | ||
| cleanup_orphaned_node_migrations_tera_gas: None, | ||
| remove_non_participant_update_votes_tera_gas: None, | ||
| attestation_max_validity_duration_seconds: None, | ||
| }; | ||
|
|
||
| assert_eq!(default_config, config_with_all_values_as_none); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where do we get the time from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment is s currently misleading. The timestamp is taken from "now" time argument during the initial verification, so it's the timestamp of when the contract first saw the attestation and not the creation date.
In a follow up PR we should refactor this internal implementation to use the timestamp embedded in the attestation instead.