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

Expose securified security state and expose methods that relied on unsecured entity control on hosts #375

Merged
merged 15 commits into from
Feb 10, 2025

Conversation

micbakos-rdx
Copy link
Contributor

  • Export a method that checks if an account is legacy olympia.
  • Export a method that returns the unsecured controlling factor instance for accounts and personas.
  • The BDFS error check, as discussed, is not needed so I removed the implementation I temporarily wrote in sargon.
  • Expose secured entity control to hosts.

@micbakos-rdx micbakos-rdx added Rust 🦀 Changes in Rust Sargon Kotlin 🤖 Changes in Kotlin Sargon UniFFI 🦄 Changes in UniFFI exported APIs labels Feb 7, 2025
@micbakos-rdx micbakos-rdx self-assigned this Feb 7, 2025
@micbakos-rdx micbakos-rdx changed the title Expose securified security state and expose methods that relied on unsecurified security state on hosts Expose securified security state and expose methods that relied on unsecured entity control on hosts Feb 7, 2025
Copy link

codecov bot commented Feb 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.68%. Comparing base (d3a9f85) to head (f99d389).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #375      +/-   ##
==========================================
+ Coverage   92.64%   92.68%   +0.03%     
==========================================
  Files        1180     1184       +4     
  Lines       26658    26689      +31     
  Branches       77       85       +8     
==========================================
+ Hits        24698    24737      +39     
+ Misses       1949     1941       -8     
  Partials       11       11              
Flag Coverage Δ
kotlin 98.86% <100.00%> (+1.23%) ⬆️
rust 92.27% <100.00%> (-0.04%) ⬇️
swift 92.87% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


impl AccountIsLegacyOlympia for Account {
fn is_legacy_olympia(&self) -> bool {
let Some(unsecured_entity_control) = self.security_state.as_unsecured()
Copy link
Contributor

Choose a reason for hiding this comment

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

You can just self.address.is_legacy()

}

impl EntityUnsecuredControllingFactorInstance for Account {
fn unsecured_controlling_factor_instance(
Copy link
Contributor

Choose a reason for hiding this comment

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

I want to avoid this.

Why are we adding it?

Temporarily as part of a process of transitioning?

As so said the other day, I don't want hosts to concern themselves with FactorInstances / PublicKeys at all

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Temporarily as part of a process of transitioning?

Yes exactly for this reason. The refactoring effort is too much for now.

@@ -23,8 +27,10 @@ impl From<InternalEntitySecurityState> for EntitySecurityState {
value: value.into(),
}
}
InternalEntitySecurityState::Securified { value: _ } => {
panic!("Securified state not yet supported in the Wallet")
InternalEntitySecurityState::Securified { value } => {
Copy link
Contributor

@GhenadieVP GhenadieVP Feb 7, 2025

Choose a reason for hiding this comment

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

With this added you should be able to use InternalConversion macro on EntitySecurityState

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right will do

@micbakos-rdx
Copy link
Contributor Author

Will keep this PR open until iOS host integrates with it.

@micbakos-rdx micbakos-rdx force-pushed the mib/ABW-4155-remove-usages-security-state-host branch from 216549b to f99d389 Compare February 10, 2025 10:32
@micbakos-rdx micbakos-rdx merged commit a18623a into main Feb 10, 2025
13 checks passed
@micbakos-rdx micbakos-rdx deleted the mib/ABW-4155-remove-usages-security-state-host branch February 10, 2025 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Kotlin 🤖 Changes in Kotlin Sargon Rust 🦀 Changes in Rust Sargon UniFFI 🦄 Changes in UniFFI exported APIs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants