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

Implementation of Olaf #104

Closed
wants to merge 1 commit into from
Closed

Implementation of Olaf #104

wants to merge 1 commit into from

Conversation

Fiono11
Copy link

@Fiono11 Fiono11 commented Apr 19, 2024

This PR builds upon the previous PR #102 and implements the Olaf protocol. It is part of deliverable 1 of Web3 Foundation's grant w3f/Grants-Program#2250.

@Fiono11 Fiono11 force-pushed the olaf branch 3 times, most recently from e07a49d to 6f24720 Compare April 23, 2024 20:53

# Format comments
comment_width = 100
wrap_comments = true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your git GUI has seemingly pulled recent changes from master into its own commit. I'd expect git handles this fine, likely disapears once rebased onto master, but it's odd behavior that indicates issues with the git GUI.

Did you make any other changes to this PR since 25 April?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I messed up...Pushed some changes directly to master instead of this branch and then pulled them.

@@ -22,17 +22,25 @@ curve25519-dalek = { version = "4.1.0", default-features = false, features = [
"zeroize",
"precomputed-tables",
"legacy_compatibility",
"rand_core",
"serde",
Copy link
Collaborator

Choose a reason for hiding this comment

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

How is rand_core being added as a feature? rand_core is not an ptional dependency

Copy link
Author

Choose a reason for hiding this comment

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

I am not sure, does it have to be?

Copy link
Collaborator

Choose a reason for hiding this comment

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

rand_core is mandatory. why is this feature being added?

Copy link
Author

Choose a reason for hiding this comment

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

Because without it it does not compile, I don't know why.

@Fiono11 Fiono11 mentioned this pull request May 14, 2024
@Fiono11
Copy link
Author

Fiono11 commented May 14, 2024

Replaced by #107.

@Fiono11 Fiono11 closed this May 14, 2024
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.

2 participants