Page MenuHomePhabricator

Make CWallet IsSpent and IsLockedCoin take a COutPoint as parameter
ClosedPublic

Authored by Fabien on Jul 2 2019, 10:36.

Details

Summary

These methods currently take a transaction id and the output order,
which is the definition of a COutPoint.

This is a follow up from
https://reviews.bitcoinabc.org/D3420?id=9680#inline-21184

Test Plan
make check
./test/functional/test_runner.py wallet_*

Diff Detail

Repository
rABC Bitcoin ABC
Branch
refactor_txout
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 6653
Build 11353: Bitcoin ABC Buildbot (legacy)
Build 11352: arc lint + arc unit

Event Timeline

deadalnix requested changes to this revision.Jul 3 2019, 01:18
deadalnix added inline comments.
src/wallet/wallet.cpp
2036 ↗(On Diff #9904)

You could cache the txid in a local variable.

2082 ↗(On Diff #9904)

dito

2456 ↗(On Diff #9904)

Because entry.first seems to be wtxid - unless something mutates it in between - then creating the outpoint once is preferable.

This revision now requires changes to proceed.Jul 3 2019, 01:18
src/wallet/wallet.cpp
2390 ↗(On Diff #9904)

Why did your remove this ?

src/wallet/wallet.cpp
2036 ↗(On Diff #9904)

This is the only place where GetId() is called within this function, so adding a local variable is not worth it.

2082 ↗(On Diff #9904)

Dito :)

2390 ↗(On Diff #9904)

It was only used by IsSpent as an alias for entry.first. Building the COutPoint from entry.first make it unused.

2460 ↗(On Diff #9904)

-> here

deadalnix requested changes to this revision.Jul 3 2019, 18:30
deadalnix added inline comments.
src/wallet/wallet.cpp
2036 ↗(On Diff #9904)

It's in a loop. Unless tx->vout.size() is only called once, this is certainly called many times.

2390 ↗(On Diff #9904)

Giving things names is very useful.

This revision now requires changes to proceed.Jul 3 2019, 18:30
This revision is now accepted and ready to land.Jul 4 2019, 16:11