Prepare BitcoinUnits for explicit Amount construction
ClosedPublic
Actions

Authored by schancel on Nov 28 2017, 07:19.

Details

Reviewers

deadalnix
freetrader
mbhinder

Group Reviewers

Restricted Project

Test Plan

make check

Diff Detail

Repository

rABC Bitcoin ABC

Branch

qt-bitcoinunits

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 1254
Build 1254: arc lint + arc unit

Event Timeline

schancel created this revision.Nov 28 2017, 07:19

Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 28 2017, 07:19

Harbormaster completed remote builds in B1201: Diff 1873.Nov 28 2017, 07:19

It seems that you are build amount from several places that should take an amount in to begin with. Why don't you convert these places to take an Amount rather than patching then for explicit construction ?

schancel added inline comments.Nov 28 2017, 23:05

src/qt/overviewpage.cpp
177 ↗	(On Diff #1873)	@deadalnix Are you talking about stuff like this? If I do it the way you are suggesting, this diff grows to the entire qt package. If you can suggest something explicit I'm willing to consider it.

schancel added inline comments.Nov 28 2017, 23:07

src/qt/sendcoinsdialog.cpp
373 ↗	(On Diff #1873)	I could make this an `Amount`, and avoid the conversion down a bit, but I'm still going to have to flip it back later when getTotalTransactionAmount returns an``Amount`.

deadalnix requested changes to this revision.Dec 5 2017, 14:25

deadalnix added inline comments.

src/qt/overviewpage.cpp
177 ↗	(On Diff #1873)	If you build an `Amount` from an `Amount` you'll get a copy constructor. When you use the GetSatoshi, then you'll get an error and you'll know you need to fix it when making the constructor explicit. My concern is that we'll end up with a bunch a amount construction that do not serve any purpose.