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

Don't attempt to vote/revoke without the private key #2062

Merged
merged 1 commit into from Jun 18, 2021

Conversation

jrick
Copy link
Member

@jrick jrick commented Jun 17, 2021

This error is commonly seen on ticketbuyer wallets which will attempt
to revoke any missed tickets, even though an importing voting xpub
account prevents access to the private keys to do so. This is not an
error, and should be detected up front before ever constructing the
unsigned vote/revocation.

When the private key is recorded by the wallet, but the wallet or
account is locked, the wallet will still error when failing to sign
the revocation.

This error is commonly seen on ticketbuyer wallets which will attempt
to revoke any missed tickets, even though an importing voting xpub
account prevents access to the private keys to do so.  This is not an
error, and should be detected up front before ever constructing the
unsigned vote/revocation.

When the private key is recorded by the wallet, but the wallet or
account is locked, the wallet will still error when failing to sign
the revocation.
Copy link
Member

@JoeGruffins JoeGruffins left a comment

Choose a reason for hiding this comment

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

Works well for me.

Comment on lines +2328 to +2331
id, err := addressID(addr)
if err != nil {
return false, nil
}
Copy link
Member

Choose a reason for hiding this comment

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

Better to return err here because we were expecting P2PK or P2PKH addr which should have an ID? ofc not sure tho.

Copy link
Member

Choose a reason for hiding this comment

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

addressIDuses a check for Hash160er which also supports P2SH, Schnorr, etc. This will really only err for a completely unknown address type, in which case we don`t have the private key anyway, so this fulfills the API for this func.

Copy link
Member

@matheusd matheusd left a comment

Choose a reason for hiding this comment

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

Just noting that checking haveKey is sufficient for all currently existing uses of hasVotingAuthority, but I guess it could be useful to differentiate between owned && !haveKey vs owned && haveKey in the future.

Comment on lines +2328 to +2331
id, err := addressID(addr)
if err != nil {
return false, nil
}
Copy link
Member

Choose a reason for hiding this comment

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

addressIDuses a check for Hash160er which also supports P2SH, Schnorr, etc. This will really only err for a completely unknown address type, in which case we don`t have the private key anyway, so this fulfills the API for this func.

@jrick jrick merged commit b4b4c72 into decred:master Jun 18, 2021
@jrick jrick deleted the noprivkeytorevoke branch June 18, 2021 14:23
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