chore: add stream info on attestation table#1259
Conversation
WalkthroughAdds two nullable text columns, Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DB as Database
rect rgba(192,223,255,0.25)
Note over Client,DB: Create attestation (request_attestation)
Client->>DB: CALL request_attestation(..., data_provider, stream_id, ...)
DB-->>Client: INSERT row including data_provider, stream_id
end
rect rgba(212,255,192,0.25)
Note over Client,DB: Retrieve attestations (list_attestations)
Client->>DB: CALL list_attestations(...)
DB-->>Client: SELECT rows with data_provider, stream_id in result set
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ 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)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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: 1
🧹 Nitpick comments (1)
internal/migrations/028-attestation-add-provider-stream.sql (1)
16-26: Consider removing redundant index.The composite index
ix_att_provider_streamon(data_provider, stream_id)can efficiently serve queries filtering bydata_provideralone (due to index prefix matching). The standaloneix_att_data_providerindex is therefore redundant.If you want to keep both for clarity or future-proofing, that's fine. Otherwise, you can remove the standalone index:
--- Create index on data_provider for efficient querying -CREATE INDEX IF NOT EXISTS ix_att_data_provider - ON attestations(data_provider); - -- Create index on stream_id for efficient querying CREATE INDEX IF NOT EXISTS ix_att_stream_id ON attestations(stream_id);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
internal/migrations/024-attestation-actions.sql(4 hunks)internal/migrations/028-attestation-add-provider-stream.sql(1 hunks)tests/streams/attestation/attestation_request_test.go(3 hunks)tests/streams/attestation/attestation_retrieval_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: outerlook
Repo: trufnetwork/node PR: 1207
File: internal/migrations/023-attestation-schema.sql:20-21
Timestamp: 2025-10-10T13:00:13.731Z
Learning: In the attestations table (internal/migrations/023-attestation-schema.sql), the primary key is (requester, created_height, attestation_hash) because attestation_hash is computed deterministically from user input only (version|algo|data_provider|stream_id|action_id|args) and does not include created_height. This allows the same user to request the same attestation at different block heights.
Learnt from: outerlook
Repo: trufnetwork/node PR: 1207
File: internal/migrations/024-attestation-actions.sql:58-76
Timestamp: 2025-10-10T13:00:14.189Z
Learning: In the attestation system for internal/migrations/024-attestation-actions.sql, the attestation_hash is computed from (version|algo|data_provider|stream_id|action_id|args) and intentionally excludes created_height. This design ensures the hash is deterministic based only on user input, not network state like block height.
📚 Learning: 2025-10-10T13:00:13.731Z
Learnt from: outerlook
Repo: trufnetwork/node PR: 1207
File: internal/migrations/023-attestation-schema.sql:20-21
Timestamp: 2025-10-10T13:00:13.731Z
Learning: In the attestations table (internal/migrations/023-attestation-schema.sql), the primary key is (requester, created_height, attestation_hash) because attestation_hash is computed deterministically from user input only (version|algo|data_provider|stream_id|action_id|args) and does not include created_height. This allows the same user to request the same attestation at different block heights.
Applied to files:
internal/migrations/028-attestation-add-provider-stream.sqltests/streams/attestation/attestation_retrieval_test.gointernal/migrations/024-attestation-actions.sqltests/streams/attestation/attestation_request_test.go
📚 Learning: 2025-10-10T13:00:14.189Z
Learnt from: outerlook
Repo: trufnetwork/node PR: 1207
File: internal/migrations/024-attestation-actions.sql:58-76
Timestamp: 2025-10-10T13:00:14.189Z
Learning: In the attestation system for internal/migrations/024-attestation-actions.sql, the attestation_hash is computed from (version|algo|data_provider|stream_id|action_id|args) and intentionally excludes created_height. This design ensures the hash is deterministic based only on user input, not network state like block height.
Applied to files:
internal/migrations/028-attestation-add-provider-stream.sqltests/streams/attestation/attestation_retrieval_test.gointernal/migrations/024-attestation-actions.sqltests/streams/attestation/attestation_request_test.go
🧬 Code graph analysis (1)
tests/streams/attestation/attestation_retrieval_test.go (1)
tests/streams/attestation/test_helpers.go (2)
TestDataProviderHex(31-31)TestStreamID(30-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: acceptance-test
🔇 Additional comments (7)
internal/migrations/028-attestation-add-provider-stream.sql (1)
8-14: LGTM on column definitions.The new columns use appropriate types and constraints. The DEFAULT '' ensures existing rows (if any) get valid values, while NOT NULL enforces the requirement for new insertions.
tests/streams/attestation/attestation_retrieval_test.go (1)
163-172: LGTM on test updates.The test correctly validates the two new columns:
- Updates expected column count from 6 to 8
- Verifies
data_providerandstream_idare present at the correct positions (indices 3 and 4)- Asserts values match the test constants
TestDataProviderHexandTestStreamIDtests/streams/attestation/attestation_request_test.go (2)
88-89: LGTM on new assertions.The test correctly verifies that
data_providerandstream_idare stored and match the input values passed torequest_attestation.
185-207: LGTM on column mapping updates.The
fetchAttestationRowfunction correctly:
- Adds
data_providerandstream_idto the SELECT statement- Maps them from indices 2 and 3
- Shifts all subsequent column indices by 2 positions
The mapping is consistent with the updated schema.
internal/migrations/024-attestation-actions.sql (3)
150-156: INSERT statement structure is correct.Assuming the migration ordering issue is resolved, the INSERT statement correctly:
- Adds
data_providerandstream_idto the column list- Inserts the corresponding parameter values in the correct positions
- Maintains proper alignment between columns and values
287-292: RETURN TABLE signature correctly updated.The
list_attestationsaction's return signature now includesdata_provider TEXTandstream_id TEXT, maintaining consistency with the updated schema and matching test expectations.
315-351: All SELECT statements correctly include new columns.Each of the five SELECT branches in
list_attestationsconsistently includesdata_providerandstream_idin the projection, ensuring uniform output regardless of which query path is taken.
Time Submission Status
|
|
log: test passed, this issue dragged too long, I will merge then continue |
1 participant
resolves: https://github.com/trufnetwork/trufscan/issues/125
Summary by CodeRabbit
New Features
Chores
Tests