feat: crash loop even in the event reorg_flag.json is unparsable#7464
feat: crash loop even in the event reorg_flag.json is unparsable#7464
reorg_flag.json is unparsable#7464Conversation
|
📝 WalkthroughWalkthroughThe PR replaces optional reorg returns with a structured Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller (validator / relayer)
participant Trait as CheckpointSyncer Trait
participant Storage as Storage Impl (GCS / S3 / Local)
rect rgb(245,250,240)
Note over Caller,Trait: reorg_status now returns ReorgEventResponse
Caller->>Trait: reorg_status()
Trait->>Storage: fetch reorg object (read)
alt object not found
Storage-->>Trait: ReorgEventResponse { exists: false, event: None, content: None }
else object found & parsed ok
Storage-->>Trait: ReorgEventResponse { exists: true, event: Some(event), content: Some(raw) }
else object found but parse error
Storage-->>Trait: ReorgEventResponse { exists: true, event: None, content: Some(raw) }
Note right of Trait: parse error is logged
end
Trait-->>Caller: Result<ReorgEventResponse>
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![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 (4)
rust/main/hyperlane-core/src/types/reorg.rs (1)
6-13: Solid new type for the reorg response.This
ReorgEventResponsewrapper nicely captures the "file exists but couldn't be parsed" scenario - which is exactly what the PR's tryin' to address. The separation betweenexistsand the optionaleventis thoughtful.One small thing to consider: the doc comment on line 11 says "Details on the actual reorg if parsable" - you might want to add a brief note explaining that
exists: truewithevent: Nonemeans the file was present but couldn't be understood. Just so future travelers in this swamp know what they're dealin' with.rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs (1)
136-143: This is the heart of the change - looks like it does what's advertised.The logic now properly handles the "file exists but couldn't be parsed" scenario by still triggering the crash loop with a default
ReorgEvent. That's exactly what the PR objective calls for - future compatibility when an older validator can't parse a newer format.A couple of things worth ponderin':
Using
ReorgEvent::default()means all fields will be zeroed out. Anyone investigatin' will see zeros forlocal_merkle_root,canonical_merkle_root,checkpoint_index, etc. Might be helpful to add a log line before line 142 explainin' that the reorg file existed but couldn't be parsed - gives folks a trail to follow.The test at the bottom of this file validates the happy path (parseable reorg event), but doesn't cover the "unparsable file" case. Consider addin' a test that writes garbage to the reorg file and verifies the system still returns a
CheckpointSyncerBuildError::ReorgEventwith the default values.Ok(event) => { if event.exists { if let Some(reorg_event) = event.event { return Err(CheckpointSyncerBuildError::ReorgEvent(reorg_event)); } // failed to parse the reorg event, so just use default + error!("Reorg flag file exists but could not be parsed. Treating as reorg event with default values."); return Err(CheckpointSyncerBuildError::ReorgEvent(ReorgEvent::default())); } }rust/main/hyperlane-base/src/types/local_storage.rs (1)
136-159: ReorgStatus semantics look good; consider tightening local read‑error handlingThe new
ReorgEventResponsebehavior (exists=true butevent: Noneon parse error) matches the other backends and should keep the agent “seeing” a reorg even ifreorg_flag.jsonis from a newer format. The only rough edge is that any read error (not justNotFound) is treated asexists: false, whereas S3/GCS only do that for missing objects and otherwise bubble the error; if you want the swamp to be consistent across backends, you might later special‑caseErrorKind::NotFoundhere and surface other I/O errors instead of masking them as “no flag”.rust/main/hyperlane-base/src/types/s3_storage.rs (1)
15-19: S3 reorg_status behavior matches the intended contract and other backendsThe S3 implementation cleanly distinguishes “no flag” (object missing) from “flag present but unparsable” (exists=true,
event: None), logging parse failures and otherwise reusing the existing anonymous read path; behavior is consistent with Local/GCS and should keep reorg signaling robust even when the JSON shape drifts.If this pattern spreads further, you might later factor the bytes→
ReorgEventResponseparsing into a small helper to avoid repeating the same match blocks across storage types.Also applies to: 296-322
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
rust/main/agents/validator/src/submit/tests.rs(2 hunks)rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs(2 hunks)rust/main/hyperlane-base/src/tests/mock_checkpoint_syncer.rs(3 hunks)rust/main/hyperlane-base/src/traits/checkpoint_syncer.rs(2 hunks)rust/main/hyperlane-base/src/types/gcs_storage.rs(2 hunks)rust/main/hyperlane-base/src/types/local_storage.rs(2 hunks)rust/main/hyperlane-base/src/types/s3_storage.rs(2 hunks)rust/main/hyperlane-core/src/accumulator/merkle.rs(1 hunks)rust/main/hyperlane-core/src/types/reorg.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
rust/main/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Gas price escalation in transaction management must follow the formula:
Max(Min(Max(Escalate(oldGasPrice), newEstimatedGasPrice), gasPriceCapMultiplier × newEstimatedGasPrice), oldGasPrice)to prevent indefinite escalation while maintaining competitiveness and RBF compatibility, with configurablegasPriceCapMultiplierper chain in transactionOverrides (default: 3)
Files:
rust/main/hyperlane-core/src/types/reorg.rsrust/main/hyperlane-base/src/traits/checkpoint_syncer.rsrust/main/hyperlane-core/src/accumulator/merkle.rsrust/main/hyperlane-base/src/types/gcs_storage.rsrust/main/hyperlane-base/src/settings/checkpoint_syncer.rsrust/main/agents/validator/src/submit/tests.rsrust/main/hyperlane-base/src/types/s3_storage.rsrust/main/hyperlane-base/src/tests/mock_checkpoint_syncer.rsrust/main/hyperlane-base/src/types/local_storage.rs
rust/main/agents/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Rust agents must implement three agent types: Relayer (indexes origin chains and delivers messages), Validator (signs checkpoints for message verification), and Scraper (indexes chain data for analytics)
Files:
rust/main/agents/validator/src/submit/tests.rs
🧠 Learnings (3)
📓 Common learnings
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/mainnet_config.json:965-965
Timestamp: 2025-08-26T13:46:37.695Z
Learning: In the repository hyperlane-xyz/hyperlane-monorepo, skip reviewing the file rust/main/config/testnet_config.json in future code reviews as requested by paulbalaji.
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/mainnet_config.json:965-965
Timestamp: 2025-08-26T13:46:37.695Z
Learning: In the repository hyperlane-xyz/hyperlane-monorepo, skip reviewing the file rust/main/config/mainnet_config.json in future code reviews as requested by paulbalaji.
📚 Learning: 2025-11-24T17:19:38.332Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:19:38.332Z
Learning: Applies to rust/main/agents/**/*.rs : Rust agents must implement three agent types: Relayer (indexes origin chains and delivers messages), Validator (signs checkpoints for message verification), and Scraper (indexes chain data for analytics)
Applied to files:
rust/main/agents/validator/src/submit/tests.rs
📚 Learning: 2025-11-24T17:19:38.332Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:19:38.332Z
Learning: Applies to rust/main/chains/**/*.rs : Rust chain implementations must support `hyperlane-ethereum` for EVM, `hyperlane-cosmos` for Cosmos ecosystem, `hyperlane-sealevel` for Solana/SVM, and `hyperlane-fuel` for Fuel VM
Applied to files:
rust/main/agents/validator/src/submit/tests.rs
🧬 Code graph analysis (5)
rust/main/hyperlane-base/src/traits/checkpoint_syncer.rs (4)
rust/main/hyperlane-base/src/tests/mock_checkpoint_syncer.rs (1)
reorg_status(133-140)rust/main/hyperlane-base/src/types/gcs_storage.rs (1)
reorg_status(286-312)rust/main/hyperlane-base/src/types/local_storage.rs (1)
reorg_status(136-160)rust/main/hyperlane-base/src/types/s3_storage.rs (1)
reorg_status(296-323)
rust/main/hyperlane-base/src/types/gcs_storage.rs (3)
rust/main/hyperlane-base/src/traits/checkpoint_syncer.rs (1)
reorg_status(54-54)rust/main/hyperlane-base/src/types/local_storage.rs (1)
reorg_status(136-160)rust/main/hyperlane-base/src/types/s3_storage.rs (1)
reorg_status(296-323)
rust/main/agents/validator/src/submit/tests.rs (5)
rust/main/hyperlane-base/src/tests/mock_checkpoint_syncer.rs (1)
reorg_status(133-140)rust/main/hyperlane-base/src/traits/checkpoint_syncer.rs (1)
reorg_status(54-54)rust/main/hyperlane-base/src/types/gcs_storage.rs (1)
reorg_status(286-312)rust/main/hyperlane-base/src/types/local_storage.rs (1)
reorg_status(136-160)rust/main/hyperlane-base/src/types/s3_storage.rs (1)
reorg_status(296-323)
rust/main/hyperlane-base/src/types/s3_storage.rs (4)
rust/main/hyperlane-base/src/tests/mock_checkpoint_syncer.rs (1)
reorg_status(133-140)rust/main/hyperlane-base/src/traits/checkpoint_syncer.rs (1)
reorg_status(54-54)rust/main/hyperlane-base/src/types/gcs_storage.rs (1)
reorg_status(286-312)rust/main/hyperlane-base/src/types/local_storage.rs (1)
reorg_status(136-160)
rust/main/hyperlane-base/src/types/local_storage.rs (4)
rust/main/hyperlane-base/src/tests/mock_checkpoint_syncer.rs (1)
reorg_status(133-140)rust/main/hyperlane-base/src/traits/checkpoint_syncer.rs (1)
reorg_status(54-54)rust/main/hyperlane-base/src/types/gcs_storage.rs (1)
reorg_status(286-312)rust/main/hyperlane-base/src/types/s3_storage.rs (1)
reorg_status(296-323)
🔇 Additional comments (8)
rust/main/hyperlane-core/src/accumulator/merkle.rs (1)
234-235: Looks fine, just a wee bit of formatting.This blank line doesn't change any behavior - just adds a touch of breathing room before the hash recomputation. No concerns from me swamp.
rust/main/agents/validator/src/submit/tests.rs (2)
11-12: Import update looks proper.Bringing in
ReorgEventResponsealongsideReorgEventmakes sense now that the API's been updated. Nothin' fancy, just what's needed.
63-63: Mock trait signature aligns with the real thing.This matches the updated
CheckpointSyncertrait signature. The mock's ready to return the newReorgEventResponsetype when tests need it.rust/main/hyperlane-base/src/traits/checkpoint_syncer.rs (2)
6-8: Import looks good.Bringin' in
ReorgEventResponseright where it's needed. Clean and tidy.
54-54: Trait signature update is consistent with the implementations.From what I can see in the relevant snippets, all the storage backends (Local, S3, GCS) and the mock have been updated to return
ReorgEventResponse. The API change flows through proper-like.rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs (1)
1-15: Import cleanup looks tidy.Just movin' things around in the imports section. Nothin' to fuss about here.
rust/main/hyperlane-base/src/tests/mock_checkpoint_syncer.rs (1)
39-40: Mocks align cleanly with the new ReorgEventResponse APIThe updated response list type and
reorg_statusimplementation stay in lockstep with the trait and real storage backends, keeping the test harness simple and predictable. From this side of the swamp, no issues here.Also applies to: 133-140
rust/main/hyperlane-base/src/types/gcs_storage.rs (1)
5-7: GCS reorg_status is consistent and robust against unparsable flagsThis version cleanly maps 404s to
exists: false, keeps other storage errors bubbling viabail!, and treats JSON decode failures asexists: truewith a logged error andevent: None, same as the S3 and local implementations. The whole swamp of backends now behaves consistently under the newReorgEventResponsecontract.Also applies to: 286-311
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs (1)
202-249: Test update looks right, but coverage is still incomplete.The variant name change to
ReorgFlagis applied correctly. However, as mentioned in previous reviews, additional test cases are needed:
- Test for unparsable
reorg_flag.json(the main PR objective)- Test for
exists=truewithevent=NonescenarioThese would verify the new
ReorgEventResponsebehavior properly.Based on previous review feedback from ameten.
🧹 Nitpick comments (1)
rust/main/hyperlane-base/src/types/local_storage.rs (1)
136-162: This works for the swamp, though there's one wee thing to consider.The implementation is consistent with S3 and GCS storage in structure. However, I notice a subtle difference: when
tokio::fs::readfails (lines 139-146), it returnsexists: falseregardless of the error type. This treats a missing file the same as a permission error or other I/O failure.In the S3 implementation,
exists: falseis only returned when the file genuinely doesn't exist (thecheck_metadatareturnsOk(false)for missing objects).For LocalStorage, you might want to distinguish
NotFoundfrom other errors to be more precise:let data = match tokio::fs::read(self.reorg_flag_path()).await { Ok(s) => s, Err(err) => { - error!(?err, "Failed to read file"); - return Ok(ReorgEventResponse { - exists: false, - event: None, - contents: None, - }); + if err.kind() == std::io::ErrorKind::NotFound { + return Ok(ReorgEventResponse { + exists: false, + event: None, + contents: None, + }); + } + error!(?err, "Failed to read file"); + return Ok(ReorgEventResponse { + exists: true, + event: None, + contents: None, + }); } };That said, the current approach is pragmatic - if we can't read it, we can't act on it either way.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
rust/main/agents/relayer/src/msg/metadata/base_builder.rs(2 hunks)rust/main/agents/relayer/src/msg/metadata/multisig/base.rs(1 hunks)rust/main/agents/validator/src/validator.rs(1 hunks)rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs(4 hunks)rust/main/hyperlane-base/src/types/gcs_storage.rs(2 hunks)rust/main/hyperlane-base/src/types/local_storage.rs(2 hunks)rust/main/hyperlane-base/src/types/s3_storage.rs(2 hunks)rust/main/hyperlane-core/src/types/reorg.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- rust/main/hyperlane-core/src/types/reorg.rs
🧰 Additional context used
📓 Path-based instructions (2)
rust/main/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Gas price escalation in transaction management must follow the formula:
Max(Min(Max(Escalate(oldGasPrice), newEstimatedGasPrice), gasPriceCapMultiplier × newEstimatedGasPrice), oldGasPrice)to prevent indefinite escalation while maintaining competitiveness and RBF compatibility, with configurablegasPriceCapMultiplierper chain in transactionOverrides (default: 3)
Files:
rust/main/hyperlane-base/src/types/s3_storage.rsrust/main/agents/validator/src/validator.rsrust/main/hyperlane-base/src/types/gcs_storage.rsrust/main/hyperlane-base/src/settings/checkpoint_syncer.rsrust/main/agents/relayer/src/msg/metadata/base_builder.rsrust/main/hyperlane-base/src/types/local_storage.rsrust/main/agents/relayer/src/msg/metadata/multisig/base.rs
rust/main/agents/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Rust agents must implement three agent types: Relayer (indexes origin chains and delivers messages), Validator (signs checkpoints for message verification), and Scraper (indexes chain data for analytics)
Files:
rust/main/agents/validator/src/validator.rsrust/main/agents/relayer/src/msg/metadata/base_builder.rsrust/main/agents/relayer/src/msg/metadata/multisig/base.rs
🧠 Learnings (4)
📓 Common learnings
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/mainnet_config.json:965-965
Timestamp: 2025-08-26T13:46:37.695Z
Learning: In the repository hyperlane-xyz/hyperlane-monorepo, skip reviewing the file rust/main/config/testnet_config.json in future code reviews as requested by paulbalaji.
📚 Learning: 2025-11-20T12:20:42.691Z
Learnt from: yjamin
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7414
File: rust/main/chains/hyperlane-aleo/src/provider/traits.rs:252-0
Timestamp: 2025-11-20T12:20:42.691Z
Learning: In rust/main/chains/hyperlane-aleo/src/provider/traits.rs, the `get_state_paths_for_commitments` and `get_state_paths_for_commitments_async` methods intentionally use `.unwrap_or_default()` to return an empty Vec on any error. This behavior matches SnarkVM's internal QueryTrait implementation and should not be changed to propagate errors.
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-11-24T17:19:38.332Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:19:38.332Z
Learning: Applies to rust/main/chains/**/*.rs : Rust chain implementations must support `hyperlane-ethereum` for EVM, `hyperlane-cosmos` for Cosmos ecosystem, `hyperlane-sealevel` for Solana/SVM, and `hyperlane-fuel` for Fuel VM
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-11-24T17:19:38.332Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:19:38.332Z
Learning: Applies to rust/main/agents/**/*.rs : Rust agents must implement three agent types: Relayer (indexes origin chains and delivers messages), Validator (signs checkpoints for message verification), and Scraper (indexes chain data for analytics)
Applied to files:
rust/main/agents/relayer/src/msg/metadata/base_builder.rsrust/main/agents/relayer/src/msg/metadata/multisig/base.rs
🧬 Code graph analysis (1)
rust/main/hyperlane-base/src/types/s3_storage.rs (4)
rust/main/hyperlane-base/src/types/gcs_storage.rs (1)
reorg_status(286-315)rust/main/hyperlane-base/src/types/local_storage.rs (1)
reorg_status(136-163)rust/main/hyperlane-base/src/tests/mock_checkpoint_syncer.rs (1)
reorg_status(133-140)rust/main/hyperlane-base/src/traits/checkpoint_syncer.rs (1)
reorg_status(54-54)
⏰ 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). (61)
- GitHub Check: cli-evm-e2e-matrix (warp-init)
- GitHub Check: cli-evm-e2e-matrix (warp-check-3)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-basic)
- GitHub Check: cli-evm-e2e-matrix (warp-read)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
- GitHub Check: cli-evm-e2e-matrix (warp-send)
- GitHub Check: cli-evm-e2e-matrix (warp-check-4)
- GitHub Check: cli-evm-e2e-matrix (warp-check-2)
- GitHub Check: cli-evm-e2e-matrix (warp-rebalancer)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-2)
- GitHub Check: cli-evm-e2e-matrix (warp-check-5)
- GitHub Check: cli-evm-e2e-matrix (warp-extend-config)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-2)
- GitHub Check: cli-evm-e2e-matrix (warp-deploy-1)
- 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-read)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-2)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-ism-updates)
- GitHub Check: cli-evm-e2e-matrix (core-apply)
- GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
- GitHub Check: cli-evm-e2e-matrix (warp-apply-1)
- GitHub Check: cli-evm-e2e-matrix (core-check)
- GitHub Check: cli-evm-e2e-matrix (core-init)
- GitHub Check: cli-evm-e2e-matrix (relay)
- GitHub Check: cosmos-sdk-e2e-run
- GitHub Check: cli-evm-e2e-matrix (core-deploy)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
- GitHub Check: env-test-matrix (testnet4, sepolia, core)
- GitHub Check: env-test-matrix (mainnet3, inevm, igp)
- GitHub Check: env-test-matrix (mainnet3, inevm, core)
- GitHub Check: env-test-matrix (mainnet3, ethereum, core)
- GitHub Check: env-test-matrix (mainnet3, optimism, igp)
- GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
- GitHub Check: env-test-matrix (mainnet3, optimism, core)
- GitHub Check: env-test-matrix (mainnet3, ethereum, igp)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
- GitHub Check: cli-radix-e2e-matrix (core-deploy)
- GitHub Check: cli-cross-chain-e2e-matrix (warp-deploy)
- GitHub Check: cli-cosmos-e2e-matrix (core-apply)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-route-extension)
- GitHub Check: cli-radix-e2e-matrix (warp-apply-ownership-updates)
- GitHub Check: cli-radix-e2e-matrix (warp-deploy)
- 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 (warp-read)
- GitHub Check: cli-cosmos-e2e-matrix (core-check)
- GitHub Check: cli-install-test-run
- GitHub Check: coverage-run
- GitHub Check: infra-test
- GitHub Check: build-and-push-to-gcr
- GitHub Check: lint-rs
- GitHub Check: lander-coverage
- GitHub Check: test-rs
- GitHub Check: e2e-matrix (sealevel)
- GitHub Check: e2e-matrix (radix)
- GitHub Check: e2e-matrix (starknet)
- GitHub Check: e2e-matrix (evm)
- GitHub Check: e2e-matrix (cosmosnative)
- GitHub Check: e2e-matrix (cosmwasm)
🔇 Additional comments (10)
rust/main/agents/relayer/src/msg/metadata/multisig/base.rs (1)
240-244: Looks good, this variant rename aligns nicely with the rest of the swamp.The change from
ReorgEventtoReorgFlagis consistent with the upstream rename inCheckpointSyncerBuildError. The error handling flow remains intact - when a reorg flag is detected, metadata building is properly refused with a descriptive message.rust/main/agents/validator/src/validator.rs (1)
558-564: Aye, this bit's sorted proper.The pattern match update from
ReorgEventtoReorgFlagis consistent with the variant rename. The reorg reporting logic remains sound - when a checkpoint syncer fails to build due to a reorg flag, the validator properly reports it using the storedreorg_period.rust/main/agents/relayer/src/msg/metadata/base_builder.rs (1)
320-333: This here's exactly what we need for the swamp.The variant rename from
ReorgEventtoReorgFlagis consistent with the codebase changes. The branching logic is solid:
- When
ignore_reorg_reportsis enabled, it logs a warning with full context (event, config, validator) and gracefully continues- Otherwise, it properly short-circuits with the error, preventing checkpoint syncer builds when a reorg has been detected
The added context in the warning log will be handy for debugging why a particular syncer was skipped.
rust/main/hyperlane-base/src/types/s3_storage.rs (1)
296-326: Well done, this handles the layers of the onion nicely.The new
ReorgEventResponsereturn type properly captures three states:
- No reorg file exists →
exists: false- File exists and parses →
exists: truewith the parsed event- File exists but is unparsable →
exists: truewithevent: Nonebut raw contents preservedThis achieves the PR objective - the system will now detect a reorg even when the file format is unrecognized (future compatibility). The error logging on parse failure and the inclusion of raw
contentswill help with debugging.The implementation is consistent with
LocalStorageandGcsStorageClientas shown in the relevant snippets.rust/main/hyperlane-base/src/types/local_storage.rs (1)
5-9: Imports look tidy.The new imports for
ReorgEventResponseandtracing::errorare properly organized and necessary for the updatedreorg_statusimplementation.rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs (2)
53-53: Good rename fromReorgEventtoReorgFlag.This makes it clearer that it's a signal rather than the event itself. Lines up well with what was suggested before.
136-141: The review comment is incorrect —ReorgEventdoes implementDefault.The code on line 139 will compile without issues.
ReorgEventis defined atrust/main/hyperlane-core/src/types/reorg.rs:19and explicitly derivesDefault:#[derive(Debug, Clone, Serialize, Deserialize, new, PartialEq, Default)] pub struct ReorgEvent { pub local_merkle_root: H256, pub canonical_merkle_root: H256, pub checkpoint_index: u32, pub unix_timestamp: u64, pub reorg_period: ReorgPeriod, }The
unwrap_or_default()call will work correctly becauseReorgEventderivesDefaulton line 18.Likely an incorrect or invalid review comment.
rust/main/hyperlane-base/src/types/gcs_storage.rs (3)
5-7: Import addition looks good.The
ReorgEventResponseimport is properly added to support the new return type.
286-286: API change aligns with PR objectives.The return type change from
Result<Option<ReorgEvent>>toResult<ReorgEventResponse>is the intended breaking change to support unparsable reorg flags.
287-314: Well-implemented reorg detection logic.The implementation properly handles all cases:
- NOT_FOUND returns
exists: false- Successful parse returns
exists: truewith parsed event- Parse failure logs the error and returns
exists: truewithevent: None, achieving the PR objective of detecting reorgs even when the file format is unrecognizedThe use of
String::from_utf8_lossyfor thecontentsfield is appropriate for preserving raw data even if it's not valid UTF-8.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs (1)
1-1: Unused import:PathBufin the module root.The
PathBufimport on line 1 doesn't appear to be used anywhere in this file. It's used in theCheckpointSyncerConfstruct fields (line 23, 94) but those are already covered by the type definition context. Unless you're planning to use it soon, might want to let it go.-use std::{env, path::PathBuf}; +use std::env;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
rust/main/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Gas price escalation in transaction management must follow the formula:
Max(Min(Max(Escalate(oldGasPrice), newEstimatedGasPrice), gasPriceCapMultiplier × newEstimatedGasPrice), oldGasPrice)to prevent indefinite escalation while maintaining competitiveness and RBF compatibility, with configurablegasPriceCapMultiplierper chain in transactionOverrides (default: 3)
Files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
🧠 Learnings (7)
📚 Learning: 2025-11-20T12:20:42.691Z
Learnt from: yjamin
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7414
File: rust/main/chains/hyperlane-aleo/src/provider/traits.rs:252-0
Timestamp: 2025-11-20T12:20:42.691Z
Learning: In rust/main/chains/hyperlane-aleo/src/provider/traits.rs, the `get_state_paths_for_commitments` and `get_state_paths_for_commitments_async` methods intentionally use `.unwrap_or_default()` to return an empty Vec on any error. This behavior matches SnarkVM's internal QueryTrait implementation and should not be changed to propagate errors.
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-11-24T17:19:38.362Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:19:38.362Z
Learning: Applies to rust/main/chains/**/*.rs : Rust chain implementations must support `hyperlane-ethereum` for EVM, `hyperlane-cosmos` for Cosmos ecosystem, `hyperlane-sealevel` for Solana/SVM, and `hyperlane-fuel` for Fuel VM
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-11-24T17:19:38.362Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:19:38.362Z
Learning: Applies to rust/main/agents/**/*.rs : Rust agents must implement three agent types: Relayer (indexes origin chains and delivers messages), Validator (signs checkpoints for message verification), and Scraper (indexes chain data for analytics)
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-08-26T13:46:37.695Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/mainnet_config.json:965-965
Timestamp: 2025-08-26T13:46:37.695Z
Learning: In the repository hyperlane-xyz/hyperlane-monorepo, skip reviewing the file rust/main/config/testnet_config.json in future code reviews as requested by paulbalaji.
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-08-26T13:46:37.695Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/mainnet_config.json:965-965
Timestamp: 2025-08-26T13:46:37.695Z
Learning: In the repository hyperlane-xyz/hyperlane-monorepo, skip reviewing the file rust/main/config/mainnet_config.json in future code reviews as requested by paulbalaji.
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-08-26T13:45:52.227Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/testnet_config.json:34-35
Timestamp: 2025-08-26T13:45:52.227Z
Learning: Skip reviewing mainnet_config.json and testnet_config.json configuration files in typescript/infra/config/ and rust/main/config/ directories as requested by paulbalaji to reduce review noise.
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-08-26T13:45:44.656Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/testnet_config.json:3380-3439
Timestamp: 2025-08-26T13:45:44.656Z
Learning: Skip reviewing mainnet_config.json and testnet_config.json files in rust/main/config/ directory as requested by paulbalaji.
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
🧬 Code graph analysis (1)
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs (1)
rust/main/hyperlane-base/src/types/local_storage.rs (1)
reorg_flag_path(44-46)
⏰ 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: e2e-matrix (sealevel)
- GitHub Check: e2e-matrix (cosmosnative)
- GitHub Check: e2e-matrix (starknet)
- GitHub Check: e2e-matrix (radix)
- GitHub Check: e2e-matrix (evm)
- GitHub Check: e2e-matrix (cosmwasm)
- GitHub Check: lander-coverage
- GitHub Check: lint-rs
- GitHub Check: test-rs
🔇 Additional comments (3)
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs (3)
53-53: Good rename fromReorgEventtoReorgFlag.This addresses the previous feedback about naming and makes it clearer that this is a flag/status rather than conflating with the event data type itself. The verbose error message is appropriate given the severity—better safe than sorry when it comes to validator integrity.
136-139: Clean handling of the new response structure.The conditional check on
event.existsis straightforward and handles both parsable and unparsable reorg flags correctly by propagating the entireReorgEventResponse. Works well.
259-292: Excellent test coverage for the unparsable JSON case.This test properly validates the core PR requirement—handling forward-incompatible
reorg_flag.jsonformats. The file creation in the scoped block (lines 269-274) works correctly since the file handle is dropped and flushed before the assertion. Good work addressing the previous feedback about test coverage.
Co-authored-by: Danil Nemirovsky <ameten@users.noreply.github.com>
…monorepo into jeff/reorg-flag
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs (1)
284-289: Inconsistent assertion message - should say "response" not "event".Line 252 says "Reported reorg response doesn't match" but here it says "event". Keep 'em consistent, would ya?
Err(CheckpointSyncerBuildError::ReorgFlag(e)) => { assert_eq!( e, dummy_reorg_response, - "Reported reorg event doesn't match" + "Reported reorg response doesn't match" ); }rust/main/agents/validator/src/validator.rs (1)
558-575: ReorgFlag handling looks correct; consider usingexists/contentsto sharpen the log and contextThis flow does what you’re after: any
ReorgFlagerror now drivesreport_with_reorg_period, falling back toReorgPeriod::Nonewhen there’s no parsedReorgEvent, so we’ll still get the “reorg happened, but we don’t know the period” behavior instead of silently skipping it.Couple of small things you might want to tidy up while you’re in this swamp:
- The
Nonearm assumesevent == Nonealways means “failed to parse reorg event”, but the type carries anexistsflag. If upstream ever returnsReorgFlagwithexists == false, this message becomes a bit misleading. You could gate the message onreorg_resp.existsor tweak the wording to something more generic like “Missing or unparsable reorg event…”.- Since
ReorgEventResponseexposescontents, adding a bit of extra context to the log (e.g.exists, maybecontents.is_some()or a truncated preview) would make future debugging much easier without changing the reporting behavior.Something along these lines keeps behavior the same but makes the story clearer:
if let Err(CheckpointSyncerBuildError::ReorgFlag(reorg_resp)) = checkpoint_syncer_result.as_ref() { match reorg_resp.event.as_ref() { Some(reorg_event) => { reorg_reporter .report_with_reorg_period(&reorg_event.reorg_period) .await; } None => { tracing::error!( exists = reorg_resp.exists, has_contents = reorg_resp.contents.is_some(), "Failed to obtain structured reorg event, reporting with default reorg period", ); reorg_reporter .report_with_reorg_period(&ReorgPeriod::None) .await; } } }Non-blocking, but it’ll save some head‑scratching later when folks are wading back through logs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
rust/main/agents/validator/src/validator.rs(1 hunks)rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
rust/main/agents/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Rust agents must implement three agent types: Relayer (indexes origin chains and delivers messages), Validator (signs checkpoints for message verification), and Scraper (indexes chain data for analytics)
Files:
rust/main/agents/validator/src/validator.rs
rust/main/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Gas price escalation in transaction management must follow the formula:
Max(Min(Max(Escalate(oldGasPrice), newEstimatedGasPrice), gasPriceCapMultiplier × newEstimatedGasPrice), oldGasPrice)to prevent indefinite escalation while maintaining competitiveness and RBF compatibility, with configurablegasPriceCapMultiplierper chain in transactionOverrides (default: 3)
Files:
rust/main/agents/validator/src/validator.rsrust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
🧠 Learnings (7)
📚 Learning: 2025-11-24T17:19:38.362Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:19:38.362Z
Learning: Applies to rust/main/agents/**/*.rs : Rust agents must implement three agent types: Relayer (indexes origin chains and delivers messages), Validator (signs checkpoints for message verification), and Scraper (indexes chain data for analytics)
Applied to files:
rust/main/agents/validator/src/validator.rsrust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-11-20T12:20:42.691Z
Learnt from: yjamin
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7414
File: rust/main/chains/hyperlane-aleo/src/provider/traits.rs:252-0
Timestamp: 2025-11-20T12:20:42.691Z
Learning: In rust/main/chains/hyperlane-aleo/src/provider/traits.rs, the `get_state_paths_for_commitments` and `get_state_paths_for_commitments_async` methods intentionally use `.unwrap_or_default()` to return an empty Vec on any error. This behavior matches SnarkVM's internal QueryTrait implementation and should not be changed to propagate errors.
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-11-24T17:19:38.362Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:19:38.362Z
Learning: Applies to rust/main/chains/**/*.rs : Rust chain implementations must support `hyperlane-ethereum` for EVM, `hyperlane-cosmos` for Cosmos ecosystem, `hyperlane-sealevel` for Solana/SVM, and `hyperlane-fuel` for Fuel VM
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-08-26T13:46:37.695Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/mainnet_config.json:965-965
Timestamp: 2025-08-26T13:46:37.695Z
Learning: In the repository hyperlane-xyz/hyperlane-monorepo, skip reviewing the file rust/main/config/testnet_config.json in future code reviews as requested by paulbalaji.
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-08-26T13:46:37.695Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/mainnet_config.json:965-965
Timestamp: 2025-08-26T13:46:37.695Z
Learning: In the repository hyperlane-xyz/hyperlane-monorepo, skip reviewing the file rust/main/config/mainnet_config.json in future code reviews as requested by paulbalaji.
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-08-26T13:45:52.227Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/testnet_config.json:34-35
Timestamp: 2025-08-26T13:45:52.227Z
Learning: Skip reviewing mainnet_config.json and testnet_config.json configuration files in typescript/infra/config/ and rust/main/config/ directories as requested by paulbalaji to reduce review noise.
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-08-26T13:45:44.656Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/testnet_config.json:3380-3439
Timestamp: 2025-08-26T13:45:44.656Z
Learning: Skip reviewing mainnet_config.json and testnet_config.json files in rust/main/config/ directory as requested by paulbalaji.
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
🧬 Code graph analysis (1)
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs (1)
rust/main/hyperlane-base/src/types/local_storage.rs (1)
reorg_flag_path(44-46)
⏰ 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)
- GitHub Check: lint-rs
- GitHub Check: lander-coverage
- GitHub Check: test-rs
- GitHub Check: e2e-matrix (cosmwasm)
- GitHub Check: e2e-matrix (sealevel)
- GitHub Check: e2e-matrix (evm)
- GitHub Check: e2e-matrix (starknet)
- GitHub Check: e2e-matrix (radix)
- GitHub Check: e2e-matrix (cosmosnative)
- GitHub Check: build-and-push-to-gcr
🔇 Additional comments (5)
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs (5)
1-16: LGTM! These imports are lookin' proper now, donkey.The switch to
ReorgEventResponseand the necessarystd::{env, path::PathBuf}additions are all in order. The import is indeed needed here sinceReorgEventResponseis used directly in the error enum variant.
48-60: The rename fromReorgEventtoReorgFlagis a solid move.This makes it clearer we're dealin' with a flag/status rather than just an event. Wrappin' the full
ReorgEventResponsegives callers access toexists,event, andcontents- whether we could parse the file or not. Good layers to this onion.
135-149: The reorg check logic looks right proper.Checkin'
event.existsinstead of pattern matching onSomehandles both parseable and unparseable reorg flags - which is exactly what we want for forward compatibility. If that file exists but we can't make heads or tails of it, we still crash loop. Smart.
240-257: Test for valid reorg_flag.json looks good.Constructin' the expected
ReorgEventResponsewith all three fields populated and comparin' it against what we get back - that's the right approach. Covers the happy path where JSON parses correctly.
259-292: Nice addition - testin' the unparseable JSON case.This is exactly the scenario the PR is meant to handle. When
reorg_flag.jsoncontains garbage we can't parse, we still detect it exists and return the raw contents. The test validatesexists: true,event: None, andcontents: Some("abc")- covers the forward-incompatibility case nicely.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
rust/main/hyperlane-base/src/types/local_storage.rs (1)
136-162: Inconsistent error handling compared to S3/GCS implementations.Now listen here, this works and all, but there's a wee difference in how errors are handled across the storage swamp. When reading the reorg flag file:
- LocalStorage (here): Treats all read errors as "file doesn't exist" (
exists: false)- S3Storage (s3_storage.rs:297-299): Propagates read errors with
?- GcsStorageClient (gcs_storage.rs:287-298): Propagates non-NOT_FOUND errors with
bail!If there's a permission error or disk issue on local storage, this will silently report no reorg exists, which could mask a real problem. Consider checking for
std::io::ErrorKind::NotFoundspecifically and propagating other errors, similar to the GCS approach.async fn reorg_status(&self) -> Result<ReorgEventResponse> { let data = match tokio::fs::read(self.reorg_flag_path()).await { Ok(s) => s, Err(err) => { - error!(?err, "Failed to read file"); - return Ok(ReorgEventResponse { - exists: false, - event: None, - content: None, - }); + if err.kind() == std::io::ErrorKind::NotFound { + return Ok(ReorgEventResponse { + exists: false, + event: None, + content: None, + }); + } + return Err(err.into()); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs(5 hunks)rust/main/hyperlane-base/src/types/gcs_storage.rs(2 hunks)rust/main/hyperlane-base/src/types/local_storage.rs(2 hunks)rust/main/hyperlane-base/src/types/s3_storage.rs(2 hunks)rust/main/hyperlane-core/src/types/reorg.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
rust/main/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Gas price escalation in transaction management must follow the formula:
Max(Min(Max(Escalate(oldGasPrice), newEstimatedGasPrice), gasPriceCapMultiplier × newEstimatedGasPrice), oldGasPrice)to prevent indefinite escalation while maintaining competitiveness and RBF compatibility, with configurablegasPriceCapMultiplierper chain in transactionOverrides (default: 3)
Files:
rust/main/hyperlane-core/src/types/reorg.rsrust/main/hyperlane-base/src/settings/checkpoint_syncer.rsrust/main/hyperlane-base/src/types/local_storage.rsrust/main/hyperlane-base/src/types/s3_storage.rsrust/main/hyperlane-base/src/types/gcs_storage.rs
🧠 Learnings (7)
📚 Learning: 2025-11-24T17:19:38.362Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:19:38.362Z
Learning: Applies to rust/main/agents/**/*.rs : Rust agents must implement three agent types: Relayer (indexes origin chains and delivers messages), Validator (signs checkpoints for message verification), and Scraper (indexes chain data for analytics)
Applied to files:
rust/main/hyperlane-core/src/types/reorg.rsrust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-11-20T12:20:42.691Z
Learnt from: yjamin
Repo: hyperlane-xyz/hyperlane-monorepo PR: 7414
File: rust/main/chains/hyperlane-aleo/src/provider/traits.rs:252-0
Timestamp: 2025-11-20T12:20:42.691Z
Learning: In rust/main/chains/hyperlane-aleo/src/provider/traits.rs, the `get_state_paths_for_commitments` and `get_state_paths_for_commitments_async` methods intentionally use `.unwrap_or_default()` to return an empty Vec on any error. This behavior matches SnarkVM's internal QueryTrait implementation and should not be changed to propagate errors.
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-11-24T17:19:38.362Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:19:38.362Z
Learning: Applies to rust/main/chains/**/*.rs : Rust chain implementations must support `hyperlane-ethereum` for EVM, `hyperlane-cosmos` for Cosmos ecosystem, `hyperlane-sealevel` for Solana/SVM, and `hyperlane-fuel` for Fuel VM
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-08-26T13:46:37.695Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/mainnet_config.json:965-965
Timestamp: 2025-08-26T13:46:37.695Z
Learning: In the repository hyperlane-xyz/hyperlane-monorepo, skip reviewing the file rust/main/config/testnet_config.json in future code reviews as requested by paulbalaji.
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-08-26T13:46:37.695Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/mainnet_config.json:965-965
Timestamp: 2025-08-26T13:46:37.695Z
Learning: In the repository hyperlane-xyz/hyperlane-monorepo, skip reviewing the file rust/main/config/mainnet_config.json in future code reviews as requested by paulbalaji.
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-08-26T13:45:52.227Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/testnet_config.json:34-35
Timestamp: 2025-08-26T13:45:52.227Z
Learning: Skip reviewing mainnet_config.json and testnet_config.json configuration files in typescript/infra/config/ and rust/main/config/ directories as requested by paulbalaji to reduce review noise.
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
📚 Learning: 2025-08-26T13:45:44.656Z
Learnt from: paulbalaji
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6943
File: rust/main/config/testnet_config.json:3380-3439
Timestamp: 2025-08-26T13:45:44.656Z
Learning: Skip reviewing mainnet_config.json and testnet_config.json files in rust/main/config/ directory as requested by paulbalaji.
Applied to files:
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs
🧬 Code graph analysis (4)
rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs (1)
rust/main/hyperlane-base/src/types/local_storage.rs (1)
reorg_flag_path(44-46)
rust/main/hyperlane-base/src/types/local_storage.rs (3)
rust/main/hyperlane-base/src/types/gcs_storage.rs (1)
reorg_status(286-315)rust/main/hyperlane-base/src/types/s3_storage.rs (1)
reorg_status(296-326)rust/main/hyperlane-base/src/traits/checkpoint_syncer.rs (1)
reorg_status(54-54)
rust/main/hyperlane-base/src/types/s3_storage.rs (3)
rust/main/hyperlane-base/src/types/gcs_storage.rs (1)
reorg_status(286-315)rust/main/hyperlane-base/src/types/local_storage.rs (1)
reorg_status(136-163)rust/main/hyperlane-base/src/traits/checkpoint_syncer.rs (1)
reorg_status(54-54)
rust/main/hyperlane-base/src/types/gcs_storage.rs (3)
rust/main/hyperlane-base/src/types/local_storage.rs (1)
reorg_status(136-163)rust/main/hyperlane-base/src/types/s3_storage.rs (1)
reorg_status(296-326)rust/main/hyperlane-base/src/traits/checkpoint_syncer.rs (1)
reorg_status(54-54)
⏰ 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: build-and-push-to-gcr
- GitHub Check: e2e-matrix (cosmosnative)
- GitHub Check: e2e-matrix (starknet)
- GitHub Check: e2e-matrix (evm)
- GitHub Check: e2e-matrix (radix)
- GitHub Check: e2e-matrix (cosmwasm)
- GitHub Check: e2e-matrix (sealevel)
- GitHub Check: yarn-install
- GitHub Check: lint-rs
- GitHub Check: lander-coverage
- GitHub Check: test-rs
🔇 Additional comments (5)
rust/main/hyperlane-core/src/types/reorg.rs (1)
6-15: Looks good to me, this does the job nicely.The new
ReorgEventResponsestruct is well-designed for its purpose. It cleanly separates existence (exists), parsed data (event), and raw content (content) - which lets the system crash loop even when the JSON can't be parsed. The derives are minimal and appropriate. Past review feedback about renamingcontentstocontentand trimming unnecessary derives has been addressed.rust/main/hyperlane-base/src/types/s3_storage.rs (1)
296-326: Implementation looks proper and consistent.This handles all the layers of the onion nicely - file not found returns
exists: false, parse failures still flagexists: truewith the raw content preserved, and successful parses give you the full picture. The error propagation via?on the bucket read is the right call here. Matches up well with the GCS implementation.rust/main/hyperlane-base/src/settings/checkpoint_syncer.rs (2)
136-140: Clean and simple check for reorg existence.This is exactly what we need in our swamp - a simple existence check that works whether or not the JSON could be parsed. The whole
ReorgEventResponsegets passed along in the error, so callers have access to whatever info is available.
259-292: Good test coverage for the unparsable JSON case.This test does exactly what the PR set out to do - verifies that even when
reorg_flag.jsoncontains garbage that can't be parsed, the system still reports a reorg exists and includes the raw content. That's the whole point of this change, and it's properly covered now.rust/main/hyperlane-base/src/types/gcs_storage.rs (1)
286-314: Solid implementation, consistent with the other storage backends.The error handling here is proper - NOT_FOUND gets treated as no reorg, other errors bubble up, and parse failures preserve the raw content. This matches the S3 implementation nicely. Using
String::from_utf8_lossyis a good defensive choice in case the file content isn't valid UTF-8.
🦀 Rust Agent Docker Image Built SuccessfullyImage Tags: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7464 +/- ##
============================
============================
🚀 New features to boost your workflow:
|
3 participants
Description
reorg_flag.jsonand a partner is running an older version that doesn't understand the new formatInstead of returning a
ReorgEvent, returnRelated issues
Summary by CodeRabbit
New Features
Refactor
Tests
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.