feat: enable recvcheck linter and corrected the reported issues#1433
Open
cryptodj413 wants to merge 1 commit intoblinklabs-io:mainfrom
Open
feat: enable recvcheck linter and corrected the reported issues#1433cryptodj413 wants to merge 1 commit intoblinklabs-io:mainfrom
cryptodj413 wants to merge 1 commit intoblinklabs-io:mainfrom
Conversation
Signed-off-by: cryptodj413 <shinjirohara2@gmail.com>
Contributor
📝 WalkthroughWalkthroughThis pull request systematically converts method receivers across the codebase from value receivers to pointer receivers to resolve linter violations. The Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (52)
💤 Files with no reviewable changes (1)
🧰 Additional context used📓 Path-based instructions (8)ledger/**/*.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*_test.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
ledger/**/*_test.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
.golangci.yml📄 CodeRabbit inference engine (AGENTS.md)
Files:
ledger/**/rules.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
ledger/common/certs.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
ledger/**/certs.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (22)📓 Common learnings📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
📚 Learning: 2025-11-04T15:54:22.683ZApplied to files:
📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
📚 Learning: 2026-01-13T02:47:14.599ZApplied to files:
🧬 Code graph analysis (37)ledger/shelley/pparams.go (1)
ledger/common/witness.go (1)
ledger/shelley/genesis_test.go (1)
ledger/babbage/pparams.go (1)
ledger/babbage/rules.go (1)
ledger/common/metadata_test.go (2)
ledger/babbage/rules_test.go (3)
ledger/leios/leios.go (1)
ledger/mary/rules_test.go (1)
ledger/alonzo/pparams.go (1)
ledger/conway/pparams.go (2)
ledger/common/verify.go (1)
ledger/conway/rules.go (2)
ledger/shelley/rules_test.go (1)
ledger/babbage/errors_test.go (1)
ledger/shelley/genesis.go (1)
ledger/byron/genesis.go (3)
ledger/allegra/rules_test.go (1)
ledger/conway/gov.go (1)
ledger/shelley/rules.go (2)
ledger/conway/conway_test.go (1)
ledger/error_test.go (3)
ledger/conway/rules_test.go (3)
ledger/byron/genesis_test.go (1)
ledger/leios/pparams.go (1)
ledger/common/script_test.go (1)
ledger/conway/errors_test.go (1)
ledger/conway/conway.go (3)
ledger/common/script.go (2)
ledger/byron/byron.go (1)
ledger/alonzo/rules_test.go (2)
ledger/common/address.go (2)
ledger/common/certs.go (1)
ledger/allegra/allegra.go (1)
ledger/shelley/shelley.go (2)
ledger/mary/mary.go (4)
ledger/babbage/babbage.go (2)
⏰ 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). (2)
✏️ Tip: You can disable this entire section by setting 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
ledger/error.go (2)
680-696: Critical: Loop variable pointer capture bug.Taking
&inputinside arangeloop captures the address of the loop variable, which is reused across iterations. AllUtxoentries in the slice will point to the same memory location containing the last iteration's value.🐛 Proposed fix
e.Utxos = make([]common.Utxo, 0, len(shelleyUtxoMap)) for input, outputCbor := range shelleyUtxoMap { + input := input // create a new variable scoped to this iteration // Decode output using era-agnostic function (handles all eras) output, err := NewTransactionOutputFromCbor(outputCbor) if err != nil { return fmt.Errorf( "failed to decode transaction output: %w", err, ) } e.Utxos = append(e.Utxos, common.Utxo{ Id: &input, Output: output, }) }
700-720: Same loop variable capture bug in Byron path.The Byron input decoding path has the identical issue with
&inputcapturing the loop variable address.🐛 Proposed fix
e.Utxos = make([]common.Utxo, 0, len(byronUtxoMap)) for input, outputCbor := range byronUtxoMap { + input := input // create a new variable scoped to this iteration // Decode output using era-agnostic function (handles all eras) output, err := NewTransactionOutputFromCbor(outputCbor) if err != nil { return fmt.Errorf( "failed to decode transaction output: %w", err, ) } e.Utxos = append(e.Utxos, common.Utxo{ Id: &input, Output: output, }) }ledger/common/certs.go (1)
322-328: Add nil check for StakeCredential pointer before dereferencing in Utxorpc() method.The
StakeCredentialfield is defined as a pointer (*Credential) at line 326, which differs from all other certificate types in the file that use non-pointerCredentialfields. At line 344,c.StakeCredential.Utxorpc()is called without a nil check. Since CBOR array decoding can leave pointer fields uninitialized (nil), this creates a nil pointer dereference risk if the CBOR data is malformed or incomplete.Either add a nil check before calling
Utxorpc():if c.StakeCredential == nil { return nil, fmt.Errorf("StakeCredential must not be nil") } stakeCred, err := c.StakeCredential.Utxorpc()Or change
StakeCredentialto a non-pointer field to match the pattern used byStakeRegistrationCertificateandStakeDeregistrationCertificate.ledger/babbage/babbage.go (1)
716-727: Inconsistent nil semantics inDatumHash()return value.When
o.DatumOptionisnil, this method returns&common.Blake2b256{}(a pointer to a zero-value hash) instead ofnil. This differs fromShelleyTransactionOutput.DatumHash()which returnsnil, and may confuse callers that checkif datumHash != nilto determine if a datum exists.Consider returning
nilwhen there's no datum option for consistent behavior across eras:Suggested fix
func (o *BabbageTransactionOutput) DatumHash() *common.Blake2b256 { if o.DatumOption != nil { if o.DatumOption.hash != nil { return o.DatumOption.hash } if o.DatumOption.data != nil { hash := o.DatumOption.data.Hash() return &hash } } - return &common.Blake2b256{} + return nil }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (52)
.golangci.ymlcbor/cbor.gocbor/value.gointernal/test/conformance/conformance_test.goledger/allegra/allegra.goledger/allegra/rules_test.goledger/alonzo/alonzo.goledger/alonzo/pparams.goledger/alonzo/rules_test.goledger/babbage/babbage.goledger/babbage/errors_test.goledger/babbage/pparams.goledger/babbage/rules.goledger/babbage/rules_test.goledger/byron/byron.goledger/byron/genesis.goledger/byron/genesis_test.goledger/common/address.goledger/common/auxiliary_data_integration_test.goledger/common/certs.goledger/common/gov.goledger/common/metadata.goledger/common/metadata_test.goledger/common/rules.goledger/common/script.goledger/common/script/context_test.goledger/common/script_test.goledger/common/verify.goledger/common/witness.goledger/conway/conway.goledger/conway/conway_test.goledger/conway/errors_test.goledger/conway/gov.goledger/conway/pparams.goledger/conway/rules.goledger/conway/rules_test.goledger/error.goledger/error_test.goledger/leios/leios.goledger/leios/pparams.goledger/mary/mary.goledger/mary/pparams.goledger/mary/rules_test.goledger/shelley/genesis.goledger/shelley/genesis_test.goledger/shelley/pparams.goledger/shelley/rules.goledger/shelley/rules_test.goledger/shelley/shelley.goprotocol/common/dmq.goprotocol/localstatequery/client_test.goprotocol/localstatequery/queries.go
💤 Files with no reviewable changes (1)
- ledger/common/rules.go
🧰 Additional context used
📓 Path-based instructions (8)
ledger/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
ledger/**/*.go: Usecbor.StructAsArrayembedded type to enable CBOR array encoding for struct types
Usecbor.DecodeStoreCborembedded type to preserve original CBOR bytes for hashing operations. Implement customUnmarshalCBORmethod and callSetCbor(cborData)to store original bytes
Always use preserved original CBOR bytes viaCbor()method for hash computation, not re-encoded data
Encode language views map with keys sorted by 'shortLex' order (length first, then lexicographic) as per Cardano Ledger Specification
For PlutusV1 cost models in language views: use double-bagged encoding with tag0x4100and wrap indefinite list in bytestring
For PlutusV2/V3 cost models in language views: use tag0x01or0x02with definite list encoding (no bytestring wrapper)
ScriptDataHash must be computed as Blake2b256(redeemers || datums || language_views) following Cardano Ledger Specification
Files:
ledger/shelley/pparams.goledger/common/witness.goledger/shelley/genesis_test.goledger/babbage/pparams.goledger/babbage/rules.goledger/common/metadata_test.goledger/babbage/rules_test.goledger/leios/leios.goledger/mary/rules_test.goledger/alonzo/pparams.goledger/conway/pparams.goledger/common/auxiliary_data_integration_test.goledger/common/gov.goledger/common/verify.goledger/conway/rules.goledger/shelley/rules_test.goledger/babbage/errors_test.goledger/shelley/genesis.goledger/mary/pparams.goledger/common/script/context_test.goledger/byron/genesis.goledger/allegra/rules_test.goledger/conway/gov.goledger/shelley/rules.goledger/conway/conway_test.goledger/error_test.goledger/conway/rules_test.goledger/byron/genesis_test.goledger/error.goledger/leios/pparams.goledger/common/script_test.goledger/conway/errors_test.goledger/common/metadata.goledger/conway/conway.goledger/common/script.goledger/byron/byron.goledger/alonzo/rules_test.goledger/common/address.goledger/common/certs.goledger/allegra/allegra.goledger/shelley/shelley.goledger/mary/mary.goledger/babbage/babbage.goledger/alonzo/alonzo.go
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Enforce 80 character line limit in Go code usinggolinesformatter
Apply Apache 2.0 license header to all new Go files
Usegofmtandgoimportsfor automatic code formatting
Pass all linter checks withmake lintorgolangci-lint run ./...before submission
Files:
ledger/shelley/pparams.goledger/common/witness.gointernal/test/conformance/conformance_test.goledger/shelley/genesis_test.goledger/babbage/pparams.goledger/babbage/rules.goledger/common/metadata_test.goledger/babbage/rules_test.goledger/leios/leios.goledger/mary/rules_test.goledger/alonzo/pparams.goledger/conway/pparams.goledger/common/auxiliary_data_integration_test.goledger/common/gov.goledger/common/verify.goledger/conway/rules.goledger/shelley/rules_test.goledger/babbage/errors_test.goprotocol/common/dmq.goledger/shelley/genesis.goledger/mary/pparams.goledger/common/script/context_test.gocbor/cbor.goledger/byron/genesis.goledger/allegra/rules_test.goledger/conway/gov.goledger/shelley/rules.goledger/conway/conway_test.goledger/error_test.goledger/conway/rules_test.goprotocol/localstatequery/queries.goledger/byron/genesis_test.goledger/error.goledger/leios/pparams.goledger/common/script_test.gocbor/value.goprotocol/localstatequery/client_test.goledger/conway/errors_test.goledger/common/metadata.goledger/conway/conway.goledger/common/script.goledger/byron/byron.goledger/alonzo/rules_test.goledger/common/address.goledger/common/certs.goledger/allegra/allegra.goledger/shelley/shelley.goledger/mary/mary.goledger/babbage/babbage.goledger/alonzo/alonzo.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Usetestifyassertion library for test assertions in Go tests
All Go tests must pass with race detection viamake testbefore submission
Files:
internal/test/conformance/conformance_test.goledger/shelley/genesis_test.goledger/common/metadata_test.goledger/babbage/rules_test.goledger/mary/rules_test.goledger/common/auxiliary_data_integration_test.goledger/shelley/rules_test.goledger/babbage/errors_test.goledger/common/script/context_test.goledger/allegra/rules_test.goledger/conway/conway_test.goledger/error_test.goledger/conway/rules_test.goledger/byron/genesis_test.goledger/common/script_test.goprotocol/localstatequery/client_test.goledger/conway/errors_test.goledger/alonzo/rules_test.go
ledger/**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
Use
MockLedgerStatefrominternal/test/ledger/ledger.gofor validation tests instead of creating custom mocks
Files:
ledger/shelley/genesis_test.goledger/common/metadata_test.goledger/babbage/rules_test.goledger/mary/rules_test.goledger/common/auxiliary_data_integration_test.goledger/shelley/rules_test.goledger/babbage/errors_test.goledger/common/script/context_test.goledger/allegra/rules_test.goledger/conway/conway_test.goledger/error_test.goledger/conway/rules_test.goledger/byron/genesis_test.goledger/common/script_test.goledger/conway/errors_test.goledger/alonzo/rules_test.go
.golangci.yml
📄 CodeRabbit inference engine (AGENTS.md)
Configure
.golangci.ymlwith enabled linters includinggosec,errorlint, andexhaustive
Files:
.golangci.yml
ledger/**/rules.go
📄 CodeRabbit inference engine (AGENTS.md)
ledger/**/rules.go: Implement validation rule functions with signature:func UtxoValidate{RuleName}(tx common.Transaction, slot uint64, ls common.LedgerState, pp common.ProtocolParameters) error- return nil on success or a specific error type on failure
Add new validation rules to theUtxoValidationRulesslice in the appropriate era package at the correct position in the rule order
Reference inputs are resolved but never consumed from the UTxO set
Collateral inputs are only consumed from UTxO set when transactionIsValidflag is false
Datum lookup must check witness set, inline datums, AND reference inputs in that order
Verify cost models exist for each Plutus version used in transaction before script execution
Later eras must call parent era functions for shared validation logic, not duplicate logic
Files:
ledger/babbage/rules.goledger/conway/rules.goledger/shelley/rules.go
ledger/common/certs.go
📄 CodeRabbit inference engine (AGENTS.md)
Define certificate structs in
ledger/common/certs.gowith CBOR tags before adding new certificate types
Files:
ledger/common/certs.go
ledger/**/certs.go
📄 CodeRabbit inference engine (AGENTS.md)
Update certificate parsing switch statement when adding support for new certificate types
Files:
ledger/common/certs.go
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to ledger/**/*.go : Use `cbor.StructAsArray` embedded type to enable CBOR array encoding for struct types
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to ledger/**/rules.go : Collateral inputs are only consumed from UTxO set when transaction `IsValid` flag is false
Applied to files:
ledger/common/witness.goledger/babbage/rules.goledger/babbage/rules_test.goledger/mary/rules_test.goledger/babbage/errors_test.goledger/common/script/context_test.goledger/allegra/rules_test.goledger/error_test.goledger/conway/rules_test.goledger/conway/errors_test.goledger/conway/conway.goledger/alonzo/rules_test.goledger/allegra/allegra.goledger/mary/mary.go
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to ledger/**/*.go : Use `cbor.DecodeStoreCbor` embedded type to preserve original CBOR bytes for hashing operations. Implement custom `UnmarshalCBOR` method and call `SetCbor(cborData)` to store original bytes
Applied to files:
internal/test/conformance/conformance_test.goledger/common/metadata_test.goledger/shelley/genesis.gocbor/cbor.goledger/error_test.goledger/error.gocbor/value.goledger/common/metadata.goledger/byron/byron.goledger/common/address.goledger/common/certs.goledger/shelley/shelley.goledger/babbage/babbage.goledger/alonzo/alonzo.go
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to ledger/**/*.go : Always use preserved original CBOR bytes via `Cbor()` method for hash computation, not re-encoded data
Applied to files:
internal/test/conformance/conformance_test.goledger/common/metadata_test.goledger/babbage/rules_test.goledger/common/verify.goledger/shelley/genesis.gocbor/cbor.goledger/shelley/rules.goledger/conway/conway_test.goledger/error_test.goledger/byron/genesis_test.goledger/error.gocbor/value.goledger/common/metadata.goledger/byron/byron.goledger/common/address.goledger/shelley/shelley.goledger/mary/mary.goledger/babbage/babbage.goledger/alonzo/alonzo.go
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to ledger/**/*.go : Use `cbor.StructAsArray` embedded type to enable CBOR array encoding for struct types
Applied to files:
internal/test/conformance/conformance_test.goledger/shelley/genesis.gocbor/cbor.goledger/error_test.goledger/error.gocbor/value.goledger/common/metadata.goledger/conway/conway.goledger/byron/byron.goledger/common/address.goledger/babbage/babbage.goledger/alonzo/alonzo.go
📚 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:
internal/test/conformance/conformance_test.gocbor/cbor.goledger/error_test.goledger/common/script_test.go
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to .golangci.yml : Configure `.golangci.yml` with enabled linters including `gosec`, `errorlint`, and `exhaustive`
Applied to files:
.golangci.yml
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to **/*.go : Pass all linter checks with `make lint` or `golangci-lint run ./...` before submission
Applied to files:
.golangci.yml
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to ledger/**/*.go : ScriptDataHash must be computed as Blake2b256(redeemers || datums || language_views) following Cardano Ledger Specification
Applied to files:
ledger/babbage/rules.goledger/conway/rules.goledger/babbage/errors_test.goledger/shelley/rules.goledger/conway/conway_test.goledger/error_test.goledger/conway/rules_test.goledger/error.goledger/common/script_test.goledger/conway/errors_test.goledger/common/script.goledger/byron/byron.goledger/common/address.goledger/shelley/shelley.goledger/mary/mary.goledger/babbage/babbage.go
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to ledger/**/rules.go : Verify cost models exist for each Plutus version used in transaction before script execution
Applied to files:
ledger/babbage/rules.goledger/conway/rules.goledger/babbage/errors_test.goledger/conway/conway_test.goledger/conway/errors_test.go
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to ledger/**/rules.go : Add new validation rules to the `UtxoValidationRules` slice in the appropriate era package at the correct position in the rule order
Applied to files:
ledger/babbage/rules.goledger/babbage/rules_test.goledger/mary/rules_test.goledger/shelley/rules_test.goledger/common/script/context_test.goledger/allegra/rules_test.goledger/shelley/rules.goledger/error_test.goledger/conway/rules_test.goledger/alonzo/rules_test.go
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to ledger/**/rules.go : Implement validation rule functions with signature: `func UtxoValidate{RuleName}(tx common.Transaction, slot uint64, ls common.LedgerState, pp common.ProtocolParameters) error` - return nil on success or a specific error type on failure
Applied to files:
ledger/babbage/rules.goledger/babbage/rules_test.goledger/mary/rules_test.goledger/shelley/rules_test.goledger/babbage/errors_test.goledger/common/script/context_test.goledger/allegra/rules_test.goledger/shelley/rules.goledger/error_test.goledger/conway/rules_test.goledger/conway/errors_test.goledger/alonzo/rules_test.go
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to ledger/**/certs.go : Update certificate parsing switch statement when adding support for new certificate types
Applied to files:
ledger/babbage/rules.goledger/common/verify.goledger/conway/rules.goledger/shelley/rules.goledger/common/certs.go
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to ledger/**/rules.go : Reference inputs are resolved but never consumed from the UTxO set
Applied to files:
ledger/babbage/rules.goledger/babbage/rules_test.goledger/mary/rules_test.goledger/shelley/rules_test.goledger/babbage/errors_test.goledger/common/script/context_test.goledger/allegra/rules_test.goledger/error_test.goledger/conway/rules_test.goledger/error.goledger/conway/errors_test.goledger/byron/byron.goledger/alonzo/rules_test.goledger/allegra/allegra.go
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to ledger/**/native.go : Implement `NativeScript.Evaluate(slot uint64, validityStart, validityEnd uint64, keyHashes map[Blake2b224]bool) bool` for native script validation
Applied to files:
ledger/babbage/rules.goledger/conway/rules.goledger/common/script.go
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to ledger/**/*.go : For PlutusV2/V3 cost models in language views: use tag `0x01` or `0x02` with definite list encoding (no bytestring wrapper)
Applied to files:
ledger/babbage/rules.goledger/conway/rules.goledger/byron/byron.goledger/babbage/babbage.go
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to ledger/**/*.go : For PlutusV1 cost models in language views: use double-bagged encoding with tag `0x4100` and wrap indefinite list in bytestring
Applied to files:
ledger/babbage/rules.goledger/conway/rules.goledger/byron/byron.goledger/babbage/babbage.go
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to ledger/**/*_test.go : Use `MockLedgerState` from `internal/test/ledger/ledger.go` for validation tests instead of creating custom mocks
Applied to files:
ledger/common/metadata_test.goledger/babbage/rules_test.goledger/mary/rules_test.goledger/common/auxiliary_data_integration_test.goledger/babbage/errors_test.goledger/allegra/rules_test.goledger/error_test.goledger/conway/rules_test.goledger/byron/genesis_test.goledger/conway/errors_test.goledger/alonzo/rules_test.go
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to ledger/**/rules.go : Datum lookup must check witness set, inline datums, AND reference inputs in that order
Applied to files:
ledger/common/verify.go
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Create new era package structure following pattern: `ledger/{era_name}/` with types extending previous era, `rules.go` inheriting parent rules, and compatibility exports at `ledger/{era_name}.go`
Applied to files:
ledger/error_test.go
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to ledger/**/rules.go : Later eras must call parent era functions for shared validation logic, not duplicate logic
Applied to files:
ledger/error_test.go
📚 Learning: 2026-01-13T02:47:14.599Z
Learnt from: CR
Repo: blinklabs-io/gouroboros PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-13T02:47:14.599Z
Learning: Applies to ledger/common/certs.go : Define certificate structs in `ledger/common/certs.go` with CBOR tags before adding new certificate types
Applied to files:
cbor/value.goledger/common/certs.go
🧬 Code graph analysis (37)
ledger/shelley/pparams.go (1)
ledger/shelley.go (1)
ShelleyProtocolParameterUpdate(32-32)
ledger/common/witness.go (1)
ledger/common/address.go (2)
AddressPayloadKeyHash(691-693)AddressPayloadKeyHash(695-695)
ledger/shelley/genesis_test.go (1)
ledger/common/address.go (1)
Address(62-70)
ledger/babbage/pparams.go (1)
ledger/babbage.go (1)
BabbageProtocolParameterUpdate(31-31)
ledger/babbage/rules.go (1)
ledger/common/script.go (4)
PlutusV1Script(140-140)PlutusV1Script(142-142)PlutusV2Script(216-216)PlutusV2Script(218-218)
ledger/common/metadata_test.go (2)
ledger/common/metadata.go (4)
MetaMap(77-80)MetaMap(86-86)MetaInt(51-54)MetaInt(82-82)ledger/common/script/wrappers.go (1)
Pairs(46-46)
ledger/babbage/rules_test.go (3)
ledger/shelley.go (1)
ShelleyTransactionOutput(29-29)ledger/babbage/babbage.go (1)
BabbageTransactionOutput(556-563)ledger/babbage.go (1)
BabbageTransactionOutput(28-28)
ledger/leios/leios.go (1)
ledger/leios.go (1)
LeiosRankingBlock(26-26)
ledger/mary/rules_test.go (1)
ledger/shelley/shelley.go (1)
ShelleyTransactionOutput(433-438)
ledger/alonzo/pparams.go (1)
ledger/alonzo.go (1)
AlonzoProtocolParameterUpdate(31-31)
ledger/conway/pparams.go (2)
ledger/conway.go (1)
ConwayProtocolParameterUpdate(30-30)ledger/common/script/wrappers.go (1)
ToPlutusData(28-30)
ledger/common/verify.go (1)
ledger/common/address.go (2)
AddressPayloadKeyHash(691-693)AddressPayloadKeyHash(695-695)
ledger/conway/rules.go (2)
ledger/common/script.go (6)
PlutusV1Script(140-140)PlutusV1Script(142-142)PlutusV2Script(216-216)PlutusV2Script(218-218)PlutusV3Script(292-292)PlutusV3Script(294-294)ledger/common/address.go (4)
AddressPayloadKeyHash(691-693)AddressPayloadKeyHash(695-695)AddressPayloadScriptHash(697-699)AddressPayloadScriptHash(701-701)
ledger/shelley/rules_test.go (1)
ledger/shelley/shelley.go (1)
ShelleyTransactionOutput(433-438)
ledger/babbage/errors_test.go (1)
ledger/common/script.go (2)
PlutusV1Script(140-140)PlutusV1Script(142-142)
ledger/shelley/genesis.go (1)
ledger/shelley/shelley.go (1)
ShelleyTransactionOutput(433-438)
ledger/byron/genesis.go (3)
ledger/byron/byron.go (2)
ByronTransactionInput(681-685)ByronTransactionOutput(765-770)protocol/txsubmission/messages.go (1)
TxId(191-195)ledger/common/common.go (1)
Blake2b256Hash(90-102)
ledger/allegra/rules_test.go (1)
ledger/shelley/shelley.go (1)
ShelleyTransactionOutput(433-438)
ledger/conway/gov.go (1)
ledger/common/script/wrappers.go (1)
ToPlutusData(28-30)
ledger/shelley/rules.go (2)
ledger/common/metadata.go (10)
MetaText(61-64)MetaText(84-84)MetaBytes(56-59)MetaBytes(83-83)MetaInt(51-54)MetaInt(82-82)MetaList(66-69)MetaList(85-85)MetaMap(77-80)MetaMap(86-86)ledger/common/address.go (4)
AddressPayloadKeyHash(691-693)AddressPayloadKeyHash(695-695)AddressPayloadScriptHash(697-699)AddressPayloadScriptHash(701-701)
ledger/conway/conway_test.go (1)
ledger/common/script.go (3)
Script(38-42)PlutusV1Script(140-140)PlutusV1Script(142-142)
ledger/error_test.go (3)
ledger/common/address.go (1)
Address(62-70)ledger/shelley/shelley.go (1)
ShelleyTransactionOutput(433-438)ledger/byron/byron.go (1)
ByronTransactionInput(681-685)
ledger/conway/rules_test.go (3)
ledger/common/script.go (2)
PlutusV1Script(140-140)PlutusV1Script(142-142)ledger/shelley/shelley.go (2)
ShelleyTransactionOutput(433-438)NewShelleyTransactionInput(388-400)ledger/babbage/babbage.go (1)
BabbageTransactionOutput(556-563)
ledger/byron/genesis_test.go (1)
ledger/common/address.go (1)
Address(62-70)
ledger/leios/pparams.go (1)
ledger/leios.go (1)
LeiosProtocolParameterUpdate(31-31)
ledger/common/script_test.go (1)
ledger/common/script.go (1)
Script(38-42)
ledger/conway/errors_test.go (1)
ledger/common/script.go (6)
PlutusV2Script(216-216)PlutusV2Script(218-218)PlutusV1Script(140-140)PlutusV1Script(142-142)PlutusV3Script(292-292)PlutusV3Script(294-294)
ledger/conway/conway.go (3)
ledger/common/redeemer.go (3)
RedeemerKey(32-36)RedeemerValue(38-42)RedeemerTag(21-21)ledger/common/witness.go (2)
VkeyWitness(21-25)BootstrapWitness(27-33)ledger/common/tx.go (4)
TransactionWitnessRedeemers(98-102)TransactionInput(66-72)TransactionOutput(74-85)Utxo(104-108)
ledger/common/script.go (2)
cbor/decode.go (1)
Decode(29-44)ledger/common/common.go (1)
Blake2b224Hash(151-163)
ledger/byron/byron.go (1)
ledger/byron.go (5)
ByronTransaction(28-28)NewByronTransactionInput(48-48)ByronTransactionInput(29-29)ByronTransactionOutput(30-30)ByronEpochBoundaryBlock(24-24)
ledger/alonzo/rules_test.go (2)
ledger/shelley/shelley.go (1)
ShelleyTransactionOutput(433-438)ledger/alonzo/alonzo.go (1)
AlonzoTransactionOutput(351-358)
ledger/common/address.go (2)
ledger/compat.go (2)
AddrKeyHash(41-41)Address(40-40)ledger/common/script.go (1)
ScriptHash(36-36)
ledger/common/certs.go (1)
ledger/compat.go (3)
StakeDelegationCertificate(68-68)PoolRegistrationCertificate(67-67)PoolRetirementCertificate(66-66)
ledger/allegra/allegra.go (1)
ledger/common/pparams.go (1)
ProtocolParameterUpdate(24-27)
ledger/shelley/shelley.go (2)
ledger/common/script/wrappers.go (1)
ToPlutusData(28-30)ledger/common/tx.go (3)
TransactionInput(66-72)TransactionOutput(74-85)Utxo(104-108)
ledger/mary/mary.go (4)
ledger/common/tx.go (3)
TransactionInput(66-72)TransactionOutput(74-85)Utxo(104-108)ledger/common/pparams.go (1)
ProtocolParameterUpdate(24-27)ledger/common/certs.go (1)
Certificate(128-133)ledger/common/script.go (2)
ScriptRef(66-69)Script(38-42)
ledger/babbage/babbage.go (2)
ledger/common/script/wrappers.go (1)
ToPlutusData(28-30)ledger/common/tx.go (3)
TransactionInput(66-72)TransactionOutput(74-85)Utxo(104-108)
⏰ 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). (2)
- GitHub Check: go-test (1.24.x, ubuntu-latest)
- GitHub Check: cubic · AI code reviewer
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
9 issues found across 52 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="internal/test/conformance/conformance_test.go">
<violation number="1" location="internal/test/conformance/conformance_test.go:1076">
P1: Taking the address of the reused range variable makes all UTxOs share the same output pointer, corrupting decoded UTxOs.</violation>
</file>
<file name="ledger/babbage/babbage.go">
<violation number="1" location="ledger/babbage/babbage.go:416">
P1: All entries in ProtocolParameterUpdates map share pointer to last loop value due to taking address of reused range variable</violation>
</file>
<file name="ledger/error.go">
<violation number="1" location="ledger/error.go:692">
P1: Addresses the reused range variable in Byron UTxO loop, so all Id pointers can alias to the last input.</violation>
</file>
<file name="ledger/conway/conway.go">
<violation number="1" location="ledger/conway/conway.go:529">
P1: Range loop stores address of reused loop variable, so all protocol parameter updates alias the last value.</violation>
</file>
<file name="protocol/common/dmq.go">
<violation number="1" location="protocol/common/dmq.go:164">
P1: Potential nil-pointer panic: typed-nil *RejectReasonData is matched and dereferenced without a nil check</violation>
</file>
<file name="ledger/mary/mary.go">
<violation number="1" location="ledger/mary/mary.go:264">
P1: Range variable address reused: all map entries point to last update value</violation>
</file>
<file name="ledger/alonzo/alonzo.go">
<violation number="1" location="ledger/alonzo/alonzo.go:298">
P1: All map entries share the same protocol update pointer due to taking address of the reused range variable</violation>
</file>
<file name="ledger/common/address.go">
<violation number="1" location="ledger/common/address.go:631">
P1: Changing MarshalJSON to a pointer receiver removes json.Marshaler for Address values, causing json.Marshal(Address) to emit an empty object and breaking usage on non-addressable values.</violation>
</file>
<file name="cbor/value.go">
<violation number="1" location="cbor/value.go:353">
P1: Constructor.MarshalJSON moved to pointer receiver but generateAstJson marshals a value, so MarshalJSON is skipped and constructors serialize as `{}`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| OutputIndex: uint32(utxoId.Idx), | ||
| }, | ||
| Output: output, | ||
| Output: &output, |
Contributor
There was a problem hiding this comment.
P1: Taking the address of the reused range variable makes all UTxOs share the same output pointer, corrupting decoded UTxOs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/test/conformance/conformance_test.go, line 1076:
<comment>Taking the address of the reused range variable makes all UTxOs share the same output pointer, corrupting decoded UTxOs.</comment>
<file context>
@@ -1073,7 +1073,7 @@ func (s *initialStateUtxos) UnmarshalCBOR(data []byte) error {
OutputIndex: uint32(utxoId.Idx),
},
- Output: output,
+ Output: &output,
})
}
</file context>
| updateMap := make(map[common.Blake2b224]common.ProtocolParameterUpdate) | ||
| for k, v := range b.Update.ProtocolParamUpdates { | ||
| updateMap[k] = v | ||
| updateMap[k] = &v |
Contributor
There was a problem hiding this comment.
P1: All entries in ProtocolParameterUpdates map share pointer to last loop value due to taking address of reused range variable
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ledger/babbage/babbage.go, line 416:
<comment>All entries in ProtocolParameterUpdates map share pointer to last loop value due to taking address of reused range variable</comment>
<file context>
@@ -413,7 +413,7 @@ func (b *BabbageTransactionBody) ProtocolParameterUpdates() (uint64, map[common.
updateMap := make(map[common.Blake2b224]common.ProtocolParameterUpdate)
for k, v := range b.Update.ProtocolParamUpdates {
- updateMap[k] = v
+ updateMap[k] = &v
}
return b.Update.Epoch, updateMap
</file context>
Suggested change
| updateMap[k] = &v | |
| for k, v := range b.Update.ProtocolParamUpdates { | |
| val := v | |
| updateMap[k] = &val | |
| } |
|
|
||
| e.Utxos = append(e.Utxos, common.Utxo{ | ||
| Id: input, | ||
| Id: &input, |
Contributor
There was a problem hiding this comment.
P1: Addresses the reused range variable in Byron UTxO loop, so all Id pointers can alias to the last input.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ledger/error.go, line 692:
<comment>Addresses the reused range variable in Byron UTxO loop, so all Id pointers can alias to the last input.</comment>
<file context>
@@ -689,7 +689,7 @@ func (e *ScriptsNotPaidUtxo) UnmarshalCBOR(data []byte) error {
e.Utxos = append(e.Utxos, common.Utxo{
- Id: input,
+ Id: &input,
Output: output,
})
</file context>
| updateMap := make(map[common.Blake2b224]common.ProtocolParameterUpdate) | ||
| for k, v := range b.Update.ProtocolParamUpdates { | ||
| updateMap[k] = v | ||
| updateMap[k] = &v |
Contributor
There was a problem hiding this comment.
P1: Range loop stores address of reused loop variable, so all protocol parameter updates alias the last value.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ledger/conway/conway.go, line 529:
<comment>Range loop stores address of reused loop variable, so all protocol parameter updates alias the last value.</comment>
<file context>
@@ -526,7 +526,7 @@ func (b *ConwayTransactionBody) ProtocolParameterUpdates() (uint64, map[common.B
updateMap := make(map[common.Blake2b224]common.ProtocolParameterUpdate)
for k, v := range b.Update.ProtocolParamUpdates {
- updateMap[k] = v
+ updateMap[k] = &v
}
return b.Update.Epoch, updateMap
</file context>
Suggested change
| updateMap[k] = &v | |
| vCopy := v | |
| updateMap[k] = &vCopy |
Comment on lines
+164
to
+165
| case *RejectReasonData: | ||
| return *v |
Contributor
There was a problem hiding this comment.
P1: Potential nil-pointer panic: typed-nil *RejectReasonData is matched and dereferenced without a nil check
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At protocol/common/dmq.go, line 164:
<comment>Potential nil-pointer panic: typed-nil *RejectReasonData is matched and dereferenced without a nil check</comment>
<file context>
@@ -161,16 +161,16 @@ func ToRejectReasonData(rr RejectReason) RejectReasonData {
return RejectReasonData{Type: 3, Message: v.Message}
- case RejectReasonData:
- return v
+ case *RejectReasonData:
+ return *v
default:
</file context>
Suggested change
| case *RejectReasonData: | |
| return *v | |
| case *RejectReasonData: | |
| if v == nil { | |
| return RejectReasonData{Type: 3} | |
| } | |
| return *v |
| updateMap := make(map[common.Blake2b224]common.ProtocolParameterUpdate) | ||
| for k, v := range b.Update.ProtocolParamUpdates { | ||
| updateMap[k] = v | ||
| updateMap[k] = &v |
Contributor
There was a problem hiding this comment.
P1: Range variable address reused: all map entries point to last update value
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ledger/mary/mary.go, line 264:
<comment>Range variable address reused: all map entries point to last update value</comment>
<file context>
@@ -261,7 +261,7 @@ func (b *MaryTransactionBody) ProtocolParameterUpdates() (uint64, map[common.Bla
updateMap := make(map[common.Blake2b224]common.ProtocolParameterUpdate)
for k, v := range b.Update.ProtocolParamUpdates {
- updateMap[k] = v
+ updateMap[k] = &v
}
return b.Update.Epoch, updateMap
</file context>
Suggested change
| updateMap[k] = &v | |
| vCopy := v | |
| updateMap[k] = &vCopy |
| updateMap := make(map[common.Blake2b224]common.ProtocolParameterUpdate) | ||
| for k, v := range b.Update.ProtocolParamUpdates { | ||
| updateMap[k] = v | ||
| updateMap[k] = &v |
Contributor
There was a problem hiding this comment.
P1: All map entries share the same protocol update pointer due to taking address of the reused range variable
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ledger/alonzo/alonzo.go, line 298:
<comment>All map entries share the same protocol update pointer due to taking address of the reused range variable</comment>
<file context>
@@ -295,7 +295,7 @@ func (b *AlonzoTransactionBody) ProtocolParameterUpdates() (uint64, map[common.B
updateMap := make(map[common.Blake2b224]common.ProtocolParameterUpdate)
for k, v := range b.Update.ProtocolParamUpdates {
- updateMap[k] = v
+ updateMap[k] = &v
}
return b.Update.Epoch, updateMap
</file context>
| } | ||
|
|
||
| func (a Address) MarshalJSON() ([]byte, error) { | ||
| func (a *Address) MarshalJSON() ([]byte, error) { |
Contributor
There was a problem hiding this comment.
P1: Changing MarshalJSON to a pointer receiver removes json.Marshaler for Address values, causing json.Marshal(Address) to emit an empty object and breaking usage on non-addressable values.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ledger/common/address.go, line 631:
<comment>Changing MarshalJSON to a pointer receiver removes json.Marshaler for Address values, causing json.Marshal(Address) to emit an empty object and breaking usage on non-addressable values.</comment>
<file context>
@@ -628,7 +628,7 @@ func (a Address) String() string {
}
-func (a Address) MarshalJSON() ([]byte, error) {
+func (a *Address) MarshalJSON() ([]byte, error) {
return []byte(`"` + a.String() + `"`), nil
}
</file context>
Suggested change
| func (a *Address) MarshalJSON() ([]byte, error) { | |
| func (a Address) MarshalJSON() ([]byte, error) { |
| } | ||
|
|
||
| func (v Constructor) MarshalJSON() ([]byte, error) { | ||
| func (v *Constructor) MarshalJSON() ([]byte, error) { |
Contributor
There was a problem hiding this comment.
P1: Constructor.MarshalJSON moved to pointer receiver but generateAstJson marshals a value, so MarshalJSON is skipped and constructors serialize as {}.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cbor/value.go, line 353:
<comment>Constructor.MarshalJSON moved to pointer receiver but generateAstJson marshals a value, so MarshalJSON is skipped and constructors serialize as `{}`.</comment>
<file context>
@@ -350,7 +350,7 @@ func (c Constructor) MarshalCBOR() ([]byte, error) {
}
-func (v Constructor) MarshalJSON() ([]byte, error) {
+func (v *Constructor) MarshalJSON() ([]byte, error) {
var sb strings.Builder
sb.WriteString(fmt.Sprintf(`{"constructor":%d,"fields":[`, v.constructor))
</file context>
Suggested change
| func (v *Constructor) MarshalJSON() ([]byte, error) { | |
| func (v Constructor) MarshalJSON() ([]byte, error) { |
Contributor
Author
|
@coderabbitai @cubic-dev-ai |
Contributor
@cryptodj413 I have started the AI code review. It will take a few minutes to complete. |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🌐 Web query: 💡 Result: Short summary — Go 1.22 changes for-loop variable semantics to be per-iteration:
Sources: Go blog (Fixing For Loops in Go 1.22) [1]; proposal/issue and design doc (less error-prone loop variable scoping) [2]; Go 1.22 release notes [3]. 🏁 Script executed: Length of output: 1659 🏁 Script executed: Length of output: 1738 🏁 Script executed: Length of output: 954 🏁 Script executed: Length of output: 922 🏁 Script executed: Length of output: 722
Go 1.22 introduced per-iteration loop variables, where loop variables are created anew on each iteration instead of being shared across iterations. This is critical for the patterns used in this PR. Key Findings✅ The code is correct for Go 1.24.0 (declared in go.mod) The PR contains several patterns that rely on Go 1.22+ loop semantics: 1. Safe pattern in for idx, output := range outputs {
id := NewByronTransactionInput(t.Id().String(), idx)
ret = append(
ret,
common.Utxo{
Id: &id, // Taking address of loop-scoped variable
Output: output,
},
)
}Why this works: With Go 1.22+, 2. Safe pattern with slice indexing: for i := range t.TxInputs {
ret = append(ret, &t.TxInputs[i])
}Why this works: This pattern is safe in all Go versions because we're taking the address of a slice element, not a loop variable. 3. Safe pattern in ConwayTransactionBody.ProposalProcedures(): for i := range b.TxProposalProcedures {
ret[i] = &b.TxProposalProcedures[i]
}Important Note
The changes align well with the |
Contributor
There was a problem hiding this comment.
8 issues found across 52 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="ledger/mary/mary.go">
<violation number="1" location="ledger/mary/mary.go:264">
P1: All map entries point to the same update because &v takes the address of the reused range variable</violation>
</file>
<file name="protocol/common/dmq.go">
<violation number="1" location="protocol/common/dmq.go:164">
P1: Dereferences a typed-nil *RejectReasonData in ToRejectReasonData, causing panic</violation>
</file>
<file name="ledger/alonzo/alonzo.go">
<violation number="1" location="ledger/alonzo/alonzo.go:298">
P1: Range variable address escapes: map stores pointers to reused loop variable so all entries can reference the last update</violation>
</file>
<file name="internal/test/conformance/conformance_test.go">
<violation number="1" location="internal/test/conformance/conformance_test.go:1076">
P1: All UTxOs take the address of the reused range variable `output`, so every `Output` pointer aliases the same value (last iteration) instead of the per-UTxO output.</violation>
</file>
<file name="ledger/conway/conway.go">
<violation number="1" location="ledger/conway/conway.go:529">
P1: Map range takes address of loop variable, so all updates map entries share the last value</violation>
</file>
<file name="ledger/babbage/babbage.go">
<violation number="1" location="ledger/babbage/babbage.go:416">
P1: Protocol parameter updates map stores pointer to reused range variable, causing every entry to reference the last update</violation>
</file>
<file name="ledger/shelley/shelley.go">
<violation number="1" location="ledger/shelley/shelley.go:309">
P1: Range variable address stored; all map entries point to the same update</violation>
</file>
<file name="cbor/value.go">
<violation number="1" location="cbor/value.go:353">
P1: Constructor MarshalJSON changed to pointer receiver but callers still marshal Constructor values, so custom JSON output is bypassed</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| updateMap := make(map[common.Blake2b224]common.ProtocolParameterUpdate) | ||
| for k, v := range b.Update.ProtocolParamUpdates { | ||
| updateMap[k] = v | ||
| updateMap[k] = &v |
Contributor
There was a problem hiding this comment.
P1: All map entries point to the same update because &v takes the address of the reused range variable
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ledger/mary/mary.go, line 264:
<comment>All map entries point to the same update because &v takes the address of the reused range variable</comment>
<file context>
@@ -261,7 +261,7 @@ func (b *MaryTransactionBody) ProtocolParameterUpdates() (uint64, map[common.Bla
updateMap := make(map[common.Blake2b224]common.ProtocolParameterUpdate)
for k, v := range b.Update.ProtocolParamUpdates {
- updateMap[k] = v
+ updateMap[k] = &v
}
return b.Update.Epoch, updateMap
</file context>
| return RejectReasonData{Type: 3, Message: v.Message} | ||
| case RejectReasonData: | ||
| return v | ||
| case *RejectReasonData: |
Contributor
There was a problem hiding this comment.
P1: Dereferences a typed-nil *RejectReasonData in ToRejectReasonData, causing panic
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At protocol/common/dmq.go, line 164:
<comment>Dereferences a typed-nil *RejectReasonData in ToRejectReasonData, causing panic</comment>
<file context>
@@ -161,16 +161,16 @@ func ToRejectReasonData(rr RejectReason) RejectReasonData {
return RejectReasonData{Type: 3, Message: v.Message}
- case RejectReasonData:
- return v
+ case *RejectReasonData:
+ return *v
default:
</file context>
| updateMap := make(map[common.Blake2b224]common.ProtocolParameterUpdate) | ||
| for k, v := range b.Update.ProtocolParamUpdates { | ||
| updateMap[k] = v | ||
| updateMap[k] = &v |
Contributor
There was a problem hiding this comment.
P1: Range variable address escapes: map stores pointers to reused loop variable so all entries can reference the last update
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ledger/alonzo/alonzo.go, line 298:
<comment>Range variable address escapes: map stores pointers to reused loop variable so all entries can reference the last update</comment>
<file context>
@@ -295,7 +295,7 @@ func (b *AlonzoTransactionBody) ProtocolParameterUpdates() (uint64, map[common.B
updateMap := make(map[common.Blake2b224]common.ProtocolParameterUpdate)
for k, v := range b.Update.ProtocolParamUpdates {
- updateMap[k] = v
+ updateMap[k] = &v
}
return b.Update.Epoch, updateMap
</file context>
Suggested change
| updateMap[k] = &v | |
| vv := v | |
| updateMap[k] = &vv |
| OutputIndex: uint32(utxoId.Idx), | ||
| }, | ||
| Output: output, | ||
| Output: &output, |
Contributor
There was a problem hiding this comment.
P1: All UTxOs take the address of the reused range variable output, so every Output pointer aliases the same value (last iteration) instead of the per-UTxO output.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/test/conformance/conformance_test.go, line 1076:
<comment>All UTxOs take the address of the reused range variable `output`, so every `Output` pointer aliases the same value (last iteration) instead of the per-UTxO output.</comment>
<file context>
@@ -1073,7 +1073,7 @@ func (s *initialStateUtxos) UnmarshalCBOR(data []byte) error {
OutputIndex: uint32(utxoId.Idx),
},
- Output: output,
+ Output: &output,
})
}
</file context>
| updateMap := make(map[common.Blake2b224]common.ProtocolParameterUpdate) | ||
| for k, v := range b.Update.ProtocolParamUpdates { | ||
| updateMap[k] = v | ||
| updateMap[k] = &v |
Contributor
There was a problem hiding this comment.
P1: Map range takes address of loop variable, so all updates map entries share the last value
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ledger/conway/conway.go, line 529:
<comment>Map range takes address of loop variable, so all updates map entries share the last value</comment>
<file context>
@@ -526,7 +526,7 @@ func (b *ConwayTransactionBody) ProtocolParameterUpdates() (uint64, map[common.B
updateMap := make(map[common.Blake2b224]common.ProtocolParameterUpdate)
for k, v := range b.Update.ProtocolParamUpdates {
- updateMap[k] = v
+ updateMap[k] = &v
}
return b.Update.Epoch, updateMap
</file context>
Suggested change
| updateMap[k] = &v | |
| for k, v := range b.Update.ProtocolParamUpdates { | |
| vv := v | |
| updateMap[k] = &vv | |
| } |
| updateMap := make(map[common.Blake2b224]common.ProtocolParameterUpdate) | ||
| for k, v := range b.Update.ProtocolParamUpdates { | ||
| updateMap[k] = v | ||
| updateMap[k] = &v |
Contributor
There was a problem hiding this comment.
P1: Protocol parameter updates map stores pointer to reused range variable, causing every entry to reference the last update
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ledger/babbage/babbage.go, line 416:
<comment>Protocol parameter updates map stores pointer to reused range variable, causing every entry to reference the last update</comment>
<file context>
@@ -413,7 +413,7 @@ func (b *BabbageTransactionBody) ProtocolParameterUpdates() (uint64, map[common.
updateMap := make(map[common.Blake2b224]common.ProtocolParameterUpdate)
for k, v := range b.Update.ProtocolParamUpdates {
- updateMap[k] = v
+ updateMap[k] = &v
}
return b.Update.Epoch, updateMap
</file context>
Suggested change
| updateMap[k] = &v | |
| vCopy := v | |
| updateMap[k] = &vCopy |
| updateMap := make(map[common.Blake2b224]common.ProtocolParameterUpdate) | ||
| for k, v := range b.Update.ProtocolParamUpdates { | ||
| updateMap[k] = v | ||
| updateMap[k] = &v |
Contributor
There was a problem hiding this comment.
P1: Range variable address stored; all map entries point to the same update
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ledger/shelley/shelley.go, line 309:
<comment>Range variable address stored; all map entries point to the same update</comment>
<file context>
@@ -306,7 +306,7 @@ func (b *ShelleyTransactionBody) ProtocolParameterUpdates() (uint64, map[common.
updateMap := make(map[common.Blake2b224]common.ProtocolParameterUpdate)
for k, v := range b.Update.ProtocolParamUpdates {
- updateMap[k] = v
+ updateMap[k] = &v
}
return b.Update.Epoch, updateMap
</file context>
| } | ||
|
|
||
| func (v Constructor) MarshalJSON() ([]byte, error) { | ||
| func (v *Constructor) MarshalJSON() ([]byte, error) { |
Contributor
There was a problem hiding this comment.
P1: Constructor MarshalJSON changed to pointer receiver but callers still marshal Constructor values, so custom JSON output is bypassed
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cbor/value.go, line 353:
<comment>Constructor MarshalJSON changed to pointer receiver but callers still marshal Constructor values, so custom JSON output is bypassed</comment>
<file context>
@@ -350,7 +350,7 @@ func (c Constructor) MarshalCBOR() ([]byte, error) {
}
-func (v Constructor) MarshalJSON() ([]byte, error) {
+func (v *Constructor) MarshalJSON() ([]byte, error) {
var sb strings.Builder
sb.WriteString(fmt.Sprintf(`{"constructor":%d,"fields":[`, v.constructor))
</file context>
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
1 participant
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.
Closes #936
Summary by cubic
Enable recvcheck in golangci-lint and fix receiver usage across the codebase. Methods now consistently use pointer receivers, and call sites, type switches, and tests were updated to use pointer concrete types to ensure interface conformance and avoid unintended copies.
Refactors
Migration
Written for commit bd53e47. Summary will update on new commits.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.