feat: introduce balius-proto for runtime <-> WASM schema#98
Open
scarmuega wants to merge 8 commits into
Open
Conversation
Decouple the WASM ABI from the upstream utxorpc spec. The runtime previously shipped utxorpc::spec::cardano::* prost-encoded bytes across the WIT boundary as cbor; that implicitly made the upstream u5c spec the WASM ABI, so the 0.17 -> 0.18 BigInt break propagated to every existing worker. balius-proto owns a frozen schema (mirrors utxorpc-spec 0.17.0 wire format). The runtime converts upstream u5c types into this schema once at chainsync ingress and at ledger reads. BigInt -> u64/i64 is fallible and halts the worker on overflow. Workers built against the pre-BigInt SDK keep decoding cleanly because the new bytes are wire-identical to what they expect. balius-sdk drops the utxorpc-spec dep in favor of balius-proto; Utxo<D>::coin() and PParams field access become plain integers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (15)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR establishes a new ChangesProtobuf migration and integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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.
🧹 Nitpick comments (1)
balius-runtime/src/lib.rs (1)
315-319: 💤 Low valueConsider making
from_bytesfallible for robustness.The
unwrap()onBlock::decodewill panic if the stored bytes are malformed. While blocks are converted at chain-sync ingress (so stored data should be valid), returningResultwould provide graceful handling for edge cases like store corruption.♻️ Optional: Return Result instead of panicking
- pub fn from_bytes(data: &[u8]) -> Self { + pub fn from_bytes(data: &[u8]) -> Result<Self, prost::DecodeError> { use prost::Message; - Self::Cardano(balius_proto::cardano::Block::decode(data).unwrap()) + Ok(Self::Cardano(balius_proto::cardano::Block::decode(data)?)) }This would require updating call sites to handle the
Result.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@balius-runtime/src/lib.rs` around lines 315 - 319, Change from_bytes to be fallible: replace the current panic-on-error behavior in from_bytes (which constructs Self::Cardano from balius_proto::cardano::Block::decode(data).unwrap()) with a Result-returning signature (e.g., Result<Self, prost::DecodeError> or your crate error type) and propagate the decode error using ? or map_err so malformed bytes are returned as Err; update callers of from_bytes to handle the Result accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@balius-runtime/src/lib.rs`:
- Around line 315-319: Change from_bytes to be fallible: replace the current
panic-on-error behavior in from_bytes (which constructs Self::Cardano from
balius_proto::cardano::Block::decode(data).unwrap()) with a Result-returning
signature (e.g., Result<Self, prost::DecodeError> or your crate error type) and
propagate the decode error using ? or map_err so malformed bytes are returned as
Err; update callers of from_bytes to handle the Result accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 80355eac-cee7-4a23-82f6-a522fe0c6b2f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
Cargo.tomlbalius-proto/Cargo.tomlbalius-proto/src/cardano.rsbalius-proto/src/cardano.serde.rsbalius-proto/src/convert.rsbalius-proto/src/lib.rsbalius-proto/tests/wire_compat.rsbalius-runtime/Cargo.tomlbalius-runtime/src/drivers/chainsync.rsbalius-runtime/src/ledgers/u5c.rsbalius-runtime/src/lib.rsbalius-sdk/Cargo.tomlbalius-sdk/src/qol.rsbalius-sdk/src/txbuilder/dsl.rsbalius-sdk/src/txbuilder/mod.rsexamples/asteria-tracker/Cargo.tomlexamples/asteria-tracker/src/lib.rs
Drop everything from cardano.rs except the wire surface workers
actually consume:
TxOutput { address, coin, assets }
Tx { inputs, outputs, fee, hash }
TxInput { tx_hash, output_index, as_output }
Multiasset, Asset (output_coin only)
Tags absent from these structs (datum, script, certificates,
withdrawals, mint, witnesses, validity, auxiliary, proposals, ...) are
deliberately dropped — workers needing them must extend the schema and
the converter explicitly. This shrinks balius-proto from ~5400 LOC of
generated cardano + 14k LOC of pbjson serde to 181 LOC total.
Block/BlockBody/BlockHeader move out of balius-proto since they never
cross WIT. The runtime's Block enum reverts to wrapping
utxorpc::spec::cardano::Block; per-Tx conversion happens at apply_block
/ undo_block via the fallible Block::txs() accessor.
PParams gone too: the SDK txbuilder now defines a minimal
PParams { coins_per_utxo_byte: u64 } locally, and the runtime's
read-params emits a matching minimal JSON shape.
asteria-tracker example trimmed to use only multiasset fields (datum-
derived position is dropped since the datum tag is gone).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The datum payload (PlutusData) and the witness set are common worker inputs — bring them back into the trimmed schema. Both were wire-stable across utxorpc-spec 0.17 -> 0.18, so the converter just roundtrips them via prost. New types in cardano.rs: Datum, PlutusData (+ Constr, PlutusDataPair, PlutusDataMap, PlutusDataArray, BigInt), Script (+ NativeScript, NativeScriptList, ScriptNOfK), WitnessSet, VKeyWitness. Tags on PlutusData and Constr.any_constructor match upstream 0.17 verbatim (verified by failing-then-fixing wire_compat tests). asteria-tracker example restores its datum-derived position tracking. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SupernaviX
approved these changes
May 1, 2026
Collaborator
SupernaviX
left a comment
SupernaviX
left a comment
There was a problem hiding this comment.
Hopefully tx3 integration lets you throw out all this compat stuff
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ompat
Pre-BigInt workers ship a utxorpc-spec 0.17 pbjson deserializer for the
JSON `read-params` returns. The prior approach in this PR — emitting
`{"coins_per_utxo_byte": N}` snake_case — broke that contract: the key
doesn't match, the value type doesn't match, and 30 other fields the
worker may read are gone entirely.
PParams crosses the WIT boundary as JSON, not protobuf, so prost
wire-compat (the trick that anchors Tx/UTxO bytes) buys nothing for it.
Instead, `pparams_to_legacy_json` walks the upstream `PParams`
imperatively and emits exactly the JSON shape utxorpc-spec 0.17's pbjson
serializer would produce: camelCase keys, u64/i64 as JSON strings,
u32/i32 as plain numbers, None / proto3-default fields omitted. BigInt
fields collapse via `unwrap_u64`; overflow halts the worker as before.
The SDK side keeps a minimal `PParams` (only `coins_per_utxo_byte` is
actually consumed today) but now declares it with
`serde_with::DisplayFromStr` and `rename = "coinsPerUtxoByte"` so it
reads the same pbjson shape as old workers' decoder.
`mock_pparams.json` is restored to the canonical 0.17 pbjson snapshot,
so the mock ledger and the real ledger now hand workers identical bytes.
Tests:
- `convert_pparams_decodes_under_017_pbjson` — load-bearing: converted
PParams round-trips through utxorpc-spec 0.17's actual pbjson decoder.
- `convert_pparams_overflow_propagates` — BigUInt halts.
- `convert_pparams_omits_default_fields` — pbjson omit-default preserved.
- `e2e::faucet_claim` exercises the full path end-to-end (mock JSON →
SDK PParams → MinUtxoLovelace → tx build) and passes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@balius-proto/src/convert.rs`:
- Around line 38-43: The unwrap_u64 function currently only accepts
upstream::big_int::BigInt::Int and treats BigUInt as Overflow; update unwrap_u64
to also handle upstream::big_int::BigInt::BigUInt by decoding its byte
representation into a u64 (attempt a checked-from-bytes or bigint-to-u64
conversion) and return Ok(u64) when it fits, otherwise return
ConvertError::Overflow; retain the existing Int branch but ensure negative Ints
still map to ConvertError::Overflow and positive Ints convert to u64 as before.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 327f5355-3225-4b03-8465-462021065b3f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
balius-proto/Cargo.tomlbalius-proto/src/convert.rsbalius-proto/tests/wire_compat.rsbalius-runtime/src/ledgers/u5c.rsbalius-sdk/src/txbuilder/mod.rs
✅ Files skipped from review due to trivial changes (1)
- balius-proto/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (2)
- balius-sdk/src/txbuilder/mod.rs
- balius-runtime/src/ledgers/u5c.rs
Comment on lines
+38
to
+43
| fn unwrap_u64(b: Option<&upstream::BigInt>) -> Result<u64, ConvertError> { | ||
| match b.and_then(|x| x.big_int.as_ref()) { | ||
| None => Ok(0), | ||
| Some(upstream::big_int::BigInt::Int(v)) if *v >= 0 => Ok(*v as u64), | ||
| _ => Err(ConvertError::Overflow), | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In utxorpc-spec 0.18.1, how is utxorpc::v1alpha::cardano::big_int::BigIntencoded, and are positive values that do not fitInt(i64)expected to use theBigUInt variant?
💡 Result:
In utxorpc-spec 0.18.1, the utxorpc::v1alpha::cardano::big_int::BigInt message is encoded as a oneof field in Protocol Buffers, allowing it to represent integers of varying sizes and signs [1]. The BigInt structure typically includes the following variants to handle different integer ranges [1]: 1. int: Used for values that fit within a standard 64-bit signed integer (int64) [1]. 2. big_uint: Used for positive values that exceed the range of a 64-bit signed integer [1]. This variant is expected to store the value as raw bytes (typically representing an unsigned big integer) [1]. 3. big_n_int: Used for negative values that fall outside the range of a 64-bit signed integer [1]. Yes, positive values that do not fit into a standard int64 are expected to use the big_uint variant [1]. When encoding such values, the integer is converted into its raw byte representation (often using big-endian encoding) and stored within the big_uint field [1]. Conversely, when decoding, the system checks which variant is present and reconstructs the integer accordingly [1].
Citations:
🏁 Script executed:
# First, check if the file exists and view the function in context
find . -name "convert.rs" -type f | head -5Repository: txpipe/balius
Length of output: 87
🏁 Script executed:
# View the convert.rs file to understand the implementation
cat -n ./balius-proto/src/convert.rs | head -80Repository: txpipe/balius
Length of output: 3253
🏁 Script executed:
# View more of the convert.rs file to see if there are other BigInt handling functions
cat -n ./balius-proto/src/convert.rs | sed -n '38,150p'Repository: txpipe/balius
Length of output: 4516
🏁 Script executed:
# Search for the BigInt protobuf definition or imports to understand the structure
rg "big_int|BigInt" --type rs -A 3 -B 3 | head -100Repository: txpipe/balius
Length of output: 84
🏁 Script executed:
# Search for BigInt references in the codebase
rg "big_int|BigInt" -A 3 -B 3 | head -150Repository: txpipe/balius
Length of output: 9974
🏁 Script executed:
# Check the complete BigInt enum definition in cardano.rs
cat -n ./balius-proto/src/cardano.rs | grep -A 20 "pub enum BigInt"Repository: txpipe/balius
Length of output: 841
🏁 Script executed:
# Look at the tests to understand the expected behavior
cat -n ./balius-proto/tests/wire_compat.rs | grep -B 5 -A 15 "convert_overflow_big_uint_errors"Repository: txpipe/balius
Length of output: 856
🏁 Script executed:
# Check if there's any handling of BigUInt elsewhere in convert.rs to understand complete scope
cat -n ./balius-proto/src/convert.rs | wc -lRepository: txpipe/balius
Length of output: 61
🏁 Script executed:
# Check if there are other BigInt/BigUInt handling patterns in convert.rs
cat -n ./balius-proto/src/convert.rs | grep -i "bigint\|bigunt" -B 2 -A 2Repository: txpipe/balius
Length of output: 1877
🏁 Script executed:
# Check how many places unwrap_u64 is called to understand the impact scope
rg "unwrap_u64" balius-proto/src/convert.rsRepository: txpipe/balius
Length of output: 837
Decode BigUInt values that fit within u64 instead of unconditionally treating them as overflow.
Line 41 only accepts Int(v), so a positive upstream BigUInt currently always becomes ConvertError::Overflow even when the value fits the u64 target. This blocks valid conversions for the i64::MAX + 1 ..= u64::MAX range, turning legitimate data into worker-stopping errors across multiple critical fields (coins, fees, protocol parameters).
The unwrap_u64 function needs to:
- Decode
BigUIntbytes to u64 when they represent values in that range - Keep
Overflowonly for truly out-of-range values and negative integers
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@balius-proto/src/convert.rs` around lines 38 - 43, The unwrap_u64 function
currently only accepts upstream::big_int::BigInt::Int and treats BigUInt as
Overflow; update unwrap_u64 to also handle upstream::big_int::BigInt::BigUInt by
decoding its byte representation into a u64 (attempt a checked-from-bytes or
bigint-to-u64 conversion) and return Ok(u64) when it fits, otherwise return
ConvertError::Overflow; retain the existing Int branch but ensure negative Ints
still map to ConvertError::Overflow and positive Ints convert to u64 as before.
…apter balius-proto is the Balius-owned wire schema crate; coupling it to an arbitrary upstream service (utxorpc-spec) defeats the point of having a schema crate in the first place. The conversion is u5c-specific and only consumed by balius-runtime, so it moves into the u5c ledger adapter. balius-proto/src/convert.rs is deleted along with its `convert` feature and the `utxorpc-spec` / `serde_json` deps. The crate is now pure types plus wire-format invariant assertions. The TryFrom impls become plain `convert_tx` / `convert_tx_output` / etc. functions inside `balius-runtime/src/ledgers/u5c.rs` (the orphan rule forbids the impls here — neither side is local to balius-runtime). `ConvertError` and `pparams_to_legacy_json` move alongside them. Call sites in `lib.rs::Block::txs` and the ledger paths update from `.try_into()` to the named functions. No behavior change. Conversion tests move from `balius-proto/tests/wire_compat.rs` to a new `balius-runtime/tests/u5c_convert.rs`. balius-proto keeps only the two `wire_compat_*` tests — the wire-format invariant of the schema itself, no upstream involved. Verified: cargo check --workspace clean; 2/2 wire-compat tests in balius-proto; 8/8 u5c convert tests in balius-runtime; e2e::faucet_claim still passes (end-to-end ABI compat path through the mock ledger); u5c-chainsync::wallet_balance still fails on the same pre-existing stale-wasm 5-vs-7 WIT variant mismatch, unrelated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The u5c ledger module grew large enough that the ledger-facing surface
(Config, Ledger, wit ↔ utxorpc From impls, chain_utxo_to_wit) was sharing
a file with the u5c → balius_proto conversion layer. Promote
`ledgers/u5c.rs` to a folder with two siblings:
ledgers/u5c/
mod.rs — ledger adapter: Config, Ledger, pattern conversions
convert.rs — ConvertError, convert_tx / convert_tx_output / ...,
pparams_to_legacy_json, all the BigInt-flattening
helpers, and the unit tests that assert ABI compat
against utxorpc-spec 0.17 (prost + pbjson)
The previously integration-style tests at
`balius-runtime/tests/u5c_convert.rs` move into a `#[cfg(test)] mod tests`
inside `convert.rs` — they're testing internals of that module, so unit
tests are the right home. Test count unchanged; they now run as part of
the runtime's lib tests.
Call sites in `lib.rs` updated to the new path
(`ledgers::u5c::convert::convert_tx`, `ledgers::u5c::convert::ConvertError`).
No re-exports — the structure is explicit.
Verified: cargo check --workspace clean; 9/9 lib tests pass (8 convert
tests + 1 router test); e2e::faucet_claim still passes; pre-existing
u5c-chainsync::wallet_balance stale-wasm failure unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (3)
balius-proto/tests/wire_compat.rs (1)
48-50: ⚡ Quick winAssert decoded datum payload bytes to complete the TxOutput wire-compat invariant.
Line 29-35 constructs
datum.payload, but the test never checks it post-decode. Adding that assertion would catch regressions where hash/CBOR survive but payload mapping drifts.Suggested test addition
let datum = decoded.datum.as_ref().expect("datum present"); assert_eq!(datum.hash.to_vec(), vec![0xEE; 32]); assert_eq!(datum.original_cbor.to_vec(), vec![0xFF; 8]); + let payload = datum.payload.as_ref().expect("datum payload present"); + match payload.plutus_data.as_ref() { + Some(v17::plutus_data::PlutusData::BoundedBytes(bytes)) => { + assert_eq!(bytes.to_vec(), b"datum-payload".to_vec()); + } + other => panic!("unexpected datum payload variant: {other:?}"), + } // Dropped TxOutput tags decode as defaults: assert!(decoded.script.is_none());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@balius-proto/tests/wire_compat.rs` around lines 48 - 50, The test currently checks decoded.datum.hash and decoded.datum.original_cbor but omits asserting the decoded datum.payload bytes; update the wire_compat.rs test to also assert that decoded.datum.as_ref().expect("datum present").payload (or its byte representation) equals the original payload constructed earlier (the bytes set in datum.payload in lines 29-35) so the TxOutput wire-compat invariant covers hash, original_cbor, and payload together.balius-runtime/src/ledgers/u5c/convert.rs (2)
312-546: ⚡ Quick winTest gap: datum and witnesses roundtrip is load-bearing but not exercised.
Lines 63–66 make the explicit assumption that
DatumandWitnessSetshare an identical wire format between utxorpc-spec 0.17 and 0.18, and the wholeroundtrip_optpath depends on it. None of the current tests round-trip a non-Nonedatum or witness set throughconvert_tx_output/convert_txand then re-decode under v17 prost. If the assumption is ever invalidated (e.g., a new field added at a low tag with a non-default value, or a tag reused), the failure mode is silent garbled bytes / decode error in production rather than at test time.A small extension of
convert_then_decode_with_017covering a populated datum and witness set would lock the invariant down.🧪 Sketch of additional coverage
#[test] fn convert_then_decode_with_017_datum_and_witnesses() { let upstream_out = v18::TxOutput { address: vec![0xAA; 28].into(), coin: Some(v18::BigInt { big_int: Some(v18::big_int::BigInt::Int(1_000_000)) }), assets: vec![], datum: Some(v18::Datum { /* populate a representative payload */ ..Default::default() }), script: None, }; let bal = convert_tx_output(upstream_out).expect("convert"); let decoded = v17::TxOutput::decode(bal.encode_to_vec().as_slice()).expect("0.17 decode"); assert!(decoded.datum.is_some()); let upstream_tx = v18::Tx { witnesses: Some(v18::WitnessSet { /* populate representative witnesses */ ..Default::default() }), hash: vec![0xCC; 32].into(), ..Default::default() }; let bal_tx = convert_tx(upstream_tx).expect("convert"); let decoded_tx = v17::Tx::decode(bal_tx.encode_to_vec().as_slice()).expect("0.17 decode"); assert!(decoded_tx.witnesses.is_some()); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@balius-runtime/src/ledgers/u5c/convert.rs` around lines 312 - 546, Add tests to exercise datum and witness set round-trips: extend the existing convert_then_decode_with_017 coverage by adding a new test (e.g., convert_then_decode_with_017_datum_and_witnesses) that builds a v18::TxOutput with a non-None datum and calls convert_tx_output, encodes the resulting legacy::TxOutput and decodes it with v17::TxOutput::decode asserting decoded.datum.is_some(); likewise build a v18::Tx with a non-None witnesses, call convert_tx, encode the legacy::Tx and decode with v17::Tx::decode asserting decoded_tx.witnesses.is_some(); ensure the datum and witness payloads are representative (populate non-default fields) so the roundtrip via convert_tx_output and convert_tx will catch wire-format mismatches.
229-307: ⚡ Quick winu32 PParams fields are emitted as JSON strings, contradicting the documented pbjson convention.
The module documentation (lines 130–131) states the output follows pbjson rules: "u64/i64 emitted as JSON strings, u32/i32 as plain numbers". However,
put_u64_strunconditionally emits asValue::String, and is used for fields that areuint32in utxorpc-spec 0.17 —max_tx_size,max_block_body_size,max_block_header_size,pool_retirement_epoch_bound,desired_number_of_pools,max_value_size,collateral_percentage,max_collateral_inputs,committee_term_limit,governance_action_validity_period,drep_inactivity_period. Notablymin_committee_size(alsouint32) is correctly emitted as a plain number viaValue::from, so the file is internally inconsistent.The test passes because pbjson deserializers accept both strings and numbers for 32-bit ints, but the output is no longer byte-identical to what utxorpc-spec 0.17's canonical pbjson serializer would emit — violating the stated ABI guarantee.
Add a
put_u32helper and route u32 fields through it:♻️ Proposed fix sketch
let put_u64_str = |o: &mut serde_json::Map<String, Value>, k: &str, v: u64| { if v != 0 { o.insert(k.into(), Value::String(v.to_string())); } }; + let put_u32 = |o: &mut serde_json::Map<String, Value>, k: &str, v: u32| { + if v != 0 { + o.insert(k.into(), Value::from(v)); + } + }; let cput = unwrap_u64(p.coins_per_utxo_byte.as_ref())?; put_u64_str(&mut o, "coinsPerUtxoByte", cput); - put_u64_str(&mut o, "maxTxSize", p.max_tx_size); + put_u32(&mut o, "maxTxSize", p.max_tx_size); let mfc = unwrap_u64(p.min_fee_coefficient.as_ref())?; put_u64_str(&mut o, "minFeeCoefficient", mfc); let mfcst = unwrap_u64(p.min_fee_constant.as_ref())?; put_u64_str(&mut o, "minFeeConstant", mfcst); - put_u64_str(&mut o, "maxBlockBodySize", p.max_block_body_size); - put_u64_str(&mut o, "maxBlockHeaderSize", p.max_block_header_size); + put_u32(&mut o, "maxBlockBodySize", p.max_block_body_size); + put_u32(&mut o, "maxBlockHeaderSize", p.max_block_header_size); // ...and similarly for the remaining u32 fields below.Also fold
min_committee_sizeinto the same helper for consistency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@balius-runtime/src/ledgers/u5c/convert.rs` around lines 229 - 307, The code currently uses put_u64_str to emit many uint32 PParams fields as JSON strings, violating the pbjson convention; add a new helper put_u32 = |o: &mut serde_json::Map<String, Value>, k: &str, v: u32| { if v != 0 { o.insert(k.into(), Value::from(v)); } } and replace uses of put_u64_str for the uint32 fields (max_tx_size, max_block_body_size, max_block_header_size, pool_retirement_epoch_bound, desired_number_of_pools, max_value_size, collateral_percentage, max_collateral_inputs, committee_term_limit, governance_action_validity_period, drep_inactivity_period) to call put_u32 instead; also fold min_committee_size into this helper path for consistency and leave unwrap_u64 and put_u64_str for true u64 fields like coins_per_utxo_byte, min_fee_coefficient, stake_key_deposit, pool_deposit, min_pool_cost, governance_action_deposit, drep_deposit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@balius-proto/tests/wire_compat.rs`:
- Around line 48-50: The test currently checks decoded.datum.hash and
decoded.datum.original_cbor but omits asserting the decoded datum.payload bytes;
update the wire_compat.rs test to also assert that
decoded.datum.as_ref().expect("datum present").payload (or its byte
representation) equals the original payload constructed earlier (the bytes set
in datum.payload in lines 29-35) so the TxOutput wire-compat invariant covers
hash, original_cbor, and payload together.
In `@balius-runtime/src/ledgers/u5c/convert.rs`:
- Around line 312-546: Add tests to exercise datum and witness set round-trips:
extend the existing convert_then_decode_with_017 coverage by adding a new test
(e.g., convert_then_decode_with_017_datum_and_witnesses) that builds a
v18::TxOutput with a non-None datum and calls convert_tx_output, encodes the
resulting legacy::TxOutput and decodes it with v17::TxOutput::decode asserting
decoded.datum.is_some(); likewise build a v18::Tx with a non-None witnesses,
call convert_tx, encode the legacy::Tx and decode with v17::Tx::decode asserting
decoded_tx.witnesses.is_some(); ensure the datum and witness payloads are
representative (populate non-default fields) so the roundtrip via
convert_tx_output and convert_tx will catch wire-format mismatches.
- Around line 229-307: The code currently uses put_u64_str to emit many uint32
PParams fields as JSON strings, violating the pbjson convention; add a new
helper put_u32 = |o: &mut serde_json::Map<String, Value>, k: &str, v: u32| { if
v != 0 { o.insert(k.into(), Value::from(v)); } } and replace uses of put_u64_str
for the uint32 fields (max_tx_size, max_block_body_size, max_block_header_size,
pool_retirement_epoch_bound, desired_number_of_pools, max_value_size,
collateral_percentage, max_collateral_inputs, committee_term_limit,
governance_action_validity_period, drep_inactivity_period) to call put_u32
instead; also fold min_committee_size into this helper path for consistency and
leave unwrap_u64 and put_u64_str for true u64 fields like coins_per_utxo_byte,
min_fee_coefficient, stake_key_deposit, pool_deposit, min_pool_cost,
governance_action_deposit, drep_deposit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 68f3deb4-1401-44f0-b76c-303544d55df8
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
balius-proto/Cargo.tomlbalius-proto/src/lib.rsbalius-proto/tests/wire_compat.rsbalius-runtime/Cargo.tomlbalius-runtime/src/ledgers/u5c/convert.rsbalius-runtime/src/ledgers/u5c/mod.rsbalius-runtime/src/lib.rs
✅ Files skipped from review due to trivial changes (1)
- balius-proto/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (2)
- balius-proto/src/lib.rs
- balius-runtime/src/lib.rs
…::v0 The crate's job has been "the Balius-owned wire schema", but the name "balius-proto" advertised protobuf as the only thing it carries. `balius-core` is a roomier name for what is really the foundational types crate for the runtime <-> WASM interface — protobuf today, more things possibly later. The schema module also gets nested: `balius_proto::cardano` becomes `balius_core::proto::v0::cardano`. The `v0` segment makes the versioning explicit: this schema is wire-compatible with utxorpc-spec 0.17 by construction (the entire point of this PR), so it is frozen for pre-BigInt worker compat. A future breaking change to the schema goes under `proto::v1`, leaving `v0` consumers untouched. Mechanical changes: - Crate dir balius-proto/ -> balius-core/ - Package name -> balius-core; description updated - src/cardano.rs -> src/proto/v0/cardano.rs (plus new mod.rs files) - Workspace member + all 3 dependent Cargo.tomls updated (balius-runtime, balius-sdk, examples/asteria-tracker) - Import paths: balius_proto::cardano -> balius_core::proto::v0::cardano across balius-sdk, balius-runtime, examples/asteria-tracker - Doc/comment references updated No behavior change. cargo check --workspace --tests clean; 2/2 wire-compat tests in balius-core; 9/9 lib tests in balius-runtime (incl. all 8 u5c convert tests); e2e::faucet_claim still passes; only the pre-existing u5c-chainsync::wallet_balance stale-wasm failure remains. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
balius-proto, a Balius-owned protobuf schema (mirrorsutxorpc-spec 0.17.0wire format) that defines the WASM ABI independently of upstream u5c.balius_proto::cardano::*at chainsync ingress and at ledger reads; BigInt → u64/i64 is fallible and halts on overflow.balius-sdkdrops theutxorpc-specdep in favor ofbalius-proto;Utxo<D>::coin()andPParamsaccess become plain integer reads (no BigInt unwrap).utxorpc-spec 0.17pbjson shape via an imperative converter (pparams_to_legacy_json) — workers built against pre-BigInt SDKs decode the bytes unchanged with their own pbjson decoder.balius-runtime/src/ledgers/u5c.rsso the schema crate has no coupling to any upstream service.Why
The runtime was shipping raw
utxorpc::spec::cardano::*prost bytes across the WIT boundary, which implicitly made the upstream u5c spec the WASM ABI. When u5c 0.17 → 0.18.1 flippedTxOutput.coin(and similar fields acrossAsset,Tx.fee,Collateral, certs, governance actions,PParams) fromuint64toBigInt, every existing pre-BigInt worker started decoding garbage. This PR breaks that coupling: workers built against the old SDK keep working because the new bytes are byte-identical to what they expect, and future upstream u5c churn is absorbed by a single hardcoded conversion layer inside the runtime — not inside the schema crate.Architecture
balius-protoowns the wire schema (cardano.rs) and a wire-format invariant test (wire_compat_*: balius_proto bytes decode asutxorpc-spec 0.17prost). No upstream deps.balius-runtime/src/ledgers/u5c.rsowns u5c → balius_proto conversion (convert_tx,convert_tx_output,convert_tx_input,convert_asset,convert_multiasset,pparams_to_legacy_json+ helpers,ConvertError).balius-runtime/tests/u5c_convert.rsasserts the end-to-end ABI contract: bytes/JSON produced by the conversion functions decode under the actualutxorpc-spec 0.17decoder.Notes
balius_proto::cardano::*so old workers decode unchanged. Tags absent from balius_proto (script, certs, withdrawals, mint, reference_inputs, validity, collateral, proposals, ...) are deliberately dropped — workers needing them must re-add them here and update the converter.coinsPerUtxoByte— keep working.BigUInt/BigNIntand out-of-rangeIntproduceConvertError::Overflow. Surfaces asError::Convertin the runtime andwit::LedgerError::Upstreamfor ledger paths — workers halt rather than receive lossy values.balius-protoships pre-generatedcardano.rs(no protoc required at build); its only dep isprost+bytes. Tests have one dev-dep (utxorpc-spec 0.17) used solely to assert wire-format invariance.PParamsis minimal (coins_per_utxo_byteonly — the field actually consumed byMinUtxoLovelace) but declared withserde_with::DisplayFromStr+rename = "coinsPerUtxoByte"so it parses the same pbjson shape old workers expect. Add fields here as needed; serde ignores unknown ones the runtime emits.mock_pparams.jsonis the canonical 0.17 pbjson snapshot, so the mock ledger and the real ledger hand workers identical bytes.Test plan
cargo check --workspaceclean.cargo test -p balius-proto— 2/2 wire-format invariant tests (schema in, 0.17 prost out).cargo test -p balius-runtime --test u5c_convert— 8/8:convert_pparams_decodes_under_017_pbjson— load-bearing PParams ABI assertion (converted JSON deserializes under real 0.17 pbjson decoder).convert_then_decode_with_017— load-bearing Tx/UTxO ABI assertion (converted bytes decode under real 0.17 prost decoder).cargo test --workspace— all pass except the pre-existingu5c-chainsync::wallet_balance(5-vs-7 WIT variant mismatch from a stale checked-inwallet.wasm; verified failing identically onmain).e2e::faucet_claimexercises the full PParams path end-to-end (mock ledger shipsmock_pparams.json→ SDKPParamsdeserializes →MinUtxoLovelacereadscoins_per_utxo_byte→ tx build succeeds).utxo_handled/tx_handledmetrics advance and no decode errors appear in logs.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
balius-corelibrary providing versioned Cardano protocol definitions with improved wire compatibility.Breaking Changes
Block::txsnow returnsResult<Vec<Tx>, Error>instead ofVec<Tx>(transaction conversion may fail).