Page MenuHomePhabricator
Differential D16757

[avalanche] Vote on conflicting txs
ClosedPublic
Actions

Authored by Fabien on Sep 13 2024, 12:42.

Details

Reviewers
PiRK
roqqit
Group Reviewers
Restricted Project
Commits
rABC6e61b8cf6581: [avalanche] Vote on conflicting txs
Summary

This lets avalanche accept or reject transactions based on their conflicting state.
This pulls back the conflicting transaction upon rejection, or invalidate it upon finalization of its counterpart.

Depends on D16767.

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.Sep 13 2024, 12:42
Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 13 2024, 12:42
Fabien requested review of this revision.Sep 13 2024, 12:42
Harbormaster completed remote builds in B30278: Diff 49619.Sep 13 2024, 12:46
teamcity added a comment.Sep 13 2024, 12:46
Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) failed.

Tail of the build log:

: && /usr/bin/c++ -Werror -g -O2 -fuse-ld=gold -Wl,-z,noexecstack -Wl,-z,relro -Wl,-z,now -lstdc++fs -fPIE -pie src/CMakeFiles/bitcoin-chainstate.dir/bitcoin-chainstate.cpp.o -o src/bitcoin-chainstate  /usr/lib/x86_64-linux-gnu/libjemalloc_pic.a  -lstdc++fs  src/libbitcoinkernel.a  src/crypto/libcrypto.a  src/crypto/libcrypto_sse4.1.a  src/crypto/libcrypto_avx2.a  src/crypto/libcrypto_shani.a  src/univalue/libunivalue.a  src/secp256k1/libsecp256k1.a  src/leveldb/libleveldb.a  src/leveldb/libleveldb-sse4.2.a  src/leveldb/libmemenv.a  /usr/lib/x86_64-linux-gnu/libjemalloc_pic.a  -lm  -pthread  -ldl  -lstdc++fs && :
../../src/avalanche/processor.cpp:1198: error: undefined reference to 'TxPool::GetTx(TxId const&) const'
../../src/avalanche/processor.cpp:1261: error: undefined reference to 'TxPool::HaveTx(TxId const&) const'
collect2: error: ld returned 1 exit status
[586/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/bench_bitcoin.cpp.o
[587/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/cashaddr.cpp.o
[588/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/chacha_poly_aead.cpp.o
[589/640] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/intro.cpp.o
[590/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/chacha20.cpp.o
[591/640] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/bitcoin.cpp.o
[592/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/ccoins_caching.cpp.o
[593/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/crypto_hash.cpp.o
[594/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/examples.cpp.o
[595/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/checkqueue.cpp.o
[596/640] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsdialog.cpp.o
[597/640] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/optionsmodel.cpp.o
[598/640] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/guiutil.cpp.o
[599/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/block_assemble.cpp.o
[600/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/data.cpp.o
[601/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/lockedpool.cpp.o
[602/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/hashpadding.cpp.o
[603/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/gcs_filter.cpp.o
[604/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/merkle_root.cpp.o
[605/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/crypto_aes.cpp.o
[606/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/checkblock.cpp.o
[607/640] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/peertablemodel.cpp.o
[608/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/pool.cpp.o
[609/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/poly1305.cpp.o
[610/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/rollingbloom.cpp.o
[611/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/prevector.cpp.o
[612/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/duplicate_inputs.cpp.o
[613/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/streams_findbyte.cpp.o
[614/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/util_time.cpp.o
[615/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/bench.cpp.o
[616/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/chained_tx.cpp.o
[617/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/load_external.cpp.o
[618/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/mempool_eviction.cpp.o
[619/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/verify_script.cpp.o
[620/640] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qvalidatedlineedit.cpp.o
[621/640] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/platformstyle.cpp.o
[622/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/peer_eviction.cpp.o
[623/640] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qvaluecombobox.cpp.o
[624/640] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qrc_bitcoin.cpp.o
[625/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/mempool_stress.cpp.o
[626/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/rpc_mempool.cpp.o
[627/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/rpc_blockchain.cpp.o
[628/640] Building CXX object src/bench/CMakeFiles/bitcoin-bench.dir/nanobench.cpp.o
[629/640] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/utilitydialog.cpp.o
[630/640] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/trafficgraphwidget.cpp.o
[631/640] Linking CXX executable src/bench/bitcoin-bench
[632/640] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/splashscreen.cpp.o
[633/640] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/qrc_bitcoin_locale.cpp.o
[634/640] Building CXX object src/qt/CMakeFiles/bitcoin-qt-base.dir/rpcconsole.cpp.o
[635/640] Linking CXX static library src/qt/libbitcoin-qt-base.a
[636/640] Automatic MOC for target bitcoin-qt
[637/640] Building CXX object src/qt/CMakeFiles/bitcoin-qt.dir/bitcoin-qt_autogen/mocs_compilation.cpp.o
[638/640] Building CXX object src/qt/CMakeFiles/bitcoin-qt.dir/main.cpp.o
[639/640] Linking CXX executable src/qt/bitcoin-qt
ninja: build stopped: cannot make progress due to previous errors.
Build build-without-wallet failed with exit code 1
teamcity added a comment.Sep 13 2024, 12:56
Build Bitcoin ABC Diffs / Diff Testing (build-debug) failed.

Failed tests logs:

====== Bitcoin ABC functional tests: abc_p2p_avalanche_transaction_voting.py ======

------- Stderr: -------
File "/work/test/functional/abc_p2p_avalanche_transaction_voting.py", line 375
    with node.assert_debug_log([f"stored conflicting tx {conflicting_tx["txid"]}"]):
                                                                         ^
SyntaxError: f-string: unmatched '['

Each failure log is accessible here:
Bitcoin ABC functional tests: abc_p2p_avalanche_transaction_voting.py

teamcity added a comment.Sep 13 2024, 13:02
Build Bitcoin ABC Diffs / Diff Testing (build-diff) failed.

Failed tests logs:

====== Bitcoin ABC functional tests: abc_p2p_avalanche_transaction_voting.py ======

------- Stderr: -------
File "/work/test/functional/abc_p2p_avalanche_transaction_voting.py", line 375
    with node.assert_debug_log([f"stored conflicting tx {conflicting_tx["txid"]}"]):
                                                                         ^
SyntaxError: f-string: unmatched '['
====== Bitcoin ABC functional tests with the next upgrade activated: abc_p2p_avalanche_transaction_voting.py ======

------- Stderr: -------
File "/work/test/functional/abc_p2p_avalanche_transaction_voting.py", line 375
    with node.assert_debug_log([f"stored conflicting tx {conflicting_tx["txid"]}"]):
                                                                         ^
SyntaxError: f-string: unmatched '['

Each failure log is accessible here:
Bitcoin ABC functional tests: abc_p2p_avalanche_transaction_voting.py
Bitcoin ABC functional tests with the next upgrade activated: abc_p2p_avalanche_transaction_voting.py

Fabien updated this revision to Diff 49622.Sep 13 2024, 16:05

Fix the bitcoin-chainstate build, and fix python f-string fol older python versions

Harbormaster completed remote builds in B30280: Diff 49622.Sep 13 2024, 16:23
Fabien planned changes to this revision.Sep 16 2024, 13:01

Will add unit tests for GetTx and GetConflictTx, and maybe split this out

Fabien updated this revision to Diff 49654.Sep 16 2024, 18:58

Rebase on D16767

Fabien edited the summary of this revision. (Show Details)Sep 16 2024, 18:58
Fabien added a parent revision: D16767: Add methods to get a transaction and the conflicting ones from the tx pool.
Fabien updated this revision to Diff 49655.Sep 16 2024, 19:00

Remove unecessary header

Harbormaster completed remote builds in B30303: Diff 49654.Sep 16 2024, 19:17
Harbormaster completed remote builds in B30304: Diff 49655.Sep 16 2024, 19:32
roqqit added a subscriber: roqqit.Sep 16 2024, 23:57
roqqit added inline comments.
src/net_processing.cpp
6731 ↗(On Diff #49655)

Is the "and the finalized tree" part true? Shouldn't this never happen?

PiRK added a subscriber: PiRK.Sep 17 2024, 06:58
PiRK added inline comments.
test/functional/abc_p2p_avalanche_transaction_voting.py
468 ↗(On Diff #49655)
Fabien edited the summary of this revision. (Show Details)Sep 17 2024, 07:04
Fabien updated this revision to Diff 49667.Sep 17 2024, 07:08

Updated the summary to avoid confusion, fixed the typo and added a comment about the rejected finalized tx being a safety net

Harbormaster completed remote builds in B30315: Diff 49667.Sep 17 2024, 07:22
PiRK accepted this revision.Sep 17 2024, 07:34
This revision is now accepted and ready to land.Sep 17 2024, 07:34
Fabien edited the summary of this revision. (Show Details)Sep 17 2024, 19:57
roqqit accepted this revision.Sep 17 2024, 20:56
roqqit added inline comments.
test/functional/abc_p2p_avalanche_transaction_voting.py
199 ↗(On Diff #49667)

this comment should follow the moved block of code

Closed by commit rABC6e61b8cf6581: [avalanche] Vote on conflicting txs (authored by Fabien). · Explain WhySep 18 2024, 07:46
This revision was automatically updated to reflect the committed changes.
Fabien added a commit: rABC6e61b8cf6581: [avalanche] Vote on conflicting txs.

Revision Contents
Changeset List

PathSize
src/
1 line
avalanche/
4 lines
22 lines
149 lines
test/
functional/
270 lines
test_framework/
11 lines

Diff 49704

View Options

src/CMakeLists.txt

Loading...
View Options

src/avalanche/processor.h

Loading...
View Options

src/avalanche/processor.cpp

Loading...
View Options

src/net_processing.cpp

Loading...
View Options

test/functional/abc_p2p_avalanche_transaction_voting.py

Loading...
View Options

test/functional/test_framework/avatools.py

Loading...