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

#133: Avoid code duplication in process_message_2 and process_message_3 #134

Merged

Conversation

malishav
Copy link
Contributor

@malishav malishav commented Nov 8, 2023

The PR removes common code between process_message_2 and process_message_3 into a single credential_check_or_fetch function. This is a first step to modularizing the steps that need to be done by the application code in terms of authentication credential processing.

@malishav malishav requested a review from geonnave November 8, 2023 13:23
Copy link
Collaborator

@geonnave geonnave left a comment

Choose a reason for hiding this comment

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

Looks good; left a minor comment below.

lib/src/edhoc.rs Outdated
public_key = public_key_expected;
let credentials_match = match id_cred_received {
IdCred::CompactKid(kid) => kid == kid_expected,
IdCred::FullCredential(cred_received) => credential_expected == cred_received,
Copy link
Collaborator

Choose a reason for hiding this comment

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

For consistency we could have:

            IdCred::CompactKid(kid_received) => kid_received == kid_expected,
            IdCred::FullCredential(cred_received) => credential_received == credential_expected,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed

@malishav malishav force-pushed the 133-refactor_process_message_2_3 branch from fc54564 to c8e1731 Compare November 9, 2023 09:30
@geonnave geonnave merged commit 444e2aa into openwsn-berkeley:main Nov 10, 2023
@geonnave geonnave added the enhancement New feature or request label Nov 10, 2023
@malishav malishav deleted the 133-refactor_process_message_2_3 branch November 15, 2023 08:51
@geonnave geonnave mentioned this pull request Nov 15, 2023
geonnave added a commit that referenced this pull request Nov 15, 2023
To solve conflicts with main, I had to temporarily overwrite the
changes of PR #134 (avoid code duplication).
This will be solved in a follow up commit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants