Page MenuHomePhabricator
Differential D17717

[avalanche] Also finalize the ancestors of a transaction
ClosedPublic
Actions

Authored by Fabien on Thu, Feb 27, 22:07.

Details

Reviewers
PiRK
roqqit
Group Reviewers
Restricted Project
Commits
rABC37ee28b3d040: [avalanche] Also finalize the ancestors of a transaction
Summary

When avalanche finalizes a transaction, we want to make sure all the ancestors are final as well. This is a step toward using the radix tree of finalized transactions as a source for the block template.

Note that this diff does not add the ancestors to the list of the recently finalized items, this will be done in a later diff. This implies that the node could poll for the ancestors despite the child being final already.

Test Plan
ninja check-all

Diff Detail

Repository
rABC Bitcoin ABC
Branch
avalanche_tx_finalize_parents
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 32561
Build 64610: Build Difflint-circular-dependencies · build-without-wallet · build-diff · build-debug · build-clang-tidy · build-clang
Build 64609: arc lint + arc unit

Event Timeline

Fabien created this revision.Thu, Feb 27, 22:07
Herald added a reviewer: Restricted Project. · View Herald TranscriptThu, Feb 27, 22:07
Harbormaster completed remote builds in B32529: Diff 52825.Thu, Feb 27, 22:38
teamcity added a comment.Thu, Feb 27, 22:38
Build Bitcoin ABC Diffs / Diff Testing (build-clang) failed.

Tail of the build log:

-- Installing: /results/artifacts/include/secp256k1_preallocated.h
-- Installing: /results/artifacts/include/secp256k1_recovery.h
-- Installing: /results/artifacts/include/secp256k1_schnorr.h
[396/579] Building CXX object src/test/CMakeFiles/testutil.dir/util/logging.cpp.o
[397/579] Building CXX object src/test/CMakeFiles/testutil.dir/util/random.cpp.o
[398/579] Building CXX object src/test/CMakeFiles/testutil.dir/util/transaction_utils.cpp.o
[399/579] Building C object src/secp256k1/CMakeFiles/internal-bench.dir/src/bench_internal.c.o
[400/579] Linking C executable src/secp256k1/internal-bench
[401/579] Building CXX object src/test/CMakeFiles/testutil.dir/util/str.cpp.o
[402/579] Building CXX object src/CMakeFiles/bitcoin-cli.dir/bitcoin-cli.cpp.o
[403/579] Linking CXX executable src/bitcoin-cli
[404/579] Building CXX object src/CMakeFiles/server.dir/rpc/rawtransaction.cpp.o
[405/579] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockfilter.cpp.o
[406/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/context.cpp.o
[407/579] Building CXX object src/CMakeFiles/server.dir/wallet/init.cpp.o
[408/579] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[409/579] Linking CXX executable src/bitcoin-tx
[410/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[411/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[412/579] Building CXX object src/CMakeFiles/server.dir/rpc/blockchain.cpp.o
[413/579] Building CXX object src/test/CMakeFiles/testutil.dir/util/validation.cpp.o
[414/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[415/579] Building CXX object src/test/CMakeFiles/testutil.dir/util/net.cpp.o
[416/579] Building CXX object src/test/CMakeFiles/testutil.dir/util/mining.cpp.o
[417/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[418/579] Building CXX object src/test/CMakeFiles/testutil.dir/util/txmempool.cpp.o
[419/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/bdb.cpp.o
[420/579] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[421/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[422/579] Building CXX object src/test/CMakeFiles/testutil.dir/util/setup_common.cpp.o
[423/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[424/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/util.cpp.o
[425/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/signmessage.cpp.o
[426/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/receive.cpp.o
[427/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/transaction.cpp.o
[428/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/encrypt.cpp.o
[429/579] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[430/579] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[431/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/sqlite.cpp.o
[432/579] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[433/579] Building CXX object src/CMakeFiles/server.dir/validation.cpp.o
[434/579] Building CXX object src/iguana/CMakeFiles/iguana.dir/iguana_formatter.cpp.o
[435/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/salvage.cpp.o
[436/579] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/options.cpp.o
[437/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[438/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/spend.cpp.o
[439/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/interfaces.cpp.o
[440/579] Building CXX object src/iguana/CMakeFiles/iguana.dir/iguana_interpreter.cpp.o
[441/579] Building CXX object src/iguana/CMakeFiles/iguana.dir/iguana.cpp.o
[442/579] Linking CXX executable src/iguana/iguana
[443/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[444/579] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
[445/579] Building CXX object src/wallet/CMakeFiles/wallet-tool.dir/wallettool.cpp.o
[446/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[447/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/backup.cpp.o
[448/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
[449/579] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
[450/579] Linking CXX static library src/wallet/libwallet.a
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang failed with exit code 1
teamcity added a comment.Thu, Feb 27, 22:40
Build Bitcoin ABC Diffs / Diff Testing (build-clang-tidy) failed.

Tail of the build log:

[392/577] Building CXX object src/test/CMakeFiles/testutil.dir/util/coins.cpp.o
[393/577] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockindex.cpp.o
[394/577] Building CXX object src/test/CMakeFiles/testutil.dir/util/logging.cpp.o
[395/577] Building CXX object src/test/CMakeFiles/testutil.dir/util/str.cpp.o
[396/577] Building CXX object src/CMakeFiles/bitcoin-cli.dir/bitcoin-cli.cpp.o
[397/577] Linking CXX executable src/bitcoin-cli
[398/577] Building CXX object src/CMakeFiles/server.dir/rpc/rawtransaction.cpp.o
[399/577] Building CXX object src/CMakeFiles/server.dir/torcontrol.cpp.o
[400/577] Building C object src/secp256k1/CMakeFiles/ecmult-bench.dir/src/bench_ecmult.c.o
[401/577] Linking C executable src/secp256k1/ecmult-bench
[402/577] Building CXX object src/test/CMakeFiles/testutil.dir/util/random.cpp.o
[403/577] Building CXX object src/test/CMakeFiles/testutil.dir/util/transaction_utils.cpp.o
[404/577] Building CXX object src/CMakeFiles/server.dir/rpc/blockchain.cpp.o
[405/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/context.cpp.o
[406/577] Building CXX object src/CMakeFiles/bitcoin-tx.dir/bitcoin-tx.cpp.o
[407/577] Linking CXX executable src/bitcoin-tx
[408/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/bdb.cpp.o
[409/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/coinselection.cpp.o
[410/577] Building CXX object src/test/CMakeFiles/testutil.dir/util/blockfilter.cpp.o
[411/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/crypter.cpp.o
[412/577] Building CXX object src/CMakeFiles/server.dir/wallet/init.cpp.o
[413/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/db.cpp.o
[414/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/coincontrol.cpp.o
[415/577] Building CXX object src/test/CMakeFiles/testutil.dir/util/wallet.cpp.o
[416/577] Building CXX object src/test/CMakeFiles/testutil.dir/util/mining.cpp.o
[417/577] Building CXX object src/test/CMakeFiles/testutil.dir/util/net.cpp.o
[418/577] Building CXX object src/test/CMakeFiles/testutil.dir/util/validation.cpp.o
[419/577] Building CXX object src/test/CMakeFiles/testutil.dir/util/txmempool.cpp.o
[420/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/receive.cpp.o
[421/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/sqlite.cpp.o
[422/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/encrypt.cpp.o
[423/577] Building CXX object src/test/CMakeFiles/testutil.dir/util/setup_common.cpp.o
[424/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/transaction.cpp.o
[425/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/fees.cpp.o
[426/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/load.cpp.o
[427/577] Building CXX object src/CMakeFiles/server.dir/validation.cpp.o
[428/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletutil.cpp.o
[429/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/signmessage.cpp.o
[430/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/util.cpp.o
[431/577] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/dns.cpp.o
[432/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/walletdb.cpp.o
[433/577] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/options.cpp.o
[434/577] Building CXX object src/wallet/CMakeFiles/wallet-tool.dir/wallettool.cpp.o
[435/577] Building CXX object src/iguana/CMakeFiles/iguana.dir/iguana_formatter.cpp.o
[436/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/salvage.cpp.o
[437/577] Building CXX object src/iguana/CMakeFiles/iguana.dir/iguana_interpreter.cpp.o
[438/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/interfaces.cpp.o
[439/577] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/db.cpp.o
[440/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/spend.cpp.o
[441/577] Building CXX object src/iguana/CMakeFiles/iguana.dir/iguana.cpp.o
[442/577] Linking CXX executable src/iguana/iguana
[443/577] Building CXX object src/seeder/CMakeFiles/bitcoin-seeder.dir/main.cpp.o
[444/577] Building CXX object src/seeder/CMakeFiles/seeder-base.dir/bitcoin.cpp.o
[445/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/scriptpubkeyman.cpp.o
[446/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpc/backup.cpp.o
[447/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/wallet.cpp.o
[448/577] Building CXX object src/wallet/CMakeFiles/wallet.dir/rpcwallet.cpp.o
[449/577] Linking CXX static library src/wallet/libwallet.a
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1
Fabien updated this revision to Diff 52828.Fri, Feb 28, 10:52

Fix shadowing

Fabien edited the summary of this revision. (Show Details)Fri, Feb 28, 10:54
Harbormaster completed remote builds in B32532: Diff 52828.Fri, Feb 28, 11:06
Fabien published this revision for review.Fri, Feb 28, 11:28
PiRK accepted this revision.Fri, Feb 28, 13:44
PiRK added a subscriber: PiRK.
PiRK added inline comments.
test/functional/abc_p2p_avalanche_transaction_finalization.py
108–114 ↗(On Diff #52828)

I got confused because i initially thought the DAG direction was top to bottom. But the tx numbering should be enough to clarify the flow.

This revision is now accepted and ready to land.Fri, Feb 28, 13:44
Mengerian added a subscriber: Mengerian.Fri, Feb 28, 17:54
Mengerian added inline comments.
test/functional/abc_p2p_avalanche_transaction_finalization.py
104 ↗(On Diff #52828)
roqqit accepted this revision.Fri, Feb 28, 18:29
roqqit added a subscriber: roqqit.

re: the note at the end of the summary: when you write this other diff, be sure to include test coverage that attempt to poll finalized ancestors. there is risk of regression here otherwise.

test/functional/abc_p2p_avalanche_transaction_finalization.py
108–114 ↗(On Diff #52828)

I thought the same

Fabien updated this revision to Diff 52876.Mon, Mar 3, 08:23

Better log message, unleash my full ascii art creativity skills

Harbormaster completed remote builds in B32561: Diff 52876.Mon, Mar 3, 08:44
Closed by commit rABC37ee28b3d040: [avalanche] Also finalize the ancestors of a transaction (authored by Fabien). · Explain WhyMon, Mar 3, 08:58
This revision was automatically updated to reflect the committed changes.
Fabien added a commit: rABC37ee28b3d040: [avalanche] Also finalize the ancestors of a transaction.

Revision Contents
Changeset List

PathSize
src/
10 lines
30 lines
test/
functional/
175 lines

Diff 52876

View Options

src/txmempool.h

Loading...
View Options

src/txmempool.cpp

Loading...
View Options

test/functional/abc_p2p_avalanche_transaction_finalization.py

Loading...