Page MenuHomePhabricator
Differential D8867

[validation] Remove NotifyEntryRemoved callback from ConnectTrace
ClosedPublic
Actions

Authored by PiRK on Jan 11 2021, 10:39.

Details

Reviewers
majcosta
Group Reviewers
Restricted Project
Commits
rABC3cf77d19ea8c: [validation] Remove NotifyEntryRemoved callback from ConnectTrace
Summary

ConnectTrace used to subscribe to the mempool's NotifyEntryRemoved
callback to be notified of transactions removed for conflict. Since
PerBlockConnectTrace no longer tracks conflicted transactions,
ConnectTrace no longer requires these notifications.

This is a backport of Core PR17477 [4 & 5/6]
https://github.com/bitcoin/bitcoin/pull/17477/commits/969b65f3f527631ede1a31c7855151e5c5d91f8f
https://github.com/bitcoin/bitcoin/pull/17477/commits/2dd561f36124972d2364f941de9c3417c65f05b6

Backport note: these 2 commits have been squashed because the first one left an unused private attribute 'pool', causing a warning / error for build-clang

Depends on D8866

Test Plan

ninja all check-all

Diff Detail

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

Event Timeline

PiRK created this revision.Jan 11 2021, 10:39
Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 11 2021, 10:39
PiRK requested review of this revision.Jan 11 2021, 10:39
teamcity edited the summary of this revision. (Show Details)Jan 11 2021, 10:39
PiRK added a child revision: D8868: [validation] Remove pool member from ConnectTrace.Jan 11 2021, 10:41
Harbormaster completed remote builds in B14693: Diff 26837.Jan 11 2021, 10:51
teamcity added a comment.Jan 11 2021, 10:51
Build Bitcoin ABC Diffs / Diff Testing (build-clang) failed.

Tail of the build log:

-- Installing: /results/artifacts/include/secp256k1_schnorr.h
[325/499] Linking C executable src/secp256k1/recover-bench
[326/499] Building CXX object src/CMakeFiles/util.dir/util/error.cpp.o
[327/499] Building CXX object src/CMakeFiles/util.dir/util/message.cpp.o
[328/499] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/pubkey.cpp.o
[329/499] Building CXX object src/test/CMakeFiles/testutil.dir/util/str.cpp.o
[330/499] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/consensus/tx_check.cpp.o
[331/499] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/util/strencodings.cpp.o
[332/499] Building CXX object src/CMakeFiles/util.dir/util/settings.cpp.o
[333/499] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/primitives/transaction.cpp.o
[334/499] Building CXX object src/test/CMakeFiles/testutil.dir/util/logging.cpp.o
[335/499] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[336/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/context.cpp.o
[337/499] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockfilter.cpp.o
[338/499] Building CXX object src/test/CMakeFiles/testutil.dir/util/transaction_utils.cpp.o
[339/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[340/499] Building CXX object src/test/CMakeFiles/testutil.dir/util/net.cpp.o
[341/499] Building CXX object src/CMakeFiles/util.dir/util/time.cpp.o
[342/499] Building CXX object src/test/CMakeFiles/testutil.dir/util/mining.cpp.o
[343/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[344/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[345/499] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[346/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[347/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/bdb.cpp.o
[348/499] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqabstractnotifier.cpp.o
[349/499] Building CXX object src/CMakeFiles/util.dir/util/system.cpp.o
[350/499] Linking CXX static library src/libutil.a
[351/499] Linking CXX static library src/librpcclient.a
[352/499] Linking CXX static library src/libbitcoinconsensus.a
[353/499] Linking CXX static library src/libscript.a
[354/499] Linking CXX static library src/libcommon.a
[355/499] Linking CXX shared library src/libbitcoinconsensus.so.0.22.12
[356/499] Creating library symlink src/libbitcoinconsensus.so.0 src/libbitcoinconsensus.so
[357/499] Linking CXX executable src/bitcoin-cli
[358/499] Linking CXX executable src/bitcoin-tx
[359/499] Building CXX object src/test/CMakeFiles/testutil.dir/util/setup_common.cpp.o
[360/499] Building CXX object src/wallet/CMakeFiles/wallet-tool.dir/wallettool.cpp.o
[361/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[362/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[363/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[364/499] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqnotificationinterface.cpp.o
[365/499] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[366/499] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[367/499] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqpublishnotifier.cpp.o
[368/499] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqrpc.cpp.o
[369/499] Linking CXX static library src/zmq/libzmq.a
[370/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/salvage.cpp.o
[371/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/__/interfaces/wallet.cpp.o
[372/499] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
[373/499] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[374/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[375/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[376/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcdump.cpp.o
[377/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
[378/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
[379/499] Linking CXX static library src/wallet/libwallet.a
[380/499] Linking CXX static library src/wallet/libwallet-tool.a
[381/499] Linking CXX executable src/bitcoin-wallet
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang failed with exit code 1
majcosta added a subscriber: majcosta.Jan 11 2021, 12:06

@bot build-clang

teamcity awarded a token.Jan 11 2021, 12:07
teamcity added a comment.Jan 11 2021, 12:10
Build Bitcoin ABC Diffs / Diff Testing (build-clang) failed.

Tail of the build log:

-- Installing: /results/artifacts/include/secp256k1_schnorr.h
[325/499] Linking C executable src/secp256k1/recover-bench
[326/499] Building CXX object src/CMakeFiles/util.dir/util/error.cpp.o
[327/499] Building CXX object src/CMakeFiles/util.dir/util/message.cpp.o
[328/499] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/pubkey.cpp.o
[329/499] Building CXX object src/CMakeFiles/util.dir/util/settings.cpp.o
[330/499] Building CXX object src/test/CMakeFiles/testutil.dir/util/str.cpp.o
[331/499] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/util/strencodings.cpp.o
[332/499] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/primitives/transaction.cpp.o
[333/499] Building CXX object src/CMakeFiles/bitcoinconsensus.dir/consensus/tx_check.cpp.o
[334/499] Building CXX object src/test/CMakeFiles/testutil.dir/util/logging.cpp.o
[335/499] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[336/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/context.cpp.o
[337/499] Building CXX object src/test/CMakeFiles/testutil.dir/util/transaction_utils.cpp.o
[338/499] Building CXX object src/test/CMakeFiles/testutil.dir/util/net.cpp.o
[339/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[340/499] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockfilter.cpp.o
[341/499] Building CXX object src/CMakeFiles/util.dir/util/time.cpp.o
[342/499] Building CXX object src/test/CMakeFiles/testutil.dir/util/mining.cpp.o
[343/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[344/499] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[345/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/bdb.cpp.o
[346/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[347/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[348/499] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqabstractnotifier.cpp.o
[349/499] Building CXX object src/CMakeFiles/util.dir/util/system.cpp.o
[350/499] Linking CXX static library src/libutil.a
[351/499] Linking CXX static library src/librpcclient.a
[352/499] Linking CXX static library src/libbitcoinconsensus.a
[353/499] Linking CXX static library src/libscript.a
[354/499] Linking CXX static library src/libcommon.a
[355/499] Building CXX object src/wallet/CMakeFiles/wallet-tool.dir/wallettool.cpp.o
[356/499] Linking CXX shared library src/libbitcoinconsensus.so.0.22.12
[357/499] Creating library symlink src/libbitcoinconsensus.so.0 src/libbitcoinconsensus.so
[358/499] Linking CXX executable src/bitcoin-cli
[359/499] Linking CXX executable src/bitcoin-tx
[360/499] Building CXX object src/test/CMakeFiles/testutil.dir/util/setup_common.cpp.o
[361/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[362/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[363/499] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqnotificationinterface.cpp.o
[364/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[365/499] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[366/499] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[367/499] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqpublishnotifier.cpp.o
[368/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/salvage.cpp.o
[369/499] Building CXX object src/zmq/CMakeFiles/zmq.dir/zmqrpc.cpp.o
[370/499] Linking CXX static library src/zmq/libzmq.a
[371/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/__/interfaces/wallet.cpp.o
[372/499] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
[373/499] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[374/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[375/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[376/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcdump.cpp.o
[377/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
[378/499] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
[379/499] Linking CXX static library src/wallet/libwallet.a
[380/499] Linking CXX static library src/wallet/libwallet-tool.a
[381/499] Linking CXX executable src/bitcoin-wallet
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang failed with exit code 1
PiRK added a comment.Jan 11 2021, 12:17

I think the clang warning that is treated as an error goes away with the following commit (D8868). Can I just make sure I land them in rapid succession, or do I need to squash them?

majcosta added a comment.Jan 11 2021, 12:23
This comment was removed by majcosta.
majcosta added a comment.Jan 11 2021, 12:25
In D8867#203607, @PiRK wrote:

I think the clang warning that is treated as an error goes away with the following commit (D8868). Can I just make sure I land them in rapid succession, or do I need to squash them?

Squashing them together is best.

majcosta requested changes to this revision.Jan 11 2021, 12:31
This revision now requires changes to proceed.Jan 11 2021, 12:31
PiRK updated this revision to Diff 26842.Jan 11 2021, 12:55

squash D8867 and D8868

PiRK edited the summary of this revision. (Show Details)Jan 11 2021, 12:59
PiRK mentioned this in D8868: [validation] Remove pool member from ConnectTrace.
PiRK added a child revision: D8869: [mempool] Remove NotifyEntryAdded and NotifyEntryRemoved callbacks.
Harbormaster completed remote builds in B14697: Diff 26842.Jan 11 2021, 13:09
majcosta accepted this revision.Jan 11 2021, 13:20
This revision is now accepted and ready to land.Jan 11 2021, 13:20
Closed by commit rABC3cf77d19ea8c: [validation] Remove NotifyEntryRemoved callback from ConnectTrace (authored by John Newbery <john@johnnewbery.com>, committed by PiRK). · Explain WhyJan 11 2021, 14:21
This revision was automatically updated to reflect the committed changes.
PiRK added a commit: rABC3cf77d19ea8c: [validation] Remove NotifyEntryRemoved callback from ConnectTrace.

Revision Contents
Changeset List

PathSize
src/
15 lines

Diff 26852

View Options

src/validation.cpp

Loading...