Skip to content

Check the transactions before pushing them into the new block body#4791

Open
jackzhhuang wants to merge 7 commits intodual-verse-dagfrom
check-blue-trx
Open

Check the transactions before pushing them into the new block body#4791
jackzhhuang wants to merge 7 commits intodual-verse-dagfrom
check-blue-trx

Conversation

@jackzhhuang
Copy link
Contributor

@jackzhhuang jackzhhuang commented Jan 21, 2026

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Other information

Summary by CodeRabbit

  • New Features

    • Block creation now uses chain-aware fetching and a new cross-VM state-check to select only continuous, sequential multi-VM transactions from recent blocks and the pending pool, with post-filter partitioning back into VM-specific pending lists.
  • Tests

    • Added unit tests validating cross-VM sequence checking, per-sender caching, and filtering behavior.
  • Chores

    • Added workspace dependencies to support the new state-checking functionality.

✏️ Tip: You can customize this high-level summary in your review settings.

@jackzhhuang jackzhhuang requested a review from sanlee42 as a code owner January 21, 2026 08:38
@coderabbitai
Copy link

coderabbitai bot commented Jan 21, 2026

📝 Walkthrough

Walkthrough

Adds a new StateCheck module and tests, changes fetch_transactions to accept a BlockChain (using its state readers) and apply state-aware filtering/partitioning of VM1/VM2 transactions, and adds two VM2-related workspace dependencies in miner/Cargo.toml.

Changes

Cohort / File(s) Summary
New Dependencies
\miner/Cargo.toml``
Added workspace dependencies starcoin-vm2-state-api and starcoin-vm2-statedb.
Block Builder Service
\miner/src/create_block_template/block_builder_service.rs``
fetch_transactions signature changed to accept &BlockChain; aggregates blue + pool txns, sorts by sender, creates StateCheck with chain state readers, applies filter_continuous_transactions, then repartitions results into VM1/VM2 pending lists; call sites updated.
StateCheck module
\miner/src/create_block_template/state_check.rs``
New StateCheck<'a, R1, R2> providing per-sender caches and methods: new, get_next_expected_sequence, check_and_accept, filter_continuous_transactions, clear_cache.
StateCheck tests
\miner/src/create_block_template/test_state_check.rs``
New unit tests with mock VM1/VM2 state readers and helpers verifying continuous-sequence filtering and cache behavior.
Module export & tests
\miner/src/create_block_template/mod.rs``
Added pub mod state_check; and #[cfg(test)] mod test_state_check;.

Sequence Diagram(s)

sequenceDiagram
    participant BlockBuilder as Block Builder
    participant Chain as BlockChain
    participant TxPool as Tx Pool
    participant StateCheck as StateCheck
    participant VM1 as VM1 State Reader
    participant VM2 as VM2 State Reader

    rect rgba(200,200,255,0.5)
    BlockBuilder->>Chain: fetch_transactions(chain, blue_blocks, max_txns)
    end

    rect rgba(200,255,200,0.5)
    Chain->>TxPool: query pending txs (VM1 + VM2)
    TxPool-->>Chain: return aggregated tx list
    end

    rect rgba(255,220,200,0.5)
    Chain->>StateCheck: instantiate with (VM1 reader, VM2 reader)
    Chain->>StateCheck: filter_continuous_transactions(all_txns)
    StateCheck->>VM1: get_next_expected_sequence(sender_vm1)
    VM1-->>StateCheck: next_seq_vm1
    StateCheck->>VM2: get_next_expected_sequence(sender_vm2)
    VM2-->>StateCheck: next_seq_vm2
    StateCheck-->>Chain: return filtered txs
    end

    rect rgba(240,240,240,0.5)
    Chain-->>BlockBuilder: return (VM1 txns, VM2 txns)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • sanlee42
  • welbon
  • simonjiao

"I nibble through transactions bright,
Two VMs hopping in the moonlight,
Caches hum to keep the sequence true,
Filters hop in tidy queue,
The rabbit grins — a block or two!" 🐇🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.06% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main objective: adding transaction validation checks before block inclusion.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch check-blue-trx

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@miner/src/create_block_template/block_builder_service.rs`:
- Around line 652-682: The current code appends blue_blocks' transactions into
all_transactions so filter_continuous_transactions may return blue-block txns
back into the new block body; instead, only send txpool transactions
(pending_multi_transactions) to filter_continuous_transactions and use
blue_blocks only to advance/prime the sequence state before filtering.
Concretely: stop pushing blue_blocks' VM1/VM2 txns into all_transactions;
iterate blue_blocks and call a sequence-priming method on state_check (e.g.,
state_check.prime_with_transaction / state_check.advance_sequence or add such a
helper) to update sequence counters from block.transactions() and
block.transactions2(); then call state_check.filter_continuous_transactions on
the txpool-derived all_transactions and proceed to split results into
pending_transactions / pending_transactions2 as before. Ensure
MultiSignedUserTransaction handling and sender/sequence ordering remain
unchanged.
🧹 Nitpick comments (1)
miner/src/create_block_template/block_builder_service.rs (1)

633-650: Rename or remove the “[jacktest]” log tag.
This looks like a temporary debug label; consider aligning with existing log prefixes before merging.

Also applies to: 684-689

@jackzhhuang jackzhhuang force-pushed the check-blue-trx branch 2 times, most recently from 56b7805 to d04a759 Compare January 23, 2026 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant