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

add ability to show a receiving address on ledger screen #3538

Merged
merged 4 commits into from
Jan 10, 2018

Conversation

mzhou
Copy link
Contributor

@mzhou mzhou commented Dec 17, 2017

No description provided.

@coveralls
Copy link

coveralls commented Dec 17, 2017

Coverage Status

Coverage decreased (-0.005%) to 42.927% when pulling 8d2e990 on mzhou:ledger_confirm_address into 3851e78 on spesmilo:master.

@SomberNight
Copy link
Member

Tested, and works :)

However, there is equivalent functionality for Trezor (and KeepKey and Digital Bitbox), implemented differently. I think it would be better if this PR was made to be similar to that.

run_hook('receive_menu', menu, addrs, self.wallet)

@hook
def receive_menu(self, menu, addrs, wallet):
if type(wallet) is not Standard_Wallet:
return
keystore = wallet.get_keystore()
if type(keystore) == self.keystore_class and len(addrs) == 1:
def show_address():
keystore.thread.add(partial(self.show_address, wallet, addrs[0]))
menu.addAction(_("Show on %s") % self.device, show_address)

@coveralls
Copy link

coveralls commented Dec 24, 2017

Coverage Status

Coverage decreased (-0.01%) to 42.856% when pulling 4df087c on mzhou:ledger_confirm_address into 13bf539 on spesmilo:master.

@coveralls
Copy link

coveralls commented Dec 24, 2017

Coverage Status

Coverage decreased (-0.009%) to 42.861% when pulling 4df087c on mzhou:ledger_confirm_address into 13bf539 on spesmilo:master.

def show_address(self, sequence, txin_type):
self.signing = True
# prompt for the PIN before displaying the dialog if necessary
client = self.get_client()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this variable necessary?

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

4 participants