Page MenuHomePhabricator

Prepare BitcoinUnits for explicit Amount construction
ClosedPublic

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
format
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 1201
Build 1201: arc lint + arc unit

Event Timeline

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 ?

src/qt/overviewpage.cpp
177

@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.

src/qt/sendcoinsdialog.cpp
373

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

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.

This revision now requires changes to proceed.Dec 5 2017, 14:25
This revision is now accepted and ready to land.Dec 6 2017, 21:35

Phab apparently conflated my second diff without me noticing. D747 closed when landed.