Conversation
WalkthroughAdds end-to-end attestation support: new exported AttestationAction with request/get/list methods, attestation types and validation, argument-encoding utilities, client integration and exports, examples and README docs, integration tests, and safer test-runner docker invocation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Application
participant Client as TN Client
participant Action as AttestationAction
participant Remote as Remote Network
App->>Client: loadAttestationAction()
Client->>Action: construct AttestationAction
Action-->>App: AttestationAction instance
rect rgba(100,150,230,0.06)
App->>Action: requestAttestation(input)
Action->>Action: validate + encode args
Action->>Remote: execute(request_attestation)
Remote-->>Action: { tx_hash }
Action-->>App: { requestTxId }
end
Note over App: wait for tx confirmation (external)
rect rgba(120,200,120,0.06)
App->>Action: getSignedAttestation(requestTxId)
Action->>Remote: view(get_signed_attestation)
Remote-->>Action: Either<err, base64_payload>
Action->>Action: decode payload -> Uint8Array
Action-->>App: { payload }
end
rect rgba(230,190,100,0.05)
App->>Action: listAttestations(filters)
Action->>Action: validate list input
Action->>Remote: view(list_attestations)
Remote-->>Action: rows[]
Action->>Action: parse rows -> AttestationMetadata[]
Action-->>App: AttestationMetadata[]
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ 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 |
Time Submission Status
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/contracts-api/action.ts (1)
690-699: Inconsistent compose metadata (value vs ref) – disable won’t find what allow stored
- allowComposeStream stores only streamId in value.
- getAllowedComposeStreams expects value in format "streamId:dataProvider".
- disableComposeStream filters by wallet.toString() (StreamLocator is a POJO ⇒ "[object Object]") against $ref, not the stored value.
Result: disable call can’t locate the row; getter parsing breaks unless value actually contains both parts. Store and query a consistent composite key and use it in all three methods.
Apply this fix:
- public async allowComposeStream( - stream: StreamLocator, - wallet: StreamLocator, - ): Promise<Types.GenericResponse<Types.TxReceipt>> { - return await this.setMetadata( - stream, - MetadataKey.AllowComposeStreamKey, - wallet.streamId.getId(), - ); - } + public async allowComposeStream( + stream: StreamLocator, + child: StreamLocator, + ): Promise<Types.GenericResponse<Types.TxReceipt>> { + // Store "streamId:dataProvider" to match getAllowedComposeStreams parsing. + const val = `${child.streamId.getId()}:${child.dataProvider.getAddress()}`; + return await this.setMetadata( + stream, + MetadataKey.AllowComposeStreamKey, + val, + ); + } @@ - public async disableComposeStream( - stream: StreamLocator, - wallet: StreamLocator, - ): Promise<Types.GenericResponse<Types.TxReceipt>> { - const result = await this.getMetadata( - stream, - MetadataKey.AllowComposeStreamKey, - wallet.toString(), - ); + public async disableComposeStream( + stream: StreamLocator, + child: StreamLocator, + ): Promise<Types.GenericResponse<Types.TxReceipt>> { + // Filter by the same composite value we stored. + const ref = `${child.streamId.getId()}:${child.dataProvider.getAddress()}`; + const result = await this.getMetadata( + stream, + MetadataKey.AllowComposeStreamKey, + ref, + );Optionally, backfill/normalize existing rows to the composite format to avoid mixed historical data. If insert_metadata populates value_ref differently, mirror the composite in ref as well during insert.
Also applies to: 704-723, 754-770
🧹 Nitpick comments (22)
tests/integration/trufnetwork.setup.ts (2)
35-35: Avoid :latest for reproducible CI:latest drifts and can break tests. Pin to a tag or digest (e.g., ghcr.io/trufnetwork/node@sha256:...). Consider requiring TN_DB_IMAGE to be set in CI and fail fast if absent.
229-244: Network deletion check can miss matchesdocker network ls --filter name=... returns partial matches and may print multiple lines; comparing stdout.trim() === NETWORK_NAME can fail. Prefer docker network inspect to check existence or split lines and look for exact equality.
-const { stdout } = await runDockerCommand(["network", "ls", "--filter", `name=${NETWORK_NAME}`, "--format", "{{.Name}}"]); -if (stdout.trim() === NETWORK_NAME) { +const { stdout } = await runDockerCommand(["network", "ls", "--filter", `name=^${NETWORK_NAME}$`, "--format", "{{.Name}}"]); +if (stdout.split("\n").map(s => s.trim()).includes(NETWORK_NAME)) {src/util/AttestationEncoding.ts (3)
22-22: Tighten types for argsargs can be typed to kwil-js’s ValueType to improve safety.
-export function encodeActionArgs(args: any[]): Uint8Array { +export function encodeActionArgs(args: Types.ValueType[]): Uint8Array {
75-80: Optional: prefer DataView for LE read/writeManual bit ops are fine; DataView reduces edge cases and clarifies intent.
-function writeUint32LE(buffer: Uint8Array, value: number, offset: number): void { - buffer[offset] = value & 0xff; - buffer[offset + 1] = (value >> 8) & 0xff; - buffer[offset + 2] = (value >> 16) & 0xff; - buffer[offset + 3] = (value >> 24) & 0xff; -} +function writeUint32LE(buffer: Uint8Array, value: number, offset: number): void { + new DataView(buffer.buffer, buffer.byteOffset, buffer.byteLength).setUint32(offset, value >>> 0, true); +} @@ -export function readUint32LE(buffer: Uint8Array, offset: number): number { - return ( - buffer[offset] | - (buffer[offset + 1] << 8) | - (buffer[offset + 2] << 16) | - (buffer[offset + 3] << 24) - ) >>> 0; // Convert to unsigned 32-bit integer -} +export function readUint32LE(buffer: Uint8Array, offset: number): number { + return new DataView(buffer.buffer, buffer.byteOffset, buffer.byteLength).getUint32(offset, true); +}Also applies to: 89-96
98-210: Export surface: readUint32LE likely internalIf readUint32LE is exported only for tests, mark it @internal or keep it unexported and test via black-box helpers to avoid bloating the public API.
src/client/client.ts (1)
14-14: Attestation loader is consistent with other loadersImport + loadAttestationAction look good and align with BaseTNClient patterns.
Optional: for API symmetry, consider AttestationAction.fromClient(...) similar to RoleManagement.fromClient(...).
Also applies to: 193-203
examples/attestation/README.md (3)
26-30: Avoid author-specific absolute pathsUse a project-relative path to improve portability.
- cd /home/micbun/trufnetwork/sdk-js + cd path/to/sdk-js
1-267: Fix markdown hard tabsmarkdownlint flags multiple hard tabs (MD010). Replace tabs with spaces in code blocks and lists to satisfy CI and keep rendering consistent.
155-195: Ethers v5 vs v6 utils API differencesThe snippet uses ethers.utils.*. If the repo standardizes on ethers v6 in other places, adapt to v6 (e.g., import { keccak256, recoverAddress, hexlify } from "ethers"). Verify version and align examples accordingly.
README.md (3)
239-256: Prefer exponential backoff with jitter for pollingFixed 2s polling can cause synchronized load and flakiness. Use capped exponential backoff with jitter and a clear max timeout. This improves resilience and reduces leader pressure.
- for (let i = 0; i < 15; i++) { + const started = Date.now(); + const maxMs = 60_000; // 60s cap + for (let i = 0; Date.now() - started < maxMs; i++) { try { // ... if (signed.payload.length > 65) break; // Got signature } catch (e) { - await new Promise(resolve => setTimeout(resolve, 2000)); + const base = Math.min(2000 * 2 ** i, 10_000); + const jitter = Math.floor(Math.random() * 500); + await new Promise(res => setTimeout(res, base + jitter)); } }
261-264: Provide browser‑safe address‑to‑bytes snippetBuffer isn’t available in browsers. Add an alternative snippet using a hex decoder for BrowserTNClient contexts.
-const myAddress = client.address().getAddress(); -const myAddressBytes = new Uint8Array(Buffer.from(myAddress.slice(2), "hex")); +const myAddress = client.address().getAddress(); +// Node: +const myAddressBytes = + typeof Buffer !== "undefined" + ? new Uint8Array(Buffer.from(myAddress.slice(2), "hex")) + // Browser: + : (() => { + const h = myAddress.slice(2); + const out = new Uint8Array(h.length / 2); + for (let i = 0; i < h.length; i += 2) out[i / 2] = parseInt(h.substr(i, 2), 16); + return out; + })();
279-291: Clarify payload spec: fixed‑length vs length‑prefixed fields“Data provider (20 bytes, length‑prefixed)” and “Stream ID (32 bytes, length‑prefixed)” are contradictory. If these are fixed 20/32‑byte values, drop “length‑prefixed”; if they’re variable with a length prefix, avoid stating fixed sizes. Align docs with the canonical encoding used in the implementation.
examples/attestation/index.ts (2)
91-118: Replace fixed‑interval polling with capped exponential backoffImproves success rate and reduces load under variable block times.
-const maxAttempts = 15; // 30 seconds max (2 seconds per attempt) -for (let i = 0; i < maxAttempts; i++) { +const started = Date.now(); +const maxMs = 60_000; +for (let i = 0; Date.now() - started < maxMs; i++) { try { // ... - } catch (err) { + } catch (err) { // Not signed yet, continue polling } - // Progress indicator - process.stdout.write(`Attempt ${i + 1}/${maxAttempts}...`); - if (i < maxAttempts - 1) { - process.stdout.write("\r"); - await new Promise((resolve) => setTimeout(resolve, 2000)); - } else { - process.stdout.write("\n"); - } + const base = Math.min(2000 * 2 ** i, 10_000); + const jitter = Math.floor(Math.random() * 500); + await new Promise((r) => setTimeout(r, base + jitter)); }
71-78: Gate verbose logs behind an env flagThe example is very chatty; gate DEBUG output to keep it readable by default.
- console.log(`DEBUG: Full requestAttestation result:`, JSON.stringify(requestResult, null, 2)); + if (process.env.DEBUG) console.log(`DEBUG: Full requestAttestation result:`, JSON.stringify(requestResult, null, 2)); - console.log(`DEBUG: TX confirmed at height ${txResult.height}, hash: ${txResult.tx_hash}`); + if (process.env.DEBUG) console.log(`DEBUG: TX confirmed at height ${txResult.height}, hash: ${txResult.tx_hash}`); - console.log(`DEBUG: TX result - code: ${(txResult as any).tx_result?.code}, log: ${(txResult as any).tx_result?.log}`); + if (process.env.DEBUG) console.log(`DEBUG: TX result - code: ${(txResult as any).tx_result?.code}, log: ${(txResult as any).tx_result?.log}`); - console.log(`DEBUG Attempt ${i + 1}: getSignedAttestation returned:`, {/* ... */}); + if (process.env.DEBUG) console.log(`DEBUG Attempt ${i + 1}: getSignedAttestation returned:`, {/* ... */}); - console.log(`First 64 bytes (hex): ${Buffer.from(signedAttestation.payload.slice(0, 64)).toString("hex")}`); + if (process.env.DEBUG) console.log(`First 64 bytes (hex): ${Buffer.from(signedAttestation.payload.slice(0, 64)).toString("hex")}`); - console.log(` Attestation hash: ${Buffer.from(att.attestationHash).toString("hex")}`); + if (process.env.DEBUG) console.log(` Attestation hash: ${Buffer.from(att.attestationHash).toString("hex")}`);Also applies to: 90-106, 125-129, 146-153
tests/integration/attestation.test.ts (3)
84-115: Use capped exponential backoff with jitter in pollingReduces flakiness under slow blocks or CI load; keeps total wait bounded.
-const maxAttempts = 15; // 30 seconds max -for (let i = 0; i < maxAttempts; i++) { +const started = Date.now(); +const maxMs = 60_000; +for (let i = 0; Date.now() - started < maxMs; i++) { try { // ... } catch (e: any) { // Not signed yet, continue polling console.log(`Attempt ${i + 1}...`); } - // Wait 2 seconds before next attempt - await new Promise((resolve) => setTimeout(resolve, 2000)); + const base = Math.min(2000 * 2 ** i, 10_000); + const jitter = Math.floor(Math.random() * 500); + await new Promise((resolve) => setTimeout(resolve, base + jitter)); }
71-78: Tame test log noiseWrap DEBUG logs behind an env flag to keep CI output lean.
- console.log(`DEBUG: Full requestAttestation result:`, JSON.stringify(requestResult, null, 2)); + if (process.env.DEBUG) console.log(`DEBUG: Full requestAttestation result:`, JSON.stringify(requestResult, null, 2)); - console.log(`DEBUG: TX confirmed at height ${txResult.height}, hash: ${txResult.tx_hash}`); + if (process.env.DEBUG) console.log(`DEBUG: TX confirmed at height ${txResult.height}, hash: ${txResult.tx_hash}`); - console.log(`DEBUG: TX result - code: ${(txResult as any).tx_result?.code}, log: ${(txResult as any).tx_result?.log}`); + if (process.env.DEBUG) console.log(`DEBUG: TX result - code: ${(txResult as any).tx_result?.code}, log: ${(txResult as any).tx_result?.log}`); - console.log(`DEBUG Attempt ${i + 1}: getSignedAttestation returned:`, {/* ... */}); + if (process.env.DEBUG) console.log(`DEBUG Attempt ${i + 1}: getSignedAttestation returned:`, {/* ... */});Also applies to: 90-106
155-161: Avoid unbounded result sets in testslistAttestations({}) uses the default (5000). Specify a small limit for faster, deterministic tests.
-const attestations = await attestationAction.listAttestations({}); +const attestations = await attestationAction.listAttestations({ limit: 25 });src/contracts-api/attestationAction.ts (3)
236-246: Prefer Either.throw() and lower the default limitUse throw() for clarity and reduce default limit to avoid large responses by default. Keep 5000 as an explicit opt‑in.
- const limit = input.limit ?? 5000; + const limit = input.limit ?? 100; @@ - const result = await this.call<any[]>('list_attestations', params); - - // Check for errors - if (result.isLeft()) { - throw new Error( - `Failed to list attestations: HTTP status ${result.value}` - ); - } - - // Extract the right value from Either - const rows = result.value as unknown as any[]; + const result = await this.call<any[]>('list_attestations', params); + const rows = result.throw();Also applies to: 249-260
317-324: Add length checks for decoded BYTEA fieldsCatch malformed rows early and surface clear errors.
return { - requestTxId, - attestationHash, - requester, + requestTxId, + attestationHash: (() => { + if (attestationHash.length !== 32) { + throw new Error(`Row ${idx}: attestation_hash must be 32 bytes, got ${attestationHash.length}`); + } + return attestationHash; + })(), + requester: (() => { + if (requester.length !== 20) { + throw new Error(`Row ${idx}: requester must be 20 bytes, got ${requester.length}`); + } + return requester; + })(), createdHeight, signedHeight, encryptSig, };
340-359: Optionally accept hex‑encoded BYTEA stringsSome backends return “\x…” or “0x…” hex. Supporting these formats improves robustness across environments.
if (typeof value === 'string') { try { + // Hex formats: \x... (PG) or 0x... (generic) + if (/^\\x[0-9a-fA-F]+$/.test(value)) { + const hex = value.slice(2); + const out = new Uint8Array(hex.length / 2); + for (let i = 0; i < hex.length; i += 2) out[i / 2] = parseInt(hex.substr(i, 2), 16); + return out; + } + if (/^0x[0-9a-fA-F]+$/.test(value)) { + const hex = value.slice(2); + const out = new Uint8Array(hex.length / 2); + for (let i = 0; i < hex.length; i += 2) out[i / 2] = parseInt(hex.substr(i, 2), 16); + return out; + } // Node.js environment if (typeof Buffer !== 'undefined') { return new Uint8Array(Buffer.from(value, 'base64')); } else {src/types/attestation.ts (2)
202-221: Simplify orderBy validation (case‑insensitive) and avoid duplicatesNormalize to lowercase and check against a canonical list.
-const VALID_ORDER_BY_VALUES = [ - 'created_height asc', - 'created_height desc', - 'signed_height asc', - 'signed_height desc', - 'created_height ASC', - 'created_height DESC', - 'signed_height ASC', - 'signed_height DESC', -]; +const VALID_ORDER_BY_VALUES = [ + 'created_height asc', + 'created_height desc', + 'signed_height asc', + 'signed_height desc', +]; @@ -export function isValidOrderBy(orderBy: string): boolean { - return VALID_ORDER_BY_VALUES.includes(orderBy); -} +export function isValidOrderBy(orderBy: string): boolean { + return VALID_ORDER_BY_VALUES.includes(orderBy.toLowerCase()); +}
160-197: validateAttestationRequest: solid; consider adding stricter streamId check (optional)If stream IDs have a fixed alphabet/prefix, validate format (not just length) to fail fast on typos.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
README.md(2 hunks)examples/attestation/README.md(1 hunks)examples/attestation/index.ts(1 hunks)src/client/client.ts(2 hunks)src/contracts-api/action.ts(2 hunks)src/contracts-api/attestationAction.ts(1 hunks)src/index.common.ts(1 hunks)src/internal.ts(2 hunks)src/types/attestation.ts(1 hunks)src/util/AttestationEncoding.ts(1 hunks)tests/integration/attestation.test.ts(1 hunks)tests/integration/trufnetwork.setup.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/contracts-api/attestationAction.ts (3)
src/contracts-api/action.ts (1)
Action(72-1080)src/types/attestation.ts (8)
RequestAttestationInput(12-48)RequestAttestationResult(53-59)validateAttestationRequest(160-197)GetSignedAttestationInput(64-69)SignedAttestationResult(74-83)ListAttestationsInput(88-117)AttestationMetadata(122-152)validateListAttestationsInput(229-259)src/util/AttestationEncoding.ts (1)
encodeActionArgs(22-65)
src/client/client.ts (3)
src/contracts-api/attestationAction.ts (1)
AttestationAction(53-269)src/index.common.ts (1)
AttestationAction(31-31)src/internal.ts (1)
AttestationAction(15-15)
src/types/attestation.ts (2)
src/index.common.ts (6)
RequestAttestationInput(33-33)RequestAttestationResult(34-34)GetSignedAttestationInput(35-35)SignedAttestationResult(36-36)ListAttestationsInput(37-37)AttestationMetadata(38-38)src/internal.ts (6)
RequestAttestationInput(61-61)RequestAttestationResult(62-62)GetSignedAttestationInput(63-63)SignedAttestationResult(64-64)ListAttestationsInput(65-65)AttestationMetadata(66-66)
tests/integration/attestation.test.ts (2)
tests/integration/trufnetwork.setup.ts (1)
setupTrufNetwork(251-297)tests/integration/utils.ts (1)
testWithDefaultWallet(9-11)
🪛 markdownlint-cli2 (0.18.1)
examples/attestation/README.md
52-52: Hard tabs
Column: 1
(MD010, no-hard-tabs)
53-53: Hard tabs
Column: 1
(MD010, no-hard-tabs)
54-54: Hard tabs
Column: 1
(MD010, no-hard-tabs)
55-55: Hard tabs
Column: 1
(MD010, no-hard-tabs)
56-56: Hard tabs
Column: 1
(MD010, no-hard-tabs)
57-57: Hard tabs
Column: 1
(MD010, no-hard-tabs)
58-58: Hard tabs
Column: 1
(MD010, no-hard-tabs)
91-91: Hard tabs
Column: 1
(MD010, no-hard-tabs)
92-92: Hard tabs
Column: 1
(MD010, no-hard-tabs)
93-93: Hard tabs
Column: 1
(MD010, no-hard-tabs)
94-94: Hard tabs
Column: 1
(MD010, no-hard-tabs)
102-102: Hard tabs
Column: 1
(MD010, no-hard-tabs)
128-128: Hard tabs
Column: 1
(MD010, no-hard-tabs)
129-129: Hard tabs
Column: 1
(MD010, no-hard-tabs)
130-130: Hard tabs
Column: 1
(MD010, no-hard-tabs)
152-152: Hard tabs
Column: 1
(MD010, no-hard-tabs)
153-153: Hard tabs
Column: 1
(MD010, no-hard-tabs)
154-154: Hard tabs
Column: 1
(MD010, no-hard-tabs)
155-155: Hard tabs
Column: 1
(MD010, no-hard-tabs)
156-156: Hard tabs
Column: 1
(MD010, no-hard-tabs)
157-157: Hard tabs
Column: 1
(MD010, no-hard-tabs)
158-158: Hard tabs
Column: 1
(MD010, no-hard-tabs)
159-159: Hard tabs
Column: 1
(MD010, no-hard-tabs)
160-160: Hard tabs
Column: 1
(MD010, no-hard-tabs)
161-161: Hard tabs
Column: 1
(MD010, no-hard-tabs)
162-162: Hard tabs
Column: 1
(MD010, no-hard-tabs)
163-163: Hard tabs
Column: 1
(MD010, no-hard-tabs)
168-168: Hard tabs
Column: 1
(MD010, no-hard-tabs)
169-169: Hard tabs
Column: 1
(MD010, no-hard-tabs)
174-174: Hard tabs
Column: 1
(MD010, no-hard-tabs)
185-185: Hard tabs
Column: 1
(MD010, no-hard-tabs)
186-186: Hard tabs
Column: 1
(MD010, no-hard-tabs)
191-191: Hard tabs
Column: 1
(MD010, no-hard-tabs)
192-192: Hard tabs
Column: 1
(MD010, no-hard-tabs)
216-216: Hard tabs
Column: 1
(MD010, no-hard-tabs)
217-217: Hard tabs
Column: 1
(MD010, no-hard-tabs)
218-218: Hard tabs
Column: 1
(MD010, no-hard-tabs)
219-219: Hard tabs
Column: 1
(MD010, no-hard-tabs)
220-220: Hard tabs
Column: 1
(MD010, no-hard-tabs)
221-221: Hard tabs
Column: 1
(MD010, no-hard-tabs)
222-222: Hard tabs
Column: 1
(MD010, no-hard-tabs)
223-223: Hard tabs
Column: 1
(MD010, no-hard-tabs)
224-224: Hard tabs
Column: 1
(MD010, no-hard-tabs)
225-225: Hard tabs
Column: 1
(MD010, no-hard-tabs)
226-226: Hard tabs
Column: 1
(MD010, no-hard-tabs)
227-227: Hard tabs
Column: 1
(MD010, no-hard-tabs)
228-228: Hard tabs
Column: 1
(MD010, no-hard-tabs)
245-245: Hard tabs
Column: 1
(MD010, no-hard-tabs)
246-246: Hard tabs
Column: 1
(MD010, no-hard-tabs)
247-247: Hard tabs
Column: 1
(MD010, no-hard-tabs)
248-248: Hard tabs
Column: 1
(MD010, no-hard-tabs)
249-249: Hard tabs
Column: 1
(MD010, no-hard-tabs)
250-250: Hard tabs
Column: 1
(MD010, no-hard-tabs)
251-251: Hard tabs
Column: 1
(MD010, no-hard-tabs)
252-252: Hard tabs
Column: 1
(MD010, no-hard-tabs)
267-267: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🔇 Additional comments (8)
tests/integration/trufnetwork.setup.ts (1)
41-41: Entrypoint/path compatibilityYou override entrypoint to /app/kwild. Confirm this path exists for all intended images/tags; otherwise the container fails immediately. Consider making ENTRYPOINT override configurable or omit it when the image already sets it correctly.
Also applies to: 268-269
src/contracts-api/action.ts (2)
90-109: JSDoc improvements look goodClearer contract and return docs for executeWithNamedParams.
110-121: New executeWithActionBody helper is fineSignature and pass-through behavior to kwilClient.execute are consistent.
src/internal.ts (1)
15-15: New exports wired correctlyAttestationAction and attestation types are cleanly re-exported; no circular hints here.
Also applies to: 59-67
src/index.common.ts (1)
30-40: Public surface extension looks goodAttestationAction and types are exposed coherently with the rest of the API.
README.md (1)
429-431: Quick reference additions look goodAPI signatures match the public surface in code and tests.
src/contracts-api/attestationAction.ts (1)
85-118: requestAttestation flow LGTMValidation, encoding, and tx_hash handling are sound.
src/types/attestation.ts (1)
261-476: Inline tests are helpful and focusedGood coverage of validation paths and orderBy checks.
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)
tests/integration/trufnetwork.setup.ts (1)
88-102: Add.toString()for Buffer-to-string conversion in error handling.When
execFilethrows an error, theerror.stdoutanderror.stderrproperties may be Buffers rather than strings. Directly returning or concatenating them can cause type issues.Apply this diff to safely convert Buffers to strings:
async function runDockerCommand(args: string[], check = false) { // Use execFile with argument array to prevent shell injection try { const { stdout, stderr } = await execFile("docker", args); if (stderr) { console.debug(stderr); } return { stdout, stderr }; } catch (error: any) { if (check) { - throw new Error(error.stderr || error.message); + throw new Error(error.stderr?.toString() || error.message); } - return { stdout: error.stdout || "", stderr: error.stderr || error.message }; + return { + stdout: error.stdout?.toString() || "", + stderr: error.stderr?.toString() || error.message + }; } }
🧹 Nitpick comments (2)
src/contracts-api/attestationAction.ts (1)
153-161: Consider adding hex format validation for requestTxId.The normalization logic correctly strips the optional
0xprefix and lowercases the input, but it doesn't validate that the result is exactly 64 hexadecimal characters. This could allow malformed transaction IDs to be passed to the backend.Add validation after normalization:
// Strip optional "0x" prefix and lowercase for normalization const normalizedRequestTxId = trimmed.startsWith('0x') ? trimmed.slice(2).toLowerCase() : trimmed.toLowerCase(); + +// Validate hex format (64 characters) +if (!/^[0-9a-f]{64}$/.test(normalizedRequestTxId)) { + throw new Error('requestTxId must be 64 hex characters (optional 0x prefix)'); +}src/types/attestation.ts (1)
202-221: Consider case-insensitive validation to simplify the whitelist.The current approach explicitly lists both uppercase and lowercase variants, which works correctly but is redundant. A case-insensitive comparison would be more concise.
const VALID_ORDER_BY_VALUES = [ 'created_height asc', 'created_height desc', 'signed_height asc', 'signed_height desc', - 'created_height ASC', - 'created_height DESC', - 'signed_height ASC', - 'signed_height DESC', ]; export function isValidOrderBy(orderBy: string): boolean { - return VALID_ORDER_BY_VALUES.includes(orderBy); + return VALID_ORDER_BY_VALUES.includes(orderBy.toLowerCase()); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/contracts-api/attestationAction.ts(1 hunks)src/types/attestation.ts(1 hunks)tests/integration/trufnetwork.setup.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/types/attestation.ts (2)
src/index.common.ts (6)
RequestAttestationInput(33-33)RequestAttestationResult(34-34)GetSignedAttestationInput(35-35)SignedAttestationResult(36-36)ListAttestationsInput(37-37)AttestationMetadata(38-38)src/internal.ts (6)
RequestAttestationInput(61-61)RequestAttestationResult(62-62)GetSignedAttestationInput(63-63)SignedAttestationResult(64-64)ListAttestationsInput(65-65)AttestationMetadata(66-66)
src/contracts-api/attestationAction.ts (3)
src/contracts-api/action.ts (1)
Action(72-1080)src/types/attestation.ts (8)
RequestAttestationInput(12-48)RequestAttestationResult(53-59)validateAttestationRequest(160-197)GetSignedAttestationInput(64-69)SignedAttestationResult(74-83)ListAttestationsInput(88-117)AttestationMetadata(122-152)validateListAttestationsInput(229-259)src/util/AttestationEncoding.ts (1)
encodeActionArgs(22-65)
⏰ 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: test
🔇 Additional comments (15)
tests/integration/trufnetwork.setup.ts (4)
2-2: LGTM! Correct import and promisification.The addition of
execFileimport and its promisification correctly sets up the safer command execution mechanism.Also applies to: 8-8
51-54: Good practice to validate environment input.The validation call correctly prevents shell injection by checking the image name before use. Once the regex in
validateDockerImageis fixed (see previous comment), this will provide robust protection.
58-58: LGTM! Correct usage of validated image.The container spec correctly uses the validated
TN_DB_IMAGEvariable, enabling flexible image selection while maintaining security.
287-287: LGTM! Improved logging for debugging.Including the image name in the log message provides useful visibility into which Docker image is being used during test setup.
src/contracts-api/attestationAction.ts (7)
1-53: LGTM!The imports are well-organized, and the class documentation provides clear examples demonstrating the attestation workflow.
85-118: LGTM!The method correctly validates input, encodes arguments, executes the attestation request, and returns the transaction hash as requestTxId.
164-208: LGTM!The method correctly calls the view action with the normalized ID, handles Either results properly, validates the response structure, and includes cross-platform base64 decoding for both Node.js and browser environments.
236-274: LGTM!The method correctly validates input, applies sensible defaults, handles Either results, and maps rows to AttestationMetadata using the helper function.
288-331: LGTM!The parser robustly handles both array and object row formats, provides clear error messages with row context, and correctly processes nullable fields.
336-371: LGTM!The helper function provides robust decoding with cross-platform support, comprehensive type checking, and informative error messages including row and column context.
374-440: LGTM!The inline tests provide good coverage of helper functions, testing both happy paths and error conditions for base64 decoding and row parsing in both array and object formats.
src/types/attestation.ts (4)
1-152: LGTM!The interface definitions are comprehensive and well-documented, providing clear guidance on expected formats, constraints, and examples for each field.
160-197: LGTM!The validation function thoroughly checks all input parameters with appropriate error messages. The use of snake_case in error messages (e.g.,
data_provider,stream_id) appears intentional for consistency with backend parameter names.
251-258: LGTM!The orderBy validation correctly uses the isValidOrderBy function and provides a helpful error message listing the valid options.
262-476: Good test coverage overall.The inline tests comprehensively cover validation functions with both valid inputs and various error conditions. Note that the requester validation test issue has been flagged separately in the previous comment.
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 (2)
src/types/attestation.ts(1 hunks)tests/integration/trufnetwork.setup.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/integration/trufnetwork.setup.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/types/attestation.ts (2)
src/index.common.ts (6)
RequestAttestationInput(33-33)RequestAttestationResult(34-34)GetSignedAttestationInput(35-35)SignedAttestationResult(36-36)ListAttestationsInput(37-37)AttestationMetadata(38-38)src/internal.ts (6)
RequestAttestationInput(61-61)RequestAttestationResult(62-62)GetSignedAttestationInput(63-63)SignedAttestationResult(64-64)ListAttestationsInput(65-65)AttestationMetadata(66-66)
⏰ 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: test
🔇 Additional comments (4)
src/types/attestation.ts (4)
1-152: LGTM: Well-structured type definitions.The attestation type interfaces are comprehensive and well-documented with clear constraints and examples. The use of
any[]for action arguments (line 35) provides the necessary flexibility for different action signatures.
160-197: LGTM: Comprehensive validation logic.The validation function properly validates all input fields with appropriate error messages. The checks for dataProvider format (0x-prefixed 40 hex chars), streamId length (32 chars), and other constraints are correct and thorough.
229-259: LGTM: Validation logic is correct.The validation properly checks all optional fields with appropriate constraints. The requester validation at lines 244-249 correctly requires exactly 20 bytes, which aligns with Ethereum address requirements.
Note: The error message at line 255 shows only the first 4 values using
.slice(0, 4)to avoid displaying the redundant uppercase duplicates. If the orderBy refactor (suggested in previous comment) is applied, this slice operation can be removed.
261-484: LGTM: Comprehensive test coverage, past review comment addressed.The unit tests provide excellent coverage of all validation paths:
- validateAttestationRequest: 10 test cases covering all validation branches
- isValidOrderBy: 3 test cases for valid/invalid inputs
- validateListAttestationsInput: 9 test cases covering all constraints
The past review comment has been properly addressed—lines 460-474 now include tests for both requester lengths that are too long (21 bytes) and too short (19 bytes), both correctly expecting "requester must be exactly 20 bytes".
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/types/attestation.ts (2)
35-35: Consider documenting the type safety trade-off for action arguments.The
any[]type forargsis necessary given that action signatures vary, but it means type safety for arguments is deferred to runtime. Consider adding a brief comment explaining this trade-off or referencing where argument validation occurs.
253-253: Remove unnecessary.slice(0, 4)call.Since
VALID_ORDER_BY_VALUEScontains exactly 4 elements (lines 203-206), the slice is redundant. Moreover, if more values are added in the future, the error message should show all valid options, not just the first 4.Apply this diff:
- `Invalid orderBy value. Must be one of: ${VALID_ORDER_BY_VALUES.slice(0, 4).join(', ')}` + `Invalid orderBy value. Must be one of: ${VALID_ORDER_BY_VALUES.join(', ')}`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/types/attestation.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/types/attestation.ts (2)
src/index.common.ts (6)
RequestAttestationInput(33-33)RequestAttestationResult(34-34)GetSignedAttestationInput(35-35)SignedAttestationResult(36-36)ListAttestationsInput(37-37)AttestationMetadata(38-38)src/internal.ts (6)
RequestAttestationInput(61-61)RequestAttestationResult(62-62)GetSignedAttestationInput(63-63)SignedAttestationResult(64-64)ListAttestationsInput(65-65)AttestationMetadata(66-66)
🔇 Additional comments (4)
src/types/attestation.ts (4)
160-197: LGTM! Comprehensive validation with one minor note.The validation logic is thorough and correct. The error messages use snake_case field names (
data_provider,stream_id,action_name,max_fee) while the TypeScript properties are camelCase. This is likely intentional to match backend field names for easier debugging, but worth confirming consistency across the codebase.
215-219: Excellent fix! True case-insensitive validation now implemented.The previous review comment has been properly addressed. The implementation now normalizes input to lowercase and validates against a lowercase-only whitelist, providing true case-insensitive matching.
465-479: Excellent! Previous review feedback properly addressed.Both test cases for requester validation have been correctly updated to match the new validation logic that requires exactly 20 bytes. The tests now properly check both the "too long" (21 bytes) and "too short" (19 bytes) cases with the correct error message.
395-423: Great test coverage for case-insensitive orderBy validation.The tests thoroughly validate the case-insensitive behavior, including lowercase, uppercase, mixed case, and whitespace trimming. This provides excellent coverage for the implementation at lines 215-219.
2 participants
needs: trufnetwork/kwil-js#143
resolves: https://github.com/trufnetwork/truf-network/issues/1274
Summary by CodeRabbit
New Features
Documentation
Tests
Chores