-
Notifications
You must be signed in to change notification settings - Fork 8
chore(deps): gouroboros 0.142.0 and plutigo 0.0.16 #552
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThis PR updates module requirements (github.com/blinklabs-io/gouroboros → v0.142.0, github.com/blinklabs-io/plutigo → v0.0.16), adds Swagger template delimiters in docs/docs.go (LeftDelim: "{{", RightDelim: "}}"), changes TransactionEvent.Metadata from *cbor.LazyValue to lcommon.TransactionMetadatum and removes the cbor import, refactors metadata handling in output/push/push.go to operate on structured metadatum types with new helpers (metadatumToAny, keyToString), adds BlockBodyHash support to test mocks in input/chainsync/block_test.go, and includes minor formatting/import cleanups. Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 8 files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
docs/docs.go(2 hunks)event/tx.go(1 hunks)go.mod(1 hunks)input/chainsync/block_test.go(3 hunks)internal/config/config.go(1 hunks)openapi/test/api_default_test.go(1 hunks)output/push/push.go(3 hunks)
⏰ 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: cubic · AI code reviewer
- GitHub Check: Analyze (go)
🔇 Additional comments (9)
openapi/test/api_default_test.go (1)
12-19: LGTM!Import organization looks correct. The
testingimport is properly grouped with standard library imports.docs/docs.go (1)
175-176: LGTM!The addition of
LeftDelimandRightDelimfields with default Go template delimiters ("{{"and"}}") is consistent with regenerated swagger documentation. These are the default values and don't change template parsing behavior.input/chainsync/block_test.go (2)
34-34: LGTM!The
blockBodyHashfield and accessor method are correctly added toMockBlockHeader, following the same pattern as other hash fields (hash,prevHash).Also applies to: 69-71
95-97: LGTM!
MockBlock.BlockBodyHash()correctly delegates to the embeddedMockBlockHeader.BlockBodyHash(), maintaining consistency with the interface pattern.go.mod (1)
10-11: Dependency versions verified and appropriate for the PR objectives.Both gouroboros v0.142.0 and plutigo v0.0.16 are valid releases with proper checksums in go.sum. The plutigo bump to v0.0.16 supersedes the v0.0.15 mentioned in issue #550.
event/tx.go (1)
34-34: No action required. TheomitemptyJSON tag on theMetadatafield is correct for its type. The nil check and conditional assignment pattern (line 82) ensures that when metadata is not present, the field remains at its zero value and is properly omitted during JSON serialization.Likely an incorrect or invalid review comment.
output/push/push.go (2)
22-22: LGTM!Import additions align with the new gouroboros metadata types:
math/bigfor*big.Intcomparison andcommonpackage for the ledger metadata interfaces.Also applies to: 29-29
310-367: LGTM!The refactored function correctly uses gouroboros ledger metadata types with proper type assertions and structured iteration over
Pairs. The key lookups (674, "msg") are type-safe and the error handling is clear.internal/config/config.go (1)
76-85: LGTM!Formatting-only changes to align struct field tags. No functional impact.
fdf3a7c to
f85b99b
Compare
Signed-off-by: Chris Gianelloni <[email protected]>
f85b99b to
cb88405
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
output/push/push.go (1)
17-25: Fix hex‑format lint by usingencoding/hexinstead offmt.Sprintf
keyToStringcurrently formatsMetaByteskeys withfmt.Sprintf("%x", v.Value), which golangci‑lint (perfsprint/hex-format) flags. Switching tohex.EncodeToStringis both faster and will satisfy the linter.@@ -import ( - "context" - "encoding/json" - "errors" - "fmt" - "math/big" - "os" +import ( + "context" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "math/big" + "os" @@ func keyToString(md common.TransactionMetadatum) string { switch v := md.(type) { case common.MetaInt: return v.Value.String() case common.MetaBytes: - return fmt.Sprintf("%x", v.Value) + return hex.EncodeToString(v.Value) case common.MetaText: return v.Value default: return fmt.Sprintf("%v", metadatumToAny(md)) } }Also applies to: 369-380
🧹 Nitpick comments (2)
input/chainsync/block_test.go (1)
24-35: NewblockBodyHashfield is consistent; consider populating it in test fixturesAdding
blockBodyHash common.Blake2b256toMockBlockHeadermatches the existing hash fields and keeps the mock aligned with the header interface. To fully exercise any logic that consumes the block body hash (e.g., inevent.NewBlockContext), consider setting a non‑zeroblockBodyHashin at least one test case and asserting the derived behavior where applicable.output/push/push.go (1)
149-159: CIP‑20 extraction call site is wired correctly; consider using logger instead offmt.PrintlnThe new
te.Metadata != nilguard and call toextractCIP20FromMetadata(te.Metadata)correctly reflect the new metadata representation and avoid nil dereferences. On error you currentlyfmt.Println("Error:", err); for consistency with the rest of this file, consider logging this vialogging.GetLogger().Error(...)instead of printing to stdout, so failures in CIP‑20 parsing are surfaced in structured logs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
docs/docs.go(2 hunks)event/tx.go(1 hunks)go.mod(1 hunks)input/chainsync/block_test.go(3 hunks)internal/config/config.go(1 hunks)openapi/test/api_default_test.go(1 hunks)output/push/push.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- go.mod
- docs/docs.go
- openapi/test/api_default_test.go
🧰 Additional context used
🪛 GitHub Actions: golangci-lint
output/push/push.go
[error] 374-374: golangci-lint: hex-format: fmt.Sprintf can be replaced with faster hex.EncodeToString (perfsprint)
🪛 GitHub Check: lint
output/push/push.go
[failure] 374-374:
hex-format: fmt.Sprintf can be replaced with faster hex.EncodeToString (perfsprint)
⏰ 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 (5)
input/chainsync/block_test.go (2)
69-71: HeaderBlockBodyHashaccessor matches existing patternThe
MockBlockHeader.BlockBodyHash()implementation is straightforward and consistent with the other accessors (Hash,PrevHash, etc.), so the header mock should cleanly satisfy the updated interface.
95-97: Block-levelBlockBodyHashpassthrough looks goodDelegating
MockBlock.BlockBodyHash()toMockBlockHeader.BlockBodyHash()is the right choice and keepsMockBlockbehavior consistent with the embedded header.internal/config/config.go (1)
75-88: Config tag alignment is cosmetic and safeOnly horizontal spacing in the struct tags was adjusted; YAML and
envconfigtag values are unchanged. No behavioral impact, and the change is fine as is.event/tx.go (1)
30-44: Verify JSON marshalling behavior ofTransactionMetadatuminterface in downstream codeThe type migration of
TransactionEvent.Metadatafrom*cbor.LazyValuetolcommon.TransactionMetadatumis architecturally consistent with the new gouroboros API. However, confirm that:
- All concrete implementations of
TransactionMetadatumproperly implementjson.Marshaleror have correct struct tags for JSON serialization- Any code paths that directly serialize
TransactionEvent(e.g., in output/push handlers) produce expected JSON structure- Edge cases where
Metadatais nil are correctly omitted by theomitemptytagoutput/push/push.go (1)
310-367: CIP‑20 traversal overTransactionMetadatumand JSON conversion are correctly implementedThe
extractCIP20FromMetadata,metadatumToAny, andkeyToStringfunctions work together safely:
- Top-level metadata is enforced as a
MetaMapwith proper type assertion.- CIP‑20 label
674is located viaMetaIntkey withbig.Intcomparison.- The nested
"msg"key is retrieved from aMetaMapviaMetaTextlookup.- Recursive
metadatumToAnyconversion buildsmap[string]anystructures, ensuring JSON marshalling will not panic.keyToStringconsistently converts all metadata keys (MetaInt, MetaBytes, MetaText) to strings for safe map construction.This implementation safely handles typical CIP‑20 payloads without the previous
map[any]anyJSON issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
output/push/push.go (1)
383-408: Well-structured recursive conversion.The past review concern about JSON marshalling with non-string keys has been addressed—
MetaMapnow correctly builds amap[string]any.One consideration:
MetaBytesreturns[]byte, whichjson.Marshalwill encode as a base64 string. This is typically acceptable for metadata, but if hex encoding is preferred for consistency withkeyToString, you could returnhex.EncodeToString(v.Value)instead.If hex encoding is preferred for JSON output consistency:
case common.MetaBytes: - return v.Value + return hex.EncodeToString(v.Value)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
docs/docs.go(2 hunks)event/tx.go(1 hunks)go.mod(1 hunks)input/chainsync/block_test.go(3 hunks)internal/config/config.go(1 hunks)openapi/test/api_default_test.go(1 hunks)output/push/push.go(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- internal/config/config.go
🚧 Files skipped from review as they are similar to previous changes (3)
- openapi/test/api_default_test.go
- event/tx.go
- docs/docs.go
🔇 Additional comments (6)
input/chainsync/block_test.go (2)
34-34: LGTM!The new
blockBodyHashfield follows the existing pattern for other hash fields in the mock.
69-97: LGTM!The
BlockBodyHash()methods are correctly implemented:
MockBlockHeaderreturns the field value directlyMockBlockproperly delegates to its embedded headerThis satisfies the updated interface requirements from gouroboros v0.142.0.
output/push/push.go (3)
17-31: LGTM!The new imports are properly added and all are utilized in the refactored metadata handling code.
311-368: Clean refactoring of CIP-20 extraction.The function correctly adapts to the new
common.TransactionMetadatumtype system:
- Proper type assertion to
MetaMap- Correct use of
big.Intcomparison for the integer key 674- Iterative key lookup via
Pairsslice is appropriate for the MetaMap structure- Clear error messages at each validation step
370-381: LGTM!The
keyToStringhelper handles all expected Cardano metadata key types appropriately. The fallback tofmt.Sprintf("%v", metadatumToAny(md))provides a reasonable default for unexpected types.go.mod (1)
10-11: Dependency updates look good.The version bumps for
gouroboros(v0.142.0) andplutigo(v0.0.16) are both published and available, bringing in upstream improvements and fixes.
Closes #550
Closes #551
Summary by cubic
Upgraded gouroboros to 0.142.0 and plutigo to 0.0.16, updating metadata handling and CIP-20 parsing to the new ledger/common types. Added block body hash support in chain sync tests and set Swagger template delimiters.
Dependencies
Refactors
Written for commit cb88405. Summary will update automatically on new commits.
Summary by CodeRabbit
New Features
Refactor
Style
Tests
✏️ Tip: You can customize this high-level summary in your review settings.