Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Make implicit_provider optional #23

Merged
merged 1 commit into from
Apr 21, 2020

Conversation

ionut-arm
Copy link
Member

This commit makes the implicit_provider optional, allowing the
BasicClient to start with no such provider. When in this state, core
operations are allowed but crypto operations will fail with
NoProvider.

Signed-off-by: Ionut Mihalcea ionut.mihalcea@arm.com

@ionut-arm ionut-arm added the enhancement New feature or request label Apr 21, 2020
@ionut-arm ionut-arm requested review from hug-dev and anta5010 April 21, 2020 11:18
@ionut-arm ionut-arm self-assigned this Apr 21, 2020
Copy link
Member

@hug-dev hug-dev left a comment

Choose a reason for hiding this comment

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

Thanks!

Comment on lines +517 to 523
fn can_provide_crypto(&self) -> Result<ProviderID> {
match self.implicit_provider {
ProviderID::Core => Err(Error::Client(ClientErrorKind::InvalidProvider)),
_ => Ok(()),
None => Err(Error::Client(ClientErrorKind::NoProvider)),
Some(ProviderID::Core) => Err(Error::Client(ClientErrorKind::InvalidProvider)),
Some(crypto_provider) => Ok(crypto_provider),
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Pretty nice!

/// and it is the responsibility of the user to identify and set an appropriate
/// one. As such, it is critical that before attempting to use cryptographic
/// operations users call [`list_providers`](#method.list_providers)
/// and [`list_provider_operations`](#method.list_provider_operations)
Copy link
Member

Choose a reason for hiding this comment

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

Not totally related with this PR but is there a reason why using list_provider_operations instead of list_opcodes?

Copy link
Member Author

@ionut-arm ionut-arm Apr 21, 2020

Choose a reason for hiding this comment

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

I think that sounded more developer-friendly to me at the time, but maybe the low-level one should be named consistently - I'll change

Copy link
Member

@hug-dev hug-dev Apr 21, 2020

Choose a reason for hiding this comment

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

Thanks! I think so too.

This commit makes the `implicit_provider` optional, allowing the
`BasicClient` to start with no such provider. When in this state, core
operations are allowed but crypto operations will fail with
`NoProvider`.

Signed-off-by: Ionut Mihalcea <ionut.mihalcea@arm.com>
Copy link
Member

@hug-dev hug-dev left a comment

Choose a reason for hiding this comment

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

Thanks!

@ionut-arm ionut-arm merged commit 2b27aa9 into parallaxsecond:master Apr 21, 2020
@ionut-arm ionut-arm deleted the option-provider branch April 21, 2020 13:26
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants