Skip to content

Conversation

@wolf31o2
Copy link
Member

@wolf31o2 wolf31o2 commented Nov 6, 2025

Add custom CBOR encoding/decoding for ScriptsNotPaidUtxo that matches the Haskell cardano-ledger specification.

Changes

  • Add MarshalCBOR converting slice to map format matching Haskell specification
  • Add UnmarshalCBOR with multi-strategy decoding for complex CBOR map structures
  • Add comprehensive test with mock interfaces for TransactionInput/TransactionOutput
  • Maintain compatibility with existing error handling patterns

CBOR Structure

The implementation uses [constructor_index, utxo_map] format where:

  • Constructor index: 14 (UtxoFailureScriptsNotPaidUtxo)
  • UTxO map: CBOR map with TxIn keys and TxOut values

Testing

Includes round-trip CBOR marshal/unmarshal test that validates the structure matches the Haskell cardano-ledger specification.

Closes #847

Summary by CodeRabbit

  • New Features

    • Error payload now includes a structured list of affected UTxOs and reports the count for clearer diagnostics.
    • Added era‑agnostic on‑wire CBOR serialization/deserialization for the error payload to support both Byron and Shelley formats, with validation and decoding fallbacks.
  • Tests

    • Added CBOR round‑trip tests across eras validating UTxO identity, outputs, cross‑era handling, and type field assignment.

@wolf31o2 wolf31o2 requested a review from a team as a code owner November 6, 2025 23:23
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

Warning

Rate limit exceeded

@wolf31o2 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 26 minutes and 27 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 18ba039 and c7f3323.

📒 Files selected for processing (2)
  • ledger/error.go (4 hunks)
  • ledger/error_test.go (1 hunks)
📝 Walkthrough

Walkthrough

Adds an exported error type ScriptsNotPaidUtxo to package ledger that embeds UtxoFailureErrorBase and replaces a raw CBOR Value with a slice Utxos []common.Utxo. Implements MarshalCBOR and UnmarshalCBOR to perform era-agnostic CBOR encoding/decoding (attempting Shelley-era map decoding, falling back to Byron-era map), with validation for nil Id/Output and constructor index checks. Updates Error() to report the number of UTxOs. Adds tests exercising CBOR round-trip, cross-era (Byron and Shelley) decoding, and ensuring the Type field is set after UnmarshalCBOR.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • ledger/error.go: review era-agnostic MarshalCBOR/UnmarshalCBOR logic, tag/array shape, constructor index checks, nil Id/Output validation, and error message change.
  • ledger/error.go: verify decoding fallback order (Shelley then Byron) and correct reconstruction of common.Utxo fields (Id, Output).
  • ledger/error_test.go: validate test cases for round-trip fidelity, cross-era coverage, and Type field assignment; ensure deterministic CBOR comparisons.

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: implementing CBOR marshal/unmarshal functionality for the ScriptsNotPaidUtxo error type, which matches the primary objective.
Linked Issues check ✅ Passed The PR fully implements the coding requirements from issue #847: defines CBOR structure as [constructor_index, utxo_map], sets constructor_index to 14, implements MarshalCBOR and UnmarshalCBOR with era-agnostic decoding, and includes comprehensive round-trip tests.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing CBOR marshal/unmarshal for ScriptsNotPaidUtxo. The type definition, methods, and tests are all necessary and in-scope for the stated objective.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@wolf31o2 wolf31o2 force-pushed the feat/scripts-not-paid-utxo-cbor branch from 57bfd22 to bbea07c Compare November 6, 2025 23:27
@wolf31o2
Copy link
Member Author

wolf31o2 commented Nov 6, 2025

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
ledger/error_test.go (2)

44-57: Mock type lacks CBOR marshaling implementation.

The mock doesn't implement MarshalCBOR/UnmarshalCBOR methods. When ScriptsNotPaidUtxo.MarshalCBOR() encodes a map with this type as a key, the CBOR encoder will use default struct encoding, which may not match the expected Cardano TxIn format.

Consider implementing CBOR methods on the mock to match the real TxIn encoding, or use actual concrete types from the codebase that already implement proper CBOR serialization.

</invoke_comment_end -->


59-85: Mock type lacks CBOR marshaling implementation.

Similar to mockTxInput, this mock doesn't implement MarshalCBOR/UnmarshalCBOR. The default CBOR encoding may not match the expected Cardano TxOut format.

</invoke_comment_end -->

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 39d18fd and bbea07c.

📒 Files selected for processing (2)
  • ledger/error.go (2 hunks)
  • ledger/error_test.go (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: wolf31o2
Repo: blinklabs-io/gouroboros PR: 1093
File: ledger/mary/pparams.go:143-150
Timestamp: 2025-11-04T15:54:22.683Z
Learning: In the blinklabs-io/gouroboros repository, the design goal for CBOR round-trip tests is to achieve byte-identical encoding WITHOUT using stored CBOR (cbor.DecodeStoreCbor). Instead, the approach uses proper field types (pointers for optional fields) and relies on the cbor package's deterministic encoding (SortCoreDeterministic) to ensure reproducible output. The stored CBOR pattern should not be suggested as a solution for round-trip fidelity in this codebase.
📚 Learning: 2025-11-04T15:54:22.683Z
Learnt from: wolf31o2
Repo: blinklabs-io/gouroboros PR: 1093
File: ledger/mary/pparams.go:143-150
Timestamp: 2025-11-04T15:54:22.683Z
Learning: In the blinklabs-io/gouroboros repository, the design goal for CBOR round-trip tests is to achieve byte-identical encoding WITHOUT using stored CBOR (cbor.DecodeStoreCbor). Instead, the approach uses proper field types (pointers for optional fields) and relies on the cbor package's deterministic encoding (SortCoreDeterministic) to ensure reproducible output. The stored CBOR pattern should not be suggested as a solution for round-trip fidelity in this codebase.

Applied to files:

  • ledger/error_test.go
🔇 Additional comments (1)
ledger/error.go (1)

538-544: Well-documented type definition.

The comments clearly document the Haskell equivalent and CBOR format, making it easy to understand the expected structure.

</invoke_comment_end -->

@wolf31o2 wolf31o2 force-pushed the feat/scripts-not-paid-utxo-cbor branch 2 times, most recently from 1bf9acc to 5efe84f Compare November 7, 2025 00:05
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
ledger/error.go (1)

562-623: Critical: UnmarshalCBOR still loses all UTxO data, breaking round-trip serialization.

This issue was previously flagged but remains unresolved. The unmarshaling implementation creates placeholder Utxo structs with nil Id and Output fields (lines 594-596, 611-613) instead of decoding the actual transaction input/output data.

Impact:

  1. Complete data loss: All TxIn/TxOut information is discarded; only the count is preserved
  2. Broken round-trip: marshal(unmarshal(bytes)) produces different output than the input
  3. Unusable data: Downstream code accessing decoded.Utxos[0].Id will get nil
  4. Violates design goal: Per retrieved learning, this codebase aims for byte-identical CBOR round-trips without stored CBOR

Root cause:
Decoding to map[any]any gives untyped data. You need to decode to the concrete types that implement the TransactionInput and TransactionOutput interfaces.

Recommended fix:

#!/bin/bash
# Find concrete types implementing TransactionInput/TransactionOutput used in tests
echo "=== Find concrete TxIn/TxOut types used in the codebase ==="
rg -n "type.*struct" --type go -C 5 ledger/ | rg -B 3 -A 3 "TransactionInput|TransactionOutput"

echo -e "\n=== Check test file for concrete types used ==="
rg -n "ScriptsNotPaidUtxo" --type go -C 10 ledger/

Decode each map entry to the concrete types (e.g., shelley.TransactionInput, shelley.TransactionOutput) instead of creating nil placeholders. Example approach:

 	// Attempt direct decoding
-	var directMap map[any]any
+	// Decode to concrete types - adjust based on which era types are used
+	type txInType struct {
+		// Define or import the concrete TxIn struct
+	}
+	type txOutType struct {
+		// Define or import the concrete TxOut struct  
+	}
+	var directMap map[txInType]txOutType
 	if _, err := cbor.Decode(tmpData.UtxoMapCbor, &directMap); err == nil {
 		e.Utxos = make([]common.Utxo, 0, len(directMap))
-		for range directMap {
-			// Create placeholder entries that preserve structure count
-			// Note: Full decoding of complex interface types requires
-			// specialized handling to avoid circular encoding issues
+		for k, v := range directMap {
 			e.Utxos = append(e.Utxos, common.Utxo{
-				Id:     nil, // Placeholder - actual decoding would handle type casting
-				Output: nil, // Placeholder - actual decoding would handle type casting
+				Id:     &k,  // or appropriate conversion to interface
+				Output: &v,  // or appropriate conversion to interface
 			})
 		}
 		return nil
 	}

If proper decoding isn't feasible in this PR, at minimum:

  1. Document that UnmarshalCBOR only preserves count, not data
  2. Update struct/method comments to warn that Utxos will contain nil entries
  3. Consider storing raw CBOR for future processing
🧹 Nitpick comments (1)
ledger/error.go (1)

546-560: Consider returning an error for nil UTxO entries instead of silently skipping.

The nil checks on lines 552-555 prevent panics, which is good. However, silently skipping nil entries could hide bugs and cause confusion when len(e.Utxos) doesn't match the number of entries in the marshaled CBOR map.

Consider this alternative approach:

 func (e *ScriptsNotPaidUtxo) MarshalCBOR() ([]byte, error) {
 	utxoMap := make(
 		map[common.TransactionInput]common.TransactionOutput,
 		len(e.Utxos),
 	)
 	for _, u := range e.Utxos {
-		// Skip entries with nil Id or Output to avoid dereference panic
 		if u.Id == nil || u.Output == nil {
-			continue
+			return nil, fmt.Errorf("ScriptsNotPaidUtxo: cannot marshal UTxO with nil Id or Output")
 		}
 		utxoMap[u.Id] = u.Output
 	}
 	arr := []any{UtxoFailureScriptsNotPaidUtxo, utxoMap}
 	return cbor.Encode(arr)
 }

Alternatively, if skipping is the intended behavior, document this in the method comment to clarify that nil entries are excluded from the CBOR output.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bbea07c and 5efe84f.

📒 Files selected for processing (2)
  • ledger/error.go (2 hunks)
  • ledger/error_test.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • ledger/error_test.go
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: wolf31o2
Repo: blinklabs-io/gouroboros PR: 1093
File: ledger/mary/pparams.go:143-150
Timestamp: 2025-11-04T15:54:22.683Z
Learning: In the blinklabs-io/gouroboros repository, the design goal for CBOR round-trip tests is to achieve byte-identical encoding WITHOUT using stored CBOR (cbor.DecodeStoreCbor). Instead, the approach uses proper field types (pointers for optional fields) and relies on the cbor package's deterministic encoding (SortCoreDeterministic) to ensure reproducible output. The stored CBOR pattern should not be suggested as a solution for round-trip fidelity in this codebase.
📚 Learning: 2025-11-04T15:54:22.683Z
Learnt from: wolf31o2
Repo: blinklabs-io/gouroboros PR: 1093
File: ledger/mary/pparams.go:143-150
Timestamp: 2025-11-04T15:54:22.683Z
Learning: In the blinklabs-io/gouroboros repository, the design goal for CBOR round-trip tests is to achieve byte-identical encoding WITHOUT using stored CBOR (cbor.DecodeStoreCbor). Instead, the approach uses proper field types (pointers for optional fields) and relies on the cbor package's deterministic encoding (SortCoreDeterministic) to ensure reproducible output. The stored CBOR pattern should not be suggested as a solution for round-trip fidelity in this codebase.

Applied to files:

  • ledger/error.go
🧬 Code graph analysis (1)
ledger/error.go (6)
ledger/common/tx.go (3)
  • Utxo (110-114)
  • TransactionInput (64-70)
  • TransactionOutput (72-83)
ledger/tx.go (3)
  • Utxo (30-30)
  • TransactionInput (28-28)
  • TransactionOutput (29-29)
cbor/encode.go (1)
  • Encode (27-40)
cbor/cbor.go (2)
  • StructAsArray (45-48)
  • RawMessage (36-36)
cbor/decode.go (1)
  • Decode (29-44)
cbor/value.go (1)
  • Value (29-33)
⏰ 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: Analyze (go)
🔇 Additional comments (2)
ledger/error.go (2)

538-544: LGTM: Type definition aligns with Cardano ledger specification.

The struct definition correctly represents the ScriptsNotPaidUTxO error with a slice of common.Utxo entries. The documentation clearly references the Haskell specification and CBOR encoding format.


625-627: LGTM: Error message formatting is appropriate.

The error message correctly reports the count of UTxOs. Note that the usefulness of this count depends on the UnmarshalCBOR implementation properly populating the Utxos slice with actual data rather than nil placeholders.

@wolf31o2 wolf31o2 force-pushed the feat/scripts-not-paid-utxo-cbor branch 2 times, most recently from 1396912 to f9d8ec0 Compare November 7, 2025 02:59
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
ledger/error_test.go (1)

40-48: Test still skips round-trip fidelity checks.
The test continues to assert only the UTxO count, so data-loss bugs (like the loop-variable pointer leak below) slip through unnoticed. Please validate every decoded TxIn/TxOut and ensure the re-marshaled bytes match the original payload—ideally with more than one UTxO so per-entry corruption is caught.

@@
-import (
-	"testing"
-
-	"github.com/blinklabs-io/gouroboros/ledger/common"
-	"github.com/blinklabs-io/gouroboros/ledger/shelley"
-)
+import (
+	"bytes"
+	"testing"
+
+	"github.com/blinklabs-io/gouroboros/ledger/common"
+	"github.com/blinklabs-io/gouroboros/ledger/shelley"
+)
@@
-	utxo := common.Utxo{
-		Id:     input,
-		Output: output,
-	}
+	utxo1 := common.Utxo{Id: input, Output: output}
+	utxo2 := common.Utxo{
+		Id: shelley.NewShelleyTransactionInput(
+			"deadbeef00000000000000000000000000000000000000000000000000000001",
+			1,
+		),
+		Output: &shelley.ShelleyTransactionOutput{
+			OutputAddress: addr,
+			OutputAmount:  2000,
+		},
+	}
@@
-	errVal := &ScriptsNotPaidUtxo{
-		Utxos: []common.Utxo{utxo},
-	}
+	errVal := &ScriptsNotPaidUtxo{
+		Utxos: []common.Utxo{utxo1, utxo2},
+	}
@@
-	if len(decoded.Utxos) != 1 {
-		t.Fatalf("Expected 1 Utxo, got %d", len(decoded.Utxos))
-	}
-	// For now, just verify we have the expected number of UTxOs
-	// Full round-trip testing would require more complex CBOR map handling
-	t.Logf(
-		"Successfully marshaled and unmarshaled ScriptsNotPaidUtxo with %d UTxOs",
-		len(decoded.Utxos),
-	)
+	if len(decoded.Utxos) != len(errVal.Utxos) {
+		t.Fatalf("Expected %d UTxOs, got %d", len(errVal.Utxos), len(decoded.Utxos))
+	}
+	for i := range errVal.Utxos {
+		got := decoded.Utxos[i]
+		want := errVal.Utxos[i]
+		if got.Id.String() != want.Id.String() {
+			t.Fatalf("TxIn[%d] mismatch: want %s, got %s", i, want.Id.String(), got.Id.String())
+		}
+		if got.Output.Amount() != want.Output.Amount() ||
+			got.Output.Address().String() != want.Output.Address().String() {
+			t.Fatalf("TxOut[%d] mismatch after round-trip", i)
+		}
+	}
+
+	remarshaled, err := decoded.MarshalCBOR()
+	if err != nil {
+		t.Fatalf("MarshalCBOR after decode failed: %v", err)
+	}
+	if !bytes.Equal(cborData, remarshaled) {
+		t.Fatalf("CBOR mismatch after round-trip")
+	}

Based on learnings.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5efe84f and f9d8ec0.

📒 Files selected for processing (2)
  • ledger/error.go (2 hunks)
  • ledger/error_test.go (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: wolf31o2
Repo: blinklabs-io/gouroboros PR: 1093
File: ledger/mary/pparams.go:143-150
Timestamp: 2025-11-04T15:54:22.683Z
Learning: In the blinklabs-io/gouroboros repository, the design goal for CBOR round-trip tests is to achieve byte-identical encoding WITHOUT using stored CBOR (cbor.DecodeStoreCbor). Instead, the approach uses proper field types (pointers for optional fields) and relies on the cbor package's deterministic encoding (SortCoreDeterministic) to ensure reproducible output. The stored CBOR pattern should not be suggested as a solution for round-trip fidelity in this codebase.
📚 Learning: 2025-11-04T15:54:22.683Z
Learnt from: wolf31o2
Repo: blinklabs-io/gouroboros PR: 1093
File: ledger/mary/pparams.go:143-150
Timestamp: 2025-11-04T15:54:22.683Z
Learning: In the blinklabs-io/gouroboros repository, the design goal for CBOR round-trip tests is to achieve byte-identical encoding WITHOUT using stored CBOR (cbor.DecodeStoreCbor). Instead, the approach uses proper field types (pointers for optional fields) and relies on the cbor package's deterministic encoding (SortCoreDeterministic) to ensure reproducible output. The stored CBOR pattern should not be suggested as a solution for round-trip fidelity in this codebase.

Applied to files:

  • ledger/error_test.go
  • ledger/error.go
🧬 Code graph analysis (2)
ledger/error_test.go (1)
ledger/error.go (1)
  • ScriptsNotPaidUtxo (541-544)
ledger/error.go (6)
ledger/common/tx.go (3)
  • Utxo (110-114)
  • TransactionInput (64-70)
  • TransactionOutput (72-83)
ledger/tx.go (3)
  • Utxo (30-30)
  • TransactionInput (28-28)
  • TransactionOutput (29-29)
cbor/encode.go (1)
  • Encode (27-40)
cbor/cbor.go (1)
  • StructAsArray (45-48)
ledger/shelley/shelley.go (2)
  • ShelleyTransactionInput (337-341)
  • ShelleyTransactionOutput (388-393)
cbor/decode.go (1)
  • Decode (29-44)

@wolf31o2 wolf31o2 force-pushed the feat/scripts-not-paid-utxo-cbor branch from f9d8ec0 to 77856ab Compare November 7, 2025 03:09
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f9d8ec0 and 77856ab.

📒 Files selected for processing (2)
  • ledger/error.go (2 hunks)
  • ledger/error_test.go (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: wolf31o2
Repo: blinklabs-io/gouroboros PR: 1093
File: ledger/mary/pparams.go:143-150
Timestamp: 2025-11-04T15:54:22.683Z
Learning: In the blinklabs-io/gouroboros repository, the design goal for CBOR round-trip tests is to achieve byte-identical encoding WITHOUT using stored CBOR (cbor.DecodeStoreCbor). Instead, the approach uses proper field types (pointers for optional fields) and relies on the cbor package's deterministic encoding (SortCoreDeterministic) to ensure reproducible output. The stored CBOR pattern should not be suggested as a solution for round-trip fidelity in this codebase.
📚 Learning: 2025-11-04T15:54:22.683Z
Learnt from: wolf31o2
Repo: blinklabs-io/gouroboros PR: 1093
File: ledger/mary/pparams.go:143-150
Timestamp: 2025-11-04T15:54:22.683Z
Learning: In the blinklabs-io/gouroboros repository, the design goal for CBOR round-trip tests is to achieve byte-identical encoding WITHOUT using stored CBOR (cbor.DecodeStoreCbor). Instead, the approach uses proper field types (pointers for optional fields) and relies on the cbor package's deterministic encoding (SortCoreDeterministic) to ensure reproducible output. The stored CBOR pattern should not be suggested as a solution for round-trip fidelity in this codebase.

Applied to files:

  • ledger/error_test.go
  • ledger/error.go
🧬 Code graph analysis (2)
ledger/error_test.go (1)
ledger/error.go (1)
  • ScriptsNotPaidUtxo (541-544)
ledger/error.go (6)
ledger/common/tx.go (3)
  • Utxo (110-114)
  • TransactionInput (64-70)
  • TransactionOutput (72-83)
ledger/tx.go (3)
  • Utxo (30-30)
  • TransactionInput (28-28)
  • TransactionOutput (29-29)
cbor/encode.go (1)
  • Encode (27-40)
cbor/cbor.go (1)
  • StructAsArray (45-48)
ledger/shelley/shelley.go (2)
  • ShelleyTransactionInput (337-341)
  • ShelleyTransactionOutput (388-393)
cbor/decode.go (1)
  • Decode (29-44)
⏰ 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: Analyze (go)

@wolf31o2 wolf31o2 force-pushed the feat/scripts-not-paid-utxo-cbor branch from 77856ab to 9962db5 Compare November 7, 2025 04:07
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
ledger/error.go (1)

548-562: Keep the embedded Type field in sync during marshal.

Minor, but for symmetry with the decode fix and other UtxoFailure types it’s worth assigning e.Type = UtxoFailureScriptsNotPaidUtxo before encoding. That way the struct always advertises the right tag even if reused elsewhere.

 func (e *ScriptsNotPaidUtxo) MarshalCBOR() ([]byte, error) {
+	e.Type = UtxoFailureScriptsNotPaidUtxo
 	utxoMap := make(
 		map[common.TransactionInput]common.TransactionOutput,
 		len(e.Utxos),
 	)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 77856ab and 9962db5.

📒 Files selected for processing (2)
  • ledger/error.go (2 hunks)
  • ledger/error_test.go (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: wolf31o2
Repo: blinklabs-io/gouroboros PR: 1093
File: ledger/mary/pparams.go:143-150
Timestamp: 2025-11-04T15:54:22.683Z
Learning: In the blinklabs-io/gouroboros repository, the design goal for CBOR round-trip tests is to achieve byte-identical encoding WITHOUT using stored CBOR (cbor.DecodeStoreCbor). Instead, the approach uses proper field types (pointers for optional fields) and relies on the cbor package's deterministic encoding (SortCoreDeterministic) to ensure reproducible output. The stored CBOR pattern should not be suggested as a solution for round-trip fidelity in this codebase.
📚 Learning: 2025-11-04T15:54:22.683Z
Learnt from: wolf31o2
Repo: blinklabs-io/gouroboros PR: 1093
File: ledger/mary/pparams.go:143-150
Timestamp: 2025-11-04T15:54:22.683Z
Learning: In the blinklabs-io/gouroboros repository, the design goal for CBOR round-trip tests is to achieve byte-identical encoding WITHOUT using stored CBOR (cbor.DecodeStoreCbor). Instead, the approach uses proper field types (pointers for optional fields) and relies on the cbor package's deterministic encoding (SortCoreDeterministic) to ensure reproducible output. The stored CBOR pattern should not be suggested as a solution for round-trip fidelity in this codebase.

Applied to files:

  • ledger/error_test.go
  • ledger/error.go
🧬 Code graph analysis (2)
ledger/error_test.go (1)
ledger/error.go (1)
  • ScriptsNotPaidUtxo (543-546)
ledger/error.go (9)
ledger/common/tx.go (3)
  • Utxo (110-114)
  • TransactionInput (64-70)
  • TransactionOutput (72-83)
ledger/tx.go (4)
  • Utxo (30-30)
  • TransactionInput (28-28)
  • TransactionOutput (29-29)
  • NewTransactionOutputFromCbor (82-99)
cbor/encode.go (1)
  • Encode (27-40)
cbor/cbor.go (2)
  • StructAsArray (45-48)
  • RawMessage (36-36)
cbor/decode.go (1)
  • Decode (29-44)
ledger/shelley/shelley.go (1)
  • ShelleyTransactionInput (337-341)
ledger/shelley.go (1)
  • ShelleyTransactionInput (28-28)
ledger/byron/byron.go (1)
  • ByronTransactionInput (327-331)
ledger/byron.go (1)
  • ByronTransactionInput (29-29)
⏰ 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: Analyze (go)

@wolf31o2 wolf31o2 force-pushed the feat/scripts-not-paid-utxo-cbor branch from 9962db5 to 13ab1a7 Compare November 7, 2025 04:20
@wolf31o2
Copy link
Member Author

wolf31o2 commented Nov 7, 2025

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 7, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
ledger/error_test.go (1)

171-255: Strengthen Byron-era assertions
Since this suite guards the multi-era decode, consider mirroring the Shelley test by checking the decoded Byron outputs (address/amount) and re-marshaled bytes. It would catch regressions where Byron paths decode but lose data fidelity.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9962db5 and 13ab1a7.

📒 Files selected for processing (2)
  • ledger/error.go (2 hunks)
  • ledger/error_test.go (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: wolf31o2
Repo: blinklabs-io/gouroboros PR: 1093
File: ledger/mary/pparams.go:143-150
Timestamp: 2025-11-04T15:54:22.683Z
Learning: In the blinklabs-io/gouroboros repository, the design goal for CBOR round-trip tests is to achieve byte-identical encoding WITHOUT using stored CBOR (cbor.DecodeStoreCbor). Instead, the approach uses proper field types (pointers for optional fields) and relies on the cbor package's deterministic encoding (SortCoreDeterministic) to ensure reproducible output. The stored CBOR pattern should not be suggested as a solution for round-trip fidelity in this codebase.
📚 Learning: 2025-11-04T15:54:22.683Z
Learnt from: wolf31o2
Repo: blinklabs-io/gouroboros PR: 1093
File: ledger/mary/pparams.go:143-150
Timestamp: 2025-11-04T15:54:22.683Z
Learning: In the blinklabs-io/gouroboros repository, the design goal for CBOR round-trip tests is to achieve byte-identical encoding WITHOUT using stored CBOR (cbor.DecodeStoreCbor). Instead, the approach uses proper field types (pointers for optional fields) and relies on the cbor package's deterministic encoding (SortCoreDeterministic) to ensure reproducible output. The stored CBOR pattern should not be suggested as a solution for round-trip fidelity in this codebase.

Applied to files:

  • ledger/error_test.go
  • ledger/error.go
🔇 Additional comments (1)
ledger/error.go (1)

548-641: Robust multi-era decode path
The guardrails on nil inputs plus the Shelley→Byron fallback keep the CBOR flow sound and match the ledger constructor tag. Nice work lining it up with the spec.

@wolf31o2 wolf31o2 force-pushed the feat/scripts-not-paid-utxo-cbor branch from 13ab1a7 to 18ba039 Compare November 7, 2025 14:53
Add custom CBOR encoding/decoding for ScriptsNotPaidUtxo that matches
the Haskell cardano-ledger specification. The implementation uses
`[constructor_index, utxo_map]` format with multi-strategy decoding
to handle complex CBOR map structures.

- Add `MarshalCBOR` converting slice to map format
- Add `UnmarshalCBOR` with fallback decoding strategies
- Add comprehensive test with mock interfaces
- Maintain compatibility with existing error handling

Signed-off-by: Chris Gianelloni <[email protected]>
Co-authored-by: GitHub Copilot <[email protected]>
@wolf31o2 wolf31o2 force-pushed the feat/scripts-not-paid-utxo-cbor branch from 18ba039 to c7f3323 Compare November 7, 2025 17:13
@wolf31o2
Copy link
Member Author

wolf31o2 commented Nov 7, 2025

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 7, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@wolf31o2 wolf31o2 merged commit bd49806 into main Nov 7, 2025
10 checks passed
@wolf31o2 wolf31o2 deleted the feat/scripts-not-paid-utxo-cbor branch November 7, 2025 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Determine CBOR structure of ScriptsNotPaidUtxo error

4 participants