Page MenuHomePhabricator

Remove direct bitcoin calls from qt transaction table files
ClosedPublic

Authored by jasonbcox on Apr 29 2019, 18:46.

Details

Summary
Test Plan

make check
ninja check
bitcoin-qt # sanity checks on transaction details

Diff Detail

Repository
rABC Bitcoin ABC
Branch
txtable
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 5620
Build 9302: Bitcoin ABC Buildbot (legacy)
Build 9301: arc lint + arc unit

Event Timeline

Fabien requested changes to this revision.Apr 30 2019, 11:52
Fabien added inline comments.
src/interfaces/wallet.cpp
87

You can use LookupBlockIndex() here.
Also isn't it a LOCK(cs_main) missing ?

268

OK The lock is here. So maybe add an AssertLockHeld(cs_main) in MakeWalletTxStatus() ?

src/qt/bitcoin.cpp
28

i comes befors q

src/qt/recentrequeststablemodel.cpp
5

You updated the .h to angle brackets, don't you want to update the .cpp as well ?

src/qt/recentrequeststablemodel.h
8

<qt/walletmodel.h>

src/qt/transactiondesc.cpp
7

Please sort with the others for consistency

src/qt/transactionrecord.cpp
74

Braces

75

Dito

80

Dito

81

Dito

src/qt/transactiontablemodel.cpp
7

Sort them with others for consistency

This revision now requires changes to proceed.Apr 30 2019, 11:52
src/interfaces/wallet.cpp
87

good idea.

268

LookupBlockIndex already calls AssertLockHeld, so we're good. killed two birds with one stone :)

src/qt/bitcoin.cpp
28

Looks like the #ifdef broke up the sorting. I'll move it down to its own section.

src/qt/recentrequeststablemodel.cpp
5

Yes, thanks for flagging.

Fix according to feedback

This revision is now accepted and ready to land.Apr 30 2019, 17:56
This revision was automatically updated to reflect the committed changes.