Persist intents and Update vtxo message #644
Conversation
altafan
changed the title
...r/internal/infrastructure/db/postgres/migration/20250627185230_update_intent_and_vtxo.up.sql
Show resolved
Hide resolved
|
refresh > renew |
I'm not sure I agree. If we do this it is quite confusing from a client perspective: your only link to a spend skips a tx and links to different Id. Using the vtxo chain as is isn't a nice fit to "locate the spenders". I would prefer to have the actual next txid in that field. Nit: we should allow get vtxo chain endpoint to work in the opposite direction through a parameter so that we can see the descendant chain from a specific vtxo. (maybe for 7.1 or 8 though) |
not deep into the impact, but surely switching meaning of field not great. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces extensive changes across the codebase and data model, focusing on enhanced tracking and metadata for virtual transaction outputs (VTXOs) and transaction requests. Key modifications include new fields for settlement and transaction identification, renaming and restructuring of boolean status flags, and significant updates to database schemas, APIs, and service interfaces. The changes also include improvements to validation, serialization, and test coverage. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant Store
participant DB
Client->>API: Submit intent (TxRequest with Proof, Message, Inputs)
API->>Store: Persist intent with new fields
Store->>DB: Upsert tx_request (id, round_id, proof, message)
DB-->>Store: Ack
Store-->>API: Ack
API-->>Client: Intent persisted (regardless of batch inclusion)
sequenceDiagram
participant Service
participant VtxoStore
participant DB
Service->>VtxoStore: SettleVtxos(spentVtxos, settledBy)
VtxoStore->>DB: Update vtxo (set spent, spent_by, settled_by)
DB-->>VtxoStore: Ack
VtxoStore-->>Service: Ack
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Added the following changes after review:
NOTE: with 1) and 2) |
altafan
changed the title
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🔭 Outside diff range comments (4)
server/internal/infrastructure/db/postgres/sqlc/query.sql (1)
37-54: Add missing indexes ontxidfor all transaction tablesThe
GetTxsByTxidquery unions three tables without anytxidindexes defined in your Postgres migrations. This will lead to full-table scans as your data grows.Please add indexes on the
txidcolumn in each table, for example:-- e.g. server/internal/infrastructure/db/postgres/migration/202507XX_add_txid_indexes.up.sql CREATE INDEX IF NOT EXISTS idx_tx_txid ON tx(txid); CREATE INDEX IF NOT EXISTS idx_virtual_tx_txid ON virtual_tx(txid); CREATE INDEX IF NOT EXISTS idx_checkpoint_tx_txid ON checkpoint_tx(txid);Then rebuild your schema or run the new migration to ensure the UNION query can leverage these indexes for optimal performance.
server/internal/infrastructure/db/postgres/vtxo_repo.go (1)
262-283: Apply consistent empty string handling for arkTxid.Similar to the previous comment, unconditionally setting
Valid: trueforarkTxidmay insert empty strings instead of NULL values.- ArkTxid: sql.NullString{String: arkTxid, Valid: true}, + ArkTxid: sql.NullString{String: arkTxid, Valid: arkTxid != ""},pkg/client-sdk/client/rest/service/models/v1_tx_notification.go (2)
184-186: Critical: Early return breaks validation loopThe validation loop returns nil when encountering a zero value, which prevents validation of remaining items in the slice. This should continue to the next iteration instead.
if swag.IsZero(m.SpendableVtxos[i]) { // not required - return nil + continue }
209-211: Critical: Early return breaks validation loopSame issue as in
contextValidateSpendableVtxos- the validation loop returns nil when encountering a zero value instead of continuing to the next iteration.if swag.IsZero(m.SpentVtxos[i]) { // not required - return nil + continue }
♻️ Duplicate comments (5)
server/internal/infrastructure/db/sqlite/offchain_tx_repo.go (1)
106-108: Implement the GetOffchainTxs method or return explicit error.Same issue as the PostgreSQL implementation - the stub method returning
nil, nilcould mask failures. This should either be properly implemented or return an explicit "not implemented" error.server/internal/infrastructure/db/postgres/round_repo.go (1)
143-146: Duplicate issue: Improper null handling for Proof and Message fields.This has the same issue as the SQLite implementation. Consider the same fix:
-Proof: sql.NullString{String: request.Proof, Valid: true}, -Message: sql.NullString{String: request.Message, Valid: true}, +Proof: sql.NullString{String: request.Proof, Valid: request.Proof != ""}, +Message: sql.NullString{String: request.Message, Valid: request.Message != ""},server/internal/infrastructure/db/badger/vtxo_repo.go (1)
62-71: Well-implemented SettleVtxos method using Outpoint as map key.The implementation correctly sets both
SpentByandSettledByfields for settled vtxos. Good to see that the map key is already usingdomain.Outpointas suggested in the previous review.pkg/client-sdk/indexer/rest/service/models/v1_get_subscription_response.go (2)
185-187: Critical: Early return breaks validation loopThe validation loop returns nil when encountering a zero value, which prevents validation of remaining items in the slice.
if swag.IsZero(m.NewVtxos[i]) { // not required - return nil + continue }
210-212: Critical: Early return breaks validation loopSame validation loop issue - returns nil instead of continuing to next iteration.
if swag.IsZero(m.SpentVtxos[i]) { // not required - return nil + continue }
🧹 Nitpick comments (9)
server/internal/infrastructure/db/sqlite/sqlc.yaml (1)
10-45: LGTM! Proper nullable string handling for new database fields.The explicit Go type overrides correctly map nullable database columns to
database/sql.NullString, ensuring proper null value handling in the generated SQLC code. The configuration consistently applies overrides across all relevant views and tables.Fix formatting: Add missing newline at end of file.
# Add a newline at the end of the file +pkg/client-sdk/client/grpc/client.go (1)
294-305: Consider adding error handling for outpoint parsing.The checkpoint transaction processing correctly maps string keys to
TxDatavalues, but thewire.NewOutPointFromString(k)call ignores potential parsing errors with anolintcomment. While this may be acceptable if the data is guaranteed to be well-formed, consider adding at least basic error logging for robustness.- // nolint - out, _ := wire.NewOutPointFromString(k) + out, err := wire.NewOutPointFromString(k) + if err != nil { + // Log error but continue processing other checkpoint txs + continue + }server/internal/infrastructure/db/badger/offchain_tx_repo.go (1)
64-89: Consider completing checkpoint transaction support.The TODO comment indicates that the method doesn't fully support returning checkpoint transactions. Currently, it only returns transaction strings without associated metadata. Consider whether this limitation affects the intended functionality.
Would you like me to suggest an implementation that returns full transaction data including checkpoint transactions?
pkg/client-sdk/store/kv/vtxo_repository.go (1)
95-127: Consider using a distinct event type for settled VTXOs.The settling logic is correct, but emitting
VtxosSpentevents for settled VTXOs might be confusing to consumers. Consider adding aVtxosSettledevent type to distinguish between spending and settling operations.server/internal/core/application/service.go (1)
237-244: Add error handling for PSBT parsing.The PSBT parsing on line 240 ignores errors with
// nolint. Consider logging parsing failures to aid debugging.- // nolint - ptx, _ := psbt.NewFromRawBytes(strings.NewReader(tx), true) + ptx, err := psbt.NewFromRawBytes(strings.NewReader(tx), true) + if err != nil { + log.WithError(err).Warnf("failed to parse checkpoint tx %s", txid) + continue + }server/internal/infrastructure/db/service.go (1)
433-457: Well-implemented forfeit transaction matching logic.The function correctly associates each spent vtxo with its corresponding forfeit transaction ID. The logic handles both forfeitable and non-forfeitable vtxos appropriately.
Consider pre-indexing forfeit transactions by their inputs for better performance if rounds typically contain many forfeit transactions, though the current implementation should work fine for typical usage.
pkg/client-sdk/store/sql/vtxo_store.go (2)
56-58: Consider handling empty strings for nullable fields.The nullable fields are always marked as valid even when the string values are empty. This might store empty strings in the database instead of NULL values.
-SpentBy: sql.NullString{String: vtxo.SpentBy, Valid: true}, -SettledBy: sql.NullString{String: vtxo.SettledBy, Valid: true}, -ArkTxid: sql.NullString{String: vtxo.ArkTxid, Valid: true}, +SpentBy: sql.NullString{String: vtxo.SpentBy, Valid: vtxo.SpentBy != ""}, +SettledBy: sql.NullString{String: vtxo.SettledBy, Valid: vtxo.SettledBy != ""}, +ArkTxid: sql.NullString{String: vtxo.ArkTxid, Valid: vtxo.ArkTxid != ""},
257-267: Potential performance issue in event sending.The 100ms sleep when the event channel is full could cause performance degradation under high load. Consider alternative approaches to handle channel backpressure.
Consider these alternatives:
- Increase the channel buffer size when creating it
- Drop events and log a warning instead of sleeping
- Use a ring buffer or other bounded queue implementation
- Return an error to let the caller decide how to handle backpressure
server/internal/infrastructure/db/sqlite/sqlc/queries/query.sql.go (1)
167-167: Consider removing unused fields from the subquery.The new fields
settled_by,preconfirmed, andark_txidare selected in the subquery but not used in the aggregation. Since onlyamountis summed, these fields are unnecessary and could impact query performance.- SELECT DISTINCT v2.txid, v2.vout, v2.pubkey, v2.amount, v2.round_tx, v2.spent_by, v2.spent, v2.redeemed, v2.swept, v2.expire_at, v2.created_at, v2.request_id, v2.settled_by, v2.preconfirmed, v2.ark_txid + SELECT DISTINCT v2.txid, v2.vout, v2.amount
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
api-spec/protobuf/gen/ark/v1/admin.pb.gw.gois excluded by!**/*.pb.gw.go,!**/gen/**api-spec/protobuf/gen/ark/v1/indexer.pb.gois excluded by!**/*.pb.go,!**/gen/**api-spec/protobuf/gen/ark/v1/indexer.pb.gw.gois excluded by!**/*.pb.gw.go,!**/gen/**api-spec/protobuf/gen/ark/v1/service.pb.gw.gois excluded by!**/*.pb.gw.go,!**/gen/**api-spec/protobuf/gen/ark/v1/types.pb.gois excluded by!**/*.pb.go,!**/gen/**api-spec/protobuf/gen/ark/v1/wallet.pb.gw.gois excluded by!**/*.pb.gw.go,!**/gen/**api-spec/protobuf/gen/arkwallet/v1/bitcoin_wallet.pb.gw.gois excluded by!**/*.pb.gw.go,!**/gen/**
📒 Files selected for processing (72)
api-spec/openapi/swagger/ark/v1/indexer.swagger.json(4 hunks)api-spec/openapi/swagger/ark/v1/service.swagger.json(3 hunks)api-spec/protobuf/ark/v1/indexer.proto(2 hunks)api-spec/protobuf/ark/v1/types.proto(1 hunks)pkg/client-sdk/base_client.go(2 hunks)pkg/client-sdk/client.go(19 hunks)pkg/client-sdk/client/client.go(2 hunks)pkg/client-sdk/client/grpc/client.go(2 hunks)pkg/client-sdk/client/grpc/types.go(2 hunks)pkg/client-sdk/client/rest/client.go(4 hunks)pkg/client-sdk/client/rest/service/models/v1_tx_data.go(1 hunks)pkg/client-sdk/client/rest/service/models/v1_tx_notification.go(5 hunks)pkg/client-sdk/client/rest/service/models/v1_vtxo.go(2 hunks)pkg/client-sdk/indexer/grpc/client.go(3 hunks)pkg/client-sdk/indexer/rest/client.go(3 hunks)pkg/client-sdk/indexer/rest/service/models/v1_get_subscription_response.go(6 hunks)pkg/client-sdk/indexer/rest/service/models/v1_indexer_tx_data.go(1 hunks)pkg/client-sdk/indexer/rest/service/models/v1_indexer_vtxo.go(2 hunks)pkg/client-sdk/indexer/service.go(1 hunks)pkg/client-sdk/store/kv/transaction_repository.go(2 hunks)pkg/client-sdk/store/kv/vtxo_repository.go(4 hunks)pkg/client-sdk/store/service_test.go(3 hunks)pkg/client-sdk/store/sql/migration/20250627235432_add_settled_by.down.sql(1 hunks)pkg/client-sdk/store/sql/migration/20250627235432_add_settled_by.up.sql(1 hunks)pkg/client-sdk/store/sql/sqlc/queries/models.go(2 hunks)pkg/client-sdk/store/sql/sqlc/queries/query.sql.go(17 hunks)pkg/client-sdk/store/sql/sqlc/query.sql(3 hunks)pkg/client-sdk/store/sql/tx_store.go(5 hunks)pkg/client-sdk/store/sql/vtxo_store.go(7 hunks)pkg/client-sdk/types.go(1 hunks)pkg/client-sdk/types/interfaces.go(2 hunks)pkg/client-sdk/types/types.go(2 hunks)pkg/client-sdk/utils.go(1 hunks)server/internal/core/application/fraud.go(2 hunks)server/internal/core/application/indexer.go(7 hunks)server/internal/core/application/service.go(6 hunks)server/internal/core/application/types.go(1 hunks)server/internal/core/application/utils.go(1 hunks)server/internal/core/domain/offchain_tx_repo.go(1 hunks)server/internal/core/domain/round_test.go(2 hunks)server/internal/core/domain/tx_request.go(2 hunks)server/internal/core/domain/tx_request_test.go(2 hunks)server/internal/core/domain/vtxo.go(2 hunks)server/internal/core/domain/vtxo_repo.go(1 hunks)server/internal/infrastructure/db/badger/offchain_tx_repo.go(3 hunks)server/internal/infrastructure/db/badger/vtxo_repo.go(3 hunks)server/internal/infrastructure/db/postgres/migration/20250627185230_update_intent_and_vtxo.down.sql(1 hunks)server/internal/infrastructure/db/postgres/migration/20250627185230_update_intent_and_vtxo.up.sql(1 hunks)server/internal/infrastructure/db/postgres/offchain_tx_repo.go(2 hunks)server/internal/infrastructure/db/postgres/round_repo.go(5 hunks)server/internal/infrastructure/db/postgres/sqlc.yaml(1 hunks)server/internal/infrastructure/db/postgres/sqlc/queries/models.go(6 hunks)server/internal/infrastructure/db/postgres/sqlc/queries/query.sql.go(32 hunks)server/internal/infrastructure/db/postgres/sqlc/query.sql(6 hunks)server/internal/infrastructure/db/postgres/vtxo_repo.go(12 hunks)server/internal/infrastructure/db/service.go(5 hunks)server/internal/infrastructure/db/service_test.go(12 hunks)server/internal/infrastructure/db/sqlite/migration/20250627165356_update_intent_and_vtxo.down.sql(1 hunks)server/internal/infrastructure/db/sqlite/migration/20250627165356_update_intent_and_vtxo.up.sql(1 hunks)server/internal/infrastructure/db/sqlite/offchain_tx_repo.go(2 hunks)server/internal/infrastructure/db/sqlite/round_repo.go(5 hunks)server/internal/infrastructure/db/sqlite/sqlc.yaml(1 hunks)server/internal/infrastructure/db/sqlite/sqlc/queries/models.go(5 hunks)server/internal/infrastructure/db/sqlite/sqlc/queries/query.sql.go(32 hunks)server/internal/infrastructure/db/sqlite/sqlc/query.sql(3 hunks)server/internal/infrastructure/db/sqlite/vtxo_repo.go(3 hunks)server/internal/infrastructure/live-store/inmemory/intents.go(4 hunks)server/internal/infrastructure/live-store/live_store_test.go(0 hunks)server/internal/infrastructure/live-store/redis/forfeits.go(1 hunks)server/internal/infrastructure/live-store/redis/intents.go(4 hunks)server/internal/interface/grpc/handlers/indexer.go(3 hunks)server/internal/interface/grpc/handlers/parser.go(2 hunks)
💤 Files with no reviewable changes (1)
- server/internal/infrastructure/live-store/live_store_test.go
🧰 Additional context used
🧬 Code Graph Analysis (28)
server/internal/infrastructure/live-store/redis/forfeits.go (5)
pkg/client-sdk/indexer/service.go (1)
Outpoint(145-148)pkg/client-sdk/types/types.go (1)
Outpoint(48-51)server/internal/core/application/types.go (1)
Outpoint(181-181)api-spec/protobuf/gen/ark/v1/types.pb.go (3)
Outpoint(23-30)Outpoint(45-45)Outpoint(60-62)server/internal/core/domain/vtxo.go (1)
Outpoint(14-17)
server/internal/infrastructure/live-store/redis/intents.go (4)
pkg/client-sdk/indexer/service.go (1)
Outpoint(145-148)pkg/client-sdk/types/types.go (1)
Outpoint(48-51)api-spec/protobuf/gen/ark/v1/types.pb.go (3)
Outpoint(23-30)Outpoint(45-45)Outpoint(60-62)server/internal/core/domain/vtxo.go (1)
Outpoint(14-17)
pkg/client-sdk/client/client.go (5)
pkg/client-sdk/types/types.go (2)
Outpoint(48-51)Vtxo(57-71)api-spec/protobuf/gen/ark/v1/types.pb.go (9)
Outpoint(23-30)Outpoint(45-45)Outpoint(60-62)TxData(331-338)TxData(353-353)TxData(368-370)Vtxo(188-206)Vtxo(221-221)Vtxo(236-238)server/internal/core/domain/vtxo.go (2)
Outpoint(14-17)Vtxo(38-53)server/internal/core/domain/offchain_tx.go (1)
Tx(30-33)server/internal/infrastructure/db/badger/round_repo.go (1)
Tx(256-259)
pkg/client-sdk/utils.go (5)
pkg/client-sdk/indexer/service.go (1)
Outpoint(145-148)pkg/client-sdk/types/types.go (1)
Outpoint(48-51)server/internal/core/application/types.go (1)
Outpoint(181-181)api-spec/protobuf/gen/ark/v1/types.pb.go (3)
Outpoint(23-30)Outpoint(45-45)Outpoint(60-62)server/internal/core/domain/vtxo.go (1)
Outpoint(14-17)
pkg/client-sdk/client/rest/service/models/v1_tx_data.go (1)
pkg/client-sdk/store/sql/sqlc/queries/models.go (1)
Tx(11-20)
server/internal/infrastructure/live-store/inmemory/intents.go (5)
pkg/client-sdk/indexer/service.go (1)
Outpoint(145-148)pkg/client-sdk/types/types.go (1)
Outpoint(48-51)server/internal/core/application/types.go (1)
Outpoint(181-181)api-spec/protobuf/gen/ark/v1/types.pb.go (3)
Outpoint(23-30)Outpoint(45-45)Outpoint(60-62)server/internal/core/domain/vtxo.go (1)
Outpoint(14-17)
pkg/client-sdk/store/sql/tx_store.go (1)
pkg/client-sdk/store/sql/sqlc/queries/query.sql.go (1)
UpdateTxParams(313-318)
pkg/client-sdk/indexer/rest/service/models/v1_indexer_vtxo.go (5)
pkg/client-sdk/indexer/service.go (1)
Outpoint(145-148)pkg/client-sdk/types/types.go (1)
Outpoint(48-51)api-spec/protobuf/gen/ark/v1/types.pb.go (3)
Outpoint(23-30)Outpoint(45-45)Outpoint(60-62)server/internal/core/domain/vtxo.go (1)
Outpoint(14-17)pkg/client-sdk/indexer/rest/service/models/v1_indexer_outpoint.go (1)
V1IndexerOutpoint(18-25)
pkg/client-sdk/types/types.go (4)
pkg/client-sdk/indexer/service.go (1)
Outpoint(145-148)api-spec/protobuf/gen/ark/v1/types.pb.go (6)
Outpoint(23-30)Outpoint(45-45)Outpoint(60-62)Vtxo(188-206)Vtxo(221-221)Vtxo(236-238)server/internal/core/domain/vtxo.go (2)
Outpoint(14-17)Vtxo(38-53)pkg/client-sdk/store/sql/sqlc/queries/models.go (1)
Vtxo(22-37)
pkg/client-sdk/types.go (4)
pkg/client-sdk/types/types.go (1)
Outpoint(48-51)server/internal/core/application/types.go (1)
Outpoint(181-181)api-spec/protobuf/gen/ark/v1/types.pb.go (3)
Outpoint(23-30)Outpoint(45-45)Outpoint(60-62)server/internal/core/domain/vtxo.go (1)
Outpoint(14-17)
server/internal/interface/grpc/handlers/parser.go (3)
pkg/client-sdk/client/client.go (2)
TxData(125-128)TxNotification(130-135)api-spec/protobuf/gen/ark/v1/types.pb.go (6)
TxData(331-338)TxData(353-353)TxData(368-370)TxNotification(386-397)TxNotification(412-412)TxNotification(427-429)server/internal/core/domain/offchain_tx.go (1)
Tx(30-33)
server/internal/core/domain/tx_request.go (3)
server/internal/infrastructure/db/postgres/sqlc/queries/models.go (2)
Vtxo(160-176)TxRequest(128-133)server/internal/core/domain/vtxo.go (1)
Vtxo(38-53)server/internal/infrastructure/db/sqlite/sqlc/queries/models.go (2)
Vtxo(140-156)TxRequest(108-113)
pkg/client-sdk/indexer/rest/client.go (5)
pkg/client-sdk/indexer/service.go (2)
TxData(75-78)Outpoint(145-148)pkg/client-sdk/client/client.go (1)
TxData(125-128)server/internal/core/domain/offchain_tx.go (1)
Tx(30-33)pkg/client-sdk/types/types.go (1)
Outpoint(48-51)server/internal/core/domain/vtxo.go (1)
Outpoint(14-17)
pkg/client-sdk/client/rest/service/models/v1_tx_notification.go (2)
pkg/client-sdk/client/rest/service/models/v1_tx_data.go (1)
V1TxData(18-25)pkg/client-sdk/client/rest/service/models/v1_vtxo.go (1)
V1Vtxo(19-59)
pkg/client-sdk/client/grpc/types.go (4)
pkg/client-sdk/types/types.go (1)
Outpoint(48-51)server/internal/core/application/types.go (1)
Outpoint(181-181)api-spec/protobuf/gen/ark/v1/types.pb.go (3)
Outpoint(23-30)Outpoint(45-45)Outpoint(60-62)server/internal/core/domain/vtxo.go (1)
Outpoint(14-17)
pkg/client-sdk/client/rest/service/models/v1_vtxo.go (5)
pkg/client-sdk/indexer/service.go (1)
Outpoint(145-148)pkg/client-sdk/types/types.go (1)
Outpoint(48-51)server/internal/core/application/types.go (1)
Outpoint(181-181)server/internal/core/domain/vtxo.go (1)
Outpoint(14-17)pkg/client-sdk/client/rest/service/models/v1_outpoint.go (1)
V1Outpoint(18-25)
server/internal/core/domain/vtxo_repo.go (3)
server/internal/core/application/types.go (1)
Outpoint(181-181)api-spec/protobuf/gen/ark/v1/types.pb.go (3)
Outpoint(23-30)Outpoint(45-45)Outpoint(60-62)server/internal/core/domain/vtxo.go (1)
Outpoint(14-17)
pkg/client-sdk/indexer/rest/service/models/v1_get_subscription_response.go (1)
pkg/client-sdk/indexer/rest/service/models/v1_indexer_tx_data.go (1)
V1IndexerTxData(18-25)
server/internal/infrastructure/db/badger/offchain_tx_repo.go (4)
server/internal/infrastructure/db/postgres/sqlc/queries/models.go (2)
VirtualTx(135-143)Tx(119-126)server/internal/infrastructure/db/sqlite/sqlc/queries/models.go (2)
VirtualTx(115-123)Tx(99-106)server/internal/core/domain/offchain_tx.go (2)
OffchainTx(35-48)Tx(30-33)server/internal/infrastructure/db/badger/round_repo.go (1)
Tx(256-259)
server/internal/core/application/types.go (5)
server/internal/core/domain/offchain_tx.go (1)
Tx(30-33)server/internal/infrastructure/db/badger/round_repo.go (1)
Tx(256-259)pkg/client-sdk/indexer/service.go (1)
TxData(75-78)pkg/client-sdk/client/client.go (1)
TxData(125-128)api-spec/protobuf/gen/ark/v1/types.pb.go (3)
TxData(331-338)TxData(353-353)TxData(368-370)
pkg/client-sdk/indexer/service.go (5)
server/internal/infrastructure/db/sqlite/sqlc/queries/models.go (1)
Tx(99-106)server/internal/core/domain/offchain_tx.go (1)
Tx(30-33)server/internal/infrastructure/db/badger/round_repo.go (1)
Tx(256-259)api-spec/protobuf/gen/ark/v1/types.pb.go (3)
TxData(331-338)TxData(353-353)TxData(368-370)pkg/client-sdk/client/client.go (1)
TxData(125-128)
pkg/client-sdk/types/interfaces.go (5)
pkg/client-sdk/types/types.go (2)
Outpoint(48-51)Vtxo(57-71)server/internal/core/application/types.go (1)
Outpoint(181-181)api-spec/protobuf/gen/ark/v1/types.pb.go (6)
Outpoint(23-30)Outpoint(45-45)Outpoint(60-62)Vtxo(188-206)Vtxo(221-221)Vtxo(236-238)server/internal/core/domain/vtxo.go (2)
Outpoint(14-17)Vtxo(38-53)pkg/client-sdk/store/sql/sqlc/queries/models.go (1)
Vtxo(22-37)
server/internal/infrastructure/db/service.go (4)
server/internal/core/domain/vtxo.go (1)
Outpoint(14-17)server/internal/core/domain/round.go (2)
Round(41-57)ForfeitTx(36-39)server/internal/core/ports/tx_decoder.go (1)
TxDecoder(12-14)server/internal/core/domain/tx_request.go (1)
TxRequests(98-98)
pkg/client-sdk/client.go (5)
pkg/client-sdk/indexer/service.go (1)
Outpoint(145-148)pkg/client-sdk/types/types.go (2)
Outpoint(48-51)Vtxo(57-71)api-spec/protobuf/gen/ark/v1/types.pb.go (6)
Outpoint(23-30)Outpoint(45-45)Outpoint(60-62)Vtxo(188-206)Vtxo(221-221)Vtxo(236-238)server/internal/core/domain/vtxo.go (2)
Outpoint(14-17)Vtxo(38-53)pkg/client-sdk/types/interfaces.go (2)
VtxoStore(38-52)TransactionStore(25-36)
server/internal/infrastructure/db/badger/vtxo_repo.go (2)
server/internal/core/application/types.go (1)
Outpoint(181-181)server/internal/core/domain/vtxo.go (1)
Outpoint(14-17)
pkg/client-sdk/store/kv/vtxo_repository.go (3)
pkg/client-sdk/types/types.go (5)
Outpoint(48-51)VtxoEvent(119-122)VtxosAdded(107-107)Vtxo(57-71)VtxosSpent(108-108)server/internal/core/domain/vtxo.go (2)
Outpoint(14-17)Vtxo(38-53)pkg/client-sdk/store/sql/sqlc/queries/models.go (1)
Vtxo(22-37)
pkg/client-sdk/store/sql/vtxo_store.go (6)
pkg/client-sdk/indexer/service.go (1)
Outpoint(145-148)pkg/client-sdk/types/types.go (4)
Outpoint(48-51)Vtxo(57-71)VtxoEvent(119-122)VtxosSpent(108-108)server/internal/core/domain/vtxo.go (2)
Outpoint(14-17)Vtxo(38-53)pkg/client-sdk/store/sql/sqlc/queries/query.sql.go (1)
UpdateVtxoParams(340-346)pkg/client-sdk/store/sql/sqlc/queries/models.go (1)
Vtxo(22-37)server/internal/infrastructure/db/sqlite/sqlc/queries/models.go (1)
Vtxo(140-156)
server/internal/infrastructure/db/sqlite/sqlc/queries/query.sql.go (5)
server/internal/infrastructure/db/postgres/sqlc/queries/db.go (1)
Queries(23-25)server/internal/infrastructure/db/sqlite/sqlc/queries/db.go (1)
Queries(23-25)server/internal/infrastructure/db/postgres/sqlc/queries/query.sql.go (3)
MarkVtxoAsSettledParams(394-399)MarkVtxoAsSpentParams(415-420)UpsertTxRequestParams(1454-1459)server/internal/infrastructure/db/postgres/sqlc/queries/models.go (3)
VtxoVw(184-201)RequestReceiverVw(37-46)RequestVtxoVw(48-69)server/internal/infrastructure/db/sqlite/sqlc/queries/models.go (3)
VtxoVw(164-181)RequestReceiverVw(35-44)RequestVtxoVw(46-67)
🪛 YAMLlint (1.37.1)
server/internal/infrastructure/db/sqlite/sqlc.yaml
[error] 45-45: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (125)
server/internal/core/domain/round_test.go (1)
15-17: LGTM! Test fixture updates align with schema changes.The addition of
ProofandMessagefields to the test fixtures correctly reflects the new required fields in theTxRequestdomain model. Using hardcoded values like "proof" and "message" is appropriate for test data.Also applies to: 46-48
server/internal/core/domain/offchain_tx_repo.go (1)
8-8: LGTM! Repository interface appropriately extended for batch operations.The new
GetOffchainTxsmethod provides a logical batch retrieval capability. The return type of[]stringsuggests these are raw transaction strings rather than structured objects, which aligns with the method name pattern.pkg/client-sdk/types.go (1)
132-132: LGTM! Type change improves consistency and clarity.The change from
[]types.VtxoKeyto[]types.Outpointaligns with the broader codebase refactoring mentioned in the PR objectives. This standardizes the use ofOutpointas the canonical identifier for transaction output references, improving code clarity and consistency.server/internal/infrastructure/db/postgres/offchain_tx_repo.go (1)
76-76: LGTM: Improved error messaging for better user experience.The change from
sql.ErrNoRowsto a formatted error message provides more context to callers about which specific offchain transaction was not found.server/internal/infrastructure/db/sqlite/offchain_tx_repo.go (1)
76-76: LGTM: Consistent error messaging across database implementations.The error message improvement maintains consistency with the PostgreSQL implementation and provides better context.
pkg/client-sdk/store/sql/migration/20250627235432_add_settled_by.up.sql (1)
1-5: Down migration script exists – no action neededThe corresponding down migration
pkg/client-sdk/store/sql/migration/20250627235432_add_settled_by.down.sqlis present, ensuring the schema changes are reversible.server/internal/core/application/utils.go (1)
130-130: LGTM: Consistent with schema refactoring.The change from
RedeemTxfield toPreconfirmed: truealigns with the broader refactoring visible in the migration scripts and maintains semantic consistency for newly decoded vtxo outputs.pkg/client-sdk/utils.go (2)
143-143: LGTM: Clean type system refactoring.The function signature change from
[]types.VtxoKeyto[]types.Outpointaligns with the broader type system unification mentioned in the PR objectives.
150-156: LGTM: Sorting logic correctly adapted for Outpoint type.The sorting function properly compares
Outpointelements byTxid(string comparison) first, thenVOut(uint32 subtraction), maintaining the same deterministic ordering semantics as before.pkg/client-sdk/indexer/service.go (2)
75-78: LGTM! Well-structured transaction data container.The new
TxDatastruct provides a clean abstraction for transaction information with consistent field naming that aligns with similar structures across the codebase.
80-88: LGTM! ScriptEvent enhancements align with transaction tracking improvements.The addition of
Txfield andCheckpointTxsmap enhances the event structure to provide richer transaction data and checkpoint transaction mappings, supporting the broader transaction settlement tracking improvements.server/internal/infrastructure/db/postgres/sqlc.yaml (1)
10-45: LGTM! Consistent nullable string handling across database backends.The Go type overrides properly handle nullable database columns by mapping them to
database/sql.NullString. The configuration maintains consistency with the SQLite SQLC configuration, ensuring uniform handling of the newproofandmessagefields across both PostgreSQL and SQLite backends.server/internal/infrastructure/live-store/redis/intents.go (1)
75-75: LGTM! Consistent outpoint-based keying for Redis operations.The refactoring from
.String()to.Outpoint.String()standardizes the keying mechanism for Redis set operations, aligning with the broader codebase changes that useOutpointas the consistent identifier for transaction outputs. This ensures proper membership checking, addition, and removal operations in Redis sets.Also applies to: 104-104, 152-152, 258-258
pkg/client-sdk/store/kv/transaction_repository.go (1)
66-92: LGTM! Enhanced transaction settlement tracking.The addition of the
settledByparameter and corresponding field assignment enhances transaction settlement tracking by recording the settler identifier. The implementation correctly integrates the new field while preserving the existing settlement logic and event emission.pkg/client-sdk/store/sql/sqlc/queries/models.go (1)
19-19: LGTM: Enhanced transaction metadata fields added correctly.The addition of
SettledByto theTxstruct andSettledBy,Unrolled, andArkTxidfields to theVtxostruct aligns well with the PR objectives for enhanced vtxo tracking. The use ofsql.NullStringfor nullable fields is appropriate and maintains data integrity.Also applies to: 34-36
server/internal/core/domain/tx_request_test.go (3)
12-26: Good addition of test data constants.The addition of
proofandmessageconstants provides clear test data for the updatedTxRequestconstructor requirements.
38-66: Excellent validation test coverage.The new "invalid" test case properly validates the required
proofandmessagefields, ensuring that the domain validation logic works correctly. The test fixtures cover both missing proof and missing message scenarios with appropriate error messages.
31-31: Constructor calls properly updated.All calls to
domain.NewTxRequesthave been correctly updated to include the newproofandmessageparameters, maintaining consistency with the updated domain model.Also applies to: 71-71, 99-99
pkg/client-sdk/indexer/rest/service/models/v1_indexer_vtxo.go (1)
24-25: LGTM: Model fields properly updated to match API schema.The field changes are well-aligned with the PR objectives:
- Addition of
ArkTxidfor linking to ark transactions- Renaming
IsRedeemedtoIsUnrolledfor clearer semantics- Addition of
SettledByfor settlement trackingThese changes are consistent with the broader schema updates across the codebase.
Also applies to: 45-46, 54-55
server/internal/infrastructure/live-store/inmemory/intents.go (1)
56-56: LGTM: Consistent key standardization using Outpoint.String().The systematic replacement of direct
String()method calls withOutpoint.String()provides consistent key representation across the vtxo management operations. This aligns well with the broader refactoring to standardize on theOutpointtype for transaction output identification.The changes maintain existing functionality while improving consistency:
- Duplicate input detection (line 56)
- Vtxo registration (line 83)
- Vtxo removal tracking (line 114)
- Vtxo deletion (line 169)
Also applies to: 83-83, 114-114, 169-169
server/internal/core/application/types.go (2)
91-94: Good abstraction with TxData struct.The new
TxDatastruct provides a clean abstraction for consolidating transaction hex data (Tx) and transaction ID (Txid). This standardizes transaction data representation across the codebase and aligns with similar structures defined in other packages.
97-97: Enhanced TransactionEvent structure.The changes improve the
TransactionEventstructure:
- Embedding
TxDataeliminates redundant separate fields and provides cleaner access to transaction data- The
CheckpointTxsmap enables tracking of checkpoint transactions keyed by outpoint strings, supporting the enhanced vtxo settlement tracking objectivesThese changes align well with the PR's goal of providing richer transaction metadata.
Also applies to: 101-101
pkg/client-sdk/base_client.go (2)
184-184: LGTM! Field rename aligns with refactor objectives.The change from
vtxo.Redeemedtovtxo.Unrolledis consistent with the broader refactor to improve terminology clarity. The logic remains correct for classifying spent vtxos.
517-527: LGTM! Type change maintains correct filtering logic.The function signature and implementation correctly updated to use
types.Outpointinstead oftypes.VtxoKey. The comparison logic usingvtxo.Outpoint == outpointis appropriate and maintains the filtering functionality.pkg/client-sdk/client/client.go (2)
63-63: LGTM! Type unification improves consistency.The change from
types.VtxoKeytotypes.Outpointaligns with the broader effort to unify outpoint representation across the codebase.
125-134: LGTM! Enhanced transaction data structure improves clarity.The introduction of the
TxDatastruct and its embedding inTxNotificationprovides a cleaner, more structured approach to handling transaction data. TheCheckpointTxsmap enables enhanced tracking of checkpoint transactions associated with vtxos, which aligns with the PR objectives for better transaction linkage.pkg/client-sdk/client/grpc/client.go (1)
285-288: LGTM! Transaction data structure improvements enhance consistency.The embedding of
TxDatain bothCommitmentTxandArkTxnotifications provides a cleaner, more structured approach to transaction data handling.Also applies to: 308-311
server/internal/core/domain/vtxo.go (2)
44-50: LGTM! Enhanced vtxo metadata supports better transaction tracking.The new fields
SettledBy,ArkTxid, andPreconfirmedprovide enhanced metadata for vtxo lifecycle tracking. The updated comment forSpentByclarifies its usage for forfeit or checkpoint transaction IDs, which aligns with the PR objectives.
55-59: LGTM! String method aids debugging with JSON representation.The
String()method provides a useful debugging utility by returning a pretty-printed JSON representation. The error suppression with// nolintis acceptable for a debugging method, though it means marshaling errors won't be visible.api-spec/protobuf/ark/v1/indexer.proto (2)
278-283: LGTM! Protobuf changes support enhanced vtxo tracking.The field rename from
is_redeemedtois_unrolledimproves terminology clarity, and the newsettled_byandark_txidfields provide enhanced metadata for transaction tracking, consistent with the domain model changes.
358-365: LGTM! Enhanced subscription response supports checkpoint transactions.The addition of the
txfield andcheckpoint_txsmap toGetSubscriptionResponse, along with the newIndexerTxDatamessage, provides richer transaction data in subscription events. This aligns with the PR objectives for better transaction linkage and tracking.server/internal/core/domain/tx_request.go (3)
13-14: LGTM! New fields properly added to TxRequest.The addition of
ProofandMessagefields aligns with the PR objective to persist intent proofs and messages alongside vtxo data.
17-28: Constructor properly updated to handle new fields.The
NewTxRequestfunction correctly accepts and initializes the newProofandMessageparameters, maintaining the existing validation flow.
64-69: Validation logic ensures required fields are present.The validation correctly enforces that both
ProofandMessageare non-empty strings, regardless of theignoreOutsflag. This ensures data integrity for transaction requests.pkg/client-sdk/types/types.go (3)
48-55: Type renaming improves clarity.Renaming
VtxoKeytoOutpointaligns with Bitcoin terminology and improves code readability. TheString()method provides consistent formatting.
57-77: Vtxo struct enhanced with settlement tracking fields.The addition of
SettledByandArkTxidfields provides granular tracking of vtxo settlement and spending, which supports the PR's goal of distinguishing between spent and renewed vtxos. TheUnrolledfield rename fromRedeemedimproves clarity. TheString()method will aid in debugging.
148-148: Transaction struct gains settlement tracking.Adding the
SettledByfield to theTransactionstruct maintains consistency with the vtxo settlement tracking approach.pkg/client-sdk/indexer/rest/service/models/v1_indexer_tx_data.go (1)
1-54: Auto-generated REST model follows standard patterns.The
V1IndexerTxDatastruct is properly generated with standard validation and marshaling methods. The simple structure withTxandTxidfields supports the transaction data consolidation mentioned in the PR objectives.pkg/client-sdk/client/rest/service/models/v1_tx_data.go (1)
1-54: Auto-generated REST model is consistent with indexer version.The
V1TxDatastruct mirrors the indexer'sV1IndexerTxDatastructure and follows the same standard patterns for validation and marshaling. This consistency supports the unified transaction data approach.server/internal/core/domain/vtxo_repo.go (1)
7-8: Interface enhanced with granular vtxo tracking methods.The addition of
SettleVtxosand the updatedSpendVtxosmethods with map parameters provide more precise tracking of which transaction affects each vtxo. This supports the PR's objective of distinguishing between spent and renewed vtxos and improves settlement tracking granularity.pkg/client-sdk/indexer/rest/client.go (1)
722-737: LGTM: Field updates align with the broader refactoring.The conversion from
VtxoKeytoOutpointand the addition of new fields (SettledBy,ArkTxid,Unrolled) are consistent with the PR objectives to enhance vtxo tracking.pkg/client-sdk/store/sql/sqlc/query.sql (3)
3-4: LGTM: INSERT statement correctly includes new vtxo fields.The addition of
unrolled,settled_by, andark_txidcolumns with corresponding placeholders is correct and consistent with the schema changes.
10-12: LGTM: UPDATE statement properly handles new settlement fields.The UpdateVtxo query correctly sets the new
settled_byandark_txidfields alongside the existingspent_byfield.
28-29: LGTM: Transaction queries consistently include settled_by field.The INSERT, UPDATE, and REPLACE operations for the
txtable all properly handle the newsettled_bycolumn.Also applies to: 35-36, 46-46
pkg/client-sdk/store/sql/tx_store.go (3)
55-55: LGTM: AddTransactions correctly includes SettledBy field.The insertion of the
SettledByfield with proper null handling is consistent with the new schema.
79-79: LGTM: SettleTransactions method signature and implementation updated correctly.The addition of the
settledByparameter and its proper handling in both the in-memory transaction objects and database updates is correct. The breaking change to the method signature aligns with the PR objectives.Also applies to: 91-96
186-186: LGTM: RbfTransactions and rowToTx properly handle SettledBy field.Both the RBF operation and the row-to-struct conversion correctly handle the new
SettledByfield with appropriate null handling.Also applies to: 307-307
server/internal/interface/grpc/handlers/parser.go (2)
49-58: LGTM: Boolean field renaming and new vtxo fields are correctly implemented.The consistent renaming of boolean fields with the "Is" prefix (
IsSpent,IsSwept,IsPreconfirmed,IsUnrolled) and the addition ofSettledByandArkTxidfields align with the updated protobuf schema and PR objectives.
68-81: LGTM: Transaction event conversion properly handles checkpoint transactions.The addition of the
CheckpointTxsmap with proper nil checking and the conversion to protobufTxDatastructs is correctly implemented. The replacement of theHexfield withTxaligns with the schema changes.pkg/client-sdk/client/grpc/types.go (1)
112-127: LGTM: Vtxo conversion updated correctly for new field structure.The replacement of
VtxoKeywithOutpointand the addition of new fields (SettledBy,ArkTxid) along with the proper use of "Is" prefixed getters for boolean fields are all consistent with the updated protobuf schema and broader refactoring goals.server/internal/infrastructure/db/sqlite/round_repo.go (2)
396-402: LGTM! Improved error handling for no rows found.The change to return
nil, nilwhen no rows are found instead of propagatingsql.ErrNoRowsis appropriate and consistent with the Postgres implementation. The addition ofIds3parameter aligns with the PR's objective to include checkpoint transaction data.
474-475: Field population logic is correct.The implementation properly retrieves
ProofandMessagefields from database rows and populates the domain objects consistently across all code paths inrowsToRounds.Also applies to: 487-488, 514-515
server/internal/infrastructure/db/postgres/round_repo.go (2)
400-403: LGTM! Consistent error handling improvement.The error handling change matches the SQLite implementation and properly handles the no-rows-found case.
474-475: Consistent field population implementation.The field retrieval and population logic is correctly implemented and matches the SQLite version.
Also applies to: 487-488, 514-515
server/internal/infrastructure/db/sqlite/migration/20250627165356_update_intent_and_vtxo.down.sql (1)
1-38: Migration script structure looks comprehensive.The down migration properly:
- Drops dependent views before altering tables
- Removes all new columns introduced in the up migration
- Recreates the
spent_bycolumn with proper constraints- Restores views with original join logic
The migration follows proper order of operations and should successfully rollback the schema changes.
api-spec/openapi/swagger/ark/v1/indexer.swagger.json (4)
1018-1028: Well-defined new schema for transaction data.The
v1IndexerTxDataschema is appropriately structured with clear field definitions for representing transaction information.
796-805: Appropriate additions for checkpoint transaction support.The new
txandcheckpointTxsfields enhance subscription responses with transaction data as described in the PR objectives.
1093-1093: Schema evolution aligns with PR objectives.The rename from
isRedeemedtoisUnrolledand the addition ofsettledByandarkTxidfields support the enhanced vtxo settlement tracking described in the PR.Also applies to: 1108-1113
1052-1054: Consistent addition of settlement tracking.The
settledByfield addition to transaction history records maintains consistency with the vtxo schema changes.server/internal/infrastructure/db/postgres/migration/20250627185230_update_intent_and_vtxo.down.sql (1)
1-40: Postgres migration mirrors SQLite implementation correctly.The down migration properly removes all new columns and recreates views with original structure. The Postgres-specific syntax (VARCHAR vs TEXT, string_agg vs group_concat) is correctly used.
pkg/client-sdk/client/rest/service/models/v1_vtxo.go (2)
24-26: LGTM! New fields added for enhanced transaction tracking.The addition of
ArkTxidandSettledByfields aligns with the PR objectives to better track vtxo spending and settlement relationships.Also applies to: 54-56
36-46: Good naming convention improvement.The boolean fields have been renamed with the
Isprefix for better clarity and consistency. The change fromRedeemedtoIsUnrolledalso provides more accurate terminology.api-spec/protobuf/ark/v1/types.proto (3)
29-36: Excellent naming consistency and new field additions.The boolean field renaming with
is_prefix improves clarity, and the newsettled_byandark_txidfields will enable better transaction relationship tracking as outlined in the PR objectives.
38-41: Well-designed TxData message structure.The new
TxDatamessage provides a clean abstraction for transaction ID and raw transaction data, which will be useful for the checkpoint transaction handling mentioned in the PR.
44-50: Good enhancement to TxNotification for offchain transactions.The addition of
txfield andcheckpoint_txsmap specifically for offchain transactions aligns well with the PR's goal to provide checkpoint transaction information while maintaining clarity.api-spec/openapi/swagger/ark/v1/service.swagger.json (3)
834-844: Well-structured TxData schema definition.The new
v1TxDataschema properly encapsulates transaction ID and raw transaction data, consistent with the protobufTxDatamessage.
851-874: Good enhancement to transaction notifications.The addition of the
txfield andcheckpointTxsmap for offchain transactions provides the checkpoint transaction information mentioned in the PR objectives, while maintaining backward compatibility.
904-924: Consistent field naming and new metadata fields.The boolean field renaming with
isprefix and addition ofsettledByandarkTxidfields align perfectly with the protobuf definitions and support the enhanced vtxo tracking capabilities.pkg/client-sdk/client/rest/client.go (4)
22-22: Appropriate import addition.The
wirepackage import is necessary for parsing outpoint strings in the checkpoint transaction handling.
457-464: Good refactoring to use TxData structure.The embedding of transaction data in the
TxDatastruct provides better organization and aligns with the protobuf message structure.
480-487: Well-implemented TxData usage and checkpoint transaction handling.The implementation correctly constructs the
TxDatastruct and populates theCheckpointTxsmap, supporting the enhanced transaction tracking capabilities.
554-570: Excellent field mapping updates.The
vtxosFromRestfunction correctly maps the new fields (SettledBy,ArkTxid) and uses the renamed boolean fields, maintaining consistency with the updated data model.pkg/client-sdk/indexer/grpc/client.go (3)
528-537: Well-implemented checkpoint transaction handling.The checkpoint transactions map construction is clean and properly handles the conversion from protobuf messages to the client SDK's
TxDatastructure.
539-546: Good integration of new fields in ScriptEvent.The addition of
TxandCheckpointTxsfields to theScriptEventprovides the enhanced transaction information needed for the updated vtxo tracking system.
611-627: Excellent field mapping consistency.The
newIndexerVtxofunction correctly maps all the new fields (Unrolled,SettledBy,ArkTxid) and uses theOutpointtype consistently, aligning with the codebase-wide type unification.server/internal/infrastructure/db/sqlite/vtxo_repo.go (3)
233-255: LGTM! Well-implemented settlement tracking.The new
SettleVtxosmethod correctly handles marking vtxos as settled with both spender and settler identifiers, maintaining transactional integrity.
257-279: Good improvement to support per-vtxo spender tracking.The updated
SpendVtxosmethod signature and implementation correctly handle individual spender tracking for each vtxo and associates them with the Ark transaction ID.
419-425: Field mappings correctly handle new settlement metadata.The
rowToVtxofunction properly maps the newSettledBy,ArkTxid, andPreconfirmedfields from database rows.pkg/client-sdk/types/interfaces.go (1)
27-48: Well-designed interface enhancements for granular tracking.The interface changes appropriately support:
- Settlement tracking with the
settledByparameter- Per-vtxo spender identification through map-based parameters
- Explicit settlement operations via the new
SettleVtxosmethod- Cleaner type usage with
Outpointinstead ofVtxoKeyserver/internal/infrastructure/db/sqlite/sqlc/query.sql (3)
174-187: Database schema properly updated for enhanced vtxo tracking.The
UpsertVtxoquery correctly handles the newsettled_by,preconfirmed, andark_txidfields in both INSERT and UPDATE operations.
221-223: New settlement query enables dual tracking of spender and settler.The
MarkVtxoAsSettledquery correctly implements the ability to track both who spent a vtxo and which transaction settled it.
224-225: Enhanced spending query now tracks Ark transaction linkage.The updated
MarkVtxoAsSpentquery appropriately associates spent vtxos with their Ark transaction ID for improved traceability.server/internal/interface/grpc/handlers/indexer.go (2)
499-508: Efficient checkpoint transaction handling.The implementation correctly maps checkpoint transactions only when they exist, avoiding unnecessary allocations.
690-697: Field mappings correctly updated to match new schema.The changes properly reflect the new vtxo fields and renamed boolean flags as described in the PR objectives.
server/internal/infrastructure/db/sqlite/migration/20250627165356_update_intent_and_vtxo.up.sql (1)
6-13: Verify migration compatibility with existing data.The NOT NULL constraints on
proof,message(lines 6-7), andpreconfirmed(line 10) could cause migration failures if there's existing data in these tables. Consider either:
- Adding default values for these columns
- Making them nullable initially and updating them in a separate step
- Confirming that these tables are empty or will be recreated
Please verify that this migration will work correctly with any existing data in the database.
server/internal/core/application/indexer.go (3)
214-221: Correct usage of SettledBy field for commitment transaction indexing.The logic properly uses the new
SettledByfield to index commitment transactions, aligning with the PR's goal of preserving the original linkage.
289-289: Consistent update to direct field access for Preconfirmed.All instances correctly use the
Preconfirmedfield directly instead of the removed method call.Also applies to: 492-492, 550-550, 574-574
574-593: Well-implemented settlement tracking logic.The new helper functions correctly distinguish between settled and spent vtxos using the appropriate fields (
SettledByfor settlements,ArkTxidfor spends).server/internal/infrastructure/db/badger/vtxo_repo.go (2)
73-82: Updated SpendVtxos correctly tracks ark transaction IDs.The method properly sets both
SpentByandArkTxidfields, enabling precise tracking of spending transactions.
322-358: Helper methods correctly implement settlement and spending logic.Both
settleVtxoandspendVtxoproperly set the appropriate fields and handle errors gracefully, making the operations idempotent.server/internal/infrastructure/db/badger/offchain_tx_repo.go (2)
52-55: Correctly persists checkpoint transactions alongside offchain transactions.The method properly stores both the main transaction and its associated checkpoint transactions.
145-200: Well-implemented checkpoint transaction persistence and retrieval.The helper methods correctly handle storing and querying checkpoint transactions, with proper support for transactional contexts.
server/internal/core/application/fraud.go (4)
35-50: LGTM! Clean refactoring using the new vtxo fields.The logic is now more straightforward - using
SettledByto determine the fraud reaction type is clearer than the previous indirect approach.
83-96: Good simplification of the method signature.Using
vtxo.SettledByto retrieve the round is cleaner and reduces coupling.
31-49: LGTM! Clean refactoring using new vtxo fieldsThe simplified logic using
vtxo.SettledByto determine whether to broadcast a checkpoint or forfeit transaction is much cleaner and easier to understand.
83-96: LGTM! Good API improvementThe function signature change to accept
domain.Vtxodirectly makes the API cleaner and more intuitive. The use ofvtxo.SettledByandvtxo.Outpointis consistent with the new data model.pkg/client-sdk/store/kv/vtxo_repository.go (3)
45-45: Good standardization of database keys.Using
Outpoint.String()as the key is the correct approach since it uniquely identifies a VTXO.Also applies to: 82-82, 116-116, 131-131
61-93: Well-designed method signature for spending VTXOs.The map-based approach allows different spender IDs per VTXO while the
arkTxidparameter tracks the overall ark transaction. Good separation of concerns.
55-55: Important fix for event accuracy.Emitting events with the actual modified VTXOs instead of the requested ones ensures event consumers receive accurate state change information.
Also applies to: 89-89, 123-123
server/internal/core/application/service.go (2)
179-180: Clean refactoring with TxData struct.Grouping transaction ID and data into a single struct improves code organization and reduces the likelihood of mismatched tx/txid pairs.
Also applies to: 187-188, 254-254, 269-269
948-957: Good addition of proof persistence.Encoding and storing the BIP322 signature proof enhances auditability and enables future verification of transaction authorization.
pkg/client-sdk/client.go (3)
1168-1171: LGTM! Type migration is consistent.The change from
types.VtxoKeytotypes.Outpointis applied consistently throughout the maps used for tracking spendable and spent vtxos.
3000-3019: Good semantic distinction between settling and spending.The change to use a map for
vtxosToSpendand the extraction ofSpentByinformation fromindexedSpentVtxosenables proper tracking of which transaction spent each vtxo. This aligns well with the PR's objective of distinguishing between vtxos that were spent versus renewed.
3164-3190: Correct implementation of checkpoint transaction tracking.The mapping of vtxo outpoints to their corresponding checkpoint transaction IDs through
arkTx.CheckpointTxs[vtxo.Outpoint].Txidproperly implements the new tracking requirements. This enables clients to trace the complete spending path.server/internal/infrastructure/db/service_test.go (3)
430-432: Good test coverage for new TxRequest fields.The addition of
ProofandMessagefields to the test fixtures ensures the new fields are properly handled during round repository operations.Also applies to: 451-452
584-666: Excellent test coverage for vtxo spending and settling logic.The tests properly verify:
SpendVtxossets bothSpentBy(checkpoint txid) andArkTxid(ark txid)SettleVtxossets bothSpentBy(forfeit txid) andSettledBy(commitment txid)This validates the PR's goal of providing clear distinction between spent and renewed vtxos.
794-839: Good refactoring of test assertions.The change from a boolean comparison function to direct assertions with
requireimproves test diagnostics by providing clear failure messages for each field that doesn't match.server/internal/infrastructure/db/service.go (1)
369-387: Correct checkpoint transaction tracking for offchain vtxos.The implementation properly extracts transaction IDs from decoded checkpoint transactions and associates them with the spent vtxos. This enables precise tracking of the spending path as intended by the PR.
server/internal/infrastructure/db/postgres/sqlc/queries/models.go (1)
1-202: LGTM! Auto-generated model changes align with schema updates.The auto-generated models correctly reflect the database schema changes, adding new fields for transaction request metadata (
Proof,Message) and VTXO settlement tracking (SettledBy,ArkTxid,Preconfirmed).pkg/client-sdk/store/service_test.go (1)
18-299: Well-structured test updates with comprehensive coverage.The test file properly exercises the new settlement functionality with:
- Clear test data organization using
Outpointinstead of deprecatedVtxoKey- Comprehensive test coverage for the new
SettleVtxosmethod- Proper validation of both
SpentByandSettledByfields- Idempotency checks ensuring operations can't be repeated
server/internal/infrastructure/db/postgres/sqlc/query.sql (1)
236-240: Clear separation of VTXO state transitions.The distinct
MarkVtxoAsSettledandMarkVtxoAsSpentupdate statements provide clear semantics for different VTXO lifecycle events, aligning well with the PR's goal of distinguishing between spending and renewal.server/internal/infrastructure/db/sqlite/sqlc/queries/models.go (1)
1-182: LGTM! SQLite models consistent with Postgres changes.The auto-generated SQLite models correctly mirror the Postgres model changes, ensuring consistency across different database backends.
server/internal/infrastructure/db/sqlite/sqlc/queries/query.sql.go (6)
277-334: LGTM!The extension of
GetTxsByTxidto includevirtual_txandcheckpoint_txtables is implemented correctly with proper parameter handling.
425-444: LGTM!The new
MarkVtxoAsSettledmethod is correctly implemented with proper handling of nullable fields.
446-465: LGTM!The
MarkVtxoAsSpentmethod correctly updated to include theark_txidfield.
735-828: LGTM!The
SelectRoundWithRoundIdmethod correctly updated to includeproofandmessagefields in the appropriate views.
1472-1495: LGTM!The
UpsertTxRequestmethod correctly updated to includeproofandmessagefields with consistent use of nullable types.
1533-1585: LGTM!The
UpsertVtxomethod correctly updated to includesettled_by,preconfirmed, andark_txidfields with proper field ordering and type handling.server/internal/infrastructure/db/postgres/vtxo_repo.go (3)
86-215: LGTM!The migration from the previous type to
queries.VtxoVwis consistently applied across all query methods.
238-260: LGTM!The new
SettleVtxosmethod is well-implemented with proper transaction handling and flexible per-vtxo spender mapping.
410-430: LGTM!The
rowToVtxofunction correctly maps the new fields from the database row to the domain model.pkg/client-sdk/store/sql/sqlc/queries/query.sql.go (2)
63-104: Field rename from redeemed to unrolled is correctly implemented.The renaming of
redeemedtounrolledand addition ofsettled_byandark_txidfields are properly handled in the INSERT operation.
330-357: LGTM!The
UpdateVtxomethod correctly updates vtxos with spent status and new tracking fields.server/internal/infrastructure/db/postgres/sqlc/queries/query.sql.go (2)
273-318: Efficient parameter handling for PostgreSQL.The PostgreSQL implementation efficiently uses a single array parameter for all three UNION queries, which is better than the SQLite approach of separate parameters.
447-1565: Consistent implementation of new vtxo fields.All vtxo-related queries correctly updated to use
VtxoVwtype and properly scan the newsettled_by,ark_txid, andpreconfirmedfields.
pkg/client-sdk/store/sql/migration/20250627235432_add_settled_by.down.sql
Show resolved
Hide resolved
...r/internal/infrastructure/db/postgres/migration/20250627185230_update_intent_and_vtxo.up.sql
Show resolved
Hide resolved
...r/internal/infrastructure/db/postgres/migration/20250627185230_update_intent_and_vtxo.up.sql
Show resolved
Hide resolved
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
server/internal/core/application/fraud.go(2 hunks)server/internal/core/application/indexer.go(6 hunks)server/internal/core/domain/vtxo.go(3 hunks)server/internal/infrastructure/db/badger/ark_repo.go(14 hunks)server/internal/infrastructure/db/badger/offchain_tx_repo.go(0 hunks)server/internal/infrastructure/db/postgres/offchain_tx_repo.go(1 hunks)server/internal/infrastructure/db/postgres/vtxo_repo.go(12 hunks)server/internal/infrastructure/db/service.go(8 hunks)server/internal/infrastructure/db/sqlite/offchain_tx_repo.go(1 hunks)server/internal/infrastructure/db/sqlite/vtxo_repo.go(3 hunks)
💤 Files with no reviewable changes (1)
- server/internal/infrastructure/db/badger/offchain_tx_repo.go
🚧 Files skipped from review as they are similar to previous changes (7)
- server/internal/infrastructure/db/postgres/offchain_tx_repo.go
- server/internal/infrastructure/db/sqlite/offchain_tx_repo.go
- server/internal/core/domain/vtxo.go
- server/internal/core/application/fraud.go
- server/internal/infrastructure/db/sqlite/vtxo_repo.go
- server/internal/infrastructure/db/postgres/vtxo_repo.go
- server/internal/core/application/indexer.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
server/internal/infrastructure/db/badger/ark_repo.go (5)
server/internal/core/domain/round_repo.go (2)
RoundRepository(9-23)RoundStats(25-34)server/internal/core/domain/offchain_tx_repo.go (1)
OffchainTxRepository(5-9)server/internal/core/domain/round.go (2)
ForfeitTx(36-39)Stage(30-34)common/tree/tx_graph.go (1)
TxGraphChunk(20-26)server/internal/core/domain/offchain_tx.go (2)
OffchainTx(35-48)OffchainTxUndefinedStage(9-9)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: server unit tests
- GitHub Check: common unit tests
- GitHub Check: sdk unit tests
- GitHub Check: integration tests
🔇 Additional comments (5)
server/internal/infrastructure/db/service.go (2)
332-338: Correct implementation of vtxo settling logic.The change from
SpendVtxostoSettleVtxosand the use of a map to track forfeit transaction IDs aligns well with the PR objectives for enhanced vtxo tracking.
371-390: Proper implementation of checkpoint transaction tracking.The logic correctly maps spent vtxos to their checkpoint transaction IDs and passes the virtual transaction ID as the ark transaction ID, which aligns with the PR's design for improved transaction linkage.
server/internal/infrastructure/db/badger/ark_repo.go (3)
16-16: Breaking change: store directory renamed.Changing the store directory from "rounds" to "ark" will break existing deployments. Consider adding migration logic or documenting this breaking change prominently.
Do you want to add migration logic to handle existing "rounds" directories, or should this be documented as a breaking change requiring manual migration?
22-25: Good architectural decision to unify repositories.Combining RoundRepository and OffchainTxRepository into a single ArkRepository interface is a clean design that reflects the related nature of these data types.
432-447: Well-implemented transaction search across both repositories.The
findTxsmethod correctly searches both round and offchain transactions, providing a unified view of all transactions as intended by the PR objectives.
This make the server persist intent proofs and messages along with the existing decoded data.
This also changes the tx referenced by
spent_byfield of the vtxo message when this is renewed: in this case, the spent_by references to the forfeit tx, while originally it referenced the commitment txid where it was forfeited.Since this was a useful link for clients, it's been preserved by adding a new
settled_byfield to the vtxo message:only when vtxos are renewed, their
spent_byfield is set to the related forfeit txid, and theirsettled_byfield is set to the new commitment txid. This way it also makes it easier for clients to understand if a vtxo was spent or renewed (including also collab exit).When vtxos are spent,
spent_byreferences the ark txid instead of the checkpoint one. I believe this is fine as it simplifies the message by hiding this detail in this vtxo context, while checkpoints can be easily retrieved with "tx context" endpoints, like the vtxo chain one or the tx stream.NIT: This also adds String() method to server's domain.Vtxo in server and sdk's types.Vtxo that pretty prints the vtxo as JSON, useful for debugging. And also, this way the linter stops converting every
vtxo.Outpoint.String()tovtxo.String().NIT-2: in sdk,
types.VtxoKeyhas been renamedtypes.Outpoint.Closes #575.
Please @louisinger @Kukks review.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Database
Tests
Documentation