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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fabien requested changes to this revision.Apr 30 2019, 11:52
Fabien added inline comments.
src/interfaces/wallet.cpp
87 ↗(On Diff #8299)

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

268 ↗(On Diff #8299)

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

src/qt/bitcoin.cpp
28 ↗(On Diff #8299)

i comes befors q

src/qt/recentrequeststablemodel.cpp
5 ↗(On Diff #8299)

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

src/qt/recentrequeststablemodel.h
8 ↗(On Diff #8299)

<qt/walletmodel.h>

src/qt/transactiondesc.cpp
7 ↗(On Diff #8299)

Please sort with the others for consistency

src/qt/transactionrecord.cpp
74 ↗(On Diff #8299)

Braces

75 ↗(On Diff #8299)

Dito

80 ↗(On Diff #8299)

Dito

81 ↗(On Diff #8299)

Dito

src/qt/transactiontablemodel.cpp
7 ↗(On Diff #8299)

Sort them with others for consistency

This revision now requires changes to proceed.Apr 30 2019, 11:52
src/interfaces/wallet.cpp
87 ↗(On Diff #8299)

good idea.

268 ↗(On Diff #8299)

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

src/qt/bitcoin.cpp
28 ↗(On Diff #8299)

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

src/qt/recentrequeststablemodel.cpp
5 ↗(On Diff #8299)

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.