Skip to content

Comments

feat: do branch and bound#78

Open
SWvheerden wants to merge 1 commit intotari-project:mainfrom
SWvheerden:sw_branch_bound
Open

feat: do branch and bound#78
SWvheerden wants to merge 1 commit intotari-project:mainfrom
SWvheerden:sw_branch_bound

Conversation

@SWvheerden
Copy link
Contributor

Description

do utxo selection via branch and bound

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @SWvheerden, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the wallet's transaction management capabilities by integrating a sophisticated 'branch and bound' algorithm for UTXO selection, which promises more efficient transaction construction. Concurrently, it introduces a comprehensive refactoring of the transaction display and history processing, making it more resilient to blockchain reorgs through explicit reversal tracking in the database. These changes collectively improve the accuracy, efficiency, and robustness of how the wallet handles and presents user transactions.

Highlights

  • Branch and Bound UTXO Selection: Implemented a more advanced 'branch and bound' algorithm for UTXO selection, replacing the previous greedy approach. This aims to find more optimal combinations of UTXOs for transactions, potentially improving efficiency and reducing transaction fees.
  • Enhanced Transaction Display Logic: The DisplayedTransactionProcessor has been completely refactored to improve how balance changes are grouped and presented as user-facing transactions. This includes new logic for matching inputs and outputs, and handling the creation and updating of displayed transactions.
  • Blockchain Reorg Handling for Balance Changes: Introduced new database fields (is_reversal, reversal_of_balance_change_id, is_reversed) and associated logic to the balance_changes table. This allows for robust tracking and reversal of balance changes during blockchain reorgs, ensuring data consistency.
  • Centralized Block Event Accumulation: A new BlockEventAccumulator struct centralizes the collection of detected outputs, spent inputs, and balance changes for each processed block, streamlining event handling and transaction reconstruction.
  • Dependency Updates: Updated various tari-project related dependencies to newer revisions, likely incorporating upstream changes necessary for the new features.
Changelog
  • Cargo.lock
    • Updated lightweight_wallet_libs and several minotari_app_grpc, minotari_ledger_wallet_common, tari_common, tari_common_types, tari_features, tari_hashing, tari_jellyfish, tari_max_size, tari_node_components, tari_script, tari_sidechain, tari_transaction_components dependencies to newer revisions.
  • Cargo.toml
    • Updated lightweight_wallet_libs, tari_common, tari_common_types, tari_script, tari_transaction_components to newer revisions.
  • docs/db/db_schema.sql
    • Added is_reversal (BOOLEAN, default FALSE), reversal_of_balance_change_id (INTEGER, foreign key to balance_changes), and is_reversed (BOOLEAN, default FALSE) columns to the balance_changes table.
    • Added idx_balance_changes_reversal_of and idx_balance_changes_is_reversed indexes.
  • migrations/00025-add_reversal_flags_to_balance_changes/up.sql
    • Added a new migration script to introduce the is_reversal, reversal_of_balance_change_id, and is_reversed columns and their respective indexes to the balance_changes table.
  • openapi.json
    • Changed the required field for TransactionInput from is_matched to matched_output_id.
    • Updated the type of matched_output_id from [integer, null] to integer.
  • src/db/balance_changes.rs
    • Modified insert_balance_change to return the i64 ID of the inserted row and to accept new is_reversal, reversal_of_balance_change_id, and is_reversed fields.
    • Added get_all_active_balance_changes_by_account_id to retrieve non-reversed balance changes.
    • Added get_balance_change_id_by_output, get_balance_change_id_by_input, and mark_balance_change_as_reversed functions.
  • src/db/inputs.rs
    • Added DbInput struct and get_input_by_id function.
    • Removed get_input_details_for_balance_change_by_id.
    • Updated soft_delete_inputs_from_height to mark original balance changes as reversed and create new reversal balance changes.
  • src/db/mod.rs
    • Updated public exports for outputs, balance_changes, and inputs modules to include new functions and structs.
  • src/db/outputs.rs
    • Added DbOutput struct and get_output_by_id function.
    • Updated soft_delete_outputs_from_height to mark original balance changes as reversed and create new reversal balance changes.
    • DbWalletOutput now implements the UtxoValue trait.
  • src/models/mod.rs
    • Added is_reversal, reversal_of_balance_change_id, and is_reversed fields to the BalanceChange struct.
  • src/scan/block_event_accumulator.rs
    • Added a new file defining BlockEventAccumulator for collecting block processing events.
  • src/scan/block_processor.rs
    • Refactored block processing logic to use BlockEventAccumulator.
    • Removed find_matching_pending_outbound and merge_pending_with_scanned methods.
    • Updated record_output_balance_change and record_input_balance_change to include new balance change flags.
    • Removed outputs_confirmed from BlockProcessedEvent.
  • src/scan/events.rs
    • Added Id import.
    • Removed outputs_confirmed from BlockProcessedEvent.
    • Added output_id and mined_in_block_height to SpentInput.
  • src/scan/mod.rs
    • Added block_event_accumulator module.
  • src/transactions/displayed_transaction_processor/grouping.rs
    • Removed file.
  • src/transactions/displayed_transaction_processor/mod.rs
    • Removed grouping and resolver modules.
  • src/transactions/displayed_transaction_processor/processor.rs
    • Rewrote transaction processing logic, removing old grouping and resolver mechanisms.
    • Introduced create_new_updated_display_transactions_for_height and create_new_updated_display_transactions for managing displayed transactions.
    • Added iter_search_for_matching_inputs for UTXO selection.
    • Updated process_all_stored_with_conn to use the new processing flow.
  • src/transactions/displayed_transaction_processor/resolver/database.rs
    • Removed file.
  • src/transactions/displayed_transaction_processor/resolver/in_memory.rs
    • Removed file.
  • src/transactions/displayed_transaction_processor/resolver/mod.rs
    • Removed file.
  • src/transactions/displayed_transaction_processor/types.rs
    • Changed matched_output_id in TransactionInput from Option to Id and removed is_matched.
  • src/transactions/input_selector.rs
    • Added ServiceError to UtxoSelectionError.
    • Integrated BranchAndBoundUtxoSelectionBuilder for UTXO selection.
    • Updated fund checking logic to align with the new selection algorithm.
  • src/transactions/manager.rs
    • Updated ProcessedTransaction to store Vec.
    • Adjusted UTXO handling and TransactionInput creation to use DbWalletOutput IDs.
  • src/transactions/mod.rs
    • Removed ProcessingContext export.
  • src/transactions/transaction_history.rs
    • Modified rebuild_from_balance_changes to accept required_confirmations and return a tuple of updated and new DisplayedTransaction's.
    • Updated DisplayedTransactionProcessor instantiation and usage.
Activity
  • The pull request introduces a new feature, 'branch and bound' UTXO selection, and a significant refactoring of transaction processing and display. This is a substantial change that will require thorough review, especially of the new algorithm and the updated transaction history logic. The addition of database fields for reversal tracking indicates a focus on robustness against blockchain reorgs.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a significant and valuable refactoring of the UTXO selection and transaction history processing logic. The adoption of a branch-and-bound algorithm for coin selection is a major improvement, and the enhanced handling of reorgs through explicit reversal tracking in the database greatly improves data integrity. The code is generally well-structured. I've identified a performance issue in the transaction history rebuilding logic and a minor code cleanup opportunity in the input selector. Overall, this is a strong contribution that modernizes key components of the wallet.

@SWvheerden SWvheerden marked this pull request as ready for review February 9, 2026 09:58
@SWvheerden
Copy link
Contributor Author

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces reversal tracking to balance changes, adds a BlockEventAccumulator to collect block processing data, and refactors displayed transaction processing. The Cargo.lock and Cargo.toml files are updated to reflect changes in tari and tari-wallet dependencies. The database schema is modified to include reversal flags in the balance_changes table, and corresponding migrations are added. The openapi.json file is updated to reflect changes in the matched output ID. The balance_changes.rs file is modified to include reversal tracking fields, and the inputs.rs and outputs.rs files are updated to handle soft deletes and balance change reversals. The block_processor.rs file is refactored to use the BlockEventAccumulator and improve displayed transaction processing. The review comments highlight an issue in the original code where the logic for checking available funds was incorrect and could lead to a panic due to subtracting locked amounts from the total unspent balance, which has been addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant