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

feat(abstract-utxo): support trustless change outputs from explaintx #4454

Merged
merged 1 commit into from
Apr 19, 2024

Conversation

saravanan7mani
Copy link
Contributor

@saravanan7mani saravanan7mani commented Apr 19, 2024

explainTransaction uses change addresses from platform for its change outputs
Now, they are trustlessly deduced from PSBT

TICKET: BTC-1112

@saravanan7mani saravanan7mani changed the title feat(abstract-utxo): support for trustless change outputs from explai… feat(abstract-utxo): support trustless change outputs from explaintx Apr 19, 2024
@saravanan7mani saravanan7mani force-pushed the BTC-1112-change-outputs branch 5 times, most recently from c04c726 to da11751 Compare April 19, 2024 11:38
@saravanan7mani saravanan7mani marked this pull request as ready for review April 19, 2024 12:05
@saravanan7mani saravanan7mani requested review from a team as code owners April 19, 2024 12:05
.findInternalOutputIndices(psbt)
.map((i) => utxolib.address.fromOutputScript(txOutputs[i].script, network));
} catch (e) {
if (e instanceof Error && e.message === 'No multi sig input found') {
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of relying on error message we should have a dedicated error class

class ErrorNoMultiSigInputFound extends Error { ... }

...
if (e instanceof ErrorNoMultiSigInputFound) { ... }

but why not propagate the error instead?

Copy link
Contributor Author

@saravanan7mani saravanan7mani Apr 19, 2024

Choose a reason for hiding this comment

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

why not propagate the error instead?

As per existing test cases, explainTransaction should return empty change outputs for a PSBT with no multi-sig inputs (ex: PSBT only with p2shP2pk). It makes sense because it's not in explainTransaction's scope to assert for presence of mult-sig inputs in PSBTs.

explainTransaction uses change addresses from platform for its change outputs
Now, they are trustlessly deduced from PSBT

Ticket: BTC-1112
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