Page MenuHomePhabricator
Differential D3420

Diagnose unsuitable outputs in lockunspent().
ClosedPublic
Actions

Authored by Fabien on Jun 26 2019, 09:17.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
rSTAGING9fa49a61c3b4: Diagnose unsuitable outputs in lockunspent().
rABC9fa49a61c3b4: Diagnose unsuitable outputs in lockunspent().
Summary
Fixes "lockunspent returns true even for non-existent outputs"

Backport of core PR11087
https://github.com/bitcoin/bitcoin/pull/11087/files

Test Plan
make check
./test/functional/test_runner.py wallet_basic

Diff Detail

Repository
rABC Bitcoin ABC
Branch
PR11087
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 6491
Build 11029: Bitcoin ABC Buildbot (legacy)
Build 11028: arc lint + arc unit

Event Timeline

Fabien created this revision.Jun 26 2019, 09:17
Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 26 2019, 09:17
Herald added a reviewer: deadalnix. · View Herald Transcript
teamcity added a comment.Jun 26 2019, 09:18

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

teamcity added a comment.Jun 26 2019, 09:24

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

Harbormaster completed remote builds in B6491: Diff 9680.Jun 26 2019, 09:24
deadalnix requested changes to this revision.Jun 26 2019, 23:01
deadalnix added inline comments.
src/wallet/rpcwallet.cpp
2983

As it turns out, Univalue's size is an unsigned int so size_t is not correct here.

3018

Looks like IsSpent should take an outpoint, but what do I know...

It's out of scope anyways.

This revision now requires changes to proceed.Jun 26 2019, 23:01
Fabien requested review of this revision.Jun 27 2019, 06:13
Fabien added inline comments.
src/wallet/rpcwallet.cpp
2983

From univalue.h:

class Univalue {
public:
[...]
    size_t size() const { return values.size(); }

Unless I'm missing Univalue's size seems to be a size_t ?

3018

Agreed, and it's not the only function (see IsLockedCoin). I noticed it and added to my todo-list, if the impact is not crazy I'll submit a follow up refactor anytime soon.

deadalnix accepted this revision.Jun 27 2019, 15:02
deadalnix added inline comments.
src/wallet/rpcwallet.cpp
2983

ok :)

This revision is now accepted and ready to land.Jun 27 2019, 15:02
Closed by commit rABC9fa49a61c3b4: Diagnose unsuitable outputs in lockunspent(). (authored by Eelis <eelis@bitonic.nl>, committed by Fabien). · Explain WhyJun 27 2019, 16:11
This revision was automatically updated to reflect the committed changes.
Fabien added a commit: rABC9fa49a61c3b4: Diagnose unsuitable outputs in lockunspent()..
Fabien mentioned this in D3515: Make CWallet IsSpent and IsLockedCoin take a COutPoint as parameter.Jul 2 2019, 10:36
Fabien mentioned this in rABC6e34d1c830d8: Make CWallet IsSpent and IsLockedCoin take a COutPoint as parameter.Jul 5 2019, 13:41
nakihito mentioned this in D3997: Merge #11050: Avoid treating null RPC arguments different from missing arguments.Sep 4 2019, 21:36
Fabien added a commit: rSTAGING9fa49a61c3b4: Diagnose unsuitable outputs in lockunspent()..Sep 19 2019, 00:37
Fabien mentioned this in rSTAGING6e34d1c830d8: Make CWallet IsSpent and IsLockedCoin take a COutPoint as parameter.Sep 19 2019, 00:57

Revision Contents
Changeset List

PathSize
src/
wallet/
59 lines
test/
functional/
14 lines

Diff 9680

View Options

src/wallet/rpcwallet.cpp

Loading...
View Options

test/functional/wallet_basic.py

Loading...