-
Notifications
You must be signed in to change notification settings - Fork 529
feat: report reorg if checkpoint index decreased but block height stayed the same or increased #7212
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
Conversation
…yed the same or increased
|
📝 WalkthroughWalkthroughAdds a new public CheckpointInfo type and DB APIs to store/retrieve it, implements them in RocksDB and test mocks, updates ValidatorSubmitter to load/track/persist latest checkpoint info and centralizes reorg detection/reporting, and changes ReorgEvent to carry local vs canonical indices. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant VS as ValidatorSubmitter
participant DB as HyperlaneDb
participant L as Logger
Note over VS,DB: startup — load persisted latest checkpoint info
VS->>DB: retrieve_latest_checkpoint_info()
DB-->>VS: Option<CheckpointInfo>
Note over VS: on new on-chain checkpoint
VS->>VS: compare incoming.checkpoint_index vs latest_seen.checkpoint_index
alt regression (incoming < latest_seen)
VS->>VS: panic_with_reorg(incoming, latest_seen)
VS->>L: report_reorg_with_checkpoint(...)
VS-->>VS: abort/panic
else advance (incoming > latest_seen)
VS->>DB: store_latest_checkpoint_info(CheckpointInfo{checkpoint_index, block_height})
VS->>L: log advancement
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)rust/main/agents/validator/**/src/**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
rust/main/**/src/**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧬 Code graph analysis (1)rust/main/agents/validator/src/submit.rs (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
🔇 Additional comments (4)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7212 +/- ##
=====================================
Coverage 0.00% 0
=====================================
Files 1 0 -1
Lines 14 0 -14
=====================================
+ Misses 14 0 -14
🚀 New features to boost your workflow:
|
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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
rust/main/agents/relayer/src/msg/db_loader.rs(1 hunks)rust/main/agents/relayer/src/msg/pending_message.rs(1 hunks)rust/main/agents/validator/src/submit.rs(5 hunks)rust/main/agents/validator/src/submit/tests.rs(1 hunks)rust/main/hyperlane-base/src/db/mod.rs(1 hunks)rust/main/hyperlane-base/src/db/rocks/hyperlane_db.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
rust/main/agents/validator/**/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain validator agent Rust sources under rust/main/agents/validator
Files:
rust/main/agents/validator/src/submit.rsrust/main/agents/validator/src/submit/tests.rs
rust/main/**/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Run cargo clippy for Rust code linting
Files:
rust/main/agents/validator/src/submit.rsrust/main/agents/validator/src/submit/tests.rsrust/main/hyperlane-base/src/db/mod.rsrust/main/hyperlane-base/src/db/rocks/hyperlane_db.rsrust/main/agents/relayer/src/msg/db_loader.rsrust/main/agents/relayer/src/msg/pending_message.rs
rust/main/{hyperlane-core,hyperlane-base}/**/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Keep shared Rust core crates in rust/main/{hyperlane-core,hyperlane-base}
Files:
rust/main/hyperlane-base/src/db/mod.rsrust/main/hyperlane-base/src/db/rocks/hyperlane_db.rs
rust/main/agents/relayer/**/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain relayer agent Rust sources under rust/main/agents/relayer
Files:
rust/main/agents/relayer/src/msg/db_loader.rsrust/main/agents/relayer/src/msg/pending_message.rs
🧬 Code graph analysis (6)
rust/main/agents/validator/src/submit.rs (3)
rust/main/chains/hyperlane-ethereum/src/contracts/merkle_tree_hook.rs (3)
latest_checkpoint(253-277)block_height(326-331)tree(301-311)rust/main/chains/hyperlane-starknet/src/merkle_tree_hook.rs (3)
latest_checkpoint(67-91)tree(95-130)tree(107-111)rust/main/hyperlane-core/src/traits/merkle_tree_hook.rs (2)
latest_checkpoint(49-50)tree(37-37)
rust/main/agents/validator/src/submit/tests.rs (2)
rust/main/hyperlane-base/src/db/mod.rs (4)
store_latest_checkpoint_block_height(178-178)retrieve_latest_checkpoint_block_height(180-180)store_latest_checkpoint_index(183-183)retrieve_latest_checkpoint_index(185-185)rust/main/hyperlane-base/src/db/rocks/hyperlane_db.rs (4)
store_latest_checkpoint_block_height(704-710)retrieve_latest_checkpoint_block_height(711-713)store_latest_checkpoint_index(715-717)retrieve_latest_checkpoint_index(718-720)
rust/main/hyperlane-base/src/db/mod.rs (1)
rust/main/hyperlane-base/src/db/rocks/hyperlane_db.rs (4)
store_latest_checkpoint_block_height(704-710)retrieve_latest_checkpoint_block_height(711-713)store_latest_checkpoint_index(715-717)retrieve_latest_checkpoint_index(718-720)
rust/main/hyperlane-base/src/db/rocks/hyperlane_db.rs (1)
rust/main/hyperlane-base/src/db/mod.rs (4)
store_latest_checkpoint_block_height(178-178)retrieve_latest_checkpoint_block_height(180-180)store_latest_checkpoint_index(183-183)retrieve_latest_checkpoint_index(185-185)
rust/main/agents/relayer/src/msg/db_loader.rs (2)
rust/main/hyperlane-base/src/db/mod.rs (4)
store_latest_checkpoint_block_height(178-178)retrieve_latest_checkpoint_block_height(180-180)store_latest_checkpoint_index(183-183)retrieve_latest_checkpoint_index(185-185)rust/main/hyperlane-base/src/db/rocks/hyperlane_db.rs (4)
store_latest_checkpoint_block_height(704-710)retrieve_latest_checkpoint_block_height(711-713)store_latest_checkpoint_index(715-717)retrieve_latest_checkpoint_index(718-720)
rust/main/agents/relayer/src/msg/pending_message.rs (2)
rust/main/hyperlane-base/src/db/mod.rs (4)
store_latest_checkpoint_block_height(178-178)retrieve_latest_checkpoint_block_height(180-180)store_latest_checkpoint_index(183-183)retrieve_latest_checkpoint_index(185-185)rust/main/hyperlane-base/src/db/rocks/hyperlane_db.rs (4)
store_latest_checkpoint_block_height(704-710)retrieve_latest_checkpoint_block_height(711-713)store_latest_checkpoint_index(715-717)retrieve_latest_checkpoint_index(718-720)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (51)
- GitHub Check: infra-test
- GitHub Check: cli-evm-e2e-matrix (warp-read)
- GitHub Check: env-test-matrix (mainnet3, inevm, igp)
- GitHub Check: cli-evm-e2e-matrix (warp-send)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-config)
- GitHub Check: env-test-matrix (mainnet3, ethereum, core)
- GitHub Check: env-test-matrix (testnet4, sepolia, core)
- GitHub Check: env-test-matrix (mainnet3, inevm, core)
- GitHub Check: env-test-matrix (mainnet3, ethereum, igp)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
- GitHub Check: env-test-matrix (mainnet3, optimism, igp)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
- GitHub Check: cli-evm-e2e-matrix (warp-check-3)
- GitHub Check: env-test-matrix (mainnet3, optimism, core)
- GitHub Check: cli-evm-e2e-matrix (warp-check-2)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-basic)
- GitHub Check: cli-evm-e2e-matrix (warp-init)
- GitHub Check: cli-evm-e2e-matrix (warp-rebalancer)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-2)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
- GitHub Check: cli-evm-e2e-matrix (core-init)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-2)
- GitHub Check: cli-evm-e2e-matrix (core-read)
- GitHub Check: cli-evm-e2e-matrix (warp-check-1)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-submitters)
- GitHub Check: cli-evm-e2e-matrix (core-check)
- GitHub Check: cli-evm-e2e-matrix (core-apply)
- GitHub Check: cli-evm-e2e-matrix (relay)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-1)
- GitHub Check: cli-evm-e2e-matrix (core-deploy)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-deploy)
- GitHub Check: cosmos-sdk-e2e-run
- GitHub Check: cli-cosmos-e2e-matrix (core-apply)
- GitHub Check: cli-cosmos-e2e-matrix (warp-read)
- GitHub Check: cli-cosmos-e2e-matrix (core-read)
- GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (core-check)
- GitHub Check: build-and-push-to-gcr
- GitHub Check: lander-coverage
- GitHub Check: lint-rs
- GitHub Check: test-rs
- GitHub Check: e2e-matrix (starknet)
- GitHub Check: e2e-matrix (cosmosnative)
- GitHub Check: e2e-matrix (radix)
- GitHub Check: e2e-matrix (sealevel)
- GitHub Check: e2e-matrix (evm)
- GitHub Check: e2e-matrix (cosmwasm)
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
rust/main/agents/relayer/src/msg/db_loader.rs(1 hunks)rust/main/agents/relayer/src/msg/pending_message.rs(1 hunks)rust/main/agents/validator/src/submit.rs(5 hunks)rust/main/agents/validator/src/submit/tests.rs(1 hunks)rust/main/hyperlane-base/src/db/mod.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- rust/main/agents/relayer/src/msg/pending_message.rs
- rust/main/agents/validator/src/submit/tests.rs
🧰 Additional context used
📓 Path-based instructions (4)
rust/main/{hyperlane-core,hyperlane-base}/**/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Keep shared Rust core crates in rust/main/{hyperlane-core,hyperlane-base}
Files:
rust/main/hyperlane-base/src/db/mod.rs
rust/main/**/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Run cargo clippy for Rust code linting
Files:
rust/main/hyperlane-base/src/db/mod.rsrust/main/agents/relayer/src/msg/db_loader.rsrust/main/agents/validator/src/submit.rs
rust/main/agents/relayer/**/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain relayer agent Rust sources under rust/main/agents/relayer
Files:
rust/main/agents/relayer/src/msg/db_loader.rs
rust/main/agents/validator/**/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain validator agent Rust sources under rust/main/agents/validator
Files:
rust/main/agents/validator/src/submit.rs
🧬 Code graph analysis (3)
rust/main/hyperlane-base/src/db/mod.rs (1)
rust/main/hyperlane-base/src/db/rocks/hyperlane_db.rs (4)
store_latest_checkpoint_block_height(704-710)retrieve_latest_checkpoint_block_height(711-713)store_latest_checkpoint_index(715-717)retrieve_latest_checkpoint_index(718-720)
rust/main/agents/relayer/src/msg/db_loader.rs (2)
rust/main/hyperlane-base/src/db/mod.rs (4)
store_latest_checkpoint_block_height(178-178)retrieve_latest_checkpoint_block_height(180-180)store_latest_checkpoint_index(183-183)retrieve_latest_checkpoint_index(185-185)rust/main/hyperlane-base/src/db/rocks/hyperlane_db.rs (4)
store_latest_checkpoint_block_height(704-710)retrieve_latest_checkpoint_block_height(711-713)store_latest_checkpoint_index(715-717)retrieve_latest_checkpoint_index(718-720)
rust/main/agents/validator/src/submit.rs (3)
rust/main/chains/hyperlane-ethereum/src/contracts/merkle_tree_hook.rs (3)
latest_checkpoint(253-277)block_height(326-331)tree(301-311)rust/main/chains/hyperlane-starknet/src/merkle_tree_hook.rs (3)
latest_checkpoint(67-91)tree(95-130)tree(107-111)rust/main/hyperlane-core/src/traits/merkle_tree_hook.rs (2)
latest_checkpoint(49-50)tree(37-37)
🔇 Additional comments (6)
rust/main/agents/relayer/src/msg/db_loader.rs (1)
740-749: Mock methods match the trait interface.The four checkpoint persistence methods are properly declared in the mock, following the existing mockall pattern. These'll work just fine for testing the new checkpoint tracking behavior.
rust/main/agents/validator/src/submit.rs (5)
141-162: Reorg detection logic looks solid.The check catches the case where the checkpoint index goes backwards but the block height doesn't - that's exactly what we're after. The nested conditionals make sense since not all chains provide block_height.
164-194: Checkpoint persistence addresses the previous review.Good work persisting both index and block_height after updates. The in-memory state stays consistent, and we'll still detect reorgs on the next loop even if the writes fail. The error logs give us visibility into storage issues without crashing the validator.
Based on learnings (past review comment).
307-316: Clean refactor to use the new helper.Consolidating the reorg panic logic here makes sense. All the necessary context is passed through properly.
339-385: Helpers centralize reorg handling nicely.The
panic_with_reorgfunction pulls together all the reorg response logic in one spot - building the event, logging, reporting, and writing status. Thereport_reorg_with_checkpointhelper handles the block-height-or-period decision cleanly. These make the code easier to follow and maintain.
121-130: Verify logging of DB retrieval failures: at lines 121–130 we callretrieve_latest_checkpoint_index()andretrieve_latest_checkpoint_block_height()thenunwrap_or_default(), which swallows errors and defaults to 0; log or propagate failures so we don’t silently reset our checkpoint state.
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
rust/main/agents/relayer/src/msg/db_loader.rs(2 hunks)rust/main/agents/relayer/src/msg/pending_message.rs(1 hunks)rust/main/agents/validator/src/submit.rs(5 hunks)rust/main/agents/validator/src/submit/tests.rs(3 hunks)rust/main/hyperlane-base/src/db/mod.rs(2 hunks)rust/main/hyperlane-base/src/db/rocks/hyperlane_db.rs(3 hunks)rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs(1 hunks)rust/main/hyperlane-core/src/types/checkpoint.rs(2 hunks)rust/main/hyperlane-core/src/types/reorg.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- rust/main/agents/relayer/src/msg/db_loader.rs
- rust/main/hyperlane-base/src/db/rocks/hyperlane_db.rs
🧰 Additional context used
📓 Path-based instructions (4)
rust/main/{hyperlane-core,hyperlane-base}/**/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Keep shared Rust core crates in rust/main/{hyperlane-core,hyperlane-base}
Files:
rust/main/hyperlane-core/src/types/checkpoint.rsrust/main/hyperlane-base/src/db/mod.rsrust/main/hyperlane-base/src/settings/checkpoint_syncer.rsrust/main/hyperlane-core/src/types/reorg.rs
rust/main/**/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Run cargo clippy for Rust code linting
Files:
rust/main/hyperlane-core/src/types/checkpoint.rsrust/main/hyperlane-base/src/db/mod.rsrust/main/agents/relayer/src/msg/pending_message.rsrust/main/hyperlane-base/src/settings/checkpoint_syncer.rsrust/main/agents/validator/src/submit/tests.rsrust/main/agents/validator/src/submit.rsrust/main/hyperlane-core/src/types/reorg.rs
rust/main/agents/relayer/**/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain relayer agent Rust sources under rust/main/agents/relayer
Files:
rust/main/agents/relayer/src/msg/pending_message.rs
rust/main/agents/validator/**/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain validator agent Rust sources under rust/main/agents/validator
Files:
rust/main/agents/validator/src/submit/tests.rsrust/main/agents/validator/src/submit.rs
🧬 Code graph analysis (6)
rust/main/hyperlane-core/src/types/checkpoint.rs (1)
rust/main/chains/hyperlane-ethereum/src/contracts/merkle_tree_hook.rs (1)
block_height(326-331)
rust/main/hyperlane-base/src/db/mod.rs (1)
rust/main/hyperlane-base/src/db/rocks/hyperlane_db.rs (2)
store_latest_checkpoint_info(703-705)retrieve_latest_checkpoint_info(706-708)
rust/main/agents/relayer/src/msg/pending_message.rs (2)
rust/main/hyperlane-base/src/db/mod.rs (2)
store_latest_checkpoint_info(178-178)retrieve_latest_checkpoint_info(180-180)rust/main/hyperlane-base/src/db/rocks/hyperlane_db.rs (2)
store_latest_checkpoint_info(703-705)retrieve_latest_checkpoint_info(706-708)
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs (1)
rust/main/hyperlane-core/src/chain.rs (1)
from_blocks(53-57)
rust/main/agents/validator/src/submit/tests.rs (3)
rust/main/hyperlane-core/src/test_utils.rs (1)
dummy_domain(44-53)rust/main/hyperlane-base/src/db/mod.rs (2)
store_latest_checkpoint_info(178-178)retrieve_latest_checkpoint_info(180-180)rust/main/hyperlane-base/src/db/rocks/hyperlane_db.rs (2)
store_latest_checkpoint_info(703-705)retrieve_latest_checkpoint_info(706-708)
rust/main/agents/validator/src/submit.rs (4)
rust/main/chains/hyperlane-ethereum/src/contracts/merkle_tree_hook.rs (3)
block_height(326-331)latest_checkpoint(253-277)tree(301-311)rust/main/chains/hyperlane-starknet/src/merkle_tree_hook.rs (3)
latest_checkpoint(67-91)tree(95-130)tree(107-111)rust/main/chains/hyperlane-sealevel/src/merkle_tree_hook.rs (2)
latest_checkpoint(31-41)tree(20-27)rust/main/hyperlane-core/src/traits/merkle_tree_hook.rs (2)
latest_checkpoint(49-50)tree(37-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: infra-test
- GitHub Check: build-and-push-to-gcr
- GitHub Check: lander-coverage
- GitHub Check: lint-rs
- GitHub Check: e2e-matrix (radix)
- GitHub Check: test-rs
- GitHub Check: e2e-matrix (cosmwasm)
- GitHub Check: e2e-matrix (sealevel)
- GitHub Check: e2e-matrix (cosmosnative)
- GitHub Check: e2e-matrix (evm)
- GitHub Check: e2e-matrix (starknet)
🔇 Additional comments (11)
rust/main/hyperlane-core/src/types/reorg.rs (1)
13-16: LGTM! Field renaming clarifies local vs. canonical perspective.Splitting
checkpoint_indexintolocal_checkpoint_indexandcanonical_checkpoint_indexmakes it clear which checkpoint index we're talking about. The comments match up nicely too.rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs (1)
210-221: Test updates look solid.The test now uses
local_checkpoint_indexandcanonical_checkpoint_index, matching the updated ReorgEvent structure. Variable names are clear and consistent.rust/main/agents/relayer/src/msg/pending_message.rs (1)
1206-1207: Mock methods properly extend the test harness.These additions align with the new
CheckpointInfostorage methods added to theHyperlaneDbtrait. Test-only, no impact on production.rust/main/hyperlane-base/src/db/mod.rs (1)
177-180: Trait extension looks good.The new
store_latest_checkpoint_infoandretrieve_latest_checkpoint_infomethods follow the existing DB trait patterns. Doc comments are clear and accurate.rust/main/hyperlane-core/src/types/checkpoint.rs (2)
122-135: Decode implementation follows the pattern correctly.The deserialization order (checkpoint_index first, then block_height) matches the serialization order in the
Encodeimplementation. Error propagation using?is appropriate.
111-120: Keep saturating_add for byte counting
The swamp’sEncodeimplementations consistently usesaturating_addto aggregate written bytes; switching tochecked_addwould diverge from project conventions.Likely an incorrect or invalid review comment.
rust/main/agents/validator/src/submit/tests.rs (2)
135-136: Mock DB methods properly implemented.These methods complete the test harness for the new
CheckpointInfostorage functionality.
225-236: Test assertions updated correctly.The assertions now check
canonical_checkpoint_indexagainst the on-chain merkle tree index (lines 225-228) andlocal_checkpoint_indexagainst the expected local tree (line 234). This aligns with the new ReorgEvent structure.rust/main/agents/validator/src/submit.rs (3)
121-125: Database initialization looks good.Loading the latest checkpoint info from the DB with a sensible default fallback is the right approach. This lets the validator resume from where it left off.
315-349: Helper method centralizes reorg handling nicely.The
panic_with_reorgmethod consolidates reorg event construction, logging, reporting, and checkpoint syncer updates in one place. Clean separation of concerns.
351-362: Reporting helper handles block height availability gracefully.Using block height when available, falling back to reorg period otherwise, is a pragmatic approach for chains that don't support block height queries.
…z/hyperlane-monorepo into jeff/validator-block-height
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
rust/main/agents/validator/src/submit.rs (1)
174-190: Missing persistence after checkpoint update.The checkpoint fields get updated in memory, but there's no call to persist them back to the DB. After a restart, you're back to comparing against whatever was there before—or defaults if nothing was stored. That means a reorg that happened while things were down won't ring any bells.
Add persistence right after updating the fields:
latest_seen_checkpoint.block_height = block_height; latest_seen_checkpoint.checkpoint_index = latest_checkpoint.index; + + if let Err(err) = self + .db + .store_latest_checkpoint_info(&latest_seen_checkpoint) + { + tracing::warn!( + ?err, + ?latest_seen_checkpoint, + "Failed to persist latest checkpoint info" + ); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
rust/main/agents/validator/src/submit.rs(6 hunks)rust/main/agents/validator/src/submit/tests.rs(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- rust/main/agents/validator/src/submit/tests.rs
🧰 Additional context used
📓 Path-based instructions (2)
rust/main/agents/validator/**/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain validator agent Rust sources under rust/main/agents/validator
Files:
rust/main/agents/validator/src/submit.rs
rust/main/**/src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Run cargo clippy for Rust code linting
Files:
rust/main/agents/validator/src/submit.rs
🧬 Code graph analysis (1)
rust/main/agents/validator/src/submit.rs (4)
rust/main/chains/hyperlane-starknet/src/merkle_tree_hook.rs (3)
tree(95-130)tree(107-111)latest_checkpoint(67-91)rust/main/chains/hyperlane-ethereum/src/contracts/merkle_tree_hook.rs (3)
tree(301-311)latest_checkpoint(253-277)block_height(326-331)rust/main/hyperlane-core/src/traits/merkle_tree_hook.rs (2)
tree(37-37)latest_checkpoint(49-50)rust/main/agents/validator/src/submit/tests.rs (1)
verify_checkpoint(587-660)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: e2e-matrix (radix)
- GitHub Check: e2e-matrix (starknet)
- GitHub Check: e2e-matrix (cosmosnative)
- GitHub Check: e2e-matrix (cosmwasm)
- GitHub Check: e2e-matrix (sealevel)
- GitHub Check: e2e-matrix (evm)
🔇 Additional comments (6)
rust/main/agents/validator/src/submit.rs (6)
19-20: Import changes look solid.The
CheckpointInfoaddition supports the new checkpoint tracking, and the reorg handling setup is clean.
122-126: Neat handling of the DB retrieval.The double
unwrap_or_defaultcovers both the retrieval error and the missing data cases cleanly. This'll keep things rolling even when the DB's empty or having a bad day.
137-138: Good call extracting the verification.Moving this logic into its own helper keeps the main loop from getting too swampy. Nice and tidy.
268-277: Centralizing the reorg handling makes sense.Moving this into
panic_with_reorgkeeps the error handling consistent and cuts down on repeated code. Much better than having it scattered around.
340-374: Solid consolidation of the reorg panic path.Bringing all the reorg handling into one place keeps things consistent. The error messages are clear about what went wrong, and you're handling the write_reorg_status failure gracefully by appending it to the panic message. Good stuff.
376-387: Clean separation for the reporting logic.Handling both block-height-based and reorg-period-based reporting in one helper keeps things flexible. The fallback with the log message is a nice touch for when block height isn't available.
Description
Related issues
Summary by CodeRabbit
New Features
Bug Fixes
Tests