-
Notifications
You must be signed in to change notification settings - Fork 6
feat(database): Track transaction Datum #1053
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
Signed-off-by: Akhil Repala <arepala@blinklabs.io>
📝 WalkthroughWalkthroughThis pull request adds datum tracking across three layers: a new Database method Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-10-24T21:56:15.978ZApplied to files:
🧬 Code graph analysis (1)database/plugin/metadata/sqlite/transaction.go (3)
⏰ 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). (3)
🔇 Additional comments (3)
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.
Actionable comments posted: 2
🧹 Nitpick comments (2)
database/datum.go (1)
45-47: Consider a more descriptive error message for invalid input.The error message "datum not found" for an empty hash is misleading, as this is an invalid input rather than a missing datum. Consider using
errors.New("invalid hash: hash cannot be empty")to clarify this is a validation error.database/plugin/metadata/sqlite/transaction.go (1)
727-736: Consider logging when datum marshaling produces empty data.The function silently returns
nilwhen a non-nil datum marshals to empty bytes (line 734-736). While this might be intentional for handling optional datums, it could mask unexpected cases where a datum should contain data but doesn't.Consider adding a debug log:
if len(rawDatum) == 0 { var err error rawDatum, err = datum.MarshalCBOR() if err != nil { return fmt.Errorf("marshal datum: %w", err) } } if len(rawDatum) == 0 { + d.logger.Debug("skipping empty datum", "slot", slot) return nil }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
database/datum.go(2 hunks)database/plugin/metadata/sqlite/transaction.go(1 hunks)ledger/state.go(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-24T21:56:15.978Z
Learnt from: wolf31o2
Repo: blinklabs-io/dingo PR: 975
File: database/models/transaction.go:21-26
Timestamp: 2025-10-24T21:56:15.978Z
Learning: In database/models/transaction.go, the Transaction struct uses a dual foreign-key pattern for UTXO associations: Outputs []Utxo uses TransactionID→ID (where UTXOs were created), and Inputs []Utxo uses SpentAtTxId→Hash (where UTXOs were consumed). Explicit GORM tags `gorm:"foreignKey:SpentAtTxId;references:Hash"` for Inputs and `gorm:"foreignKey:TransactionID;references:ID"` for Outputs prevent association conflicts and preserve UTXO provenance.
Applied to files:
database/plugin/metadata/sqlite/transaction.go
📚 Learning: 2025-10-22T20:19:26.360Z
Learnt from: wolf31o2
Repo: blinklabs-io/dingo PR: 971
File: utxorpc/sync_test.go:91-91
Timestamp: 2025-10-22T20:19:26.360Z
Learning: In utxorpc/sync_test.go, tests should not construct database/models.Block directly for sync server use. Instead, build a models.Block only as a container for CBOR and call models.Block.Decode() to obtain a github.com/blinklabs-io/gouroboros/ledger/common.Block (or the repository’s ledger.Block interface), and use that decoded value in the test.
Applied to files:
database/datum.go
🧬 Code graph analysis (3)
ledger/state.go (1)
database/models/datum.go (2)
Datum(17-22)Datum(24-26)
database/plugin/metadata/sqlite/transaction.go (3)
database/plugin/metadata/sqlite/database.go (1)
MetadataStoreSqlite(37-45)database/models/transaction.go (2)
Transaction(20-34)Transaction(36-38)database/models/datum.go (2)
Datum(17-22)Datum(24-26)
database/datum.go (4)
database/database.go (2)
Database(45-50)New(118-154)database/txn.go (1)
Txn(27-34)database/models/datum.go (2)
Datum(17-22)Datum(24-26)database/plugin/metadata/sqlite/database.go (1)
New(48-120)
⏰ 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 (4)
database/datum.go (1)
18-22: LGTM!The added imports support the new
GetDatumfunctionality appropriately.ledger/state.go (1)
198-201: LGTM!The
Datummethod provides a clean public API for datum retrieval, properly delegating to the database layer. The comment clearly indicates its purpose for implementing issue #741.database/plugin/metadata/sqlite/transaction.go (2)
685-690: LGTM!The integration of datum storage into
SetTransactionis well-placed after certificate processing, with proper error handling and propagation.
692-715: LGTM!The function correctly handles both inline datums from transaction outputs and witness-set datums. The copy at line 709 is good practice to avoid loop variable reuse issues when taking addresses.
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 3 files
Signed-off-by: Akhil Repala <arepala@blinklabs.io>
Closes #1015
Summary by cubic
Track and persist transaction datums in the metadata DB and add a lookup to fetch them by hash. This makes inline output and witness-set datums accessible via LedgerState.Datum.
Written for commit 1587c41. Summary will update automatically on new commits.
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.