feat: Support proper encoding/decoding of TX metadata #1159
Conversation
agaffney
left a comment
agaffney
left a comment
There was a problem hiding this comment.
We'll need something like a TransactionMetadatumWrapper type, which can be used in a Transaction type and call the decode functions, similar to what you're doing in TransactionMetadataSet.
ledger/common/tx.go
Outdated
| Type() int | ||
| Cbor() []byte | ||
| Metadata() *cbor.LazyValue | ||
| Metadata() TransactionMetadataSet |
There was a problem hiding this comment.
We don't want to use TransactionMetadataSet here, only TransactionMetadatum. The Set is used at the block level to return the metadatum for all TXs included in the block.
ledger/common/tx.go
Outdated
| type TransactionMetadataSet map[uint64]TransactionMetadatum | ||
|
|
||
| type TransactionMetadatum interface { | ||
| TypeName() string |
There was a problem hiding this comment.
Add a isTransactionMetadatum() function here, so that other types can't accidentally satisfy this interface.
ledger/conway/conway.go
Outdated
| TransactionBodies []ConwayTransactionBody | ||
| TransactionWitnessSets []ConwayTransactionWitnessSet | ||
| TransactionMetadataSet map[uint]*cbor.LazyValue | ||
| TransactionMetadataSet map[uint]common.TransactionMetadataSet |
There was a problem hiding this comment.
This should be TransactionMetadataSet TransactionMetadataSet. What you've got here will give you nested maps, which isn't what we want
There was a problem hiding this comment.
Do you mean TransactionMetadataSet.TransactionMetadataSet?
There was a problem hiding this comment.
Err, I meant TransactionMetadataSet common.TransactionMetadataSet
|
Warning Rate limit exceeded@arepala-uml has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 20 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR replaces uses of raw CBOR lazy values for transaction metadata with a typed metadata model in ledger/common. It introduces TransactionMetadatum types (MetaInt, MetaBytes, MetaText, MetaList, MetaPair, MetaMap), TransactionMetadataSet with UnmarshalCBOR/MarshalCBOR, and DecodeMetadatumRaw. The Transaction interface now returns TransactionMetadatum and all era implementations (Byron, Shelley, Mary, Allegra, Alonzo, Babbage, Conway) were updated to use the new types and to embed metadata values directly in CBOR output. Adds metadata round-trip tests and minor test adjustment in Allegra. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Files/areas to focus on:
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
ledger/babbage/babbage.go (2)
353-359: Bug: address of range variable yields duplicate pointersUsing &input appends the address of the loop variable, so all entries point to the same memory (the last element). Return the value, not its address.
func (b *BabbageTransactionBody) ReferenceInputs() []common.TransactionInput { ret := []common.TransactionInput{} for _, input := range b.TxReferenceInputs.Items() { - ret = append(ret, &input) + ret = append(ret, input) } return ret }
593-599: Bug: leaking zero hash when datum is absentReturning a pointer to an empty Blake2b256 causes RPC encoding to emit 32 zero bytes instead of “no datum”. Return nil when absent.
func (o BabbageTransactionOutput) DatumHash() *common.Blake2b256 { - if o.DatumOption != nil { - return o.DatumOption.hash - } - return &common.Blake2b256{} + if o.DatumOption != nil && o.DatumOption.hash != nil { + return o.DatumOption.hash + } + return nil }ledger/conway/conway.go (1)
468-474: Bug: address of range variable yields duplicate pointersSame issue as Babbage: &input captures the loop variable address; all entries will be the same. Append the value.
func (b *ConwayTransactionBody) ReferenceInputs() []common.TransactionInput { ret := []common.TransactionInput{} for _, input := range b.TxReferenceInputs.Items() { - ret = append(ret, &input) + ret = append(ret, input) } return ret }
♻️ Duplicate comments (2)
ledger/allegra/block_test.go (1)
63-76: Reinstate exact CBOR byte-for-byte round‑trip checkWe should keep the strict equality check in addition to the structural assertions, as previously requested. If it fails, that indicates an encoding divergence we must fix rather than relaxing the test.
Apply this diff right after encoding:
@@ if encoded == nil || len(encoded) == 0 { t.Fatal("Custom encoded CBOR from AllegraBlock is nil or empty") } - // Ensure the re-encoded CBOR is structurally valid and decodes back + // Exact byte-for-byte match with original CBOR + assert.Equal(t, dataBytes, encoded, "Re-encoded CBOR must match original CBOR") + + // Ensure the re-encoded CBOR is structurally valid and decodes back var redecoded allegra.AllegraBlock if err := redecoded.UnmarshalCBOR(encoded); err != nil { t.Fatalf("Re-encoded AllegraBlock failed to decode: %v", err) }ledger/common/tx.go (1)
34-35: Seal theTransactionMetadatuminterface.Add an unexported marker (e.g.,
isTransactionMetadatum()) so unrelated types can’t accidentally satisfy the interface. This was requested previously and is still applicable.Apply this diff:
type TransactionMetadatum interface { - TypeName() string + isTransactionMetadatum() + TypeName() string } type MetaInt struct { Value int64 } +func (MetaInt) isTransactionMetadatum() {} type MetaBytes struct { Value []byte } +func (MetaBytes) isTransactionMetadatum() {} type MetaText struct { Value string } +func (MetaText) isTransactionMetadatum() {} type MetaList struct { Items []TransactionMetadatum } +func (MetaList) isTransactionMetadatum() {} type MetaPair struct { Key TransactionMetadatum Value TransactionMetadatum } type MetaMap struct { Pairs []MetaPair } +func (MetaMap) isTransactionMetadatum() {}
🧹 Nitpick comments (4)
ledger/conway/conway.go (1)
58-60: Metadata model update acknowledged; ensure CBOR optional field encodingThe switch to common.TransactionMetadataSet and appending the value directly is good. As with other eras, please verify whether the 4th element should be omitted when metadata is absent instead of appending nil, to preserve canonical CBOR and hashes.
@@ func (t *ConwayTransaction) Cbor() []byte { - if t.TxMetadata != nil { - tmpObj = append(tmpObj, t.TxMetadata) - } else { - tmpObj = append(tmpObj, nil) - } + if t.TxMetadata != nil { + tmpObj = append(tmpObj, t.TxMetadata) + }Also applies to: 521-522, 635-637, 701-703
ledger/shelley/shelley.go (1)
56-56: Type alignment looks good; consider standardizing key width.Using
map[uint]common.TransactionMetadataSetworks, but elsewhere labels useuint64. Consider switching tomap[uint64]...for consistency and to avoid platform‑dependentuintwidth.ledger/allegra/allegra.go (1)
52-53: Type alignment looks good; consider standardizing key width.
map[uint]common.TransactionMetadataSetworks; aligning onuint64across packages would remove platform‐dependent width.ledger/mary/mary.go (1)
55-56: Type alignment looks good; consider key width consistency.As with other eras, consider
map[uint64]...for cross‑platform consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
ledger/allegra/allegra.go(4 hunks)ledger/allegra/block_test.go(1 hunks)ledger/alonzo/alonzo.go(4 hunks)ledger/babbage/babbage.go(4 hunks)ledger/byron/byron.go(2 hunks)ledger/common/tx.go(3 hunks)ledger/conway/conway.go(4 hunks)ledger/mary/mary.go(4 hunks)ledger/shelley/shelley.go(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
ledger/byron/byron.go (1)
ledger/common/tx.go (1)
TransactionMetadataSet(41-41)
ledger/conway/conway.go (2)
ledger/common/tx.go (1)
TransactionMetadataSet(41-41)ledger/conway.go (1)
ConwayTransaction(26-26)
ledger/common/tx.go (3)
cbor/decode.go (1)
Decode(29-44)cbor/cbor.go (1)
RawMessage(36-36)cbor/encode.go (1)
Encode(27-40)
ledger/allegra/block_test.go (1)
ledger/allegra/allegra.go (2)
AllegraBlock(46-53)AllegraBlock(66-68)
ledger/mary/mary.go (1)
ledger/common/tx.go (1)
TransactionMetadataSet(41-41)
ledger/shelley/shelley.go (2)
ledger/common/tx.go (1)
TransactionMetadataSet(41-41)ledger/shelley.go (1)
ShelleyTransaction(26-26)
ledger/alonzo/alonzo.go (1)
ledger/common/tx.go (1)
TransactionMetadataSet(41-41)
ledger/allegra/allegra.go (2)
ledger/common/tx.go (1)
TransactionMetadataSet(41-41)ledger/allegra.go (1)
AllegraTransaction(25-25)
ledger/babbage/babbage.go (1)
ledger/common/tx.go (1)
TransactionMetadataSet(41-41)
🔇 Additional comments (12)
ledger/alonzo/alonzo.go (1)
58-60: Verify canonical metadata encoding against Cardano specificationThe review comment assumes "canonical transaction encoding" should omit nil metadata rather than append it. However, CBOR's canonical/deterministic rules (RFC 8949) do not mandate omitting or appending (encoding) absent/null values — that is an application- or codec-level decision.
The current implementation (appending nil) is systematic across all era files (shelley, mary, conway, babbage, alonzo, allegra at lines 714, 416, 704, 919, 770, 416 respectively), suggesting this is an intentional design choice rather than an oversight. Before refactoring, confirm that Cardano's canonical transaction encoding specification actually requires omission over nil appending. Changing the serialization format could break compatibility with existing encoded transactions if the current behavior is what the protocol expects.
ledger/babbage/babbage.go (1)
58-60: Fix CBOR encoding to omit optional metadata field instead of appending nullThe current implementation at line 919 appends
nilwhen metadata is absent, but Cardano requires omitting optional metadata fields rather than encoding them as null. This breaks canonical CBOR encoding. Apply the proposed fix:if t.TxMetadata != nil { tmpObj = append(tmpObj, t.TxMetadata) -} else { - tmpObj = append(tmpObj, nil) }Likely an incorrect or invalid review comment.
ledger/byron/byron.go (1)
145-146: Remove LazyValue concern; verify semantic mapping with Byron test dataThe LazyValue semantics concern is unfounded—ByronTransaction.Attributes is directly typed as common.TransactionMetadataSet, not lazily evaluated. However, the core concern remains valid: there is no decode/round-trip test to verify that Byron transaction attributes correctly map to the TransactionMetadataSet semantics expected by the common Transaction interface. Byron's historical "attributes" field may have different constraints than Shelley-era aux metadata, and this mapping should be validated with real Byron on-chain data.
ledger/shelley/shelley.go (3)
543-544: Good: switch from LazyValue to typed metadata set.Replacing
*cbor.LazyValuewithcommon.TransactionMetadataSetimproves type safety and enables proper encode/decode.
653-655: Accessor is consistent with new interface.Returning
common.TransactionMetadataSetmatches the updatedcommon.Transactioninterface.
711-713: CBOR path: appending the typed metadata is correct.Appending
t.TxMetadatarelies on itsMarshalCBOR()— this is the right direction and avoids double-encoding. No issues here.ledger/allegra/allegra.go (3)
242-243: Typed metadata set adoption LGTM.
352-354: Accessor matches interface change.
413-415: CBOR encoding path is correct.Using the value directly ensures
MarshalCBOR()is honored.ledger/mary/mary.go (3)
250-251: Typed metadata set adoption LGTM.
364-366: Accessor matches interface change.
413-415: CBOR encoding path is correct.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
ledger/alonzo/alonzo.go (1)
763-769: Metadata CBOR encoding regressedAppending
t.TxMetadata(aMeta*struct) directly intotmpObjmeans the CBOR encoder now serializes the Go struct itself (producing a map with a"Value"field) instead of the canonical metadata item we used to append viaRawMessage. This breaks round-trips and yields wire formats that no longer match the on-chain CDDL. Please restore proper encoding by marshaling the metadatum to its raw CBOR form (e.g., via a dedicatedMarshalCBORon the metadata types or a helper that converts to the underlying CBOR primitives) before appending.ledger/shelley/shelley.go (1)
708-713: Metadata CBOR encoding regressed
ShelleyTransaction.Cbor()now feeds theMeta*structs straight into the encoder, so the wire output becomes a struct-encoded map (with"Value"keys, etc.) rather than the intended metadata CBOR. Please marshal the metadata to its canonical CBOR representation (or wrap it as aRawMessage) before appending so encoding stays compliant.ledger/allegra/allegra.go (1)
409-415: Metadata CBOR encoding regressedHere too we append the metadata interface value directly, so CBOR encoding emits Go-struct semantics instead of the metadata item defined in the CDDL. Please marshal the metadatum to raw CBOR (or convert it to primitive CBOR values) before appending to keep Allegra transactions encoding correctly.
ledger/mary/mary.go (1)
409-415: Metadata CBOR encoding regressedBy passing the
Meta*struct straight tocbor.Encode, we emit struct-shaped CBOR rather than the intended metadata payload. Please marshal the metadata value to its canonical CBOR (or wrap it in aRawMessage) before appending so Mary transactions remain wire-compatible.
🧹 Nitpick comments (1)
ledger/common/metadata_test.go (1)
12-32: Round-trip test validates CBOR fidelity correctly.The test properly validates byte-identical encoding without relying on stored CBOR, consistent with the repository's design goals. Using real on-chain metadata is excellent for ensuring spec compliance.
Consider adding more test cases to cover diverse metadata structures.
The PR objectives mention "decoding tests using metadata extracted from several on-chain transactions." Consider adding test cases that cover:
- Different metadata types (int, bytes, text, lists, maps)
- Nested structures
- Edge cases (empty maps, single-element lists, etc.)
- Samples from other eras (Shelley, Mary, Alonzo, Babbage, Conway)
Example structure:
func Test_Metadata_RoundTrip_Samples(t *testing.T) { testCases := []struct{ name string hex string }{ {"AllegraSample", allegraBlockHex}, {"ShellySample", shelleyBlockHex}, // ... more cases } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { // ... test logic }) } }Optionally verify decoded structure in addition to round-trip.
While byte-identical round-trips prove encoding correctness, you could also add assertions on the decoded structure to document expected metadata shapes:
// After decode if len(set) == 0 { t.Fatal("expected non-empty metadata set") } // Verify specific labels/types are present if _, ok := set[0x19ef64]; !ok { t.Error("expected label 0x19ef64 in metadata") }This aids debugging and serves as documentation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
ledger/allegra/allegra.go(4 hunks)ledger/alonzo/alonzo.go(4 hunks)ledger/babbage/babbage.go(4 hunks)ledger/byron/byron.go(2 hunks)ledger/common/metadata.go(1 hunks)ledger/common/metadata_test.go(1 hunks)ledger/common/tx.go(1 hunks)ledger/conway/conway.go(4 hunks)ledger/mary/mary.go(4 hunks)ledger/shelley/shelley.go(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/common/tx.goledger/common/metadata.goledger/common/metadata_test.go
🧬 Code graph analysis (10)
ledger/common/tx.go (1)
ledger/common/metadata.go (1)
TransactionMetadatum(40-43)
ledger/byron/byron.go (2)
ledger/common/metadata.go (1)
TransactionMetadatum(40-43)ledger/byron.go (1)
ByronTransaction(28-28)
ledger/babbage/babbage.go (1)
ledger/common/metadata.go (2)
TransactionMetadataSet(38-38)TransactionMetadatum(40-43)
ledger/shelley/shelley.go (1)
ledger/common/metadata.go (2)
TransactionMetadataSet(38-38)TransactionMetadatum(40-43)
ledger/alonzo/alonzo.go (1)
ledger/common/metadata.go (2)
TransactionMetadataSet(38-38)TransactionMetadatum(40-43)
ledger/mary/mary.go (1)
ledger/common/metadata.go (2)
TransactionMetadataSet(38-38)TransactionMetadatum(40-43)
ledger/conway/conway.go (2)
ledger/common/metadata.go (2)
TransactionMetadataSet(38-38)TransactionMetadatum(40-43)ledger/conway.go (1)
ConwayTransaction(26-26)
ledger/common/metadata.go (4)
cbor/decode.go (1)
Decode(29-44)cbor/cbor.go (1)
RawMessage(36-36)cbor/bytestring.go (2)
ByteString(27-29)NewByteString(31-34)cbor/encode.go (1)
Encode(27-40)
ledger/common/metadata_test.go (2)
ledger/common/metadata.go (1)
TransactionMetadataSet(38-38)cbor/decode.go (1)
Decode(29-44)
ledger/allegra/allegra.go (1)
ledger/common/metadata.go (2)
TransactionMetadataSet(38-38)TransactionMetadatum(40-43)
🪛 GitHub Actions: go-test
ledger/alonzo/alonzo.go
[error] 106-106: cannot use b.TransactionMetadataSet (variable of map type "github.com/blinklabs-io/gouroboros/ledger/common".TransactionMetadataSet) as map[uint]*"github.com/blinklabs-io/gouroboros/cbor".LazyValue value in struct literal
🪛 GitHub Actions: golangci-lint
ledger/alonzo/alonzo.go
[error] 106-106: cannot use b.TransactionMetadataSet (type map["github.com/blinklabs-io/gouroboros/ledger/common".TransactionMetadataSet) as map[uint]*"github.com/blinklabs-io/gouroboros/cbor".LazyValue value in struct literal (typecheck)
🪛 GitHub Actions: nilaway
ledger/alonzo/alonzo.go
[error] 106-106: cannot use b.TransactionMetadataSet (type map["github.com/blinklabs-io/gouroboros/ledger/common".TransactionMetadataSet) as map[uint]*"github.com/blinklabs-io/gouroboros/cbor".LazyValue in struct literal. Lint/compile error encountered during 'nilaway ./...' step.
⏰ 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 (11)
ledger/common/tx.go (1)
31-31: Clean interface update to support structured metadata.The return type change from
*cbor.LazyValuetoTransactionMetadatumprovides a typed API for transaction metadata, replacing the raw CBOR approach. This is consistent with implementations across all era modules in this PR.ledger/byron/byron.go (2)
145-145: LGTM! Field type updated consistently.The change from
*cbor.LazyValuetocommon.TransactionMetadatumaligns with the new typed metadata model.
278-280: LGTM! Method signature matches interface.The
Metadata()implementation correctly returns theTransactionMetadatuminterface value.ledger/conway/conway.go (4)
58-58: LGTM! Block-level metadata set correctly typed.The change from
map[uint]*cbor.LazyValuetocommon.TransactionMetadataSetprovides structured metadata handling at the block level.
521-521: LGTM! Transaction metadata field updated correctly.The field now uses the typed
TransactionMetadatuminterface instead of raw CBOR.
635-637: LGTM! Method signature matches interface.The
Metadata()implementation correctly returnsTransactionMetadatum.
684-712: LGTM! CBOR encoding correctly delegates to metadata's marshaler.The change from wrapping metadata in
cbor.RawMessage(t.TxMetadata.Cbor())to appendingt.TxMetadatadirectly is correct. This delegates encoding to the metadata type'sMarshalCBORimplementation, enabling proper round-trip encoding. The consistent append behavior (always including the field, even if nil) aligns with CBOR array structure requirements.ledger/babbage/babbage.go (4)
58-58: LGTM! Block-level metadata set correctly typed.Consistent with the Conway changes, this provides structured metadata handling at the block level.
736-736: LGTM! Transaction metadata field updated correctly.The field type change is consistent with the new metadata model.
850-852: LGTM! Method signature matches interface.The
Metadata()implementation is correct and consistent across eras.
899-927: LGTM! CBOR encoding pattern consistent with Conway.The encoding correctly delegates to the metadata's
MarshalCBORimplementation and consistently includes the metadata field (even if nil) in the transaction array structure.
Signed-off-by: Akhil Repala <[email protected]>
Signed-off-by: Akhil Repala <[email protected]>
Signed-off-by: Akhil Repala <[email protected]>
Signed-off-by: Akhil Repala <[email protected]>
Signed-off-by: Akhil Repala <[email protected]>
Signed-off-by: Akhil Repala <[email protected]>
arepala-uml
force-pushed
the
tx_metadata_encode
branch
from
8194f2a to
8768206
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
ledger/common/metadata.go (3)
45-45: Fix formatting and address the int64 range limitation.The pipeline reports a
gofumptformatting issue on this line. Please rungofumpt -wto fix it.Additionally, as noted in previous reviews,
MetaInt.Value int64cannot represent the full range of CBOR integers allowed by the metadata CDDL. This limitation causesdecodeMapUint(line 147-149) to explicitly reject any map key exceedingmath.MaxInt64, which will fail to decode valid on-chain metadata. Consider using*big.Intor at least supporting the fulluint64range to avoid rejecting legitimate metadata.
124-129: Do not coerce unsupported CBOR types into text—reject them explicitly.As noted in previous reviews, this code decodes CBOR major types 6 (tags) and 7 (floats/simples) and converts them to text via
fmt.Sprintf. The metadata CDDL does not permit these types, so this silently accepts invalid metadata and misrepresents values like floats, booleans, and nulls as strings.Instead, return an explicit error when encountering these major types to reject non-conformant metadata.
Apply this diff:
- case cborTypeTag, cborTypeFloatSim: - var x any - if _, err := cbor.Decode(b, &x); err != nil { - return nil, err - } - return MetaText{Value: fmt.Sprintf("%v", x)}, nil + case cborTypeTag, cborTypeFloatSim: + return nil, fmt.Errorf("unsupported CBOR major type %#x in metadatum (tags/floats/simples not allowed)", b[0]&cborTypeMask)
244-310: Mixed-key maps are dropped, causing data loss.As noted in previous reviews, when a metadata map contains keys of varying types (which the spec allows), this function falls through all three homogeneous-key checks and returns
nilat line 305. This causesMarshalCBORto encode the value as CBOR null, permanently losing the map data.To preserve such maps, detect the mixed-key case and encode as an array of key-value pairs instead of returning
nil.Apply this diff to preserve mixed-key maps:
if allBytes { mm := make(map[cbor.ByteString]any, len(t.Pairs)) for _, p := range t.Pairs { bs := p.Key.(MetaBytes).Value key := cbor.NewByteString(bs) mm[key] = metadatumToInterface(p.Value) } return mm } - return nil + // Mixed-key map: encode as array of [key, value] pairs + out := make([]any, len(t.Pairs)) + for i, p := range t.Pairs { + out[i] = []any{metadatumToInterface(p.Key), metadatumToInterface(p.Value)} + } + return out default: return nil } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
ledger/allegra/allegra.go(4 hunks)ledger/allegra/block_test.go(1 hunks)ledger/alonzo/alonzo.go(5 hunks)ledger/babbage/babbage.go(4 hunks)ledger/byron/byron.go(2 hunks)ledger/common/metadata.go(1 hunks)ledger/common/metadata_test.go(1 hunks)ledger/common/tx.go(1 hunks)ledger/conway/conway.go(4 hunks)ledger/mary/mary.go(4 hunks)ledger/shelley/shelley.go(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- ledger/allegra/block_test.go
- ledger/common/metadata_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/common/tx.goledger/common/metadata.go
🧬 Code graph analysis (9)
ledger/conway/conway.go (2)
ledger/common/metadata.go (2)
TransactionMetadataSet(38-38)TransactionMetadatum(40-43)ledger/conway.go (1)
ConwayTransaction(26-26)
ledger/common/tx.go (1)
ledger/common/metadata.go (1)
TransactionMetadatum(40-43)
ledger/mary/mary.go (2)
ledger/common/metadata.go (2)
TransactionMetadataSet(38-38)TransactionMetadatum(40-43)ledger/mary.go (1)
MaryTransaction(26-26)
ledger/babbage/babbage.go (2)
ledger/common/metadata.go (2)
TransactionMetadataSet(38-38)TransactionMetadatum(40-43)ledger/babbage.go (1)
BabbageTransaction(26-26)
ledger/byron/byron.go (2)
ledger/common/metadata.go (1)
TransactionMetadatum(40-43)ledger/byron.go (1)
ByronTransaction(28-28)
ledger/alonzo/alonzo.go (1)
ledger/common/metadata.go (2)
TransactionMetadataSet(38-38)TransactionMetadatum(40-43)
ledger/allegra/allegra.go (2)
ledger/common/metadata.go (2)
TransactionMetadataSet(38-38)TransactionMetadatum(40-43)ledger/allegra.go (1)
AllegraTransaction(25-25)
ledger/shelley/shelley.go (2)
ledger/common/metadata.go (2)
TransactionMetadataSet(38-38)TransactionMetadatum(40-43)ledger/shelley.go (1)
ShelleyTransaction(26-26)
ledger/common/metadata.go (4)
cbor/decode.go (1)
Decode(29-44)cbor/cbor.go (1)
RawMessage(36-36)cbor/bytestring.go (2)
ByteString(27-29)NewByteString(31-34)cbor/encode.go (1)
Encode(27-40)
🪛 GitHub Actions: golangci-lint
ledger/common/metadata.go
[error] 45-45: golangci-lint detected gofumpt formatting issues. File is not properly formatted (gofumpt).
🪛 GitHub Check: lint
ledger/common/metadata.go
[failure] 192-192:
unnecessary leading newline (whitespace)
[failure] 174-174:
error is not nil (line 173) but it returns nil (nilerr)
[failure] 158-158:
error is not nil (line 157) but it returns nil (nilerr)
[failure] 139-139:
error is not nil (line 138) but it returns nil (nilerr)
[failure] 45-45:
File is not properly formatted (gofumpt)
⏰ 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 (13)
ledger/common/tx.go (1)
31-31: LGTM! Clean interface update for typed metadata.The signature change from
*cbor.LazyValuetoTransactionMetadatumprovides a strongly-typed interface for transaction metadata, aligning with the new common metadata model. All implementations across Byron, Shelley, Allegra, Mary, Alonzo, Babbage, and Conway have been consistently updated.ledger/mary/mary.go (3)
55-55: LGTM! Consistent metadata type update.The field type change from
map[uint]*cbor.LazyValuetocommon.TransactionMetadataSetaligns with the repository-wide metadata refactor and provides proper typing for transaction metadata sets.
271-271: LGTM! Metadata field and accessor updated consistently.Both the field type and method signature are correctly updated to use
common.TransactionMetadatum, maintaining consistency with the interface change inledger/common/tx.go.Also applies to: 385-387
434-435: LGTM! CBOR encoding correctly uses typed metadata.The encoding now appends
t.TxMetadatadirectly instead ofcbor.RawMessage(t.TxMetadata.Cbor()), which is correct sinceTransactionMetadatumtypes implementcbor.Marshaler. The nil check ensures proper handling when metadata is absent.ledger/byron/byron.go (1)
145-145: LGTM! Byron transaction metadata updated consistently.Both the
Attributesfield andMetadata()method are correctly updated to usecommon.TransactionMetadatum, maintaining consistency with other era implementations.Also applies to: 278-280
ledger/alonzo/alonzo.go (1)
58-58: LGTM! Alonzo metadata migration is complete and consistent.All metadata-related changes (block-level set, transaction field, accessor method, and CBOR encoding) are correctly updated to use the new typed metadata model. The encoding path properly appends metadata directly when present.
Also applies to: 636-636, 750-752, 809-810
ledger/conway/conway.go (1)
58-58: LGTM! Conway metadata changes are consistent and correct.All aspects of the metadata refactor are properly implemented: block-level TransactionMetadataSet, transaction-level TransactionMetadatum field, updated accessor method, and direct CBOR encoding when metadata is present.
Also applies to: 521-521, 635-637, 701-702
ledger/babbage/babbage.go (1)
58-58: LGTM! Babbage metadata refactor is complete.The systematic update of metadata types and encoding logic is correctly applied across the block, transaction, accessor, and CBOR encoding paths.
Also applies to: 791-791, 905-907, 971-972
ledger/shelley/shelley.go (1)
56-56: LGTM! Shelley metadata migration is consistent with other eras.All metadata-related changes align with the repository-wide refactor: typed metadata set at block level, typed metadata field at transaction level, updated accessor signature, and direct CBOR encoding.
Also applies to: 543-543, 653-655, 711-712
ledger/allegra/allegra.go (1)
52-52: LGTM! Allegra completes the cross-era metadata refactor.The changes are consistent with all other era implementations, successfully migrating from
*cbor.LazyValueto the strongly-typedcommon.TransactionMetadatummodel. The nil handling in CBOR encoding is correct.Also applies to: 242-242, 352-354, 413-414
ledger/common/metadata.go (3)
25-36: LGTM!The CBOR major type constants are correctly defined and aligned with RFC 8949.
59-69: LGTM!The interface marker methods and
TypeNameimplementations are straightforward and correct.
233-242: LGTM!The
MarshalCBORimplementation correctly handles both nil and populated metadata sets, delegating tometadatumToInterfacefor type conversion.
Signed-off-by: Akhil Repala <[email protected]>
Signed-off-by: Akhil Repala <[email protected]>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
| func (t *ByronTransaction) Metadata() *cbor.LazyValue { | ||
| return t.Attributes | ||
| func (t *ByronTransaction) Metadata() common.TransactionMetadatum { | ||
| return nil |
There was a problem hiding this comment.
This is definitely not correct. This probably needs its own Byron-specific type, since it'll be different from things afterwards.
…metdata part Signed-off-by: Akhil Repala <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
ledger/common/metadata.go (1)
318-334: Mixed-key maps are lost during encoding.When a
MetaMaphas keys of varying types (allowed by the CDDL spec), line 334 returnsnil, causingMarshalCBORto encode it as CBOR null and permanently lose the map data. This is the issue flagged in the previous review.Based on the past review suggestion, preserve mixed-key maps by encoding them as an array of key/value pairs:
+ // Mixed key types: encode as array of pairs + pairArray := make([]any, 0, len(t.Pairs)) + for _, p := range t.Pairs { + pairArray = append(pairArray, []any{ + metadatumToInterface(p.Key), + metadatumToInterface(p.Value), + }) + } - return nil + return pairArray
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ledger/allegra/block_test.go(0 hunks)ledger/common/metadata.go(1 hunks)
💤 Files with no reviewable changes (1)
- ledger/allegra/block_test.go
🧰 Additional context used
🧠 Learnings (1)
📚 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/common/metadata.go
🧬 Code graph analysis (1)
ledger/common/metadata.go (6)
cbor/value.go (1)
Value(29-33)cbor/encode.go (1)
Encode(27-40)cbor/decode.go (1)
Decode(29-44)cbor/cbor.go (1)
RawMessage(36-36)cbor/bytestring.go (2)
ByteString(27-29)NewByteString(31-34)cbor/tags.go (1)
Set(177-177)
⏰ 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: cubic · AI code reviewer
🔇 Additional comments (6)
ledger/common/metadata.go (6)
45-45: Correct use of*big.Intfor arbitrary precision.The use of
*big.Intproperly addresses the previous concern about integer range limitations. On-chain metadata can contain arbitrary-sized integers, and this implementation now handles them correctly.
80-96: LGTM: Clean helper for hashable integer keys.The
metaIntKeyapproach using string representation is a clever solution to makebig.Intvalues hashable for use as map keys while preserving arbitrary precision.
151-155: Correct rejection of unsupported CBOR types.This properly returns an error for tags and floats/simpleValues instead of coercing them into text, addressing the previous concern about misrepresenting invalid metadata.
162-213: LGTM: Map decoders handle multiple key types correctly.The three specialized decoders (
decodeMapUint,decodeMapText,decodeMapBytes) properly handle different CBOR map key types and include good defensive practices like the byte copy at line 208.
215-253: LGTM: Flexible unmarshaling supports both map and array forms.The function correctly tries both encodings and handles null-skipping for array form, providing good flexibility for CBOR decoding.
255-264: Clean marshaling implementation.The
MarshalCBORfunction is well-structured. Note that its correctness depends onmetadatumToInterfaceproperly handling all cases (see related comments on that function).
…metdata part Signed-off-by: Akhil Repala <[email protected]>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 11 files
Prompt for AI agents (all 6 issues)
Understand the root cause of the following 6 issues and fix them.
<file name="ledger/allegra/allegra.go">
<violation number="1" location="ledger/allegra/allegra.go:414">
Metadata is appended as a raw struct, so the generated transaction CBOR will encode Go struct fields instead of the Cardano metadata representation.</violation>
</file>
<file name="ledger/common/metadata.go">
<violation number="1" location="ledger/common/metadata.go:163">
Metadata maps that use negative integer keys cannot be decoded because `decodeMapUint` only attempts `map[uint]` and therefore rejects valid CBOR with signed keys.</violation>
<violation number="2" location="ledger/common/metadata.go:261">
Marshaling silently replaces `MetaMap` values with `null` whenever the map contains mixed key types, so valid metadata is dropped without reporting an error.</violation>
</file>
<file name="ledger/mary/mary.go">
<violation number="1" location="ledger/mary/mary.go:435">
Metadata is appended to the transaction CBOR as a Meta* struct, so it serializes as a Go struct (with `Value` fields) instead of the proper metadata CBOR, breaking Mary transaction encoding when metadata exists.</violation>
</file>
<file name="ledger/conway/conway.go">
<violation number="1" location="ledger/conway/conway.go:702">
Metadata is appended as a raw struct, so cbor.Encode emits the struct fields (`Value`, `Items`, etc.) instead of the actual metadata value, breaking transaction serialization whenever metadata is present.</violation>
</file>
<file name="ledger/shelley/shelley.go">
<violation number="1" location="ledger/shelley/shelley.go:712">
Tx metadata is now appended to the CBOR array as a raw TransactionMetadatum struct, which the CBOR encoder turns into a Go-struct representation rather than valid metadata, breaking serialization whenever metadata is present.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| } | ||
| if t.TxMetadata != nil { | ||
| tmpObj = append(tmpObj, cbor.RawMessage(t.TxMetadata.Cbor())) | ||
| tmpObj = append(tmpObj, t.TxMetadata) |
There was a problem hiding this comment.
Metadata is appended as a raw struct, so the generated transaction CBOR will encode Go struct fields instead of the Cardano metadata representation.
Prompt for AI agents
Address the following comment on ledger/allegra/allegra.go at line 414:
<comment>Metadata is appended as a raw struct, so the generated transaction CBOR will encode Go struct fields instead of the Cardano metadata representation.</comment>
<file context>
@@ -411,7 +411,7 @@ func (t *AllegraTransaction) Cbor() []byte {
}
if t.TxMetadata != nil {
- tmpObj = append(tmpObj, cbor.RawMessage(t.TxMetadata.Cbor()))
+ tmpObj = append(tmpObj, t.TxMetadata)
} else {
tmpObj = append(tmpObj, nil)
</file context>
| } | ||
| tmpMap := make(map[uint]any, len(s)) | ||
| for k, v := range s { | ||
| tmpMap[k] = metadatumToInterface(v) |
There was a problem hiding this comment.
Marshaling silently replaces MetaMap values with null whenever the map contains mixed key types, so valid metadata is dropped without reporting an error.
Prompt for AI agents
Address the following comment on ledger/common/metadata.go at line 261:
<comment>Marshaling silently replaces `MetaMap` values with `null` whenever the map contains mixed key types, so valid metadata is dropped without reporting an error.</comment>
<file context>
@@ -0,0 +1,339 @@
+ }
+ tmpMap := make(map[uint]any, len(s))
+ for k, v := range s {
+ tmpMap[k] = metadatumToInterface(v)
+ }
+ return cbor.Encode(&tmpMap)
</file context>
| } | ||
|
|
||
| func decodeMapUint(b []byte) (TransactionMetadatum, bool, error) { | ||
| var m map[uint]cbor.RawMessage |
There was a problem hiding this comment.
Metadata maps that use negative integer keys cannot be decoded because decodeMapUint only attempts map[uint] and therefore rejects valid CBOR with signed keys.
Prompt for AI agents
Address the following comment on ledger/common/metadata.go at line 163:
<comment>Metadata maps that use negative integer keys cannot be decoded because `decodeMapUint` only attempts `map[uint]` and therefore rejects valid CBOR with signed keys.</comment>
<file context>
@@ -0,0 +1,339 @@
+}
+
+func decodeMapUint(b []byte) (TransactionMetadatum, bool, error) {
+ var m map[uint]cbor.RawMessage
+ if _, err := cbor.Decode(b, &m); err != nil {
+ return nil, false, nil //nolint:nilerr // not this shape
</file context>
| } | ||
| if t.TxMetadata != nil { | ||
| tmpObj = append(tmpObj, cbor.RawMessage(t.TxMetadata.Cbor())) | ||
| tmpObj = append(tmpObj, t.TxMetadata) |
There was a problem hiding this comment.
Metadata is appended to the transaction CBOR as a Meta* struct, so it serializes as a Go struct (with Value fields) instead of the proper metadata CBOR, breaking Mary transaction encoding when metadata exists.
Prompt for AI agents
Address the following comment on ledger/mary/mary.go at line 435:
<comment>Metadata is appended to the transaction CBOR as a Meta* struct, so it serializes as a Go struct (with `Value` fields) instead of the proper metadata CBOR, breaking Mary transaction encoding when metadata exists.</comment>
<file context>
@@ -432,7 +432,7 @@ func (t *MaryTransaction) Cbor() []byte {
}
if t.TxMetadata != nil {
- tmpObj = append(tmpObj, cbor.RawMessage(t.TxMetadata.Cbor()))
+ tmpObj = append(tmpObj, t.TxMetadata)
} else {
tmpObj = append(tmpObj, nil)
</file context>
| } | ||
| if t.TxMetadata != nil { | ||
| tmpObj = append(tmpObj, cbor.RawMessage(t.TxMetadata.Cbor())) | ||
| tmpObj = append(tmpObj, t.TxMetadata) |
There was a problem hiding this comment.
Metadata is appended as a raw struct, so cbor.Encode emits the struct fields (Value, Items, etc.) instead of the actual metadata value, breaking transaction serialization whenever metadata is present.
Prompt for AI agents
Address the following comment on ledger/conway/conway.go at line 702:
<comment>Metadata is appended as a raw struct, so cbor.Encode emits the struct fields (`Value`, `Items`, etc.) instead of the actual metadata value, breaking transaction serialization whenever metadata is present.</comment>
<file context>
@@ -699,7 +699,7 @@ func (t *ConwayTransaction) Cbor() []byte {
}
if t.TxMetadata != nil {
- tmpObj = append(tmpObj, cbor.RawMessage(t.TxMetadata.Cbor()))
+ tmpObj = append(tmpObj, t.TxMetadata)
} else {
tmpObj = append(tmpObj, nil)
</file context>
| } | ||
| if t.TxMetadata != nil { | ||
| tmpObj = append(tmpObj, cbor.RawMessage(t.TxMetadata.Cbor())) | ||
| tmpObj = append(tmpObj, t.TxMetadata) |
There was a problem hiding this comment.
Tx metadata is now appended to the CBOR array as a raw TransactionMetadatum struct, which the CBOR encoder turns into a Go-struct representation rather than valid metadata, breaking serialization whenever metadata is present.
Prompt for AI agents
Address the following comment on ledger/shelley/shelley.go at line 712:
<comment>Tx metadata is now appended to the CBOR array as a raw TransactionMetadatum struct, which the CBOR encoder turns into a Go-struct representation rather than valid metadata, breaking serialization whenever metadata is present.</comment>
<file context>
@@ -709,7 +709,7 @@ func (t *ShelleyTransaction) Cbor() []byte {
}
if t.TxMetadata != nil {
- tmpObj = append(tmpObj, cbor.RawMessage(t.TxMetadata.Cbor()))
+ tmpObj = append(tmpObj, t.TxMetadata)
} else {
tmpObj = append(tmpObj, nil)
</file context>
There was a problem hiding this comment.
4 issues found across 10 files
Prompt for AI agents (all 4 issues)
Understand the root cause of the following 4 issues and fix them.
<file name="ledger/alonzo/alonzo.go">
<violation number="1" location="ledger/alonzo/alonzo.go:810">
`AlonzoTransaction.Cbor()` now appends the typed metadatum struct directly, so the CBOR encoder serializes Go struct fields instead of canonical metadata. Convert the metadata to its CBOR form (as done in TransactionMetadataSet.MarshalCBOR) before appending to tmpObj or keep using the raw CBOR bytes.</violation>
</file>
<file name="ledger/common/metadata.go">
<violation number="1" location="ledger/common/metadata.go:163">
decodeMapUint rejects metadata maps whose keys are negative integers, even though metadata allows signed int keys, so valid CBOR fails to decode.</violation>
<violation number="2" location="ledger/common/metadata.go:261">
MarshalCBOR drops any metadata map that mixes key kinds by encoding it as null instead of returning an error or preserving the pairs, causing data loss.</violation>
</file>
<file name="ledger/allegra/allegra.go">
<violation number="1" location="ledger/allegra/allegra.go:414">
`TransactionMetadatum` is appended to the CBOR array as a Go struct, so CBOR encoding no longer matches the metadata CDDL and produces invalid transactions whenever metadata is present.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| } | ||
| if t.TxMetadata != nil { | ||
| tmpObj = append(tmpObj, cbor.RawMessage(t.TxMetadata.Cbor())) | ||
| tmpObj = append(tmpObj, t.TxMetadata) |
There was a problem hiding this comment.
AlonzoTransaction.Cbor() now appends the typed metadatum struct directly, so the CBOR encoder serializes Go struct fields instead of canonical metadata. Convert the metadata to its CBOR form (as done in TransactionMetadataSet.MarshalCBOR) before appending to tmpObj or keep using the raw CBOR bytes.
Prompt for AI agents
Address the following comment on ledger/alonzo/alonzo.go at line 810:
<comment>`AlonzoTransaction.Cbor()` now appends the typed metadatum struct directly, so the CBOR encoder serializes Go struct fields instead of canonical metadata. Convert the metadata to its CBOR form (as done in TransactionMetadataSet.MarshalCBOR) before appending to tmpObj or keep using the raw CBOR bytes.</comment>
<file context>
@@ -807,7 +807,7 @@ func (t *AlonzoTransaction) Cbor() []byte {
}
if t.TxMetadata != nil {
- tmpObj = append(tmpObj, cbor.RawMessage(t.TxMetadata.Cbor()))
+ tmpObj = append(tmpObj, t.TxMetadata)
} else {
tmpObj = append(tmpObj, nil)
</file context>
| } | ||
| tmpMap := make(map[uint]any, len(s)) | ||
| for k, v := range s { | ||
| tmpMap[k] = metadatumToInterface(v) |
There was a problem hiding this comment.
MarshalCBOR drops any metadata map that mixes key kinds by encoding it as null instead of returning an error or preserving the pairs, causing data loss.
Prompt for AI agents
Address the following comment on ledger/common/metadata.go at line 261:
<comment>MarshalCBOR drops any metadata map that mixes key kinds by encoding it as null instead of returning an error or preserving the pairs, causing data loss.</comment>
<file context>
@@ -0,0 +1,340 @@
+ }
+ tmpMap := make(map[uint]any, len(s))
+ for k, v := range s {
+ tmpMap[k] = metadatumToInterface(v)
+ }
+ return cbor.Encode(&tmpMap)
</file context>
| } | ||
|
|
||
| func decodeMapUint(b []byte) (TransactionMetadatum, bool, error) { | ||
| var m map[uint]cbor.RawMessage |
There was a problem hiding this comment.
decodeMapUint rejects metadata maps whose keys are negative integers, even though metadata allows signed int keys, so valid CBOR fails to decode.
Prompt for AI agents
Address the following comment on ledger/common/metadata.go at line 163:
<comment>decodeMapUint rejects metadata maps whose keys are negative integers, even though metadata allows signed int keys, so valid CBOR fails to decode.</comment>
<file context>
@@ -0,0 +1,340 @@
+}
+
+func decodeMapUint(b []byte) (TransactionMetadatum, bool, error) {
+ var m map[uint]cbor.RawMessage
+ if _, err := cbor.Decode(b, &m); err != nil {
+ return nil, false, nil //nolint:nilerr // not this shape
</file context>
| } | ||
| if t.TxMetadata != nil { | ||
| tmpObj = append(tmpObj, cbor.RawMessage(t.TxMetadata.Cbor())) | ||
| tmpObj = append(tmpObj, t.TxMetadata) |
There was a problem hiding this comment.
TransactionMetadatum is appended to the CBOR array as a Go struct, so CBOR encoding no longer matches the metadata CDDL and produces invalid transactions whenever metadata is present.
Prompt for AI agents
Address the following comment on ledger/allegra/allegra.go at line 414:
<comment>`TransactionMetadatum` is appended to the CBOR array as a Go struct, so CBOR encoding no longer matches the metadata CDDL and produces invalid transactions whenever metadata is present.</comment>
<file context>
@@ -411,7 +411,7 @@ func (t *AllegraTransaction) Cbor() []byte {
}
if t.TxMetadata != nil {
- tmpObj = append(tmpObj, cbor.RawMessage(t.TxMetadata.Cbor()))
+ tmpObj = append(tmpObj, t.TxMetadata)
} else {
tmpObj = append(tmpObj, nil)
</file context>
Closes #1136
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.