Page MenuHomePhabricator
Differential D14022

Explicitly filter out partial groups when we don't want them
ClosedPublic
Actions

Authored by Fabien on Jun 8 2023, 21:03.

Details

Reviewers
PiRK
Group Reviewers
Restricted Project
Commits
rABC8c9a764afae1: Explicitly filter out partial groups when we don't want them
Summary
Instead of hacking OutputGroup::m_ancestors to discourage the inclusion of partial groups via the eligibility filter, add a parameter to the eligibility filter that indicates whether we want to include the group.
Then for those partial groups, don't return them in GroupOutputs if we indicate they aren't desired.

This is a partial backport of core#20040:
https://github.com/bitcoin/bitcoin/pull/20040/commits/f6b305273910db0e46798d361413a7e878cb45f7

Depends on D14021.

Test Plan
ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fabien created this revision.Jun 8 2023, 21:03
Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 8 2023, 21:03
Fabien requested review of this revision.Jun 8 2023, 21:03
teamcity edited the summary of this revision. (Show Details)Jun 8 2023, 21:03
Fabien added a child revision: D14023: Rewrite OutputGroups to be clearer and to use scriptPubKeys.Jun 8 2023, 21:04
Harbormaster completed remote builds in B23958: Diff 40672.Jun 8 2023, 21:32
teamcity added a comment.Jun 8 2023, 21:32
Build Bitcoin ABC Diffs / Diff Testing (build-clang-tidy) failed.

Tail of the build log:

[375/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/context.cpp.o
[376/537] Building CXX object src/test/CMakeFiles/testutil.dir/util/transaction_utils.cpp.o
[377/537] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockfilter.cpp.o
[378/537] Building CXX object src/test/CMakeFiles/testutil.dir/util/mining.cpp.o
[379/537] Building CXX object src/test/CMakeFiles/testutil.dir/util/net.cpp.o
[380/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[381/537] Building CXX object src/test/CMakeFiles/testutil.dir/util/validation.cpp.o
[382/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/bdb.cpp.o
[383/537] Building CXX object src/CMakeFiles/server.dir/validation.cpp.o
[384/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[385/537] Building CXX object src/test/CMakeFiles/testutil.dir/util/setup_common.cpp.o
[386/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[387/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[388/537] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[389/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[390/537] Building CXX object src/CMakeFiles/server.dir/wallet/init.cpp.o
[391/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/sqlite.cpp.o
[392/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[393/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/transaction.cpp.o
[394/537] Building CXX object src/wallet/CMakeFiles/wallet-tool.dir/wallettool.cpp.o
[395/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[396/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/receive.cpp.o
[397/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/util.cpp.o
[398/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/signmessage.cpp.o
[399/537] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[400/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/encrypt.cpp.o
[401/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/spend.cpp.o
FAILED: src/wallet/CMakeFiles/wallet.dir/spend.cpp.o 
/usr/bin/cmake -E __run_co_compile --launcher=/usr/bin/ccache --tidy="/usr/bin/clang-tidy-12;-warnings-as-errors=*;--extra-arg-before=--driver-mode=g++" --source=../../src/wallet/spend.cpp -- /usr/bin/clang++ -DBOOST_ALL_NO_LIB -DBUILD_BITCOIN_INTERNAL -DENABLE_AVX2 -DENABLE_SHANI -DENABLE_SSE41 -DHAVE_BUILD_INFO -DHAVE_CONFIG_H -DHAVE_CONSENSUS_LIB -DLEVELDB_ATOMIC_PRESENT -DLEVELDB_PLATFORM_POSIX -DOS_LINUX -I../../src/. -Isrc -I../../src/univalue/include -Isrc/crypto/.. -I../../src/secp256k1/include -I../../src/leveldb/include -isystem /usr/include/jemalloc -isystem /usr/include/miniupnpc -g -O2 -fPIC -fvisibility=hidden -fstack-protector-all -Wstack-protector -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wall -Wextra -Wformat -Wgnu -Wvla -Wcast-align -Wunused-parameter -Wmissing-braces -Wthread-safety -Wrange-loop-analysis -Wredundant-decls -Wunreachable-code-loop-increment -Wsign-compare -Wconditional-uninitialized -Wdocumentation -Wformat-security -Wredundant-move -Woverloaded-virtual -Wshadow -Wshadow-field -Wno-unused-parameter -Wno-implicit-fallthrough -Wno-psabi -pthread -std=gnu++17 -MD -MT src/wallet/CMakeFiles/wallet.dir/spend.cpp.o -MF src/wallet/CMakeFiles/wallet.dir/spend.cpp.o.d -o src/wallet/CMakeFiles/wallet.dir/spend.cpp.o -c ../../src/wallet/spend.cpp
/work/abc-ci-builds/build-clang-tidy/../../src/wallet/spend.cpp:560:36: error: argument name 'include_partial_groups' in comment does not match parameter name 'include_partial' [bugprone-argument-comment,-warnings-as-errors]
                                   /*include_partial_groups=*/true),
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
                                   /*include_partial=*/
../../src/./wallet/coinselection.h:81:32: note: 'include_partial' declared here
                          bool include_partial)
                               ^
/work/abc-ci-builds/build-clang-tidy/../../src/wallet/spend.cpp:568:36: error: argument name 'include_partial_groups' in comment does not match parameter name 'include_partial' [bugprone-argument-comment,-warnings-as-errors]
                                   /*include_partial_groups=*/true),
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
                                   /*include_partial=*/
../../src/./wallet/coinselection.h:81:32: note: 'include_partial' declared here
                          bool include_partial)
                               ^
3242 warnings generated.
Suppressed 3240 warnings (3240 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
2 warnings treated as errors
[402/537] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/options.cpp.o
[403/537] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[404/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[405/537] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[406/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/salvage.cpp.o
[407/537] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
[408/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/interfaces.cpp.o
[409/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/backup.cpp.o
[410/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[411/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
[412/537] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1
Fabien planned changes to this revision.Jun 9 2023, 05:51

Needs a clang-tidy fix

Fabien updated this revision to Diff 40682.Jun 9 2023, 06:00

Rease

PiRK requested changes to this revision.Jun 9 2023, 06:16
PiRK added a subscriber: PiRK.
PiRK added inline comments.
src/wallet/spend.cpp
560 ↗(On Diff #40672)
568 ↗(On Diff #40672)
This revision now requires changes to proceed.Jun 9 2023, 06:16
PiRK accepted this revision.Jun 9 2023, 06:17
This revision is now accepted and ready to land.Jun 9 2023, 06:17
Harbormaster completed remote builds in B23964: Diff 40682.Jun 9 2023, 06:49
Closed by commit rABC8c9a764afae1: Explicitly filter out partial groups when we don't want them (authored by Andrew Chow <achow101-github@achow101.com>, committed by Fabien). · Explain WhyJun 9 2023, 13:24
This revision was automatically updated to reflect the committed changes.
Fabien added a commit: rABC8c9a764afae1: Explicitly filter out partial groups when we don't want them.

Revision Contents
Changeset List

PathSize
src/
wallet/
9 lines
3 lines
34 lines

Diff 40692

View Options

src/wallet/coinselection.h

Loading...
View Options

src/wallet/spend.h

Loading...
View Options

src/wallet/spend.cpp

Loading...