BlockchainCommons / GordianSeedTool-iOS

Cryptographic Seed Manager for iOS
Other
36 stars 8 forks source link

Sparrow Integration Failed in GST 1.6 (Testflight) #183

Closed shannona closed 12 months ago

shannona commented 1 year ago

I created a new seed in the current GST 1.6 from Testflight, choose the import from Seed Tool option in Sparrow, and did what it expected, which was Authenticate > Derive Key > Other Key Derivations > Master Key > Secondary Derivation / Account Descriptor. When I shared the QR, I got: "Error parsing UR CBOR". This should be what it was trying to share:

ur:crypto-account/oeadcylpvefyjeaolttaadeetaadmutaaddlonaxhdclaxhkfzdphtkplevtqzprkgnnsacagesgctzmspytctdstocsbgmkamurrdpffmtsseaahdcxpdnbdysgfeaycfsegwmhwfjewzfwdmesrlqdhglagahkytcflbrtsedraefwbweyamtaaddyoeadlncsdwykaeykaeykaocylpvefyjeattaaddyoyadlslraewkadwklawkaycypsjpsbfntaadeetaadmhtaadmwtaaddlonaxhdclaouypakomsrlhlqzvwlymesadevybkueqdoxhdcximrdhgfhtybwvyhdkeyklddpmwaahdcxwnvsaxgsdldtmurknyfpcedyiaiopautyafrpejoaxjncfhhhtpfdetteotnknsramtaaddyoeadlncsehykaeykaeykaocylpvefyjeattaaddyoyadlslraewkadwklawkaycyoeyagmmstaadeetaadmwtaaddlonaxhdclaorernoyonguhlfsdecnmohnuydwnnchjpuyroftdnaadkatcfkirdtkispsiajycxaahdcxlyutkbnymklbdskesbihbelysedwlupklfmhnntlfwsegsvwwspkghkgvwheiejoamtaaddyoeadlncsghykaeykaeykaocylpvefyjeattaaddyoyadlslraewkadwklawkaycywyqzwzkptaadeetaadmhtaadnytaaddlonaxhdclaoiorpcxecynfloxtamhlsaarkkotpclcydahtamluhnimkertlgftknoerymobaneaahdcxrsatfdqdenvtspvdieindaztrpcazsknzsoywkghfhtyswdkkpjstbneayfncxoxamtaaddyoeadlfcsdpykaocylpvefyjeattaaddyoyadlslraewkadwklawkaycylpvefyjetaadeetaadmhtaadmetaadnytaaddlonaxhdclaoldpfglrkwntlvwlarsdmnbzefmghkeeeoteerdpfframuoidstpacswmnnutcwkkaahdcxhkrozmjneceoldplnluetscyjnbdhsldenontncafgdkmdvlsnidamlnvwfzsstaamtaaddyoeadlocsdyykaeykaeykadykaocylpvefyjeattaaddyoyadlslraewkadwklawkaycyrhimryuotaadeetaadmetaadnytaaddlonaxhdclaowysfahwsqdclsnfmwmjlgoeerhzoteherosnmyhkinolrlspdadsasswlarpvtpfaahdcxwybnryoewshycwghlfhhwtbscxpyylenpkolmoftgymksbdpvsgtlbdefhrswnyaamtaaddyoeadlocsdyykaeykaeykaoykaocylpvefyjeattaaddyoyadlslraewkadwklawkaycyrhimryuotaadeetaadnltaaddlonaxhdclaxpmctzmjzemjspsplgrlgtyvssffrzcrlgogojnmncwbdtytelblyhlflmtdnkifdaahdcxgytkgujnjtaszejprdpmoskseehkamvlcersoxdlghsglagmjkjewmrlpfonwldwamtaaddyoeadlncshfykaeykaeykaocylpvefyjeattaaddyoyadlslraewkadwklawkaycynsmwdtfzryrorncp

I reverted back to GST 1.5, the one in the Apple Store, and created a new seed and went through the same process, and it worked fine, so unless there was something problematic with the Seed I created in 1.6, something incompatible happened to the account descriptor between 1.5 and 1.6.

Looking at the CBOR, it looks like that might be the introduction of the crypto-keypath for children in entry 7 of each output descriptor. Can we get verification if that's all in spec (and for that matter that the entire account descriptor is in 1.6). I suspect that's the case, but want to double check first. If so, I'll alert Craig.

shannona commented 1 year ago

I realized my Sparrow was out of date and updated it to the newest (1.7.4). Same deal. If you want any further diagnostics, let me know.

wolfmcnally commented 1 year ago

You are right that this has to do with how we're encoding children in output descriptors. We introduced the use of "pair components" (see here) but failed to update our docs to reflect how we're encoding them in CBOR, which means @craigraw may not have implemented it in the same way SeedTool does, or may not have implemented it at all. This was my oversight.

This is the commit that fixes the docs:

https://github.com/BlockchainCommons/Research/pull/124/commits/f966a0513217a345a202f8c0570e03f794333dc2

Here's a restatement of the updated section:

; Metadata for the complete or partial derivation path of a key.
;
; `source-fingerprint`, if present, is the fingerprint of the
; ancestor key from which the associated key was derived.
;
; If `components` is empty, then `source-fingerprint` MUST be a fingerprint of
; a master key.
;
; `depth`, if present, represents the number of derivation steps in
; the path of the associated key, regardless of whether steps are present in the `components` element
; of this structure.

crypto-keypath = {
    components: [path-component], ; If empty, source-fingerprint MUST be present
    ? source-fingerprint: uint32 .ne 0 ; fingerprint of ancestor key, or master key if components is empty
    ? depth: uint8 ; 0 if this is a public key derived directly from a master key
}

path-component = (
    child-index-component /     ; A single child, possibly hardened
    child-range-component /     ; A specific range of children, all possibly hardened
    child-wildcard-component /  ; An inspecific range of children, all possibly hardened
    child-pair-component        ; Used in output descriptors,
                                ; see https://github.com/bitcoin/bitcoin/pull/22838
)

uint32 = uint .size 4
uint31 = uint32 .lt 0x80000000
child-index-component = (child-index, is-hardened)
child-range-component = ([child-index, child-index], is-hardened) ; [low, high] where low < high
child-wildcard-component = ([], is-hardened)
child-pair-component = [
    child-index-component,  ; Child to use for external addresses, possibly hardened
    child-index-component   ; Child to use for internal addresses, possibly hardened
]

child-index = uint31
is-hardened = bool

components = 1
source-fingerprint = 2
depth = 3

Here is the children entry of an output descriptor:

7: ; children
304( ; crypto-keypath
   {
      1: ; components
      [
         [0, false, 1, false],  ; child-pair-component
         [], false              ; child-wildcard-component
      ]
   }
),

Here is a text output descriptor:

wpkh([d632e6bf/84'/0'/0']xpub6Br2GVhLvcR5sg7oF3hRb2LJCeocBwgvCgVimeQvCC8R1RxHge5PcoGUTYbXyyxDSTW699MyBdyKLSgYpSovYcxqnWqbs9g164xRBWo9pVd/<0;1>/*)#9ns7mz8s

The children entry above represents the <0;1>/* part of the output descriptor.

All of the output descriptors SeedTool currently generates should conform to the syntax described above.

craigraw commented 1 year ago

Thanks Wolf. I've added support for pair and range path components in https://github.com/sparrowwallet/hummingbird/commit/99d779c657d73fb7dddff3a95924af9908284626.

craigraw commented 1 year ago

Sparrow v1.7.7 has been released with support for pair components, so I think this can be closed.

shannona commented 12 months ago

Verified it with 1.7.7 & closing.

Thanks, @craigraw