Paths

Table of Contentst

Differential D3515

Make CWallet IsSpent and IsLockedCoin take a COutPoint as parameter
ClosedPublic
Actions

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

Tags

None

Subscribers

None

Details

Reviewers

Group Reviewers

Restricted Project

Commits

rSTAGING6e34d1c830d8: Make CWallet IsSpent and IsLockedCoin take a COutPoint as parameter
rABC6e34d1c830d8: Make CWallet IsSpent and IsLockedCoin take a COutPoint as parameter

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

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

Fabien created this revision.Jul 2 2019, 10:36

Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 2 2019, 10:36

Herald added a reviewer: deadalnix. · View Herald Transcript

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B6618: Diff 9904.Jul 2 2019, 10:44

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

Address comments.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B6653: Diff 9959.Jul 3 2019, 15:08

deadalnix added inline comments.Jul 3 2019, 16:05

src/wallet/wallet.cpp
2390 ↗	(On Diff #9904)	Why did your remove this ?

Fabien added inline comments.Jul 3 2019, 16:12

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

Address feedback

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B6660: Diff 9975.Jul 4 2019, 14:23

deadalnix accepted this revision.Jul 4 2019, 16:11

This revision is now accepted and ready to land.Jul 4 2019, 16:11

Closed by commit rABC6e34d1c830d8: Make CWallet IsSpent and IsLockedCoin take a COutPoint as parameter (authored by Fabien). · Explain WhyJul 5 2019, 13:41

This revision was automatically updated to reflect the committed changes.

Fabien added a commit: rABC6e34d1c830d8: Make CWallet IsSpent and IsLockedCoin take a COutPoint as parameter.

Fabien added a commit: rSTAGING6e34d1c830d8: Make CWallet IsSpent and IsLockedCoin take a COutPoint as parameter.Sep 19 2019, 00:57

Revision Contents
Changeset List

Path

Size

src/

interfaces/

4 lines

wallet/

5 lines

4 lines

24 lines

Diff 10005

src/interfaces/wallet.cpp

Loading...

src/wallet/rpcwallet.cpp

Loading...

src/wallet/wallet.h

Loading...

src/wallet/wallet.cpp

Loading...