-
Notifications
You must be signed in to change notification settings - Fork 0
chore: publish attestations #12
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
…lows - Added LICENSE file for Apache License 2.0 compliance. - Updated package.json to reflect new version, public access, and additional scripts for building and testing. - Enhanced README with installation instructions and Solidity/TypeScript usage examples. - Introduced CI workflow for automated testing and release processes. - Added release workflow for version management and npm publishing.
- Updated the copyright year in the LICENSE file from 2024 to 2025. - Modified OffchainLogic test setup to include a conditional skip for CI environments, improving test execution control. - Refactored variable declarations for better clarity and organization in the test file.
WalkthroughAdds CI and Release GitHub Actions workflows, prepares package for public npm publishing (package.json, tsconfig.build), adds Apache-2.0 license, updates README/docs, guards Hardhat task loading behind SKIP_HARDHAT_TASKS, and centralizes test setup with a CI skip guard. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions
participant Runner as Runner (ubuntu-latest / ACT)
participant Setup as pnpm & Node Setup
participant Install as Install deps
participant Build as Build & Compile
participant Test as Test Suite
participant Version as Version Resolver
participant NPM as npm Registry
participant Git as Git Operations
GH->>Runner: trigger (push / pull_request / workflow_dispatch)
Runner->>Setup: checkout + setup pnpm
alt ACT != "true"
Setup->>Setup: setup Node, enable pnpm cache
Setup->>Install: install deps (--frozen-lockfile, cached)
else ACT == "true"
Setup->>Install: install deps (no cache, act path)
end
Install->>Build: compile contracts & build
Build->>Test: run tests (SKIP_HARDHAT_TASKS respected)
Test->>Version: compute release version (input / tag / package)
Version->>Runner: update package.json version
Runner->>NPM: verify version not published
Runner->>NPM: publish package (dist_tag)
NPM-->>Runner: publish result
Runner->>Git: commit bump, create & push tag
Runner->>GH: create GitHub release
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
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 ignored due to path filters (1)
📒 Files selected for processing (1)
💤 Files with no reviewable 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 |
- Added conditional loading of tasks in hardhat.config.cts based on SKIP_HARDHAT_TASKS environment variable. - Updated package.json to reflect new Mocha version and adjusted test script formatting. - Enhanced CI workflows to include conditional steps for local testing with ACT environment variable, ensuring proper dependency installation and task execution. - Added compilation and testing steps in CI workflows with SKIP_HARDHAT_TASKS environment variable for better control over task execution.
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
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)
17-45: Duplicate subtask definitions—only the second takes effect.Two separate
subtask(TASK_COMPILE_SOLIDITY).setAction(...)blocks are defined. Hardhat will only honor the last one, so the first block (writing to artifacts/package.json) is effectively dead code.If both package.json files are required, merge them into a single subtask:
-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.artifacts, "package.json"), + '{ "type": "commonjs" }' + ); await writeFile( join(config.paths.root, "typechain-types", "package.json"), '{ "type": "commonjs" }' ); } catch (error) { console.error("Error writing package.json: ", error); } return superRes; });
🧹 Nitpick comments (4)
test/requestv1/OffchainLogic.test.ts (1)
8-32: Merge duplicate before hooks.The CI skip guard and variable initialization are both appropriate, but having two separate
before()hooks at the same level violates best practices and triggers the static analyzer.Combine them into a single hook:
- before(async function () { - if (process.env.CI) { - this.skip(); - } - }); // Test setup let dataProviderAddress: string; let streamId: string; let secrets: { PRIVATE_KEY: string; }; let source: string; let abiCoder: ethers.AbiCoder; - - - before(async function () { + + before(async function () { + if (process.env.CI) { + this.skip(); + } + dataProviderAddress = "0x4710a8d8f0d845da110086812a32de6d90d7ff5c"; streamId = "stfcfa66a7c2e9061a6fac8b32027ee8"; secrets = { PRIVATE_KEY: getEnv("TN_READER_PRIVATE_KEY") }; source = getSource("requestv1"); abiCoder = ethers.AbiCoder.defaultAbiCoder(); });As per static analysis hints.
.github/workflows/ci.yml (1)
12-12: Use event_name to explicitly guard access to pull_request context.The condition works correctly for both push and pull_request events, but it doesn't follow GitHub Actions best practices. For pull_request events, github.event.pull_request is populated with properties like draft; for push events, it's absent and treated as null. Best practice is to check the event name before using pull_request-specific properties.
The suggested refactor makes intent explicit and avoids null property access:
- if: github.event.pull_request.draft == false || github.event.pull_request == null + if: github.event_name != 'pull_request' || github.event.pull_request.draft == falseThis clearly expresses the logic: run on all push events, or on non-draft pull requests.
.github/workflows/release.yml (2)
127-145: Reconsider|| trueongit addto avoid masking errors.Line 139 uses
|| trueto suppress errors fromgit add, which also hides permission or filesystem failures—not just the benign "nothing to add" case. The subsequentgit diff --cached --quietcheck handles the no-changes case, but errors are still silently swallowed.Consider refactoring to explicitly handle the no-changes case:
- git add package.json pnpm-lock.yaml || true - if git diff --cached --quiet; then + if ! git add package.json pnpm-lock.yaml; then + echo "Warning: git add failed; proceeding anyway" + fi + if git diff --cached --quiet; thenAlternatively, if these files are expected to always exist, remove
|| trueand let failures surface.
170-184: Prerelease flag logic has a redundant condition.Line 179 checks both
!= "latest"and-n "${{ github.event.inputs.dist_tag }}", but since dist_tag defaults to"beta"(line 13), the emptiness check is unnecessary.Simplify to:
- if [[ "${{ github.event.inputs.dist_tag }}" != "latest" && -n "${{ github.event.inputs.dist_tag }}" ]]; then + if [[ "${{ github.event.inputs.dist_tag }}" != "latest" ]]; thenThis is a minor clarity improvement with no behavioral change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.github/workflows/ci.yml(1 hunks).github/workflows/release.yml(1 hunks)LICENSE(1 hunks)README.md(1 hunks)docs/AttestationLibrary.md(3 hunks)hardhat.config.cts(1 hunks)package.json(2 hunks)test/requestv1/OffchainLogic.test.ts(1 hunks)tsconfig.build.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-20T15:59:08.068Z
Learnt from: outerlook
PR: trufnetwork/evm-contracts#9
File: contracts/attestation/TrufAttestation.sol:151-161
Timestamp: 2025-10-20T15:59:08.068Z
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:
docs/AttestationLibrary.md
🧬 Code graph analysis (1)
test/requestv1/OffchainLogic.test.ts (2)
test/helpers/environment.ts (1)
getEnv(5-11)src/getSource.ts (1)
getSource(25-38)
🪛 Biome (2.1.2)
test/requestv1/OffchainLogic.test.ts
[error] 24-32: Disallow duplicate setup and teardown hooks.
Disallow before duplicacy inside the describe function.
(lint/suspicious/noDuplicateTestHooks)
🔇 Additional comments (14)
LICENSE (1)
1-13: LGTM!The Apache License 2.0 is appropriate for this project, and the copyright notice is correctly formatted.
hardhat.config.cts (1)
13-15: LGTM!The conditional task loading based on
SKIP_HARDHAT_TASKSis a good optimization for CI environments where tasks aren't needed.tsconfig.build.json (1)
1-17: LGTM!The build configuration is well-structured for a published library. ES2020 target, declaration output to
dist, and proper exclusions are all appropriate.docs/AttestationLibrary.md (2)
3-3: LGTM!The updated installation guidance with the npm package name is clear and aligns with the new publishing setup.
54-54: LGTM!The import path change from
@trufnetwork/evm-contracts/srcto@trufnetwork/evm-contractscorrectly reflects the package.json exports configuration.README.md (1)
1-46: LGTM!The README restructuring clearly presents the package's primary purpose (attestation verification) while maintaining backward compatibility documentation for legacy Chainlink Functions tooling. Installation instructions and usage examples are helpful.
.github/workflows/ci.yml (1)
46-57: LGTM!The compile, build, and test steps are properly structured. Setting
SKIP_HARDHAT_TASKS=trueduring compilation and testing correctly aligns with the conditional task loading in hardhat.config.cts.package.json (3)
3-10: LGTM!Version bump to 0.1.0-beta, setting
private: false, and adding repository metadata are all appropriate for preparing the package for public npm publishing.
15-16: JSON syntax error: missing comma.Line 15 is missing a trailing comma, which will cause a parse error.
Apply this fix:
- "prepare": "pnpm run build", + "prepare": "pnpm run build", "test": "hardhat test test/attestation/TrufAttestation.test.ts test/attestation/TrufAttestationConsumer.test.ts"Likely an incorrect or invalid review comment.
53-72: Package distribution configuration is correctly structured.The
filesarray includes all necessary artifacts, and themain,module,types, andexportsentries properly reference the built output. Thetsconfig.build.jsonis configured with"outDir": "./dist"and"declaration": true, ensuring thatdist/index.jsanddist/index.d.tswill be generated during the build process. Thepreparescript automatically runs the build before publishing, so the distribution artifacts will be present at publish time..github/workflows/release.yml (4)
1-71: Setup and configuration look solid.The workflow trigger, permissions, concurrency handling, and Node.js setup are well-structured. The ACT-conditional paths (for local
acttesting) maintain consistency with your CI workflow patterns. The explicit version pins (pnpm v9, Node v22) are good practice.
72-98: Version computation logic is comprehensive.The multi-branch logic (explicit version → dist_tag channel → auto-patch) covers release scenarios well. The pre-release format (
v{BASE_VERSION}-{CHANNEL}.{SHORT_SHA}, e.g.,v0.1.0-beta.abc1234) is valid semver, though note that dots in pre-release identifiers may interact unexpectedly with some semver tooling. If this aligns with your project's versioning conventions, no action needed.Please confirm this pre-release format aligns with your project's versioning strategy and any downstream consumer expectations.
106-126: Verify npm version supports--provenanceflag.Line 125 uses
--provenancewhich requires npm ≥10.8.3. While Node v22 typically includes a recent npm, ubuntu-latest is a moving target and may not guarantee this version. Consider either:
- Pinning npm version in the setup step, or
- Testing in the
actenvironment to confirm availability.Please verify or add an npm version check to ensure
--provenancesupport before publishing.
147-168: Git tag and release safeguards are well‑implemented.The checks to ensure tags don't already exist (lines 147–160) and the clean tag/push logic (lines 162–168) are solid. The GitHub release creation with conditional prerelease flag (lines 170–184) is appropriate.
Time Submission Status
|
- Removed redundant error handling in Hardhat configuration for package.json writing. - Streamlined the OffchainLogic test setup by relocating the CI skip condition to the main before hook, improving clarity and organization.
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
|
The CI is failing will you fix it here, or be fixed in another issue? @outerlook |
- Removed pnpm-lock.yaml from .gitignore to allow tracking of pnpm lock file. - Added pnpm-lock.yaml to the repository to manage package dependencies consistently with pnpm.
|
thanks for spotting, @MicBun, now fixed |
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
Chores