Skip to content

fix(bitgo): attempt to sign using fallback derivation for v1 wallet#2254

Merged
tylerlevine merged 1 commit into
masterfrom
BG-46497-add-v1-derivation-path-fallback
May 5, 2022
Merged

fix(bitgo): attempt to sign using fallback derivation for v1 wallet#2254
tylerlevine merged 1 commit into
masterfrom
BG-46497-add-v1-derivation-path-fallback

Conversation

@tylerlevine

Copy link
Copy Markdown
Contributor

We have seen an issue for at least one production v1 wallet where an
input is unable to be signed for due to using an incorrect derivation
path for the user keychain. We believe this wallet has a non-standard
derivation scheme may have been broken in a change to the v1
transaction builder.

In the event of a signing failure, try to use a fallback path derivation
scheme which we believe will work for the wallet in question. If the
fallback path derivation fails, log the error which caused the fallback
derivation signing to fail, but throw the original error to maintain
previous error behavior.

Ticket: BG-46497

We have seen an issue for at least one production v1 wallet where an
input is unable to be signed for due to using an incorrect derivation
path for the user keychain. We believe this wallet has a non-standard
derivation scheme may have been broken in a change to the v1
transaction builder.

In the event of a signing failure, try to use a fallback path derivation
scheme which we believe will work for the wallet in question. If the
fallback path derivation fails, log the error which caused the fallback
derivation signing to fail, but throw the original error to maintain
previous error behavior.

Ticket: BG-46497
@codecov

codecov Bot commented May 4, 2022

Copy link
Copy Markdown

Codecov Report

Merging #2254 (433620d) into master (bfdc82c) will increase coverage by 52.61%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master    #2254       +/-   ##
===========================================
+ Coverage        0   52.61%   +52.61%     
===========================================
  Files           0        6        +6     
  Lines           0      574      +574     
  Branches        0       94       +94     
===========================================
+ Hits            0      302      +302     
- Misses          0      262      +262     
- Partials        0       10       +10     
Flag Coverage Δ
unit 52.61% <ø> (?)

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

Impacted Files Coverage Δ
modules/express/src/errors.ts 82.35% <0.00%> (ø)
modules/express/src/expressApp.ts 89.51% <0.00%> (ø)
modules/express/src/args.ts 100.00% <0.00%> (ø)
modules/express/src/config.ts 94.73% <0.00%> (ø)
modules/express/src/retryPromise.ts 11.76% <0.00%> (ø)
modules/express/src/clientRoutes.ts 32.48% <0.00%> (ø)

@sangaman sangaman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, interested to see if this resolves the underlying issue.

@tylerlevine tylerlevine merged commit bfa6347 into master May 5, 2022
@brandonblack brandonblack deleted the BG-46497-add-v1-derivation-path-fallback branch August 3, 2022 14:51
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.

3 participants