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

Supporting FOG enabled accounts #254

Merged
merged 19 commits into from
Feb 18, 2022
Merged

Conversation

briancorbin
Copy link
Contributor

@briancorbin briancorbin commented Feb 16, 2022

This allows accounts to be imported or created with FOG credentials. When that happens, a few things must happen to mimic how the mobile SDK works so that the accounts are compatible and all txos appear on on the mobile SDK.

  1. A FOG account sends its change to its main subaddress (index 0) instead of the default change subaddress (index 1)
  2. Disable the ability to assign or get subaddresses other than the main subaddress (index 0)

@@ -114,6 +114,9 @@ pub enum JsonCommandRequest {
},
create_account {
name: Option<String>,
fog_report_url: Option<String>,
Copy link
Contributor

@Shramp Shramp Feb 17, 2022

Choose a reason for hiding this comment

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

no idea if this would be the rust way of doing things, but in TS I thought it was nice to keep the number of args passed into a fn down to ~<=3 and group similar arguments into a single object. For this fn, we could do:

fn create_account(name: string, fog_options: Option<FogOptions>) ...

struct FogOptions: {
        fog_report_url: String,
        fog_report_id: String,
        fog_authority_spki: String,
}

Could do the same for AccountService.import_account, and import_account_from_legacy_root_entropy. Then we wouldn't have to pass in a bunch of nones when calling these functions for non-fog accounts.

what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nvm, after chatting and realizing that this would require an API change, let's not mess with it now

Copy link
Contributor

@Shramp Shramp left a comment

Choose a reason for hiding this comment

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

lgtm!

@briancorbin briancorbin merged commit c8b5326 into develop Feb 18, 2022
@briancorbin briancorbin deleted the feature/fog-enabled-accounts branch February 18, 2022 19:40
christian-oudard pushed a commit that referenced this pull request Feb 18, 2022
Co-authored-by: Colin Carey <colin.carey1@gmail.com>
briancorbin added a commit that referenced this pull request Mar 4, 2022
* adding max limit to some API endpoints (#248)

* Remove unnecessary foreign keys from gift_codes table. (#249)

* Update readme installation instructions (#250)

* Enable foreign key constraints. Fix transient FK errors when deleting an account. (#251)

* Remove foreign key check before running migrations. This allows databases with existing foreign key errors to be fixed by the migrations. (#252)

* Fix a bug causing sync to create many tiny chunks. (#253)

* Remove target block arg from manually sync account fn (#255)

* Initial action to build containers for tag pushes (#256)

* Supporting FOG enabled accounts (#254)

* FOG Creds default to empty string if not provided from API (#257)

* update readme (#259)

* Api key guard (#205)

* fixing issue with ledger not syncing automatically (#261)

* sync up to the last block instead of excluding it (#262)
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