feat: attestations #9
Conversation
- Added TrufAttestation.sol library to parse and validate TrufNetwork attestations, including structured representation and error handling. - Introduced TrufAttestationHarness.sol for testing the library's functionalities. - Updated documentation to include the Attestation Library guide, detailing usage and payload structure. - Enhanced testing framework with comprehensive unit tests for the TrufAttestation library, ensuring robust validation and verification processes. - Added helper functions in TypeScript for building canonical attestations and signatures, facilitating easier integration in consumer contracts.
WalkthroughAdds a Solidity TrufAttestation library and harness for parsing, hashing, verifying, and decoding canonical attestations; TypeScript canonical builders and test helpers; comprehensive tests and a golden fixture; new documentation and README updates; re-exports in the package API and conditional Sepolia handling in Hardhat config. Changes
Sequence Diagram(s)sequenceDiagram
participant TS as TypeScript Builder
participant Dev as Developer
participant Consumer as Consumer Contract
participant Lib as TrufAttestation (Solidity)
participant ECDSA as ecrecover
Dev->>TS: buildSignedAttestation(fields, signature)
TS->>TS: validate lengths, encode canonical, append signature
TS-->>Dev: payload
Dev->>Consumer: submit(payload)
Consumer->>Lib: parse(payload)
Lib->>Lib: validate version/algorithm, split signature, decode fields
Lib-->>Consumer: Attestation struct
Consumer->>Lib: verify(att, expectedValidator)
Lib->>Lib: sha256(canonical fields)
Lib->>ECDSA: ecrecover(hash, v,r,s)
ECDSA-->>Lib: recovered signer
Lib-->>Consumer: verification result
Consumer->>Lib: decodeDataPoints(att)
Lib->>Lib: abi.decode(result) -> (timestamps[], values[])
Lib-->>Consumer: DataPoint[]
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly linked issues (from PR-linked list)
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hardhat.config.cts (1)
15-44: Only the last TASK_COMPILE_SOLIDITY override runs; first one is shadowed.Calling setAction twice on the same subtask replaces the previous action; the artifacts/package.json write never executes. Merge both writes into a single override.
Apply:
-subtask(TASK_COMPILE_SOLIDITY).setAction(async (_, { config }, runSuper) => { - const superRes = await runSuper(); - - try { - await writeFile( - join(config.paths.artifacts, "package.json"), - '{ "type": "commonjs" }' - ); - } catch (error) { - console.error("Error writing package.json: ", error); - } - - return superRes; -}); - -subtask(TASK_COMPILE_SOLIDITY).setAction(async (_, { config }, runSuper) => { - const superRes = await runSuper(); - - try { - await writeFile( - join(config.paths.root, "typechain-types", "package.json"), - '{ "type": "commonjs" }' - ); - } catch (error) { - console.error("Error writing package.json: ", error); - } - - return superRes; -}); +subtask(TASK_COMPILE_SOLIDITY).setAction(async (args, { config }, runSuper) => { + const superRes = await runSuper(args); + const writes = [ + writeFile(join(config.paths.artifacts, "package.json"), '{ "type": "commonjs" }'), + writeFile(join(config.paths.root, "typechain-types", "package.json"), '{ "type": "commonjs" }'), + ]; + for (const p of writes) { + try { await p; } catch (error) { + console.error("Error writing package.json: ", error); + } + } + return superRes; +});
🧹 Nitpick comments (18)
hardhat.config.cts (1)
70-75: Omit undefined accounts property; guard it via conditional spread.Avoid passing accounts: undefined; let Hardhat default when no PRIVATE_KEY.
Apply:
- ...(ETHEREUM_SEPOLIA_RPC_URL && { - sepolia: { - url: ETHEREUM_SEPOLIA_RPC_URL, - accounts: PRIVATE_KEY ? [PRIVATE_KEY] : undefined, - }, - }), + ...(ETHEREUM_SEPOLIA_RPC_URL ? { + sepolia: { + url: ETHEREUM_SEPOLIA_RPC_URL, + ...(PRIVATE_KEY ? { accounts: [PRIVATE_KEY] } : {}), + }, + } : {}),README.md (1)
82-84: Nice cross-link. Consider pinning the canonical.go reference.Optional: link to a specific commit SHA to avoid docs drifting if the node repo changes file layout.
docs/DeveloperGuide.md (1)
146-146: Minor: consider linking to a sample script/CLI for EOA interactions.Optional: a one-liner with cast/etherscan tx example improves dev UX.
docs/AttestationLibrary.md (2)
49-55: Import path: prefer package root over “/src” for consumers.Recommend
import { … } from "@trufnetwork/evm-contracts"(or whatever the package export is) to decouple docs from repo layout.
19-31: Helpful layout table. Add explicit note that the hash is sha256 (not keccak256).Avoids common confusion for EVM devs expecting keccak256. A one-liner here would help.
test/attestation/TrufAttestation.test.ts (1)
162-189: Good negative case for unsupported algorithm. Add two more negatives to match spec.Add tests for:
- decodeDataPoints: mismatched array lengths must revert.
- parse: truncated payload should revert with invalid length.
Apply near the end of the file:
@@ it("reverts verification for unsupported algorithm", async function () { @@ }); + + it("reverts when result arrays have mismatched lengths", async function () { + const harness = await (await ethers.getContractFactory("TrufAttestationHarness")).deploy(); + const wallet = ethers.Wallet.createRandom(); + const badResult = ethers.AbiCoder.defaultAbiCoder().encode( + ["uint256[]", "int256[]"], + [[1n, 2n], [0n]] // lengths differ + ); + const fields: CanonicalFields = { + version: 1, + algorithm: 0, + blockHeight: 1n, + dataProvider: wallet.address, + streamId: ethers.hexlify(ethers.randomBytes(32)), + actionId: 1, + args: ethers.getBytes("0x"), + result: ethers.getBytes(badResult), + }; + const canonical = buildCanonical(fields); + const digest = ethers.sha256(canonical); + const sig = ethers.Signature + .from(wallet.signingKey.sign(ethers.getBytes(digest))).serialized; + const payload = buildPayload(fields, ethers.getBytes(sig)); + await expect(harness.decodeDataPoints(payload)).to.be.reverted; // consider asserting custom error + }); + + it("reverts on truncated payload (invalid length)", async function () { + const harness = await (await ethers.getContractFactory("TrufAttestationHarness")).deploy(); + const bytes = ethers.getBytes(GOLDEN_PAYLOAD); + const truncated = ethers.hexlify(bytes.slice(0, bytes.length - 1)); + await expect(harness.parse(truncated)).to.be.reverted; // consider asserting AttestationInvalidLength + });If there is a specific custom error for the length mismatch, please confirm its name from contracts/attestation/TrufAttestation.sol and assert it here.
test/helpers/attestation.ts (3)
29-35: Add explicit range validation for version/algorithm/actionId to prevent silent truncation.Masking with
& 0xfftruncates out-of-range values without warning. Validate ranges and fail fast (and enforce actionId 1..255 to match the Solidity library’s expectations).Apply this diff inside
buildCanonicalbefore assemblingpieces:const stream = ethers.getBytes(fields.streamId); if (stream.length !== 32) { throw new Error("streamId must be 32 bytes"); } + if (!Number.isInteger(fields.version) || fields.version < 0 || fields.version > 0xff) { + throw new Error("version must be in [0,255]"); + } + if (!Number.isInteger(fields.algorithm) || fields.algorithm < 0 || fields.algorithm > 0xff) { + throw new Error("algorithm must be in [0,255]"); + } + if (!Number.isInteger(fields.actionId) || fields.actionId < 1 || fields.actionId > 0xff) { + throw new Error("actionId must be in [1,255]"); + }
50-60: Validate integer ranges before encoding (avoid negative/overflow masking).Currently values are bit-masked; negative inputs or out-of-range values will silently wrap.
function encodeUint16BE(value: number): Buffer { - const buffer = Buffer.alloc(2); - buffer.writeUInt16BE(value & 0xffff, 0); + if (!Number.isInteger(value) || value < 0 || value > 0xffff) { + throw new Error("uint16 out of range"); + } + const buffer = Buffer.alloc(2); + buffer.writeUInt16BE(value, 0); return buffer; } function encodeUint64BE(value: bigint): Buffer { - const buffer = Buffer.alloc(8); - buffer.writeBigUInt64BE(value & BigInt("0xffffffffffffffff"), 0); + if (value < 0n || value > 0xffffffffffffffffn) { + throw new Error("uint64 out of range"); + } + const buffer = Buffer.alloc(8); + buffer.writeBigUInt64BE(value, 0); return buffer; }
42-48: Normalize signature v (0/1 → 27/28) for OZ ECDSA compatibility.OpenZeppelin’s
ECDSA.recovertypically expectsv∈ {27,28}. Normalizing improves interop with signers that emit 0/1.export function buildPayload(fields: CanonicalFields, signature: Uint8Array): Uint8Array { if (signature.length !== 65) { throw new Error("signature must be 65 bytes"); } - const canonical = buildCanonical(fields); - return Buffer.concat([Buffer.from(canonical), Buffer.from(signature)]); + const canonical = buildCanonical(fields); + const sig = Buffer.from(signature); + if (sig[64] === 0 || sig[64] === 1) sig[64] = sig[64] + 27; // normalize v + return Buffer.concat([Buffer.from(canonical), sig]); }If your signer already outputs 27/28 this is a no-op. Please confirm your signer behavior in tests.
src/attestation.ts (5)
31-38: Guard against silent truncation; enforce actionId domain.Add range checks for
version,algorithm(0..255) andactionId(1..255) before encoding.export function buildCanonicalAttestation(fields: CanonicalAttestationFields): Uint8Array { const provider = ethers.getBytes(fields.dataProvider); @@ const args = Uint8Array.from(fields.args); const result = Uint8Array.from(fields.result); + + if (!Number.isInteger(fields.version) || fields.version < 0 || fields.version > 0xff) { + throw new Error("version must be in [0,255]"); + } + if (!Number.isInteger(fields.algorithm) || fields.algorithm < 0 || fields.algorithm > 0xff) { + throw new Error("algorithm must be in [0,255]"); + } + if (!Number.isInteger(fields.actionId) || fields.actionId < 1 || fields.actionId > 0xff) { + throw new Error("actionId must be in [1,255]"); + }
56-66: Validate integer ranges in encoders.Match on-chain expectations and fail fast on invalid inputs.
function encodeUint16BE(value: number): Buffer { - const buffer = Buffer.alloc(2); - buffer.writeUInt16BE(value & 0xffff, 0); + if (!Number.isInteger(value) || value < 0 || value > 0xffff) { + throw new Error("uint16 out of range"); + } + const buffer = Buffer.alloc(2); + buffer.writeUInt16BE(value, 0); return buffer; } function encodeUint64BE(value: bigint): Buffer { - const buffer = Buffer.alloc(8); - buffer.writeBigUInt64BE(value & BigInt("0xffffffffffffffff"), 0); + if (value < 0n || value > 0xffffffffffffffffn) { + throw new Error("uint64 out of range"); + } + const buffer = Buffer.alloc(8); + buffer.writeBigUInt64BE(value, 0); return buffer; }
48-54: Normalize signature v (0/1 → 27/28) for on-chain recover.Prevents verification failures when signers output 0/1.
export function buildSignedAttestation(fields: CanonicalAttestationFields, signature: Uint8Array): Uint8Array { if (signature.length !== 65) { throw new Error("signature must be 65 bytes"); } const canonical = buildCanonicalAttestation(fields); - return Buffer.concat([Buffer.from(canonical), Buffer.from(signature)]); + const sig = Buffer.from(signature); + if (sig[64] === 0 || sig[64] === 1) sig[64] = sig[64] + 27; // normalize v + return Buffer.concat([Buffer.from(canonical), sig]); }If you prefer not to mutate inputs, copy and normalize into a new buffer (as above).
17-18: Deduplicate with tests to avoid drift.
test/helpers/attestation.tsre-implements the same logic. Consider importing these exported helpers in tests instead of duplicating them.
31-42: Optional: Prefer Uint8Array-only assembly for browser portability.Using
Bufferties this to Node; if you expect browser use, switch toUint8Arrayconcatenation (e.g., precompute total length, allocate once, andsetslices).contracts/attestation/TrufAttestationHarness.sol (1)
7-9: Remove unusedusingstatements.
using TrufAttestation for bytes;andfor TrufAttestation.Attestation;aren’t used in this harness.contracts/attestation/TrufAttestation.sol (3)
76-85: Add a minimal header-length precheck to fail fast.Before copying, you can reject payloads whose canonical portion is smaller than the smallest valid header (no args/result): 80 bytes canonical + 65 signature = 145 bytes total.
function parse(bytes calldata payload) internal pure returns (Attestation memory att) { - if (payload.length <= SIGNATURE_LENGTH) revert AttestationInvalidLength(); + if (payload.length <= SIGNATURE_LENGTH) revert AttestationInvalidLength(); + unchecked { + // 1(ver)+1(alg)+8(height)+ (4+20 provider)+ (4+32 stream)+2(action)+4(len0)+4(len0) = 80 + if (payload.length < SIGNATURE_LENGTH + 80) revert AttestationInvalidLength(); + }
126-133: Optional: Normalize v (0/1 → 27/28) before recover to improve interop.Some signers emit 0/1. Normalizing avoids false negatives without weakening security.
function verify(Attestation memory att, address expectedValidator) internal pure returns (bool) { if (att.signature.length != SIGNATURE_LENGTH) revert AttestationInvalidSignatureLength(); if (att.algorithm != ALGORITHM_SECP256K1) revert AttestationInvalidAlgorithm(att.algorithm); bytes32 digest = hash(att); - address recovered = ECDSA.recover(digest, att.signature); + bytes memory sig = att.signature; + // If v is 0/1, normalize to 27/28 in-place + assembly { + let v := byte(0, mload(add(add(sig, 0x20), 64))) + if lt(v, 27) { mstore8(add(add(sig, 0x20), 64), add(v, 27)) } + } + address recovered = ECDSA.recover(digest, sig); return recovered == expectedValidator; }Please confirm your signing flow’s v semantics to justify adding this normalization.
228-245: Gas: replace byte-by-byte copy with a word copy in_readLengthPrefixed.The loop copies one byte at a time. A 32-byte word copy (with a small tail handler) reduces gas for large blobs.
I can provide an assembly
memcpyvariant if you want to prioritize this; kept out here to avoid footguns in memory bounds handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
README.md(2 hunks)contracts/attestation/TrufAttestation.sol(1 hunks)contracts/attestation/TrufAttestationHarness.sol(1 hunks)docs/AttestationLibrary.md(1 hunks)docs/DeveloperGuide.md(2 hunks)hardhat.config.cts(3 hunks)src/attestation.ts(1 hunks)src/index.ts(1 hunks)test/attestation/TrufAttestation.test.ts(1 hunks)test/attestation/fixtures/attestation_golden.json(1 hunks)test/helpers/attestation.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
test/attestation/TrufAttestation.test.ts (1)
test/helpers/attestation.ts (3)
CanonicalFields(3-12)buildCanonical(14-40)buildPayload(42-48)
🔇 Additional comments (11)
hardhat.config.cts (1)
45-46: Sanity-check PRIVATE_KEY format before use.Ensure it’s a 0x-prefixed 32‑byte hex string; otherwise Hardhat will fail at runtime.
You can add a runtime assert:
if (PRIVATE_KEY && !/^0x[0-9a-fA-F]{64}$/.test(PRIVATE_KEY)) { throw new Error("Invalid PRIVATE_KEY format; expected 0x + 64 hex chars"); }README.md (1)
5-5: Good addition—concise context for the new attestation library.docs/DeveloperGuide.md (1)
67-70: Clear, actionable addition.Section concisely explains when/how to use the library and where to look next.
test/attestation/fixtures/attestation_golden.json (1)
1-21: Golden fixture is well-formed and aligns with tests.test/attestation/TrufAttestation.test.ts (2)
37-105: End-to-end happy-path looks solid.
107-160: Golden-path checks are thorough.src/index.ts (1)
1-2: This review comment contradicts the package configuration and should be disregarded.The original review assumes "@trufnetwork/evm-contracts" is a public npm package that consumers will import from. However, package.json explicitly declares
"private": true, which prevents publication to npm registries. Additionally, there are no build scripts, no exports/main/types configuration, and no compiled output directory—indicating this package is not currently set up for external distribution.If the package is genuinely meant to be private (development-only or internal), the review comment is incorrect. If it should be public, that requires separate changes: marking
"private": false, adding build scripts, configuring exports/main/types in package.json, and generating compiled outputs.Likely an incorrect or invalid review comment.
contracts/attestation/TrufAttestationHarness.sol (1)
14-44: LGTM on wrapper coverage and signatures.Thin, pure wrappers for parse/hash/verify/decodeDataPoints/metadata/body/toAction align with the library API.
contracts/attestation/TrufAttestation.sol (2)
211-221: Canonical encoding matches TS (BE, length-prefixes).Bytes layout (1+1+8, 4+20, 4+32, 2, 4+len(args), 4+len(result)) and big-endian ordering align with helpers.
275-283: Action bounds are well-handled.
_normalizeActionIdand_requireKnownActionIdproperly reject 0 and out-of-range values.test/helpers/attestation.ts (1)
1-1: Review comment is incorrect—no version mismatch exists.Both src and tests use ethers v6. The package versions show ethers v6.13.4 in
dependenciesand@nomicfoundation/hardhat-ethersv^3.0.8 indevDependencies. @nomicfoundation/hardhat-ethers v3 is compatible with ethers v6, so tests importingethersfrom Hardhat receive the same v6 library as src code. The claimed v5 vs v6 mismatch does not exist, andgetBytes(a v6 function) will work correctly.Likely an incorrect or invalid review comment.
Time Submission Status
|
- Replaced the dotenv import position and updated the syntax for setting the BCRYPTO_FORCE_FALLBACK environment variable to use the nullish coalescing assignment operator. - Streamlined the configuration file for better readability and maintainability.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
docs/AttestationLibrary.md (1)
1-62: LGTM: Comprehensive attestation library documentation.The documentation provides clear usage guidance, payload structure, and best practices. The Solidity quickstart and TypeScript helpers sections are particularly helpful for developers.
Optional enhancement: Consider clarifying the signature format in the table at line 29:
-| `signature (65 bytes)` | Validator secp256k1 signature. | +| `signature (65 bytes)` | Validator secp256k1 signature (32-byte r + 32-byte s + 1-byte v). |This would help developers understand the signature structure when working with raw bytes.
contracts/attestation/TrufAttestationHarness.sol (1)
7-9: Remove unusedusingdirectivesThe contract never calls extension methods (
payload.parse(),att.toAction()). Drop these to avoid warnings.- using TrufAttestation for bytes; - using TrufAttestation for TrufAttestation.Attestation; +contracts/attestation/TrufAttestation.sol (4)
126-133: Prefer non‑reverting verification viaECDSA.tryRecover
ECDSA.recoverreverts on malformed signatures, turning a boolean check into a hard revert. UsingtryRecoverkeepsverifytotal, returningfalseon bad sigs while still guarding length/algorithm.function verify(Attestation memory att, address expectedValidator) internal pure returns (bool) { if (att.signature.length != SIGNATURE_LENGTH) revert AttestationInvalidSignatureLength(); if (att.algorithm != ALGORITHM_SECP256K1) revert AttestationInvalidAlgorithm(att.algorithm); bytes32 digest = hash(att); - address recovered = ECDSA.recover(digest, att.signature); - return recovered == expectedValidator; + (address recovered, ECDSA.RecoverError err) = ECDSA.tryRecover(digest, att.signature); + if (err != ECDSA.RecoverError.NoError) { + return false; + } + return recovered == expectedValidator; }
75-85: Avoid copying the entire canonical blob to memory (optional perf)You copy
payload[:len-65]intobytesmemory, then parse. With ^0.8.27, you can slice calldata to work zero‑copy and only allocate small chunks as needed (or refactor helpers to read from calldata).- uint256 canonicalLength = payload.length - SIGNATURE_LENGTH; - bytes memory canonical = new bytes(canonicalLength); - bytes memory signature = new bytes(SIGNATURE_LENGTH); - assembly { - calldatacopy(add(canonical, 0x20), payload.offset, canonicalLength) - calldatacopy(add(signature, 0x20), add(payload.offset, canonicalLength), SIGNATURE_LENGTH) - } + uint256 canonicalLength = payload.length - SIGNATURE_LENGTH; + bytes calldata canonical = payload[:canonicalLength]; + bytes calldata signature = payload[canonicalLength:]; + // If adopting this, adjust downstream readers to accept calldata where possible.Please confirm calldata slicing availability in your compiler settings.
228-246: Use word-sized copies in_readLengthPrefixed(optional micro‑opt)Byte‑by‑byte loop is gas‑heavy. A simple 32‑byte chunked copy in assembly reduces gas for large blobs. Keep the bounds checks as-is.
- chunk = new bytes(length); - for (uint256 i; i < length; ) { - chunk[i] = data[offset + i]; - unchecked { - ++i; - } - } + chunk = new bytes(length); + assembly { + let src := add(add(data, 0x20), offset) + let dst := add(chunk, 0x20) + // copy full words + for { let end := add(src, and(add(length, 31), not(31))) } lt(src, end) { src := add(src, 32) dst := add(dst, 32) } { + mstore(dst, mload(src)) + } + // copy tail bytes + let rem := and(length, 31) + if gt(rem, 0) { + // mask = 0xffff.. << (32 - rem)*8 + let mask := shl(mul(sub(32, rem), 8), not(0)) + let srcw := mload(src) + let dstw := mload(dst) + mstore(dst, or(and(srcw, not(mask)), and(dstw, mask))) + } + }
200-209: Parsing allows unknownactionId;toActionenforces range (OK, just noting)Current behavior accepts any 1..255 at parse and only enforces enum bounds when converting to
Action. This is fine if callers always usetoActionbefore branching. If you prefer earlier fail-fast, move_requireKnownActionIdintoparse.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
README.md(2 hunks)contracts/attestation/TrufAttestation.sol(1 hunks)contracts/attestation/TrufAttestationHarness.sol(1 hunks)docs/AttestationLibrary.md(1 hunks)docs/DeveloperGuide.md(2 hunks)hardhat.config.cts(3 hunks)src/attestation.ts(1 hunks)src/index.ts(1 hunks)test/attestation/TrufAttestation.test.ts(1 hunks)test/attestation/fixtures/attestation_golden.json(1 hunks)test/helpers/attestation.ts(1 hunks)
🔇 Additional comments (15)
hardhat.config.cts (2)
1-2: LGTM! Previous critical issue correctly addressed.The dotenv import is now at the top before any environment variable access, and the nullish coalescing assignment operator (
??=) correctly allows.envvalues to take precedence while providing a default.
70-75: Verify the intended behavior when PRIVATE_KEY is not set.When
ETHEREUM_SEPOLIA_RPC_URLis defined butPRIVATE_KEYis not, Hardhat will use default test accounts (which have no funds on Sepolia). This will cause deployment or transaction attempts to fail with "insufficient funds" errors, which may confuse developers.Is this intentional to support read-only operations, or should the config warn/error when
ETHEREUM_SEPOLIA_RPC_URLis set withoutPRIVATE_KEY?src/attestation.ts (2)
1-12: LGTM: Clean type definition.The type definition correctly models the canonical attestation fields as specified in the documentation, with appropriate types for each field.
48-72: LGTM: Helper functions correctly implement encoding.The signature validation, endianness handling, and length-prefixing logic are all correct. The 65-byte signature format aligns with ECDSA secp256k1 signatures (r + s + v).
README.md (2)
5-5: LGTM: Clear description update.The updated description appropriately mentions the attestation verification library alongside existing functionality.
82-84: LGTM: Well-structured attestation library documentation.The new section provides a clear overview with appropriate cross-references to the detailed guide and implementation.
docs/DeveloperGuide.md (1)
67-69: LGTM: Clear integration documentation.The new subsection appropriately documents the attestation library integration points and references the detailed guide.
test/attestation/fixtures/attestation_golden.json (1)
1-21: LGTM: Well-structured golden fixture.The fixture provides comprehensive deterministic test data with all necessary fields for validating parsing, hashing, verification, and decoding logic.
test/attestation/TrufAttestation.test.ts (4)
1-31: LGTM: Clean test setup with golden fixture.The test file properly loads the golden fixture and extracts constants for deterministic testing.
38-105: LGTM: Comprehensive end-to-end test.This test validates the complete attestation flow:
- Canonical payload construction
- Signing with a random wallet
- Parsing and field validation
- Hash computation
- Signature verification (both positive and negative cases)
- Data point decoding
- Metadata and body extraction
The test coverage is thorough and assertions are appropriate.
107-160: LGTM: Thorough golden fixture validation.This test validates parsing, verification, and decoding against a deterministic golden fixture, ensuring:
- Correct field extraction
- Action ID resolution including the edge case for unsupported IDs
- Data point decoding with proper decimal handling
- Signature verification
- Canonical and signature byte extraction
The assertions against the golden values provide strong regression protection.
162-189: LGTM: Proper error handling test.The test correctly validates that unsupported algorithms (algorithm: 2) trigger the
AttestationInvalidAlgorithmerror during verification, ensuring the library fails fast for unsupported signature schemes.src/index.ts (1)
1-2: No action required—the "./lib" module exists as src/lib.ts.The verification confirms that
src/lib.tsexists and is correctly referenced by the export statement insrc/index.ts. The module is present in the codebase as expected.Likely an incorrect or invalid review comment.
contracts/attestation/TrufAttestationHarness.sol (1)
10-44: Harness surface looks goodThin, pure wrappers are appropriate for testing the library behavior. No issues spotted.
contracts/attestation/TrufAttestation.sol (1)
11-17: Constants, enums, and custom errors read cleanlyGood coverage of invariants (version, algorithm, lengths) and action surfacing. Names and visibility are consistent.
Also applies to: 28-34, 35-51
… src/attestation - Replaced the existing implementation of canonical attestation building and payload creation with new functions from the src/attestation module. - Updated type definitions to align with the new structure, enhancing code clarity and maintainability.
Description
Related Problem
How Has This Been Tested?
Summary by CodeRabbit
New Features
Documentation
Tests