feat: track witness set for transaction #1027
Conversation
Signed-off-by: Jenita <jkawan@blinklabs.io>
📝 WalkthroughWalkthroughAdds GORM models to persist transaction witness data: KeyWitness, WitnessScripts, Redeemer, PlutusData, and Script (script content) with TableName methods and appropriate gorm tags (primaryKey, indices, unique on script hash). Registers these models in MigrateModels and extends Transaction with associations for KeyWitnesses, WitnessScripts, Redeemers, and PlutusData. Updates SQLite persistence logic (database/plugin/metadata/sqlite/transaction.go) to ensure a transaction ID after upsert, delete existing associated witness/script/redeemer/plutus_data rows when present, and insert vkey/bootstrap witnesses, native and Plutus scripts (and Script content), datums (PlutusData), and redeemers (including ExUnits). Adds MetadataStore/GetScript and its sqlite implementation to fetch Script by hash. Adds workflow timeout-minutes: 10 to nilaway job. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (3 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)📓 Common learnings⏰ 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). (6)
🔇 Additional comments (1)
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: 0
🧹 Nitpick comments (2)
database/models/witness.go (1)
29-29: Consider using typed enum for stronger type safety.The
Typefield usesuint8for GORM compatibility, but usingWitnessTypedirectly would provide compile-time type safety while GORM still stores it as uint8 under the hood.Apply this diff if you want stronger typing:
- Type uint8 `gorm:"index"` // WitnessType (0=Vkey, 1=Bootstrap) + Type WitnessType `gorm:"index"`The same suggestion applies to
Script.TypeandRedeemer.Tagin their respective files.database/models/redeemer.go (1)
32-34: Consider adding compound index for redeemer lookups.Queries that filter redeemers by tag and index within a transaction would benefit from a compound index. The combination
(TransactionID, Tag, Index)is likely a common query pattern.Apply this diff to add a compound index:
type Redeemer struct { ID uint `gorm:"primaryKey"` - TransactionID uint `gorm:"index"` - Tag uint8 `gorm:"index"` // RedeemerTag - Index uint32 `gorm:"index"` + TransactionID uint `gorm:"index;index:idx_redeemer_lookup,priority:1"` + Tag uint8 `gorm:"index:idx_redeemer_lookup,priority:2"` // RedeemerTag + Index uint32 `gorm:"index:idx_redeemer_lookup,priority:3"` Data []byte `gorm:"type:bytea"` // Plutus dataThis maintains the individual indexes while adding a compound index for efficient lookup by (TransactionID, Tag, Index).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
database/models/datum.go(1 hunks)database/models/models.go(1 hunks)database/models/redeemer.go(1 hunks)database/models/script.go(1 hunks)database/models/transaction.go(1 hunks)database/models/witness.go(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
📚 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/models/transaction.godatabase/models/redeemer.go
📚 Learning: 2025-10-26T14:12:53.587Z
Learnt from: wolf31o2
Repo: blinklabs-io/dingo PR: 977
File: database/plugin/metadata/sqlite/transaction.go:71-81
Timestamp: 2025-10-26T14:12:53.587Z
Learning: In Babbage-era transactions (gouroboros ledger), the Produced() method behavior depends on transaction validity: for valid transactions (IsValid() == true), Produced() returns all transaction outputs; for invalid transactions (IsValid() == false), Produced() returns only the collateral return UTXO (if non-nil) with index len(t.Outputs()). The collateral return Output is set to t.CollateralReturn() in the returned Utxo struct.
Applied to files:
database/models/transaction.go
🧬 Code graph analysis (6)
database/models/witness.go (1)
database/models/transaction.go (2)
Transaction(18-34)Transaction(36-38)
database/models/transaction.go (5)
database/models/utxo.go (1)
Utxo(23-38)database/models/witness.go (2)
Witness(26-36)Witness(38-40)database/models/script.go (2)
Script(28-34)Script(36-38)database/models/redeemer.go (2)
Redeemer(30-39)Redeemer(41-43)database/models/datum.go (2)
PlutusData(29-34)PlutusData(36-38)
database/models/datum.go (1)
database/models/transaction.go (2)
Transaction(18-34)Transaction(36-38)
database/models/script.go (1)
database/models/transaction.go (2)
Transaction(18-34)Transaction(36-38)
database/models/redeemer.go (1)
database/models/transaction.go (2)
Transaction(18-34)Transaction(36-38)
database/models/models.go (4)
database/models/witness.go (2)
Witness(26-36)Witness(38-40)database/models/script.go (2)
Script(28-34)Script(36-38)database/models/redeemer.go (2)
Redeemer(30-39)Redeemer(41-43)database/models/datum.go (2)
PlutusData(29-34)PlutusData(36-38)
⏰ 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: nilaway
- GitHub Check: Analyze (go)
🔇 Additional comments (6)
database/models/transaction.go (1)
26-29: LGTM! Witness set relationships properly defined.The foreign key relationships for witness set components correctly use
TransactionID→ID, which is appropriate since these entities are owned by the transaction (unlike inputs which reference by hash). The pattern is consistent across all four new relationships and aligns with the child model definitions.database/models/models.go (1)
51-54: LGTM! All new models registered for migration.All four witness set models are correctly added to the migration list, ensuring database schema updates will include the new tables.
database/models/datum.go (1)
28-38: LGTM! PlutusData model correctly structured.The model follows GORM conventions with proper primary key, indexed foreign key, and bytea type for binary data. The back-reference to Transaction enables bidirectional navigation.
database/models/script.go (1)
20-25: Based on my verification, the ScriptType enum values in the reviewed code appear to align with Cardano's ledger implementation. The cardano-ledger Language type usestoCBOR = toCBOR . fromEnumandfromCBOR = toEnum . fromIntegral <$> decodeWord64, indicating CBOR encoding uses enum ordinal values. The PlutusScript encoding shows PlutusV1 at CBOR value 1, and the pattern continues sequentially. The values specified in the review (Native=0, PlutusV1=1, PlutusV2=2, PlutusV3=3) follow the expected sequential encoding consistent with Cardano's script type serialization.database/models/witness.go (1)
20-23: Enum values are correctly aligned with Cardano specification.Verification confirms that
WitnessTypeVkey = 0andWitnessTypeBootstrap = 1match the Cardano witness type specification. Vkey (type 0) represents the classic Shelley-era Ed25519 witness, and Bootstrap (type 1) represents the legacy Byron-era witness structure. The enum definition in database/models/witness.go is correct, and the struct comment accurately documents this mapping.database/models/redeemer.go (1)
20-27: RedeemerTag enum values verified as correct against Cardano specification.The enum values (0—Spend, 1—Mint, 2—Cert, 3—Reward, 4—Voting, 5—Proposing) precisely match the Cardano CDDL specification and ledger types. The Conway-era governance tags (Voting=4, Proposing=5) are properly defined. The direct type cast to
cardano.RedeemerPurposeinutxorpc/submit.goline 217 is semantically correct because the local enum values align with the utxorpc library's standards. The implementation is consistent across the codebase (database models, conway.go, submit.go) with no type conversion issues.
agaffney
left a comment
agaffney
left a comment
There was a problem hiding this comment.
There's nothing inserting data into these tables
database/models/redeemer.go
Outdated
| RedeemerTagReward RedeemerTag = 3 | ||
| RedeemerTagVoting RedeemerTag = 4 | ||
| RedeemerTagProposing RedeemerTag = 5 | ||
| ) |
There was a problem hiding this comment.
We should reuse the constants from gouroboros/ledger/common
Signed-off-by: Jenita <jkawan@blinklabs.io>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
database/models/witness.go (1)
17-33: KeyWitness model looks good; consider typed constants forTypeStruct layout and GORM tags look consistent with the rest of the models. To avoid magic numbers for
Typein both the model and the SQLite persistence code, consider adding typed constants here and reusing them at call sites.package models -// KeyWitness represents a key witness entry (Vkey or Bootstrap) -// Type: 0 = VkeyWitness, 1 = BootstrapWitness +// KeyWitnessType values identify the concrete witness encoding. +const ( + KeyWitnessTypeVkey uint8 = 0 + KeyWitnessTypeBootstrap uint8 = 1 +) + +// KeyWitness represents a key witness entry (Vkey or Bootstrap). +// See KeyWitnessType* constants for Type values. type KeyWitness struct { @@ TransactionID uint `gorm:"index"` - Type uint8 `gorm:"index"` // 0=Vkey, 1=Bootstrap + Type uint8 `gorm:"index"` // KeyWitnessTypeVkey / KeyWitnessTypeBootstrapThen in
database/plugin/metadata/sqlite/transaction.go, you can referencemodels.KeyWitnessTypeVkey/models.KeyWitnessTypeBootstrapinstead of raw0/1.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
database/models/models.go(1 hunks)database/models/redeemer.go(1 hunks)database/models/script.go(1 hunks)database/models/transaction.go(1 hunks)database/models/witness.go(1 hunks)database/plugin/metadata/sqlite/transaction.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- database/models/models.go
- database/models/script.go
- database/models/redeemer.go
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
📚 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.godatabase/models/witness.godatabase/models/transaction.go
📚 Learning: 2025-10-26T14:12:53.587Z
Learnt from: wolf31o2
Repo: blinklabs-io/dingo PR: 977
File: database/plugin/metadata/sqlite/transaction.go:71-81
Timestamp: 2025-10-26T14:12:53.587Z
Learning: In Babbage-era transactions (gouroboros ledger), the Produced() method behavior depends on transaction validity: for valid transactions (IsValid() == true), Produced() returns all transaction outputs; for invalid transactions (IsValid() == false), Produced() returns only the collateral return UTXO (if non-nil) with index len(t.Outputs()). The collateral return Output is set to t.CollateralReturn() in the returned Utxo struct.
Applied to files:
database/models/transaction.go
🧬 Code graph analysis (3)
database/plugin/metadata/sqlite/transaction.go (4)
database/models/witness.go (2)
KeyWitness(19-29)KeyWitness(31-33)database/models/script.go (2)
Script(23-29)Script(31-33)database/models/datum.go (2)
PlutusData(29-34)PlutusData(36-38)database/models/redeemer.go (2)
Redeemer(18-27)Redeemer(29-31)
database/models/witness.go (1)
database/models/transaction.go (2)
Transaction(18-34)Transaction(36-38)
database/models/transaction.go (4)
database/models/witness.go (2)
KeyWitness(19-29)KeyWitness(31-33)database/models/script.go (2)
Script(23-29)Script(31-33)database/models/redeemer.go (2)
Redeemer(18-27)Redeemer(29-31)database/models/datum.go (2)
PlutusData(29-34)PlutusData(36-38)
🔇 Additional comments (1)
database/models/transaction.go (1)
19-30: Witness/script relationships onTransactionare consistent with existing FK patternThe added
KeyWitnesses,Scripts,Redeemers, andPlutusDatafields useforeignKey:TransactionID;references:ID, matching the established pattern used forOutputsand other associations, so they should preload and join cleanly with GORM. Based on learnings.
Signed-off-by: Jenita <jkawan@blinklabs.io>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
database/plugin/metadata/sqlite/transaction.go(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
📚 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
🧬 Code graph analysis (1)
database/plugin/metadata/sqlite/transaction.go (4)
database/models/witness.go (2)
KeyWitness(19-29)KeyWitness(31-33)database/models/script.go (2)
Script(23-29)Script(31-33)database/models/redeemer.go (2)
Redeemer(18-27)Redeemer(29-31)database/models/datum.go (2)
PlutusData(29-34)PlutusData(36-38)
🪛 GitHub Actions: golangci-lint
database/plugin/metadata/sqlite/transaction.go
[error] 416-416: golangci-lint: G115 integer overflow conversion int64 -> uint64 (gosec)
⏰ 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: Analyze (go)
- GitHub Check: nilaway
🔇 Additional comments (5)
database/plugin/metadata/sqlite/transaction.go (5)
92-102: Good fix for ID population on upsert conflict path.This correctly addresses the concern from the previous review about
tmpTx.IDremaining zero when theON CONFLICTpath is taken. By explicitly fetching the transaction ID when needed, you ensure all subsequent witness records will have a validTransactionID.
293-306: Idempotency fix looks good.This correctly addresses the concern from the previous review about duplicate witness records on retry. The delete-before-insert pattern ensures that re-running
SetTransactionfor the same transaction hash will replace rather than accumulate witness data.The check
if tmpTx.ID != 0is defensive programming—after lines 92-102, the ID should always be non-zero or the function would have returned with an error. This safeguard is reasonable.
311-336: LGTM: Vkey and Bootstrap witness insertion.The mapping is correct: Type 0 for Vkey witnesses (with Vkey and Signature fields) and Type 1 for Bootstrap witnesses (with PublicKey, Signature, ChainCode, and Attributes). Error handling is properly implemented.
339-384: LGTM: Script persistence across all types.The implementation correctly handles Native, PlutusV1, PlutusV2, and PlutusV3 scripts with appropriate Type values from
lcommon.ScriptRefType*constants. The distinction between CBOR encoding for native scripts and raw bytes for Plutus scripts is properly maintained.
387-395: LGTM: PlutusData persistence.The datum persistence correctly stores CBOR-encoded PlutusData with proper error handling.
Signed-off-by: Jenita <jkawan@blinklabs.io>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
database/plugin/metadata/sqlite/transaction.go (2)
397-412: Redeemer persistence addresses past concern, but consider explicit validation.The
max(0, ...)approach on lines 405-406 prevents the integer overflow issue flagged in the previous review by clamping negativeExUnitsvalues to zero. The//nolint:goseccomments suppress the linter warnings.However, silently clamping negative values could mask data corruption or upstream bugs. Consider whether explicit validation with an error return would be more appropriate:
if value.ExUnits.Memory < 0 || value.ExUnits.Steps < 0 { return fmt.Errorf("invalid ExUnits for redeemer tag=%d index=%d: Memory=%d, Steps=%d", key.Tag, key.Index, value.ExUnits.Memory, value.ExUnits.Steps) } redeemer := models.Redeemer{ TransactionID: tmpTx.ID, Tag: uint8(key.Tag), Index: key.Index, Data: value.Data.Cbor(), ExUnitsMemory: uint64(value.ExUnits.Memory), ExUnitsCPU: uint64(value.ExUnits.Steps), }The current implementation is acceptable if negative values are expected to be impossible or if silent clamping is the intended behavior for robustness.
307-413: Consider batch inserts for witness data (optional optimization).The current implementation inserts witness records one at a time in loops. While this is functionally correct and maintains good error handling, consider using GORM's
CreateInBatchesfor better performance when dealing with transactions that have many witnesses:// Example for Vkey witnesses vkeyWitnesses := make([]models.KeyWitness, 0, len(ws.Vkey())) for _, vkey := range ws.Vkey() { vkeyWitnesses = append(vkeyWitnesses, models.KeyWitness{ TransactionID: tmpTx.ID, Type: 0, Vkey: vkey.Vkey, Signature: vkey.Signature, }) } if len(vkeyWitnesses) > 0 { if result := txn.CreateInBatches(vkeyWitnesses, 100); result.Error != nil { return fmt.Errorf("create vkey witnesses: %w", result.Error) } }This optimization is not essential but could improve performance for transactions with large witness sets.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
database/plugin/metadata/sqlite/transaction.go(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
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.
📚 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:13:35.972Z
Learnt from: wolf31o2
Repo: blinklabs-io/dingo PR: 971
File: utxorpc/sync_test.go:339-339
Timestamp: 2025-10-22T20:13:35.972Z
Learning: In utxorpc code paths, always operate on github.com/blinklabs-io/gouroboros/ledger/common.Block. Use database/models.Block only as the storage type and call models.Block.Decode() to obtain a ledger/common.Block before further processing.
Applied to files:
database/plugin/metadata/sqlite/transaction.go
📚 Learning: 2025-10-22T20:13:35.972Z
Learnt from: wolf31o2
Repo: blinklabs-io/dingo PR: 971
File: utxorpc/sync_test.go:339-339
Timestamp: 2025-10-22T20:13:35.972Z
Learning: Unit tests under utxorpc should assert that models.Block.Decode() returns a github.com/blinklabs-io/gouroboros/ledger/common.Block, not rely on models.Block directly in protocol/ledger-facing logic.
Applied to files:
database/plugin/metadata/sqlite/transaction.go
📚 Learning: 2025-10-22T21:28:48.396Z
Learnt from: wolf31o2
Repo: blinklabs-io/dingo PR: 971
File: utxorpc/sync_test.go:339-339
Timestamp: 2025-10-22T21:28:48.396Z
Learning: In utxorpc/sync_test.go, tests must not pass database/models.Block into chain.Chain.AddBlock. They should call models.Block.Decode() and pass the resulting github.com/blinklabs-io/gouroboros/ledger/common.Block to AddBlock.
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/plugin/metadata/sqlite/transaction.go
🧬 Code graph analysis (1)
database/plugin/metadata/sqlite/transaction.go (4)
database/models/witness.go (2)
KeyWitness(19-29)KeyWitness(31-33)database/models/script.go (2)
Script(23-29)Script(31-33)database/models/redeemer.go (2)
Redeemer(18-27)Redeemer(29-31)database/models/datum.go (2)
PlutusData(29-34)PlutusData(36-38)
⏰ 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: Analyze (go)
- GitHub Check: nilaway
🔇 Additional comments (5)
database/plugin/metadata/sqlite/transaction.go (5)
92-102: Excellent fix for ID population after upsert!This correctly addresses the concern from the previous review. When SQLite's
ON CONFLICT DO UPDATEpath is taken, GORM may not populatetmpTx.ID. Fetching by hash ensures the ID is always available for subsequent witness record inserts.
291-306: Idempotency correctly implemented!This addresses the concern from the previous review about duplicate witness rows on retry. Deleting existing child records before re-inserting ensures that
SetTransactionis idempotent, even after partial failures or re-execution for the same transaction hash.
310-336: LGTM: Vkey and Bootstrap witness persistence.The mapping from ledger witness types to
KeyWitnessmodel records is correct. The Type discriminator (0 for Vkey, 1 for Bootstrap) and field assignments align with the model definition. Error handling is appropriate.
338-384: LGTM: Script persistence for all script types.The persistence logic correctly handles Native, PlutusV1, PlutusV2, and PlutusV3 scripts with appropriate Type discriminators from
lcommonconstants. The distinction between.Cbor()for native scripts and direct byte usage for Plutus scripts aligns with the witness set interface. Error handling is comprehensive.
386-395: LGTM: PlutusData/Datums persistence.The mapping from witness set datums to
PlutusDatamodel records is correct. Field assignments and error handling are appropriate.
database/models/redeemer.go
Outdated
| TransactionID uint `gorm:"index"` | ||
| Tag uint8 `gorm:"index"` // Redeemer tag | ||
| Index uint32 `gorm:"index"` | ||
| Data []byte `gorm:"type:bytea"` // Plutus data |
There was a problem hiding this comment.
This is a Postgres data type, which is not supported by SQLite. We should let GORM determine the correct type to use under the hood where it can.
database/models/script.go
Outdated
| ID uint `gorm:"primaryKey"` | ||
| TransactionID uint `gorm:"index"` | ||
| Type uint8 `gorm:"index"` // Script type | ||
| ScriptData []byte `gorm:"type:bytea"` |
There was a problem hiding this comment.
This is a Postgres data type, which is not supported by SQLite. We should let GORM determine the correct type to use under the hood where it can.
| if result := txn.Where("transaction_id = ?", tmpTx.ID).Delete(&models.PlutusData{}); result.Error != nil { | ||
| return fmt.Errorf("delete existing plutus data: %w", result.Error) | ||
| } | ||
| } |
There was a problem hiding this comment.
Instead of deleting existing records, we should probably use ON CONFLICT clauses to update the existing record(s)
| } | ||
| } | ||
| if tx.Witnesses() != nil { | ||
| ws := tx.Witnesses() |
There was a problem hiding this comment.
You can save a repeated function call by doing:
if ws := tx.Witnesses(); ws != nil {
| for _, vkey := range ws.Vkey() { | ||
| keyWitness := models.KeyWitness{ | ||
| TransactionID: tmpTx.ID, | ||
| Type: 0, // VkeyWitness |
There was a problem hiding this comment.
We should define our own constants for the "key witness" type, since there aren't any provided by gOuroboros.
| for _, bootstrap := range ws.Bootstrap() { | ||
| keyWitness := models.KeyWitness{ | ||
| TransactionID: tmpTx.ID, | ||
| Type: 1, // BootstrapWitness |
There was a problem hiding this comment.
We should define our own constants for the "key witness" type, since there aren't any provided by gOuroboros.
| Type uint8 `gorm:"index"` // Script type | ||
| ScriptData []byte `gorm:"type:bytea"` | ||
| Transaction *Transaction | ||
| } |
There was a problem hiding this comment.
Storing the entire script for each transaction that it appears on is going to kill us for PlutusV1 scripts where the same script content was included in the TX witness set of literally thousands of TXs. We don't want to store all of those copies in the database.
We do want a way to know which script hash was provided in the witness set so we can look up the content, but we don't need a direct link in the DB between a particular TX and the script content
database/models/witness.go
Outdated
| package models | ||
|
|
||
| // KeyWitness represents a key witness entry (Vkey or Bootstrap) | ||
| // Type: 0 = VkeyWitness, 1 = BootstrapWitness |
There was a problem hiding this comment.
Create constants for these
|
I've added the first 200 chunks of immutable DB data from the Haskell cardano-node to the code. You can access it in database/immutable/testdata and there's even a make test-load which will import it using "dingo load" mechanics "go run ./cmd/dingo load database/immutable/testdata" which is useful for testing. This is the first 40k blocks of the preview network from genesis and it takes under a minute to load. |
Signed-off-by: Jenita <jkawan@blinklabs.io>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
database/models/models.go (1)
29-43: New models correctly registered in migrations; consider restoring strict alpha order
KeyWitness,PlutusData,Redeemer,Script, andScriptContentare all properly added toMigrateModels, so schema migrations will see the new tables. The only nit is that the slice is no longer strictly alphabetical (e.g.,Redeemercomes afterRegistration*). If you still want to keep this list sorted as per earlier feedback, a small reorder would address it, but it’s not functionally required.
🧹 Nitpick comments (1)
database/models/script_content.go (1)
17-30: ScriptContent layout matches deduplication goalHash+unique plus a separate Script table that stores only hashes is a good shape for avoiding duplicate script payloads, and
CreatedSlotprovides a clear “first seen” marker. If you later need to query scripts by slot range, consider addinggorm:"index"onCreatedSlot, but it’s fine to defer until there’s a concrete use case.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
database/models/models.go(1 hunks)database/models/redeemer.go(1 hunks)database/models/script.go(1 hunks)database/models/script_content.go(1 hunks)database/models/witness.go(1 hunks)database/plugin/metadata/sqlite/script.go(1 hunks)database/plugin/metadata/sqlite/transaction.go(2 hunks)database/plugin/metadata/store.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- database/models/redeemer.go
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
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.
📚 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.godatabase/models/script.go
📚 Learning: 2025-10-22T20:13:35.972Z
Learnt from: wolf31o2
Repo: blinklabs-io/dingo PR: 971
File: utxorpc/sync_test.go:339-339
Timestamp: 2025-10-22T20:13:35.972Z
Learning: In utxorpc code paths, always operate on github.com/blinklabs-io/gouroboros/ledger/common.Block. Use database/models.Block only as the storage type and call models.Block.Decode() to obtain a ledger/common.Block before further processing.
Applied to files:
database/plugin/metadata/sqlite/transaction.go
📚 Learning: 2025-10-22T20:13:35.972Z
Learnt from: wolf31o2
Repo: blinklabs-io/dingo PR: 971
File: utxorpc/sync_test.go:339-339
Timestamp: 2025-10-22T20:13:35.972Z
Learning: Unit tests under utxorpc should assert that models.Block.Decode() returns a github.com/blinklabs-io/gouroboros/ledger/common.Block, not rely on models.Block directly in protocol/ledger-facing logic.
Applied to files:
database/plugin/metadata/sqlite/transaction.go
📚 Learning: 2025-10-22T21:28:48.396Z
Learnt from: wolf31o2
Repo: blinklabs-io/dingo PR: 971
File: utxorpc/sync_test.go:339-339
Timestamp: 2025-10-22T21:28:48.396Z
Learning: In utxorpc/sync_test.go, tests must not pass database/models.Block into chain.Chain.AddBlock. They should call models.Block.Decode() and pass the resulting github.com/blinklabs-io/gouroboros/ledger/common.Block to AddBlock.
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/plugin/metadata/sqlite/transaction.go
🧬 Code graph analysis (5)
database/plugin/metadata/sqlite/script.go (2)
database/models/script_content.go (2)
ScriptContent(20-26)ScriptContent(28-30)database/models/script.go (2)
Script(27-33)Script(35-37)
database/models/models.go (5)
database/models/witness.go (2)
KeyWitness(26-36)KeyWitness(38-40)database/models/datum.go (2)
PlutusData(29-34)PlutusData(36-38)database/models/redeemer.go (2)
Redeemer(18-27)Redeemer(29-31)database/models/script.go (2)
Script(27-33)Script(35-37)database/models/script_content.go (2)
ScriptContent(20-26)ScriptContent(28-30)
database/models/script.go (1)
database/models/transaction.go (2)
Transaction(18-34)Transaction(36-38)
database/plugin/metadata/store.go (2)
database/models/script_content.go (2)
ScriptContent(20-26)ScriptContent(28-30)database/models/script.go (2)
Script(27-33)Script(35-37)
database/models/witness.go (1)
database/models/transaction.go (2)
Transaction(18-34)Transaction(36-38)
🪛 GitHub Actions: golangci-lint
database/plugin/metadata/sqlite/transaction.go
[error] 379-379: golangci-lint: unnecessary leading newline (whitespace) in transaction.go:379
🪛 GitHub Check: lint
database/plugin/metadata/sqlite/transaction.go
[failure] 379-379:
unnecessary leading newline (whitespace)
⏰ 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). (4)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: nilaway
- GitHub Check: go-test (1.24.x, macos-latest)
- GitHub Check: Analyze (go)
🔇 Additional comments (5)
database/models/witness.go (1)
17-40: KeyWitness model and type constants look solidThe enum-style
Typeplus separate fields for vkey vs bootstrap data cleanly model the two witness kinds, and the indexing onTransactionID/Typematches how SetTransaction persists them. No changes needed here.database/plugin/metadata/store.go (1)
82-93: Script-related MetadataStore extensions are consistent and minimalThe
GetScriptContentandGetScriptsByTransactionmethods fit the existing API style (hash/ID + optional txn) and line up with the sqlite implementation signatures. Looks good.database/plugin/metadata/sqlite/script.go (1)
25-58: Script lookup helpers for SQLite are straightforward and consistentBoth
GetScriptContentandGetScriptsByTransactionfollow the existing nil-txn fallback and error-handling patterns, and they query on the expected keys (hashandtransaction_id). Returning(nil, nil)on not-found for content is also consistent with other getters.database/models/script.go (1)
17-37: Script-as-hash reference aligns well with ScriptContent deduplicationModeling
Scriptas just(transaction_id, type, script_hash)with comments tyingTypeto theScriptRefType*values keeps per-transaction rows lean and delegates payload storage toScriptContentas intended. This looks good.database/plugin/metadata/sqlite/transaction.go (1)
92-104: Upsert ID fix correctly guarantees a valid TransactionID for child recordsThe explicit
GetTransactionByHashfallback whentmpTx.ID == 0closes the gap where SQLite’sON CONFLICTpath wouldn’t populate the ID, so all subsequent witness rows now get a non-zeroTransactionID. This addresses the earlier orphaning risk; the logic here looks good.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Signed-off-by: Jenita <jkawan@blinklabs.io>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
database/plugin/metadata/sqlite/transaction.go (2)
295-308: Consider usingON CONFLICTclauses instead of deleting witness records.The delete-before-insert pattern achieves idempotency but requires extra DELETE operations. Using
ON CONFLICT DO UPDATE(orDO NOTHINGwhere appropriate) on the witness tables would be more database-idiomatic and potentially more efficient, as it would only touch rows that actually changed.This would require adding appropriate unique constraints to the witness tables (e.g.,
(transaction_id, type, vkey)for KeyWitness,(transaction_id, tag, index)for Redeemer, etc.).Based on learnings from previous reviews, consider whether this aligns with the codebase's approach to GORM associations and foreign keys.
309-310: Combine witness retrieval with nil check.You can make this more concise by combining the assignment and condition check:
- ws := tx.Witnesses() - if ws != nil { + if ws := tx.Witnesses(); ws != nil {This is a common Go idiom and avoids declaring
wsin the broader scope.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
database/plugin/metadata/sqlite/transaction.go(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
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.
📚 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:13:35.972Z
Learnt from: wolf31o2
Repo: blinklabs-io/dingo PR: 971
File: utxorpc/sync_test.go:339-339
Timestamp: 2025-10-22T20:13:35.972Z
Learning: In utxorpc code paths, always operate on github.com/blinklabs-io/gouroboros/ledger/common.Block. Use database/models.Block only as the storage type and call models.Block.Decode() to obtain a ledger/common.Block before further processing.
Applied to files:
database/plugin/metadata/sqlite/transaction.go
📚 Learning: 2025-10-22T20:13:35.972Z
Learnt from: wolf31o2
Repo: blinklabs-io/dingo PR: 971
File: utxorpc/sync_test.go:339-339
Timestamp: 2025-10-22T20:13:35.972Z
Learning: Unit tests under utxorpc should assert that models.Block.Decode() returns a github.com/blinklabs-io/gouroboros/ledger/common.Block, not rely on models.Block directly in protocol/ledger-facing logic.
Applied to files:
database/plugin/metadata/sqlite/transaction.go
📚 Learning: 2025-10-22T21:28:48.396Z
Learnt from: wolf31o2
Repo: blinklabs-io/dingo PR: 971
File: utxorpc/sync_test.go:339-339
Timestamp: 2025-10-22T21:28:48.396Z
Learning: In utxorpc/sync_test.go, tests must not pass database/models.Block into chain.Chain.AddBlock. They should call models.Block.Decode() and pass the resulting github.com/blinklabs-io/gouroboros/ledger/common.Block to AddBlock.
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/plugin/metadata/sqlite/transaction.go
⏰ 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). (4)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: nilaway
- GitHub Check: go-test (1.24.x, macos-latest)
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
database/plugin/metadata/sqlite/transaction.go (2)
92-104: ID lookup after upsert is correctly implemented.This block properly addresses the previously flagged critical issue where
tmpTx.IDwould remain zero on theON CONFLICT DO UPDATEpath. The explicit fetch by hash ensures witness records can always be associated with the correct transaction.The comment clearly explains the SQLite/GORM behavior. @agaffney, regarding your question about this code block: when the
ON CONFLICT DO UPDATEpath executes (transaction hash already exists), SQLite doesn't return the existing row's ID in the RETURNING clause. This forces us to explicitly query for the ID before inserting witness records, otherwiseTransactionIDwould be zero and orphan the witness data.
312-470: KeyWitnessType constants confirmed and code is correct.The constants
models.KeyWitnessTypeVkeyandmodels.KeyWitnessTypeBootstrapare properly defined indatabase/models/witness.go(lines 19 and 21). The witness insertion logic is sound: all witness types, script types, plutus data, and redeemers are handled correctly with proper error context, script deduplication viaON CONFLICT DO NOTHINGis implemented, and ExUnits overflow protection withmax(0, ...)is in place.
database/models/datum.go
Outdated
| type PlutusData struct { | ||
| ID uint `gorm:"primaryKey"` | ||
| TransactionID uint `gorm:"index"` | ||
| Data []byte `gorm:"type:bytea"` |
There was a problem hiding this comment.
Remove the GORM field type override. This is a Postgres column type that won't work in SQLite
database/models/script.go
Outdated
| // To avoid storing duplicate script data for the same script used in multiple | ||
| // transactions, we store only the script hash here. The actual script content | ||
| // is stored separately in ScriptContent table, indexed by hash. | ||
| type Script struct { |
There was a problem hiding this comment.
Rename this to WitnessScripts to be a bit more specific about its function (mapping TX ID to script hash when the script appears in the TX witness)
database/models/script_content.go
Outdated
| // ScriptContent represents the content of a script, indexed by its hash | ||
| // This avoids storing duplicate script data when the same script appears | ||
| // in multiple transactions | ||
| type ScriptContent struct { |
database/models/transaction.go
Outdated
| Collateral []Utxo `gorm:"foreignKey:CollateralByTxId;references:Hash"` | ||
| CollateralReturn *Utxo `gorm:"foreignKey:TransactionID;references:ID"` | ||
| KeyWitnesses []KeyWitness `gorm:"foreignKey:TransactionID;references:ID"` | ||
| Scripts []Script `gorm:"foreignKey:TransactionID;references:ID"` |
database/models/witness.go
Outdated
| // KeyWitnessTypeVkey represents a Vkey witness | ||
| KeyWitnessTypeVkey uint8 = iota | ||
| // KeyWitnessTypeBootstrap represents a Bootstrap witness | ||
| KeyWitnessTypeBootstrap |
There was a problem hiding this comment.
Be explicit with the values here. It's generally a bad idea to use iota in cases where the value can persist between older and newer versions of the code in case values are added/removed.
| ) | ||
|
|
||
| // GetScriptContent returns the script content by its hash | ||
| func (d *MetadataStoreSqlite) GetScriptContent( |
| } | ||
|
|
||
| // GetScriptsByTransaction returns all scripts (references) used in a particular transaction | ||
| func (d *MetadataStoreSqlite) GetScriptsByTransaction( |
There was a problem hiding this comment.
GetScriptWitnessesByTransaction
But is this even needed? We probably don't need to query script witnesses for a particular TX without querying for the entire TX, which will include that data.
| } | ||
| // Extract and save witness set data | ||
| // Delete existing witness records to ensure idempotency on retry | ||
| if tmpTx.ID != 0 { |
There was a problem hiding this comment.
Remove this condition. We assign the ID above when it's 0 and throw an error otherwise, so this check will always succeed.
| if result := txn.Where("transaction_id = ?", tmpTx.ID).Delete(&models.KeyWitness{}); result.Error != nil { | ||
| return fmt.Errorf("delete existing key witnesses: %w", result.Error) | ||
| } | ||
| if result := txn.Where("transaction_id = ?", tmpTx.ID).Delete(&models.Script{}); result.Error != nil { |
| }).Create(&scriptContent); result.Error != nil { | ||
| return fmt.Errorf("create plutus v3 script content: %w", result.Error) | ||
| } | ||
| } |
There was a problem hiding this comment.
You could de-duplicate the above 4 blocks by processing them all at once:
scripts := []lcommon.Script{
ws.NativeScripts(),
ws.PlutusV1Scripts(),
...
}
for _, script := range scripts {
...
var scriptType int
switch script.(type) {
case lcommon.NativeScript:
scriptType = lcommon.ScriptTypeNative
...
}
Signed-off-by: Jenita <jkawan@blinklabs.io>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
database/models/models.go (1)
29-55: KeepMigrateModelsalphabetized for easier maintenanceThe new
Redeemerentry breaks the otherwise alphabetical ordering in this slice. To keep things consistent with earlier feedback, consider reordering the relevant lines like:- &PParams{}, - &PParamUpdate{}, - &PlutusData{}, - &Registration{}, - &RegistrationDrep{}, - &Redeemer{}, + &PParams{}, + &PParamUpdate{}, + &PlutusData{}, + &Redeemer{}, + &Registration{}, + &RegistrationDrep{},This keeps the list sorted and makes future merges and scans simpler.
🧹 Nitpick comments (2)
database/models/script.go (1)
17-37: WitnessScripts model matches intended mapping TX → script hashThe struct and
TableNamealign with how the witness scripts are persisted and queried, and the comment clearly tiesTypeto theScriptRefTypeconstants.If you expect frequent lookups or integrity checks by
(transaction_id, script_hash), you might consider adding a composite index or unique index on those columns later, but the current shape is perfectly fine to ship.database/plugin/metadata/sqlite/transaction.go (1)
92-104: Robust ID resolution after transaction upsertThe post-
ON CONFLICTfallback toGetTransactionByHashwhentmpTx.ID == 0closes the gap where witness records could be written with a zeroTransactionID. This makes the subsequent witness inserts safe even on the conflict path.If you ever see this path hit frequently, you might consider a lighter-weight helper that just selects
idbyhash(withoutPreload(clause.Associations)), but functionally this is solid.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
database/models/datum.go(1 hunks)database/models/models.go(2 hunks)database/models/script.go(1 hunks)database/models/script_content.go(1 hunks)database/models/transaction.go(1 hunks)database/models/witness.go(1 hunks)database/plugin/metadata/sqlite/script.go(1 hunks)database/plugin/metadata/sqlite/transaction.go(2 hunks)database/plugin/metadata/store.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- database/plugin/metadata/store.go
- database/models/witness.go
- database/models/script_content.go
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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.
📚 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/models/transaction.godatabase/models/datum.godatabase/plugin/metadata/sqlite/transaction.go
📚 Learning: 2025-10-26T14:12:53.587Z
Learnt from: wolf31o2
Repo: blinklabs-io/dingo PR: 977
File: database/plugin/metadata/sqlite/transaction.go:71-81
Timestamp: 2025-10-26T14:12:53.587Z
Learning: In Babbage-era transactions (gouroboros ledger), the Produced() method behavior depends on transaction validity: for valid transactions (IsValid() == true), Produced() returns all transaction outputs; for invalid transactions (IsValid() == false), Produced() returns only the collateral return UTXO (if non-nil) with index len(t.Outputs()). The collateral return Output is set to t.CollateralReturn() in the returned Utxo struct.
Applied to files:
database/models/transaction.go
📚 Learning: 2025-10-22T20:13:35.972Z
Learnt from: wolf31o2
Repo: blinklabs-io/dingo PR: 971
File: utxorpc/sync_test.go:339-339
Timestamp: 2025-10-22T20:13:35.972Z
Learning: In utxorpc code paths, always operate on github.com/blinklabs-io/gouroboros/ledger/common.Block. Use database/models.Block only as the storage type and call models.Block.Decode() to obtain a ledger/common.Block before further processing.
Applied to files:
database/plugin/metadata/sqlite/transaction.go
📚 Learning: 2025-10-22T20:13:35.972Z
Learnt from: wolf31o2
Repo: blinklabs-io/dingo PR: 971
File: utxorpc/sync_test.go:339-339
Timestamp: 2025-10-22T20:13:35.972Z
Learning: Unit tests under utxorpc should assert that models.Block.Decode() returns a github.com/blinklabs-io/gouroboros/ledger/common.Block, not rely on models.Block directly in protocol/ledger-facing logic.
Applied to files:
database/plugin/metadata/sqlite/transaction.go
📚 Learning: 2025-10-22T21:28:48.396Z
Learnt from: wolf31o2
Repo: blinklabs-io/dingo PR: 971
File: utxorpc/sync_test.go:339-339
Timestamp: 2025-10-22T21:28:48.396Z
Learning: In utxorpc/sync_test.go, tests must not pass database/models.Block into chain.Chain.AddBlock. They should call models.Block.Decode() and pass the resulting github.com/blinklabs-io/gouroboros/ledger/common.Block to AddBlock.
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/plugin/metadata/sqlite/transaction.go
🧬 Code graph analysis (4)
database/models/script.go (1)
database/models/transaction.go (2)
Transaction(18-34)Transaction(36-38)
database/models/datum.go (1)
database/models/transaction.go (2)
Transaction(18-34)Transaction(36-38)
database/plugin/metadata/sqlite/script.go (2)
database/plugin/metadata/sqlite/database.go (1)
MetadataStoreSqlite(36-42)database/models/script_content.go (2)
Script(20-26)Script(28-30)
database/models/models.go (5)
database/models/witness.go (2)
KeyWitness(26-36)KeyWitness(38-40)database/models/datum.go (2)
PlutusData(29-34)PlutusData(36-38)database/models/redeemer.go (2)
Redeemer(18-27)Redeemer(29-31)database/models/script_content.go (2)
Script(20-26)Script(28-30)database/models/script.go (2)
WitnessScripts(27-33)WitnessScripts(35-37)
⏰ 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). (6)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: go-test (1.24.x, ubuntu-latest)
- GitHub Check: go-test (1.24.x, macos-latest)
- GitHub Check: lint
- GitHub Check: nilaway
- GitHub Check: Analyze (go)
🔇 Additional comments (4)
database/plugin/metadata/sqlite/script.go (1)
25-41:GetScriptlookup and not-found semantics look goodThe method cleanly handles optional transactions, correctly queries by the byte hash, and uses
(nil, nil)for “not found,” which matches the pattern used by other getters in this package.database/models/datum.go (1)
28-38: PlutusData schema is consistent with existing transaction child tablesThe PlutusData model correctly mirrors the existing FK pattern (TransactionID → Transaction.ID) and avoids driver-specific type overrides, which keeps it compatible with SQLite.
database/models/transaction.go (1)
19-33: Witness-related associations on Transaction are wired correctlyThe new
KeyWitnesses,WitnessScripts,Redeemers, andPlutusDatafields follow the established pattern of usingTransactionID → IDfor child tables and align with the waySetTransactionpopulates those records.database/plugin/metadata/sqlite/transaction.go (1)
293-435: Witness, script, datum, and redeemer persistence is coherent and idempotentThe pattern of deleting existing
KeyWitness,WitnessScripts,Redeemer, andPlutusDatarows bytransaction_idand then repopulating fromtx.Witnesses()gives you a clean, idempotent mapping for retries and re-runs.The
processScriptshelper nicely unifies the four script-type loops, correctly setsWitnessScripts.TypeusingScriptRefType*constants, and de-duplicates script content withON CONFLICT (hash) DO NOTHING. ExUnits clamping withmax(0, …)before casting touint64also addresses the previous overflow concern.Overall, this block looks ready to ship.
jkawan
force-pushed
the
feat-track-witnessset
branch
from
f776f13 to
9dff05d
Compare
Signed-off-by: Jenita <jkawan@blinklabs.io>
Signed-off-by: Jenita <jkawan@blinklabs.io>
jkawan
force-pushed
the
feat-track-witnessset
branch
from
9dff05d to
d351aa8
Compare
Signed-off-by: Jenita <jkawan@blinklabs.io>
Closes Track transaction witness set #874
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.