feat(abstract-utxo): add psbt support backup recovery #3755
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
saravanan7mani
force-pushed
the
psbtBackupkeyRecovery
branch
3 times, most recently
from
493a951
to
7cb08ff
Compare
saravanan7mani
changed the title
| @@ -62,16 +55,17 @@ export interface FormattedOfflineVaultTxInfo { | |||
| * @param txHex | |||
| * @returns {{txHex: *, txInfo: {unspents: *}, feeInfo: {}, coin: void}} | |||
| */ | |||
| function formatForOfflineVault<TNumber extends number | bigint = number>( | |||
There was a problem hiding this comment.
why would you take away TNumber here?
There was a problem hiding this comment.
As per new change, OfflineVaultTxInfo uses valueString to carry large values. value is always number. So no need to for Generics.
Comment on lines
+477
to
+481
| const safeRootWalletKeys = new RootWalletKeys( | ||
| rootWalletKeys.triple.map((bip32) => bip32.neutered()) as Triple<BIP32Interface>, | ||
| rootWalletKeys.derivationPrefixes | ||
| ); | ||
| const xPubs = safeRootWalletKeys.triple.map( |
There was a problem hiding this comment.
maybe we need RootWalletPublicKeys
Primarily what you mentioned (prevTx id change changes txHex) and also few cases returns PSBT hex instread transaction hex, new response fields |
saravanan7mani
force-pushed
the
psbtBackupkeyRecovery
branch
from
7cb08ff
to
8c385f0
Compare
OttoAllmendinger
requested changes
Jul 31, 2023
| @@ -1,42 +0,0 @@ | |||
| /** | |||
There was a problem hiding this comment.
this could have been a separate commit
| export interface OfflineVaultTxInfo<TNumber extends number | bigint = number> { | ||
| inputs: WalletUnspent<TNumber>[]; | ||
| export interface OfflineVaultTxInfo { | ||
| inputs: WalletUnspent<number>[]; |
There was a problem hiding this comment.
I suppose OfflineVault always takes number? Probably would have been a good separate commit as well.
| * Builds a funds recovery transaction without BitGo | ||
| * Builds a funds recovery transaction without BitGo. | ||
| * | ||
| * Returns transaction hex for unsigned sweep transaction, half signed backup recovery transaction with KRS provider (only keyternal), |
There was a problem hiding this comment.
Suggested change
| * Returns transaction hex for unsigned sweep transaction, half signed backup recovery transaction with KRS provider (only keyternal), | |
| * Returns transaction hex in legacy format for unsigned sweep transaction, half signed backup recovery transaction with KRS provider (only keyternal), |
| @@ -276,6 +283,7 @@ export async function backupKeyRecovery<TNumber extends number | bigint = number | |||
|
|
|||
| const isKrsRecovery = getIsKrsRecovery(params); | |||
| const isUnsignedSweep = getIsUnsignedSweep(params); | |||
| const respondWithTx = isUnsignedSweep || !isKrsRecovery || params.krsProvider === 'keyternal'; | |||
There was a problem hiding this comment.
Suggested change
| const respondWithTx = isUnsignedSweep || !isKrsRecovery || params.krsProvider === 'keyternal'; | |
| const responseFormat = (isUnsignedSweep || !isKrsRecovery || params.krsProvider === 'keyternal') ? 'legacy' : 'psbt'; |
|
|
||
| // Construct a transaction | ||
| txInfo.inputs = unspents; | ||
| txInfo.inputs = respondWithTx |
There was a problem hiding this comment.
Suggested change
| txInfo.inputs = respondWithTx | |
| txInfo.inputs = responseFormat === 'legacy' ? |
| @@ -203,32 +237,38 @@ function run( | |||
|
|
|||
| if (params.hasUserSignature && params.hasBackupSignature) { | |||
| it('has no placeholder signatures', function () { | |||
There was a problem hiding this comment.
Suggested change
| it('has no placeholder signatures', function () { | |
| it('has no placeholder signatures', function (this: mocha.Context) { |
then you can get rid of the @ts-expect-error further down
Comment on lines
+477
to
+481
| const safeRootWalletKeys = new RootWalletKeys( | ||
| rootWalletKeys.triple.map((bip32) => bip32.neutered()) as Triple<BIP32Interface>, | ||
| rootWalletKeys.derivationPrefixes | ||
| ); | ||
| const xPubs = safeRootWalletKeys.triple.map( |
There was a problem hiding this comment.
maybe we need RootWalletPublicKeys
saravanan7mani
force-pushed
the
psbtBackupkeyRecovery
branch
from
8c385f0
to
8ec4522
Compare
PSBT is used to build and sign backup recovery and unsigned sweep Ticket: BTC-317
saravanan7mani
force-pushed
the
psbtBackupkeyRecovery
branch
from
8ec4522
to
b312a86
Compare
OttoAllmendinger
approved these changes
Jul 31, 2023
zahin-mohammad
approved these changes
Jul 31, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.