feat: enable attestation querying and retrieval#1216
Conversation
…ethod - Updated the attestation schema to use request_tx_id as the primary key, improving uniqueness and clarity. - Introduced the verify_payload method in the tn_attestation extension for validating attestation signatures against canonical payloads. - Refactored existing actions to accommodate the new request_tx_id and adjusted related SQL queries. - Enhanced integration tests to cover new functionality and ensure robust validation of attestation processes. - Improved error handling and logging throughout the attestation workflow for better operational visibility. These changes enhance the reliability and maintainability of the attestation system, ensuring accurate processing and verification of attestations.
…station extension - Deleted the verify_payload method from the tn_attestation extension, simplifying the precompile registration. - Removed associated tests and the verify_precompile.go file to streamline the codebase. - Updated comments in precompile.go to reflect the removal of the verify_payload method. These changes enhance the maintainability of the tn_attestation extension by eliminating unused functionality.
WalkthroughAdds request_tx_id as the primary identifier across schema, stored procedures, processing, signing, submission, and tests; propagates it through prepared signing payloads and broadcaster/test harnesses; normalizes ECDSA V values; wires extension to App during leadership transitions; introduces test helpers and retrieval/listing tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Requester
participant Engine as Engine/Extension
participant DB as DB (actions)
participant Worker as Signer Worker
participant Signer as ECDSA Signer
Note over Requester,DB: request_attestation → returns request_tx_id
Requester->>Engine: call request_attestation(args)
Engine->>DB: request_attestation(...)
DB-->>Engine: (request_tx_id, attestation_hash)
Engine-->>Requester: (request_tx_id, attestation_hash)
Note over Engine,Worker: prepare includes request_tx_id
Engine->>Worker: prepareSigningWork(RequestTxID, Digest, ...)
Worker->>Signer: SignDigest(digest)
Signer-->>Worker: signature (65 bytes, V normalized)
Worker->>DB: sign_attestation(request_tx_id, signature)
alt DB success
DB-->>Worker: code=OK
else DB failure
DB-->>Worker: code!=OK
Worker-->>Worker: retry / log and return
end
Note over Requester,DB: retrieval by request_tx_id
Requester->>Engine: call get_signed_attestation(request_tx_id)
Engine->>DB: get_signed_attestation(request_tx_id)
DB-->>Engine: canonical_payload + signature
Engine-->>Requester: signed attestation payload
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
extensions/tn_attestation/tn_attestation.go (1)
174-180: Guard block may be nil in loggingblock can be nil; using block.Height will panic. Log conditionally.
- logger.Info("tn_attestation: processing queued attestations", - "count", len(hashes), - "block_height", block.Height) + if block != nil { + logger.Info("tn_attestation: processing queued attestations", + "count", len(hashes), + "block_height", block.Height) + } else { + logger.Info("tn_attestation: processing queued attestations", + "count", len(hashes)) + }
🧹 Nitpick comments (12)
internal/migrations/023-attestation-schema.sql (1)
40-43: Prefer a partial index for unsigned attestationsFor queries like WHERE attestation_hash = $1 AND signature IS NULL, a partial index on (attestation_hash) WHERE signature IS NULL is typically smaller and faster than a composite including the signature column.
Proposed addition (keep existing if you still need it for other patterns):
CREATE INDEX IF NOT EXISTS ix_att_unsigned_by_hash ON attestations(attestation_hash) WHERE signature IS NULL;tests/streams/attestation/test_helpers.go (2)
123-129: Surface action errors by default (or add a “CallActionOK” helper)CallAction doesn’t fail the test when res.Error is set. Either assert res.Error == nil here or add a separate helper (e.g., CallActionOK) that does, and use it in tests that expect success.
Example:
func (h *AttestationTestHelper) CallAction(actionName string, args []any, resultFn func(*common.Row) error) *common.CallResult { engineCtx := h.NewEngineContext() res, err := h.platform.Engine.Call(engineCtx, h.platform.DB, "", actionName, args, resultFn) require.NoError(h.t, err, "call action %s", actionName) - return res + require.Nil(h.t, res.Error, "action %s returned error: %v", actionName, res.Error) + return res }Or:
func (h *AttestationTestHelper) CallActionOK(...) *common.CallResult { /* assert res.Error == nil */ }
159-190: Nice helper flow; validate signature format if API normalizes VIf sign_attestation expects normalized ECDSA V, consider asserting the produced signature conforms (length 65 and V normalized) to catch regressions.
extensions/tn_attestation/processor.go (1)
198-203: *Make bytesCloneOrNil resilient to []byte (and avoid panics)Decode drivers can yield either []byte or *[]byte. Handle both and return a descriptive error for unexpected types.
-func bytesCloneOrNil(v any) []byte { - if v == nil { - return nil - } - return bytes.Clone(v.([]byte)) -} +func bytesCloneOrNil(v any) []byte { + if v == nil { + return nil + } + switch b := v.(type) { + case []byte: + return bytes.Clone(b) + case *[]byte: + if b == nil { + return nil + } + return bytes.Clone(*b) + default: + // Be defensive; upstream callers should handle this error if you choose to plumb it. + panic(fmt.Errorf("unexpected BYTEA type %T", v)) + } +}extensions/tn_attestation/signer.go (1)
63-72: V normalization to {27,28} is appropriate; add tests for variantsLogic correctly maps 0/1, 27/28, and 27+N forms to 27/28. Add unit tests covering each variant to prevent regressions. Optionally assert signature length is 65 before indexing.
extensions/tn_attestation/processor_test.go (1)
76-85: TestPrepareSigningWork: assert RequestTxID as wellSince the engine row now carries request_tx_id, assert it in ps to catch mapping regressions.
prepared, err := ext.prepareSigningWork(context.Background(), hex.EncodeToString(hash[:])) require.NoError(t, err) require.Len(t, prepared, 1) ps := prepared[0] +assert.Equal(t, "0xprepare", ps.RequestTxID) assert.Equal(t, hash[:], ps.Hash) assert.Equal(t, payload, ps.Payload) assert.Equal(t, int64(123), ps.CreatedHeight) assert.Len(t, ps.Signature, 65)Also applies to: 95-104
tests/streams/attestation/attestation_retrieval_test.go (5)
54-67: get_signed_attestation: happy path assertionsVerifying payload non-empty and larger than signature is a good proxy. Consider also asserting signature length=65 after splitting (as in PayloadStructure).
85-103: PayloadStructure: split canonical||signature correctlySolid structural checks. Optionally assert canonical starts with version/algo bytes if you want extra guardrails.
146-164: list_attestations: no-filter path validates shapeCounts and column count checks look good. You might also assert attestation_hash length (32) and requester non-empty for a sample row.
176-188: Pagination logic: basic coverage OKTwo pages of 3 results each is fine. Consider adding an explicit order_by override test (ASC) to ensure ordering behavior is respected.
190-193: MaxLimit test exercises high limitGiven server clamps to 5000, consider a separate test that passes >5000 and asserts that returned count never exceeds 5000 when dataset is large (future-proofing).
extensions/tn_attestation/worker.go (1)
71-86: Minor: don’t hard-fail when Requester is missing (log instead)submitSignature currently returns an error if Requester is empty, but Requester is only used for logging/status. Consider downgrading to a warning and proceed with signing to avoid blocking if DB ever yields NULLs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
extensions/tn_attestation/harness_integration_test.go(7 hunks)extensions/tn_attestation/integration_test.go(5 hunks)extensions/tn_attestation/precompile.go(1 hunks)extensions/tn_attestation/processor.go(3 hunks)extensions/tn_attestation/processor_test.go(4 hunks)extensions/tn_attestation/signer.go(1 hunks)extensions/tn_attestation/tn_attestation.go(3 hunks)extensions/tn_attestation/tn_attestation_test.go(1 hunks)extensions/tn_attestation/worker.go(3 hunks)internal/migrations/023-attestation-schema.sql(3 hunks)internal/migrations/024-attestation-actions.sql(4 hunks)tests/streams/attestation/attestation_request_test.go(3 hunks)tests/streams/attestation/attestation_retrieval_test.go(1 hunks)tests/streams/attestation/test_helpers.go(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-10T13:00:13.731Z
Learnt from: outerlook
PR: trufnetwork/node#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/023-attestation-schema.sqlinternal/migrations/024-attestation-actions.sql
📚 Learning: 2025-10-10T13:00:14.189Z
Learnt from: outerlook
PR: trufnetwork/node#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/023-attestation-schema.sqlinternal/migrations/024-attestation-actions.sql
🧬 Code graph analysis (3)
tests/streams/attestation/attestation_retrieval_test.go (4)
tests/streams/attestation/test_helpers.go (9)
NewTestAddresses(41-50)NewAttestationTestHelper(60-66)TestActionIDGet(23-23)AttestationTestHelper(53-57)SignatureLength(27-27)InvalidTxID(30-30)MinCanonicalLength(28-28)TestActionIDList(24-24)TestAddresses(34-38)tests/streams/utils/runner.go (1)
RunSchemaTest(37-116)internal/migrations/migration.go (1)
GetSeedScriptPaths(15-48)tests/streams/utils/utils.go (1)
GetTestOptionsWithCache(75-88)
tests/streams/attestation/attestation_request_test.go (2)
tests/streams/attestation/test_helpers.go (4)
NewTestAddresses(41-50)NewAttestationTestHelper(60-66)TestActionIDRequest(22-22)AttestationTestHelper(53-57)extensions/tn_utils/serialization.go (2)
EncodeActionArgs(45-76)EncodeQueryResultCanonical(145-184)
tests/streams/attestation/test_helpers.go (1)
extensions/tn_utils/serialization.go (1)
EncodeActionArgs(45-76)
⏰ 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: lint
- GitHub Check: acceptance-test
🔇 Additional comments (30)
extensions/tn_attestation/precompile.go (1)
16-16: Comment clarification LGTMShorter comment is accurate; keeping Cache nil here is correct since effects are leader-local and non-deterministic state is avoided.
extensions/tn_attestation/tn_attestation_test.go (3)
531-535: Good: App carries Engine and DB for end-block scanIncluding Engine and DB in App aligns with ext.setApp(app) usage and fallback scan path.
112-132: go.mod already specifies Go 1.24.1 (>=1.22). No changes required.
559-567: toInt helper is defined in this package’s test files; no changes needed.extensions/tn_attestation/tn_attestation.go (1)
105-105: setApp(app) addition LGTMEnsures extension has full App (Engine/DB) during leadership lifecycle and end-block processing.
Also applies to: 135-135, 165-165
extensions/tn_attestation/processor.go (4)
24-27: PreparedSignature.RequestTxID addition looks goodCarrying request_tx_id end-to-end is correct and unblocks the 2‑arg sign flow. No issues.
49-53: Query shape update aligns with schema; consider verifying index usageSelecting request_tx_id first and filtering by attestation_hash with signature IS NULL matches the new workflow. Ensure the migration’s index supports this filter and order (e.g., partial or composite index on (attestation_hash, signature IS NULL, created_height)).
If helpful, I can draft an EXPLAIN plan check for the exact SQL to confirm index usage in CI.
43-45: Confirm attestation_hash material (spec vs implementation)Comments say SQL computes hash from request parameters. Acceptance notes suggest including created_height and result in the canonical bytes used for hash. Today computeAttestationHash excludes those. Please confirm the intended spec and align SQL + Go accordingly; at minimum, document the chosen hash material to avoid future divergence.
124-131: Including RequestTxID in prepared payload is correctPlumbing request_tx_id here keeps the worker simple and matches the new 2‑arg action.
extensions/tn_attestation/harness_integration_test.go (5)
103-131: Validating request_attestation now returns (request_tx_id, attestation_hash)Good coverage of types and arity; asserting non-empty txID/hash prevents silent regressions.
200-207: PreparedSignature now includes RequestTxID in tests — goodAsserting RequestTxID continuity strengthens end-to-end guarantees.
315-337: Row mapping includes request_tx_id — OKSelect list/order matches mapping; the decode handles nullable signature/pubkey/height correctly.
374-381: Broadcaster decodes 2‑arg layout (request_tx_id, signature) correctlyShape checks and decoding paths look solid; aligns with worker payload.
Also applies to: 406-407
434-447: decodeStringArg helper: concise and safeHandles string/*string variants with require checks. LGTM.
extensions/tn_attestation/processor_test.go (2)
182-189: 2‑arg decoding check in submit path is correctVerifying request_tx_id and signature length = 65 is sufficient here.
223-237: decodeStringArg test helper is fineCovers string/*string; mirrors harness helper. LGTM.
tests/streams/attestation/attestation_retrieval_test.go (3)
17-52: Top-level retrieval tests are well-structuredSchema harness + per-scenario subtests provide clear coverage. Good use of require.
69-84: Error cases (not found / not yet signed) coveredClear expectation messages; aligns with acceptance criteria.
165-174: Filter by requester: good targeted assertionIndex 2 requester check matches declared return schema.
extensions/tn_attestation/integration_test.go (2)
62-73: Integration: requestTxID propagation validatedAsserting HashHex, CreatedHeight, and RequestTxID covers the new data flow. Good.
Also applies to: 112-118
143-148: Broadcaster captures request_tx_id and signature correctlyRobust argument decoding with clear errors; state capture is useful for assertions.
Also applies to: 159-187
extensions/tn_attestation/worker.go (2)
126-133: Switch to 2‑argument payload (request_tx_id, signature) is correctEncoding request_tx_id as string and trimming payload to two args aligns with the new action signature.
Also applies to: 139-140
267-289: Status worker: clearer success/failure logging and early returnLogging branches and early return on resolution look good. Behavior: one confirmed/failure result ends the loop; unresolved continues with backoff. LGTM.
Also applies to: 291-296
internal/migrations/024-attestation-actions.sql (4)
18-20: LGTM! Transaction ID properly captured as primary identifier.The function signature correctly returns both
request_tx_idandattestation_hash, withrequest_tx_idcaptured from@txid. This aligns with the PR objective of using transaction ID as the natural identifier.
104-106: LGTM! Simplified signature and consistent request_tx_id usage.The action signature correctly reduces to 2 required parameters as specified in the PR objectives. All validation, lookups, and error messages consistently use
request_tx_idas the identifier.Also applies to: 121-150
154-199: LGTM! Attestation retrieval implementation meets all requirements.The
get_signed_attestationaction correctly:
- Uses
request_tx_idas the lookup key (per PR objectives).- Validates both existence and signature presence.
- Returns the concatenated canonical payload + signature.
- Documents client-side verification responsibility clearly.
201-274: LGTM! Attestation listing implementation meets all requirements.The
list_attestationsaction correctly implements:
- Optional requester filtering (NULL returns all attestations).
- Pagination with reasonable limits (default 5000, max 5000).
- Sorting by
created_heightwithrequest_tx_idas secondary sort for deterministic ordering.- All required return columns as specified in PR objectives.
tests/streams/attestation/attestation_request_test.go (3)
21-34: LGTM! Helper-based test setup improves maintainability.The refactoring to use
AttestationTestHelperand centralized address management makes the tests more maintainable and reduces boilerplate code.
52-72: LGTM! Test correctly validates request_tx_id handling.The test properly:
- Captures both return values from
request_attestation.- Validates that
request_tx_idis non-empty and matches stored value.- Uses the helper pattern consistently.
97-140: LGTM! Row unpacking correctly handles request_tx_id field.The
attestationRowstruct andfetchAttestationRowfunction correctly:
- Add the
requestTxIDfield to the struct.- Include
request_tx_idin the SELECT statement as the first column.- Unpack row values with proper indexing (0=request_tx_id, 1=requester, etc.).
- Use defensive copying for byte slices to prevent aliasing issues.
Time Submission Status
|
…sion - Improved the fetchUnsignedAttestations method by adding type checks and error handling for various fields, ensuring robust data processing. - Introduced helper functions for cloning byte slices and extracting int64 values, enhancing code clarity and maintainability. - Updated the bytesCloneOrNil function to handle different input types more effectively. These changes improve the reliability and maintainability of the attestation processing workflow, ensuring accurate handling of unsigned attestations.
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)
extensions/tn_attestation/processor.go(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: acceptance-test
- Removed the bytesCloneOrNil function to simplify the codebase, retaining only the bytesClone function for cloning byte slices. - Ensured consistent handling of the Requester field in the prepareSigningWork method. These changes enhance code clarity and maintainability by reducing redundancy in byte handling functions.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
extensions/tn_attestation/processor.go (1)
55-96: Safer row decoding with explicit checks — nice; consider []byte fallback for request_tx_idGreat move to validated assertions and clear errors. To be driver-agnostic, accept []byte for request_tx_id as a fallback.
Apply this diff:
- txID, ok := row.Values[0].(string) - if !ok { - return fmt.Errorf("unexpected request_tx_id type %T", row.Values[0]) - } + var txID string + switch v := row.Values[0].(type) { + case string: + txID = v + case []byte: + txID = string(v) + default: + return fmt.Errorf("unexpected request_tx_id type %T", row.Values[0]) + }
🧹 Nitpick comments (3)
extensions/tn_attestation/processor.go (3)
43-54: Query shape is fine; ensure index supports filter/sortPlan should be index-friendly: WHERE attestation_hash = ? AND signature IS NULL ORDER BY created_height ASC. Confirm composite index order matches this (e.g., (attestation_hash, signature, created_height)) to avoid sorts.
154-162: Avoid re-signing identical digests (same hash across multiple requesters)Many records for one attestation_hash likely share the same canonical payload; cache signatures per digest to cut duplicate signing calls.
Apply this diff:
- prepared := make([]*PreparedSignature, 0, len(records)) + prepared := make([]*PreparedSignature, 0, len(records)) + // Cache signatures per digest to avoid repeated signing work. + seen := make(map[[32]byte][]byte) for _, rec := range records { payload, err := ParseCanonicalPayload(rec.canonical) if err != nil { return nil, fmt.Errorf("parse canonical payload: %w", err) } // Validate stored hash matches caller inputs; SQL computes it from request parameters. expectedHash := computeAttestationHash(payload) if !bytes.Equal(expectedHash[:], rec.hash) { return nil, fmt.Errorf("attestation hash mismatch: expected %x, db %x", expectedHash, rec.hash) } - digest := payload.SigningDigest() - signature, err := signer.SignDigest(digest[:]) - if err != nil { - return nil, fmt.Errorf("sign digest: %w", err) - } + digest := payload.SigningDigest() + var signature []byte + if cached, ok := seen[digest]; ok { + signature = cached + } else { + sig, err := signer.SignDigest(digest[:]) + if err != nil { + return nil, fmt.Errorf("sign digest: %w", err) + } + signature = sig + seen[digest] = bytesClone(signature) + }
260-271: Broaden extractInt64 to common driver variantsSome drivers return int32/*int32 for BIGINT in narrow ranges. Add these cases.
Apply this diff:
func extractInt64(value any) (int64, error) { switch v := value.(type) { case int64: return v, nil case *int64: if v == nil { return 0, fmt.Errorf("value is NULL") } return *v, nil + case int32: + return int64(v), nil + case *int32: + if v == nil { + return 0, fmt.Errorf("value is NULL") + } + return int64(*v), nil default: return 0, fmt.Errorf("unexpected type %T", value) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
extensions/tn_attestation/processor.go(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: acceptance-test
🔇 Additional comments (5)
extensions/tn_attestation/processor.go (5)
16-16: Adding requestTxID to attestationRecord looks goodKeeps the natural key close to the record.
24-34: Propagating RequestTxID via PreparedSignature is correctThis surfaces the natural identifier to callers for logging/auditing.
142-146: Confirm attestation-hash algorithm matches DB; consider hashing canonical bytesIf the contract is “hash over result_canonical,” compute directly over rec.canonical to eliminate drift between encoders.
Apply this diff if appropriate:
- expectedHash := computeAttestationHash(payload) - if !bytes.Equal(expectedHash[:], rec.hash) { - return nil, fmt.Errorf("attestation hash mismatch: expected %x, db %x", expectedHash, rec.hash) - } + expected := sha256.Sum256(rec.canonical) + if !bytes.Equal(expected[:], rec.hash) { + return nil, fmt.Errorf("attestation hash mismatch: expected %x, db %x", expected, rec.hash) + }
229-241: cloneBytesValue: solid defensive helperClear errors on NULL/unexpected types; matches usage for NOT NULL bytea columns.
243-258: cloneBytesValueAllowNil: good nullability handlingAppropriate for nullable requester; keeps surprises surfaced via errors for unexpected types.
2 participants
Description
Enables users to query and retrieve attestations using transaction IDs, introducing new retrieval endpoints and simplifying the attestation workflow.
New User-Facing Features
get_signed_attestation($request_tx_id): Retrieve a specific signed attestation by its transaction IDlist_attestations($requester, $limit, $offset, $order_by): Query attestations with pagination and filteringrequest_attestationnow returns bothrequest_tx_idandattestation_hashfor flexible retrievalSimplified API
sign_attestationfrom 4 required parameters to 2 (request_tx_id,signature)Technical Improvements
request_tx_idas primary key for simpler queriesRelated Problem
How Has This Been Tested?
New comprehensive test suite covering:
Unit test added for DB handle update bug fix.
Summary by CodeRabbit