Bitcoin-Qt: fix known addressbook bugs #2157

pull Diapolo wants to merge 1 commits into bitcoin:master from Diapolo:Qt_addrlist changing 4 files +64 −33

Diapolo commented at 1:39 PM on January 8, 2013: none
- add qSort() for cachedAddressTable, as qLowerBound() and qUpperBound() require the list to be in ascending order (see http://harmattan-dev.nokia.com/docs/library/html/qt4/qtalgorithms.html#qLowerBound)
- add a new check in AddressTableModel::setData() to just return, when no changes were made to a label or an address (prevents entry duplication issue)
- remove "rec->label = value.toString();" from AddressTableModel::setData() as the label gets updated by AddressTablePriv::updateEntry() anyway (seems @sipa added this line via https://github.com/bitcoin/bitcoin/commit/1025440184ef100a22d07c7bb543ee45cf169d64#L6R225)
- add another new check in AddressTableModel::setData() to just return, if a duplicate address was found (prevents address overwrite)
- add a new check to EditAddressDialog::setModel() to prevent setting an invalid model
- re-work the switch-case statement in AddressTableModel::accept() to always break (as return get's called anyway) and order the list to match the enum definition
- make accept() in editaddressdialog.h a public slot, which it should be
- misc small coding style changes
Intended to fix: #2137 and #1839.
BitcoinPullTester commented at 1:53 PM on January 8, 2013: none
Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/ae1c11d44a4ea90001ea777883cc2cef8a60c846 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)
Diapolo commented at 2:44 PM on January 8, 2013: none

@TheBlueMatt Can you fix @BitcoinPullTester? fatal error: error writing to /tmp/ccBCY0YL.s: No space left on device
laanwj commented at 4:23 PM on January 8, 2013: member

Cool

btw: add qSort() for cachedAddressTable, as qLowerBound() and qUpperBound() require the list to be in ascending order (see http://harmattan-dev.nokia.com/docs/library/html/qt4/qtalgorithms.html#qLowerBound)

Are you sure this is needed? It should already stay in sorted order by picking the insertion positions carefully
Diapolo commented at 5:14 PM on January 8, 2013: none

@laanwj Take a look here, the unsorted list caused some weird issues for me.
BitcoinPullTester commented at 7:31 PM on January 8, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/ae1c11d44a4ea90001ea777883cc2cef8a60c846 for binaries and test log.
Diapolo commented at 1:12 PM on January 9, 2013: none

I consider this a rather important bugfix and would love to see it in 0.8. I'll try to get some bug-reporters, to test this.

Bitcoin-Qt: fix known addressbook bugs

- add qSort() for cachedAddressTable, as qLowerBound() and qUpperBound()
  require the list to be in ascending order (see
  http://harmattan-dev.nokia.com/docs/library/html/qt4/qtalgorithms.html#qLowerBound)
- add a new check in AddressTableModel::setData() to just return, when no
  changes were made to a label or an address (prevents entry duplication
  issue)
- remove "rec->label = value.toString();" from
  AddressTableModel::setData() as the label gets updated by
  AddressTablePriv::updateEntry() anyway (seems @sipa added this line via
  https://github.com/bitcoin/bitcoin/commit/1025440184ef100a22d07c7bb543ee45cf169d64#L6R225)
- add another new check in AddressTableModel::setData() to just return, if
  a duplicate address was found (prevents address overwrite)
- add a new check to EditAddressDialog::setModel() to prevent setting an
  invalid model
- re-work the switch-case statement in AddressTableModel::accept() to
  always break (as return get's called anyway) and order the list to match
  the enum definition
- make accept() in editaddressdialog.h a public slot, which it should be
- misc small coding style changes

e6d2300562

in src/qt/addresstablemodel.cpp:None in ae1c11d44a outdated
```
 360 | @@ -342,9 +361,8 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con
 361 |          strAddress = CBitcoinAddress(newKey.GetID()).ToString();
 362 |      }
 363 |      else
 364 | -    {
```
laanwj commented at 3:46 PM on January 9, 2013:

Please don't do this. I prefer explicit {} around blocks, as it prevents a class of bugs when a statement is added.

Diapolo commented at 3:56 PM on January 9, 2013:

You are right, I reverted this one.

Edit: Do you have any clue, why my Firefox refuses to display Commits and Files Changed on Github? I'm getting crazy because of this...

Edit 2: Crazy, I had dom.storage.enabled set to false, which prevents Github page from working correctly!
laanwj commented at 3:58 PM on January 9, 2013: member

Code changes are OK, but if some people can confirm that this solves their bug that'd be great :)
BitcoinPullTester commented at 4:17 PM on January 9, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/e6d230056202b05a27f187dff2648eb5c76fcfee for binaries and test log.
Diapolo commented at 7:18 AM on January 11, 2013: none

@laanwj Take a look at #1839, it seems this indeed fixes the problem.
Diapolo commented at 8:44 PM on January 17, 2013: none

@laanwj ping² :-P
laanwj referenced this in commit bd85cf3df7 on Jan 19, 2013
laanwj merged this on Jan 19, 2013
laanwj closed this on Jan 19, 2013
laudney referenced this in commit b7b2be0c1c on Mar 19, 2014
HashUnlimited referenced this in commit fe38bf1c00 on Jul 2, 2018
HashUnlimited referenced this in commit 0328610d23 on Jul 2, 2018
HashUnlimited referenced this in commit a5846a858d on Jul 3, 2018
sidhujag referenced this in commit e92249e738 on Jul 6, 2018
owlhooter referenced this in commit fda74b4a8c on Oct 11, 2018
guruvan referenced this in commit 2ccfffc87d on Nov 8, 2018
DrahtBot locked this on Sep 8, 2021

Contributors

Diapolo

BitcoinPullTester

laanwj

Linked (view graph)

#1839 Editing address book labels sometimes duplicates entry #2137 Editing label single address in address book also changes another label