Prune chain #4470
Prune chain #4470
Conversation
WalkthroughThe pull request simplifies the logic for blockchain pruning heights and block header upgrades. In the blockchain module, the Changes
Sequence Diagram(s)sequenceDiagram
participant BC as BlockChain
participant Ch as Chain Checker
participant Client as Client
BC->>Ch: Check if chain is "vega"
alt If chain is vega
Ch-->>BC: Yes
BC->>Client: Return 3500000
else
Ch-->>BC: No
BC->>Client: Return 0
end
sequenceDiagram
participant BH as BlockHeader
participant Num as BlockNumber
participant Chk as ChainID Checker
participant Client as Client
BH->>Num: Retrieve block number
alt Block number is 0
Num-->>BH: 0
BH->>Client: Return false
else
BH->>Chk: Check if chain is "vega"
alt If chain is vega and block number ≥ 3,300,000
Chk-->>BH: Yes
BH->>Client: Return true
else
Chk-->>BH: No or below threshold
BH->>Client: Return false
end
end
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
sanlee42
requested review from
jackzhhuang
and removed request for
jolestar and
nkysg
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
chain/src/chain.rs (1)
1658-1667: Code simplification for pruning height determination.The function has been simplified to handle only three cases:
- Vega chain - return 3500000
- Test/development chains - return MAX value
- All other chains - return 0 (changed from 1)
The comment indicates this is a legacy function where pruning should always start from genesis (block 0). This aligns with the default return value change from 1 to 0.
Consider documenting the significance of the specific pruning height value (3500000) for the Vega chain, as this "magic number" lacks context in the current implementation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
chain/src/chain.rs(1 hunks)types/src/block/mod.rs(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
chain/src/chain.rs (9)
chain/api/src/chain.rs (1)
get_pruning_height(118-118)types/src/block/mod.rs (1)
chain_id(392-394)rpc/client/src/lib.rs (1)
chain_id(745-748)vm/types/src/transaction/mod.rs (2)
chain_id(275-277)chain_id(535-537)chain/open-block/src/lib.rs (1)
chain_id(160-162)config/src/genesis_config.rs (4)
chain_id(142-144)chain_id(256-258)chain_id(356-361)chain_id(512-514)vm/types/src/block_metadata/mod.rs (1)
chain_id(141-143)types/src/startup_info.rs (1)
chain_id(35-37)vm/types/src/proptest_types.rs (1)
chain_id(192-194)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Generate code coverage report
🔇 Additional comments (2)
types/src/block/mod.rs (2)
601-601: Added legacy comment to block header deserialization.The "legacy" comment has been added to mark code related to backwards compatibility during block header deserialization, specifically around the block upgrade check logic.
508-517:❓ Verification inconclusive
Simplified block upgrade logic for chain types.
The function has been simplified to only check for the Vega chain specifically (requiring block number ≥ 3300000), while applying a default upgrade behavior for all other chains. The removed checks for
halleyandproximachains streamline the codebase by consolidating legacy chain handling.The comment suggests uncertainty about whether only main blocks need upgrading. Verify whether this simplification impacts any existing chain types that relied on the specific thresholds that were removed:
🏁 Script executed:
#!/bin/bash # Search for uses of is_halley and is_proxima in the codebase echo "Searching for uses of is_halley and is_proxima:" rg "is_halley|is_proxima" --type rustLength of output: 830
Please run the above script and share the output so we can verify if the simplified logic in
check_upgradeimpacts any chain types expected to have threshold checks (like halley or proxima).
🏁 Script executed:
#!/bin/bash # Search for uses of check_upgrade in the codebase to see if it's conditionally used for specific chain types rg "check_upgrade(" --type rustLength of output: 112
Simplified Logic Verified – Please Manually Confirm Chain–Specific Behavior
The updated
check_upgradeintypes/src/block/mod.rsnow exclusively checks for the Vega chain (requiring a block number ≥ 3300000) while using a default upgrade (true) for all other chains. Our search confirmed that legacy chain conditions (e.g. for Halley and Proxima) still exist elsewhere (see usage invm/vm-runtime/src/force_upgrade_management.rsand in variousconfigfiles) and appear to be handled independently. However, since our automated search for direct invocations ofcheck_upgradewas inconclusive, please manually verify that no chain-specific upgrade thresholds are being inadvertently affected by this simplification.
Benchmark for 809ba30Click to view benchmark
|
Pull request type
Please check the type of change your PR introduces:
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Other information
Summary by CodeRabbit