Page MenuHomePhabricator

qt: Use IsMine to validate custom change address
ClosedPublic

Authored by dagurval on Sep 13 2017, 13:28.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

dagurval created this revision.Sep 13 2017, 13:28
Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 13 2017, 13:28
deadalnix requested changes to this revision.Sep 14 2017, 16:45
deadalnix added a subscriber: deadalnix.

Honestly I'm not sure what this code is doing. It is rather concerning that changing it that way doesn't require changing any test. What would it take to write a test for this ?

src/qt/sendcoinsdialog.cpp
836 ↗(On Diff #1367)

Please keep the comment within the blocks.

840 ↗(On Diff #1367)

dito

This revision now requires changes to proceed.Sep 14 2017, 16:45

Looking at D540, I understand this better. It still would be beneficial to see if that can be unit tested in some way.

dagurval edited edge metadata.Sep 17 2017, 21:01
dagurval updated this revision to Diff 1387.

formatting

dagurval marked 2 inline comments as done.Sep 17 2017, 21:15

Rebased and pushed formatting changes.

I don't see how SendCoinsDialog would be testable at all. WalletModel may be testable with some work.

But at least the IsMine function has some test coverage.

deadalnix accepted this revision.Sep 21 2017, 19:24
This revision is now accepted and ready to land.Sep 21 2017, 19:24
Closed by commit rABCe2ecbe002807: qt: Use IsMine to validate custom change address (authored by Chris Moore <dooglus@gmail.com>, committed by dagurval). · Explain WhySep 21 2017, 23:40
This revision was automatically updated to reflect the committed changes.