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

pull runeksvendsen wants to merge 1 commits into bitcoin:master from runeksvendsen:master changing 1 files +10 −1

runeksvendsen commented at 9:41 AM on September 30, 2012: contributor

No description provided.
Diapolo commented at 10:07 AM on September 30, 2012: none

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 commented at 10:37 AM on September 30, 2012: member

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 commented at 11:57 AM on September 30, 2012: contributor

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.

in src/qt/askpassphrasedialog.cpp:None in d0184c6798 outdated

 107 | @@ -108,7 +108,16 @@ void AskPassphraseDialog::accept()
 108 |                  if(model->setWalletEncrypted(true, newpass1))
 109 |                  {
 110 |                      QMessageBox::warning(this, tr("Wallet encrypted"),
 111 | -                                         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."));
 112 | +                                         "<qt>" + 
 113 | +                                         tr("Bitcoin will close now to finish the encryption process. \

laanwj commented at 12:09 PM on September 30, 2012:

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.

runeksvendsen commented at 1:13 PM on September 30, 2012:

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

laanwj commented at 1:25 PM on September 30, 2012: member

ACK, please squash into one commit

When encrypting the wallet, warn user that old backups will become useless.

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

Prettier lines

1bf66fcc0a

BitcoinPullTester commented at 5:52 PM on October 1, 2012: none
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 commented at 8:22 PM on October 1, 2012: contributor
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 merged this on Oct 1, 2012
gavinandresen closed this on Oct 1, 2012
KolbyML referenced this in commit bec361e86f on Dec 5, 2020
DrahtBot locked this on Sep 8, 2021

Contributors

Linked (view graph)

#1893 Issue 1884: Add a backup warning to the encryptwallet RPC command