novasamatech / parity-signer

Air-gapped crypto wallet.
https://www.parity.io/signer/
GNU General Public License v3.0
554 stars 164 forks source link

Trying to set up Polkadot Vault with a custom Viewer Certificate can lead to irrecoverable crash #1846

Closed alecdwm closed 8 months ago

alecdwm commented 1 year ago

Hey! I'm going to list a few scenarios for a user, one of them leads to a crash which requires the user to delete and re-install the app from scratch.

I'm hoping this can be fixed!

Scenario 1 (no crash):

  1. Set up fresh vault
  2. Remove viewer certificate
  3. Scan chainspec with new viewer certificate
  4. Add key set and carry on

Scenario 2 (crash):

  1. Set up fresh vault
  2. Remove viewer certificate
  3. Add key set
  4. Scan chainspec with new viewer certificate
  5. Close QR scanner -> app crashes
  6. Open app, enter passcode -> app crashes

Scenario 3 (no crash):

  1. Set up fresh vault
  2. Remove viewer certificate
  3. Add key set
  4. Remove the three default derived keys (with blank derivation paths) from the key set (Polkadot, Kusama, Westend)
  5. Scan chainspec with new viewer certicate
  6. Close QR scanner and carry on
alecdwm commented 1 year ago

I'm guessing that the three root accounts created by default are somehow set up with the default viewer certificate. Perhaps the fix is to not create them if it has been removed?

krodak commented 1 year ago

@alecdwm thanks for detailed description on the issue 🙇🏻‍♂️ Pushed changes that will fix error presentation and avoid crash on iOS app 🙏🏻

So now @alecdwm if you run this from Xcode you should see following (keys are not deleted, just not returned from Rust state machine as error state takes precedence)

@prybalko I've started #1847 for this. Currently to fetch list of key sets, we are making navigation call to .start and then .navbarKeys (as currently with native navigation, each Service that interacts with backend through navigation state, assumes start from .start state), we are getting following error on every call to .start or consequent calls that tries to transition to different state:

▿ Optional<ActionResult>
  ▿ some : ActionResult
    - screenLabel : "Select seed"
    - back : false
    - footer : true
    ▿ footerButton : Optional<FooterButton>
      - some : PolkadotVault.FooterButton.keys
    ▿ rightButton : Optional<RightButton>
      - some : PolkadotVault.RightButton.newSeed
    - screenNameType : PolkadotVault.ScreenNameType.h1
    ▿ screenData : ScreenData
      ▿ settings : 1 element
        ▿ f : MSettings
          - publicKey : nil
          - identicon : nil
          - encryption : nil
          - error : nil
    - modalData : nil
    ▿ alertData : Optional<AlertData>
      ▿ some : AlertData
        ▿ errorData : 1 element
          - f : "Could not find network specs for network specs key 0191b171bb158e2d3848fa23a9f1c25182fb8e20313b2c1eb49219da7a70ce90c3."

I followed Scenario 2, scanned Ajuna chain spec QR code: image

prybalko commented 1 year ago

It seems that default networks are not being cleaned up after the general verifier has been removed. I'll fix that

krodak commented 8 months ago

Resolved as of current version