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

[GUI][Bug] Fix editing of CS address labels #1588

Merged
merged 1 commit into from
May 5, 2020

Conversation

Fuzzbawls
Copy link
Collaborator

Fixes a bug that prevented the editing of Cold Staking address labels in
the address book due to the passing of an empty purpose, which
CWallet::ParseIntoAddress() needs in order to determine if an address
is a cold staking type address.

@Fuzzbawls Fuzzbawls self-assigned this May 1, 2020
@Fuzzbawls Fuzzbawls force-pushed the 2020_cs-label-edit branch 2 times, most recently from 3719984 to ef83a3d Compare May 3, 2020 11:01
@Fuzzbawls Fuzzbawls added this to the 4.1.0 milestone May 3, 2020
@Fuzzbawls Fuzzbawls added the Needs Backport Placeholder tag for anything needing a backport to prior version branches label May 3, 2020
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

Concept ACK.
Just some points on the proposed code changes.

src/wallet/wallet.cpp Outdated Show resolved Hide resolved
src/wallet/wallet.cpp Outdated Show resolved Hide resolved
src/wallet/wallet.cpp Outdated Show resolved Hide resolved
std::string addressStr = ParseIntoAddress(address, strPurpose).ToString();
if (!strPurpose.empty() && !CWalletDB(strWalletFile).WritePurpose(addressStr, strPurpose))
std::string addressStr = ParseIntoAddress(address, (strPurpose.empty() && fUpdated) ? mapAddressBook.at(address).purpose : strPurpose).ToString();
if ((!fUpdated || fUpdatePurpose) && !CWalletDB(strWalletFile).WritePurpose(addressStr, strPurpose))
Copy link

Choose a reason for hiding this comment

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

To be totally sure over the write purpose process, would use the IsMine method call that is few lines above to verify that the address is really from this wallet (no need to do any extra processing, just use what we already have here). And throw an exception if the wallet, for whatever reason, is trying to store an external address with the receive/cold_staking purpose.

@Fuzzbawls Fuzzbawls force-pushed the 2020_cs-label-edit branch from ef83a3d to f6aa59c Compare May 4, 2020 09:19
Fixes a bug that prevented the editing of Cold Staking address labels in
the address book due to the passing of an empty purpose, which
`CWallet::ParseIntoAddress()` needs in order to determine if an address
is a cold staking type address.
@Fuzzbawls Fuzzbawls force-pushed the 2020_cs-label-edit branch from f6aa59c to 805e436 Compare May 5, 2020 10:08
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

ACK 805e436

Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

utACK 805e436

@random-zebra random-zebra merged commit 7694d5f into PIVX-Project:master May 5, 2020
Fuzzbawls added a commit to Fuzzbawls/PIVX that referenced this pull request May 5, 2020
Fixes a bug that prevented the editing of Cold Staking address labels in
the address book due to the passing of an empty purpose, which
`CWallet::ParseIntoAddress()` needs in order to determine if an address
is a cold staking type address.

Github-Pull: PIVX-Project#1588
Rebased-From: 805e436
@random-zebra random-zebra removed the Needs Backport Placeholder tag for anything needing a backport to prior version branches label May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants