Skip to content
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

feat: Basic verification logic for mock MainPOD #43

Merged
merged 7 commits into from
Feb 10, 2025
Merged

Conversation

ax0
Copy link
Collaborator

@ax0 ax0 commented Feb 7, 2025

This PR fills in some of the gaps in the verification of mock MainPODs.

@arnaucube arnaucube added this to the Milestone 1 milestone Feb 7, 2025
@arnaucube arnaucube mentioned this pull request Feb 7, 2025
4 tasks
@arnaucube arnaucube requested a review from ed255 February 7, 2025 16:22
Copy link
Collaborator

@ed255 ed255 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@@ -0,0 +1 @@

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀

// get the input_statements from the self.statements
let input_statements = &self.statements[input_statement_offset..];
// get the id out of the public statements, and ensure it is equal to self.id
let ids_match = self.id == PodId(hash_statements(&self.public_statements).unwrap());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for myself: the MockMainPod has the field public_statements which is built from statements[statements.len() - params.max_public_statements..], so we could get rid of it.

&& if let StatementArg::Key(AnchoredKey(pod_id, key_hash)) = s.1[0] {
pod_id == SELF && key_hash == hash_str(KEY_TYPE)
} else {
false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be a malformed invalid POD right?

if let StatementArg::Key(ak) = &s.1[0] {
vec![(i, ak.1, ak.0)]
} else {
vec![]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be a malformed POD right?

.map(|(i, s)| {
(
// Separate private from public statements.
if i < self.params.max_priv_statements() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that I can have a ValueOf for the same key with different values as long as one is in priv_statements and the other is in pub_statements?

.check(s.clone())
})
.collect::<Result<Vec<_>>>()
.unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would panic if any check returns Err right?

}
// TODO: Argument checking.
/// Checks the given operation against a statement.
pub fn check(&self, output_statement: Statement) -> Result<bool> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About the return type:
Is the Err case reserved for malformed operation/statements, and the Ok(false) for properly formed data but invalid deduction?

,
// Check that the operation acts on a statement *and* the
// output is equal to this statement.
CopyStatement => Ok(output_statement == self.args()[0].statement()?)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a design question: In the Devcon implementation Operation and Statement were more strongly typed:
https://github.com/0xPARC/parcnet/blob/f7ef5e2fedea10f349c6343fb24d84374edbbad2/pod2/src/pod/operation.rs#L17-L33
https://github.com/0xPARC/parcnet/blob/f7ef5e2fedea10f349c6343fb24d84374edbbad2/pod2/src/pod/statement.rs#L79-L90

Here they are very generic, where the arguments are not strongly typed to be correct.
This generic code leads to a more verbose check implementation, but I also think it has benefits in other parts of the code.

Do you have any thoughts on this approach VS the previous one? Do you like this implementation of check?

@ed255 ed255 linked an issue Feb 10, 2025 that may be closed by this pull request
4 tasks
@ed255 ed255 mentioned this pull request Feb 10, 2025
@ed255 ed255 merged commit 90e9782 into 0xPARC:main Feb 10, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

implement MainMockPod
3 participants