Query tip on cardano cli #829
Conversation
Add security policy outlining supported versions and reporting process.
📝 WalkthroughWalkthroughAdds a Unix statequery protocol module and integrates it into NodeServer: mod.rs now launches both chainsync and statequery tasks concurrently (tokio::try_join). statequery.rs implements session lifecycle, CBOR request/response handling, and ledger/era queries. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Session as statequery\nSession
participant Domain
participant Local as localstate\nServer
rect rgb(250,250,250)
Note over Session: Idle
Client->>Session: Acquire(point)
Session->>Domain: Read tip & validate point
Session->>Local: Open acquired connection
Session-->>Client: AcquireResult (Acquired / PointNotOnChain)
end
rect rgb(240,255,240)
Note over Session: Acquired
Client->>Session: Query (Ledger / Block / HardFork)
alt Ledger / Block queries
Session->>Local: Fetch ledger tip / block info
Session-->>Client: CBOR-encoded response
else HardFork (era history)
Session->>Domain: Load era summaries
Session->>Session: build_era_history_response()
Session-->>Client: CBOR-era-history
end
end
Client->>Session: Release
Note over Session: Return to Idle
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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. 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 (3)
src/serve/o7s_unix/statequery.rs (3)
96-108: Consider derivingsafeFromTipandgenesisWindowfrom configuration.The value
4320(2 × k where k=2160) is hardcoded and mainnet-specific. For testnets or other networks with different security parameters, this could produce incorrect era history responses.Consider deriving these values from the genesis configuration if available, or at minimum define a named constant with documentation.
🔎 Proposed refactor
+// Default stability window for mainnet: 2 * k where k = 2160 +const DEFAULT_SAFE_FROM_TIP: u64 = 4320; + fn build_era_history_response( eras: &[DolosEraSummary], genesis: &Genesis, ) -> Result<Vec<u8>, Error> {Then use
DEFAULT_SAFE_FROM_TIPat lines 99 and 107.
237-242:ChainPoint::Slotconverted toOriginloses information.In both
GetChainPointandGetLedgerTiphandlers,ChainPoint::Slot(s)is converted toOPoint::Origin, discarding the slot number. This may produce confusing results if a slot-only point was acquired.Consider whether this should instead construct an
OPoint::Specificwith an empty/placeholder hash, or ifChainPoint::Slotshould be prevented from reaching this code path.Also applies to: 288-293
269-277: Hardcoded era-to-index mapping may require updates for future protocol versions.This mapping between Cardano protocol versions and era indices is hardcoded. The catch-all
_ => 6assumes any future protocol version maps to Conway (era 6).Consider adding a comment documenting when this needs updating, or extracting this mapping to a centralized location.
+ // NOTE: Update this mapping when new eras are added + // Protocol versions: 0-1 = Byron, 2 = Shelley, 3 = Allegra, + // 4 = Mary, 5-6 = Alonzo, 7 = Babbage, 8+ = Conway let era_index = match edge.protocol { 0..=1 => 0, // Byron 2 => 1, // Shelley 3 => 2, // Allegra 4 => 3, // Mary 5..=6 => 4, // Alonzo 7 => 5, // Babbage _ => 6, // Conway };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/serve/o7s_unix/mod.rssrc/serve/o7s_unix/statequery.rs
🧰 Additional context used
🧬 Code graph analysis (1)
src/serve/o7s_unix/mod.rs (3)
src/serve/o7s_unix/statequery.rs (1)
handle_session(384-411)src/relay/mod.rs (1)
handle_session(19-41)src/serve/o7s_unix/chainsync.rs (1)
handle_session(188-210)
⏰ 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). (1)
- GitHub Check: Check Build
🔇 Additional comments (6)
src/serve/o7s_unix/mod.rs (2)
21-26: LGTM!Good use of destructuring with
..to ignore unused fields fromNodeServer. This cleanly extracts only the needed components.
28-44: LGTM!The concurrent task handling is well-structured. The error propagation pattern (
result.0?; result.1?) correctly handles errors from both spawned tasks, which is more thorough than thelet _ = tokio::try_join!(...)pattern used inrelay/mod.rs.src/serve/o7s_unix/statequery.rs (4)
1-11: LGTM!Imports are appropriate for the functionality.
DatelikeandTimeliketraits are correctly imported for date/time operations used inGetSystemStartquery handling.
162-171: Verify intent:ChainPoint::Slotassumes existence without verification.Unlike
ChainPoint::Specificwhich verifies the block hash exists,ChainPoint::Slot(_)unconditionally returnstrue. If a client acquires a non-existent slot, this would succeed but subsequent queries might behave unexpectedly.Is this intentional, or should slot-only points also be validated?
344-381: LGTM!The request processing loop correctly implements the localstate protocol state machine:
- Outer loop handles idle state, waiting for
Acquire- Inner loop handles acquired state, processing
Query/ReAcquire/ReleaseReleasecorrectly breaks to idle state
384-411: LGTM!The session entry point follows the established pattern from
chainsync::handle_sessionwith proper cancellation support viatokio::select!and appropriate lifecycle logging.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/serve/o7s_unix/statequery.rs (3)
98-116: Extractsecurity_paramonce to eliminate duplication.The
security_paramis fetched from genesis twice (lines 98-101 and 112-115). Extract it once before the SafeZone conditional to improve readability and avoid redundant lookups.🔎 Proposed refactor
encoder .u64(era.slot_length * 1000) .map_err(|e| Error::server(e.to_string()))?; + let k = genesis + .shelley + .security_param + .ok_or_else(|| Error::server("missing security param"))?; + // SafeZone if era.end.is_none() { // UnsafeIndefiniteSafeZone: [1, 1] encoder.array(1).map_err(|e| Error::server(e.to_string()))?; encoder.u8(1).map_err(|e| Error::server(e.to_string()))?; } else { // StandardSafeZone: [3, 0, safeFromTip, safeBeforeEpoch] // safeFromTip = 2 * k (stability window) // safeBeforeEpoch is encoded as [1, 0] for NoLowerBound encoder.array(3).map_err(|e| Error::server(e.to_string()))?; encoder.u8(0).map_err(|e| Error::server(e.to_string()))?; - let k = genesis - .shelley - .security_param - .ok_or_else(|| Error::server("missing security param"))?; let safe_from_tip = 2 * k; encoder .u64(safe_from_tip.into()) .map_err(|e| Error::server(e.to_string()))?; // safeBeforeEpoch: NoLowerBound = [1, 0] encoder.array(1).map_err(|e| Error::server(e.to_string()))?; encoder.u8(0).map_err(|e| Error::server(e.to_string()))?; } - let k = genesis - .shelley - .security_param - .ok_or_else(|| Error::server("missing security param"))?; let genesis_window = 2 * k; encoder .u64(genesis_window.into()) .map_err(|e| Error::server(e.to_string()))?;
20-37: Consider validating that the eras slice is non-empty.If
erasis empty, the function succeeds but produces an empty CBOR array. Depending on protocol expectations, this might be invalid. Consider adding an early check.🔎 Proposed validation
fn build_era_history_response( eras: &[DolosEraSummary], genesis: &Genesis, ) -> Result<Vec<u8>, Error> { + if eras.is_empty() { + return Err(Error::server("era summary is empty")); + } + let mut bytes = vec![]; let mut encoder = Encoder::new(&mut bytes);
268-270: Roundabout CBOR encoding/decoding.The bytes from
build_era_history_responseare immediately decoded back intoAnyCbor. Consider havingbuild_era_history_responsereturn theAnyCbordirectly or accept an encoder to avoid this extra step.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/serve/o7s_unix/statequery.rs
⏰ 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). (1)
- GitHub Check: Check Build
🔇 Additional comments (2)
src/serve/o7s_unix/statequery.rs (2)
396-423: LGTM!The session entry point correctly initializes the session, handles cancellation via
tokio::select!, and provides appropriate logging for lifecycle events.
356-393: LGTM!The state machine correctly implements the idle → acquire → (query/reacquire)* → release cycle expected by the local state query protocol.
src/serve/o7s_unix/statequery.rs
Outdated
| .map_err(|e| Error::server(e.to_string()))?; | ||
|
|
||
| encoder | ||
| .u64(era.slot_length * 1000) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Cardano local state query protocol EraParams slot length units specification
💡 Result:
Short answer: EraParams.slotLength is a time duration expressed in seconds (a NominalDiffTime). Epoch/epochLength and other counts are in slots (integer slot counts). Sources: [1][2][3][4].
Sources
- CIP-9 / protocol parameters (slotLength described “The length of each slot (in seconds)”). [1]
- ouroboros-consensus EraParams API (eraSlotLength / SlotLength type; eraEpochSize is an EpochSize). [2]
- cardano-node / cardano-api docs (protocolParamSlotLength is a NominalDiffTime — time duration). [3]
- Ogmios local-state-query overview (ledger/era-dependent queries return Era-related params). [4]
🏁 Script executed:
# First, let me examine the code file to understand the context
head -n 100 src/serve/o7s_unix/statequery.rs | tail -n 30Repository: txpipe/dolos
Length of output: 1151
🏁 Script executed:
# Search for related context around line 83
sed -n '70,95p' src/serve/o7s_unix/statequery.rsRepository: txpipe/dolos
Length of output: 1017
🏁 Script executed:
# Look for any documentation or comments about slot_length in the codebase
rg -i "slot.?length" -A 2 -B 2 --type rustRepository: txpipe/dolos
Length of output: 14312
🌐 Web query:
Ogmios slotLength unit EraParams response milliseconds or seconds specification
💡 Result:
Short answer: EraParams.slotLength is defined as a duration in seconds by the Cardano consensus (SlotLength = seconds). Ogmios exposes the era summaries/eraParams from the node (consensus types). Some client libraries/wrappers (e.g. kupmios / Go wrappers) reformat that value into an object with milliseconds (so you may see slotLength.milliseconds) — convert accordingly. [1][2][3]
Sources:
- Ogmios API (eraSummaries / eraStart / ledger-state queries). [1]
- Ouroboros consensus EraParams (eraSlotLength / SlotLength is a time duration). [2]
- Example client wrapper showing slotLength serialized as milliseconds. [3]
🌐 Web query:
Ogmios eraSummaries slotLength CBOR encoding response format milliseconds
💡 Result:
Short answer: Ogmios' eraSummaries gives slotLength as a duration in milliseconds (an integer under a slotLength.milliseconds field in the JSON schema / client types). The underlying Cardano genesis/era param is expressed in seconds in many protocol docs (slotLength = seconds), but Ogmios/clients expose it as milliseconds for unambiguous arithmetic; if CBOR bytes are requested they are returned encoded (hex/base16) per Ogmios' CBOR options. [1][2][3]
Sources
- Ogmios API / TypeScript schema (QueryLedgerState/eraSummaries). Shows eraSummaries endpoint in the schema / client. [1]
- Go client type example exposing SlotLength as { "milliseconds": int }. (pkg.go.dev excerpt). [2]
- Cardano protocol / CIP and references: slotLength in genesis is given in seconds (e.g. slotLength: 1 means 1 second). Use eraSummaries for precise slot arithmetic. [3]
If you want, I can fetch an actual eraSummaries JSON example from a running node (including the exact field names and CBOR encoding) — would you like that?
🏁 Script executed:
# Check if there are any related specs or documentation files in the repo
find . -type f \( -name "*.md" -o -name "*.txt" -o -name "*.toml" \) | head -20Repository: txpipe/dolos
Length of output: 579
🏁 Script executed:
# Look for any specification or protocol documentation references
rg -i "ogmios\|era.*summary\|era.*params" -A 3 -B 1 --type rust | head -100Repository: txpipe/dolos
Length of output: 38
🏁 Script executed:
# Check the function context more thoroughly to understand what protocol is being served
sed -n '1,120p' src/serve/o7s_unix/statequery.rs | head -50Repository: txpipe/dolos
Length of output: 1962
🏁 Script executed:
# Look for any comments or documentation about EraParams format
sed -n '50,100p' src/serve/o7s_unix/statequery.rsRepository: txpipe/dolos
Length of output: 1965
🏁 Script executed:
# Check if there's any reference to Ogmios or the API spec in the codebase
rg -i "ogmios" -A 2 -B 2 --type rust --type mdRepository: txpipe/dolos
Length of output: 38
🏁 Script executed:
# Look for any Byron/historical slot length handling to understand the pattern better
sed -n '1,30p' src/serve/o7s_unix/statequery.rsRepository: txpipe/dolos
Length of output: 1203
🏁 Script executed:
# Check the genesis and era structures to confirm slot_length is in seconds
rg "struct.*Era\|pub slot_length" -A 5 --type rust crates/cardano/src/ | head -50Repository: txpipe/dolos
Length of output: 38
🏁 Script executed:
# Search for any test data or examples that show what values are expected
rg "slot_length|slotLength" -B 5 -A 5 --type rust src/serve/ crates/minibf/Repository: txpipe/dolos
Length of output: 10651
🌐 Web query:
Ogmios GetInterpreter query CBOR response format slot length encoding consensus
💡 Result:
Short answer
- Ogmios doesn't invent a special encoding: the node's GetInterpreter response is a consensus (Cardano / ouroboros-consensus) data structure encoded in CBOR by the node; Ogmios will (by default) convert that CBOR to JSON for its API, and — if configured — also include the raw CBOR bytes as a base16 string in a "cbor" field. [1][2][3]
Key points you need to know
- Raw CBOR inclusion: enable with Ogmios flags (e.g. --include-cbor or the more specific --include-transaction-cbor / --include-metadata-cbor / --include-script-cbor). When present the CBOR is returned as base16 text. [1]
- Source/shape of the data: GetInterpreter is the ouroboros-consensus Interpreter / GetInterpreter query (used for slot/epoch/time translations and era parameters). The structure and fields (EraParams, Interpreter, etc.) are defined by the consensus code and encoded in standard CBOR. Ogmios simply relays the node’s result. [3]
- Integer and length encoding: CBOR uses its standard major types. Slot numbers, epoch indices and era-parameter sizes are encoded as CBOR unsigned (major type 0) or negative integers where appropriate, and compound objects (maps/arrays) use the usual definite or indefinite-length encodings the node emitted. There is no Ogmios-specific “slot-length” encoding — it’s whatever the node/consensus library CBOR encoder emits. [2][3]
- Decoding pitfalls: mismatches happen when client-side decoders (or mismatched tooling versions) expect a different era-parameter shape/length than the node produced (e.g., "Size mismatch when decoding EraParams" errors). Ensure your client libraries (cardano-node / cardano-cli / any decoding code) are compatible with the node version used. [4]
If you want to inspect the exact bytes
- Enable raw CBOR in Ogmios (see flag above), take the base16 string from the "cbor" field, decode it to bytes and run a CBOR inspector (or the consensus CDDL/decoder) to see the exact major types and length encodings produced by that node version. [1][2]
Sources
[1] Ogmios — Configuring (CBOR outputs & include-cbor flags).
[2] Local State Query / node-to-client CBOR description (mini-protocols / CBOR encoding reference).
[3] ouroboros-consensus — GetInterpreter / Interpreter notes (role of GetInterpreter in slot/epoch/time translation).
[4] Cardano forum thread showing a "Size mismatch when decoding EraParams" error caused by version / schema mismatch.
🏁 Script executed:
# Look at how era.slot_length is loaded/initialized to confirm it's in seconds
rg "slot_length.*=" -B 3 -A 1 --type rust crates/cardano/src/genesis/ crates/cardano/src/forks.rs | head -60Repository: txpipe/dolos
Length of output: 1637
Remove the * 1000 multiplication — slotLength must be in seconds per ouroboros-consensus specification.
The Cardano/Ouroboros consensus protocol specifies slotLength in the EraParams as a time duration in seconds (per CIP-9 and ouroboros-consensus SlotLength type). The code loads era.slot_length in seconds (Byron converts slot_duration / 1000, Shelley stores it directly), but then multiplies by 1000 before encoding, which incorrectly sends milliseconds to clients expecting seconds. This causes a unit mismatch with the GetInterpreter response format that Ogmios relays directly from the node.
🤖 Prompt for AI Agents
In src/serve/o7s_unix/statequery.rs around line 83, the code multiplies
era.slot_length by 1000 before encoding which sends milliseconds; remove the *
1000 so slot_length is encoded in seconds per ouroboros-consensus spec. Change
the expression to encode era.slot_length directly (convert to u64 if necessary)
so the serialized value is seconds, not milliseconds.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
src/serve/o7s_unix/statequery.rs (4)
53-53: This critical issue has already been flagged in previous reviews.The multiplication by 1000 incorrectly converts slot_length from seconds to milliseconds, violating the ouroboros-consensus specification. Remove the
* 1000to encode slot_length in seconds as required by the protocol.
177-186: This critical issue has already been flagged in previous reviews.The code allows acquisition of
ChainPoint::Slotwithout verifying that the slot exists in the archive. WhileChainPoint::Slotshouldn't originate from client input, iftip_cursor()or other internal logic produces it, the acquisition succeeds without validation. This can lead to malformed protocol responses downstream (as seen at lines 258 and 305).
251-260: This major issue has already been flagged in previous reviews.When the tip is
ChainPoint::Slot(_), the code returnsOPoint::Originwhich creates a malformed protocol response. Sincetip_cursor()at line 253 should resolveChainPoint::SlottoChainPoint::Specific, this arm should be unreachable. However, as defensive coding, it should either resolve the slot properly or return an error rather than mapping to Origin.
295-308: This major issue has already been flagged in previous reviews.Similar to the GetChainPoint handler, this code maps
ChainPoint::Slot(_)toOPoint::Originat line 305, creating a malformed protocol response. The same fix should be applied here as recommended for lines 251-260.
🧹 Nitpick comments (3)
src/serve/o7s_unix/statequery.rs (3)
55-55: Consider checked arithmetic for security_param multiplication.While overflow is extremely unlikely with realistic Cardano security parameters (~2160), using
checked_mulorsaturating_mulwould make the code more defensive against unexpected values.🔎 Proposed defensive fix
- let safe_from_tip = self.security_param * 2; + let safe_from_tip = self.security_param.saturating_mul(2);
283-291: Consider externalizing era index mapping for maintainability.The hardcoded protocol-version-to-era-index mapping works for current Cardano eras but could become outdated if new eras are introduced or protocol versions change. Consider extracting this mapping to a configuration or using a more declarative approach that can be updated without code changes.
💡 Alternative approach
You could define a static lookup table or use the era data structures more directly:
// At module level const ERA_INDEX_MAP: &[(u16, u16)] = &[ (0, 0), (1, 0), // Byron (2, 1), // Shelley (3, 2), // Allegra (4, 3), // Mary (5, 4), (6, 4), // Alonzo (7, 5), // Babbage // Future eras can be added here ]; fn protocol_to_era_index(protocol: u16) -> u16 { ERA_INDEX_MAP .iter() .find(|(p, _)| *p == protocol) .map(|(_, era)| *era) .unwrap_or(6) // Conway or future }
322-324: Consider checked conversion for epoch number cast.Line 324 casts the epoch to
u32without checking for overflow. While practically impossible (u32::MAX epochs would take billions of years at Cardano's epoch duration), for consistency with the block number handling at line 242, consider using a checked conversion.🔎 Proposed consistent handling
let (epoch, _) = chain_summary.slot_epoch(tip_slot); + let epoch_u32 = u32::try_from(epoch).unwrap_or(u32::MAX); - AnyCbor::from_encode((epoch as u32,)) + AnyCbor::from_encode((epoch_u32,))
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/serve/o7s_unix/statequery.rs
🧰 Additional context used
🧬 Code graph analysis (1)
src/serve/o7s_unix/statequery.rs (5)
src/facade.rs (1)
dolos_core(31-31)src/prelude.rs (1)
server(56-58)crates/cardano/src/eras.rs (1)
edge(96-99)crates/cardano/src/model.rs (1)
epoch(195-200)src/serve/o7s_unix/mod.rs (1)
handle_session(16-49)
⏰ 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). (1)
- GitHub Check: Check Build
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.