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

[wallet] Use BrowserView ConfirmDialog on SignTx #3519

Merged
merged 3 commits into from Jun 29, 2021

Conversation

matheusd
Copy link
Member

@matheusd matheusd commented Jun 25, 2021

Requires #3515

This switches the wallet's SignTx function to confirm with the user the transaction to be signed via the recently introduced confirmation dialog that runs in a BrowserView.

This ensures that any attempts by UI code to modify the transaction are visible to the user before the tx is sent to the underlying wallet to be signed.

The dialog is formatted such that only outputs and amounts that are not returning to the wallet (i.e. addresses in outputs that do not correspond to one of the wallet's BIP0044 account) are shown.

@matheusd matheusd changed the title [wallet [wallet] Use BrowserView ConfirmDialog on SignTx Jun 25, 2021
Copy link
Collaborator

@bgptr bgptr left a comment

Choose a reason for hiding this comment

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

LGTM
Tested in dev and production mode on linux.

This ensures UI code does not have the ability to change the tx to be signed
before it is passed to the wallet.
Copy link
Member

@alexlyp alexlyp left a comment

Choose a reason for hiding this comment

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

tACK

@alexlyp alexlyp merged commit ad2d8c3 into decred:master Jun 29, 2021
@matheusd matheusd deleted the confirm-sign-tx branch June 30, 2021 10:38
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.

None yet

3 participants