-
Notifications
You must be signed in to change notification settings - Fork 405
feat(chain): Add extract_subgraph() and roots() methods to CanonicalView #2041
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
evanlinjin
wants to merge
12
commits into
bitcoindevkit:master
Choose a base branch
from
evanlinjin:canonical-view-extract-rbf
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
feat(chain): Add extract_subgraph() and roots() methods to CanonicalView #2041
evanlinjin
wants to merge
12
commits into
bitcoindevkit:master
from
evanlinjin:canonical-view-extract-rbf
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add `CanonicalView` structure with canonical transaction methods - Move methods from `TxGraph` to `CanonicalView` (txs, filter_outpoints, balance, etc.) - Add canonical view methods to `IndexedTxGraph` - Update all tests and examples to use new API - Optimize examples to reuse canonical view instances 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Add min_confirmations parameter to control confirmation depth requirements: - min_confirmations = 0: Include all confirmed transactions (same as 1) - min_confirmations = 1: Standard behavior - require at least 1 confirmation - min_confirmations = 6: High security - require at least 6 confirmations Transactions with fewer than min_confirmations are treated as trusted/untrusted pending based on the trust_predicate. This restores the minimum confirmation functionality that was available in the old TxGraph::balance doctest but with a more intuitive API since CanonicalView has the tip internally. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Add test file `tests/test_canonical_view.rs` with three comprehensive test cases: 1. `test_min_confirmations_parameter`: Tests basic min_confirmations functionality - Verifies min_confirmations = 0 and 1 behave identically - Tests edge case where transaction has exactly required confirmations - Tests case where transaction has insufficient confirmations 2. `test_min_confirmations_with_untrusted_tx`: Tests trust predicate interaction - Verifies insufficient confirmations + untrusted predicate = untrusted_pending - Ensures trust predicate is respected when confirmations are insufficient 3. `test_min_confirmations_multiple_transactions`: Tests complex scenarios - Multiple transactions with different confirmation counts - Verifies correct categorization based on min_confirmations threshold - Tests both min_confirmations = 5 and min_confirmations = 10 scenarios These tests validate that the min_confirmations parameter correctly controls when transactions are treated as confirmed vs trusted/untrusted pending. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
BREAKING: Remove `CanonicalTx` as it's no longer needed.
Add comprehensive docs for CanonicalView module, structs, and methods. Fix all doctests to compile and pass with proper imports.
Also update submodule-level docs for `tx_graph`'s Canonicalization section.
BREAKING CHANGE: `CanonicalView::balance()` now takes `additional_confirmations` instead of `min_confirmations`. The new parameter represents confirmations beyond the first (e.g., 5 means 6 total confirmations required). - Rename `why` to `reason` in CanonicalView for clarity - Update docs to clarify topological-spending order - Simplify docs by removing redundant conflict mentions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
BREAKING CHANGE: The trust_predicate parameter in CanonicalView::balance() now takes &FullTxOut<A> instead of ScriptBuf as its second argument. This provides more context to the predicate function. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
These methods are intended to help implement RBF logic.
Test extraction of transaction subgraphs with descendants, verifying that spends and txs fields maintain consistency. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cACK f081300
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds two new methods to
CanonicalView
to support RBF (Replace-By-Fee) transaction workflows:extract_subgraph()
: Extracts a transaction and all its descendants from the canonical viewroots()
: Returns transactions that have no parent transactions in the viewThese methods are essential for RBF support as they allow extracting specific transaction chains that can be replaced.
Note: This PR depends on #2029 which introduces the
CanonicalView
struct.Notes to the reviewers
The
extract_subgraph()
method removes the extracted transactions from the original view and returns them in a newCanonicalView
. This is useful for isolating transaction chains that need to be replaced in RBF scenarios.Changelog notice
Added
CanonicalView::extract_subgraph()
method to extract transactions with all their descendantsCanonicalView::roots()
method to find root transactions without parents in the viewChecklists
All Submissions:
New Features:
🤖 Generated with Claude Code