Page MenuHomePhabricator

[avalanche] Add a debug check that we never remove a finalized transaction for conflict
ClosedPublic

Authored by Fabien on Jun 5 2025, 13:45.

Details

Summary

We reject blocks that contain a tx that conflicts with a finalized one, so this assumption should never break. It's cheap enough that we can assert this on debug bulds to increase confidence.

Test Plan

With debug:

ninja check-functional

Diff Detail

Repository
rABC Bitcoin ABC
Branch
avalanche_assume_finalized_conflict
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 33548
Build 66575: Build Diffbuild-diff · lint-circular-dependencies · build-clang · build-clang-tidy · build-debug · build-without-wallet
Build 66574: arc lint + arc unit

Event Timeline

Fabien requested review of this revision.Jun 5 2025, 13:45
bytesofman added a subscriber: bytesofman.
bytesofman added inline comments.
src/txmempool.cpp
310

if I understand correctly, we are actually already assuming this condition before this diff?

But now, in calling Assume, we are asserting it is not true -- meaning we can catch if this happens in debug builds?

so we aren't really adding an assumption so much as a check for if this does end up ever happening

This revision is now accepted and ready to land.Jun 5 2025, 16:44
src/txmempool.cpp
310

100% correct.

This function is called when a block is connected, and removes from the mempool the txs that conflict with the txs from the block. With preconsensus the blocks that contain a transaction conflicting with a finalized one are rejected so this should never happen. If this ever happens we are either forcing the situation in a test or we have a bug, in both cases it's better to raise an error so we can examine the situation