-
Notifications
You must be signed in to change notification settings - Fork 0
chore: include attestation docs & example #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Introduced TrufAttestationConsumer.sol contract for verifying and storing TrufNetwork attestations. - Added comprehensive tests in TrufAttestationConsumer.test.ts to validate attestation consumption and error handling. - Updated package.json to include a test script for the new consumer contract. - Enhanced documentation to include details about the new contract and its usage.
WalkthroughThis PR introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Consumer as TrufAttestationConsumer
participant TrufAttest as TrufAttestation (lib)
participant State as Contract State
User->>Consumer: consume(payload)
Note over Consumer: Validate leader is set
Consumer->>TrufAttest: parse(payload)
TrufAttest-->>Consumer: Attestation struct
Note over Consumer: Extract signer, data
alt Signer matches leader
Consumer->>TrufAttest: decodeDataPoints(data)
TrufAttest-->>Consumer: DataPoint[]
alt Array not empty
Consumer->>State: Update lastValidator, lastBlockHeight,<br/>lastStreamId, lastActionId,<br/>lastTimestamp, lastValue
Consumer->>Consumer: emit AttestationConsumed
Consumer-->>User: Success
else Array empty
Consumer-->>User: revert AttestationConsumerEmptyResult
end
else Signer mismatch
Consumer-->>User: revert AttestationConsumerInvalidSigner
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The PR spans multiple file types with consistent, well-structured patterns (new contract, test suites, helpers, docs). The TrufAttestationConsumer contract is straightforward with clear ownership and access control logic. Test coverage is comprehensive but follows a repetitive structure across test files. Documentation and configuration changes are minimal. The main review burden lies in validating the consumer contract's integration with the TrufAttestation library and ensuring test assertions correctly exercise all specified behaviors. Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
package.json (1)
8-8: Consider using a glob pattern for better maintainability.The explicit file listing works, but as more attestation tests are added, you'll need to update this script each time. Consider using a pattern like
"hardhat test test/attestation/**/*.test.ts"for automatic inclusion of new tests.Apply this diff if you prefer the glob approach:
- "test": "hardhat test test/attestation/TrufAttestation.test.ts test/attestation/TrufAttestationConsumer.test.ts" + "test": "hardhat test test/attestation/**/*.test.ts"test/attestation/TrufAttestation.test.ts (1)
165-169: Test name could be more precise.The test truncates the payload by 130 hex characters (65 bytes), which removes the entire signature, leaving only canonical bytes. The test name says "truncated canonical bytes" but it's actually testing a payload missing the signature entirely. Consider renaming to "rejects payloads missing signature" for clarity.
Apply this diff for a more accurate test name:
- it("rejects payloads with truncated canonical bytes", async function () { + it("rejects payloads missing signature", async function () { const harness = await deployHarness(); const truncated = GOLDEN_PAYLOAD.slice(0, GOLDEN_PAYLOAD.length - 130); await expect(harness.parse(truncated)).to.be.revertedWithCustomError(harness, "AttestationInvalidLength");contracts/attestation/TrufAttestationConsumer.sol (1)
41-46: Consider adding validation for production use.While this is an example contract, you might want to demonstrate best practices:
function setLeader(address newLeader) external { if (msg.sender != owner) revert AttestationConsumerOnlyOwner(); + if (newLeader == address(0)) revert AttestationConsumerInvalidLeader(); + if (newLeader == leader) return; // Skip if unchanged leader = newLeader; emit LeaderUpdated(newLeader); }Note: Allowing
address(0)might be intentional to "unset" the leader, so consider your use case.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
contracts/attestation/TrufAttestationConsumer.sol(1 hunks)docs/AttestationLibrary.md(1 hunks)package.json(1 hunks)test/attestation/TrufAttestation.test.ts(3 hunks)test/attestation/TrufAttestationConsumer.test.ts(1 hunks)test/attestation/golden.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: outerlook
PR: trufnetwork/evm-contracts#9
File: contracts/attestation/TrufAttestation.sol:151-161
Timestamp: 2025-10-20T15:59:08.048Z
Learning: The TrufAttestation library in contracts/attestation/TrufAttestation.sol intentionally does not impose upper bounds on decoded datapoints or other business logic constraints, leaving such validation to consumer contracts that have specific use-case requirements.
📚 Learning: 2025-10-20T15:59:08.048Z
Learnt from: outerlook
PR: trufnetwork/evm-contracts#9
File: contracts/attestation/TrufAttestation.sol:151-161
Timestamp: 2025-10-20T15:59:08.048Z
Learning: The TrufAttestation library in contracts/attestation/TrufAttestation.sol intentionally does not impose upper bounds on decoded datapoints or other business logic constraints, leaving such validation to consumer contracts that have specific use-case requirements.
Applied to files:
contracts/attestation/TrufAttestationConsumer.soldocs/AttestationLibrary.md
🧬 Code graph analysis (2)
test/attestation/TrufAttestationConsumer.test.ts (1)
test/attestation/golden.ts (2)
goldenFixture(28-28)GOLDEN_PAYLOAD(31-31)
test/attestation/TrufAttestation.test.ts (1)
test/attestation/golden.ts (2)
GOLDEN_PAYLOAD(31-31)goldenFixture(28-28)
🔇 Additional comments (19)
docs/AttestationLibrary.md (1)
64-67: LGTM! Clear documentation with appropriate production warnings.The example contract section provides helpful guidance and correctly references the new consumer contract and its test suite. The warning to replace leader management before mainnet deployment is important and well-placed.
test/attestation/golden.ts (3)
6-26: LGTM! Well-structured fixture type.The
GoldenFixturetype provides clear structure for the test fixture data and aligns with the canonical attestation format.
28-31: LGTM! Clean fixture export pattern.The golden fixture constants are properly formatted with 0x prefixes for use with ethers.js, and the synchronous file read is appropriate for test setup.
4-4: The fixture file exists at the expected path. No issues found.test/attestation/TrufAttestation.test.ts (5)
4-9: LGTM! Good refactor with centralized fixtures and deployment helper.The
deployHarnesshelper and golden fixture imports reduce duplication and improve test maintainability.
82-82: LGTM! Consistent use of deployment helper.
171-180: LGTM! Good edge case coverage.This test properly validates that truncated signatures are rejected, addressing the malformed payload requirement from issue #7.
182-191: LGTM! Proper verification of signature validation.This test confirms that tampered payloads return false from
verify()rather than reverting, which correctly validates the library's behavior for mismatched signatures as required by issue #7.
193-229: LGTM! Excellent coverage of varying array sizes.This loop-based test provides fuzz-like coverage for different array sizes including the important edge case of empty results, directly addressing the requirement from issue #7 for varying array size tests.
test/attestation/TrufAttestationConsumer.test.ts (5)
7-38: LGTM! Comprehensive happy path test.This test thoroughly validates successful attestation consumption by checking both event emission and state variable updates. The use of golden fixtures ensures consistent test data.
40-54: LGTM! Proper validation of untrusted signers.This test confirms that the consumer correctly rejects attestations not signed by the trusted leader.
56-83: LGTM! Important edge case for consumer business logic.This test validates that the consumer contract enforces non-empty results, which is appropriate business logic at the consumer level rather than in the library. Based on learnings.
85-92: LGTM! Proper access control validation.
94-100: LGTM! Validates prerequisite for consumption.This test ensures that the consumer requires explicit leader configuration before accepting attestations.
contracts/attestation/TrufAttestationConsumer.sol (5)
1-10: LGTM! Clean contract structure and imports.
12-24: LGTM! Appropriate error definitions and state variables.The custom errors and state variables are well-designed for tracking attestation metadata and the latest datapoint.
26-39: LGTM! Well-designed events and simple initialization.The events provide good observability with appropriate indexed parameters, and the constructor correctly establishes ownership.
48-58: LGTM! Proper validation flow.The function correctly validates prerequisites, parses the attestation, and verifies the signature before processing.
60-73: LGTM! Correct datapoint processing and state updates.The function properly validates non-empty results, extracts the latest datapoint, updates state atomically, and emits the event. Note that replay protection and freshness checks are intentionally omitted as this is a minimal example—as documented, production implementations should add these safeguards.
Time Submission Status
|
MicBun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Description
Related Problem
How Has This Been Tested?
Summary by CodeRabbit
New Features
Documentation