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

When encrypting the wallet, warn user that he should make new backups. A fix for Issue #1884. #1890

Merged
merged 1 commit into from
Oct 1, 2012

Conversation

runeksvendsen
Copy link
Contributor

No description provided.

@Diapolo
Copy link

Diapolo commented Sep 30, 2012

Idea is a good one, what I would love to see is avoid HTML-format tags in strings we use in translations. This is not always possible, but I would split that mega string into smaller pieces and use e.g. + QString("<br>") + or + "<br>" + in between.

Edit: What does the <qt> tag do or achieve there?

@laanwj
Copy link
Member

laanwj commented Sep 30, 2012

No comment on the message, though I'd indeed suggest splitting it over multiple lines in the source code for readability.

The <qt> tag marks the text as rich text (for Qt::AutoText). It is not always necessary, but it's safe.

@runeksvendsen
Copy link
Contributor Author

Good point about HTML in translation strings. I've removed the tags from the strings in tr(). And also made it somewhat more readable.

I'm wondering if we should include a more detailed description in the message, as previous backups don't exactly become useless as soon as the wallet is encrypted. Only when the user starts sending transactions, and the change is received by the encrypted addresses. But I figure it might just be better to not include that technicality.

@@ -108,7 +108,16 @@ void AskPassphraseDialog::accept()
if(model->setWalletEncrypted(true, newpass1))
{
QMessageBox::warning(this, tr("Wallet encrypted"),
tr("Bitcoin will close now to finish the encryption process. Remember that encrypting your wallet cannot fully protect your bitcoins from being stolen by malware infecting your computer."));
"<qt>" +
tr("Bitcoin will close now to finish the encryption process. \
Copy link
Member

Choose a reason for hiding this comment

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

Personally I'd use this syntax to spread the message over multiple lines, ie surround every line with ":

        tr("Bitcoin will close now to finish the encryption process. "
        "Remember that encrypting your wallet cannot fully protect "
        "..."
        ...)

With the current way, all the indentation whitespace ends up in the translation string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. That does look better. I didn't know tr() accepts multiple arguments like that.

@laanwj
Copy link
Member

laanwj commented Sep 30, 2012

ACK, please squash into one commit

…eless.

Don't include HTML in translation strings. Do split the huge message over several lines.

Prettier lines
@BitcoinPullTester
Copy link

Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/1bf66fcc0aaf0bd12bbb1d1026d52b6388033d38 for binaries and test log.

This could happen for one of several reasons:

  1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts
  2. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
  3. The test suite fails on either Linux i386 or Win32
  4. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)

@runeksvendsen
Copy link
Contributor Author

Cool that we have an automatic build and test bot!

I don't quite get the error though:

+ cp bitcoind test_bitcoin out/
+ git apply /mnt/test-scripts/bitcoind-comparison.patch
error: patch failed: src/main.cpp:2368
error: src/main.cpp: patch does not apply

since this patch doesn't touch main.cpp.

@gavinandresen gavinandresen merged commit 1bf66fc into bitcoin:master Oct 1, 2012
KolbyML pushed a commit to KolbyML/bitcoin that referenced this pull request Dec 5, 2020
…o IsSolvable

b425466 [Script] Add fColdStaking bool to IsSolvable (random-zebra)

Pull request description:

  Fixing a minor bug introduced in bitcoin#1757.
  `IsSolvable` always tries to produce a signature with the owner key, so it logs an error (and reports the coin as not solvable) for cold stakers.
  Fix it introducing a boolean argument in `IsSolvable`, to check the appropriate public key in the keystore.

ACKs for top commit:
  furszy:
    Great catch 👌 , ACK b425466
  Fuzzbawls:
    ACK b425466

Tree-SHA512: 9d4537218344c8176f1de500664905e78419fd852c9607b40cf83eedb55e5fb965977f31bba0a605fac35c9bee373613467d63c6d3f72386c598e7452b545101
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants