Details

Reviewers

freetrader
deadalnix

Group Reviewers

Restricted Project

Summary

Converts qt and wallet modules to use Amount struct together.
These two modules are fairly intertwined, so converting them
together avoids a significant amount of intermediate patches.

Test Plan

make VERBOSE=1 check

Diff Detail

Repository

rABC Bitcoin ABC

Branch

shammah/amount-wallet-and-qt

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 991
Build 991: arc lint + arc unit

Event Timeline

schancel created this revision.Oct 14 2017, 18:04

Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 14 2017, 18:04

Harbormaster completed remote builds in B991: Diff 1555.Oct 14 2017, 18:04

deadalnix requested changes to this revision.Oct 15 2017, 18:48

deadalnix added inline comments.

src/qt/bitcoinamountfield.h
23	Worth looking at what that discussion says and update the comment accordingly. Why did you start using qint64 instead of Amount here ? Is the discussion still relevant ?
src/qt/bitcoingui.cpp
1028	Remove reference
src/qt/bitcoingui.h
201	Remove reference.
src/qt/bitcoinunits.cpp
94	Use C++ style constructor qint64(nIn.GetSatoshis());
src/qt/bitcoinunits.h
80	Remove reference
84	Remove reference
88	Remove reference
91	Same question about the qint64
118	Maybe that should be a static method on Amount instead ?
src/qt/coincontroldialog.cpp
455	Remove reference
src/qt/guiutil.h
56	Remove reference
src/qt/overviewpage.h
45	Remove reference x6
src/qt/paymentserver.cpp
565	This comment is not accurate anymore.
src/qt/paymentserver.h
92	Remove reference
src/qt/sendcoinsdialog.h
62	Remove reference x6
src/qt/transactionfilterproxy.h
41	Remove reference
src/qt/transactionrecord.cpp
135	Maybe implementing the unary minus op would be worth it ?
src/qt/transactionrecord.h
111	Remove reference x2
src/qt/walletmodel.h
43	Remove reference
268	Remove reference x6
src/qt/walletmodeltransaction.h
30	Remove reference
src/qt/walletview.h
133	Remove reference
src/validation.h
386	This should probably not be boxed in that diff, please split it. Consensus code change should not be bundled with all the QT changes.
src/wallet/test/wallet_tests.cpp
40	Remove reference

This revision now requires changes to proceed.Oct 15 2017, 18:48

Rebase on top of D609

I have a few more changes to make after the latest diff, not quite ready for review yet.

src/qt/bitcoinamountfield.h
23	I wanted to ask someone more familiar with QT. The moc files that are generated when using Amount do not work properly. I haven't been able to figure out how to fix them. The comment is still applicable. It's because the auto-generated code does not include amount.h.
src/qt/bitcoinunits.cpp
94	Sounds good to me. Just to clarify: We're okay with including stylistic changes in patches? Normally I've been required to do stylistic changes as part of a larger patch to change things everywhere at once.
src/qt/bitcoinunits.h
118	Yeah, this was a bit weird. There's already a max money around. I can move it.

schancel added inline comments.Oct 17 2017, 10:43

src/qt/transactionrecord.cpp
135	I think there's only about 3 uses in the entire codebase. I also tend to think that negating money shouldn't be a convenient operation.

Rebase on D609

Harbormaster completed remote builds in B1019: Diff 1599.Oct 21 2017, 23:44

Rebase and fixes

Rebase

schancel abandoned this revision.Nov 7 2017, 19:19

Convert wallet and qt modules to use Amount struct (T104)AbandonedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 1555

src/bench/coin_selection.cpp

src/qt/bitcoin.cpp

src/qt/bitcoinamountfield.h

src/qt/bitcoinamountfield.cpp

src/qt/bitcoingui.h

src/qt/bitcoingui.cpp

src/qt/bitcoinunits.h

src/qt/bitcoinunits.cpp

src/qt/coincontroldialog.h

src/qt/coincontroldialog.cpp

src/qt/guiutil.h

src/qt/guiutil.cpp

src/qt/overviewpage.h

src/qt/overviewpage.cpp

src/qt/paymentrequestplus.h

src/qt/paymentrequestplus.cpp

src/qt/paymentserver.h

src/qt/paymentserver.cpp

src/qt/receiverequestdialog.cpp

src/qt/sendcoinsdialog.h

src/qt/sendcoinsdialog.cpp

src/qt/sendcoinsentry.cpp

src/qt/test/paymentservertests.cpp

src/qt/transactiondesc.cpp

src/qt/transactionfilterproxy.h

src/qt/transactionfilterproxy.cpp

src/qt/transactionrecord.h

src/qt/transactionrecord.cpp

src/qt/transactiontablemodel.cpp

src/qt/transactionview.cpp

src/qt/walletmodel.h

src/qt/walletmodel.cpp

src/qt/walletmodeltransaction.h

src/qt/walletmodeltransaction.cpp

src/qt/walletview.h

src/qt/walletview.cpp

src/validation.h

src/validation.cpp

src/wallet/coincontrol.h

src/wallet/rpcwallet.cpp

src/wallet/test/accounting_tests.cpp

src/wallet/test/wallet_tests.cpp

src/wallet/wallet.h

src/wallet/wallet.cpp

src/wallet/walletdb.h

src/wallet/walletdb.cpp

Convert wallet and qt modules to use Amount struct (T104)
AbandonedPublic
Actions

Revision Contents
Changeset List