Support for writing validity range using posix time#733
Support for writing validity range using posix time#733sofia-bobbiesi wants to merge 13 commits intotxpipe:mainfrom
Conversation
WalkthroughThe PR updates the TRP compiler initialization to construct a tx3_cardano::ChainPoint tip from the current StateStore (slot, hash, timestamp) and pass it to tx3_cardano::Compiler::new; it adds a TipNotResolved error variant and code, and a minor whitespace change in a test string. Changes
Sequence DiagramsequenceDiagram
participant LC as load_compiler
participant SS as StateStore
participant TC as tx3_cardano::Compiler
LC->>SS: get state & read_cursor
alt state or tip missing
LC-->>LC: return Error::TipNotResolved
else tip available
LC->>SS: extract slot & hash
LC->>SS: load era summary (for slot_time)
LC-->>LC: compute timestamp = slot_time(slot) * 1000
LC-->>LC: build ChainPoint{slot, hash, timestamp}
LC->>TC: Compiler::new(pparams, config, tip)
TC-->>LC: Compiler instance
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
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 comments (1)
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: 1
🧹 Nitpick comments (1)
crates/trp/src/error.rs (1)
56-58: Consider attaching diagnostics to TipNotResolved.Carrying minimal context (e.g., had_cursor: bool, had_hash: bool, slot: Option) would ease client troubleshooting; you could also surface it via Error::data().
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.gitignore(1 hunks)crates/trp/src/compiler.rs(1 hunks)crates/trp/src/error.rs(3 hunks)crates/trp/src/methods.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/trp/src/compiler.rs (2)
crates/core/src/lib.rs (2)
slot(490-490)hash(491-491)crates/cardano/src/eras.rs (1)
load_era_summary(186-198)
⏰ 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)
.gitignore (1)
5-5: LGTM; minor path check.If this is for test artifacts (e.g., insta), double‑check whether the directory is “/snapshot” vs “/snapshots”.
crates/trp/src/methods.rs (1)
191-196: No functional impact.Whitespace change in SUBJECT_PROTOCOL is fine; keeps formatting readable.
crates/trp/src/error.rs (2)
134-134: LGTM on code assignment.CODE_TIP_NOT_RESOLVED (-32004) fits the existing custom error range.
156-157: Mapping looks correct.TipNotResolved → CODE_TIP_NOT_RESOLVED as expected.
crates/trp/src/compiler.rs (2)
76-81: Tip construction is clear.Struct assembly reads well; once units are confirmed and hash handling fixed, this looks solid.
72-75: Validate externalChainPoint.timestampunit
Slot time is multiplied by 1 000 (ms), buttx3_cardano::ChainPoint.timestampmay expect seconds. We can’t confirm its unit locally—please verify in thetx3_cardanocrate (e.g. in its docs or source) whethertimestampis POSIX seconds or milliseconds, and adjust/remove the* 1000accordingly.
crates/trp/src/compiler.rs
Outdated
| let cursor = domain.state().read_cursor()?.ok_or(Error::TipNotResolved)?; | ||
| let slot = cursor.slot(); | ||
| let hash = cursor.hash().map(|h| h.to_vec()).unwrap_or_default(); | ||
|
|
There was a problem hiding this comment.
Don’t default to empty hash; fail if missing.
An empty tip hash weakens correctness; prefer surfacing TipNotResolved when the hash isn’t available.
Apply this diff:
- let cursor = domain.state().read_cursor()?.ok_or(Error::TipNotResolved)?;
- let slot = cursor.slot();
- let hash = cursor.hash().map(|h| h.to_vec()).unwrap_or_default();
+ let cursor = domain.state().read_cursor()?.ok_or(Error::TipNotResolved)?;
+ let slot = cursor.slot();
+ let hash = cursor.hash().map(|h| h.to_vec()).ok_or(Error::TipNotResolved)?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let cursor = domain.state().read_cursor()?.ok_or(Error::TipNotResolved)?; | |
| let slot = cursor.slot(); | |
| let hash = cursor.hash().map(|h| h.to_vec()).unwrap_or_default(); | |
| let cursor = domain.state().read_cursor()?.ok_or(Error::TipNotResolved)?; | |
| let slot = cursor.slot(); | |
| let hash = cursor.hash().map(|h| h.to_vec()).ok_or(Error::TipNotResolved)?; |
🤖 Prompt for AI Agents
In crates/trp/src/compiler.rs around lines 68 to 71, the code currently defaults
to an empty tip hash which weakens correctness; instead, if cursor.hash() is
None return Error::TipNotResolved. Replace the unwrap_or_default behavior so
that you convert the Option<bytearray> from cursor.hash() into a Result,
returning Err(Error::TipNotResolved) when missing, and only then convert the
present hash to a Vec for use.
| tx3_cardano::Config { | ||
| extra_fees: config.extra_fees, | ||
| }, | ||
| tip, |
There was a problem hiding this comment.
Is this awaiting a pending release on tx3_cardano to work?
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/trp/src/compiler.rs (1)
56-69: Address the unresolved hash defaulting issue from previous review.The hash extraction at line 59 still uses
unwrap_or_default(), which was flagged in a previous review comment. As noted before, defaulting to an empty hash weakens correctness—if the hash is unavailable, the code should returnError::TipNotResolvedinstead.Apply this diff to fail when hash is missing:
- let hash = cursor.hash().map(|h| h.to_vec()).unwrap_or_default(); + let hash = cursor.hash().map(|h| h.to_vec()).ok_or(Error::TipNotResolved)?;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/trp/src/compiler.rs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/trp/src/compiler.rs (3)
src/adapters.rs (1)
state(77-79)crates/core/src/lib.rs (3)
state(725-725)slot(501-501)hash(502-502)crates/cardano/src/eras.rs (2)
state(203-203)load_era_summary(186-198)
⏰ 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 (1)
crates/trp/src/compiler.rs (1)
4-4: LGTM: Import addition is necessary.The
StateStoreimport is correctly added to support theread_cursor()call introduced in the tip construction logic.
| let compiler = tx3_cardano::Compiler::new( | ||
| pparams, | ||
| tx3_cardano::Config { | ||
| extra_fees: config.extra_fees, | ||
| }, | ||
| tip, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify tx3_cardano dependency version supports this API.
The compiler is now correctly instantiated with the tip argument. However, a previous comment questioned whether this change depends on a pending tx3_cardano release. Please confirm that version 0.11.4 (currently specified) supports the three-argument Compiler::new signature.
🏁 Script executed:
#!/bin/bash
# Verify tx3_cardano::Compiler::new signature supports the tip argument
# Search for the Compiler::new usage to confirm the signature matches
rg -nP --type=rust 'tx3_cardano::Compiler::new' -A 5
# Check Cargo.toml for tx3_cardano version
fd -e toml Cargo.toml --exec rg -n 'tx3_cardano'Length of output: 340
---
🏁 Script executed:
```shell
#!/bin/bash
# Find all Cargo.toml files and search for tx3_cardano dependency
fd -type f -name "Cargo.toml" --exec rg -n 'tx3_cardano' {} +
Length of output: 170
🏁 Script executed:
#!/bin/bash
# Also check the root Cargo.toml specifically
cat Cargo.toml | head -100Length of output: 2925
🏁 Script executed:
#!/bin/bash
# And the crates/trp Cargo.toml specifically
cat crates/trp/Cargo.toml | head -150Length of output: 1323
🌐 Web query:
tx3-cardano 0.11.4 Compiler::new API signature tip parameter
💡 Result:
Do you want the exact Rust signature (types) for Compiler::new in tx3-cardano v0.11.4, or a link to the docs/source where it's defined?
🌐 Web query:
tx3-cardano crates.io version history releases
💡 Result:
You can view tx3-cardano's version history/releases on these pages:
- Crates.io (versions tab) — official release list. See the crate page on crates.io. [1]
- docs.rs — per-version documentation (examples: v0.11.4, v0.11.3). [2][3]
- GitHub (tx3/tx3 repo) — source and tags/releases. [4]
References
- crates.io crate page for tx3-cardano.
- docs.rs — tx3-cardano v0.11.4.
- docs.rs — tx3-cardano v0.11.3.
- tx3-lang / tx3 on GitHub.
🌐 Web query:
tx3-cardano Compiler struct new method docs.rs 0.11.4
💡 Result:
The Compiler::new signature and implementation (tx3-cardano v0.11.4):
pub fn new(pparams: PParams, config: Config) -> Self
It constructs a Compiler with the given pparams and config and initializes latest_tx_body to None. [1]
Sources: tx3-cardano 0.11.4 source (lib.rs). [1]
🌐 Web query:
tx3-cardano Compiler new tip parameter version changelog
💡 Result:
Do you mean the tx3-cardano compiler change that added a new "tip" (or "new tip") parameter? Which specific details do you want: the version that introduced it, the exact changelog entry, or a link to the release/commit?
Update tx3_cardano dependency version to support the tip parameter.
The code at lines 71-76 calls Compiler::new with three arguments (pparams, config, tip), but tx3-cardano v0.11.4's Compiler::new signature only accepts two arguments: pparams and config. This is a compilation error. You must upgrade the tx3-cardano version in crates/trp/Cargo.toml to a version that supports the tip parameter, or revert the code to use the two-argument signature.
🤖 Prompt for AI Agents
In crates/trp/src/compiler.rs around lines 71-77, the call to
tx3_cardano::Compiler::new(pparams, config, tip) uses a three-argument signature
that is not available in the currently pinned tx3_cardano version; either update
the tx3_cardano dependency in crates/trp/Cargo.toml to a version that exposes
the three-argument Compiler::new (and run cargo update) or change this call back
to the two-argument form Compiler::new(pparams, config) to match the current
dependency; modify Cargo.toml to the compatible version if you need the tip
behavior, otherwise revert the call to the two-argument signature and adjust any
downstream code accordingly.
scarmuega
force-pushed
the
main
branch
2 times, most recently
from
295b558 to
09d0b81
Compare
2 participants
Summary by CodeRabbit
Bug Fixes
Tests